diff --git a/CLAUDE.md b/CLAUDE.md index 5642264e..2b350962 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1577,6 +1577,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 94213506..42059576 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" @@ -188,9 +189,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 @@ -229,9 +236,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 9ac1d58c..7268677a 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 d0356fe2..586f3429 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) } @@ -1818,3 +1832,301 @@ func TestSystemNodesGrantIsObserverOnly(t *testing.T) { t.Error("claiming the observer group without the observer username must grant nothing") } } + +// -------------------------------------------------------------------------- +// 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) + } + } +} + +// 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 39b8830e..629cc059 100644 --- a/controlplane/provisioner/trino_provisioner.go +++ b/controlplane/provisioner/trino_provisioner.go @@ -225,6 +225,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- @@ -678,8 +714,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 @@ -699,6 +735,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 == "" { @@ -708,6 +757,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 @@ -719,15 +775,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 } @@ -748,6 +840,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. // @@ -1742,22 +1844,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 — @@ -1790,14 +1898,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 @@ -1824,6 +1974,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. // @@ -1911,9 +2090,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. @@ -2116,11 +2307,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() @@ -2130,6 +2329,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 @@ -2138,7 +2349,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 79be62dd..ed5ab5ea 100644 --- a/controlplane/provisioner/trino_provisioner_test.go +++ b/controlplane/provisioner/trino_provisioner_test.go @@ -747,13 +747,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" @@ -1871,3 +1875,293 @@ func TestCatalogHTTPClientTagsItsSource(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) + } +}