feat(tenants): add the tenant registry, the one enforcement point Book and Scribe now share (T-8)
A tenant has been an unchecked string since T-4/T-7: the Office's X-Office-Tenant header or a user JWT's tatlock_user claim, with Book and Scribe each running their own _require_tenant that checked only presence. A typo'd claim opened a fresh, empty namespace rather than being refused. src/domains/tenants/ (controller/models/schemas/service) adds tenants and tenant_consumers (migration 007, down_revision="006"): tenants.id is the slug a tatlock tenant carries, kind (household|person|test) and status (active|suspended) CHECK-constrained; tenant_consumers is the grant pairing a consumer to a tenant. Migration 007 seeds the two tenants in use today — jpmschweitzer (household, "Jeroen") and llm_tester (test, "Playground") — each granted to office-claude, the only Office consumer configured in Authentik. Every existing owner_tenant column (book_records, book_tenant_counters, scribe_addresses, scribe_messages, scribe_mailbox_entries — five, grepped and confirmed) now carries a foreign key to tenants.id, ON DELETE RESTRICT, at both the migration and the ORM model. TenantsService.require_tenant is the one enforcement point: no tenant is 400 tenant_required (unchanged); an unknown tenant is 403 tenant_unknown; a suspended one is 403 tenant_suspended; an Office caller whose consumer holds no grant in that tenant is 403 consumer_not_granted. One query — a LEFT JOIN against the grant — so presence, status and the grant are all read in a single round trip. Book's and Scribe's own _require_tenant now delegate to it, translating TenantError to their own exception type so every call site and each controller's single except clause keep working unchanged. Read-only this round: GET /tenants/me answers the caller's own tenant and its granted consumers (the cheap "am I allowed here" probe); GET /tenants answers every tenant for a tatlock/scheduler caller, or a list of one for a tenant-bearing caller — another tenant's existence is never revealed. No write routes; provisioning a tenant is workspace T-195. Tests: tests/test_tenants.py and tests/test_tenants_migration.py (new); a TestTenantRegistryEnforcement class added to both tests/test_book.py and tests/test_scribe.py proving all four cases (unknown, suspended, ungranted-consumer, and the control) on a live Book and Scribe route, not only on /tenants itself. tests/conftest.py gains a tenant_registry schema fixture and a suspended_tenant fixture (inserted and deleted per test; no FK references it); book_schema and scribe_schema each independently grant a second, test-environment-only consumer (office-gemini) in both seeded tenants, since test_book.py and test_scribe.py have used it as a stand-in second consumer since before this grant check existed — the migration itself still seeds only office-claude, matching the dossier. Mutation-checked: dropping the grant check fails the three consumer_not_granted tests (200 instead of 403); dropping the status check fails the three tenant_suspended tests; making Book bypass the helper fails its unknown-tenant test (200 instead of 403). All three restored and the full suite re-verified green (508 + 35 = 543 passed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+126
-3
@@ -18,6 +18,7 @@ environment and a clean cache; overriding `POSTGRES_HOST`/`USER`/
|
||||
`PASSWORD`/`DATABASE` in that child's environment, derived from
|
||||
`TEST_DATABASE_URL`, is what actually redirects the migration.
|
||||
"""
|
||||
import asyncio
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
@@ -76,12 +77,80 @@ def test_database_url() -> str:
|
||||
return url
|
||||
|
||||
|
||||
def _grant_test_office_gemini(url: str) -> None:
|
||||
"""
|
||||
Grant a second, test-environment-only consumer, `office-gemini`, in
|
||||
both seeded tenants: test_book.py and test_scribe.py have used
|
||||
`office-gemini` as a stand-in second Office consumer since before
|
||||
core-api T-8's grant check existed, to prove behaviour that depends on
|
||||
two *distinct* consumers (address ownership across consumers,
|
||||
`book_events.client` recording the actual caller) rather than on any
|
||||
one consumer being granted. Migration 007 itself seeds only
|
||||
`office-claude` — the only consumer actually configured in Authentik
|
||||
today (T-8's dossier) — so this grant is inserted directly here,
|
||||
test-only, rather than through the migration; production's seed stays
|
||||
exactly what the dossier specifies. A consumer this never grants
|
||||
(e.g. `office-ungranted` in a test) is what proves the
|
||||
`consumer_not_granted` refusal — this must never widen to cover one of
|
||||
those.
|
||||
|
||||
A plain function, not a fixture: called from each of `book_schema`,
|
||||
`scribe_schema` and `tenant_registry` right after their own `run_alembic
|
||||
upgrade head`, so the grant is restored whenever any one of them is the
|
||||
first to re-establish the schema in a session — the same
|
||||
self-healing-on-first-use property those fixtures already rely on to
|
||||
survive a migration test downgrading the schema mid-suite. A session-
|
||||
scoped fixture shared between them would cache the *string* across that
|
||||
downgrade while the real schema moved on underneath it — which is what
|
||||
a first version of this did, and `tests/test_scribe.py` failing with
|
||||
"relation tenants does not exist" is what caught it.
|
||||
"""
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.ext.asyncio import create_async_engine
|
||||
from sqlalchemy.pool import NullPool
|
||||
|
||||
asyncpg_url = url.replace("postgresql://", "postgresql+asyncpg://", 1)
|
||||
|
||||
async def _grant() -> None:
|
||||
engine = create_async_engine(asyncpg_url, poolclass=NullPool)
|
||||
try:
|
||||
async with engine.begin() as conn:
|
||||
await conn.execute(
|
||||
text(
|
||||
"INSERT INTO tenant_consumers (tenant_id, consumer) VALUES "
|
||||
"('jpmschweitzer', 'office-gemini'), ('llm_tester', 'office-gemini') "
|
||||
"ON CONFLICT DO NOTHING"
|
||||
)
|
||||
)
|
||||
finally:
|
||||
await engine.dispose()
|
||||
|
||||
asyncio.run(_grant())
|
||||
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
def book_schema(test_database_url: str) -> str:
|
||||
"""Ensure migration 005 (and everything before it) is applied to core_api_test."""
|
||||
def tenant_registry(test_database_url: str) -> str:
|
||||
"""Ensure migration 007 (and everything before it) is applied to
|
||||
core_api_test, with its seed tenants/grants plus the test-only
|
||||
office-gemini grant (see `_grant_test_office_gemini`). Used directly by
|
||||
tests/test_tenants.py; book_schema and scribe_schema do the same thing
|
||||
independently rather than depending on this, so each keeps its own
|
||||
self-healing-on-first-use property (see that function's docstring)."""
|
||||
result = run_alembic(test_database_url, "upgrade", "head")
|
||||
if result.returncode != 0:
|
||||
pytest.fail(f"alembic upgrade head failed:\n{result.stdout}\n{result.stderr}")
|
||||
_grant_test_office_gemini(test_database_url)
|
||||
return test_database_url
|
||||
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
def book_schema(test_database_url: str) -> str:
|
||||
"""Ensure migration 005 (and everything before it, now including the
|
||||
tenant registry) is applied to core_api_test."""
|
||||
result = run_alembic(test_database_url, "upgrade", "head")
|
||||
if result.returncode != 0:
|
||||
pytest.fail(f"alembic upgrade head failed:\n{result.stdout}\n{result.stderr}")
|
||||
_grant_test_office_gemini(test_database_url)
|
||||
return test_database_url
|
||||
|
||||
|
||||
@@ -125,10 +194,12 @@ async def clean_book_tables(book_session):
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
def scribe_schema(test_database_url: str) -> str:
|
||||
"""Ensure migration 006 (and everything before it) is applied to core_api_test."""
|
||||
"""Ensure migration 006 (and everything before it, now including the
|
||||
tenant registry) is applied to core_api_test."""
|
||||
result = run_alembic(test_database_url, "upgrade", "head")
|
||||
if result.returncode != 0:
|
||||
pytest.fail(f"alembic upgrade head failed:\n{result.stdout}\n{result.stderr}")
|
||||
_grant_test_office_gemini(test_database_url)
|
||||
return test_database_url
|
||||
|
||||
|
||||
@@ -164,3 +235,55 @@ async def clean_scribe_tables(scribe_session):
|
||||
)
|
||||
await scribe_session.commit()
|
||||
yield scribe_session
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def suspended_tenant(test_database_url: str):
|
||||
"""
|
||||
A third tenant, inserted with `status='suspended'`, for proving the
|
||||
`tenant_suspended` refusal (core-api T-8) without touching either
|
||||
seeded tenant that the rest of the suite depends on being active.
|
||||
Nothing else ever references this id, so inserting and deleting it
|
||||
per test is safe — no FK points at it.
|
||||
|
||||
Depends on `test_database_url` and runs its own `alembic upgrade head`
|
||||
rather than on `tenant_registry`/`book_schema`/`scribe_schema`: this
|
||||
fixture is reused from test_book.py, test_scribe.py and test_tenants.py,
|
||||
so whichever of those files runs first would permanently cache
|
||||
*any* session-scoped schema fixture it depended on — and a migration
|
||||
test downgrading the schema afterwards (test_book_migration.py,
|
||||
test_scribe_migration.py) would leave that cache stale for the other
|
||||
two files. Self-contained and function-scoped avoids it outright, at
|
||||
the cost of a redundant `alembic upgrade head` call (cheap relative to
|
||||
the many such calls the migration test files already make).
|
||||
"""
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
|
||||
from sqlalchemy.pool import NullPool
|
||||
|
||||
result = run_alembic(test_database_url, "upgrade", "head")
|
||||
if result.returncode != 0:
|
||||
pytest.fail(f"alembic upgrade head failed:\n{result.stdout}\n{result.stderr}")
|
||||
|
||||
tenant_id = "suspended-tenant"
|
||||
asyncpg_url = test_database_url.replace("postgresql://", "postgresql+asyncpg://", 1)
|
||||
engine = create_async_engine(asyncpg_url, poolclass=NullPool)
|
||||
session_factory = async_sessionmaker(bind=engine, expire_on_commit=False)
|
||||
|
||||
async with session_factory() as session:
|
||||
await session.execute(
|
||||
text(
|
||||
"INSERT INTO tenants (id, display_name, kind, status) "
|
||||
"VALUES (:id, 'Suspended Test Tenant', 'test', 'suspended') "
|
||||
"ON CONFLICT (id) DO UPDATE SET status = 'suspended'"
|
||||
),
|
||||
{"id": tenant_id},
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
yield tenant_id
|
||||
|
||||
async with session_factory() as session:
|
||||
await session.execute(text("DELETE FROM tenants WHERE id = :id"), {"id": tenant_id})
|
||||
await session.commit()
|
||||
await engine.dispose()
|
||||
|
||||
Reference in New Issue
Block a user