From 0fe0e012cec13abebc6d82ef3b625e88df30962f Mon Sep 17 00:00:00 2001 From: James Greenhill Date: Tue, 1 Sep 2026 01:26:39 +0000 Subject: [PATCH 1/2] Let every duckgres login authenticate to Trino A tenant's only Trino credential was its org root password: the projection joined duckgres_org_users on username = 'root' and wrote one password.db line per org, so anyone wanting Trino access had to be handed a shared production credential. Every login an org already has now authenticates to Trino too, as ., with the bcrypt hash copied through unchanged - it is the same hash pgwire verifies, so one password works on both engines and nothing is re-hashed, minted or reset. The bare principal survives alongside them for service-to-service use and for clients configured before this existed. Trino's password file is ONE flat namespace per cell, while duckgres keys a login on (org, username) and recovers the org from SNI - which a Trino login carries no equivalent of. Qualifying the username is what makes that flat namespace safe, and it brings three consequences that are load-bearing rather than cosmetic: * Usernames are projected through an allowlist. duckgres validates a username as little more than "not empty", while password.db is : per line and group.db is :,. A username holding ':', ',' or a newline would let whoever can create org users append arbitrary lines to those files, including a line for the admin principal. * rejectPrincipalCollisions now also holds back orgs deriving the same Trino username. Valid database_names make that unreachable, but grandfathered rows may hold a dot, and a duplicate password.db line lets one org's user authenticate against another org's entry. * The resource-group selector captures only up to the first '.'. The previous (?.*) matched the whole username, so every login would get a private leaf carrying the full per-tenant limits and an org with ten logins would quietly hold ten times its concurrency and memory budget. Project-scoped logins keep their scope rather than being excluded or silently widened. Such a login joins scope__team_ instead of the org group; that group owns the same catalog in group_catalogs - so the cross-tenant check is the unchanged check - and carries a new group_scopes document narrowing it to the team's schemas and individually granted relations. Scopes only ever subtract, which is the property that keeps this off the tenant-isolation path: a bug in the new rules can widen access only within one org's own catalog. The scope is read through OrgUserQueryAccess, the same derivation the pgwire session path uses, so the two engines cannot disagree about it, and a scoped row whose scope will not resolve is dropped rather than projected unscoped. Scoped logins get no write authority at all. duckgres has a read-only project login and a read/write one, and only the read-only half is expressible here today, so denying writes to both narrows project_user rather than widening project_reader. Two lags are worth stating: a disabled user leaves password.db only when the projected Secret is re-read (kubelet sync plus the group provider's file.refresh-period), so the kill switch takes effect on Trino in up to a couple of minutes rather than instantly as it does on pgwire; and service credentials (duckgres_service_grants) are deliberately not projected here - their TTL and revocation semantics deserve their own change. --- CLAUDE.md | 27 ++ controlplane/configstore/models.go | 63 ++++ controlplane/configstore/trino.go | 98 +++++- controlplane/provisioner/opa/builder.go | 34 +- controlplane/provisioner/opa/builder_test.go | 132 +++++++- controlplane/provisioner/opa/latency_test.go | 2 +- controlplane/provisioner/opa/policy.rego | 147 +++++++- controlplane/provisioner/opa/policy_test.go | 316 +++++++++++++++++- controlplane/provisioner/opa/types.go | 64 +++- controlplane/provisioner/trino_provisioner.go | 261 +++++++++++++-- .../provisioner/trino_provisioner_test.go | 302 ++++++++++++++++- tests/configstore/trino_postgres_test.go | 131 ++++++++ 12 files changed, 1509 insertions(+), 68 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5e9f7fd6..5fc55f11 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1633,6 +1633,33 @@ password/tenant/catalog changes never propagate. `opa.ManagedCatalogPattern`, and the regex literal inside `policy.rego`. `TestTrinoCatalogNameMatchesManagedNamePattern` + `TestPolicyRegoContainsManagedNamePattern` fail if any one moves alone. +- **Every duckgres login authenticates to Trino, under `.`.** + `ListTrinoEnabledOrgs` returns the org's logins in `Users`, and + `BuildTrinoAuthFiles` writes one `password.db` line per login with the + bcrypt hash **copied through unchanged** — it is the same hash pgwire + verifies, so one password works on both engines and nothing is re-hashed or + minted. The bare `` principal survives alongside them. Three + rules that are load-bearing rather than cosmetic: (1) usernames are + projected through an **allowlist** (`trinoUsernamePattern`) because + duckgres barely validates them and a `:`, `,` or newline would let whoever + can create org users append lines to `password.db` — including an admin + line; (2) `rejectPrincipalCollisions` now also holds back orgs that derive + the same Trino username, since the password file is ONE flat namespace per + cell and a duplicate line is a cross-tenant auth bug; (3) the resource-group + selector's `orgCaptureRegex` captures only up to the first `.`, or every + login gets a private leaf with the full per-tenant limits and an org with + ten logins holds ten times its budget. +- **A project-scoped login (`project_reader` / `project_user`) joins + `scope__team_`, NOT the org group.** The scope group owns the same + catalog in `group_catalogs` — so the cross-tenant check is the unchanged + check — and carries a `group_scopes` document that narrows it to that + team's schemas. Scopes only ever SUBTRACT; keep it that way if the rules are + restructured. The scope comes from `OrgUserQueryAccess`, the same derivation + the pgwire path uses, so Trino and DuckDB cannot disagree about it, and a + scoped row whose scope will not resolve is DROPPED rather than projected + unscoped. Scoped logins get **no write authority at all** — `project_user` + is read/write on pgwire and read-only here, a narrowing; making it writable + means gating writes per-schema, not per-catalog. - **The Rego policy is the tenant-isolation boundary.** The cell can assume every per-org duckling role, so nothing below OPA stops org A reading org B's catalog. Treat `provisioner/opa/policy.rego` as security review. diff --git a/controlplane/configstore/models.go b/controlplane/configstore/models.go index 5fdf1ed9..c71d2641 100644 --- a/controlplane/configstore/models.go +++ b/controlplane/configstore/models.go @@ -522,6 +522,69 @@ type TrinoEnabledOrg struct { CellID string RootPasswordHash string // bcrypt hash from OrgUser row where Username = "root" State ManagedWarehouseProvisioningState // current state at read time + // Users are the org's own duckgres logins, each of which authenticates + // to Trino under TrinoUserPrincipal(Username) with the very same bcrypt + // hash it uses at the pgwire handshake. Populated by a second query in + // ListTrinoEnabledOrgs, hence `gorm:"-"` -- it is not a column on the + // row the outer join scans into. + Users []TrinoOrgUser `gorm:"-"` +} + +// TrinoOrgUser is one of an org's duckgres logins, projected into the cell's +// password file so a person who already has a pgwire credential can use that +// same credential against Trino instead of sharing the org's root password. +type TrinoOrgUser struct { + Username string + // PasswordHash is duckgres_org_users.password, copied through unchanged. + // It is bcrypt at cost 10, which Trino's file authenticator accepts as + // is (its floor is cost 8), so ONE password works on both engines and no + // separate Trino credential is ever minted or stored. + PasswordHash string + // Scope, when non-nil, restricts the login to one project's schemas. + // Mirrors duckgres_org_users.access_mode's project_reader / project_user + // modes; nil means an unrestricted org-wide login. + // + // The value is whatever OrgUserQueryAccess reports for this user, so the + // scope Trino enforces and the scope pgwire enforces are the same object + // derived by the same code -- see ListTrinoEnabledOrgs. + Scope *OrgUserQueryAccess + // TeamID is the project a scoped login is bound to, and is what its Trino + // group is keyed on. Non-nil exactly when Scope is: the two are set + // together and a scoped row with no team is dropped rather than + // projected (the table's CHECK constraint already forbids that shape). + // + // Carried here rather than on OrgUserQueryAccess because that type is the + // pgwire session path's policy object and has no reason to grow a field + // only the Trino projection reads. + TeamID *int64 +} + +// TrinoPrincipalSeparator joins an org's principal to one of its usernames to +// form a cell-wide-unique Trino username. +// +// Trino's password file is ONE flat namespace per cell while duckgres keys a +// login on (org, username) and recovers the org from SNI -- which a Trino +// login carries no equivalent of. Two orgs may each have an `analyst`, and +// two identical password-file lines would let one org's user authenticate +// against the other's entry and land in the other's group. Qualifying the +// username is what makes the flat namespace safe. +// +// `.` is the separator because a valid database_name cannot contain one (see +// ValidateDatabaseName: a DNS label) and neither can a projectable username +// (see projectableTrinoUsername), so `.` splits unambiguously and +// the org prefix stays recoverable -- which the resource-group selector's +// named capture depends on. +const TrinoPrincipalSeparator = "." + +// TrinoUserPrincipal returns the Trino username for one of the org's duckgres +// logins: the org's own principal, the separator, then the duckgres username. +// Returns "" when either half is missing, which callers skip. +func (o TrinoEnabledOrg) TrinoUserPrincipal(username string) string { + principal := o.TrinoPrincipal() + if principal == "" || username == "" { + return "" + } + return principal + TrinoPrincipalSeparator + username } // TrinoPrincipal is the tenant's customer-facing identity in Trino: the diff --git a/controlplane/configstore/trino.go b/controlplane/configstore/trino.go index 70ce3faa..cf66d56f 100644 --- a/controlplane/configstore/trino.go +++ b/controlplane/configstore/trino.go @@ -3,6 +3,7 @@ package configstore import ( "errors" "fmt" + "log/slog" "time" "gorm.io/gorm" @@ -230,9 +231,15 @@ func (cs *ConfigStore) DisableTrino(orgID string) error { } // ListTrinoEnabledOrgs returns every org with ManagedWarehouseTrino.Enabled -// = true joined against its `root` OrgUser row. The provisioner needs the -// bcrypt hash to project the Trino password file, so this is a single join -// rather than two round-trips. +// = true joined against its `root` OrgUser row, each carrying the org's full +// set of projectable logins in Users. The provisioner needs the bcrypt hashes +// to project the Trino password file. +// +// The `root` join stays because database_name alone remains a principal in +// its own right (TrinoPrincipal) for service-to-service use and for every +// client configured before per-user logins existed. Users is the ADDITIONAL +// per-human projection; root therefore appears twice, as `` and as +// `.root`, and both authenticate against the same hash. // // Orgs that are Trino-enabled but have no `root` OrgUser are skipped — that // shape can't legitimately happen via the provisioning API (CreateOrgUser @@ -271,9 +278,94 @@ func (cs *ConfigStore) ListTrinoEnabledOrgs() ([]TrinoEnabledOrg, error) { if err != nil { return nil, fmt.Errorf("list trino-enabled orgs: %w", err) } + if len(out) == 0 { + return out, nil + } + if err := cs.attachTrinoOrgUsers(out); err != nil { + return nil, err + } return out, nil } +// trinoOrgUserRow is one (org, login) pair from the second listing query. +type trinoOrgUserRow struct { + OrgID string + Username string + Password string + AccessMode string + TeamID *int64 +} + +// attachTrinoOrgUsers loads every projectable duckgres login for the listed +// orgs and hangs it off the matching TrinoEnabledOrg. +// +// A second query rather than a widened join: the outer listing is one row per +// org and the provisioner's per-org steps (catalog, tenant password, state) +// all key on that shape, so fanning it out to one row per user would make +// every caller de-duplicate. One extra round trip per reconcile tick is not +// a cost worth that. +// +// Two rows are excluded in SQL, both fail-closed: +// +// - disabled = true. duckgres_org_users.disabled is the per-user kill +// switch. Trino learns about a flip only when the projected Secret is +// re-read (kubelet sync + the group provider's file.refresh-period), so +// a disable takes effect here in up to a couple of minutes rather than +// instantly as it does on pgwire. That lag is worth stating wherever the +// kill switch is surfaced to operators; it is not a reason to leave the +// row out of the exclusion. +// - a blank password. There is no hash to project and Trino would reject +// the line anyway. +// +// Project-scoped logins ARE included, and carry their scope. The scope is +// read through OrgUserQueryAccess -- the SAME derivation the pgwire session +// path uses -- so Trino and DuckDB can never disagree about which schemas a +// project login may read. A scoped row whose scope cannot be resolved is +// dropped rather than projected unscoped: an unresolvable scope must never +// silently widen into org-wide access. +func (cs *ConfigStore) attachTrinoOrgUsers(orgs []TrinoEnabledOrg) error { + ids := make([]string, 0, len(orgs)) + for _, o := range orgs { + ids = append(ids, o.OrgID) + } + var rows []trinoOrgUserRow + err := cs.db.Table("duckgres_org_users"). + Select("org_id, username, password, access_mode, team_id"). + Where("org_id IN ?", ids). + Where("disabled = ?", false). + Where("password <> ''"). + Order("org_id ASC, username ASC"). + Scan(&rows).Error + if err != nil { + return fmt.Errorf("list trino org users: %w", err) + } + + byOrg := make(map[string][]TrinoOrgUser, len(orgs)) + for _, r := range rows { + u := TrinoOrgUser{Username: r.Username, PasswordHash: r.Password} + if IsProjectScopedAccessMode(r.AccessMode) { + access, scoped := cs.OrgUserQueryAccess(r.OrgID, r.Username) + if !scoped || r.TeamID == nil { + // The row says scoped but the snapshot does not agree -- + // an unloaded or stale snapshot, or a user written since + // the last poll. Projecting it now would grant the whole + // org catalog to a login that must only see one project. + // Drop it; the next tick projects it once both agree. + slog.Warn("Trino: skipping project-scoped login whose scope is unresolved.", + "org", r.OrgID, "user", r.Username, "access_mode", r.AccessMode) + continue + } + u.Scope = &access + u.TeamID = r.TeamID + } + byOrg[r.OrgID] = append(byOrg[r.OrgID], u) + } + for i := range orgs { + orgs[i].Users = byOrg[orgs[i].OrgID] + } + return nil +} + // GetManagedWarehouseTrino reads the Trino row for an org. Returns // (nil, nil) when no row exists so callers can distinguish "never // configured" from a DB error. diff --git a/controlplane/provisioner/opa/builder.go b/controlplane/provisioner/opa/builder.go index 657611ae..863b4588 100644 --- a/controlplane/provisioner/opa/builder.go +++ b/controlplane/provisioner/opa/builder.go @@ -47,8 +47,14 @@ func NewBuilder() BundleBuilder { // activates a deny-everything policy (since no group owns any catalog). // That is the correct bootstrap behaviour: until the provisioner pushes // a populated GroupCatalogs, all customer queries are denied. -func (defaultBuilder) BuildBundle(gc GroupCatalogs) ([]byte, error) { - data, err := buildDataDocument(gc) +// +// gs carries the project scopes that narrow individual groups. A nil or empty +// GroupScopes means no group is scoped, which is the pre-scopes behaviour: a +// group reads the whole catalog it owns. Both documents are always emitted so +// the policy's `data.group_scopes[g]` lookup is undefined-on-missing-key +// rather than an error on a missing document. +func (defaultBuilder) BuildBundle(gc GroupCatalogs, gs GroupScopes) ([]byte, error) { + data, err := buildDataDocument(gc, gs) if err != nil { return nil, fmt.Errorf("build data document: %w", err) } @@ -56,7 +62,7 @@ func (defaultBuilder) BuildBundle(gc GroupCatalogs) ([]byte, error) { b := bundle.Bundle{ Manifest: bundle.Manifest{ Revision: bundleRevision, - Roots: &[]string{"trino", "group_catalogs"}, + Roots: &[]string{"trino", "group_catalogs", "group_scopes"}, }, Modules: []bundle.ModuleFile{ { @@ -85,26 +91,30 @@ func (defaultBuilder) BuildBundle(gc GroupCatalogs) ([]byte, error) { // stores under data.. We always emit `group_catalogs` even when gc is // nil so the policy's `data.group_catalogs[group][catalog]` lookup is // well-formed (undefined-on-missing-key, not error-on-missing-document). -func buildDataDocument(gc GroupCatalogs) (map[string]interface{}, error) { +func buildDataDocument(gc GroupCatalogs, gs GroupScopes) (map[string]interface{}, error) { // JSON round-trip ensures we emit canonical JSON-decoded types // (map[string]interface{} and bool) regardless of what the caller // passes in. OPA's bundle loader expects these types and treats // concrete map[string]map[string]bool as opaque if it ever leaks // through. Round-tripping is also a stable serialization for tests. + if gc == nil { + // Marshalling a nil map emits "null"; substitute an empty object so + // the policy sees `data.group_catalogs == {}` not `null`. + gc = GroupCatalogs{} + } + if gs == nil { + gs = GroupScopes{} + } raw, err := json.Marshal(struct { GroupCatalogs GroupCatalogs `json:"group_catalogs"` - }{GroupCatalogs: gc}) + GroupScopes GroupScopes `json:"group_scopes"` + }{GroupCatalogs: gc, GroupScopes: gs}) if err != nil { - return nil, fmt.Errorf("marshal group_catalogs: %w", err) - } - if gc == nil { - // Marshalling a nil map emits "null"; substitute an empty object - // so the policy sees `data.group_catalogs == {}` not `null`. - raw = []byte(`{"group_catalogs":{}}`) + return nil, fmt.Errorf("marshal bundle data: %w", err) } var data map[string]interface{} if err := json.Unmarshal(raw, &data); err != nil { - return nil, fmt.Errorf("unmarshal group_catalogs: %w", err) + return nil, fmt.Errorf("unmarshal bundle data: %w", err) } return data, nil } diff --git a/controlplane/provisioner/opa/builder_test.go b/controlplane/provisioner/opa/builder_test.go index e4d99aca..332bac16 100644 --- a/controlplane/provisioner/opa/builder_test.go +++ b/controlplane/provisioner/opa/builder_test.go @@ -4,12 +4,25 @@ import ( "bytes" "net/http" "net/http/httptest" + "slices" "strings" "testing" "github.com/open-policy-agent/opa/v1/bundle" ) +// readBundle parses built bundle bytes back through OPA's own reader, so the +// scope assertions below check what OPA will actually load rather than what +// the builder intended to write. +func readBundle(t *testing.T, raw []byte) bundle.Bundle { + t.Helper() + parsed, err := bundle.NewReader(bytes.NewReader(raw)).Read() + if err != nil { + t.Fatalf("bundle.Read: %v", err) + } + return parsed +} + // TestBuildBundleRoundTrip builds a bundle, parses it back through OPA's // bundle reader, and asserts that the round-trip preserves the policy // source and data document. @@ -20,7 +33,7 @@ func TestBuildBundleRoundTrip(t *testing.T) { AdminGroup: {"org_42": true, "org_43": true}, } - raw, err := NewBuilder().BuildBundle(gc) + raw, err := NewBuilder().BuildBundle(gc, nil) if err != nil { t.Fatalf("BuildBundle: %v", err) } @@ -68,7 +81,7 @@ func TestBuildBundleRoundTrip(t *testing.T) { // what an empty group_catalogs gives us. func TestBuildBundleEmptyInput(t *testing.T) { for _, gc := range []GroupCatalogs{nil, {}} { - raw, err := NewBuilder().BuildBundle(gc) + raw, err := NewBuilder().BuildBundle(gc, nil) if err != nil { t.Fatalf("BuildBundle(empty): %v", err) } @@ -100,7 +113,7 @@ func TestBuildBundleEmptyInput(t *testing.T) { func TestBundleStoreAndHandler200(t *testing.T) { gc := GroupCatalogs{"org_42": {"org_42": true}} - raw, err := NewBuilder().BuildBundle(gc) + raw, err := NewBuilder().BuildBundle(gc, nil) if err != nil { t.Fatalf("BuildBundle: %v", err) } @@ -140,7 +153,7 @@ func TestBundleStoreAndHandler200(t *testing.T) { func TestBundleHandler304OnIfNoneMatch(t *testing.T) { gc := GroupCatalogs{"org_42": {"org_42": true}} - raw, _ := NewBuilder().BuildBundle(gc) + raw, _ := NewBuilder().BuildBundle(gc, nil) b := NewBundle(raw) store := &BundleStore{} store.Set(b) @@ -167,7 +180,7 @@ func TestBundleHandler304OnIfNoneMatch(t *testing.T) { func TestBundleHandler200OnEtagMiss(t *testing.T) { gc := GroupCatalogs{"org_42": {"org_42": true}} - raw, _ := NewBuilder().BuildBundle(gc) + raw, _ := NewBuilder().BuildBundle(gc, nil) store := &BundleStore{} store.Set(NewBundle(raw)) @@ -203,7 +216,7 @@ func TestBundleHandler503BeforeFirstBundle(t *testing.T) { func TestBundleHandlerRejectsNonGET(t *testing.T) { store := &BundleStore{} - raw, _ := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}) + raw, _ := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}, nil) store.Set(NewBundle(raw)) srv := httptest.NewServer(NewHandler(store, allowAllForTest)) @@ -227,7 +240,7 @@ func TestBundleHandlerRejectsNonGET(t *testing.T) { func TestBundleHandlerBearerTokenAuth(t *testing.T) { store := &BundleStore{} - raw, _ := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}) + raw, _ := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}, nil) store.Set(NewBundle(raw)) srv := httptest.NewServer(NewHandler(store, BearerTokenAuth("hunter2"))) @@ -293,7 +306,7 @@ func TestNewHandlerRejectsNilArgs(t *testing.T) { // guard in ServeHTTP must still fail closed. func TestHandlerLiteralWithNilAuthFailsClosed(t *testing.T) { store := &BundleStore{} - raw, _ := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}) + raw, _ := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}, nil) store.Set(NewBundle(raw)) srv := httptest.NewServer(&Handler{Store: store}) // no Auth set @@ -322,7 +335,7 @@ func TestBundleStoreSetOverwrites(t *testing.T) { store := &BundleStore{} // First bundle. - raw1, _ := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}) + raw1, _ := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}, nil) b1 := NewBundle(raw1) store.Set(b1) got, ok := store.Current() @@ -334,7 +347,7 @@ func TestBundleStoreSetOverwrites(t *testing.T) { raw2, _ := NewBuilder().BuildBundle(GroupCatalogs{ "org_42": {"org_42": true}, "org_43": {"org_43": true}, - }) + }, nil) b2 := NewBundle(raw2) if b1.ETag == b2.ETag { t.Fatal("bundles with different content should not share an ETag (sha256 collision?)") @@ -376,7 +389,7 @@ func TestNewBundleIsolatesInputSlice(t *testing.T) { func TestBundleHasNoExportedMutableByteAccess(t *testing.T) { // Construct a bundle, store it, mutate everything we can reach. store := &BundleStore{} - raw, _ := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}) + raw, _ := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}, nil) b := NewBundle(raw) store.Set(b) @@ -422,3 +435,100 @@ func TestNewBundleETagIsContentAddressed(t *testing.T) { t.Errorf("ETag should be a quoted string, got %q", b1.ETag) } } + +// --- project scopes --- + +// The bundle must carry group_scopes as its own document, and must declare it +// as a root: OPA refuses to activate a bundle that writes outside its +// declared roots, so a missing root is a bundle that silently never applies. +func TestBuildBundleCarriesGroupScopes(t *testing.T) { + gc := GroupCatalogs{"scope_acme_team_7": {"org_acme": true}} + gs := GroupScopes{"scope_acme_team_7": NewGroupScope( + []string{"posthog_7"}, []string{"posthog.events"})} + + raw, err := NewBuilder().BuildBundle(gc, gs) + if err != nil { + t.Fatalf("BuildBundle: %v", err) + } + b := readBundle(t, raw) + + roots := *b.Manifest.Roots + if !slices.Contains(roots, "group_scopes") { + t.Errorf("manifest roots = %v, must contain group_scopes", roots) + } + + scopes, ok := b.Data["group_scopes"].(map[string]interface{}) + if !ok { + t.Fatalf("group_scopes missing or wrong type: %#v", b.Data["group_scopes"]) + } + scope, ok := scopes["scope_acme_team_7"].(map[string]interface{}) + if !ok { + t.Fatalf("scope document missing: %#v", scopes) + } + for _, key := range []string{"schemas", "relations", "relation_schemas"} { + if _, ok := scope[key].(map[string]interface{}); !ok { + t.Errorf("scope.%s missing or wrong type: %#v", key, scope[key]) + } + } +} + +// An unscoped build must still emit an EMPTY group_scopes object rather than +// null or nothing: the policy's data.group_scopes[g] lookup has to be +// undefined-on-missing-key, and a null document makes it an evaluation error +// instead -- which fails every decision, not just the scoped ones. +func TestBuildBundleAlwaysEmitsGroupScopes(t *testing.T) { + raw, err := NewBuilder().BuildBundle(GroupCatalogs{"org_42": {"org_42": true}}, nil) + if err != nil { + t.Fatalf("BuildBundle: %v", err) + } + b := readBundle(t, raw) + scopes, ok := b.Data["group_scopes"].(map[string]interface{}) + if !ok { + t.Fatalf("group_scopes must be an object even when unscoped: %#v", b.Data["group_scopes"]) + } + if len(scopes) != 0 { + t.Errorf("group_scopes = %v, want empty", scopes) + } +} + +// RelationSchemas is derived, never supplied, so it cannot disagree with +// Relations. A schema the group can read one table in must appear there or +// the client can never navigate to that table. +func TestNewGroupScopeDerivesRelationSchemas(t *testing.T) { + scope := NewGroupScope( + []string{"posthog_7", ""}, + []string{"posthog.events", "posthog.persons", "legacy.hits"}, + ) + if !scope.Schemas["posthog_7"] { + t.Error("posthog_7 must be a whole-schema grant") + } + if scope.Schemas[""] { + t.Error("blank schema names must be dropped") + } + for _, r := range []string{"posthog.events", "posthog.persons", "legacy.hits"} { + if !scope.Relations[r] { + t.Errorf("relation %s missing", r) + } + } + for _, s := range []string{"posthog", "legacy"} { + if !scope.RelationSchemas[s] { + t.Errorf("relation schema %s missing", s) + } + } + if len(scope.RelationSchemas) != 2 { + t.Errorf("relation_schemas = %v, want exactly the two relation schemas", scope.RelationSchemas) + } +} + +// A relation that is not "." is dropped rather than stored: as +// a key no decision can ever match it would read like a working grant and +// silently be none. +func TestNewGroupScopeDropsMalformedRelations(t *testing.T) { + scope := NewGroupScope(nil, []string{"events", "a.b.c", ".events", "posthog.", "", "ok.tbl"}) + if len(scope.Relations) != 1 || !scope.Relations["ok.tbl"] { + t.Errorf("relations = %v, want only ok.tbl", scope.Relations) + } + if len(scope.RelationSchemas) != 1 || !scope.RelationSchemas["ok"] { + t.Errorf("relation_schemas = %v, want only ok", scope.RelationSchemas) + } +} diff --git a/controlplane/provisioner/opa/latency_test.go b/controlplane/provisioner/opa/latency_test.go index 9478c245..9232be21 100644 --- a/controlplane/provisioner/opa/latency_test.go +++ b/controlplane/provisioner/opa/latency_test.go @@ -46,7 +46,7 @@ func largeFixture(orgs int) GroupCatalogs { // decisions against it). func preparedLargeBundle(b interface{ Fatalf(string, ...interface{}) }, orgs int) rego.PreparedEvalQuery { ctx := context.Background() - data, err := buildDataDocument(largeFixture(orgs)) + data, err := buildDataDocument(largeFixture(orgs), nil) if err != nil { b.Fatalf("buildDataDocument: %v", err) } diff --git a/controlplane/provisioner/opa/policy.rego b/controlplane/provisioner/opa/policy.rego index 246be008..7433ef17 100644 --- a/controlplane/provisioner/opa/policy.rego +++ b/controlplane/provisioner/opa/policy.rego @@ -270,6 +270,7 @@ writable_catalog(catalog) if { not is_admin tenant_owns_catalog(catalog) managed_catalog_name(catalog) + not holds_scoped_group } listable_catalog(catalog) if readable_catalog(catalog) @@ -284,6 +285,101 @@ listable_catalog(catalog) if { managed_catalog_name(catalog) } +# --------------------------------------------------------------------------- +# Project scopes: narrowing a group to part of the catalog it owns. +# +# A duckgres login can be bound to ONE project (team), in which case it reads +# only that project's schemas rather than the whole org catalog. The control +# plane projects such a login into a `scope__team_` group, grants +# that group the org's catalog in data.group_catalogs exactly like an unscoped +# group, and ADDITIONALLY publishes a scope document for it under +# data.group_scopes. +# +# The layering is deliberate and load-bearing: a scope only ever REMOVES +# access. Every schema and table decision below still requires a group that +# owns the catalog in data.group_catalogs, so the cross-tenant boundary is the +# same rule it has always been, and a bug anywhere in this section can widen +# access only WITHIN the org's own catalog -- never across tenants. That is +# the property to preserve if these rules are ever restructured. +# +# Shape (every set is an object with value true so lookups stay O(1)): +# +# data.group_scopes[g].schemas[] whole schema readable +# data.group_scopes[g].relations[".
"] one table readable +# data.group_scopes[g].relation_schemas[] a schema that appears in +# `relations`, precomputed +# so schema-level decisions +# stay O(1) instead of +# scanning `relations` +# +# A group with NO document under data.group_scopes is unscoped and sees the +# whole catalog -- which is what every org's `org_` group is, so the +# unscoped tenant path is unchanged. +# +# Scoped identities get NO write authority (see writable_catalog): duckgres +# has a read-only project login and a read/write one, and only the read-only +# half is expressible here today. Denying writes to both is a narrowing of the +# read/write login, never a widening of the read-only one. +# --------------------------------------------------------------------------- + +# The requester's own groups that own `catalog`. Both branches below draw +# their group from this set, so neither can authorize a catalog that no group +# of the requester's owns. +granting_groups(catalog) := {g | + some g in input.context.identity.groups + g != admin_group + g != observer_group + data.group_catalogs[g][catalog] == true +} + +# A group is scoped iff the bundle carries a scope document for it. +scoped_group(g) if data.group_scopes[g] + +# holds_scoped_group: the requester is in at least one project-scoped group. +# Used to deny write authority outright rather than per-object. +holds_scoped_group if { + some g in input.context.identity.groups + scoped_group(g) +} + +# readable_schema / readable_table are the scope-aware counterparts of +# readable_catalog. An unscoped granting group allows everything in its +# catalog; a scoped one allows only what its document names. + +readable_schema(catalog, _) if admin_bundle_catalog(catalog) + +readable_schema(catalog, _) if { + some g in granting_groups(catalog) + not scoped_group(g) +} + +readable_schema(catalog, schema) if { + some g in granting_groups(catalog) + data.group_scopes[g].schemas[schema] == true +} + +readable_schema(catalog, schema) if { + some g in granting_groups(catalog) + data.group_scopes[g].relation_schemas[schema] == true +} + +readable_table(catalog, _, _) if admin_bundle_catalog(catalog) + +readable_table(catalog, _, _) if { + some g in granting_groups(catalog) + not scoped_group(g) +} + +readable_table(catalog, schema, _) if { + some g in granting_groups(catalog) + data.group_scopes[g].schemas[schema] == true +} + +readable_table(catalog, schema, table) if { + some g in granting_groups(catalog) + data.group_scopes[g].relations[concat(".", [schema, table])] == true +} + # --------------------------------------------------------------------------- # Catalog-scope decisions. # --------------------------------------------------------------------------- @@ -319,12 +415,18 @@ allow if { allow if { input.action.operation == "FilterSchemas" - readable_catalog(input.action.resource.schema.catalogName) + readable_schema( + input.action.resource.schema.catalogName, + input.action.resource.schema.schemaName, + ) } allow if { input.action.operation == "ShowTables" - readable_catalog(input.action.resource.schema.catalogName) + readable_schema( + input.action.resource.schema.catalogName, + input.action.resource.schema.schemaName, + ) } # DuckLake schema DDL. Rename checks BOTH resource and targetResource even @@ -350,22 +452,38 @@ allow if { allow if { input.action.operation == "SelectFromColumns" - readable_catalog(input.action.resource.table.catalogName) + readable_table( + input.action.resource.table.catalogName, + input.action.resource.table.schemaName, + input.action.resource.table.tableName, + ) } allow if { input.action.operation == "FilterTables" - readable_catalog(input.action.resource.table.catalogName) + readable_table( + input.action.resource.table.catalogName, + input.action.resource.table.schemaName, + input.action.resource.table.tableName, + ) } allow if { input.action.operation == "ShowColumns" - readable_catalog(input.action.resource.table.catalogName) + readable_table( + input.action.resource.table.catalogName, + input.action.resource.table.schemaName, + input.action.resource.table.tableName, + ) } allow if { input.action.operation == "FilterColumns" - readable_catalog(input.action.resource.table.catalogName) + readable_table( + input.action.resource.table.catalogName, + input.action.resource.table.schemaName, + input.action.resource.table.tableName, + ) } # DuckLake table and view DDL/DML. MERGE and CTAS are composed by Trino from @@ -444,13 +562,20 @@ batch contains i if { batch contains i if { some i input.action.operation == "FilterSchemas" - readable_catalog(input.action.filterResources[i].schema.catalogName) + readable_schema( + input.action.filterResources[i].schema.catalogName, + input.action.filterResources[i].schema.schemaName, + ) } batch contains i if { some i input.action.operation == "FilterTables" - readable_catalog(input.action.filterResources[i].table.catalogName) + readable_table( + input.action.filterResources[i].table.catalogName, + input.action.filterResources[i].table.schemaName, + input.action.filterResources[i].table.tableName, + ) } # FilterColumns is the one operation whose indices point into the candidate's @@ -459,7 +584,11 @@ batch contains i if { batch contains i if { input.action.operation == "FilterColumns" count(input.action.filterResources) == 1 - readable_catalog(input.action.filterResources[0].table.catalogName) + readable_table( + input.action.filterResources[0].table.catalogName, + input.action.filterResources[0].table.schemaName, + input.action.filterResources[0].table.tableName, + ) some i, _ in input.action.filterResources[0].table.columns } diff --git a/controlplane/provisioner/opa/policy_test.go b/controlplane/provisioner/opa/policy_test.go index e56ed36e..91f60153 100644 --- a/controlplane/provisioner/opa/policy_test.go +++ b/controlplane/provisioner/opa/policy_test.go @@ -16,9 +16,17 @@ import ( // table-driven tests below so we pay the compile cost once per test // binary, not once per case. func preparedPolicy(t *testing.T, gc GroupCatalogs) rego.PreparedEvalQuery { + t.Helper() + return preparedScopedPolicy(t, gc, nil) +} + +// preparedScopedPolicy is preparedPolicy with project scopes in the bundle. +// Separate entry point so every pre-scopes test keeps calling preparedPolicy +// and keeps asserting the unscoped behaviour verbatim. +func preparedScopedPolicy(t *testing.T, gc GroupCatalogs, gs GroupScopes) rego.PreparedEvalQuery { t.Helper() ctx := context.Background() - data, err := buildDataDocument(gc) + data, err := buildDataDocument(gc, gs) if err != nil { t.Fatalf("buildDataDocument: %v", err) } @@ -883,7 +891,13 @@ func TestIsolationMatrix(t *testing.T) { // preparedBatch compiles the policy for the batched entrypoint. func preparedBatch(t *testing.T, gc GroupCatalogs) rego.PreparedEvalQuery { t.Helper() - data, err := buildDataDocument(gc) + return preparedScopedBatch(t, gc, nil) +} + +// preparedScopedBatch is preparedBatch with project scopes in the bundle. +func preparedScopedBatch(t *testing.T, gc GroupCatalogs, gs GroupScopes) rego.PreparedEvalQuery { + t.Helper() + data, err := buildDataDocument(gc, gs) if err != nil { t.Fatalf("buildDataDocument: %v", err) } @@ -1858,6 +1872,187 @@ func TestTrinoProvisionerReadsOnlySystemReadinessInventories(t *testing.T) { } } +// -------------------------------------------------------------------------- +// Project scopes. +// +// A project-scoped login is in `scope__team_`, which owns the SAME +// catalog the org group owns and additionally carries a scope document. The +// property every test here defends is that the scope only ever SUBTRACTS: it +// cannot reach another tenant, and removing the scope document must restore +// exactly the unscoped behaviour. +// -------------------------------------------------------------------------- + +// scopedFixture is one org with an unscoped group and a project-scoped group, +// both owning org_acme. Team 7 holds schema posthog_7 whole, plus the single +// relation posthog.events out of the shared legacy schema. +func scopedFixture() (GroupCatalogs, GroupScopes) { + gc := GroupCatalogs{ + "org_acme": {"org_acme": true}, + "scope_acme_team_7": {"org_acme": true}, + "org_other": {"org_other": true}, + } + gs := GroupScopes{ + "scope_acme_team_7": NewGroupScope( + []string{"posthog_7", "posthog_7_data_imports"}, + []string{"posthog.events"}, + ), + } + return gc, gs +} + +func scopedIdentity() map[string]interface{} { + return map[string]interface{}{ + "identity": map[string]interface{}{ + "user": "acme.posthog_team_7", + "groups": []interface{}{"scope_acme_team_7", "tier_free"}, + }, + } +} + +func tableInput(op, catalog, schema, table string, ctx map[string]interface{}) map[string]interface{} { + return map[string]interface{}{ + "context": ctx, + "action": map[string]interface{}{ + "operation": op, + "resource": map[string]interface{}{ + "table": map[string]interface{}{ + "catalogName": catalog, + "schemaName": schema, + "tableName": table, + }, + }, + }, + } +} + +func schemaInput(op, catalog, schema string, ctx map[string]interface{}) map[string]interface{} { + return map[string]interface{}{ + "context": ctx, + "action": map[string]interface{}{ + "operation": op, + "resource": map[string]interface{}{ + "schema": map[string]interface{}{ + "catalogName": catalog, + "schemaName": schema, + }, + }, + }, + } +} + +// A scoped login reads its own project's schemas and nothing else in the very +// same catalog. This is the whole point of the feature: without it a project +// login projected into Trino would see every other project's data. +func TestScopedGroupReadsOnlyItsOwnSchemas(t *testing.T) { + gc, gs := scopedFixture() + q := preparedScopedPolicy(t, gc, gs) + + for _, tc := range []struct { + name string + schema string + table string + want bool + }{ + {"own schema", "posthog_7", "events", true}, + {"own imports schema", "posthog_7_data_imports", "stripe_charges", true}, + {"another project's schema", "posthog_9", "events", false}, + {"a shared schema it holds no grant in", "public", "anything", false}, + } { + t.Run(tc.name, func(t *testing.T) { + for _, op := range []string{"SelectFromColumns", "FilterTables", "ShowColumns"} { + got := evalAllow(t, q, tableInput(op, "org_acme", tc.schema, tc.table, scopedIdentity())) + if got != tc.want { + t.Errorf("%s on %s.%s = %v, want %v", op, tc.schema, tc.table, got, tc.want) + } + } + }) + } +} + +// An individually granted relation is readable, and its SIBLINGS in the same +// schema are not. duckgres grants a project the shared legacy `posthog` +// schema one table at a time precisely because the schema holds every other +// project's tables too, so a grant that leaked to the whole schema would be a +// cross-project read. +func TestScopedGroupRelationGrantDoesNotLeakItsSchema(t *testing.T) { + gc, gs := scopedFixture() + q := preparedScopedPolicy(t, gc, gs) + + if !evalAllow(t, q, tableInput("SelectFromColumns", "org_acme", "posthog", "events", scopedIdentity())) { + t.Error("granted relation posthog.events must be readable") + } + if evalAllow(t, q, tableInput("SelectFromColumns", "org_acme", "posthog", "persons", scopedIdentity())) { + t.Error("posthog.persons was NOT granted and must not be readable") + } +} + +// The schema holding a granted relation must be visible at schema level, or +// the client can never navigate to the table it is allowed to read. +func TestScopedGroupSeesTheSchemaOfAGrantedRelation(t *testing.T) { + gc, gs := scopedFixture() + q := preparedScopedPolicy(t, gc, gs) + + for _, op := range []string{"FilterSchemas", "ShowTables"} { + if !evalAllow(t, q, schemaInput(op, "org_acme", "posthog", scopedIdentity())) { + t.Errorf("%s on the schema of a granted relation must be allowed", op) + } + if !evalAllow(t, q, schemaInput(op, "org_acme", "posthog_7", scopedIdentity())) { + t.Errorf("%s on a wholly granted schema must be allowed", op) + } + if evalAllow(t, q, schemaInput(op, "org_acme", "posthog_9", scopedIdentity())) { + t.Errorf("%s on another project's schema must be denied", op) + } + } +} + +// The cross-tenant boundary is unchanged for a scoped login: its scope names +// schemas, and a schema name says nothing about which catalog it is in. A +// scope group that owns only org_acme must not reach org_other even for a +// schema name its own scope happens to list. +func TestScopedGroupStillCannotCrossTenants(t *testing.T) { + gc, gs := scopedFixture() + q := preparedScopedPolicy(t, gc, gs) + + for _, op := range []string{"SelectFromColumns", "FilterTables", "ShowColumns"} { + if evalAllow(t, q, tableInput(op, "org_other", "posthog_7", "events", scopedIdentity())) { + t.Errorf("%s reached another tenant's catalog", op) + } + } + if evalAllow(t, q, schemaInput("FilterSchemas", "org_other", "posthog_7", scopedIdentity())) { + t.Error("FilterSchemas reached another tenant's catalog") + } + catalogInput := map[string]interface{}{ + "context": scopedIdentity(), + "action": map[string]interface{}{ + "operation": "AccessCatalog", + "resource": map[string]interface{}{"catalog": map[string]interface{}{"name": "org_other"}}, + }, + } + if evalAllow(t, q, catalogInput) { + t.Error("AccessCatalog reached another tenant's catalog") + } +} + +// A scoped login still needs the catalog itself, or it cannot run any query +// at all against the schemas it IS allowed to read. +func TestScopedGroupCanAccessItsOwnCatalog(t *testing.T) { + gc, gs := scopedFixture() + q := preparedScopedPolicy(t, gc, gs) + + for _, op := range []string{"AccessCatalog", "FilterCatalogs", "ShowSchemas"} { + in := map[string]interface{}{ + "context": scopedIdentity(), + "action": map[string]interface{}{ + "operation": op, + "resource": map[string]interface{}{"catalog": map[string]interface{}{"name": "org_acme"}}, + }, + } + if !evalAllow(t, q, in) { + t.Errorf("%s on its own catalog must be allowed", op) + } + } +} + func TestTrinoProvisionerCatalogStatesAreNarrow(t *testing.T) { q := preparedPolicy(t, twoOrgFixture()) for _, tc := range []struct { @@ -1877,3 +2072,120 @@ func TestTrinoProvisionerCatalogStatesAreNarrow(t *testing.T) { } } } + +// Scoped logins get no write authority anywhere, including inside the schemas +// they can read. duckgres has a read-only project login and a read/write one; +// only the read-only half is expressible here today, so both are read-only in +// Trino. That is a narrowing of the read/write login and never a widening of +// the read-only one -- if this test starts failing because writes were added +// for project_user, the scope must gate them per-schema, not per-catalog. +func TestScopedGroupHasNoWriteAuthority(t *testing.T) { + gc, gs := scopedFixture() + q := preparedScopedPolicy(t, gc, gs) + + for _, op := range []string{"CreateSchema", "DropSchema"} { + if evalAllow(t, q, schemaInput(op, "org_acme", "posthog_7", scopedIdentity())) { + t.Errorf("%s must be denied to a scoped login", op) + } + } + for _, op := range []string{"CreateTable", "DropTable", "InsertIntoTable", "DeleteFromTable", "UpdateTableColumns"} { + if evalAllow(t, q, tableInput(op, "org_acme", "posthog_7", "events", scopedIdentity())) { + t.Errorf("%s must be denied to a scoped login", op) + } + } +} + +// The unscoped org login is untouched by any of the above: it reads every +// schema in its catalog and keeps its write authority. This is the +// regression guard for every tenant that has no project logins at all. +func TestUnscopedGroupIsUnaffectedByScopesInTheBundle(t *testing.T) { + gc, gs := scopedFixture() + q := preparedScopedPolicy(t, gc, gs) + + unscoped := map[string]interface{}{ + "identity": map[string]interface{}{ + "user": "acme", + "groups": []interface{}{"org_acme", "tier_free"}, + }, + } + for _, schema := range []string{"posthog_7", "posthog_9", "public"} { + if !evalAllow(t, q, tableInput("SelectFromColumns", "org_acme", schema, "events", unscoped)) { + t.Errorf("unscoped login must read %s", schema) + } + if !evalAllow(t, q, schemaInput("FilterSchemas", "org_acme", schema, unscoped)) { + t.Errorf("unscoped login must see %s", schema) + } + } + if !evalAllow(t, q, schemaInput("CreateSchema", "org_acme", "whatever", unscoped)) { + t.Error("unscoped login must keep its write authority") + } + if evalAllow(t, q, tableInput("SelectFromColumns", "org_other", "posthog_7", "events", unscoped)) { + t.Error("unscoped login must not cross tenants") + } +} + +// A scope document with no readable namespace is the fail-closed shape +// configstore produces for a missing or disabled team. It must read NOTHING +// rather than degrading into unscoped access. +func TestEmptyScopeReadsNothing(t *testing.T) { + gc := GroupCatalogs{"scope_acme_team_7": {"org_acme": true}} + gs := GroupScopes{"scope_acme_team_7": NewGroupScope(nil, nil)} + q := preparedScopedPolicy(t, gc, gs) + + if evalAllow(t, q, tableInput("SelectFromColumns", "org_acme", "posthog_7", "events", scopedIdentity())) { + t.Error("an empty scope must read no table") + } + if evalAllow(t, q, schemaInput("FilterSchemas", "org_acme", "posthog_7", scopedIdentity())) { + t.Error("an empty scope must see no schema") + } +} + +// Batched filtering must answer identically to the non-batched path for +// scoped groups too, candidate by candidate -- the same invariant +// TestBatchedFilteringMatchesNonBatched pins for the unscoped path. The two +// entrypoints dispatch separately, so a scope rule added to one and not the +// other is exactly the drift this catches. +func TestBatchedFilteringMatchesNonBatchedForScopes(t *testing.T) { + gc, gs := scopedFixture() + single := preparedScopedPolicy(t, gc, gs) + batched := preparedScopedBatch(t, gc, gs) + ctx := context.Background() + + schemas := []string{"posthog_7", "posthog_9", "posthog", "public"} + resources := make([]interface{}, 0, len(schemas)) + for _, s := range schemas { + resources = append(resources, map[string]interface{}{ + "schema": map[string]interface{}{"catalogName": "org_acme", "schemaName": s}, + }) + } + rs, err := batched.Eval(ctx, rego.EvalInput(map[string]interface{}{ + "context": scopedIdentity(), + "action": map[string]interface{}{ + "operation": "FilterSchemas", + "filterResources": resources, + }, + })) + if err != nil { + t.Fatalf("Eval batch: %v", err) + } + allowed := map[int]bool{} + if len(rs) > 0 { + for _, v := range rs[0].Expressions[0].Value.([]interface{}) { + n, ok := v.(json.Number) + if !ok { + t.Fatalf("expected numeric index, got %T (%v)", v, v) + } + i, err := n.Int64() + if err != nil { + t.Fatalf("index %v: %v", v, err) + } + allowed[int(i)] = true + } + } + for i, s := range schemas { + want := evalAllow(t, single, schemaInput("FilterSchemas", "org_acme", s, scopedIdentity())) + if allowed[i] != want { + t.Errorf("schema %s: batch=%v single=%v", s, allowed[i], want) + } + } +} diff --git a/controlplane/provisioner/opa/types.go b/controlplane/provisioner/opa/types.go index 94ff7557..4257c747 100644 --- a/controlplane/provisioner/opa/types.go +++ b/controlplane/provisioner/opa/types.go @@ -28,6 +28,8 @@ // per-user and require a bundle-shape migration during the OIDC rollout. package opa +import "strings" + // GroupCatalogs maps a Trino group name (e.g. `org_` for customer // orgs, where `org` is the sanitized Org.Name; or the admin group for // the provisioner's smoke-test access) to the set of catalog names that @@ -44,11 +46,71 @@ package opa // bounded iteration, still O(1) in catalog count. type GroupCatalogs map[string]map[string]bool +// GroupScope narrows one group to part of the catalog it owns. A group with +// no GroupScope is unscoped and reads the whole catalog; a group WITH one +// reads only what these sets name. The policy consults a scope only after the +// group has already been found to own the catalog in GroupCatalogs, so a +// scope can subtract access but never add any -- in particular it can never +// reach another tenant's catalog. +// +// Each field is a set represented as map[string]bool with the value always +// true, for the same O(1)-lookup reason GroupCatalogs is (see above): the +// policy indexes into these objects rather than scanning them. +type GroupScope struct { + // Schemas are readable in full: every table in them is allowed. + Schemas map[string]bool `json:"schemas"` + // Relations are individually readable tables, keyed ".
", + // for schemas the group does NOT hold in full. duckgres grants these for + // a project's tables that live in the shared legacy `posthog` schema. + Relations map[string]bool `json:"relations"` + // RelationSchemas is the set of schema names appearing in Relations, + // precomputed so a schema-level decision (FilterSchemas, ShowTables) is + // an object lookup rather than a scan over Relations. Derived data -- + // build it with NewGroupScope rather than by hand, so it cannot drift + // from Relations and silently hide a schema the group can read a table + // in. + RelationSchemas map[string]bool `json:"relation_schemas"` +} + +// GroupScopes maps a Trino group name to the scope narrowing it. Only +// project-scoped groups appear; the absence of a key means "unscoped", which +// is what every org's own `org_` group is. +type GroupScopes map[string]GroupScope + +// NewGroupScope builds a GroupScope from the allowed-schema and +// allowed-relation lists duckgres derives for a project login, deriving +// RelationSchemas from relations so the two cannot disagree. +// +// A relation that is not ".
" is dropped rather than guessed +// at: it would otherwise land in the policy as a key no decision can ever +// match, which reads as a working grant and is not one. +func NewGroupScope(schemas, relations []string) GroupScope { + scope := GroupScope{ + Schemas: map[string]bool{}, + Relations: map[string]bool{}, + RelationSchemas: map[string]bool{}, + } + for _, s := range schemas { + if s != "" { + scope.Schemas[s] = true + } + } + for _, r := range relations { + schema, table, ok := strings.Cut(r, ".") + if !ok || schema == "" || table == "" || strings.Contains(table, ".") { + continue + } + scope.Relations[r] = true + scope.RelationSchemas[schema] = true + } + return scope +} + // BundleBuilder builds an OPA bundle (gzip'd tarball per OPA's bundle spec) // from a GroupCatalogs input. The returned bytes are suitable for serving // from a bundle endpoint or POSTing through OPA's bundle service API. type BundleBuilder interface { - BuildBundle(gc GroupCatalogs) ([]byte, error) + BuildBundle(gc GroupCatalogs, gs GroupScopes) ([]byte, error) } // AdminPrincipal is the Trino username the provisioner authenticates as diff --git a/controlplane/provisioner/trino_provisioner.go b/controlplane/provisioner/trino_provisioner.go index f0f1397b..a774284a 100644 --- a/controlplane/provisioner/trino_provisioner.go +++ b/controlplane/provisioner/trino_provisioner.go @@ -204,6 +204,42 @@ func TrinoGroupName(principal string) string { return "org_" + trinoSanitize(principal) } +// trinoUsernamePattern is the grammar a duckgres username must satisfy to be +// projected into the cell's auth files. +// +// This is an ALLOWLIST, and it is a security control rather than a tidiness +// one. duckgres validates a username as little more than "not empty" (see +// controlplane/validation.go), while password.db is `:` per line +// and group.db is `:,` per line. A username holding `:`, +// `,` or a newline would not merely render oddly -- it would let whoever can +// create org users append arbitrary lines to those files, including a line +// for the admin principal. Anything outside this grammar is therefore never +// written, and no amount of downstream escaping is relied on. +// +// `.` is excluded as well, so that `.` carries exactly the one +// separator TrinoPrincipalSeparator puts there and the org prefix stays +// recoverable by the resource-group selector (see orgCaptureRegex). +var trinoUsernamePattern = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9_-]*$`) + +// projectableTrinoUsername reports whether a duckgres username is safe to +// render into password.db / group.db. +func projectableTrinoUsername(username string) bool { + return len(username) <= 255 && trinoUsernamePattern.MatchString(username) +} + +// TrinoScopeGroupName returns the group label for a project-scoped login: +// one group per (org, team), carrying that team's schema scope in the OPA +// bundle. +// +// The `scope_` prefix keeps these out of TrinoGroupName's `org_` space +// and TrinoTierGroupName's `tier_` space. That separation matters: a group in +// the `org_` space that the bundle happens not to scope reads the whole +// catalog, so a scope group whose name could collide with an org group would +// be a silent widening rather than a name clash. +func TrinoScopeGroupName(principal string, teamID int64) string { + return fmt.Sprintf("scope_%s_team_%d", trinoSanitize(principal), teamID) +} + // TrinoResourceGroupName returns the resource-group selector key for // an org. Sanitized like the catalog name so a `.` in orgName doesn't // get re-interpreted as a hierarchy separator in Trino's resource- @@ -762,8 +798,8 @@ func (p *TrinoProvisioner) Reconcile(ctx context.Context) error { return nil } -// rejectPrincipalCollisions splits orgs into those safe to project and -// those whose Trino catalog name is not unique to them. +// rejectPrincipalCollisions splits orgs into those safe to project and those +// whose Trino catalog name, or whose Trino username, is not unique to them. // // Every Trino-facing name is trinoSanitize(principal), and sanitization is // injective over principals that satisfy ValidateDatabaseName — that grammar @@ -783,6 +819,19 @@ func (p *TrinoProvisioner) Reconcile(ctx context.Context) error { // the other also believes it owns. func rejectPrincipalCollisions(orgs []configstore.TrinoEnabledOrg) (projectable []configstore.TrinoEnabledOrg, collisions map[string]error) { byCatalog := make(map[string][]string, len(orgs)) + byPrincipal := make(map[string]map[string]bool, len(orgs)) + claim := func(principal, orgID string) { + if byPrincipal[principal] == nil { + byPrincipal[principal] = map[string]bool{} + } + byPrincipal[principal][orgID] = true + } + // The cell's own principals are claimed first, so a tenant that derives + // either name is treated as contesting it and is held back. Neither is + // reachable from a valid database_name, but the policy's whole admin + // conjunction rests on the name being the provisioner's alone. + claim(opa.AdminPrincipal, "") + claim(opa.ObserverPrincipal, "") for _, o := range orgs { principal := o.TrinoPrincipal() if principal == "" { @@ -792,6 +841,13 @@ func rejectPrincipalCollisions(orgs []configstore.TrinoEnabledOrg) (projectable } name := TrinoCatalogName(principal) byCatalog[name] = append(byCatalog[name], o.OrgID) + claim(principal, o.OrgID) + for _, u := range o.Users { + if !projectableTrinoUsername(u.Username) { + continue + } + claim(o.TrinoUserPrincipal(u.Username), o.OrgID) + } } contested := make(map[string]string, 0) // orgID -> catalog name @@ -803,15 +859,51 @@ func rejectPrincipalCollisions(orgs []configstore.TrinoEnabledOrg) (projectable contested[id] = name } } - if len(contested) == 0 { + // A Trino username claimed by two orgs is a cross-tenant authentication + // bug, not a cosmetic clash: password.db is one flat namespace per cell, + // so the duplicate line lets one org's user authenticate against the + // other's entry and land in the other's group. Valid database_names make + // this unreachable — they are DNS labels, so `.` splits at its + // only dot — but grandfathered rows predate that rule and may hold a dot, + // which is exactly how `acme.analytics` the org and `acme` + `analytics` + // the login come to claim one name. + contestedPrincipal := map[string]string{} // orgID -> principal + for principal, owners := range byPrincipal { + if len(owners) < 2 { + continue + } + for id := range owners { + if id == "" { + continue // the cell's own principal, not an org + } + contestedPrincipal[id] = principal + } + } + if len(contested) == 0 && len(contestedPrincipal) == 0 { return orgs, nil } - collisions = make(map[string]error, len(contested)) - projectable = make([]configstore.TrinoEnabledOrg, 0, len(orgs)-len(contested)) + collisions = make(map[string]error, len(contested)+len(contestedPrincipal)) + projectable = make([]configstore.TrinoEnabledOrg, 0, len(orgs)) for _, o := range orgs { name, bad := contested[o.OrgID] if !bad { + if principal, dup := contestedPrincipal[o.OrgID]; dup { + others := make([]string, 0, len(byPrincipal[principal])) + for id := range byPrincipal[principal] { + if id != o.OrgID { + others = append(others, orgLabel(id)) + } + } + sort.Strings(others) + collisions[o.OrgID] = fmt.Errorf( + "Trino username %q is also claimed by %s; refusing to project either — "+ + "rename the org's database_name or the colliding login so the usernames differ", + principal, strings.Join(others, ", ")) + slog.Error("Trino reconcile: refusing to project orgs whose Trino usernames collide.", + "org", o.OrgID, "principal", principal, "colliding_with", others) + continue + } projectable = append(projectable, o) continue } @@ -832,6 +924,16 @@ func rejectPrincipalCollisions(orgs []configstore.TrinoEnabledOrg) (projectable return projectable, collisions } +// orgLabel names a principal's claimant in an operator-facing message. The +// empty org id is the cell itself (see the AdminPrincipal/ObserverPrincipal +// claims in rejectPrincipalCollisions), which has no org row to name. +func orgLabel(orgID string) string { + if orgID == "" { + return "this Trino cell's own operational principals" + } + return "org " + orgID +} + // claimCellOrgs filters the fleet-wide Trino-enabled listing down to the // orgs this cell is responsible for, claiming any that have no cell yet. // @@ -1977,22 +2079,28 @@ type TrinoClusterPrincipals struct { // Format conventions: // // password.db: : -// One line per org. The principal is the org's +// Two kinds of tenant line. The org's own principal is its // database_name (see TrinoEnabledOrg.TrinoPrincipal), so the -// tenant's Trino username is the same name it uses for its -// DuckDB warehouse rather than a bare org UUID. Hash is -// copied through unchanged — it's already bcrypt in the -// configstore, and it is the SAME hash the DuckDB warehouse -// authenticates with, so one password works for both. +// tenant is known by the same name it uses for its DuckDB +// warehouse rather than a bare org UUID. Each of the org's +// duckgres logins additionally gets `.` +// (TrinoUserPrincipal). Hashes are copied through unchanged — +// they are already bcrypt in the configstore, and they are the +// SAME hashes pgwire authenticates with, so one password works +// on both engines and nothing has to be re-hashed or reset. // group.db: : // NOTE: this is the opposite direction from password.db. -// For v1 (one user per org) the value is the single -// principal. Easy to get backwards, hence this comment. +// Easy to get backwards, hence this comment. // -// Orgs without a RootPasswordHash or a principal are skipped silently -// (the listing query already filters to (org, root-user) pairs with a -// non-blank database_name, so this is just defensive against future -// changes). +// An org's unscoped principals share its `org_` group, +// which the OPA bundle grants the whole catalog. A +// project-scoped login goes into a `scope__team_` +// group INSTEAD — never both, because the org group is +// unscoped and membership in it would defeat the scope. +// +// Orgs without a principal are skipped entirely. An org with a principal but +// no RootPasswordHash still projects its per-user logins: the bare org +// principal is one credential among several now, not the only way in. // // cluster carries the bcrypt hashes for the two non-tenant principals. // Each is prepended to both files when non-empty, regardless of orgs — @@ -2025,14 +2133,56 @@ func BuildTrinoAuthFiles(orgs []configstore.TrinoEnabledOrg, cluster TrinoCluste } for _, o := range orgs { principal := o.TrinoPrincipal() - if o.RootPasswordHash == "" || principal == "" { + if principal == "" { continue } - pwLines = append(pwLines, fmt.Sprintf("%s:%s", principal, o.RootPasswordHash)) - // group_name first, comma-separated users second. For v1 this - // is one user per group (the principal only). - grpLines = append(grpLines, fmt.Sprintf("%s:%s", TrinoGroupName(principal), principal)) - tierMembers[normalizeTier(o.Tier)] = append(tierMembers[normalizeTier(o.Tier)], principal) + // The org's own principal: database_name authenticating with the + // root hash. Kept for service-to-service use and for clients + // configured before per-user logins existed. + var orgGroupMembers []string + if o.RootPasswordHash != "" { + pwLines = append(pwLines, fmt.Sprintf("%s:%s", principal, o.RootPasswordHash)) + orgGroupMembers = append(orgGroupMembers, principal) + tierMembers[normalizeTier(o.Tier)] = append(tierMembers[normalizeTier(o.Tier)], principal) + } + // Per-user logins. Each one authenticates as . with the + // very same bcrypt hash it uses on pgwire. + scopeMembers := map[string][]string{} + for _, u := range o.Users { + if u.PasswordHash == "" || !projectableTrinoUsername(u.Username) { + // An unprojectable username costs that ONE login its Trino + // access and nothing else. Holding the whole org back would + // turn one odd name into an org-wide outage. + if u.PasswordHash != "" { + slog.Warn("Trino: skipping login whose username cannot be rendered into the auth files.", + "org", o.OrgID, "user", u.Username) + } + continue + } + userPrincipal := o.TrinoUserPrincipal(u.Username) + pwLines = append(pwLines, fmt.Sprintf("%s:%s", userPrincipal, u.PasswordHash)) + // A scoped login joins its scope group INSTEAD of the org group: + // the org group is unscoped in the bundle, so putting a project + // login in it would hand it the whole catalog. + if group, ok := scopeGroupFor(o, u); ok { + scopeMembers[group] = append(scopeMembers[group], userPrincipal) + } else { + orgGroupMembers = append(orgGroupMembers, userPrincipal) + } + tierMembers[normalizeTier(o.Tier)] = append(tierMembers[normalizeTier(o.Tier)], userPrincipal) + } + // group_name first, comma-separated users second. NOTE this is the + // opposite direction from password.db; easy to get backwards. + if len(orgGroupMembers) > 0 { + sort.Strings(orgGroupMembers) + grpLines = append(grpLines, fmt.Sprintf("%s:%s", + TrinoGroupName(principal), strings.Join(orgGroupMembers, ","))) + } + for _, group := range sortedKeys(scopeMembers) { + members := scopeMembers[group] + sort.Strings(members) + grpLines = append(grpLines, fmt.Sprintf("%s:%s", group, strings.Join(members, ","))) + } } // Tier claims. These carry a tenant's tier to the resource-group // selectors, which match on userGroup — that is what keeps @@ -2059,6 +2209,35 @@ func BuildTrinoAuthFiles(orgs []configstore.TrinoEnabledOrg, cluster TrinoCluste return strings.Join(pwLines, "\n"), strings.Join(grpLines, "\n") } +// scopeGroupFor returns the scope group a login belongs in, and whether it is +// scoped at all. A login is scoped iff the config store resolved a project +// policy for it AND that policy names a team, which is what the group is +// keyed on. +// +// A scoped login whose policy resolved to NO readable namespace still gets a +// group — an empty scope in the bundle, which reads nothing. That is the +// fail-closed shape configstore produces for a team that is missing or +// disabled, and it must survive the trip rather than degrading into "no scope +// group", which would put the login in the unscoped org group. +func scopeGroupFor(o configstore.TrinoEnabledOrg, u configstore.TrinoOrgUser) (string, bool) { + if u.Scope == nil || u.TeamID == nil { + return "", false + } + return TrinoScopeGroupName(o.TrinoPrincipal(), *u.TeamID), true +} + +// sortedKeys returns a map's keys in sorted order, so every projection this +// file writes is byte-stable across ticks (an unstable file would rewrite the +// Secret every reconcile and re-trigger every coordinator's file refresh). +func sortedKeys[V any](m map[string]V) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} + // reconcileResourceGroups projects resource-groups.json into the // trino-resource-groups ConfigMap. // @@ -2146,9 +2325,21 @@ const ( // selector's named capture; orgCaptureRegex is the capture that fills it. // Together they let one templated node serve every tenant, which is what // keeps this file free of tenant names — see BuildTrinoResourceGroups. +// +// The capture stops at the first `.` because a tenant principal is either the +// org's bare database_name (`acme`) or one of its per-user logins +// (`acme.analyst`, see configstore.TrinoUserPrincipal), and BOTH must resolve +// to the SAME leaf resource group. A `(?.*)` capture -- what this was +// before per-user logins -- matches the whole username, so every user would +// get a private leaf carrying the full per-tenant limits, and an org with ten +// logins would quietly hold ten times its concurrency and memory budget. The +// selector is matched with Pattern.matcher(user).matches(), i.e. a full +// match, so the trailing group is required for qualified names to match at +// all; TestBuildTrinoResourceGroups_CapturesOrgFromQualifiedUsername pins +// both shapes. const ( orgTemplateVariable = "${org}" - orgCaptureRegex = "(?.*)" + orgCaptureRegex = `(?[^.]+)(?:\..*)?` ) // TrinoTierGroupName is the group.db claim that puts an org in a tier lane. @@ -2351,11 +2542,19 @@ func BuildTrinoResourceGroups() ([]byte, error) { // managed catalog so the provisioner's own SHOW CATALOGS idempotency // check (run as opa.AdminPrincipal) is allowed. // +// Project-scoped logins add a second kind of group, `scope__team_`, +// which owns exactly the same catalog its org group does and additionally +// carries a GroupScope. That layering is what keeps this change off the +// tenant-isolation path: the catalog grant is the same grant, and the scope +// can only subtract from it (see the "Project scopes" section of +// policy.rego). +// // ctx is currently unused (the builder is pure and the store Set is // in-memory), but kept on the signature for parity with the other // reconcile* steps and to permit instrumented builders later. func (p *TrinoProvisioner) reconcileOPABundle(_ context.Context, orgs []configstore.TrinoEnabledOrg) error { gc := make(opa.GroupCatalogs, len(orgs)+1) + gs := opa.GroupScopes{} adminCatalogs := make(map[string]bool, len(orgs)) for _, o := range orgs { principal := o.TrinoPrincipal() @@ -2365,6 +2564,18 @@ func (p *TrinoProvisioner) reconcileOPABundle(_ context.Context, orgs []configst catalog := TrinoCatalogName(principal) gc[TrinoGroupName(principal)] = map[string]bool{catalog: true} adminCatalogs[catalog] = true + // A project-scoped login sits in its own group, which owns the SAME + // catalog — the cross-tenant check is unchanged for it — and carries + // a scope document that narrows it to that project's schemas. Groups + // are per (org, team), so several logins on one team share one entry. + for _, u := range o.Users { + group, scoped := scopeGroupFor(o, u) + if !scoped || !projectableTrinoUsername(u.Username) || u.PasswordHash == "" { + continue + } + gc[group] = map[string]bool{catalog: true} + gs[group] = opa.NewGroupScope(u.Scope.AllowedSchemas, u.Scope.AllowedRelations) + } } if len(adminCatalogs) > 0 { // Admin owns every managed catalog so SHOW CATALOGS / catalog @@ -2373,7 +2584,7 @@ func (p *TrinoProvisioner) reconcileOPABundle(_ context.Context, orgs []configst // docstring). gc[opa.AdminGroup] = adminCatalogs } - bundle, err := p.bundleBuilder.BuildBundle(gc) + bundle, err := p.bundleBuilder.BuildBundle(gc, gs) if err != nil { return fmt.Errorf("build opa bundle: %w", err) } diff --git a/controlplane/provisioner/trino_provisioner_test.go b/controlplane/provisioner/trino_provisioner_test.go index 9456c7f1..4eae8f49 100644 --- a/controlplane/provisioner/trino_provisioner_test.go +++ b/controlplane/provisioner/trino_provisioner_test.go @@ -760,13 +760,17 @@ type testProvisionerHarness struct { // capturingBundleBuilder is a pass-through opa.BundleBuilder that // remembers its last input. type capturingBundleBuilder struct { - inner opa.BundleBuilder - last opa.GroupCatalogs + inner opa.BundleBuilder + last opa.GroupCatalogs + lastCatalogs opa.GroupCatalogs + lastScopes opa.GroupScopes } -func (c *capturingBundleBuilder) BuildBundle(gc opa.GroupCatalogs) ([]byte, error) { +func (c *capturingBundleBuilder) BuildBundle(gc opa.GroupCatalogs, gs opa.GroupScopes) ([]byte, error) { c.last = gc - return c.inner.BuildBundle(gc) + c.lastCatalogs = gc + c.lastScopes = gs + return c.inner.BuildBundle(gc, gs) } const testCellID = "cell-test" @@ -1928,3 +1932,293 @@ func TestTrinoHoglakeCatalogProperties(t *testing.T) { }) } } + +// --- per-user logins --- + +func teamID(id int64) *int64 { return &id } + +// The point of the feature: an org's OWN duckgres logins each authenticate to +// Trino, under ., with the very same bcrypt hash +// they use on pgwire. The bare org principal survives alongside them, so +// anything configured before per-user logins existed keeps working. +func TestBuildTrinoAuthFiles_ProjectsEveryOrgUser(t *testing.T) { + orgs := []configstore.TrinoEnabledOrg{{ + OrgID: "42", + DatabaseName: "acme", + RootPasswordHash: "$2a$10$roothash", + Users: []configstore.TrinoOrgUser{ + {Username: "root", PasswordHash: "$2a$10$roothash"}, + {Username: "analyst", PasswordHash: "$2a$10$analysthash"}, + }, + }} + pw, grp := BuildTrinoAuthFiles(orgs, TrinoClusterPrincipals{}) + + wantPW := "acme:$2a$10$roothash\n" + + "acme.root:$2a$10$roothash\n" + + "acme.analyst:$2a$10$analysthash\n" + if pw != wantPW { + t.Errorf("password.db =\n%q\nwant\n%q", pw, wantPW) + } + // All three principals share the org group, which the bundle grants the + // whole catalog, and all three carry the tier claim that routes them to + // the org's resource group. + wantGrp := "org_acme:acme,acme.analyst,acme.root\n" + + "tier_free:acme,acme.analyst,acme.root\n" + if grp != wantGrp { + t.Errorf("group.db =\n%q\nwant\n%q", grp, wantGrp) + } +} + +// A project-scoped login joins its scope group and NOT the org group. The org +// group is unscoped in the bundle, so membership in it would hand the login +// the whole catalog and defeat the scope entirely. +func TestBuildTrinoAuthFiles_ScopedUserJoinsOnlyItsScopeGroup(t *testing.T) { + orgs := []configstore.TrinoEnabledOrg{{ + OrgID: "42", + DatabaseName: "acme", + RootPasswordHash: "$2a$10$roothash", + Users: []configstore.TrinoOrgUser{ + {Username: "analyst", PasswordHash: "$2a$10$analysthash"}, + { + Username: "posthog_team_7", + PasswordHash: "$2a$10$teamhash", + TeamID: teamID(7), + Scope: &configstore.OrgUserQueryAccess{ReadOnly: true, AllowedSchemas: []string{"posthog_7"}}, + }, + }, + }} + _, grp := BuildTrinoAuthFiles(orgs, TrinoClusterPrincipals{}) + + wantGrp := "org_acme:acme,acme.analyst\n" + + "scope_acme_team_7:acme.posthog_team_7\n" + + "tier_free:acme,acme.analyst,acme.posthog_team_7\n" + if grp != wantGrp { + t.Errorf("group.db =\n%q\nwant\n%q", grp, wantGrp) + } + if strings.Contains(grp, "org_acme:acme,acme.analyst,acme.posthog_team_7") { + t.Error("the scoped login must NOT be in the unscoped org group") + } +} + +// Several logins on one team share one scope group — the group is keyed on +// (org, team), not on the user. +func TestBuildTrinoAuthFiles_ScopeGroupIsSharedPerTeam(t *testing.T) { + scope := &configstore.OrgUserQueryAccess{ReadOnly: true, AllowedSchemas: []string{"posthog_7"}} + orgs := []configstore.TrinoEnabledOrg{{ + OrgID: "42", + DatabaseName: "acme", + Users: []configstore.TrinoOrgUser{ + {Username: "reader", PasswordHash: "$2a$10$a", TeamID: teamID(7), Scope: scope}, + {Username: "writer", PasswordHash: "$2a$10$b", TeamID: teamID(7), Scope: scope}, + }, + }} + _, grp := BuildTrinoAuthFiles(orgs, TrinoClusterPrincipals{}) + if want := "scope_acme_team_7:acme.reader,acme.writer\n" + + "tier_free:acme.reader,acme.writer\n"; grp != want { + t.Errorf("group.db =\n%q\nwant\n%q", grp, want) + } +} + +// duckgres validates a username as little more than "not empty", while +// password.db is `:` per line and group.db is +// `:,`. A username carrying `:`, `,` or a newline would +// let whoever can create org users append arbitrary lines to those files — +// including a line for the admin principal. The grammar is an allowlist, so +// such a row is never rendered at all. +func TestBuildTrinoAuthFiles_RefusesUsernamesThatCouldInjectLines(t *testing.T) { + orgs := []configstore.TrinoEnabledOrg{{ + OrgID: "42", + DatabaseName: "acme", + RootPasswordHash: "$2a$10$roothash", + Users: []configstore.TrinoOrgUser{ + {Username: "ok", PasswordHash: "$2a$10$okhash"}, + {Username: "evil\n__admin_provisioner", PasswordHash: "$2a$10$attacker"}, + {Username: "has:colon", PasswordHash: "$2a$10$x"}, + {Username: "has,comma", PasswordHash: "$2a$10$x"}, + {Username: "has space", PasswordHash: "$2a$10$x"}, + {Username: "has.dot", PasswordHash: "$2a$10$x"}, + {Username: "", PasswordHash: "$2a$10$x"}, + }, + }} + pw, grp := BuildTrinoAuthFiles(orgs, TrinoClusterPrincipals{}) + + if want := "acme:$2a$10$roothash\nacme.ok:$2a$10$okhash\n"; pw != want { + t.Errorf("password.db =\n%q\nwant\n%q", pw, want) + } + if strings.Contains(pw, "$2a$10$attacker") { + t.Error("an injected admin line reached password.db") + } + for _, bad := range []string{"has:colon", "has,comma", "has space", "has.dot"} { + if strings.Contains(pw, bad) || strings.Contains(grp, bad) { + t.Errorf("username %q must not be projected", bad) + } + } + // Every rendered line must still be exactly one `user:hash` pair. + for _, line := range strings.Split(strings.TrimSuffix(pw, "\n"), "\n") { + if strings.Count(line, ":") != 1 { + t.Errorf("password.db line %q is not a single user:hash pair", line) + } + } +} + +// An org with per-user logins but no root hash still projects those logins: +// the bare org principal is one credential among several now, not the only +// way in. +func TestBuildTrinoAuthFiles_ProjectsUsersWithoutARootHash(t *testing.T) { + orgs := []configstore.TrinoEnabledOrg{{ + OrgID: "42", + DatabaseName: "acme", + Users: []configstore.TrinoOrgUser{{Username: "analyst", PasswordHash: "$2a$10$a"}}, + }} + pw, grp := BuildTrinoAuthFiles(orgs, TrinoClusterPrincipals{}) + if want := "acme.analyst:$2a$10$a\n"; pw != want { + t.Errorf("password.db = %q, want %q", pw, want) + } + if want := "org_acme:acme.analyst\ntier_free:acme.analyst\n"; grp != want { + t.Errorf("group.db = %q, want %q", grp, want) + } +} + +// Two orgs deriving the same Trino username is a cross-tenant authentication +// bug: password.db is one flat namespace per cell, so the duplicate line lets +// one org's user authenticate against the other's entry. Valid database_names +// make it unreachable, but grandfathered rows may hold a dot — which is how +// org `acme.analyst` and org `acme` + login `analyst` come to claim one name. +func TestRejectPrincipalCollisions_HoldsBackOrgsSharingATrinoUsername(t *testing.T) { + orgs := []configstore.TrinoEnabledOrg{ + {OrgID: "1", DatabaseName: "acme.analyst", RootPasswordHash: "$2a$10$a"}, + { + OrgID: "2", + DatabaseName: "acme", + RootPasswordHash: "$2a$10$b", + Users: []configstore.TrinoOrgUser{{Username: "analyst", PasswordHash: "$2a$10$c"}}, + }, + } + projectable, collisions := rejectPrincipalCollisions(orgs) + if len(projectable) != 0 { + t.Errorf("both orgs must be held back, got %d projectable", len(projectable)) + } + for _, id := range []string{"1", "2"} { + if collisions[id] == nil { + t.Errorf("org %s must be reported as colliding", id) + } + } +} + +// A tenant that derives one of the cell's own principals must be held back: +// the OPA policy's admin authority rests on that username belonging to the +// provisioner alone. +func TestRejectPrincipalCollisions_HoldsBackATenantClaimingAnOperationalPrincipal(t *testing.T) { + orgs := []configstore.TrinoEnabledOrg{{ + OrgID: "1", + DatabaseName: opa.AdminPrincipal, + RootPasswordHash: "$2a$10$a", + }} + projectable, collisions := rejectPrincipalCollisions(orgs) + if len(projectable) != 0 { + t.Errorf("the org must be held back, got %d projectable", len(projectable)) + } + if collisions["1"] == nil { + t.Error("claiming the admin principal must be reported as a collision") + } +} + +// Orgs with no user rows at all must project exactly as they did before +// per-user logins existed. This is the regression guard for every tenant on +// the cell today. +func TestBuildTrinoAuthFiles_UnchangedForOrgsWithoutUsers(t *testing.T) { + orgs := []configstore.TrinoEnabledOrg{ + {OrgID: "42", DatabaseName: "db42", RootPasswordHash: "$2a$10$hash42"}, + {OrgID: "43", DatabaseName: "db43", RootPasswordHash: "$2a$10$hash43"}, + } + pw, grp := BuildTrinoAuthFiles(orgs, TrinoClusterPrincipals{}) + if want := "db42:$2a$10$hash42\ndb43:$2a$10$hash43\n"; pw != want { + t.Errorf("password.db = %q, want %q", pw, want) + } + if want := "org_db42:db42\norg_db43:db43\ntier_free:db42,db43\n"; grp != want { + t.Errorf("group.db = %q, want %q", grp, want) + } +} + +// The resource-group selector must map BOTH the bare org principal and every +// qualified per-user login onto the SAME leaf group. The previous +// `(?.*)` capture matched the whole username, which would give each user +// a private leaf carrying the full per-tenant limits — an org with ten logins +// would quietly hold ten times its concurrency and memory budget. +// +// The constant is a Java regex (Trino compiles it with java.util.regex) and +// Go spells named groups `(?P<...>`, so the test translates that one token +// and nothing else. Trino matches with Pattern.matcher(user).matches(), i.e. +// a full match, which MustCompile + FindStringSubmatch on an anchored pattern +// reproduces. +func TestBuildTrinoResourceGroups_CapturesOrgFromQualifiedUsername(t *testing.T) { + goPattern := strings.ReplaceAll(orgCaptureRegex, "(?<", "(?P<") + re := regexp.MustCompile("^(?:" + goPattern + ")$") + idx := re.SubexpIndex("org") + if idx < 0 { + t.Fatalf("pattern %q has no `org` capture", orgCaptureRegex) + } + + for _, tc := range []struct{ user, want string }{ + {"acme", "acme"}, + {"acme.root", "acme"}, + {"acme.analyst", "acme"}, + {"acme.posthog_team_7", "acme"}, + {"acme-analytics.analyst", "acme-analytics"}, + } { + m := re.FindStringSubmatch(tc.user) + if m == nil { + t.Errorf("user %q does not match the selector at all — its queries would be rejected", tc.user) + continue + } + if m[idx] != tc.want { + t.Errorf("user %q captured org %q, want %q", tc.user, m[idx], tc.want) + } + } +} + +// The bundle must grant a scope group the SAME catalog its org group owns — +// the cross-tenant check is the same check for both — and additionally carry +// the scope that narrows it. +func TestReconcileOPABundle_ScopeGroupOwnsTheSameCatalog(t *testing.T) { + p := &TrinoProvisioner{} + captured := &capturingBundleBuilder{inner: opa.NewBuilder()} + p.bundleBuilder = captured + p.bundleStore = &opa.BundleStore{} + + orgs := []configstore.TrinoEnabledOrg{{ + OrgID: "42", + DatabaseName: "acme", + Users: []configstore.TrinoOrgUser{{ + Username: "posthog_team_7", + PasswordHash: "$2a$10$a", + TeamID: teamID(7), + Scope: &configstore.OrgUserQueryAccess{ + ReadOnly: true, + AllowedSchemas: []string{"posthog_7"}, + AllowedRelations: []string{"posthog.events"}, + }, + }}, + }} + if err := p.reconcileOPABundle(context.Background(), orgs); err != nil { + t.Fatalf("reconcileOPABundle: %v", err) + } + + gc, gs := captured.lastCatalogs, captured.lastScopes + if !gc["org_acme"]["org_acme"] { + t.Error("org group must own its catalog") + } + if !gc["scope_acme_team_7"]["org_acme"] { + t.Error("scope group must own the SAME catalog as its org group") + } + if _, ok := gs["org_acme"]; ok { + t.Error("the org group must stay unscoped") + } + scope, ok := gs["scope_acme_team_7"] + if !ok { + t.Fatal("scope group must carry a scope document") + } + if !scope.Schemas["posthog_7"] || !scope.Relations["posthog.events"] || !scope.RelationSchemas["posthog"] { + t.Errorf("scope = %#v, want the team's schemas and relations", scope) + } +} diff --git a/tests/configstore/trino_postgres_test.go b/tests/configstore/trino_postgres_test.go index 8c328d7a..6b66d1dc 100644 --- a/tests/configstore/trino_postgres_test.go +++ b/tests/configstore/trino_postgres_test.go @@ -3,6 +3,8 @@ package configstore_test import ( + "reflect" + "slices" "testing" "time" @@ -341,3 +343,132 @@ func TestEnableTrinoOnUnknownOrgViolatesForeignKeyPostgres(t *testing.T) { t.Fatal("expected a foreign-key violation enabling Trino for an org that does not exist") } } + +// Every one of an org's own logins must reach the Trino projection, not just +// `root` — that is the whole point of per-user Trino access. The listing also +// has to fail closed on a disabled user, which must never reach a password +// file. +func TestListTrinoEnabledOrgsProjectsEveryLogin(t *testing.T) { + store := newIsolatedConfigStore(t) + seedTrinoOrg(t, store, "acme") + if err := store.EnableTrino("acme", configstore.TrinoSettings{Tier: "free"}); err != nil { + t.Fatalf("EnableTrino: %v", err) + } + for _, u := range []struct{ name, hash string }{ + {"analyst", "$2a$10$analyst"}, + {"dashboards", "$2a$10$dashboards"}, + {"leaver", "$2a$10$leaver"}, + } { + if err := store.CreateOrgUser("acme", u.name, u.hash); err != nil { + t.Fatalf("CreateOrgUser(%s): %v", u.name, err) + } + } + if err := store.SetOrgUserDisabled("acme", "leaver", true); err != nil { + t.Fatalf("SetOrgUserDisabled: %v", err) + } + if err := store.ReloadSnapshot(); err != nil { + t.Fatalf("ReloadSnapshot: %v", err) + } + + got, err := store.ListTrinoEnabledOrgs() + if err != nil { + t.Fatalf("ListTrinoEnabledOrgs: %v", err) + } + if len(got) != 1 { + t.Fatalf("expected 1 org, got %d", len(got)) + } + byName := map[string]configstore.TrinoOrgUser{} + for _, u := range got[0].Users { + byName[u.Username] = u + } + // root appears here too, alongside the bare org principal, so the same + // credential works under either username. + for _, want := range []struct{ name, hash string }{ + {"root", "$2a$10$hash-acme"}, + {"analyst", "$2a$10$analyst"}, + {"dashboards", "$2a$10$dashboards"}, + } { + u, ok := byName[want.name] + if !ok { + t.Errorf("login %q missing from the projection", want.name) + continue + } + // The hash is copied through unchanged: it is the SAME bcrypt the + // pgwire handshake verifies, so one password works on both engines. + if u.PasswordHash != want.hash { + t.Errorf("login %q hash = %q, want %q", want.name, u.PasswordHash, want.hash) + } + if u.Scope != nil { + t.Errorf("login %q must be unscoped", want.name) + } + } + if _, ok := byName["leaver"]; ok { + t.Error("a disabled login must never reach the password file") + } + if len(byName) != 3 { + t.Errorf("projected logins = %v, want exactly root/analyst/dashboards", byName) + } +} + +// A project-scoped login must arrive carrying the SAME scope the pgwire +// session path enforces, so Trino and DuckDB cannot disagree about which +// schemas the login may read. +func TestListTrinoEnabledOrgsCarriesProjectScopes(t *testing.T) { + store := newIsolatedConfigStore(t) + seedTrinoOrg(t, store, "acme") + if err := store.EnableTrino("acme", configstore.TrinoSettings{Tier: "free"}); err != nil { + t.Fatalf("EnableTrino: %v", err) + } + if _, err := configstore.UpsertOrgTeamTx(store.DB(), "acme", configstore.OrgTeamUpsert{ + TeamID: 7, + SchemaName: "posthog_7", + }); err != nil { + t.Fatalf("UpsertOrgTeamTx: %v", err) + } + if err := store.CreateOrgUser("acme", "posthog_team_7", "$2a$10$team7"); err != nil { + t.Fatalf("CreateOrgUser: %v", err) + } + // No configstore mutator binds a login to a team (the admin API owns that + // surface), so bind it directly — the point under test is the listing, + // not the admin handler. + if err := store.DB().Exec( + `UPDATE duckgres_org_users SET access_mode = 'project_reader', team_id = 7 + WHERE org_id = 'acme' AND username = 'posthog_team_7'`).Error; err != nil { + t.Fatalf("bind project login: %v", err) + } + if err := store.ReloadSnapshot(); err != nil { + t.Fatalf("ReloadSnapshot: %v", err) + } + + got, err := store.ListTrinoEnabledOrgs() + if err != nil { + t.Fatalf("ListTrinoEnabledOrgs: %v", err) + } + var scoped *configstore.TrinoOrgUser + for i, u := range got[0].Users { + if u.Username == "posthog_team_7" { + scoped = &got[0].Users[i] + } + } + if scoped == nil { + t.Fatal("the project login is missing from the projection") + } + if scoped.Scope == nil { + t.Fatal("the project login must carry a scope, or it would read the whole catalog") + } + if scoped.TeamID == nil || *scoped.TeamID != 7 { + t.Fatalf("TeamID = %v, want 7 — the scope group is keyed on it", scoped.TeamID) + } + // Exactly what OrgUserQueryAccess reports for the same user, which is + // what pgwire enforces. + want, ok := store.OrgUserQueryAccess("acme", "posthog_team_7") + if !ok { + t.Fatal("OrgUserQueryAccess must report the login as scoped") + } + if !reflect.DeepEqual(*scoped.Scope, want) { + t.Errorf("scope = %+v, want %+v (the same policy pgwire enforces)", *scoped.Scope, want) + } + if !slices.Contains(scoped.Scope.AllowedSchemas, "posthog_7") { + t.Errorf("AllowedSchemas = %v, must contain the team's schema", scoped.Scope.AllowedSchemas) + } +} From b63e92f8fb0858fa6ba0dcc4e1b208f1bb3edac9 Mon Sep 17 00:00:00 2001 From: James Greenhill Date: Thu, 17 Sep 2026 00:18:13 +0000 Subject: [PATCH 2/2] Attribute per-user Trino logins to their org and advertise per-org hosts Per-user logins authenticate as ., but usage metering and the admin console still mapped a Trino principal to an org by exact match on the bare database_name. A per-user login's queries were silently dropped from usage events and shown with no org in the console. Both now resolve through configstore.NewTrinoPrincipalOwners, built from the same principals the auth files project, so a principal outside the password file is never attributed to anyone. The usage team resolver gets the duckgres username rather than the principal. A Trino client URL may now use {database_name} as its leading host label. The org detail then advertises . with username root: the Trino fork qualifies a login with the org its host names, so a user connects with the same host and username on both engines. The Trino e2e lane asserts per-user logins end to end: authentication with the pgwire password, own-catalog reads, cross-tenant denial, org attribution in the admin query list, and removal on disable. The host-qualified login is asserted when TRINO_HOST_QUALIFIED_DOMAIN is set, and logged as skipped until the lane's pinned image carries the fork change. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 17 +++++- controlplane/admin/trino.go | 55 ++++++++++++++----- controlplane/admin/trino_test.go | 54 ++++++++++++++++++- controlplane/configstore/models.go | 41 ++++++++++++++ controlplane/trino_registry.go | 7 ++- controlplane/trino_registry_test.go | 55 ++++++++++++------- controlplane/trino_usage_collector.go | 15 +++--- controlplane/trino_usage_collector_test.go | 39 +++++++++++++- tests/mw-dev/README.md | 12 +++++ tests/mw-dev/e2e/trino.sh | 62 ++++++++++++++++++++-- 10 files changed, 308 insertions(+), 49 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5fc55f11..e6e44a81 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1589,7 +1589,15 @@ password/tenant/catalog changes never propagate. and the coordinator is reachable. `DUCKGRES_TRINO_CLIENT_URL` names the HTTPS endpoint clients should dial; when unset, the control plane falls back to its coordinator URL and TLS server name. The response contains host, port and the - tenant principal, never a password or password hash. + tenant principal, never a password or password hash. A client URL whose + leading host label is `{database_name}` (registry `client_url` or the env + var; `ResolveTrinoClientURL`) advertises each org's own + `.` with username `root`: the Trino fork qualifies a + login with the org its host names + (`http-server.authentication.password.host-qualified-user.domains`), so a + user types the same username and host on both engines. Set it only once + that host actually reaches Trino — advertising an unrouted host is worse + than advertising the shared one. - **`Reconcile` order is load-bearing**: cluster secrets → auth files → resource groups → OPA bundle → tenant passwords → catalogs, and the `globalErr` gate SKIPS the catalog step if any projection failed. A @@ -1633,6 +1641,13 @@ password/tenant/catalog changes never propagate. `opa.ManagedCatalogPattern`, and the regex literal inside `policy.rego`. `TestTrinoCatalogNameMatchesManagedNamePattern` + `TestPolicyRegoContainsManagedNamePattern` fail if any one moves alone. +- **A Trino principal resolves to its org by EXACT lookup, never by splitting.** + `configstore.NewTrinoPrincipalOwners` indexes the principals the auth files + project (bare `` → root, `.` → that + login). Usage metering (`trino_usage_collector.go`, which also passes the + duckgres username — not the principal — to the team resolver) and the admin + console both use it, so a per-user login's queries are its org's usage and a + principal outside the password file is never attributed to anyone. - **Every duckgres login authenticates to Trino, under `.`.** `ListTrinoEnabledOrgs` returns the org's logins in `Users`, and `BuildTrinoAuthFiles` writes one `password.db` line per login with the diff --git a/controlplane/admin/trino.go b/controlplane/admin/trino.go index c53885a8..4c5b10eb 100644 --- a/controlplane/admin/trino.go +++ b/controlplane/admin/trino.go @@ -103,8 +103,33 @@ type TrinoOrgStatus struct { QueuedQueries int `json:"queued_queries"` } -func (c TrinoCell) connectionFor(username string) *TrinoConnection { - if username == "" { +// TrinoClientHostPlaceholder, as the leading label of a cell's client URL, +// stands for each org's database_name: `https://{database_name}.` +// gives every org the same host name it uses for pgwire. Trino qualifies a +// login with the org that host names, so the connection then advertises the +// org's own duckgres username (root) rather than the bare org principal. +const TrinoClientHostPlaceholder = "{database_name}" + +// ResolveTrinoClientURL substitutes databaseName into a client URL template. +// A URL without the placeholder is returned unchanged with perOrgHost false. +// ok is false when the placeholder is anywhere but the leading host label, or +// databaseName is not a DNS label and so cannot be a host name label. +func ResolveTrinoClientURL(template, databaseName string) (resolved string, perOrgHost bool, ok bool) { + if !strings.Contains(template, TrinoClientHostPlaceholder) { + return template, false, true + } + const prefix = "https://" + TrinoClientHostPlaceholder + "." + if !strings.HasPrefix(template, prefix) || strings.Count(template, TrinoClientHostPlaceholder) != 1 { + return "", true, false + } + if configstore.ValidateDatabaseName(databaseName) != nil { + return "", true, false + } + return strings.Replace(template, TrinoClientHostPlaceholder, databaseName, 1), true, true +} + +func (c TrinoCell) connectionFor(principal string) *TrinoConnection { + if principal == "" { return nil } @@ -112,6 +137,10 @@ func (c TrinoCell) connectionFor(username string) *TrinoConnection { if clientURL == "" { clientURL = c.CoordinatorURL } + clientURL, perOrgHost, ok := ResolveTrinoClientURL(clientURL, principal) + if !ok { + return nil + } parsedClientURL, err := url.Parse(clientURL) if err != nil || parsedClientURL.Scheme != "https" || parsedClientURL.Hostname() == "" { return nil @@ -130,6 +159,10 @@ func (c TrinoCell) connectionFor(username string) *TrinoConnection { host = c.TLSServerName } + username := principal + if perOrgHost { + username = "root" + } return &TrinoConnection{Host: host, Port: port, Username: username} } @@ -277,7 +310,7 @@ func registerTrinoAPI(r *gin.RouterGroup, api *TrinoAPI) { // principalIndex maps Trino principals to org ids, and carries the org // rows the handlers annotate with. type principalIndex struct { - orgByPrincipal map[string]string + orgByPrincipal configstore.TrinoPrincipalOwners rows []configstore.TrinoEnabledOrg } @@ -286,7 +319,7 @@ func (a *TrinoAPI) index() (principalIndex, error) { if err != nil { return principalIndex{}, err } - idx := principalIndex{orgByPrincipal: make(map[string]string, len(rows)), rows: rows} + idx := principalIndex{rows: rows} if a.filterCell { idx.rows = nil for _, row := range rows { @@ -295,11 +328,7 @@ func (a *TrinoAPI) index() (principalIndex, error) { } } } - for _, o := range idx.rows { - if p := o.TrinoPrincipal(); p != "" { - idx.orgByPrincipal[p] = o.OrgID - } - } + idx.orgByPrincipal = configstore.NewTrinoPrincipalOwners(idx.rows) return idx, nil } @@ -327,12 +356,12 @@ func (a *TrinoAPI) liveQueries(ctx context.Context, known ...principalIndex) ([] // The coordinator answered; a config-store blip should degrade the // org column, not the whole live view. slog.Warn("admin: trino org index unavailable, serving unannotated queries", "error", idxErr) - return queries, principalIndex{orgByPrincipal: map[string]string{}}, nil + return queries, principalIndex{orgByPrincipal: configstore.TrinoPrincipalOwners{}}, nil } out := make([]TrinoQuery, len(queries)) copy(out, queries) for i := range out { - out[i].Org = idx.orgByPrincipal[out[i].Principal] + out[i].Org = idx.orgByPrincipal.OrgID(out[i].Principal) } return out, idx, nil } @@ -496,7 +525,7 @@ func (a *TrinoAPI) handleQueryDetail(c *gin.Context) { return } if idx, idxErr := a.index(); idxErr == nil { - q.Org = idx.orgByPrincipal[q.Principal] + q.Org = idx.orgByPrincipal.OrgID(q.Principal) } c.JSON(http.StatusOK, q) } @@ -530,7 +559,7 @@ func (a *TrinoAPI) handleKillQuery(c *gin.Context) { targetOrg := "" if q, err := a.client.Query(c.Request.Context(), queryID); err == nil { if idx, idxErr := a.index(); idxErr == nil { - targetOrg = idx.orgByPrincipal[q.Principal] + targetOrg = idx.orgByPrincipal.OrgID(q.Principal) } } diff --git a/controlplane/admin/trino_test.go b/controlplane/admin/trino_test.go index eb7b25c6..254bb084 100644 --- a/controlplane/admin/trino_test.go +++ b/controlplane/admin/trino_test.go @@ -111,7 +111,8 @@ func trinoTestRouter(api *TrinoAPI, role Role) *gin.Engine { func twoOrgTrinoStore() *fakeTrinoOrgStore { return &fakeTrinoOrgStore{ orgs: []configstore.TrinoEnabledOrg{ - {OrgID: "org-a", DatabaseName: "db_a", Tier: "free", CellID: "cell-test", State: configstore.ManagedWarehouseStateReady, RootPasswordHash: "$2a$10$secrethash"}, + {OrgID: "org-a", DatabaseName: "db_a", Tier: "free", CellID: "cell-test", State: configstore.ManagedWarehouseStateReady, RootPasswordHash: "$2a$10$secrethash", + Users: []configstore.TrinoOrgUser{{Username: "analyst", PasswordHash: "$2a$10$analysthash"}}}, {OrgID: "org-b", DatabaseName: "db_b", Tier: "scale", CellID: "cell-test", State: configstore.ManagedWarehouseStatePending, RootPasswordHash: "$2a$10$othersecret"}, }, } @@ -158,6 +159,10 @@ func TestQueriesAreAnnotatedWithTheOwningOrg(t *testing.T) { coord := &fakeTrinoCoordinator{queries: []TrinoQuery{ {QueryID: "q1", State: "RUNNING", Principal: "db_a", ElapsedMS: 100}, {QueryID: "q2", State: "RUNNING", Principal: "db_b", ElapsedMS: 200}, + // A per-user login authenticates as .. + {QueryID: "q4", State: "RUNNING", Principal: "db_a.analyst", ElapsedMS: 300}, + // Not in the password file: the org prefix alone must not attribute it. + {QueryID: "q5", State: "RUNNING", Principal: "db_a.nobody", ElapsedMS: 400}, // A query from a principal that is not a tenant: the provisioner's // own reconcile DDL. It must appear with an EMPTY org rather than // be silently attributed to someone. @@ -174,7 +179,7 @@ func TestQueriesAreAnnotatedWithTheOwningOrg(t *testing.T) { q := raw.(map[string]any) got[q["query_id"].(string)] = q["org"].(string) } - want := map[string]string{"q1": "org-a", "q2": "org-b", "q3": ""} + want := map[string]string{"q1": "org-a", "q2": "org-b", "q3": "", "q4": "org-a", "q5": ""} for id, wantOrg := range want { if got[id] != wantOrg { t.Errorf("query %s: org = %q, want %q", id, got[id], wantOrg) @@ -596,6 +601,51 @@ func TestReadyOrgDetailReturnsTenantClientConnection(t *testing.T) { } } +// With a per-org client host, an org connects to Trino at the same +// . it uses for pgwire, as its own duckgres login: +// Trino qualifies that login with the org the host names. +func TestReadyOrgDetailAdvertisesPerOrgClientHost(t *testing.T) { + store := &fakeTrinoOrgStore{ + orgs: []configstore.TrinoEnabledOrg{{OrgID: "org-a", DatabaseName: "tenant-a", CellID: "cell-test", State: configstore.ManagedWarehouseStateReady}}, + rows: map[string]*configstore.ManagedWarehouseTrino{ + "org-a": {OrgID: "org-a", Enabled: true, TrinoCellID: "cell-test", State: configstore.ManagedWarehouseStateReady}, + }, + } + api := NewTrinoAPI(TrinoCell{ID: "cell-test", CoordinatorURL: "https://coordinator.invalid", ClientURL: "https://{database_name}.dw.example.com"}, &fakeTrinoCoordinator{}, store, nil) + r := trinoTestRouter(api, RoleViewer) + + code, body := doTrinoJSON(t, r, http.MethodGet, "/api/v1/orgs/org-a/trino", "") + if code != http.StatusOK { + t.Fatalf("expected 200, got %d", code) + } + connection := body["status"].(map[string]any)["connection"].(map[string]any) + if connection["host"] != "tenant-a.dw.example.com" || connection["port"] != float64(443) || connection["username"] != "root" { + t.Errorf("connection = %v, want tenant-a.dw.example.com:443 as root", connection) + } +} + +func TestResolveTrinoClientURL(t *testing.T) { + for _, tc := range []struct { + template, database, want string + perOrg, ok bool + }{ + {"https://trino.example.com", "tenant-a", "https://trino.example.com", false, true}, + {"https://{database_name}.dw.example.com", "tenant-a", "https://tenant-a.dw.example.com", true, true}, + {"https://{database_name}.dw.example.com:8443", "tenant-a", "https://tenant-a.dw.example.com:8443", true, true}, + // A grandfathered database_name that is no DNS label cannot be a host label. + {"https://{database_name}.dw.example.com", "db_a", "", true, false}, + {"https://{database_name}.dw.example.com", "a.b", "", true, false}, + {"https://gateway.{database_name}.example.com", "tenant-a", "", true, false}, + {"https://{database_name}.{database_name}.example.com", "tenant-a", "", true, false}, + {"http://{database_name}.dw.example.com", "tenant-a", "", true, false}, + } { + got, perOrg, ok := ResolveTrinoClientURL(tc.template, tc.database) + if got != tc.want || perOrg != tc.perOrg || ok != tc.ok { + t.Errorf("ResolveTrinoClientURL(%q, %q) = (%q, %v, %v), want (%q, %v, %v)", tc.template, tc.database, got, perOrg, ok, tc.want, tc.perOrg, tc.ok) + } + } +} + func TestTrinoCellAPIAliasPreservesStoredOwnership(t *testing.T) { for _, storedID := range []string{"stored-cell", "another-cell", "legacy", ""} { t.Run("stored="+storedID, func(t *testing.T) { diff --git a/controlplane/configstore/models.go b/controlplane/configstore/models.go index c71d2641..bf017467 100644 --- a/controlplane/configstore/models.go +++ b/controlplane/configstore/models.go @@ -587,6 +587,47 @@ func (o TrinoEnabledOrg) TrinoUserPrincipal(username string) string { return principal + TrinoPrincipalSeparator + username } +// TrinoPrincipalOwner is the duckgres login a Trino principal authenticated +// as: the org, and the org user whose password line it matched. +type TrinoPrincipalOwner struct { + OrgID string + Username string +} + +// TrinoPrincipalOwners maps each Trino principal an org projects back to the +// duckgres login it belongs to. Trino reports only the principal, and every +// consumer that attributes a query to a tenant (usage metering, the admin +// console) must see a per-user login as its org's, not as an unknown user. +// +// It is built from the SAME principals BuildTrinoAuthFiles projects, by exact +// match, rather than by splitting on the separator, so a principal that is not +// in the password file never resolves to an org. +type TrinoPrincipalOwners map[string]TrinoPrincipalOwner + +// NewTrinoPrincipalOwners indexes the projected principals of orgs. The bare +// org principal authenticates with the root login's hash, so it resolves to +// root. +func NewTrinoPrincipalOwners(orgs []TrinoEnabledOrg) TrinoPrincipalOwners { + owners := make(TrinoPrincipalOwners, len(orgs)) + for _, o := range orgs { + if p := o.TrinoPrincipal(); p != "" { + owners[p] = TrinoPrincipalOwner{OrgID: o.OrgID, Username: "root"} + } + for _, u := range o.Users { + if p := o.TrinoUserPrincipal(u.Username); p != "" { + owners[p] = TrinoPrincipalOwner{OrgID: o.OrgID, Username: u.Username} + } + } + } + return owners +} + +// OrgID returns the org that owns principal, or "" for an operational or +// unknown principal. +func (owners TrinoPrincipalOwners) OrgID(principal string) string { + return owners[principal].OrgID +} + // TrinoPrincipal is the tenant's customer-facing identity in Trino: the // username it authenticates as, and the stem every derived name is built // from (catalog, group, resource group). diff --git a/controlplane/trino_registry.go b/controlplane/trino_registry.go index dde916e7..c934fa98 100644 --- a/controlplane/trino_registry.go +++ b/controlplane/trino_registry.go @@ -14,6 +14,7 @@ import ( "strconv" "strings" + "github.com/posthog/duckgres/controlplane/admin" "github.com/posthog/duckgres/controlplane/provisioner" "k8s.io/apimachinery/pkg/util/validation" ) @@ -152,7 +153,11 @@ func parseTrinoCellRegistry(data []byte) ([]trinoRegisteredCell, error) { return nil, errors.New("Trino cells must have distinct identities, namespaces and routing groups") } identities[cell.ID], namespaces[cell.Namespace], groups[cell.RoutingGroup] = true, true, true - if _, err := trinoEndpointKey(cell.ClientURL); err != nil { + clientURL, _, ok := admin.ResolveTrinoClientURL(cell.ClientURL, "org") + if !ok { + return nil, fmt.Errorf("Trino cell %s client URL: %s may only be the leading host label", cell.ID, admin.TrinoClientHostPlaceholder) + } + if _, err := trinoEndpointKey(clientURL); err != nil { return nil, fmt.Errorf("Trino cell %s client URL: %w", cell.ID, err) } backendIDs, secrets := map[string]bool{}, map[string]bool{} diff --git a/controlplane/trino_registry_test.go b/controlplane/trino_registry_test.go index 8b3259b1..e29a97fb 100644 --- a/controlplane/trino_registry_test.go +++ b/controlplane/trino_registry_test.go @@ -98,28 +98,43 @@ func TestTrinoRegistryPreservesStoppedBackend(t *testing.T) { } } +// A per-org client URL gives every org the host name it already uses for +// pgwire; the placeholder is validated as the org label it will become. +func TestTrinoRegistryAcceptsPerOrgClientHost(t *testing.T) { + data := strings.Replace(testTrinoRegistryJSON, `https://gateway.example.test`, `https://{database_name}.example.test`, 1) + cells, err := parseTrinoCellRegistry([]byte(data)) + if err != nil { + t.Fatal(err) + } + if cells[0].ClientURL != "https://{database_name}.example.test" { + t.Fatalf("client URL = %q", cells[0].ClientURL) + } +} + func TestTrinoRegistryRejectsUnsafeConfiguration(t *testing.T) { tests := map[string]string{ - "unknown field": strings.Replace(testTrinoRegistryJSON, `"cells":`, `"typo":`, 1), - "reserved legacy identity": strings.Replace(testTrinoRegistryJSON, `"id":"cell-test"`, `"id":"legacy"`, 1), - "unsafe namespace": strings.Replace(testTrinoRegistryJSON, `"namespace":"trino-test"`, `"namespace":"../other"`, 1), - "credentials field": strings.Replace(testTrinoRegistryJSON, `"cells":`, `"password":"secret","cells":`, 1), - "empty registry": `{"cells":[]}`, - "no backends": `{"cells":[{"id":"cell-test","namespace":"trino-test","client_url":"https://gateway.example.test","routing_group":"cell-test","backends":[]}]}`, - "plain HTTP": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `http://blue.example.test`, 1), - "embedded credentials": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `https://user:secret@blue.example.test`, 1), - "URL query": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `https://blue.example.test?token=value`, 1), - "URL path": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `https://blue.example.test/catalogs`, 1), - "active backend stopped": strings.Replace(testTrinoRegistryJSON, `"running":true`, `"running":false`, 1), - "no active backend": strings.Replace(testTrinoRegistryJSON, `"routing_active":true`, `"routing_active":false`, 1), - "two active backends": strings.Replace(testTrinoRegistryJSON, `"running":false,"routing_active":false`, `"running":true,"routing_active":true`, 1), - "duplicate backend identity": strings.Replace(testTrinoRegistryJSON, `"id":"green"`, `"id":"blue"`, 1), - "duplicate endpoint": strings.Replace(testTrinoRegistryJSON, `https://green.example.test`, `https://blue.example.test`, 1), - "duplicate canonical endpoint": strings.Replace(testTrinoRegistryJSON, `https://green.example.test`, `https://BLUE.example.test.:0443/`, 1), - "shared internal secret": strings.Replace(testTrinoRegistryJSON, `green-internal`, `blue-internal`, 1), - "invalid internal secret": strings.Replace(testTrinoRegistryJSON, `green-internal`, `../other`, 1), - "header injection": strings.Replace(testTrinoRegistryJSON, `"routing_group":"cell-test"`, `"routing_group":"cell-test\r\nHost: other"`, 1), - "trailing document": testTrinoRegistryJSON + `{}`, + "unknown field": strings.Replace(testTrinoRegistryJSON, `"cells":`, `"typo":`, 1), + "reserved legacy identity": strings.Replace(testTrinoRegistryJSON, `"id":"cell-test"`, `"id":"legacy"`, 1), + "unsafe namespace": strings.Replace(testTrinoRegistryJSON, `"namespace":"trino-test"`, `"namespace":"../other"`, 1), + "credentials field": strings.Replace(testTrinoRegistryJSON, `"cells":`, `"password":"secret","cells":`, 1), + "empty registry": `{"cells":[]}`, + "no backends": `{"cells":[{"id":"cell-test","namespace":"trino-test","client_url":"https://gateway.example.test","routing_group":"cell-test","backends":[]}]}`, + "plain HTTP": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `http://blue.example.test`, 1), + "embedded credentials": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `https://user:secret@blue.example.test`, 1), + "URL query": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `https://blue.example.test?token=value`, 1), + "URL path": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `https://blue.example.test/catalogs`, 1), + "active backend stopped": strings.Replace(testTrinoRegistryJSON, `"running":true`, `"running":false`, 1), + "no active backend": strings.Replace(testTrinoRegistryJSON, `"routing_active":true`, `"routing_active":false`, 1), + "two active backends": strings.Replace(testTrinoRegistryJSON, `"running":false,"routing_active":false`, `"running":true,"routing_active":true`, 1), + "duplicate backend identity": strings.Replace(testTrinoRegistryJSON, `"id":"green"`, `"id":"blue"`, 1), + "duplicate endpoint": strings.Replace(testTrinoRegistryJSON, `https://green.example.test`, `https://blue.example.test`, 1), + "duplicate canonical endpoint": strings.Replace(testTrinoRegistryJSON, `https://green.example.test`, `https://BLUE.example.test.:0443/`, 1), + "shared internal secret": strings.Replace(testTrinoRegistryJSON, `green-internal`, `blue-internal`, 1), + "invalid internal secret": strings.Replace(testTrinoRegistryJSON, `green-internal`, `../other`, 1), + "header injection": strings.Replace(testTrinoRegistryJSON, `"routing_group":"cell-test"`, `"routing_group":"cell-test\r\nHost: other"`, 1), + "trailing document": testTrinoRegistryJSON + `{}`, + "placeholder not leading label": strings.Replace(testTrinoRegistryJSON, `https://gateway.example.test`, `https://gateway.{database_name}.example.test`, 1), + "placeholder in path": strings.Replace(testTrinoRegistryJSON, `https://gateway.example.test`, `https://gateway.example.test/{database_name}`, 1), } for name, data := range tests { t.Run(name, func(t *testing.T) { diff --git a/controlplane/trino_usage_collector.go b/controlplane/trino_usage_collector.go index cf515e4c..9a8457a2 100644 --- a/controlplane/trino_usage_collector.go +++ b/controlplane/trino_usage_collector.go @@ -8,6 +8,7 @@ import ( "time" "github.com/posthog/duckgres/controlplane/admin" + "github.com/posthog/duckgres/controlplane/configstore" "github.com/posthog/duckgres/internal/analytics" ) @@ -70,10 +71,7 @@ func (c *trinoUsageCollector) collect(ctx context.Context) { slog.Warn("Trino usage collection skipped: list enabled orgs failed", "error", err) return } - orgByPrincipal := make(map[string]string, len(orgs)) - for _, org := range orgs { - orgByPrincipal[org.TrinoPrincipal()] = org.OrgID - } + owners := configstore.NewTrinoPrincipalOwners(orgs) queries, err := c.coordinator.Queries(ctx) if err != nil { slog.Warn("Trino usage collection skipped: list queries failed", "error", err) @@ -95,13 +93,16 @@ func (c *trinoUsageCollector) collect(ctx context.Context) { if _, ok := c.seen[query.QueryID]; ok { continue } - orgID := orgByPrincipal[query.Principal] - if orgID == "" { + owner, ok := owners[query.Principal] + if !ok { continue // operator queries do not represent tenant usage } + orgID := owner.OrgID teamID := int64(0) if c.teamID != nil { - teamID = c.teamID(orgID, query.Principal) + // The duckgres username, not the Trino principal: a project login's + // team is keyed on the login it authenticated as. + teamID = c.teamID(orgID, owner.Username) } props := trinoUsageProperties(query, teamID) if query.State == "FINISHED" { diff --git a/controlplane/trino_usage_collector_test.go b/controlplane/trino_usage_collector_test.go index 1b8505d4..95e7e9b2 100644 --- a/controlplane/trino_usage_collector_test.go +++ b/controlplane/trino_usage_collector_test.go @@ -70,7 +70,8 @@ func TestTrinoUsageCollectorCapturesTerminalQueriesOnce(t *testing.T) { {QueryID: "operator", State: "FINISHED", Principal: "__observer"}, }} collector := newTrinoUsageCollector(coordinator, trinoUsageFakeOrgs{orgs: []configstore.TrinoEnabledOrg{{OrgID: "org-a", DatabaseName: "tenant-db"}}}, func(org, user string) int64 { - if org != "org-a" || user != "tenant-db" { + // The bare org principal authenticates with root's password line. + if org != "org-a" || user != "root" { t.Fatalf("team lookup = (%q, %q)", org, user) } return 42 @@ -102,3 +103,39 @@ func TestTrinoUsageCollectorCapturesTerminalQueriesOnce(t *testing.T) { t.Errorf("failed = %#v", failed) } } + +// A per-user login authenticates as .. Its queries are +// the org's usage, attributed to the duckgres login's own team, while a +// principal outside the password file stays unattributed. +func TestTrinoUsageCollectorAttributesPerUserPrincipals(t *testing.T) { + tracker := &trinoUsageFakeTracker{} + analytics.SetDefault(tracker) + t.Cleanup(func() { analytics.SetDefault(nil) }) + + coordinator := &trinoUsageFakeCoordinator{queries: []admin.TrinoQuery{ + {QueryID: "analyst", State: "FINISHED", Principal: "tenant-db.analyst"}, + {QueryID: "unprojected", State: "FINISHED", Principal: "tenant-db.nobody"}, + }} + orgs := []configstore.TrinoEnabledOrg{{ + OrgID: "org-a", + DatabaseName: "tenant-db", + Users: []configstore.TrinoOrgUser{{Username: "analyst", PasswordHash: "$2a$10$hash"}}, + }} + var lookups []string + collector := newTrinoUsageCollector(coordinator, trinoUsageFakeOrgs{orgs: orgs}, func(org, user string) int64 { + lookups = append(lookups, org+"/"+user) + return 7 + }) + + collector.collect(context.Background()) + + if len(tracker.events) != 1 { + t.Fatalf("event count = %d, want 1 (only the projected login)", len(tracker.events)) + } + if got := tracker.events[0]; got.org != "org-a" || got.props["team_id"] != int64(7) { + t.Errorf("event = %#v, want org-a with team 7", got) + } + if len(lookups) != 1 || lookups[0] != "org-a/analyst" { + t.Errorf("team lookups = %v, want [org-a/analyst]", lookups) + } +} diff --git a/tests/mw-dev/README.md b/tests/mw-dev/README.md index da41f3f1..5f66c529 100644 --- a/tests/mw-dev/README.md +++ b/tests/mw-dev/README.md @@ -175,6 +175,18 @@ tests. That fork contains the DuckLake connector and PostgreSQL dynamic catalog store; upstream `trinodb/trino` is not compatible. Update the default in `run.sh`, `e2e-mw-dev.yml`, and `scenario-dev.yml` together when promoting a Trino build. +The suite asserts per-user logins on every run: an org user authenticates as +`.` with its pgwire password, reads only its own org's +catalog, is attributed to its org in the admin query list, and stops +authenticating once disabled. The host-qualified login, where the same user +types only `` against `.`, is asserted only +when `TRINO_HOST_QUALIFIED_DOMAIN` is set, and is logged as skipped otherwise. +It needs a fork build with +`http-server.authentication.password.host-qualified-user.domains`, which the +pinned image predates. When promoting such a build, set that property to the +same domain on the lane's coordinator and pass the domain to the harness Job. +The harness sends the tenant host as the `Host` header against the lane's own +TLS name, so no DNS or certificate for the tenant host is needed. Each Trino worker has requests and limits of 1 CPU and 4Gi. Together they match the frozen perf Duckgres worker's aggregate 3 CPU and 12Gi execution budget while exercising Trino's distributed execution path. Trino permits 2GB diff --git a/tests/mw-dev/e2e/trino.sh b/tests/mw-dev/e2e/trino.sh index 1dd12741..603dcfae 100644 --- a/tests/mw-dev/e2e/trino.sh +++ b/tests/mw-dev/e2e/trino.sh @@ -111,11 +111,14 @@ wait_trino() { # org expected-principal expected-catalog # Print all result rows as compact JSON. Trino's statement protocol pages via # nextUri; every follow-up keeps both Basic auth and the tenant identity. +# TRINO_HOST, when set, is sent as the Host header: the tenant host name a +# host-qualified login is resolved against (TLS still verifies the coordinator). trino_query() { # principal password sql principal="$1" password="$2" sql="$3" + set -- -H "X-Trino-User: $principal" -H 'X-Trino-Time-Zone: UTC' + [ -z "${TRINO_HOST:-}" ] || set -- "$@" -H "Host: $TRINO_HOST" response="$(curl --connect-timeout 5 --max-time 60 --cacert "$CA" -fsS --user "$principal:$password" \ - -H "X-Trino-User: $principal" -H 'X-Trino-Time-Zone: UTC' \ - --data-binary "$sql" "$TRINO/v1/statement")" || return 1 + "$@" --data-binary "$sql" "$TRINO/v1/statement")" || return 1 rows='[]' while :; do err="$(printf %s "$response" | jq -r '.error.message // empty')" @@ -124,7 +127,7 @@ trino_query() { # principal password sql next="$(printf %s "$response" | jq -r '.nextUri // empty')" [ -n "$next" ] || break response="$(curl --connect-timeout 5 --max-time 60 --cacert "$CA" -fsS --user "$principal:$password" \ - -H "X-Trino-User: $principal" -H 'X-Trino-Time-Zone: UTC' "$next")" || return 1 + "$@" "$next")" || return 1 done printf '%s\n' "$rows" } @@ -364,6 +367,57 @@ api "$API/api/v1/audit?org=$ORG_A" | jq -e --arg q "$query_id" \ 'any(.entries[]?; .action == "trino.query.kill" and .target_user == $q and .status == 200)' >/dev/null \ || fail "Trino query kill audit row missing" +log "per-user duckgres logins authenticate to Trino as ." +# Every org login is projected into the cell's password file under its +# qualified principal, with the same bcrypt hash pgwire verifies, so the same +# password works on both engines. Its queries belong to its org, it cannot +# reach another tenant, and disabling the login removes it from Trino. +analyst=analyst +analyst_principal="$DB_A.$analyst" +analyst_pw="$(head -c 18 /dev/urandom | od -An -tx1 | tr -d ' \n')" +api -X POST -H 'Content-Type: application/json' \ + -d "{\"org_id\":\"$ORG_A\",\"username\":\"$analyst\",\"password\":\"$analyst_pw\"}" \ + "$API/api/v1/users" >/dev/null +i=0 +while [ "$i" -lt "$TRINO_AUTH_ROTATION_ATTEMPTS" ]; do + trino_query "$analyst_principal" "$analyst_pw" 'SELECT 1' >/dev/null 2>&1 && break + sleep "$TRINO_AUTH_ROTATION_RETRY_SECONDS"; i=$((i + 1)) +done +[ "$i" -lt "$TRINO_AUTH_ROTATION_ATTEMPTS" ] || fail "per-user login $analyst_principal never authenticated to Trino" +[ "$(scalar "$analyst_principal" "$analyst_pw" "SELECT count(*) FROM $CAT_A.$schema.$table")" = 1 ] \ + || fail "per-user login cannot read its own org's catalog" +trino_query "$analyst_principal" "$pw_a" 'SELECT 1' >/dev/null 2>&1 \ + && fail "per-user login authenticated with the root password" +must_fail "$analyst_principal" "$analyst_pw" "SELECT * FROM $CAT_B.main.$foreign_table" 'denied|access|catalog|not found|does not exist' +marker="per_user_attribution_$PR" +trino_query "$analyst_principal" "$analyst_pw" "SELECT '$marker'" >/dev/null +api "$API/api/v1/trino/queries?org=$ORG_A" | jq -e --arg m "$marker" --arg p "$analyst_principal" --arg org "$ORG_A" \ + 'any(.queries[]; .principal == $p and .org == $org and (.query | contains($m)))' >/dev/null \ + || fail "admin query list did not attribute the per-user login's query to its org" + +if [ -n "${TRINO_HOST_QUALIFIED_DOMAIN:-}" ]; then + log "host-qualified login: $analyst on $DB_A.$TRINO_HOST_QUALIFIED_DOMAIN authenticates as $analyst_principal" + identity="$(TRINO_HOST="$DB_A.$TRINO_HOST_QUALIFIED_DOMAIN" trino_query "$analyst" "$analyst_pw" 'SELECT current_user')" \ + || fail "host-qualified login failed for $analyst on tenant A's host" + printf %s "$identity" | jq -e --arg p "$analyst_principal" '.[0][0] == $p' >/dev/null \ + || fail "host-qualified login ran as $identity, want $analyst_principal" + TRINO_HOST="$DB_B.$TRINO_HOST_QUALIFIED_DOMAIN" trino_query "$analyst" "$analyst_pw" 'SELECT 1' >/dev/null 2>&1 \ + && fail "tenant A's login authenticated on tenant B's host" +else + # Requires a Trino image with http-server.authentication.password.host-qualified-user + # (PostHog/trino) and that property set on the lane's coordinator; see + # tests/mw-dev/README.md "Isolated Trino lane". + log "SKIP host-qualified login: TRINO_HOST_QUALIFIED_DOMAIN is unset for this Trino image" +fi + +api -X POST "$API/api/v1/orgs/$ORG_A/users/$analyst/disable" >/dev/null +i=0 +while [ "$i" -lt "$TRINO_AUTH_ROTATION_ATTEMPTS" ]; do + trino_query "$analyst_principal" "$analyst_pw" 'SELECT 1' >/dev/null 2>&1 || break + sleep "$TRINO_AUTH_ROTATION_RETRY_SECONDS"; i=$((i + 1)) +done +[ "$i" -lt "$TRINO_AUTH_ROTATION_ATTEMPTS" ] || fail "disabled per-user login still authenticates to Trino" + log "password rotation" new_pw="$(api -X POST "$API/api/v1/orgs/$ORG_A/reset-password" | jq -r .password)" [ -n "$new_pw" ] && [ "$new_pw" != null ] || fail "password reset returned no password" @@ -427,4 +481,4 @@ if [ "${TRINO_MULTICELL_ENABLED:-false}" = true ]; then . /harness/trino-shared-catalogs.sh fi fi -log "PASS: isolated Trino provisioning + verified auth + DDL/DML + OPA isolation/batching + hot-add + admin + rotation + restart + disable" +log "PASS: isolated Trino provisioning + verified auth + per-user logins + DDL/DML + OPA isolation/batching + hot-add + admin + rotation + restart + disable"