fix(rls): cover tenant-less tables; tenant-scope user_oauth_identities #207

Merged

David merged 2 commits from feat/pms-258-rls-cover-tenantless-tables into main

2026-06-13 17:47:38 +02:00

David commented

2026-06-13 17:41:10 +02:00

Owner

What

Brings the five tables the dynamic RLS loop never covered (because they lack a tenant_id column, or carried a global unique key) under fail-closed row level security. This is PMS-258 (Step 3 of the PMS-255 epic), whose migration was authored locally during the original run but never merged - its file number was taken by the PMS-257 fail-closed flip, so these gaps were still open on main.

Why it matters

user_oauth_identities had a GLOBAL UNIQUE (provider, subject). A single upstream identity (one Google sub) could therefore exist only once across the whole database, so under personal-tenant isolation one user's identity row blocks every other tenant's and the link leaks across the per-user boundary. This was flagged CRITICAL in the epic.

Changes

migrations/041_rls_cover_tenantless_tables.sql:
- user_oauth_identities: add denormalized tenant_id (backfilled from the owning user, then NOT NULL), replace the global unique key with UNIQUE (tenant_id, provider, subject), add an index, and attach the standard fail-closed tenant_isolation policy (USING + WITH CHECK + FORCE).
- kb_article_versions, invoice_lines, rate_card_items, sla_targets: fail-closed PARENT-JOIN policy (EXISTS over the parent's tenant_id). Chosen over a denormalized column to avoid backfill and INSERT-site drift; these rows are always reached through their parent. The expression mirrors 038 so behaviour matches the directly-scoped tables.
src/modules/auth/service.rs: both Google-link INSERT sites populate tenant_id; the link-to-existing path now runs inside begin_with_tenant(existing.tenant_id) so it satisfies WITH CHECK under a NOBYPASSRLS connection. A SAFETY note marks the pre-auth (provider, subject) lookup as a path PMS-285 must route onto the privileged pool once the app drops BYPASSRLS.
tests/tenantless_table_rls.rs: same subject insertable in two tenants (no cross-tenant collision) while a same-tenant duplicate is rejected; parent-join policy is fail-closed on read and enforces WITH CHECK on write (asserted under a dedicated NOSUPERUSER NOBYPASSRLS role, like rls_isolation.rs).

Testing

just check (compile + clippy + fmt): green.
New tests/tenantless_table_rls.rs: 2 passed.
Regression set: auth (17), bunyip_login (6), knowledge_base (10), contracts (6), recurring_invoicing (5), sla (2), rls_isolation (1), per_user_isolation (3): all pass.

Notes

No backfill of co-mingled rows is needed (staging will be wiped); this PR only covers the schema/RLS gap.
The 4-child-table choice deviates from the issue's soft "prefer denormalized" in favour of the minimal, no-drift parent-join, documented inline in the migration.

🤖 Generated with Claude Code

## What Brings the five tables the dynamic RLS loop never covered (because they lack a `tenant_id` column, or carried a global unique key) under fail-closed row level security. This is PMS-258 (Step 3 of the PMS-255 epic), whose migration was authored locally during the original run but never merged - its file number was taken by the PMS-257 fail-closed flip, so these gaps were still open on `main`. ## Why it matters `user_oauth_identities` had a GLOBAL `UNIQUE (provider, subject)`. A single upstream identity (one Google `sub`) could therefore exist only once across the whole database, so under personal-tenant isolation one user's identity row blocks every other tenant's and the link leaks across the per-user boundary. This was flagged CRITICAL in the epic. ## Changes - `migrations/041_rls_cover_tenantless_tables.sql`: - `user_oauth_identities`: add denormalized `tenant_id` (backfilled from the owning user, then `NOT NULL`), replace the global unique key with `UNIQUE (tenant_id, provider, subject)`, add an index, and attach the standard fail-closed `tenant_isolation` policy (USING + WITH CHECK + FORCE). - `kb_article_versions`, `invoice_lines`, `rate_card_items`, `sla_targets`: fail-closed PARENT-JOIN policy (EXISTS over the parent's `tenant_id`). Chosen over a denormalized column to avoid backfill and INSERT-site drift; these rows are always reached through their parent. The expression mirrors 038 so behaviour matches the directly-scoped tables. - `src/modules/auth/service.rs`: both Google-link INSERT sites populate `tenant_id`; the link-to-existing path now runs inside `begin_with_tenant(existing.tenant_id)` so it satisfies WITH CHECK under a NOBYPASSRLS connection. A SAFETY note marks the pre-auth `(provider, subject)` lookup as a path PMS-285 must route onto the privileged pool once the app drops BYPASSRLS. - `tests/tenantless_table_rls.rs`: same subject insertable in two tenants (no cross-tenant collision) while a same-tenant duplicate is rejected; parent-join policy is fail-closed on read and enforces WITH CHECK on write (asserted under a dedicated `NOSUPERUSER NOBYPASSRLS` role, like `rls_isolation.rs`). ## Testing - `just check` (compile + clippy + fmt): green. - New `tests/tenantless_table_rls.rs`: 2 passed. - Regression set: `auth` (17), `bunyip_login` (6), `knowledge_base` (10), `contracts` (6), `recurring_invoicing` (5), `sla` (2), `rls_isolation` (1), `per_user_isolation` (3): all pass. ## Notes - No backfill of co-mingled rows is needed (staging will be wiped); this PR only covers the schema/RLS gap. - The 4-child-table choice deviates from the issue's soft "prefer denormalized" in favour of the minimal, no-drift parent-join, documented inline in the migration. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

David added 1 commit

2026-06-13 17:41:10 +02:00

fix(rls): cover tenant-less tables; tenant-scope user_oauth_identities

E2E / Playwright against staging (pull_request) Failing after 35s

Details

Check / fmt + clippy + compile + tests (pull_request) Successful in 2m32s

Details

Build OCI container / Build and push mokosh-api image (push) Successful in 6m56s

Details

c224ffbf95

The 024/038 RLS policy loops only attach to tables that have a `tenant_id` column, so five tables were never covered: `user_oauth_identities` plus the four child tables isolated only through a parent FK. PMS-258's migration was authored locally but never merged (the file number was taken by the PMS-257 fail-closed flip), so these gaps were still open on main.

`user_oauth_identities` carried a GLOBAL `UNIQUE (provider, subject)`, so a single upstream identity (one Google `sub`) could exist only once across the whole database. Under personal-tenant isolation that lets one user's identity row block every other tenant's and leaks the link across the per-user boundary. It now gets a denormalized `tenant_id` (backfilled from the owning user, then NOT NULL), a tenant-scoped `UNIQUE (tenant_id, provider, subject)`, an index, and the standard fail-closed `tenant_isolation` policy (USING + WITH CHECK + FORCE).

`kb_article_versions`, `invoice_lines`, `rate_card_items`, and `sla_targets` get a fail-closed PARENT-JOIN policy (EXISTS over the parent's `tenant_id`) instead of a denormalized column: no backfill, no NOT NULL column to keep populated on every INSERT (no drift), and these rows are always reached through their parent. The policy mirrors 038's fail-closed expression so behaviour matches the directly-scoped tables.

Both Google-link INSERT sites now populate `tenant_id`; the link-to-existing path runs inside `begin_with_tenant(existing.tenant_id)` so it satisfies WITH CHECK under a NOBYPASSRLS connection. A SAFETY note marks the pre-auth `(provider, subject)` lookup as a path PMS-285 must route onto the privileged pool once the app connection drops BYPASSRLS.

Adds `tests/tenantless_table_rls.rs`: proves the same subject is insertable in two tenants (no cross-tenant collision) while a same-tenant duplicate is rejected, and that the parent-join policy is fail-closed on read and enforces WITH CHECK on write.

#PMS-258

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

David added 1 commit

2026-06-13 17:47:34 +02:00

Merge branch 'main' into feat/pms-258-rls-cover-tenantless-tables

Create release / Create release from merged PR (pull_request) Has been skipped

Details

E2E / Playwright against staging (pull_request) Failing after 25s

Details

Check / fmt + clippy + compile + tests (pull_request) Failing after 4m46s

Details

eca763e3ad