feat(tenancy): fail-closed isolation + tenants module (SaaS groundwork) by antosubash · Pull Request #370 · antosubash/simple_module_python

antosubash · 2026-09-27T11:35:49Z

Groundwork for running SaaS installs: tenant isolation now fails closed in the framework, and a new tenants module adds organisations, memberships (one user can belong to many) and invitations, plus the hooks a billing module will plug into. Billing itself is out of scope. Design: docs/plans/2026-09-27-saas-tenancy-design.md.

Closes #332, closes #343, closes #355, closes #356, closes #357, closes #358, closes #359, closes #363, closes #364, closes #365, closes #366.
Partly addresses #362 and #371 (details below).

Why

MultiTenantMixin existed, but tenancy was open by default:

No model used the mixin.
Queries with no tenant set returned every tenant's rows.
ORM update()/delete() weren't tenant-scoped at all.
A row could move to another tenant whenever no tenant was set.
A signed-in user with no tenant could pick any tenant through the X-Tenant-ID header.
Background jobs ran without a tenant.
There was no tenant entity, and each user could belong to only one tenant.

Framework

Strict isolation (MultiTenantMixin: reads with no tenant bound return every tenant's rows #355): strict mode is on whenever multi_tenant is on, and is stored per engine. A query or write on a tenant-scoped model with no tenant set raises MissingTenantError, a subclass of TenantIsolationError.
- Code that deliberately spans tenants opts out with all_tenants() or execution_options(all_tenants=True).
- Jobs and CLI commands act for one tenant with tenant_context(id). It takes effect even when nested inside all_tenants().
Every statement shape is scoped (Soft-delete and tenant filters are silently skipped for selects that name no mapper (select(func.count()), subquery counts) #332):
- Tenant criteria are attached for every tenant-scoped model, not only the one a query names, so joins, ORM exists()/in_()/scalar subqueries and count().select_from() are covered.
- Core statements on Model.__table__ get an explicit predicate, and a bare Core exists().where(…) has its inner select rewritten.
- With no tenant in strict mode, a direct reference raises and an indirect one matches nothing.
- Behaviour change: soft-delete filtering now covers the same shapes. A soft-deleted row no longer shows up through a join, a subquery or a count. include_deleted=True still bypasses it.
Every ORM write path is covered:
- update() and delete() are tenant-scoped, and update().values(tenant_id=…) is refused.
- insert(Model), bulk or with .values(), is stamped with the tenant and refused for a different tenant (MultiTenantMixin: bulk insert() is not stamped with tenant_id #357).
- A flush that writes or deletes another tenant's object is refused. This covers objects returned from the session cache after a tenant switch.
Tenant changes refused (MultiTenantMixin: unbound flush skips the tenant change check, letting a row move silently #356): changing a row's tenant_id is refused whether or not a tenant is set. Only code inside an all_tenants() block may move a row.
Header rules (TenantMiddleware: header chooses the tenant for any authenticated user with no tenant #358, TenantMiddleware: the tenant header value is bound unvalidated, so an over-long or malformed header becomes a 500 on write #366): TenantMiddleware uses app.state.tenant_resolver when a module registers one. On the legacy path the header is honoured only for anonymous requests. Every tenant id taken from a request must match TENANT_ID_PATTERN.
Single-tenant hosts (No default tenant setting or tenant_scope helper for MultiTenantMixin #359): HostSettings.default_tenant runs mixin tables as one tenant for requests and background tasks. It is ignored when multi_tenant is on.
Deferred work (hosting: module middleware runs outside Tenant, so deferred work loses the tenant #364): db.on_commit callbacks and FastAPI BackgroundTasks already run inside the request's tenant (now tested). For work a module middleware defers past TenantMiddleware, bind_current_tenant(fn) captures the tenant when the work is queued and restores it when it runs. Middleware order is unchanged.
background_tasks:
- The enqueuing request's tenant is stamped on the Celery message and restored when the task runs. Request code cannot enqueue a task as another tenant.
- The worker's sync session now has the tenant listeners and the host's multi_tenant setting (background_tasks: tenant-scope TaskExecution, and register the tenant listeners in the Celery worker process #371).
Doctor check SM024: warns when a unique key on a tenant-scoped table doesn't include tenant_id.
Postgres test runs (simple_module_test: fixtures hard-code in-memory SQLite, so a module suite cannot run on Postgres #343): SM_TEST_DATABASE_URL points the test fixtures at Postgres, and make test-py-pg runs the whole Python suite there. The schema is reset once per test, so app and db_session share the same data.

`tenants` module

Tables: tenant, membership (unique on (tenant_id, user_id), role owner/admin/member) and invitation (stores only a SHA-256 of the token; can only be accepted by the invited email).
Resolver: the session stores the preferred tenant, which is checked against a membership on every request.
- The membership cache is dropped across workers through InvalidationBus, and a read already in flight cannot re-cache a removed member.
- Suspended tenants resolve to no tenant, and the user is told which organisation was suspended.
- The header selects a tenant only if the user is a member of it.
- Subdomains (TenantMiddleware: anonymous tenant resolution is header only, no pluggable resolvers #363): with subdomain_base set, the tenant comes from the subdomain. It applies to anonymous visitors on public routes and to members, with their role, and never to a non-member on an authenticated route.
- With tenants installed, a membership or role change applies on the next request, with no re-login (users: tenant changes are invisible until re login because UserContext is cached in the session #362).
Per-tenant roles: the membership role becomes tenant:<role> for the active tenant only.
- Management routes also require an owner or admin role in the active tenant, so a platform-wide permission alone doesn't make a plain member a manager.
- Tenant-level routes always act on the active tenant, never on an id taken from the URL.
Invariants that hold under concurrency:
- The last-owner rule is enforced inside the UPDATE/DELETE itself, with a row lock on the tenant for Postgres.
- Seat checks lock the tenant row.
- Concurrent creates with the same slug, or accepts of the same invitation, return 409.
Invitation links: built from the public_base_url setting, never from the Host header.
Invitation rules: no accepting into a suspended tenant, and no re-inviting an existing member.
Billing hooks: an EntitlementProvider (seat limits return 402), TenantService.set_status, and events published after commit.
Pages: /tenants/, /tenants/members, /tenants/invitations/accept, /admin/tenants/.
Migration: a new independent tenants branch.

QA

Five agents tested this in parallel. Each finding was verified before being fixed, and each fix has a regression test. 18 confirmed bugs, all fixed.

Adversarial isolation: 10 bugs found and fixed:
- tenant_context was ignored inside all_tenants().
- update().values(tenant_id=…) could move rows to another tenant.
- A cached object from another tenant could be written after a tenant switch.
- The worker ran with no tenant enforcement.
- A membership-cache race let a removed member keep access.
- The last-owner and seat checks were check-then-act races.
- Invitation links were built from the Host header.
- A platform permission gave a plain member management rights in a tenant.
- A second database connection setup turned strict mode off.
- Invitations could be accepted into a suspended tenant.
API functional (~100 cases): the slug format wasn't enforced, and concurrent writes returned 500s. The last-owner race was reproduced on a real SQLite file, which led to the atomic rule above.
Browser E2E: 9 scenarios at desktop width and 375px. One bug: a suspended organisation was switched away from silently.
Regression: full suite, all lint and typecheck gates, Alembic up/down/check, make doctor compared with main (no new warnings), production-mode boot with multi_tenant on and off, a real Celery run, and back-compat checks.
Docs vs code: one wording fix.

Decisions for a reviewer

Overlap with fix(db): close four silent data-correctness gaps in the DB layer (#332, #335, #336, #339) #344: both PRs restructure listeners.py, and fix(db): close four silent data-correctness gaps in the DB layer (#332, #335, #336, #339) #344 also fixes the soft-delete half of Soft-delete and tenant filters are silently skipped for selects that name no mapper (select(func.count()), subquery counts) #332, which this PR now covers too. Suggested order:
Release: simple_module_tenants is added to the PyPI publish matrix.
Behaviour change (strict mode): installs already running multi_tenant=True get strict mode. No bundled model uses the mixin.
Behaviour change (soft delete): soft-deleted rows are hidden in joins, subqueries and counts, not only when a query names the model directly.
Worker settings: the worker now resolves host settings with merge_host_settings() at start-up (scripts/run_worker.py).
Filter overhead: measured at ~0.06–0.1 ms per query for tenant criteria on in-memory SQLite, plus ~10 µs for the exists() scan.

Not in this PR

Module adoption:
- Filed in this work: background_tasks: tenant-scope TaskExecution, and register the tenant listeners in the Celery worker process #371 (background_tasks, the model part), audit_log: tag AuditEntry with tenant_id and tenant-scope the browse/export screens #372 (audit_log), branding: no table to add a mixin to — it's a single process-global settings singleton that must become per-tenant lookup #373 (branding), dashboard: platform-wide stats cache and unscoped User counts need a tenant decision #374 (dashboard), feature_flags: tenant overrides already modeled, but no request-time dependency resolves the active tenant #375 (feature_flags), keycloak: stop hard-coding UserContext.tenant_id from the tenant_id JWT claim, and check register_setup_steps opt-out still holds #376 (keycloak), permissions: role→permission grants stay global, but must support mapping the synthetic tenant:<role> principal roles #377 (permissions), pagebuilder: tenant-scope pages/media/redirects/layout, add subdomain resolver for public visitors, tenant-loop the scheduler smpy_modules#38 (pagebuilder), news: NewsArticle/Category/Tag need tenant_id, and the page-orphan sweep + slug claim must stay tenant-consistent with pagebuilder smpy_modules#39 (news).
- Already tracked before this: file_storage: StoredFile is not tenant scoped #361 (file_storage), users: no tenant management, UserCreate, UserUpdate and create_admin have no tenant_id #360 and users: tenant changes are invisible until re login because UserContext is cached in the session #362 (users), settings: on a multi-tenant host any tenant's admin can edit system-scope settings and any other tenant's tenant-scope settings #368 (settings).
Postgres in CI: CI runs SQLite only; Postgres runs are local (make test-py-pg).
Redundant markers: about 40 test files carry an unneeded @pytest.mark.anyio. Those markers are why make test-py-pg passes -p no:anyio; removing them is a separate cleanup.

Testing

SQLite: 3,276 passed (CI green on 9eba456).
Postgres 16: 3,277 passed, 0 failed via make test-py-pg.
- The first Postgres run had 76 failures and errors, none from tenancy:
  - 73 were @pytest.mark.anyio tests running on a different event loop from their fixtures.
  - One was the fixture schema-reset bug fixed here.
  - Two were a test that relied on insert order.
Concurrent last-owner test: 15/15 on file SQLite (was 4/12 before the fix) and 10/10 on Postgres.
New tests:
- DB: strict mode, DML scoping, statement shapes (joins, subqueries, counts, Core exists()), soft-delete shapes, the session cache, nested bypass blocks.
- Middleware and header rules.
- SM024.
- Celery: tenant propagation and worker enforcement.
- tenants: 42 tests, including subdomains and the concurrency test.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

Framework: - Strict tenant isolation: with multi_tenant on, a query, ORM bulk update/delete or insert on a MultiTenantMixin model with no tenant context raises TenantIsolationError instead of spanning tenants. ORM update()/delete() are now tenant-scoped (they were not). - tenant_context() / all_tenants() and the all_tenants=True execution option for acting as, or deliberately across, tenants. - TenantMiddleware consults app.state.tenant_resolver; the tenant header is no longer honoured for an authenticated user without a tenant. - background_tasks carries the enqueuing request's tenant into tasks. - Doctor check SM024: unique keys on tenant tables must include tenant_id. tenants module: tenants, many-to-many memberships with per-tenant roles (tenant:<role> on the active tenant only), email-bound invitations, suspend/reactivate, membership-validated resolver, and the billing seams (EntitlementProvider, lifecycle, after-commit events). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

Inertia pages for the tenants module (Index, Members, AcceptInvitation, AdminBrowse) with extracted components, translated copy and API error mapping; regenerated i18n keys; tenants workspace in the lockfile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

cloudflare-workers-and-pages · 2026-09-27T11:36:32Z

Deploying simple-module-python with Cloudflare Pages

Latest commit:	`e0f30fc`
Status:	✅ Deploy successful!
Preview URL:	https://bf6e9689.simple-module-python.pages.dev
Branch Preview URL:	https://claude-saas-module-planning.simple-module-python.pages.dev

View logs

/tenants and /admin/tenants 307'd to their trailing-slash form on every navigation; 'building' has no NavIcon entry and rendered blank. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

The legacy header path bound any string, so a value over 50 characters was a 500 on the first stamped write and any junk became a tenant name. TENANT_ID_PATTERN/is_valid_tenant_id in simple_module_db are now the one rule the middleware, the tenants resolver and tenant_context() share. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

) An unbound flush skipped the check, so platform code or a job with no tenant could silently move a row to another tenant. Only an explicit all_tenants() block may now do that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

test_multi_tenancy.py went over the 300-line cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

DB layer - tenant_context() nested in all_tenants() now scopes its block; it was ignored, so a per-tenant loop inside a platform job ran unscoped. - ORM update().values(tenant_id=...) is refused; bulk/Core-style ORM insert(Model) is stamped with the bound tenant and refused for another one (#357). - A flush that writes or deletes an object of another tenant (e.g. one handed back by the identity map after a tenant switch) is refused. - Strict mode is held per engine, not in a module global, so a second DatabaseState cannot switch it off. New MissingTenantError. - The Celery worker's sync session gets the tenant listeners and the host's multi_tenant setting (#371); task headers are validated and request code cannot enqueue as another tenant. tenants module - Membership cache: a read in flight during an invalidation no longer re-caches a removed member. - Last-owner and seat checks row-lock the tenant; concurrent creates and accepts give 409 instead of a 500. - Invitation links come from a public_base_url setting (root-relative when unset), never from the Host header. - Manage routes need an owner/admin role in the active tenant, not just a platform-wide permission. - No accepting into a suspended tenant, no re-inviting a member; the slug format is actually enforced (SQLModel ignored regex=). - A suspended active org is no longer switched away from silently. - Only a missing tenant becomes the org picker; other isolation errors are 403 and logged as errors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

QA showed two owners demoting each other at once still left a tenant with no owners on a real SQLite file (8 of 12 runs): pysqlite reads outside a transaction and FOR UPDATE compiles away, so both requests counted two owners. The rule now lives in the write itself — the UPDATE/DELETE only matches while another owner exists — and the tenant row lock stays for Postgres READ COMMITTED. Regression test runs on a file-backed database (15/15 after, 4/12 before). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

…on Postgres - #332: tenant criteria are attached for every tenant-scoped model, so a tenant table reached through a join target, an ORM exists()/in_()/scalar subquery or count().select_from() is scoped; top-level Core statements on Model.__table__ get an explicit tenant_id predicate (insert: stamped). With no tenant under strict mode, direct references raise and indirect ones match nothing. Only a bare Core exists().where() stays unscoped (documented). - #359: HostSettings.default_tenant — single-tenant hosts run mixin tables as one tenant for requests and background tasks; ignored when multi_tenant is on. - #364: bind_current_tenant(fn) carries the tenant into work a module defers past the request; db.on_commit and BackgroundTasks already run in scope (tested). - #363: the tenants module resolves the tenant from the subdomain (subdomain_base), for anonymous visitors on public routes, members with their role, never for a non-member on an authenticated route. - #343: SM_TEST_DATABASE_URL runs the fixtures and the tenancy DB tests on Postgres, each test on an empty schema. Tenancy suites pass there, including the concurrent last-owner test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

… counts (#332) The two gaps left after the tenant half of #332: - A bare Core `exists().where(Model.x == ...)` is never ORM-compiled, so loader criteria never reached it. A cheap scan of the WHERE/column/HAVING clauses (~10 us, skipped when no tenant or soft-delete model exists) finds an Exists and only then rewrites nested SELECTs with the tenant and soft-delete predicates; under strict mode with no tenant it raises. - Soft-delete criteria are attached for every soft-deletable model on reads, and top-level Core statements get `is_deleted IS false`, so joins, subqueries and counts no longer surface trashed rows. Behaviour change, documented; include_deleted=True still bypasses it. The model registry moves to model_registry.py. New shape tests fail on the previous filter (9 of 21) and pass on SQLite and Postgres. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

The first full run with SM_TEST_DATABASE_URL gave 26 failed and 50 errors, none of them caused by tenancy: - 73 were from @pytest.mark.anyio tests running on anyio's event loop while their async fixtures ran on pytest-asyncio's. An asyncpg connection cannot cross loops; aiosqlite's worker thread hides this on SQLite. The new `make test-py-pg` target passes -p no:anyio, and asyncio_mode=auto still runs those tests. - `app` and `db_session` each reset the Postgres schema, so a test asking for both lost the seeded admin (a 401). The reset now happens once per test, through an autouse marker fixture. - test_user_role_model relied on insert order with no relationship(), and ran a SQLite-only PRAGMA. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

haiku for search and mechanical work, sonnet for routine implementation and testing, opus only for design and security reasoning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

#344 landed its own soft-delete/tenant SELECT filter (query_filters.py) alongside this branch's query_filter.py. Keep this branch's filter as the single do_orm_execute filter and drop #344's copy, keeping the rest of #344: writes.py (hard_delete, mark_written, DML write tracking), SQLite PRAGMAs, and the refresh-token GUID migration. - listeners: register #344's _mark_dml_written via attach_session_listeners so the Celery worker's session gets it too. - query_filter: expand Core joins in the top-level FROM so both sides of an INNER join (and the preserved side of an OUTER join) get an explicit WHERE; #344's test_query_filters covered this shape and caught the gap. - tenants tests: the concurrent same-slug test ran on the in-memory app DB, where both requests share one connection and the loser's rollback wipes the winner's tenant. With #344's foreign_keys=ON that became an FK error. Move it to the file-backed race harness beside the last-owner test. - CLAUDE.md / framework-conventions: merge both sides' wording. Claude-Session: https://claude.ai/code/session_012cpbRfLwgNeVhJFuvh86Fn

Review findings on the tenant query filter: - A bare Core exists() over aliased(Model) or table.alias() (at any depth) was not tenant- or soft-delete-scoped; aliases now resolve to their base table and the predicate is applied on the alias. - insert(M).values([{...}, {...}]) failed whenever a tenant was bound, and a foreign tenant_id inside those rows went unchecked; rows are now checked and stamped one by one. - A bulk update(M).values(tenant_id=...) could move rows on a non-strict install with no tenant bound; the refusal now runs before that shortcut, matching the flush guard (#356). The insert guard and statement-value helpers move to insert_guard.py to keep query_filter.py under the 300-line cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

chatgpt-codex-connector · 2026-09-30T20:47:00Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
🔒 Security Review	✅ Completed 2026-09-30T20:55:15.180235Z	`e0f30fc`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

claude added 3 commits September 27, 2026 11:29

docs(tenancy): ruff-format the multi-tenancy code sample

e472766

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

claude added 2 commits September 27, 2026 11:39

claude and others added 10 commits September 27, 2026 12:39

test(db): move the #356 tests next to the other strict-mode tests

9ad2114

test_multi_tenancy.py went over the 300-line cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FB4wFRUXWGHY6qp8wtDJCE

antosubash marked this pull request as ready for review September 30, 2026 20:46

Navigation Menu

feat(tenancy): fail-closed isolation + tenants module (SaaS groundwork) - #370

feat(tenancy): fail-closed isolation + tenants module (SaaS groundwork)#370
antosubash wants to merge 15 commits into
mainfrom
claude/saas-module-planning-xd7pl4

antosubash commented Sep 27, 2026 •

edited

Loading

Uh oh!

cloudflare-workers-and-pages Bot commented Sep 27, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Sep 30, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

antosubash commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Framework

tenants module

QA

Decisions for a reviewer

Not in this PR

Testing

Uh oh!

cloudflare-workers-and-pages Bot commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying simple-module-python with Cloudflare Pages

Uh oh!

chatgpt-codex-connector Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

antosubash commented Sep 27, 2026 •

edited

Loading

`tenants` module

cloudflare-workers-and-pages Bot commented Sep 27, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 30, 2026 •

edited

Loading