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

@@ -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())
+	}
+}

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) {

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)
+	}
+}