docs(authz): Wave 0 permission storage DDL spike - #666
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new API design doc that captures the Wave 0 “permission storage” DDL spike for the portable relational FGA core, then threads that reference into the API design index and ADR 032/033 follow-ups to anchor future implementation work (notably #422 migrations).
Changes:
- Introduces
docs/design/api/permission-storage.mddocumenting locked DDL/storage decisions (D1–D13), strawman schema, and query sketches. - Links the new design doc from
docs/design/api/README.mdto include it in the suggested reading order. - Updates ADR 032 and ADR 033 follow-ups to point implementers at the Wave 0 storage doc and #422.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/design/api/README.md | Adds the new permission-storage doc to the API design index/reading order. |
| docs/design/api/permission-storage.md | New Wave 0 storage/DDL spike doc with locked decisions, schema strawman, and illustrative SQL. |
| docs/adrs/033-internal-permission-management.md | Follow-up section now references the Wave 0 storage doc and links to #422. |
| docs/adrs/032-permission-catalogs.md | Follow-up section now references the Wave 0 storage doc and links to #422/#333 context. |
livio-a
left a comment
livio-a
left a comment
There was a problem hiding this comment.
Review: Wave 0 permission storage DDL spike — reviewed alongside the rest of the stack (#677, #758).
Well-reasoned spike at the right altitude — freezing tables/PKs/indexes/dual-write rules as locked decisions D1–D13 is exactly the right output for Wave 0, and the implementation in #677 tracks it closely. Two notes, both doc-level:
-
This doc is partially superseded by #677 and reads that way in isolation. #677 reverses D5 (it does create
authz_expression_edges+authz_relation_references, which D5 here says to defer until #421) and changes relation identity to(catalog_id, object_type, relation). #677 updatespermission-storage.mdto match, so head stays consistent — but a reader landing on this PR's version sees the pre-revision D5/identity. Worth a one-line "superseded by #677 for D5 / relation identity," or squashing the doc evolution so the frozen decisions and their revisions land together. -
[NIT] Illustrative check SQL contradicts the D13 home-project note. The single-resource check example hardcodes
e.project_id = a.project_idfor the team-membershipEXISTS, while the D13 discussion a few lines down states membership edges for a foreign team principal live in the principal's home project, nota.project_id. It's labeled illustrative, but the two passages read as directly contradictory — a half-sentence caveat on the example would remove the trap for whoever writes the Wave 1 resolver.
livio-a
left a comment
livio-a
left a comment
There was a problem hiding this comment.
Approving — Wave 0 design is solid and the earlier review feedback is addressed.
One small note, non-blocking: the newly-declared authz_membership_edges (project_id, set_id) → teams FK is the frozen-schema authority, so #677's migration should match it (flagged there). If it's meant to stay app-enforced for MVP, drop the line here instead so doc and migration agree.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Align check SQL with D13 via principal_home_project_id, single-source catalog tables (D14), tighter DDL CHECKs, and supersede authz.md / ADR follow-ups that still pointed at team_memberships checks and delegation/expression-edge tables. Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
517c12a to
e4e7adc
Compare
<!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## 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 `PersistCatalogVersion` for compiler `CatalogMutations` (#720). SQLite gets the same schema and dual-write path for local parity. Review / maintainability follow-ups: - Membership-edge deletes use one `DeleteAuthzMembershipEdges(filter)` API (kept `ForTeamDeactivate` for the subquery). - Shared `dialect/authz` holds multi-write user helpers, `MembershipEdgeSchema`, and catalog row builders (engines keep SQL). Removed the three identical dual-write adapter files and identity wrappers. - Typed authz discriminators (including DDL-aligned principal types), assignment scope constructors, and `AuthzMembershipEdgeKey`. - Spanner `activeUniqueKey` lives in Spanner storage (not domain). - Spanner membership-edge upsert uses a `created_at` no-op update (PK updates rejected). - Authz enum SQL parameters bind via `.String()` (matching `MembershipStatus`), including sibling ResourceKind/CatalogKind binds. - Spanner nullable authz params bind via `spannerNullString` / `spannerNullTime` (not Go pointers). - `CreateAuthzAssignment` mints empty IDs via `ensureManagedID` (`PrefixAuthzAssignment` / `asgn_`). - `GetAuthzCatalog` loads a persisted catalog + projected rows; deep Persist round-trip asserts live in `stmttest` (all dialects). Seeded `viewer→admin` closure depth is 2; Spanner relation-reference columns have DEFAULTs. - `authz_membership_edges` has Wave 0 set FK `(project_id, set_id) → teams` + MVP `CHECK (user∈team)` with `ON DELETE CASCADE` on **all** dialects (Spanner matched to Postgres/SQLite). `stmttest` `DeleteTeamCascadesEdges` asserts hard-deleting a team removes edges on every engine. ## Validation ```sh go generate ./internal/service/ go test ./internal/domain/ ./internal/service/ ./internal/storage/v2/dialect/authz/ ./internal/storage/v2/dialect/spanner/ -run ActiveUniqueKey go test -tags sqlite_integration ./internal/storage/v2/stmttest/ -run 'Authz|PersistCatalog|DualWrite' go test -tags postgres_integration ./internal/storage/v2/stmttest/ ./internal/storage/v2/dialect/postgres/ -run 'Authz|PersistCatalog|CatalogSeed|DualWrite' ``` Postgres + SQLite green (including `DeleteTeamCascadesEdges`). 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/server` minor). Review/maintainability follow-ups are Go storage/API-shape refactors — no additional changeset. ## Notes - Stacked on Wave 0 (`cursor/permission-storage-wave0-6e44` / #666). - Re-synced with rewritten Wave 0 tip after main (`7f696931`): kept Wave 1 shipped schema; folded Wave 0 RSI MATCH SIMPLE + delete notes and `system-permission-catalog` see-also link. Skipped Wave 0 `grantor_type` null CHECK (not in Wave 1 migrations). - Out of scope: resolver (#423), Leopard, filling bundles, #420 auto-compile, Filter support for team-deactivate subqueries, splitting a second `dialect/authz*` package. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-9a29c97f-a069-4f09-9bec-244a599e3354?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-9a29c97f-a069-4f09-9bec-244a599e3354&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Silvan <adlerhurst@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![https://cursor.com/assets/images/open-in-web-dark.png"><source](https://cursor.com/assets/images/open-in-web-dark.png"><source){kind=link}
![https://cursor.com/assets/images/open-in-web-light.png"><img](https://cursor.com/assets/images/open-in-web-light.png"><img){kind=link}
![https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a](https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a){kind=link}
![https://cursor.com/assets/images/open-in-cursor-dark.png"><source](https://cursor.com/assets/images/open-in-cursor-dark.png"><source){kind=link}
![https://cursor.com/assets/images/open-in-cursor-light.png"><img](https://cursor.com/assets/images/open-in-cursor-light.png"><img){kind=link}
![https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div](https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div){kind=link}
Summary
Adds the Wave 0 design spike for portable FGA relational storage (ADR 032–033 / epic #419): locked decisions D1–D14, Postgres/Spanner-oriented DDL strawman, dual-write membership edges, check/list SQL sketches, end-to-end narrative, and a #333 cross-project grant depiction (foreign user/team principal on the protected
project_id).Links the doc from the API design index and points ADR 032/033 follow-ups at it so #422 can implement migrations without re-litigating the schema shape.
Validation
$principal_home_project_id, relations+closure only (D14), tighter DDL CHECKs,authz.md/ ADR follow-up alignmentRelease notes / changeset
No changeset required — no shipped behavior changed (docs only).
Notes
internal/authz/*, resolver (Permission Resolver & SQL List Predicate Injection #423), OpenFGA compiler (OpenFGA Parser, IR & Profile Compiler #421), implementing [ARCH-ADR] Define Architecture for Cross-Project Identity and Collaboration #333 product/identity