feat(database): Wave 1 authz MVP storage (#422) - #677
Conversation
Ship #422 portable Goose migrations (Postgres + Spanner) for resource_scope_index, catalog/assignment/membership-edge tables, hand-seed cat_sys_1 placeholders, backfill existing resources, and same-tx dual-write hooks on project/team/user/membership mutations with statement tests. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 6513a0d The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
CI server:format failed on four files that needed gofmt -w. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Spanner DDL rejects table-level UNIQUE constraints; create a separate UNIQUE INDEX for (catalog_kind, owner_id, version) instead. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Spanner surfaces CHECK constraint violations as gRPC OutOfRange; wrap them as database.CheckError so authz assignment and team name checks match Postgres semantics. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
The permission compiler needs a policy shape that profile validation, closure computation, and SQL planning can consume without depending on OpenFGA protobufs, so parsing stays with the upstream language package while Zitadel owns the intermediate representation. Refs #421 Co-authored-by: Cursor <cursoragent@cursor.com>
Schema uploads must fail fast on constructs that cannot be planned on PostgreSQL and Spanner, with stable detail codes that can later wrap into a domain error at the API boundary. Refs #421 Co-authored-by: Cursor <cursoragent@cursor.com>
Expand same-object computed usersets into a relation closure and emit storage-neutral catalog mutations plus query-plan terms for direct, computed, and TTU leaves (#421). Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Centralize RSI constructors and membership authz-active policy in domain, collapse team-deactivate edge cleanup to one SQL per dialect, delegate Postgres RSI methods through *On helpers, and drop redundant project RSI deletes (FK cascade). Extend deactivate cascade tests to assert edge removal. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Drop the parallel *On helper API and call newXStatements(tx) from entity dual-write paths, matching the existing session rebinding pattern. Move ActiveUniqueKey onto domain.AuthzAssignment and collapse CreateUser initial-membership team checks to one gate. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Delete the three identical dialect dualWrite adapters and identity wrappers. Entity statements construct tx-bound RSI/edge statements and call Sync/Upsert/Delete directly; only multi-write UserCreated, UserDeactivated, and UserDeleted remain in dialect/authz. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Spanner's partial-index stand-in is a dialect concern, not domain semantics. Keep a private helper beside the Spanner assignment insert. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Introduce typed string enums for resource/member/set/catalog/principal/ scope kinds, assignment scope constructors, and AuthzMembershipEdgeKey. Inline ExpressionEdgeKind into catalog_rows and keep a single dialect/authz package for shared Wave 1 helpers. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Return plain strings from typed membership-edge Accessors for CoerceString, expand AuthzPrincipalType to match DDL, clarify the delegation CHECK test, and use NewUserTeamMembershipEdgeKey in stmttest. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
…ssion-storage-wave1-3354
There was a problem hiding this comment.
Pull request overview
Implements Wave 1 of the authz MVP storage layer across Postgres/Spanner/SQLite: adds Goose migrations for the authz tables, introduces statement interfaces + dialect implementations for catalog persistence/assignments/membership edges/resource-scope indexing, and wires same-transaction dual-write hooks from existing project/team/user/membership mutations. This aligns the DB persistence surface with the Wave 0 DDL decisions and the Wave 0.5 compiler output (compiler.CatalogMutations).
Changes:
- Add authz MVP schema migrations (catalogs/relations/closure/references/expression_edges, assignments, membership edges, resource_scope_index) and seed
cat_sys_1. - Add new statement interfaces + dialect implementations for
PersistCatalogVersion,AuthzAssignment*,AuthzMembershipEdge*, andResourceScope*. - Add/expand integration tests covering resource-scope dual-write and membership-edge projection across dialects, consolidating team-membership tests into
stmttest.
Reviewed changes
Copilot reviewed 50 out of 51 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/storage/v2/stmttest/team_membership_test.go | Expands team membership statement coverage (list filters + update-not-found). |
| internal/storage/v2/stmttest/authz_test.go | Adds cross-dialect CRUD tests for authz assignments and catalog persistence round-trip. |
| internal/storage/v2/stmttest/authz_resource_scope_test.go | Adds tests for RSI upsert/get/delete and cascade behavior on project delete. |
| internal/storage/v2/stmttest/authz_membership_edge_test.go | Adds cross-dialect tests for membership edge upsert/get/list/delete shapes. |
| internal/storage/v2/stmttest/authz_dual_write_test.go | Tests dual-write hooks from user/team/team-membership lifecycle events into RSI/edges. |
| internal/storage/v2/dialect/sqlite/user.go | Wires same-tx RSI + membership-edge dual writes on user create/deactivate/delete. |
| internal/storage/v2/dialect/sqlite/team.go | Wraps create/deactivate in tx; dual-writes RSI and clears membership edges on deactivate. |
| internal/storage/v2/dialect/sqlite/team_membership.go | Wraps create/update in tx; projects roster status changes into authz membership edges. |
| internal/storage/v2/dialect/sqlite/statement.go | Registers new statement implementations (RSI/authz assignment/edge/catalog). |
| internal/storage/v2/dialect/sqlite/resource_scope_index.go | Implements SQLite RSI upsert/get/delete statements. |
| internal/storage/v2/dialect/sqlite/project.go | Dual-writes RSI on project create (transactional). |
| internal/storage/v2/dialect/sqlite/migration/sql/000002_authz_mvp.sql | Adds SQLite authz MVP schema + seed + backfill. |
| internal/storage/v2/dialect/sqlite/authz_membership_edge.go | Implements SQLite authz membership edge statements (upsert/get/list/delete). |
| internal/storage/v2/dialect/sqlite/authz_catalog.go | Implements SQLite PersistCatalogVersion mapping compiler mutations into rows. |
| internal/storage/v2/dialect/sqlite/authz_assignment.go | Implements SQLite authz assignment CRUD statements. |
| internal/storage/v2/dialect/spanner/user.go | Wires same-tx RSI + membership-edge dual writes on user create/deactivate/delete. |
| internal/storage/v2/dialect/spanner/team.go | Wraps create/deactivate in tx; dual-writes RSI and clears membership edges on deactivate. |
| internal/storage/v2/dialect/spanner/team_membership.go | Wraps create/update in tx; projects roster status changes into membership edges. |
| internal/storage/v2/dialect/spanner/team_membership_test.go | Removes dialect-local test (superseded by stmttest coverage). |
| internal/storage/v2/dialect/spanner/statement.go | Registers new statement implementations (RSI/authz assignment/edge/catalog). |
| internal/storage/v2/dialect/spanner/resource_scope_index.go | Implements Spanner RSI upsert/get/delete statements. |
| internal/storage/v2/dialect/spanner/project.go | Dual-writes RSI on project create; clarifies RSI cascade on delete. |
| internal/storage/v2/dialect/spanner/migration/sql/000018_authz_mvp.sql | Adds Spanner authz MVP schema + seed + backfill (incl. NULL_FILTERED uniqueness). |
| internal/storage/v2/dialect/spanner/authz_membership_edge.go | Implements Spanner authz membership edge statements (upsert/get/list/delete). |
| internal/storage/v2/dialect/spanner/authz_catalog.go | Implements Spanner PersistCatalogVersion with active-guard retirement semantics. |
| internal/storage/v2/dialect/spanner/authz_catalog_seed_test.go | Adds Spanner integration test asserting seeded system catalog invariants. |
| internal/storage/v2/dialect/spanner/authz_assignment.go | Implements Spanner authz assignment CRUD and active-unique-key behavior. |
| internal/storage/v2/dialect/spanner/authz_assignment_key_test.go | Unit-tests Spanner activeUniqueKey key construction semantics. |
| internal/storage/v2/dialect/postgres/user.go | Wires same-tx RSI + membership-edge dual writes on user create/deactivate/delete. |
| internal/storage/v2/dialect/postgres/team.go | Wraps create/deactivate in tx; dual-writes RSI and clears membership edges on deactivate. |
| internal/storage/v2/dialect/postgres/team_membership.go | Wraps create/update in tx; projects roster status changes into membership edges. |
| internal/storage/v2/dialect/postgres/team_membership_test.go | Removes dialect-local test (superseded by stmttest coverage). |
| internal/storage/v2/dialect/postgres/statement.go | Registers new statement implementations (RSI/authz assignment/edge/catalog). |
| internal/storage/v2/dialect/postgres/resource_scope_index.go | Implements Postgres RSI upsert/get/delete statements. |
| internal/storage/v2/dialect/postgres/project.go | Dual-writes RSI on project create; wraps delete error. |
| internal/storage/v2/dialect/postgres/migration/sql/000015_authz_mvp.sql | Adds Postgres authz MVP schema + seed + backfill. |
| internal/storage/v2/dialect/postgres/authz_membership_edge.go | Implements Postgres authz membership edge statements (upsert/get/list/delete). |
| internal/storage/v2/dialect/postgres/authz_catalog.go | Implements Postgres PersistCatalogVersion mapping compiler mutations into rows. |
| internal/storage/v2/dialect/postgres/authz_catalog_seed_test.go | Adds Postgres integration test asserting seeded system catalog invariants. |
| internal/storage/v2/dialect/postgres/authz_catalog_persist_test.go | Adds Postgres integration tests for persistence mapping + catalog retirement behavior. |
| internal/storage/v2/dialect/postgres/authz_assignment.go | Implements Postgres authz assignment CRUD statements. |
| internal/storage/v2/dialect/authz/membership_edge_schema.go | Adds shared filter schema for membership edge deletes across dialects. |
| internal/storage/v2/dialect/authz/dual_write.go | Adds shared dual-write helpers for user lifecycle → RSI/edges projection. |
| internal/storage/v2/dialect/authz/catalog_rows.go | Adds shared mapping from compiler mutations to typed storage rows. |
| internal/service/statement.go | Extends statement interfaces and adds SyncUserTeamMembershipEdge helper + catalog interface. |
| internal/domain/team_membership.go | Adds MembershipStatus.IsAuthzActive() helper for projection rules. |
| internal/domain/authz.go | Adds domain types/constants for RSI/authz tables and constructors for scopes/edges. |
| internal/authz/compiler/output.go | Minor import formatting change. |
| docs/design/api/permission-storage.md | Updates design doc from Wave 0 spike to Wave 1 implemented details. |
| .changeset/authz-mvp-storage.md | Adds changeset for @zitadel/server minor release of authz MVP storage. |
Files not reviewed (1)
- internal/service/mocks/statement.mock.go: Generated file
Suppressed comments (5)
internal/storage/v2/dialect/postgres/authz_membership_edge.go:70
- Stringify key SetType/MemberType when using them as SQL parameters to keep enum bindings consistent with the rest of the dialect (and avoid pgx encoding surprises with named string types).
rows, err := s.client.Query(ctx, getAuthzMembershipEdgeStmt, key.ProjectID, key.SetType, key.SetID, key.MemberType, key.MemberID)
internal/storage/v2/dialect/postgres/authz_membership_edge.go:83
- Stringify memberType when binding it as a SQL parameter (consistent with other statement methods that pass enums as strings).
rows, err := s.client.Query(ctx, listAuthzMembershipEdgesByMemberStmt, projectID, memberType, memberID)
internal/storage/v2/dialect/sqlite/authz_assignment.go:100
- Bind
principalTypeasprincipalType.String()to keep enum parameter binding consistent with other statement code paths.
internal/storage/v2/dialect/sqlite/authz_membership_edge.go:71 - Bind key SetType/MemberType as strings when using them as query parameters (consistent enum binding).
internal/storage/v2/dialect/sqlite/authz_membership_edge.go:85 - Bind
memberTypeasmemberType.String()when using it as a SQL parameter to keep enum bindings consistent across dialects.
Match MembershipStatus convention: pass plain strings for typed authz enums at every SQL parameter site across postgres/spanner/sqlite. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Use spannerNullString/spannerNullTime so optional string and time params encode as NULL or concrete values, matching catalog edge binds. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
livio-a
left a comment
There was a problem hiding this comment.
Review: Wave 1 authz MVP storage — reviewed as part of the stack (#666 → #677 → #758).
Overall this is a carefully built, high-consistency change. I verified the riskiest pieces by hand: the dual-write (team_memberships + authz_membership_edges + resource_scope_index) is wrapped in the same withTransaction on every create/update/delete/deactivate path across all three dialects, so those rows can't diverge; placeholder/column counts match the DDL exactly; everything is parameterized (no injection surface); table/index parity across Postgres/Spanner/SQLite is clean; and the Spanner active_unique_key stand-in for the Postgres partial-unique index is genuinely equivalent (same tuple order, \x1f-joined, nulled on revoke). No blocker- or high-severity defect.
Worth fixing before merge
[MEDIUM] CreateAuthzAssignment doesn't mint id via ensureManagedID.
internal/storage/v2/dialect/{postgres,spanner,sqlite}/authz_assignment.go insert a.ID verbatim, whereas CreateProject/CreateTeam/CreateUser all open with ensureManagedID(&x.ID, domain.PrefixX). There's no PrefixAuthzAssignment in internal/domain. This deviates from ADR 047 / internal/storage/v2/AGENTS.md ("assign empty IDs with dialect Ensure/ensureManagedID before INSERT; don't invent IDs in domain/service/handlers"). A future grants-API caller that follows the established pattern (leave ID empty, let the dialect mint it) will hit CHECK (id <> '') instead of a minted ID. Suggest adding PrefixAuthzAssignment and minting in the dialect statement.
[MEDIUM] Catalog-persistence correctness is deep-tested only on Postgres.
Only postgres/authz_catalog_persist_test.go reads back the projected authz_relations / authz_relation_references / authz_expression_edges / authz_relation_closure rows and verifies retire-previous. There's no spanner/ or sqlite/ equivalent (confirmed absent), and the shared stmttest persist test only asserts the call succeeds. A Spanner/SQLite bug that mis-writes closure depth or drops references would pass CI. Given how meticulously the DDL is mirrored across dialects, the correctness verification should be too.
Lower severity
- [LOW] Seed closure depth drift. The hand-seeded
viewer→adminclosure row is written at depth 1, but the model chain is viewer→editor→admin, so the compiler BFS emits depth 2. No Wave 1 runtime impact (resolver joins from/to only, ignores depth), but it's a silent landmine for when #420 recompilescat_sys_1throughPersistCatalogVersion. Note the depth-invariant test in #758 can't catch this — its oracle re-implements the same BFS. - [LOW] Spanner
authz_relation_referencesomits DEFAULTs.ref_relation/wildcard/conditionareNOT NULLwith noDEFAULT, whereas Postgres/SQLite giveDEFAULT ''/DEFAULT false. An INSERT omitting those columns succeeds on PG/SQLite but hits NOT-NULL on Spanner. Latent (mapper always supplies all columns) but a real DDL parity gap. - [LOW]
authz_relations.kindhardcoded to'relation'indialect/authz/catalog_rows.go(the compiler carries no kind), so'permission'— allowed by the CHECK — can never be produced. Fine for MVP; worth a comment/follow-up. - [LOW] Team FKs use
ON DELETE CASCADEonresource_scope_index/authz_assignments, diverging from the deliberateRESTRICT/NO ACTIONonteam_memberships(ADR 024: deletion via lifecycle service, not raw cascade). Internally consistent across dialects — flagging so it's a conscious choice, not drift. - [LOW] SQLite project-delete cascade depends on
PRAGMA foreign_keys = ON.DeleteProjectByIDnow relies on FK cascade to clean RSI/edges/assignments; correct on PG/Spanner, silently orphans on SQLite without the pragma. Pre-existing global assumption, now extended to the authz tables — worth confirming the pool enforces it. - [NIT] PG/SQLite membership-edge upsert uses a no-op PK self-assignment (
DO UPDATE SET project_id = EXCLUDED.project_id) to preservecreated_at; the Spanner variant documents why (created_at = created_at) but PG/SQLite don't.
Test coverage (relocation verified, minor gaps)
The removed team_membership_test.go coverage was relocated and expanded into the shared stmttest suite (CRUD/FK/not-found now run on all three dialects; dual-write status variants moved to authz_dual_write_test.go and grew Deactivate_clears... cases) — no net loss. Remaining gaps, all LOW: membership-edge/dual-write tests only exercise user+team (no agent-member or app-set cases); and nothing asserts authz_assignments/edges are cascade-cleaned on project delete (only RSI is checked).
…talog Mint CreateAuthzAssignment IDs via ensureManagedID (PrefixAuthzAssignment / asgn_). Add GetAuthzCatalog and move deep Persist round-trip asserts into stmttest across all dialects. Fix seeded viewer→admin closure depth, Spanner relation-reference DEFAULTs, and small MVP comments. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
|
Thanks for the review — addressed the merge-blocking items and the clear LOWs in MEDIUM
LOW (fixed)
LOW (acknowledged, no code)
|
Summary
Wave 1 authz MVP storage on top of Wave 0 (#666): Goose migrations for the six tables, dual-write resource-scope identifiers + membership edges from existing project/team/user/membership statements, and
PersistCatalogVersionfor compilerCatalogMutations(#720). SQLite gets the same schema and dual-write path for local parity.Review / maintainability follow-ups:
DeleteAuthzMembershipEdges(filter)API (keptForTeamDeactivatefor the subquery).dialect/authzholds multi-write user helpers,MembershipEdgeSchema, and catalog row builders (engines keep SQL). Removed the three identical dual-write adapter files and identity wrappers.AuthzMembershipEdgeKey.activeUniqueKeylives in Spanner storage (not domain).created_atno-op update (PK updates rejected)..String()(matchingMembershipStatus), including sibling ResourceKind/CatalogKind binds.spannerNullString/spannerNullTime(not Go pointers).CreateAuthzAssignmentmints empty IDs viaensureManagedID(PrefixAuthzAssignment/asgn_).GetAuthzCatalogloads a persisted catalog + projected rows; deep Persist round-trip asserts live instmttest(all dialects). Seededviewer→adminclosure depth is 2; Spanner relation-reference columns have DEFAULTs.Validation
Postgres + SQLite green. Spanner compile-checked + ActiveUniqueKey unit tests; full emulator coverage via CI
server:test-spanner.Release notes / changeset
Changeset already present:
.changeset/authz-mvp-storage.md(@zitadel/serverminor). Review/maintainability follow-ups are Go storage/API-shape refactors — no additional changeset.Notes
cursor/permission-storage-wave0-6e44/ docs(authz): Wave 0 permission storage DDL spike #666).dialect/authz*package.