diff --git a/README.md b/README.md index 2c69adb..6f841a7 100644 --- a/README.md +++ b/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 diff --git a/asof_test.go b/asof_test.go index b310b08..79d9fe0 100644 --- a/asof_test.go +++ b/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) { diff --git a/authority_test.go b/authority_test.go index f497295..d5c4e8a 100644 --- a/authority_test.go +++ b/authority_test.go @@ -3397,48 +3397,112 @@ func TestChecksReadTheirGrantsFromAnIndexAlone(t *testing.T) { SELECT 't1', 'tenant', 't1', 'user', id, 'cluster.login' FROM others`, "rbac_grants") } } - // So the visibility map lets a scan of an index skip the rows. VACUUM marks none visible to - // all while an older snapshot elsewhere in the database, such as another run's, is open. - for deadline := time.Now().Add(30 * time.Second); ; time.Sleep(time.Second) { - if _, err := h.pool.Exec(ctx, `VACUUM rbac_grants`); err != nil { - t.Fatal(err) - } - var visible bool - if err := h.pool.QueryRow(ctx, `SELECT relallvisible >= relpages FROM pg_class WHERE oid = 'rbac_grants'::regclass`).Scan(&visible); err != nil { - t.Fatal(err) - } - if visible { - break - } - if time.Now().After(deadline) { - t.Skip("an older snapshot elsewhere in the database keeps rbac_grants' rows from being visible to all") - } - } + grantsVisibleToAll(t, h) tx, err := h.pool.Begin(ctx) if err != nil { t.Fatal(err) } defer func() { _ = tx.Rollback(ctx) }() - // The pages of the table itself, not its indexes: its rows and its visibility map. - rows := func() int64 { - t.Helper() - var n int64 - if err := tx.QueryRow(ctx, `SELECT pg_stat_get_xact_blocks_fetched('rbac_grants'::regclass)`).Scan(&n); err != nil { - t.Fatal(err) - } - return n - } - start := rows() + start := grantRowsRead(ctx, t, tx) if err := h.authz.WithTx(tx).Authorize(ctx, ann, login, cluster); err != nil { t.Fatal(err) } - if read := rows() - start; read > int64(held/4) { + if read := grantRowsRead(ctx, t, tx) - start; read > int64(held/4) { t.Errorf("a check of a subject holding %d grants read %d pages of the table's rows", held, read) } }) } } +// grantsVisibleToAll vacuums rbac_grants until its visibility map marks every row visible to all, so +// a scan of an index can skip the rows. VACUUM marks none so while an older snapshot elsewhere in +// the database, such as another run's, is open: then it skips the test. +func grantsVisibleToAll(t *testing.T, h *harness) { + t.Helper() + for deadline := time.Now().Add(30 * time.Second); ; time.Sleep(time.Second) { + if _, err := h.pool.Exec(t.Context(), `VACUUM rbac_grants`); err != nil { + t.Fatal(err) + } + var visible bool + if err := h.pool.QueryRow(t.Context(), `SELECT relallvisible >= relpages FROM pg_class WHERE oid = 'rbac_grants'::regclass`).Scan(&visible); err != nil { + t.Fatal(err) + } + if visible { + return + } + if time.Now().After(deadline) { + t.Skip("an older snapshot elsewhere in the database keeps rbac_grants' rows from being visible to all") + } + } +} + +// grantRowsRead is how many pages of rbac_grants itself, not its indexes, tx has read: its rows and +// its visibility map. +func grantRowsRead(ctx context.Context, t *testing.T, tx pgx.Tx) int64 { + t.Helper() + var n int64 + if err := tx.QueryRow(ctx, `SELECT pg_stat_get_xact_blocks_fetched('rbac_grants'::regclass)`).Scan(&n); err != nil { + t.Fatal(err) + } + return n +} + +// Filter, its condition in the host's query, and Reachable read their subject's grants from an index +// alone, under a generic plan too, as a check does: they told the broad principals' grants by their +// kind, which no index of the grants holds, so they read the row of each grant of the subject's on +// the listed types, as one on each of thousands of clusters it created, which grants made at +// different times have on pages of their own. +func TestListingsReadTheirGrantsFromAnIndexAlone(t *testing.T) { + for _, plan := range []string{"force_custom_plan", "force_generic_plan"} { + t.Run(plan, func(t *testing.T) { + h := crowded(t, pgtest.Open(t, "plan_cache_mode="+plan)) + ctx := t.Context() + hostTable(t, h) + u := h.as(t, "u") + // u's grants, each made beside others' grants, as grants made over time are. + const held = 1000 + manyMembers(t, h, held, `INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role) + SELECT 't1', 'cluster', id, 'user', id, 'cluster.login' FROM others + UNION ALL SELECT 't1', 'cluster', id, 'user', 'u', 'cluster.login' FROM others`, "rbac_grants") + grantsVisibleToAll(t, h) + tx, err := h.pool.Begin(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback(ctx) }() + a := h.authz.WithTx(tx) + for _, c := range []struct { + what string + f func() error + }{ + {"Filter and its condition", func() error { + pred, err := a.Filter(ctx, u, login, "t1", "cluster", rbac.Columns{Tenant: "tenant", Path: "path"}, 1) + if err != nil { + return err + } + var n int + return tx.QueryRow(ctx, `SELECT count(*) FROM clusters WHERE `+pred.SQL, pred.Args...).Scan(&n) + }}, + {"Reachable", func() error { + reach, err := a.Reachable(ctx, u, login, "t1", "cluster") + if err == nil && len(reach.Scopes) != held { + err = fmt.Errorf("Reachable reached %d scopes", len(reach.Scopes)) + } + return err + }}, + } { + start := grantRowsRead(ctx, t, tx) + if err := c.f(); err != nil { + t.Fatalf("%s: %v", c.what, err) + } + if read := grantRowsRead(ctx, t, tx) - start; read > held/100 { + t.Errorf("%s of a subject holding %d grants read %d pages of the table's rows", c.what, held, read) + } + } + }) + } +} + // Filter, its condition in the host's query, and Reachable read, of the grants, only the subject's // principals' on scopes of the types a resource's path holds, a type at a time, under a generic plan // too: of a range of each type's scopes, a plan can seek only one, and reading every grant of the @@ -3621,6 +3685,120 @@ func TestSearchesReadAsMuchOnceAConnectionPlansThemGenerically(t *testing.T) { } } +// Filter, Reachable and Holders read, of the grants to the tenant's members and the public, only +// those of custom roles that their subject counts, under a generic plan and one for their arguments, +// and as of a time: which custom roles those grants name was read from each of them on scopes of the +// listed types, of built-in roles too, so beside the members' grants on thousands of clusters a +// member's search read them all, and those of the anonymous and a guest, who count none of them; and +// a search of a permission no public role gives read the public's. +func TestSearchesReadOnlyTheBroadGrantsTheyCount(t *testing.T) { + for _, plan := range []string{"force_custom_plan", "force_generic_plan"} { + for _, broad := range []struct { + what string + // grants are the grants on clusters, of the numbers from $1 to $2. + grants string + p rbac.Permission + // counting are the searches that count the grants, and so read them. + counting []string + }{ + // Read by the members' key, or by the custom roles' index, either reads all of one or the + // other. + { + what: "the members' of a built-in role and others' of a custom role", + grants: `SELECT 't1', 'cluster', 'k' || i, 'members', 't1', 'cluster.login' FROM generate_series($1::int + 1, $2::int) i + UNION ALL SELECT 't1', 'cluster', 'o' || i, 'user', 'o' || i, 'custom.view' FROM generate_series($1::int + 1, $2::int) i`, + p: login, counting: []string{"member Reachable"}, + }, + { + what: "the members' of a custom role", + grants: `SELECT 't1', 'cluster', 'k' || i, 'members', 't1', 'custom.view' FROM generate_series($1::int + 1, $2::int) i`, + p: login, counting: []string{"member Filter", "member Reachable"}, + }, + { + what: "the public's of a public custom role, searched for a permission no public role gives", + grants: `SELECT 't1', 'cluster', 'k' || i, 'public', '', 'custom.open' FROM generate_series($1::int + 1, $2::int) i`, + p: root, + }, + } { + t.Run(plan+"/"+broad.what, func(t *testing.T) { + h := crowded(t, pgtest.Open(t, "plan_cache_mode="+plan)) + ctx := t.Context() + c0 := hostTable(t, h)[0] + g, m := h.guest(t, "g"), h.as(t, "m") + h.seed(t, grant(c0.Scope(), "cluster.login", rbac.User, "g"), grant(c0.Scope(), "cluster.login", rbac.User, "m")) + if _, err := h.pool.Exec(ctx, `INSERT INTO rbac_roles (tenant, id, permissions, resource_types, public) + VALUES ('t1', 'custom.view', '{cluster:view,cluster:login}', '{cluster}', false), ('t1', 'custom.open', '{cluster:view}', '{cluster}', true)`); err != nil { + t.Fatal(err) + } + searches := map[string]func(pgx.Tx, *rbac.Authorizer) error{ + "Holders": func(_ pgx.Tx, a *rbac.Authorizer) error { return second(a.Holders(ctx, broad.p, c0)) }, + } + for who, s := range map[string]rbac.Subject{"anonymous": {}, "guest": g, "member": m} { + if !slices.Contains(broad.counting, who+" Filter") { + searches[who+" Filter"] = func(_ pgx.Tx, a *rbac.Authorizer) error { + return second(a.Filter(ctx, s, broad.p, "t1", "cluster", rbac.Columns{Tenant: "tenant", Path: "path"}, 1)) + } + } + if !slices.Contains(broad.counting, who+" Reachable") { + searches[who+" Reachable"] = func(_ pgx.Tx, a *rbac.Authorizer) error { return second(a.Reachable(ctx, s, broad.p, "t1", "cluster")) } + } + } + 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) `+broad.grants, 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) + } + } + // The pages of the grants each reads now, and, planned for its arguments as AsOf plans + // it, of history as of now. + read := func(at time.Time) map[string]int64 { + t.Helper() + pages := map[string]int64{} + for what, f := range searches { + func() { + tx, err := h.pool.Begin(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback(ctx) }() + start := pagesRead(ctx, t, tx, "rbac_grants") + if err := f(tx, h.authz.WithTx(tx)); err != nil { + t.Fatalf("%s: %v", what, err) + } + pages[what] = pagesRead(ctx, t, tx, "rbac_grants") - start + if plan != "force_custom_plan" || strings.HasSuffix(what, "Filter") { + return + } + if err := h.authz.WithTx(tx).AsOf(ctx, "t1", at, func(a *rbac.Authorizer) error { + start := historyPages(ctx, t, tx) + err := f(tx, a) + pages[what+" as of a time"] = historyPages(ctx, t, tx) - start + return err + }); err != nil { + t.Fatalf("%s as of a time: %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("%s read %d pages beside 500 grants on clusters that are %s, and %d beside 5000", what, few[what], broad.what, many) + } + } + }) + } + } +} + // Filter's count of the grants that pick its form stops a little past where the form changes, // however many of a built-in role its subject holds, and beside a grant of a custom role read // before them: counted with the custom roles the grants name, which it reads from every one, the diff --git a/authorize.go b/authorize.go index a3544c5..4de1d40 100644 --- a/authorize.go +++ b/authorize.go @@ -749,7 +749,8 @@ func (a *Authorizer) holders(ctx context.Context, p Permission, r Resource, role // One time for every statement, as one snapshot, so a grant, a membership or a group's member // ending between them can't leave a holder that no single moment had. now := a.cfg.Now() - through, err := a.broadArgs(ctx, a.db, r.Tenant, p, r.Type, roles, custom, broadAt, r.path()) + through, err := a.broadArgs(ctx, a.db, r.Tenant, p, r.Type, roles, custom, a.broadAt, r.path(), + broadKeys(r.Tenant, a.counting(p, r.Type)), "", time.Time{}) if err != nil { return nil, err } @@ -1086,28 +1087,30 @@ func (a *Authorizer) reachable(ctx context.Context, s Subject, p Permission, ten if err != nil { return Reach{}, err } - principals := countedOf(keys, a.counting(p, resourceType)) + counting := a.counting(p, resourceType) + principals := countedOf(keys, counting) if len(principals) == 0 { return Reach{}, nil } reaching := a.reaching(resourceType) - through, err := a.broadArgs(ctx, a.db, tenant, p, resourceType, roles, custom, broadOn, reaching) + everyone := slices.DeleteFunc(broadKeys(tenant, counting), func(k string) bool { return !slices.Contains(principals, k) }) + through, err := a.broadArgs(ctx, a.db, tenant, p, resourceType, roles, custom, a.broadOn, reaching, everyone, "", time.Time{}) if err != nil { return Reach{}, err } - args := append(append([]any{ + args := append([]any{ tenant, principals, roles, a.cfg.Now(), reaching, publicKey, a.publicRoles(roles), string(p), resourceType, - }, through...), broadKinds) + }, through...) // The subject's grants on scopes of the types are read once, and of the custom roles those that // they name (namedCustom). rows, err := a.db.Query(ctx, ` - WITH held AS MATERIALIZED (`+byType("$5", "g.scope, g.conditional, g.role, g.principal, g.principal_kind", ` + WITH held AS MATERIALIZED (`+a.byType("$5", "g.scope, g.conditional, g.role, g.principal", ` g.tenant = $1 AND g.principal = ANY($2::text[]) AND (g.expires_at IS NULL OR g.expires_at > $4)`)+`) SELECT scope, bool_and(conditional) FROM held WHERE role = ANY(`+givingSQL("$3", "$1", "$8", "$9", custom, namedCustom("SELECT role FROM held"), false)+`) AND (principal <> $6 OR role = ANY(`+givingSQL("$7", "$1", "$8", "$9", custom, "$10", true)+`)) - AND (principal_kind <> ALL($13::text[]) OR role = ANY(`+givingSQL("$11", "$1", "$8", "$9", custom, "$12", false)+`)) + AND (`+narrowKey("principal")+` OR role = ANY(`+givingSQL("$11", "$1", "$8", "$9", custom, "$12", false)+`)) GROUP BY scope ORDER BY scope`, args...) if err != nil { return Reach{}, err @@ -1217,14 +1220,16 @@ type Predicate struct { // tenant the subject may do p to. It reads the grants in the same statement as the rows, so // sorting and pagination work and nothing changes between deciding and reading. A condition holds // for the query it was built for: it fixes the time (Config.Now), the subject's label holding, what -// the constraints answered, and which custom roles granted to the public or the tenant's members -// give through them, when Filter was called, so a later query keeps rows Authorize then refuses, as -// once a grant it counted has expired, and leaves out what a custom role first granted to them -// since gives. Build one for each query. The host's query must reach this package's tables and call -// its function rbac_tenant_keys (they are in the pool's schema), and column names are trusted host -// code; every value is an argument. It counts only unconditional grants, and leaves out every -// resource a deny covers, conditional ones included, so it never keeps one Authorize refuses: -// Reachable's Conditional and ConditionalExcept name the scopes where that may be too strict. +// the constraints answered, and which custom roles granted to the public or, while the subject is +// one of them, the tenant's members give through them, when Filter was called, so a later query +// keeps rows Authorize then refuses, as once a grant it counted has expired, and leaves out what a +// custom role first granted to them since gives, or one granted to the members gives a subject +// that has since become one. Build one for each query. The host's query must reach this package's +// tables and call its function rbac_tenant_keys (they are in the pool's schema), and column names +// are trusted host code; every value is an argument. It counts only unconditional grants, and +// leaves out every resource a deny covers, conditional ones included, so it never keeps one +// Authorize refuses: Reachable's Conditional and ConditionalExcept name the scopes where that may +// be too strict. func (a *Authorizer) Filter(ctx context.Context, s Subject, p Permission, tenant, resourceType string, cols Columns, firstArg int) (Predicate, error) { a = a.still() if cols.Tenant == "" || cols.Path == "" || firstArg < 1 { @@ -1250,7 +1255,7 @@ func (a *Authorizer) Filter(ctx context.Context, s Subject, p Permission, tenant perm, wildcard = listed, covering } keys := func(of string) string { return filterKeys(arg, of) } - grants := func(types string) string { return filterGrants(arg, custom, types) } + grants := func(types string) string { return a.filterGrants(arg, custom, types) } // The denies on scopes of the types in the array types that bind the subject (bindingDenies). narrow := counted("FALSE", "FALSE") denies := func(types string) string { @@ -1295,8 +1300,12 @@ func (a *Authorizer) Filter(ctx context.Context, s Subject, p Permission, tenant now := a.cfg.Now() // Which custom roles give p through the public and the tenant's members is read now, and the // statement keeps of them those that still give it, and through the public those still public: - // one first granted to them later gives nothing through them. - through, err := a.broadArgs(ctx, a.db, tenant, p, resourceType, roles, custom, broadOn, a.reaching(resourceType)) + // one first granted to them later gives nothing through them. Through the members, only while + // the subject is one: the anonymous never is. + counted := counts + counted.members = counted.members && s.Principal != (Principal{}) + through, err := a.broadArgs(ctx, a.db, tenant, p, resourceType, roles, custom, a.broadOn, a.reaching(resourceType), + broadKeys(tenant, counted), s.identityKey(), now) if err != nil { return Predicate{}, err } @@ -1351,21 +1360,43 @@ func (a *Authorizer) Filter(ctx context.Context, s Subject, p Permission, tenant // to a principal seek for each of its principals. Ranges ORed are none an index can seek, so a plan // reads every grant of the principals in the tenant, as a group's on thousands of resources of // another type. OFFSET 0 keeps the planner from flattening it into a join it could run the other -// way round. -func byType(types, cols, where string) string { +// way round. As of a time a scope's type is tested as its text up to its colon, an equality +// history's index of a principal's versions seeks: a range whose bounds are another table's, as the +// types' here, a plan counts as a few rows, so in a history of a few principals, where it counts a +// principal's versions as a large share, it read every version on a type's scopes in the tenant by +// history's index of scopes, the members' too, to find the public's few. +func (a *Authorizer) byType(types, cols, where string) string { + of := ofType("g.scope", "t.t") + if a.history != "" { + of = "split_part(g.scope, ':', 1) = t.t" + } return fmt.Sprintf(`SELECT %[2]s FROM unnest(%[1]s::text[]) AS t(t) CROSS JOIN LATERAL ( - SELECT %[2]s FROM rbac_grants g WHERE %[3]s AND %[4]s OFFSET 0) g`, types, cols, where, ofType("g.scope", "t.t")) + SELECT %[2]s FROM rbac_grants g WHERE %[3]s AND %[4]s OFFSET 0) g`, types, cols, where, of) } -// broadOn is SQL selecting, for broadArgs, the roles of the grants in the tenant $1 to the principal -// whose key is principal on scopes of the types in the array $4. -func broadOn(principal string) string { - return byType("$4", "g.role", "g.tenant = $1 AND g.principal = "+principal) +// broadOn is SQL selecting, for broadArgs, the roles of the grants of custom roles in the tenant $1 +// to the principal whose key is principal on scopes of the types in the array $4, a type at a time. +// Each is a range of rbac_grants_custom_principal, which holds no grant of a built-in role and leads +// to the principal, the tenant and the scope, so no plan reads them cheaper by the custom roles' +// index on the tenant alone. As of a time it selects the grants of every role, the prefix tested +// apart (namedCustom): history's index of a grant's role leads to the tenant and the role, so a plan +// for the prefix could read every version of every grant of a custom role in the tenant. +func (a *Authorizer) broadOn(principal string) string { + return a.byType("$4", "g.role", "g.tenant = $1 AND g.principal = "+principal+a.customOnly("g.role")) } -// broadAt is broadOn for the scopes in the array $4. -func broadAt(principal string) string { - return byKey("$4", "rbac_grants", "scope", "role", "tenant = $1 AND principal = "+principal) +// broadAt is broadOn for the scopes in the array $4, a scope at a time. +func (a *Authorizer) broadAt(principal string) string { + return byKey("$4", "rbac_grants", "scope", "role", "tenant = $1 AND principal = "+principal+a.customOnly("role")) +} + +// customOnly is SQL keeping, but as of a time, the grants whose role, col, is a custom one, for +// broadOn and broadAt. +func (a *Authorizer) customOnly(col string) string { + if a.history != "" { + return "" + } + return " AND " + col + " LIKE '" + CustomPrefix + "%'" } // byKey is SQL selecting cols of the rows of table for which where holds whose col is one of the @@ -1460,14 +1491,14 @@ func filterKeys(arg func(int) string, of string) string { // that give the permission, the public's only of public ones and the broad principals' of none // privileged. It reads the subject's grants there once (filterHeld), and of the custom roles those // that they name (namedCustom). arg numbers Filter's arguments. -func filterGrants(arg func(int) string, custom bool, types string) string { - return `WITH held AS MATERIALIZED (` + filterHeld(arg, types) + `) +func (a *Authorizer) filterGrants(arg func(int) string, custom bool, types string) string { + return `WITH held AS MATERIALIZED (` + a.filterHeld(arg, types) + `) SELECT scope COLLATE "default" FROM held WHERE ` + filterCounted(arg, custom) } // filterHeld is SQL selecting the subject's grants filterGrants reads, as held. -func filterHeld(arg func(int) string, types string) string { - return byType(types, "g.scope, g.role, g.principal, g.principal_kind", fmt.Sprintf(`g.tenant = %s::text AND g.principal = ANY(%s) +func (a *Authorizer) filterHeld(arg func(int) string, types string) string { + return a.byType(types, "g.scope, g.role, g.principal", fmt.Sprintf(`g.tenant = %s::text AND g.principal = ANY(%s) AND (g.expires_at IS NULL OR g.expires_at > %s) AND NOT g.conditional`, arg(1), filterKeys(arg, countedKeys(arg(13), arg(17))), arg(4))) } @@ -1475,10 +1506,10 @@ func filterHeld(arg func(int) string, types string) string { func filterCounted(arg func(int) string, custom bool) string { return fmt.Sprintf(`role = ANY(%s) AND (principal <> %s::text OR role = ANY(%s)) - AND (principal_kind <> ALL(%s) OR role = ANY(%s))`, + AND (%s OR role = ANY(%s))`, givingSQL(arg(3), arg(1), arg(8), arg(9), custom, namedCustom("SELECT role FROM held"), false), arg(6), givingSQL(arg(7), arg(1), arg(8), arg(9), custom, arg(10), true), - broadKindsSQL, givingSQL(arg(11), arg(1), arg(8), arg(9), custom, arg(12), false)) + narrowKey("principal"), givingSQL(arg(11), arg(1), arg(8), arg(9), custom, arg(12), false)) } // grantCounts counts, with Filter's arguments args, the grants its condition counts (filterGrants) @@ -1504,12 +1535,12 @@ func (a *Authorizer) grantCounts(ctx context.Context, custom, container bool, ar // form. Both counts read one materialized held, so the subject's grants are read once. count := func(types string, limit int) string { if !custom { - return fmt.Sprintf(`(SELECT count(*) FROM (%s LIMIT %d) g)`, filterGrants(arg, false, types), limit) + return fmt.Sprintf(`(SELECT count(*) FROM (%s LIMIT %d) g)`, a.filterGrants(arg, false, types), limit) } return fmt.Sprintf(`(WITH held AS MATERIALIZED (%[1]s) SELECT CASE WHEN (SELECT count(*) FROM (SELECT FROM held WHERE %[2]s LIMIT %[4]d) g) = %[4]d THEN %[4]d ELSE (SELECT count(*) FROM (SELECT FROM held WHERE %[3]s LIMIT %[4]d) g) END)`, - filterHeld(arg, types), filterCounted(arg, false), filterCounted(arg, true), limit) + a.filterHeld(arg, types), filterCounted(arg, false), filterCounted(arg, true), limit) } onResources := "0" if !container { diff --git a/docs/architecture.md b/docs/architecture.md index 0ad67de..d26e796 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -208,7 +208,11 @@ erDiagram grants is an index-only scan. The indexes by scope and by principal lead to both too and hold the same, so a connection's cached statement, once PostgreSQL plans it generically by the tables' averages, still reads only the subject's grants on the path, from the index alone, whichever - index it picks. + index it picks. A search decides which custom roles the public's and the tenant's members' grants + name before its statement, from `rbac_grants_custom_principal`, which holds only grants of custom + roles, by principal and scope, and reads only those its subject counts: the anonymous and a guest + read none of the members', and a search of a permission no public role gives none of the + public's, so their grants of built-in roles on thousands of resources cost it nothing. - **A read of a list of keys reads each key's rows on its own** (`byKey`), as of a list's scopes or principals, of the denies on a check's path, of `Activate`'s eligibilities and of the rows `DeleteScopes`, `DeletePrincipal` and `DeleteGroups` remove, and so does a walk through nested @@ -665,9 +669,11 @@ flowchart LR - **AsOf** stands temporary views of history in for the tables, in a transaction it rolls back, so every check, list and search answers as it would have at a past time, with the catalog and the label dimensions as they are now. Each statement reads history by the rows it names, through - history's indexes, as a search the custom roles its grants name, by their ids. The view of - custom roles takes each one's newest version, testing none for a later one, so `Grantable`, - which can offer any, reads each version once. + history's indexes, as a search the custom roles its grants name, by their ids, and its principals' + grants by principal and type, which history's index of a principal's versions holds beside the + scope, as a check seeks them on its resource's path. The view of custom roles takes each one's + newest version, testing none for a later one, so `Grantable`, which can offer any, reads each + version once. - **PruneHistory** deletes only what an export has read and later versions supersede. It keeps each row's last version at or before its time, so AsOf answers as before for any later time, and refuses an earlier one; of a row whose last version by then ended it, it deletes every diff --git a/migrations/0019_principal_seeks.sql b/migrations/0019_principal_seeks.sql new file mode 100644 index 0000000..11f870b --- /dev/null +++ b/migrations/0019_principal_seeks.sql @@ -0,0 +1,38 @@ +-- Both tables are locked first, history last, as 0013 locks them: locked as each statement reached +-- it, history would be locked only once the grants' index was built, and a change that wrote history +-- meanwhile, as each change to a table does, and then wrote a grant, as deleting a scope's +-- eligibilities and then its grants does, would wait for migrating while migrating waited for it, so +-- migrating would give way, or deadlock, having built the index. Reads of rbac_grants go on. +LOCK TABLE rbac_grants IN SHARE MODE; +LOCK TABLE rbac_history IN ACCESS EXCLUSIVE MODE; + +-- Filter, Reachable and Holders read which custom roles the public's and the tenant's members' +-- grants on the scopes they search name. rbac_grants_principal finds those grants with every other +-- grant of theirs there, of built-in roles too, which the tenant's members can hold on thousands of +-- resources; and rbac_grants_custom_role leads to the tenant alone, which a generic plan costs as an +-- average tenant's few grants, so it read every grant of a custom role in the tenant. This one +-- holds only grants of custom roles and leads to a principal, a tenant and a scope, so a search +-- seeks just those of the principal on a scope or a range of a type's, from the index alone, and no +-- plan finds them cheaper through the tenant. Building it keeps writes to rbac_grants waiting until +-- the migration commits, though reads go on. +CREATE INDEX rbac_grants_custom_principal ON rbac_grants (principal, tenant, scope) INCLUDE (role) WHERE role LIKE 'custom.%'; + +-- As of a time, a search reads its subject's grants, and the public's and the tenant's members', on +-- scopes of the types it lists, and a check its subject's on its resource's path, but history's index +-- of a principal's versions held no scope: a plan read every version of a principal's grants, of +-- other types too, once for each type, or by history's index of scopes every version of every grant +-- on a type's scopes in the tenant, the members' too. This one leads to the principal, then to its +-- entity, its scope's type and its scope, so a plan seeks a principal's versions of a type or on a +-- scope, and it serves what the one it replaces did, which led to the same three columns. A search +-- names a type as a scope's text up to its colon, an equality: a range of a type's scopes whose +-- bounds are another table's, as a search's types are, a plan counts as a few rows, so where it +-- counts a principal's versions as many, as in a history of a few principals, it took the index of +-- scopes. Reads of history, and every change, each of which writes history, wait until the +-- migration commits, and only a transaction under way as it starts that read history, or changed a +-- table, and then writes a grant makes it give way, before it builds either index. Building it reads +-- all of history; the analysis after it gives a plan the statistics of the types, which autovacuum +-- would take long after migrating. +DROP INDEX rbac_history_principal; +CREATE INDEX rbac_history_principal ON rbac_history (tenant, principal, entity, (split_part(scope, ':', 1)), scope, effective_at DESC) + WHERE principal IS NOT NULL; +ANALYZE rbac_history; diff --git a/rbac.go b/rbac.go index dde1ac1..ea0da99 100644 --- a/rbac.go +++ b/rbac.go @@ -112,12 +112,15 @@ var publicKey = Principal{Kind: Public}.key() // public. func (p Principal) broad() bool { return p.Kind == Members || p.Kind == Public } -// broadKinds are the kinds of the broad principals, as a statement's argument, and broadKindsSQL -// as SQL. -var ( - broadKinds = []string{string(Members), string(Public)} - broadKindsSQL = fmt.Sprintf(`ARRAY['%s', '%s']::text[]`, Members, Public) -) +// broadKinds are the kinds of the broad principals, as a statement's argument. +var broadKinds = []string{string(Members), string(Public)} + +// narrowKey is SQL for whether the principal key col is neither the public's nor a tenant's +// members', as its kind would say: the indexes leading to a principal hold its key, not its kind, so +// a read testing the kind fetches each grant's row. +func narrowKey(col string) string { + return fmt.Sprintf(`(%[1]s <> '%[2]s' AND %[1]s NOT LIKE '%[3]s:%%')`, col, publicKey, Members) +} // wellFormed reports whether p could hold grants in a tenant, whether or not its kind is // declared, as a revoke needs. diff --git a/roles.go b/roles.go index c216675..f2f3eb6 100644 --- a/roles.go +++ b/roles.go @@ -205,29 +205,39 @@ func givingSQL(builtin, tenant, perm, resourceType string, custom bool, ids stri // through grants to the public and the tenant's members: the custom ones that are public, NULL // where no grant to the public gives p, as Filter and Reachable then read none and Holders drops // the public; of roles, the built-in ones that aren't privileged; and the custom ones that aren't. -// The custom ones are those giving p there that grants to the public or the tenant's members name -// on the scopes, or scopes of the types, on: only for those grants does it matter. grants is SQL -// selecting the roles of the grants in the tenant $1 to the principal whose key is the SQL it is -// given there, on is the array $4 (broadOn, broadAt). They are decided as Authorizer.current reads -// them, as checks read them; pairing each one's permissions and types with the catalog's in SQL -// took a search seconds. +// The custom ones are those giving p there that the grants on the scopes, or scopes of the types, +// on name of the broad principals in keys, those whose grants the search counts. Only for those +// grants does it matter, and either principal's can be on thousands of resources, though the +// anonymous and a guest count none of the members', and a search of a permission no public role +// gives none of the public's. With identity, the tenant's members in keys are read only while the +// subject of that key is one of them at now, as keysSQL counts them, for a search that reads its +// subject's keys in its own statement (Filter). grants is SQL selecting the roles of the grants in +// the tenant $1 to the principal whose key is the SQL it is given there, on is the array $4 +// (Authorizer.broadOn, Authorizer.broadAt). They are decided as Authorizer.current reads them, as +// checks read them; pairing each one's permissions and types with the catalog's in SQL took a +// search seconds. func (a *Authorizer) broadArgs(ctx context.Context, q DB, tenant string, p Permission, resourceType string, roles []string, custom bool, - grants func(principal string) string, on []string, + grants func(principal string) string, on, keys []string, identity string, now time.Time, ) ([]any, error) { unprivileged := slices.DeleteFunc(slices.Clone(roles), func(id string) bool { return a.roles[Role(id)].Privileged }) // A grant to either never gives a permission to change access, so no search counts one for it. - if !custom || p.managesAccess() { + if !custom || p.managesAccess() || len(keys) == 0 { return []any{nil, unprivileged, nil}, nil } - // A broad key and a scope or a type at a time, the prefix tested apart (namedCustom), and the - // roles they name by key: matched to both keys at once, and tested for the prefix there, a - // generic plan read the grants by the custom roles' index on the tenant alone, every grant of one - // in the tenant for each scope or type; and as of a time, roles tested against the ids rather - // than read by them are each read at its newest version. - rows, err := q.Query(ctx, `SELECT `+roleColumns+` FROM (`+byKey(namedCustom(`SELECT g.role FROM unnest($5::text[]) AS b(k) + in, args := "$5::text[]", []any{tenant, string(p), resourceType, on, keys} + if identity != "" { + in = `ARRAY(SELECT k FROM unnest($5::text[]) AS c(k) WHERE k NOT LIKE 'members:%' OR EXISTS (SELECT FROM rbac_memberships + WHERE tenant = $1 AND principal = $6 AND (expires_at IS NULL OR expires_at > $7) AND NOT guest AND NOT suspended))` + args = append(args, identity, now) + } + // A broad key and a scope or a type at a time, and the roles they name by key: matched to both + // keys at once, a generic plan read the grants by the custom roles' index on the tenant alone, + // every grant of one in the tenant for each scope or type; and as of a time, roles tested + // against the ids rather than read by them are each read at its newest version. + rows, err := q.Query(ctx, `SELECT `+roleColumns+` FROM (`+byKey(namedCustom(`SELECT g.role FROM unnest(`+in+`) AS b(k) CROSS JOIN LATERAL (`+grants("b.k")+`) g`), "rbac_roles", "id", roleColumns, "tenant = $1 AND $2::text = ANY(permissions) AND $3::text = ANY(resource_types)")+`) r - ORDER BY id`, tenant, string(p), resourceType, on, broadKeys(tenant)) + ORDER BY id`, args...) if err != nil { return nil, err } @@ -250,8 +260,11 @@ func (a *Authorizer) broadArgs(ctx context.Context, q DB, tenant string, p Permi return []any{public, unprivileged, unprivilegedCustom}, nil } -// broadKeys are the keys of a tenant's broad principals: the public and its members. -func broadKeys(tenant string) []string { return []string{publicKey, TenantMembers(tenant).key()} } +// broadKeys are the keys of a tenant's broad principals, the public and its members, whose grants a +// read counts. +func broadKeys(tenant string, b broad) []string { + return countedOf([]string{publicKey, TenantMembers(tenant).key()}, b) +} // namedCustom is SQL for an array of the custom roles, each once, among those grants selects, SQL // for a column role. It tests the prefix outside grants, behind OFFSET 0 or a materialized CTE