test: edge cases of the Users, User Groups and Organisations admin controllers
- each controller driven directly on records of another type, an application that is not booted, one without a database and one whose database fails: a guard that cannot read answers a plain error, never a pass - D-30: a superuser passes the takeover guard; an unreadable membership is the opaque 500 with nothing changed - unknown partials, undeclared filters and sorts, options of fields that have none, search and sort on the three lists - a name-only update of a privileged group, validation before the guard, a seeded group protected through the config list - known gaps pinned: boolean values in a group's permissions, organisation_role after an unlink
This commit is contained in:
Jakub Zych
@@ -1,11 +1,27 @@
|
||||
package user
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"git.golem15.com/golem15/sm-user-plugin/controllers"
|
||||
"git.golem15.com/golem15/sm-user-plugin/models"
|
||||
"git.golem15.com/golem15/summercms/modules/backpack"
|
||||
"git.golem15.com/golem15/summercms/modules/bouncer"
|
||||
"git.golem15.com/golem15/summercms/modules/cabana"
|
||||
"git.golem15.com/golem15/summercms/modules/compass"
|
||||
"git.golem15.com/golem15/summercms/modules/lagoon"
|
||||
"git.golem15.com/golem15/summercms/modules/pact"
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
// TestAdminUsersGroupFilterValue (D-23): the groups filter takes a group id.
|
||||
@@ -45,5 +61,369 @@ func TestAdminUsersGroupFilterValue(t *testing.T) {
|
||||
if page := env.users(t, token, "search="+marker); page.Total != 2 {
|
||||
t.Fatalf("unfiltered list = %s", page.Raw)
|
||||
}
|
||||
_ = models.FilterByGroup
|
||||
}
|
||||
|
||||
// adminControllerSet is the plugin's three admin controllers built on one app
|
||||
// accessor, by controller id.
|
||||
type adminControllerSet map[string]pact.AdminController
|
||||
|
||||
func adminControllersOn(app func() *backpack.App) adminControllerSet {
|
||||
out := adminControllerSet{}
|
||||
for _, ctl := range controllers.AdminControllers(app) {
|
||||
out[ctl.ID()] = ctl
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// bareApp is a booted application value without a database handle, a bucket
|
||||
// or a translator.
|
||||
func bareApp(t *testing.T) *backpack.App {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(dir, "app.yaml"), []byte("name: user-bare\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
cfg, err := compass.Open(compass.Options{Dir: dir, Environ: []string{"SUMMER_ENV=development"}})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return backpack.New(cfg)
|
||||
}
|
||||
|
||||
// brokenApp is an application whose database handle fails every query: its
|
||||
// connection pool is closed.
|
||||
func brokenApp(t *testing.T) (*backpack.App, *gorm.DB) {
|
||||
t.Helper()
|
||||
adminHarnessDB(t)
|
||||
u, err := url.Parse(sessionDSN)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
u.Path = "/user_admin"
|
||||
db, err := sql.Open("pgx", u.String())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
gdb, err := lagoon.Use(t.Context(), db)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
app := bareApp(t)
|
||||
if err := lagoon.Publish(app, db, gdb); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := db.Close(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return app, gdb
|
||||
}
|
||||
|
||||
// wantPlainError asserts that err is an ordinary error: the framework answers
|
||||
// it as the opaque 500. A ForbiddenError or a ValidationError would be shown
|
||||
// to the administrator.
|
||||
func wantPlainError(t *testing.T, what string, err error) {
|
||||
t.Helper()
|
||||
var forbidden *cabana.ForbiddenError
|
||||
var invalid *cabana.ValidationError
|
||||
if err == nil || errors.As(err, &forbidden) || errors.As(err, &invalid) {
|
||||
t.Fatalf("%s: err = %v, want a plain error", what, err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminUsersControllerGuards drives the Users controller directly, the
|
||||
// way the framework calls it, on the inputs the HTTP tests cannot produce: a
|
||||
// record of another type, an application that is not booted, one without a
|
||||
// database handle and one whose database fails. Each ends in a plain error
|
||||
// (the opaque 500) or the documented empty answer; none passes a guard.
|
||||
func TestAdminUsersControllerGuards(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
notUser := &models.UserGroup{}
|
||||
var nilUser *models.User
|
||||
user := &models.User{ID: 987654001, Email: "guards@example.test"}
|
||||
|
||||
sets := map[string]adminControllerSet{
|
||||
"no accessor": adminControllersOn(nil),
|
||||
"not booted": adminControllersOn(func() *backpack.App { return nil }),
|
||||
"no database": adminControllersOn(func() *backpack.App { return bareApp(t) }),
|
||||
"a failing handle": nil,
|
||||
"the live database": nil,
|
||||
}
|
||||
broken, brokenDB := brokenApp(t)
|
||||
sets["a failing handle"] = adminControllersOn(func() *backpack.App { return broken })
|
||||
env := newAdminEnv(t)
|
||||
sets["the live database"] = adminControllersOn(func() *backpack.App { return env.app })
|
||||
|
||||
for name, set := range sets {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
ctl := set["golem15.user.users"]
|
||||
live := name == "the live database"
|
||||
|
||||
// Records of another type and nil records.
|
||||
for _, record := range []any{notUser, nilUser, nil} {
|
||||
wantPlainError(t, "FormBeforeUpdate", ctl.(pact.FormBeforeUpdate).FormBeforeUpdate(ctx, record))
|
||||
wantPlainError(t, "FormBeforeCreate", ctl.(pact.FormBeforeCreate).FormBeforeCreate(ctx, record))
|
||||
wantPlainError(t, "FormAfterCreate", ctl.(pact.FormAfterCreate).FormAfterCreate(ctx, record))
|
||||
wantPlainError(t, "FormBeforeDelete", ctl.(pact.FormBeforeDelete).FormBeforeDelete(ctx, record))
|
||||
wantPlainError(t, "FormAfterDelete", ctl.(pact.FormAfterDelete).FormAfterDelete(ctx, record))
|
||||
_, err := ctl.(pact.ListRowStates).ListRowStates(ctx, env.gdb, []any{record})
|
||||
wantPlainError(t, "ListRowStates", err)
|
||||
editor := ctl.(cabana.PermissionEditorProvider)
|
||||
_, err = editor.AdminPermissionValues(ctx, "permissions", record)
|
||||
wantPlainError(t, "AdminPermissionValues", err)
|
||||
wantPlainError(t, "AdminSetPermissionValues", editor.AdminSetPermissionValues(ctx, "permissions", record, map[string]int{"a": 1}))
|
||||
for _, action := range ctl.(pact.HasAdminBulkActions).AdminBulkActions() {
|
||||
_, err := action.Run(ctx, pact.AdminBulkActionInput{Records: []any{record}})
|
||||
wantPlainError(t, "bulk "+action.Name, err)
|
||||
}
|
||||
for _, action := range ctl.(pact.HasAdminRecordActions).AdminRecordActions() {
|
||||
_, err := action.Applies(ctx, record)
|
||||
wantPlainError(t, "applies "+action.Name, err)
|
||||
_, err = action.Run(ctx, pact.AdminRecordActionInput{Record: record})
|
||||
wantPlainError(t, "record "+action.Name, err)
|
||||
}
|
||||
}
|
||||
// The hint of a record that is not a user is empty, never an error.
|
||||
if data, err := ctl.(pact.AdminPartialData).PartialData(ctx, "hint", notUser); err != nil || !reflect.ValueOf(data).IsZero() {
|
||||
t.Fatalf("hint of another record = %+v %v", data, err)
|
||||
}
|
||||
if _, err := ctl.(pact.AdminPartialData).PartialData(ctx, "other", user); err == nil || !strings.Contains(err.Error(), `unknown partial "other"`) {
|
||||
t.Fatalf("unknown partial: %v", err)
|
||||
}
|
||||
|
||||
// Wrong field names.
|
||||
editor := ctl.(cabana.PermissionEditorProvider)
|
||||
if _, err := editor.AdminPermissionOptions(ctx, "groups"); err == nil {
|
||||
t.Fatal("permission options for another field")
|
||||
}
|
||||
if _, err := editor.AdminPermissionValues(ctx, "groups", user); err == nil {
|
||||
t.Fatal("permission values for another field")
|
||||
}
|
||||
if err := editor.AdminSetPermissionValues(ctx, "groups", user, nil); err == nil {
|
||||
t.Fatal("permission write for another field")
|
||||
}
|
||||
if lock, err := ctl.(cabana.RelationLockProvider).AdminRelationLocks(ctx, "organisation"); err != nil || len(lock.IDs) != 0 || lock.Message != "" {
|
||||
t.Fatalf("locks of another field = %+v %v", lock, err)
|
||||
}
|
||||
if options := ctl.(pact.FilterOptions).FilterOptions("other"); options != nil {
|
||||
t.Fatalf("options of another scope = %v", options)
|
||||
}
|
||||
if ctl.(pact.ListExtendQuery).ListExtendQuery(ctx, nil) != nil || ctl.(pact.FormExtendQuery).FormExtendQuery(ctx, nil) != nil {
|
||||
t.Fatal("an extend query invented a handle")
|
||||
}
|
||||
|
||||
// A bulk or record action outside a transaction never writes.
|
||||
for _, action := range ctl.(pact.HasAdminBulkActions).AdminBulkActions() {
|
||||
_, err := action.Run(ctx, pact.AdminBulkActionInput{Records: []any{user}})
|
||||
wantPlainError(t, "bulk "+action.Name+" without a transaction", err)
|
||||
}
|
||||
for _, action := range ctl.(pact.HasAdminRecordActions).AdminRecordActions() {
|
||||
_, err := action.Run(ctx, pact.AdminRecordActionInput{Record: user})
|
||||
wantPlainError(t, "record "+action.Name+" without a transaction", err)
|
||||
}
|
||||
wantPlainError(t, "FormAfterDelete without a transaction", ctl.(pact.FormAfterDelete).FormAfterDelete(ctx, user))
|
||||
|
||||
if live {
|
||||
// On a working database the same calls answer, and an update
|
||||
// of a user who does not exist is an error, not a pass.
|
||||
wantPlainError(t, "update of an unknown user", ctl.(pact.FormBeforeUpdate).FormBeforeUpdate(ctx, user))
|
||||
if options := ctl.(pact.FilterOptions).FilterOptions(models.FilterByGroup); options == nil {
|
||||
t.Fatal("no group choices on a working database")
|
||||
}
|
||||
if lock, err := ctl.(cabana.RelationLockProvider).AdminRelationLocks(ctx, "groups"); err != nil || lock.Message == "" {
|
||||
t.Fatalf("locks without a principal = %+v %v, want the locks", lock, err)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// Without a working database every read that a guard depends on
|
||||
// is an error: the guard never answers "not privileged".
|
||||
wantPlainError(t, "FormBeforeUpdate", ctl.(pact.FormBeforeUpdate).FormBeforeUpdate(ctx, user))
|
||||
wantPlainError(t, "FormBeforeCreate", ctl.(pact.FormBeforeCreate).FormBeforeCreate(ctx, user))
|
||||
wantPlainError(t, "FormBeforeDelete", ctl.(pact.FormBeforeDelete).FormBeforeDelete(ctx, user))
|
||||
_, err := ctl.(cabana.RelationLockProvider).AdminRelationLocks(ctx, "groups")
|
||||
wantPlainError(t, "AdminRelationLocks", err)
|
||||
_, err = editor.AdminPermissionOptions(ctx, "permissions")
|
||||
wantPlainError(t, "AdminPermissionOptions", err)
|
||||
_, err = ctl.(pact.AdminPartialData).PartialData(ctx, "hint", user)
|
||||
wantPlainError(t, "PartialData", err)
|
||||
for _, action := range ctl.(pact.HasAdminRecordActions).AdminRecordActions() {
|
||||
if action.Name == "activate" {
|
||||
continue // reads the record only
|
||||
}
|
||||
_, err := action.Applies(ctx, user)
|
||||
wantPlainError(t, "applies "+action.Name, err)
|
||||
}
|
||||
if options := ctl.(pact.FilterOptions).FilterOptions(models.FilterByGroup); options != nil {
|
||||
t.Fatalf("group choices without a database = %v", options)
|
||||
}
|
||||
if name == "a failing handle" {
|
||||
_, err := ctl.(pact.ListRowStates).ListRowStates(ctx, brokenDB, []any{user})
|
||||
wantPlainError(t, "ListRowStates on a failing handle", err)
|
||||
}
|
||||
|
||||
// A principal who holds the extra permission, and a superuser,
|
||||
// pass the delete guard before any query is made (D-30).
|
||||
for _, principal := range []*bouncer.Principal{
|
||||
{PermissionGrants: map[string]bool{permManagePrivileged: true}},
|
||||
{IsSuperuser: true},
|
||||
} {
|
||||
if err := ctl.(pact.FormBeforeDelete).FormBeforeDelete(bouncer.WithUser(ctx, principal), user); err != nil {
|
||||
t.Fatalf("delete guard for %+v: %v", principal, err)
|
||||
}
|
||||
if lock, err := ctl.(cabana.RelationLockProvider).AdminRelationLocks(bouncer.WithUser(ctx, principal), "groups"); err != nil || len(lock.IDs) != 0 {
|
||||
t.Fatalf("locks for %+v = %+v %v", principal, lock, err)
|
||||
}
|
||||
}
|
||||
// One who holds only the screen's permission does not.
|
||||
limited := bouncer.WithUser(ctx, &bouncer.Principal{PermissionGrants: map[string]bool{permAccessUsers: true}})
|
||||
wantPlainError(t, "delete guard without the extra permission", ctl.(pact.FormBeforeDelete).FormBeforeDelete(limited, user))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminUsersEdge covers the remaining branches of the Users screen
|
||||
// through the admin API: an unknown partial, an undeclared filter, options
|
||||
// of a field that has none, search and sort, the superuser, and a membership
|
||||
// query that fails while the takeover guard needs it (D-30).
|
||||
func TestAdminUsersEdge(t *testing.T) {
|
||||
env := newAdminEnv(t)
|
||||
token := env.admin(t, permAccessUsers)
|
||||
marker := "edge" + adminStamp()
|
||||
surname := "Sur" + marker
|
||||
first := env.seedUser(t, "a-"+marker, func(u *models.User) { u.Surname = &surname })
|
||||
second := env.seedUser(t, "b-"+marker, nil)
|
||||
third := env.seedUser(t, "c-"+marker, nil)
|
||||
id := strconv.FormatUint(uint64(first.ID), 10)
|
||||
|
||||
// Partials: only the declared hint is served.
|
||||
for _, name := range []string{"nope", "status", "_hint", "hint.htm", "..%2Fhint"} {
|
||||
rec := env.call(t, http.MethodGet, adminAPI(adminUsersPath+"/partials/"+name+"?id="+id), token, nil)
|
||||
if rec.Code != http.StatusNotFound && rec.Code != http.StatusBadRequest && rec.Code != http.StatusUnprocessableEntity {
|
||||
t.Fatalf("partial %s answers %d: %s", name, rec.Code, rec.Body.String())
|
||||
}
|
||||
assertNoSecret(t, rec.Body.String())
|
||||
}
|
||||
env.expect(t, http.StatusOK, http.MethodGet, adminAPI(adminUsersPath+"/partials/hint?id="+id), token, nil)
|
||||
env.expect(t, http.StatusNotFound, http.MethodGet, adminAPI(adminUsersPath+"/partials/hint?id=987654321"), token, nil)
|
||||
|
||||
// Filters: an undeclared name and a malformed value are 422, never a
|
||||
// query.
|
||||
for _, query := range []string{
|
||||
"filter[nope]=1", "filter[password]=x", "filter[activated]=maybe",
|
||||
"filter[created_date]=yesterday", "filter[created_date]=2020-06-30..2020-06-01x",
|
||||
} {
|
||||
env.expect(t, http.StatusUnprocessableEntity, http.MethodGet, adminAPI(adminUsersPath)+"?"+query, token, nil)
|
||||
}
|
||||
// Options exist for the groups filter and the two relation fields only.
|
||||
for _, path := range []string{
|
||||
"/filters/activated/options", "/filters/created_date/options", "/filters/nope/options",
|
||||
"/fields/name/options", "/fields/password/options", "/fields/permissions/options", "/fields/nope/options",
|
||||
} {
|
||||
rec := env.call(t, http.MethodGet, adminAPI(adminUsersPath+path), token, nil)
|
||||
if rec.Code != http.StatusNotFound && rec.Code != http.StatusUnprocessableEntity {
|
||||
t.Fatalf("%s answers %d: %s", path, rec.Code, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// Search reaches the invisible surname column; sort orders both ways and
|
||||
// refuses a column that is not sortable.
|
||||
if page := env.users(t, token, "search="+surname); page.Total != 1 || !page.has(first.ID) {
|
||||
t.Fatalf("surname search = %s", page.Raw)
|
||||
}
|
||||
order := func(query string) []uint {
|
||||
ids := []uint{}
|
||||
for _, row := range env.users(t, token, "search="+marker+"&"+query).Rows {
|
||||
ids = append(ids, uint(row["id"].(float64)))
|
||||
}
|
||||
return ids
|
||||
}
|
||||
if got, want := order("sort=email&dir=asc"), []uint{first.ID, second.ID, third.ID}; !reflect.DeepEqual(got, want) {
|
||||
t.Fatalf("email asc = %v, want %v", got, want)
|
||||
}
|
||||
if got, want := order("sort=email&dir=desc"), []uint{third.ID, second.ID, first.ID}; !reflect.DeepEqual(got, want) {
|
||||
t.Fatalf("email desc = %v, want %v", got, want)
|
||||
}
|
||||
if got, want := order("sort=id&dir=desc"), []uint{third.ID, second.ID, first.ID}; !reflect.DeepEqual(got, want) {
|
||||
t.Fatalf("id desc = %v, want %v", got, want)
|
||||
}
|
||||
for _, query := range []string{"sort=password", "sort=permissions", "sort=email&dir=sideways", "sort=email%3Bdrop", "per_page=2"} {
|
||||
env.expect(t, http.StatusUnprocessableEntity, http.MethodGet, adminAPI(adminUsersPath)+"?"+query, token, nil)
|
||||
}
|
||||
|
||||
// D-30: a superuser needs no extra permission for a privileged member.
|
||||
code := "edgepriv-" + marker
|
||||
privileged := func(cfg *compass.Config) {
|
||||
if err := cfg.Set("golem15.user.privileged_groups", []string{code}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
env = newAdminEnv(t, privileged)
|
||||
limited := env.admin(t, permAccessUsers)
|
||||
super := env.admin(t)
|
||||
if err := env.gdb.Exec(`UPDATE backend_users SET is_superuser = TRUE WHERE id = (SELECT MAX(id) FROM backend_users)`).Error; err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
group := env.seedGroup(t, "Edge privileged "+marker, code)
|
||||
member := env.seedUser(t, "member-"+marker, nil)
|
||||
env.join(t, member, group)
|
||||
env.expect(t, http.StatusForbidden, http.MethodPut, userPath(member.ID, ""), limited, map[string]any{"email": "taken-" + marker + "@example.test"})
|
||||
renamed := "super-" + marker + "@example.test"
|
||||
env.expect(t, http.StatusOK, http.MethodPut, userPath(member.ID, ""), super, map[string]any{"email": renamed})
|
||||
if got := env.reload(t, member.ID); got.Email != renamed {
|
||||
t.Fatalf("superuser update: email = %s", got.Email)
|
||||
}
|
||||
|
||||
// D-30: when the membership cannot be read, the guard does not let the
|
||||
// request through. The answer is the opaque 500 and nothing changes.
|
||||
before := env.reload(t, member.ID)
|
||||
victim := env.seedUser(t, "victim-"+marker, nil)
|
||||
rename := func(from, to string) {
|
||||
t.Helper()
|
||||
if err := env.gdb.Exec(`ALTER TABLE ` + from + ` RENAME TO ` + to).Error; err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
rename("users_groups", "users_groups_away")
|
||||
restored := false
|
||||
restore := func() {
|
||||
if !restored {
|
||||
restored = true
|
||||
rename("users_groups_away", "users_groups")
|
||||
}
|
||||
}
|
||||
t.Cleanup(restore)
|
||||
for what, rec := range map[string]interface {
|
||||
Result() *http.Response
|
||||
}{
|
||||
"email change": env.call(t, http.MethodPut, userPath(member.ID, ""), limited, map[string]any{"email": "stolen-" + marker + "@example.test"}),
|
||||
"password reset": env.call(t, http.MethodPut, userPath(member.ID, ""), limited, map[string]any{"password": "stolen-secret-1", "password_confirmation": "stolen-secret-1"}),
|
||||
"delete": env.call(t, http.MethodDelete, userPath(member.ID, ""), limited, nil),
|
||||
"bulk delete": env.bulk(t, limited, "delete", victim, member),
|
||||
"ordinary delete": env.call(t, http.MethodDelete, userPath(victim.ID, ""), limited, nil),
|
||||
} {
|
||||
res := rec.Result()
|
||||
body := make([]byte, 4096)
|
||||
n, _ := res.Body.Read(body)
|
||||
text := string(body[:n])
|
||||
if res.StatusCode != http.StatusInternalServerError {
|
||||
t.Fatalf("%s with an unreadable membership answers %d: %s", what, res.StatusCode, text)
|
||||
}
|
||||
if strings.Contains(text, "users_groups") || strings.Contains(strings.ToLower(text), "sqlstate") || strings.Contains(text, "relation") {
|
||||
t.Fatalf("%s leaks the database error: %s", what, text)
|
||||
}
|
||||
}
|
||||
restore()
|
||||
after := env.reload(t, member.ID)
|
||||
if after == nil || after.Email != before.Email || after.Password != before.Password || !reflect.DeepEqual(after.TokensValidAfter, before.TokensValidAfter) {
|
||||
t.Fatalf("a failed guard changed the member: %+v", after)
|
||||
}
|
||||
if env.reload(t, victim.ID) == nil {
|
||||
t.Fatal("a failed guard deleted a user")
|
||||
}
|
||||
if got := env.groupIDs(t, member.ID); !reflect.DeepEqual(got, []uint{group.ID}) {
|
||||
t.Fatalf("memberships after the failed guard = %v", got)
|
||||
}
|
||||
if status, _ := env.userLogin(t, before.Email, "correct-horse"); status != http.StatusOK {
|
||||
t.Fatalf("login with the old password = %d", status)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user