fix(seed): never auto-seed the shared Bunyip landing tenant (PMS-239) #174

Merged
longjacksonle merged 1 commit from fix/mapps-239-demo-seed-shared-tenant into main 2026-06-11 17:10:55 +02:00

What

Fixes PMS-239: in staging every user saw the same demo records and one user's edits showed for all.

Root cause

Not a cross-tenant leak - all those users are in the same tenant. Bunyip access tokens carry no tenant claim yet, so the OIDC JIT path (auth/middleware.rs::ensure_user_from_bunyip) upserts every SSO user into one shared landing tenant (OIDC_DEFAULT_TENANT_ID, fallback ..0001) - a deliberate v1 deferral (PMS-4 AC6 / docs §3.3). Co-tenant users share all rows; the first-visit demo seeder (PMS-157) seeded that shared tenant once, which made the sharing visible.

The PMS-139 TenantId sweep already prevents leaks across tenants and cannot separate users within one. Note mokosh is one-tenant-per-MSP-org, so colleagues are meant to share - "see only your own data" is not the real target; per-org isolation is, and that needs the tenant claim (below).

Fix (scope: the demo-data pollution)

  • Seeder skips the shared landing tenant. SeedService captures OIDC_DEFAULT_TENANT_ID at construction and refuses to auto-seed it - it is a multi-user zone, not a fresh single-owner account. Unconditional (independent of the MOKOSH_DEMO_SEED kill-switch). with_shared_tenant(..) lets tests pin the excluded tenant without process-global env, so the existing seed tests (env unset) are unaffected.
  • scripts/wipe_demo_seed.sql removes rows seeded before this fix: the Acme Corporation (Demo) company + its contacts (cascade) + tickets (deleted first, no cascade) for a given tenant. Runs in a transaction that ROLLBACKs by default (preview); operator reviews counts, then switches to COMMIT. Leaves demo_seeded set so nothing re-seeds.

Out of scope (the real fix)

Per-org isolation needs Bunyip to issue a tenant/org claim consumed here instead of the default-tenant funnel - PMS-4 AC6 / docs §3.3. Until then, SSO users genuinely share one tenant by design.

Verification

  • cargo check --all-targets, cargo clippy --all-targets: clean.
  • cargo test --test seed_demo: 3 pass, incl. new skips_seeding_the_shared_landing_tenant; the two existing seed tests unchanged and green.

Operator follow-up

Run scripts/wipe_demo_seed.sql against staging (preview first) to remove the already-seeded demo rows, then confirm the shared tenant no longer shows them. Not run here (destructive + needs staging DB access).

🤖 Generated with Claude Code

## What Fixes PMS-239: in staging every user saw the same demo records and one user's edits showed for all. ## Root cause Not a cross-tenant leak - all those users are in the **same tenant**. Bunyip access tokens carry no tenant claim yet, so the OIDC JIT path (`auth/middleware.rs::ensure_user_from_bunyip`) upserts every SSO user into one shared landing tenant (`OIDC_DEFAULT_TENANT_ID`, fallback `..0001`) - a deliberate v1 deferral (PMS-4 AC6 / docs §3.3). Co-tenant users share all rows; the first-visit demo seeder (PMS-157) seeded that shared tenant once, which made the sharing visible. The PMS-139 `TenantId` sweep already prevents leaks *across* tenants and cannot separate users *within* one. Note mokosh is one-tenant-per-MSP-org, so colleagues are *meant* to share - "see only your own data" is not the real target; per-org isolation is, and that needs the tenant claim (below). ## Fix (scope: the demo-data pollution) - **Seeder skips the shared landing tenant.** `SeedService` captures `OIDC_DEFAULT_TENANT_ID` at construction and refuses to auto-seed it - it is a multi-user zone, not a fresh single-owner account. Unconditional (independent of the `MOKOSH_DEMO_SEED` kill-switch). `with_shared_tenant(..)` lets tests pin the excluded tenant without process-global env, so the existing seed tests (env unset) are unaffected. - **`scripts/wipe_demo_seed.sql`** removes rows seeded before this fix: the `Acme Corporation (Demo)` company + its contacts (cascade) + tickets (deleted first, no cascade) for a given tenant. Runs in a transaction that ROLLBACKs by default (preview); operator reviews counts, then switches to COMMIT. Leaves `demo_seeded` set so nothing re-seeds. ## Out of scope (the real fix) Per-org isolation needs Bunyip to issue a tenant/org claim consumed here instead of the default-tenant funnel - PMS-4 AC6 / docs §3.3. Until then, SSO users genuinely share one tenant by design. ## Verification - `cargo check --all-targets`, `cargo clippy --all-targets`: clean. - `cargo test --test seed_demo`: 3 pass, incl. new `skips_seeding_the_shared_landing_tenant`; the two existing seed tests unchanged and green. ## Operator follow-up Run `scripts/wipe_demo_seed.sql` against staging (preview first) to remove the already-seeded demo rows, then confirm the shared tenant no longer shows them. Not run here (destructive + needs staging DB access). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(seed): never auto-seed the shared Bunyip landing tenant (PMS-239)
All checks were successful
E2E (staging) / Playwright against staging (pull_request) Successful in 42s
Check / fmt + clippy + compile + tests (pull_request) Successful in 2m16s
Build OCI container / Build and push mokosh-api image (push) Successful in 3m17s
Create release / Create release from merged PR (pull_request) Has been skipped
18c60ef2b9
Staging showed every user the same demo company/contacts/tickets, with one user's edits visible to all - looking like a tenant leak. It is not: Bunyip access tokens carry no tenant claim yet, so the OIDC JIT path lands every SSO user in one shared tenant (`OIDC_DEFAULT_TENANT_ID`, fallback `..0001`) - a deliberate v1 deferral (PMS-4 AC6 / docs §3.3). Co-tenant users share all rows by definition; the first-visit demo seeder just made it visible by seeding that shared tenant.

The PMS-139 `TenantId` sweep already guarantees no leak *across* tenants and cannot separate users *within* one. The fixable part here is the demo-data pollution: the seeder now refuses to auto-seed the configured shared landing tenant, since that tenant is a multi-user zone rather than a fresh single-owner account. The exclusion is unconditional (independent of the `MOKOSH_DEMO_SEED` global kill-switch) and captured at construction from `OIDC_DEFAULT_TENANT_ID`; `with_shared_tenant` lets callers/tests pin it without process-global env state, so the existing seed tests (env unset -> no exclusion) are unaffected.

`scripts/wipe_demo_seed.sql` removes demo rows seeded before this fix: it deletes the 'Acme Corporation (Demo)' company plus its contacts (cascade) and tickets (deleted first, no cascade) for a given tenant, inside a transaction that ROLLBACKs by default so the operator previews the counts before switching to COMMIT. The `demo_seeded` flag is left set so nothing re-seeds.

The real per-org isolation fix (Bunyip issuing a tenant claim, consumed here instead of the default-tenant funnel) is larger and out of scope - tracked under PMS-4 AC6 / docs §3.3.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
longjacksonle deleted branch fix/mapps-239-demo-seed-shared-tenant 2026-06-11 17:10:55 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!174
No description provided.