[2c] Org isolation enforcement - #32
Draft
andrmaz wants to merge 5 commits into
Draft
Conversation
Introduces db/org-scope: an AsyncLocalStorage-backed org context plus a Prisma client extension that automatically injects/forces organizationId filters (or relation-based equivalents) on every model operation. Access without an active context fails closed (MissingOrgContextError); a deliberate runWithoutOrgScope escape hatch exists for pre-auth system paths. Relation-scoped creates are verified via a DB round trip through the same scoped client, so cross-org foreign keys are rejected (OrgScopeViolationError). Covered by 61 unit tests exercising the pure scoping logic and a fake Prisma-extension client (no live DB needed), plus manual verification against a real local Postgres. Co-authored-by: Andrea Mazzucchelli <andrmaz@users.noreply.github.com>
- PrismaService no longer extends PrismaClient directly; every model delegate it exposes is routed through db's org-scoped Prisma client, and the raw client is a private field with no external accessor. - Wires @prisma/adapter-pg (Prisma 7 requires a driver adapter), fixing a previously-documented boot gap as a side effect of this change. - OrgContextInterceptor (registered globally via APP_INTERCEPTOR) binds the authenticated caller's organizationId as the active org context for the duration of each request, so controllers/services don't need to remember to filter by org themselves. - OrgScopeExceptionFilter maps a query-layer OrgScopeViolationError to 403 Forbidden. - Extends the db-client jest mock with a real (duplicated, dependency-free) copy of the org-context primitives so tests exercise real ALS behavior. Covered by new interceptor/filter test suites, including a regression test that a runWithOrgContext callback must synchronously consume any returned Prisma-like lazy promise (a subtlety documented on runWithOrgContext itself) and a realistic multi-hop async chain test. Co-authored-by: Andrea Mazzucchelli <andrmaz@users.noreply.github.com>
findByGoogleSub and findOrCreate run before the caller's organization is known (findOrCreate is literally what determines it, via the email-domain lookup), so they wrap their Prisma calls in runWithoutOrgScope. The runWithoutOrgScope callback must be async (or otherwise synchronously consume the Prisma call) — a bare non-async callback that merely returns the lazy promise loses the bound context once storage.run() exits. Co-authored-by: Andrea Mazzucchelli <andrmaz@users.noreply.github.com>
OrganizationsController previously had no organization scoping at all: any authenticated admin, regardless of their own org, could list every organization in the system, fetch any organization by id, and rename any organization — the clearest cross-org leak in the API surface this slice is meant to close. GET /:id and PATCH /:id now use assertAdminOrganizationAccess (the same convention already used by DepartmentsController and AdminUsersController) to return 403 for a mismatched id before ever touching the database. GET / (list) now returns only the caller's own organization instead of every tenant. POST (create) is unchanged — provisioning a brand-new organization doesn't read or modify existing tenant data. Removes OrganizationService.findAll(), which had become dead, unscoped code once the controller stopped calling it. Co-authored-by: Andrea Mazzucchelli <andrmaz@users.noreply.github.com>
Verifies the userDepartment lookup is scoped by the caller's own organization, that concurrent requests from different organizations never cross-contaminate scope resolution, and that a department only resolves when its relation filter matches the caller's org. Co-authored-by: Andrea Mazzucchelli <andrmaz@users.noreply.github.com>
Contributor
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Org-scoped filtering is now enforced at the Prisma query layer for every request, not just opted into per controller.
db's neworg-scopemodule: anAsyncLocalStorage-backed org context plus a Prisma Client Extension (createOrgScopedClient) that automatically injects/forcesorganizationIdfilters (or the equivalent relation filter for join/child models) on every model operation — reads, creates, updates, deletes. Any query made with no active org context throwsMissingOrgContextError(fail closed). A deliberaterunWithoutOrgScopeescape hatch exists for the one legitimate pre-auth path (resolving a user's org during login).PrismaServiceno longer extendsPrismaClient; every model delegate it exposes is routed through the org-scoped client, and the raw client has no external accessor — there is no way to bypass scoping from application code.OrgContextInterceptor(global, viaAPP_INTERCEPTOR) binds the authenticated caller'sorganizationIdas the active context for the whole request, so controllers/services don't need to remember to filter by org themselves.OrgScopeExceptionFiltermaps a query-layerOrgScopeViolationError(a write referencing a foreign-org record) to 403.OrganizationsControllerhad zero org scoping — any admin could list every organization, fetch any organization by id, and rename any organization. It now behaves like the existingDepartmentsController/AdminUsersControllerconvention (403 on a mismatched id; list returns only the caller's own org).@prisma/adapter-pgintoPrismaService(Prisma 7 requires a driver adapter), incidentally fixing a previously-documented boot gap.Why this design
Building this as an opt-in per-controller check (the existing pattern) is exactly what let
OrganizationsControllerslip through with no scoping at all. The Prisma extension makes scoping mandatory at the query layer regardless of whether a given service author remembers to filter — new models must be registered inORG_SCOPE_CONFIGor all queries against them fail closed.Testing
packages/dbcovering the pure scoping logic and the extension itself (via a lightweight fake client that faithfully reproduces Prisma's$extendscontract) — no live DB required.apps/api(interceptor wiring, exception filter, organizations cross-org 403s, MCP cross-org isolation and concurrency).OrgScopeViolationError,$transactionbatches stay correctly scoped, and no-context queries fail closed). This caught two real bugs the unit tests alone would have missed:.then()reaction when awaited — arunWithOrgContext/runWithoutOrgScopecallback that just returns that lazy promise (instead of beingasyncand consuming it) silently loses the bound context. Documented onrunWithOrgContextand fixed the one real call site that had this bug (UserService.findByGoogleSub).WhereUniqueInput(used byfindUnique/update/delete/upsert) requires the unique identifier to stay a direct top-level field — wrapping it inANDfails validation. Added a separate flat-merge strategy (mergeUniqueWhere) for these operations.pnpm test/pnpm check-types/pnpm lintall pass.Acceptance criteria