From 7991b539799392289cb65a8373d85150e70e2c69 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Mon, 5 Oct 2026 15:23:23 +0200 Subject: [PATCH] test: extend the permission resolver table, the admin migration history and the API token refusals - resolver: several groups, numbers stored as text, empty group sets, a code only at user level, booleans - migrations: every id once, the history applied twice, the unique code after a down and an up - personal API tokens: no application, no principal, invalid bodies, another user's token --- api_tokens_edge_test.go | 104 ++++++++++++++++++++++++++++++++++ classes/permissions_test.go | 42 ++++++++++++++ updates/admin_columns_test.go | 59 +++++++++++++++++++ 3 files changed, 205 insertions(+) create mode 100644 api_tokens_edge_test.go diff --git a/api_tokens_edge_test.go b/api_tokens_edge_test.go new file mode 100644 index 0000000..8081e2c --- /dev/null +++ b/api_tokens_edge_test.go @@ -0,0 +1,104 @@ +package user + +import ( + "net/http" + "net/http/httptest" + "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/bouncer" +) + +// TestAPITokenManagementRefusals covers the refusals of the personal API +// token routes: no application, no principal, invalid bodies, and a token +// that is not the caller's. +func TestAPITokenManagementRefusals(t *testing.T) { + app, db := sessionApp(t, "0s") + if err := app.Config.Set("golem15.user.api_tokens.prefix", "usr_"); err != nil { + t.Fatal(err) + } + owner := insertUser(t, db, "token-owner@example.test", "correct-horse") + stranger := insertUser(t, db, "token-stranger@example.test", "correct-horse") + + call := func(handler http.Handler, method, body, id string, userID uint) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(method, "/_user/api/v1/tokens", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + if id != "" { + req.SetPathValue("id", id) + } + if userID != 0 { + req = req.WithContext(bouncer.WithUser(req.Context(), &bouncer.Principal{ID: userID})) + } + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + return rec + } + + // Without an application or a principal nothing is served. + for name, handler := range map[string]http.Handler{ + "store": controllers.APITokenStore(nil), "index": controllers.APITokenIndex(nil), "destroy": controllers.APITokenDestroy(nil), + } { + if rec := call(handler, http.MethodPost, `{"name":"x"}`, "1", owner.ID); rec.Code != http.StatusInternalServerError { + t.Fatalf("%s without an application = %d", name, rec.Code) + } + } + for name, handler := range map[string]http.Handler{ + "store": controllers.APITokenStore(app), "index": controllers.APITokenIndex(app), "destroy": controllers.APITokenDestroy(app), + } { + if rec := call(handler, http.MethodPost, `{"name":"x"}`, "1", 0); rec.Code != http.StatusUnauthorized { + t.Fatalf("%s without a principal = %d %s", name, rec.Code, rec.Body.String()) + } + } + + // Invalid bodies are 422 on the field and mint nothing. + for body, field := range map[string]string{ + `{}`: "name", + `{"name":" "}`: "name", + `{"name":"` + strings.Repeat("n", 256) + `"}`: "name", + `{"name":"a","scopes":"read"}`: "scopes", + `{"name":"a","scopes":["nope"]}`: "scopes", + `{"name":"a","scopes":[1]}`: "scopes", + `{"name":"a","expires_at":"tomorrow"}`: "expires_at", + } { + rec := call(controllers.APITokenStore(app), http.MethodPost, body, "", owner.ID) + if rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), `"`+field+`"`) { + t.Fatalf("store %s = %d %s, want 422 on %s", body, rec.Code, rec.Body.String(), field) + } + } + var minted int64 + if err := db.Model(&models.APIToken{}).Where("user_id = ?", owner.ID).Count(&minted).Error; err != nil || minted != 0 { + t.Fatalf("tokens after refused creates = %d err = %v", minted, err) + } + + // A token with an expiry is minted; only its owner can revoke it. + created := call(controllers.APITokenStore(app), http.MethodPost, `{"name":"expiring","expires_at":"2099-01-02T03:04:05Z"}`, "", owner.ID) + if created.Code != http.StatusCreated { + t.Fatalf("create = %d %s", created.Code, created.Body.String()) + } + var token models.APIToken + if err := db.Where("user_id = ?", owner.ID).First(&token).Error; err != nil || token.ExpiresAt == nil { + t.Fatalf("stored token = %+v err = %v", token, err) + } + id := strconv.FormatUint(uint64(token.ID), 10) + for _, bad := range []string{"", "0", "abc", "-1", "987654321"} { + if rec := call(controllers.APITokenDestroy(app), http.MethodDelete, "", bad, owner.ID); rec.Code != http.StatusNotFound { + t.Fatalf("destroy %q = %d %s", bad, rec.Code, rec.Body.String()) + } + } + if rec := call(controllers.APITokenDestroy(app), http.MethodDelete, "", id, stranger.ID); rec.Code != http.StatusNotFound { + t.Fatalf("destroy by another user = %d %s", rec.Code, rec.Body.String()) + } + if rec := call(controllers.APITokenIndex(app), http.MethodGet, "", "", stranger.ID); rec.Code != http.StatusOK || strings.Contains(rec.Body.String(), "expiring") { + t.Fatalf("another user's list = %d %s", rec.Code, rec.Body.String()) + } + if rec := call(controllers.APITokenDestroy(app), http.MethodDelete, "", id, owner.ID); rec.Code != http.StatusOK { + t.Fatalf("destroy = %d %s", rec.Code, rec.Body.String()) + } + if rec := call(controllers.APITokenIndex(app), http.MethodGet, "", "", owner.ID); rec.Code != http.StatusOK || strings.Contains(rec.Body.String(), "expiring") { + t.Fatalf("list after the revoke = %d %s", rec.Code, rec.Body.String()) + } +} diff --git a/classes/permissions_test.go b/classes/permissions_test.go index ce3d9e4..8180715 100644 --- a/classes/permissions_test.go +++ b/classes/permissions_test.go @@ -125,6 +125,48 @@ func TestMergedPermissions(t *testing.T) { merged: map[string]int{"blog.post": 1, "blog.edit": -1}, asks: []ask{{"blog.post", true}, {"blog.bad", false}, {"blog.edit", false}}, }, + { + name: "several groups: the highest value of each code wins", + groups: []string{ + `{"blog.post":-1,"blog.edit":1,"shop.buy":0}`, + `{"blog.post":0,"blog.edit":-1,"news.read":1}`, + `{"blog.post":1,"shop.buy":-1}`, + `{}`, + }, + own: "NULL", + merged: map[string]int{"blog.post": 1, "blog.edit": 1, "shop.buy": 0, "news.read": 1}, + asks: []ask{{"blog.post", true}, {"blog.edit", true}, {"shop.buy", false}, {"news.read", true}, {"news.*", true}}, + }, + { + name: "numbers stored as text count at both levels", + groups: []string{`{"blog.post":"1","blog.edit":"1","shop.buy":"-1"}`}, + own: `{"blog.edit":"-1","shop.buy":"1","news.read":"0"}`, + merged: map[string]int{"blog.post": 1, "blog.edit": -1, "shop.buy": 1, "news.read": 0}, + asks: []ask{{"blog.post", true}, {"blog.edit", false}, {"shop.buy", true}, {"news.read", false}}, + }, + { + name: "groups that store nothing leave the user's own permissions", + groups: []string{`{}`, ``, `[]`}, + own: `{"blog.post":1}`, + merged: map[string]int{"blog.post": 1}, + asks: []ask{{"blog.post", true}, {"blog.edit", false}}, + }, + { + name: "a code present only at user level is held or denied by that value alone", + groups: []string{`{"blog.post":1}`}, + own: `{"only.allow":1,"only.deny":-1,"only.inherit":0}`, + merged: map[string]int{"blog.post": 1, "only.allow": 1, "only.deny": -1, "only.inherit": 0}, + asks: []ask{{"only.allow", true}, {"only.deny", false}, {"only.inherit", false}, {"only.*", true}, {"blog.post", true}}, + }, + { + // 12.1-SECURITY-REVIEW.md, open item 5: the resolver reads + // numbers only. A boolean is not a grant here. + name: "a boolean value is not a grant", + groups: []string{`{"blog.post":true,"blog.edit":false}`}, + own: `{"shop.buy":true}`, + merged: map[string]int{}, + asks: []ask{{"blog.post", false}, {"shop.buy", false}}, + }, } for i, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/updates/admin_columns_test.go b/updates/admin_columns_test.go index d075d96..5947233 100644 --- a/updates/admin_columns_test.go +++ b/updates/admin_columns_test.go @@ -107,3 +107,62 @@ func TestAdminColumnsMigrations(t *testing.T) { t.Fatal("re-up did not restore the admin columns") } } + +// TestAdminColumnsHistory: the three admin migrations are part of the +// plugin's history under their own ids, each id once, and applying the whole +// history twice changes nothing. +func TestAdminColumnsHistory(t *testing.T) { + sqlDB := dedicatedDB(t, "user_admin_history") + gdb, err := lagoon.Use(t.Context(), sqlDB) + if err != nil { + t.Fatal(err) + } + seen := map[string]int{} + for _, migration := range All() { + seen[migration.ID]++ + if migration.Migrate == nil || migration.Rollback == nil { + t.Fatalf("migration %s lacks an up or a down", migration.ID) + } + } + admin := []string{usersPermissionsMigrations[0].ID, usersLastSeenMigrations[0].ID, frontendPermissionsMigrations[0].ID} + for _, id := range admin { + if seen[id] != 1 { + t.Fatalf("migration %s is in the history %d times", id, seen[id]) + } + } + for id, n := range seen { + if n != 1 { + t.Fatalf("migration id %s is used %d times", id, n) + } + } + m := gormigrate.New(gdb, &gormigrate.Options{ + TableName: "summer_migrations_golem15_user", + UseTransaction: true, + }, All()) + for range 2 { + if err := m.Migrate(); err != nil { + t.Fatal(err) + } + } + var applied int64 + if err := gdb.Raw(`SELECT COUNT(*) FROM summer_migrations_golem15_user WHERE id IN ?`, admin).Scan(&applied).Error; err != nil || applied != 3 { + t.Fatalf("applied admin migrations = %d err = %v", applied, err) + } + // The permission code stays unique after a down and an up of its table. + if err := m.RollbackMigration(frontendPermissionsMigrations[0]); err != nil { + t.Fatal(err) + } + if err := m.Migrate(); err != nil { + t.Fatal(err) + } + insert := `INSERT INTO golem15_user_frontend_permissions (code, label, tab) VALUES (?, ?, ?)` + if err := gdb.Exec(insert, "history.code", "One", "Tab").Error; err != nil { + t.Fatal(err) + } + if err := gdb.Exec(insert, "history.code", "Two", "Other").Error; err == nil { + t.Fatal("a duplicate permission code was accepted after the re-up") + } + if err := gdb.Exec(insert, "History.Code", "Three", "Tab").Error; err != nil { + t.Fatalf("codes differing by case are distinct: %v", err) + } +}