fix: searches read every grant to the tenant's members and the public on the listed types even for the anonymous and guests, read each grant's row, and as of a time read every version of grants they don't count or of other types, and checks every version of their subject's grants

README.md

@@ -411,16 +411,17 @@ change that counts from its start. A prepared transaction, which `pg_stat_activi
 has ended only once it is committed or rolled back: `pg_prepared_xacts` lists it until then, by when
 it was prepared, not when it began. `AsOf` makes no change: one tried is no actor's attempt, and
 `Config.OnRefused` isn't told of it. A statement reads the history of the rows it names, as a check
-does its subject's grants, groups and the denies on its resource's path, a search the custom roles
-its grants name, and `Holders` the members of the groups it lists, not the whole tenant's: `AsOf`
-plans every statement in its transaction for its arguments, the host's too, since a connection's
-cached plan of a statement it has run often counts any tenant as an average one. A list counts the
-denies on scopes of its types by history's versions of them, at most a few, and `Grantable`, which
-can offer any custom role, reads each version of every one once. It orders a row's changes by their
-ids in `rbac_history`, which sessions take out of order once that table's id sequence caches more
-than one, so `AsOf` refuses that. Its read's hooks run in its transaction, which holds one of the
-pool's connections: a constraint there queries the `DB` it is given, as one that queries the pool
-waits for another connection, which a busy pool may never free.
+does its subject's grants, groups and the denies on its resource's path, a search its subject's
+grants, and the public's and the members' it counts, on each type it lists, and the custom roles
+they name, and `Holders` the members of the groups it lists, not the whole tenant's: `AsOf` plans
+every statement in its transaction for its arguments, the host's too, since a connection's cached
+plan of a statement it has run often counts any tenant as an average one. A list counts the denies
+on scopes of its types by history's versions of them, at most a few, and `Grantable`, which can
+offer any custom role, reads each version of every one once. It orders a row's changes by their ids
+in `rbac_history`, which sessions take out of order once that table's id sequence caches more than
+one, so `AsOf` refuses that. Its read's hooks run in its transaction, which holds one of the pool's
+connections: a constraint there queries the `DB` it is given, as one that queries the pool waits
+for another connection, which a busy pool may never free.
 
 `rbac.PruneHistory(ctx, db, before, through)` deletes what an export has read past (`through`, its
 cursor) and later records supersede: of each row, the last version at or before `before` stays, so
@@ -537,6 +538,12 @@ pg_catalog's, so none stands in for them, and refuse a search_path that names pg
   migrating while it holds what the rebuild waits for: the rebuild gives way, unless the
   transaction began waiting long enough before it, as while migrating reads a long history's deny
   records, for its `deadlock_timeout` to pass first, and PostgreSQL ends the transaction.
+  `0019_principal_seeks` builds an index on `rbac_grants`, which keeps writes to it waiting until it
+  commits, though reads go on, and rebuilds history's index of a principal's versions, which reads
+  all of history, and analyzes it. It locks `rbac_grants` and then history before it builds either,
+  so reads of history, and every change, as each writes history, wait until it commits too, and only
+  a transaction under way as it starts that read history or made a change, as deleting a scope's
+  eligibilities and then its grants does, and then writes a grant can make it give way.
 - **Transactions:** a change runs in its own transaction at READ COMMITTED, or under `WithTx` in
   the host's, which must be READ COMMITTED too. A change refused for a race, such as a deadlock
   between host transactions, matches `rbac.ErrConflict`: try it again. Changes to the same rows

asof_test.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
+	"maps"
 	"slices"
 	"strings"
 	"testing"
@@ -621,6 +622,74 @@ func TestAsOfReadsARowChangedInPlaceAsItsLastVersion(t *testing.T) {
 	}
 }
 
+// As of a time, a check, Permissions and Grantable seek their subject's grants on the resource's
+// path, however their statements are planned, beside thousands of its grants elsewhere, as one on
+// each cluster it created: history's index of a principal's versions held no scope, so in a history
+// of many tenants a plan read every version of the subject's grants to find the few on the path.
+func TestAsOfChecksSeekTheirSubjectsGrantsOnThePath(t *testing.T) {
+	h := crowded(t, pgtest.Open(t, ""))
+	ctx := t.Context()
+	u := h.as(t, "u")
+	h.seed(t, grant(cluster.Scope(), "cluster.login", rbac.User, "u"))
+	granted := 0
+	more := func(n int) {
+		t.Helper()
+		if _, err := h.pool.Exec(ctx, `INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role)
+			SELECT 't1', 'cluster', 'k' || i, 'user', 'u', 'cluster.login' FROM generate_series($1::int + 1, $2::int) i`, granted, n); err != nil {
+			t.Fatal(err)
+		}
+		granted = n
+		if _, err := h.pool.Exec(ctx, `ANALYZE rbac_grants, rbac_history`); err != nil {
+			t.Fatal(err)
+		}
+	}
+	read := func(at time.Time, plans string) map[string]int64 {
+		t.Helper()
+		tx, err := h.pool.Begin(ctx)
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer func() { _ = tx.Rollback(ctx) }()
+		pages := map[string]int64{}
+		if err := h.authz.WithTx(tx).AsOf(ctx, "t1", at, func(a *rbac.Authorizer) error {
+			if _, err := tx.Exec(ctx, `SELECT set_config('plan_cache_mode', $1, true)`, plans); err != nil {
+				return err
+			}
+			for what, f := range map[string]func() error{
+				"Authorize":   func() error { return a.Authorize(ctx, u, login, cluster) },
+				"Permissions": func() error { return second(a.Permissions(ctx, u, cluster)) },
+				"Grantable":   func() error { return second(a.Grantable(ctx, u, cluster)) },
+			} {
+				start := historyPages(ctx, t, tx)
+				if err := f(); err != nil {
+					return fmt.Errorf("%s: %w", what, err)
+				}
+				pages[what+" planned "+plans] = historyPages(ctx, t, tx) - start
+			}
+			return nil
+		}); err != nil {
+			t.Fatal(err)
+		}
+		return pages
+	}
+	plans := []string{"force_custom_plan", "force_generic_plan"}
+	more(500)
+	few := map[string]int64{}
+	at := dbNow(t, h)
+	for _, p := range plans {
+		maps.Copy(few, read(at, p))
+	}
+	more(5000)
+	at = dbNow(t, h)
+	for _, p := range plans {
+		for what, many := range read(at, p) {
+			if many > few[what]*3/2+deeper {
+				t.Errorf("as of a time, %s read %d pages of history beside 500 grants of its subject's on other clusters, and %d beside 5000", what, few[what], many)
+			}
+		}
+	}
+}
+
 // A check as of a time reads the history of its subject's grants, memberships and groups, not
 // every row's, and of a conditional grant it holds, for its condition: other principals' history
 // costs it nothing.
@@ -1279,6 +1348,174 @@ func TestMigratingLetsHistorysRoleCallWhatItsIndexCalls(t *testing.T) {
 	}
 }
 
+// Migrating to 0019 locks history before it builds the grants' index, so a change that starts while
+// it builds and writes history before a grant, as DeleteScope removes a scope's eligibilities and
+// then its grants, waits for it to commit: locked once the index was built, history was the change's
+// while the change waited for the grants, so migrating gave way, having built the index, or
+// PostgreSQL ended one of the two as a deadlock.
+func TestMigratingThePrincipalSeeksWaitsForAChangeStartedAsItBuilds(t *testing.T) {
+	// Little memory to sort in, so building the index takes a while.
+	pool := pgtest.Open(t, "maintenance_work_mem=64kB")
+	ctx := t.Context()
+	migrateFiles(t, pool, "", "0019")
+	recordMigrations(t, pool, "0019")
+	// So a Migrate that gave way would give up at once.
+	rbac.SetMigrateAttempts(t, 1, time.Millisecond)
+	for _, q := range []string{
+		`ALTER TABLE rbac_grants DISABLE TRIGGER USER`,
+		`INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role)
+		  SELECT 't1', 'cluster', 'k' || i, 'user', 'u' || i, 'custom.r' FROM generate_series(1, 100000) i`,
+		`ALTER TABLE rbac_grants ENABLE TRIGGER USER`,
+		`INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role)
+		  VALUES ('t1', 'cluster', 'c1', 'user', 'u', 'cluster.login')`,
+		`INSERT INTO rbac_eligibilities (tenant, scope_type, scope_id, principal_kind, principal_id, role, max_seconds)
+		  VALUES ('t1', 'cluster', 'c1', 'user', 'u', 'cluster.sudo', 3600)`,
+	} {
+		if _, err := pool.Exec(ctx, q); err != nil {
+			t.Fatal(err)
+		}
+	}
+	h := migrated(t, pool)
+	done := make(chan error, 1)
+	go func() { done <- rbac.Migrate(ctx, pool) }()
+	for deadline := time.Now().Add(10 * time.Second); ; time.Sleep(time.Millisecond) {
+		var building bool
+		if err := pool.QueryRow(ctx, `SELECT EXISTS (SELECT FROM pg_stat_progress_create_index WHERE relid = 'rbac_grants'::regclass)`).Scan(&building); err != nil {
+			t.Fatal(err)
+		}
+		if building {
+			break
+		}
+		select {
+		case err := <-done:
+			t.Fatalf("migrated before the grants' index was seen building: %v", err)
+		default:
+		}
+		if time.Now().After(deadline) {
+			t.Fatal("migrating never built the grants' index")
+		}
+	}
+	if err := h.authz.DeleteScope(ctx, host, "t1", cluster.Scope()); err != nil {
+		t.Errorf("deleting a scope while migrating builds the grants' index: %v", err)
+	}
+	if err := <-done; err != nil {
+		t.Errorf("Migrate while a scope is deleted: %v", err)
+	}
+}
+
+// As of a time, Filter, its condition, Reachable and Holders seek their subject's grants, and the
+// public's and the members' they count, on each listed type, in a history of one tenant or of many,
+// however their statements are planned: history's index of a principal's versions held no scope, so
+// a plan read every version of a principal's grants once for each type, a group's on thousands of
+// volumes for a list of clusters; and by history's index of scopes, every version on a type's
+// scopes in the tenant, the members' grants on thousands of clusters for the anonymous or a guest,
+// who count none of them.
+func TestAsOfSearchesSeekTheirPrincipalsGrantsOnEachType(t *testing.T) {
+	volumes := rbac.RoleDefinition{ID: "volume.admin", Permissions: []rbac.Permission{"volume:view"}, ResourceTypes: []string{"volume"}}
+	withVolumes := func(c *rbac.Config) { c.Catalog = append(slices.Clone(c.Catalog), volumes) }
+	for _, tenants := range []string{"one tenant", "many tenants"} {
+		for _, plans := range []string{"", "force_generic_plan"} {
+			t.Run(tenants+"/"+cmp.Or(plans, "as AsOf plans"), func(t *testing.T) {
+				var h *harness
+				if tenants == "many tenants" {
+					h = crowded(t, pgtest.Open(t, ""), withVolumes)
+				} else {
+					h = setup(t, withVolumes)
+				}
+				ctx := t.Context()
+				c0 := hostTable(t, h)[0]
+				g := h.guest(t, "g", "builders")
+				h.seed(t, grant(c0.Scope(), "cluster.login", rbac.User, "g"))
+				granted := 0
+				more := func(n int) {
+					t.Helper()
+					if _, err := h.pool.Exec(ctx, `INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role)
+						SELECT 't1', 'cluster', 'k' || i, 'members', 't1', 'cluster.login' FROM generate_series($1::int + 1, $2::int) i
+						UNION ALL SELECT 't1', 'volume', 'v' || i, 'group', 'builders', 'volume.admin' FROM generate_series($1::int + 1, $2::int) i`,
+						granted, n); err != nil {
+						t.Fatal(err)
+					}
+					granted = n
+					if _, err := h.pool.Exec(ctx, `ANALYZE rbac_grants, rbac_history`); err != nil {
+						t.Fatal(err)
+					}
+				}
+				read := func(at time.Time) map[string]int64 {
+					t.Helper()
+					pages := map[string]int64{}
+					searches := map[string]func(pgx.Tx, *rbac.Authorizer) error{
+						"Holders": func(_ pgx.Tx, a *rbac.Authorizer) error {
+							hs, err := a.Holders(ctx, view, c0)
+							if err == nil && !slices.Equal(holderPrincipals(hs), []rbac.Principal{g.Principal}) {
+								err = fmt.Errorf("Holders = %+v", hs)
+							}
+							return err
+						},
+					}
+					for who, c := range map[string]struct {
+						s    rbac.Subject
+						kept []string
+					}{"anonymous": {rbac.Subject{}, nil}, "guest": {g, []string{c0.ID}}} {
+						searches[who+" Filter"] = func(tx pgx.Tx, a *rbac.Authorizer) error {
+							pred, err := a.Filter(ctx, c.s, view, "t1", "cluster", rbac.Columns{Tenant: "tenant", Path: "path"}, 1)
+							if err != nil {
+								return err
+							}
+							rows, err := tx.Query(ctx, `SELECT id FROM clusters WHERE `+pred.SQL+` ORDER BY id`, pred.Args...)
+							if err != nil {
+								return err
+							}
+							ids, err := pgx.CollectRows(rows, pgx.RowTo[string])
+							if err == nil && !slices.Equal(ids, c.kept) {
+								err = fmt.Errorf("Filter kept %v", ids)
+							}
+							return err
+						}
+						searches[who+" Reachable"] = func(_ pgx.Tx, a *rbac.Authorizer) error {
+							reach, err := a.Reachable(ctx, c.s, view, "t1", "cluster")
+							if err == nil && len(reach.Scopes) != len(c.kept) {
+								err = fmt.Errorf("Reachable = %+v", reach)
+							}
+							return err
+						}
+					}
+					for what, f := range searches {
+						func() {
+							tx, err := h.pool.Begin(ctx)
+							if err != nil {
+								t.Fatal(err)
+							}
+							defer func() { _ = tx.Rollback(ctx) }()
+							if err := h.authz.WithTx(tx).AsOf(ctx, "t1", at, func(a *rbac.Authorizer) error {
+								if plans != "" {
+									if _, err := tx.Exec(ctx, `SELECT set_config('plan_cache_mode', $1, true)`, plans); err != nil {
+										return err
+									}
+								}
+								start := historyPages(ctx, t, tx)
+								err := f(tx, a)
+								pages[what] = historyPages(ctx, t, tx) - start
+								return err
+							}); err != nil {
+								t.Fatalf("%s: %v", what, err)
+							}
+						}()
+					}
+					return pages
+				}
+				more(500)
+				few := read(dbNow(t, h))
+				more(5000)
+				for what, many := range read(dbNow(t, h)) {
+					if many > few[what]*3/2+deeper {
+						t.Errorf("as of a time, %s read %d pages of history beside 500 grants of the members' on clusters and of a group of the guest's on volumes, and %d beside 5000", what, few[what], many)
+					}
+				}
+			})
+		}
+	}
+}
+
 // As of a time, a list of the grants or the eligibilities on a scope reads their history, not every
 // grant's and eligibility's in the tenant.
 func TestAsOfListsAScopesGrantsAndEligibilitiesAlone(t *testing.T) {