diff --git a/.pql/changelog/ticket_history/2026-10.sql b/.pql/changelog/ticket_history/2026-10.sql index 2c78dae..ec9a26d 100644 --- a/.pql/changelog/ticket_history/2026-10.sql +++ b/.pql/changelog/ticket_history/2026-10.sql @@ -90,3 +90,4 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, chang **Isolation (D-73).** Every Book query and write filters on the owner tenant; a test proves an `llm_tester` caller cannot read, change, link to or close a `jpmschweitzer` record (and the reverse) — the error is `not_found`, so another tenant''s ids are not even confirmed to exist. **Split 2026-10-19:** the short outward ids and the owner tenant (amending 005), and the Book''s tenant filter, moved to **T-7**, so core-api can release safely. T-2 keeps the remaining verbs and work/personal as tags.', NULL, '2026-10-19 14:06:27', '2026-10-19 14:06:27.325', '2026-10-19 14:06:27.325', NULL, '0e233f93ee1e85162d05109749d33032', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GN8Z1FF1QDGR4QE4405AP8GC', 'status', 'backlog', 'done', NULL, '2026-10-19 14:51:47', '2026-10-19 14:51:47.496', '2026-10-19 14:51:47.496', NULL, '625ca10cfc9bcaf576a2fd52a3fe2792', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/ticket_idmap/2026-10.sql b/.pql/changelog/ticket_idmap/2026-10.sql index 06aa623..a44abca 100644 --- a/.pql/changelog/ticket_idmap/2026-10.sql +++ b/.pql/changelog/ticket_idmap/2026-10.sql @@ -5,3 +5,4 @@ INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_ INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GN8F5Z00KKNGTADK9BHG99N8', 'T-5', '2026-10-19 12:57:09.632', '2026-10-19 12:57:09.632', NULL, 'a8774787d37deb533e701914864fd560', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at; INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GN8NBWVQ53ZX7WMZNE5GJJP4', 'T-6', '2026-10-19 13:24:11.102', '2026-10-19 13:24:11.102', NULL, 'f6351f2d6190ae5fbd9e55e6eadc04e5', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at; INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GN8Z1FF1QDGR4QE4405AP8GC', 'T-7', '2026-10-19 14:06:27.193', '2026-10-19 14:06:27.193', NULL, '09267e8234ec21c6d75767d7cd21bb51', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at; +INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GN99CB552PSF82EE2P8PJZP0', 'T-8', '2026-10-19 14:51:37.641', '2026-10-19 14:51:37.641', NULL, 'd8c7f6122652cb1c3e448f1adedd54ab', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at; diff --git a/.pql/changelog/tickets/2026-10.sql b/.pql/changelog/tickets/2026-10.sql index 90a850b..d44e342 100644 --- a/.pql/changelog/tickets/2026-10.sql +++ b/.pql/changelog/tickets/2026-10.sql @@ -79,3 +79,18 @@ INSERT INTO tickets (record_id, type, parent_record_id, title, description, stat **Isolation (D-73).** Every Book query and write filters on the owner tenant; a test proves an `llm_tester` caller cannot read, change, link to or close a `jpmschweitzer` record (and the reverse) — the error is `not_found`, so another tenant''s ids are not even confirmed to exist. **Split 2026-10-19:** the short outward ids and the owner tenant (amending 005), and the Book''s tenant filter, moved to **T-7**, so core-api can release safely. T-2 keeps the remaining verbs and work/personal as tags.', 'backlog', 'high', NULL, NULL, NULL, '2026-10-19 09:20:45.104', '2026-10-19 14:06:27.324', NULL, '58063dda134a948a461cd97325286e48', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GN99CB552PSF82EE2P8PJZP0', 'story', NULL, 'The tenant registry: tenants, consumer grants, and one enforcement point', 'The tenant registry: one table that says which tenants exist, which consumers may act in each, and whether a tenant is active — the anchor every service''s tenancy conforms to (workspace D-73). + +Today a tenant is an unchecked string: the Office header or a JWT''s `tatlock_user` claim is trusted as-is, and the Book and the Scribe file rows under whatever value arrives. A typo''d claim opens a fresh, empty namespace instead of being refused. + +**Tables (migration 007):** +- `tenants` — `id` (the slug, the value of `tatlock_user`; same pattern `_require_office_tenant` accepts), `display_name`, `kind` (`household` | `person` | `test`), `status` (`active` | `suspended`), timestamps. Seeded: `jpmschweitzer` (household), `llm_tester` (test). +- `tenant_consumers` — a grant: consumer X may act in tenant Y. PK `(tenant_id, consumer)`. Seeded: `office-claude` in both seeded tenants. +- Foreign keys from every `owner_tenant` column in the Book and the Scribe to `tenants.id`. + +**Enforcement:** every tenant-scoped operation goes through one helper. No tenant → 400 `tenant_required`. Tenant unknown → 403 `tenant_unknown`. Tenant suspended → 403 `tenant_suspended`. Office caller whose consumer holds no grant in that tenant → 403 `consumer_not_granted`. + +**Read API, for other services to conform to:** `GET /tenants/me` (the caller''s own tenant and its consumer''s grant); `GET /tenants` (service callers without a tenant see all; a tenant-bearing caller sees only its own — the hard boundary). + +**Not in v1:** write routes for provisioning (T-195 on the workspace board adds them, or an operator verb); per-consumer verb classes (workspace T-184 — a column on `tenant_consumers` when it lands).', 'backlog', 'high', NULL, NULL, NULL, '2026-10-19 14:51:37.641', '2026-10-19 14:51:37.641', NULL, '756a467f5dc4a84c7ee9080ef21ca5fb', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GN8Z1FF1QDGR4QE4405AP8GC', 'task', NULL, 'The Book''s final schema and tenant boundary, before 005''s first release', 'The Book''s final schema and its tenant boundary, before core-api''s first release applies migration 005 to production. Split from T-2 (whose remaining verbs stay there). Amend 005 — never run in production — rather than add a migration: per-tenant short outward numbers ("Task 12", workspace D-67), an owner tenant on every record (D-73), and every Book route filtered by it, so the routes are safe to expose the moment they deploy.', 'done', 'high', NULL, NULL, NULL, '2026-10-19 14:06:27.192', '2026-10-19 14:51:47.496', NULL, '8797eb16319aa33c8317b0a1f0defc3d', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; diff --git a/alembic/env.py b/alembic/env.py index bd05d25..2123e01 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -25,6 +25,7 @@ from src.domains.book.models import ( # noqa: F401 from src.domains.scribe.models import ( # noqa: F401 ScribeAddress, ScribeMessage, ScribeMailboxEntry, ) +from src.domains.tenants.models import Tenant, TenantConsumer # noqa: F401 # Alembic Config object config = context.config diff --git a/alembic/versions/20261019_1600_007_create_tenant_registry.py b/alembic/versions/20261019_1600_007_create_tenant_registry.py new file mode 100644 index 0000000..60e7321 --- /dev/null +++ b/alembic/versions/20261019_1600_007_create_tenant_registry.py @@ -0,0 +1,97 @@ +"""Create the tenant registry (core-api T-8, workspace D-73) + +Revision ID: 007 +Revises: 006 +Create Date: 2026-10-19 + +`tenants` and `tenant_consumers` — the registry every Book and Scribe +`owner_tenant` column now conforms to via a foreign key (`ON DELETE +RESTRICT`: a tenant with any Book or Scribe row can never be dropped out +from under it). 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 +(dossier core-api T-8). No write routes ship this round; provisioning a +third tenant is later (workspace T-195) and happens by hand until then. +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = "007" +down_revision: Union[str, None] = "006" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +# Every existing owner_tenant column (core-api T-8's dossier: "five +# owner_tenant columns exist ... find them by grep rather than trusting +# this count" — grepped and confirmed at five, here). +OWNER_TENANT_FKS = ( + ("book_records", "fk_book_records_owner_tenant"), + ("book_tenant_counters", "fk_book_tenant_counters_owner_tenant"), + ("scribe_addresses", "fk_scribe_addresses_owner_tenant"), + ("scribe_messages", "fk_scribe_messages_owner_tenant"), + ("scribe_mailbox_entries", "fk_scribe_mailbox_entries_owner_tenant"), +) + + +def upgrade() -> None: + """Create the tenant registry, seed it, then bind every existing + owner_tenant column to it.""" + + op.create_table( + "tenants", + sa.Column("id", sa.String(length=63), nullable=False), + sa.Column("display_name", sa.Text(), nullable=False), + sa.Column("kind", sa.String(length=20), nullable=False), + sa.Column("status", sa.String(length=20), nullable=False, server_default="active"), + sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False), + sa.Column("updated_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False), + sa.PrimaryKeyConstraint("id"), + sa.CheckConstraint("kind IN ('household', 'person', 'test')", name="ck_tenants_kind"), + sa.CheckConstraint("status IN ('active', 'suspended')", name="ck_tenants_status"), + ) + + op.create_table( + "tenant_consumers", + sa.Column("tenant_id", sa.String(length=63), nullable=False), + sa.Column("consumer", sa.String(length=63), nullable=False), + sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False), + sa.PrimaryKeyConstraint("tenant_id", "consumer"), + sa.ForeignKeyConstraint(["tenant_id"], ["tenants.id"], name="fk_tenant_consumers_tenant_id"), + ) + + # Seed: the two tenants in use today, both granted to office-claude — + # the only Office consumer configured in Authentik (dossier T-8). + op.execute( + """ + INSERT INTO tenants (id, display_name, kind, status) VALUES + ('jpmschweitzer', 'Jeroen', 'household', 'active'), + ('llm_tester', 'Playground', 'test', 'active') + """ + ) + op.execute( + """ + INSERT INTO tenant_consumers (tenant_id, consumer) VALUES + ('jpmschweitzer', 'office-claude'), + ('llm_tester', 'office-claude') + """ + ) + + # Every existing owner_tenant column now conforms to the registry. + for table, fk_name in OWNER_TENANT_FKS: + op.create_foreign_key( + fk_name, table, "tenants", ["owner_tenant"], ["id"], ondelete="RESTRICT" + ) + + +def downgrade() -> None: + """Unbind every owner_tenant column, then drop the registry.""" + for table, fk_name in OWNER_TENANT_FKS: + op.drop_constraint(fk_name, table, type_="foreignkey") + + op.drop_table("tenant_consumers") + op.drop_table("tenants") diff --git a/src/domains/book/models.py b/src/domains/book/models.py index 2544f1f..4f7424a 100644 --- a/src/domains/book/models.py +++ b/src/domains/book/models.py @@ -75,8 +75,11 @@ class BookRecord(Base): # The tatlock tenant the record belongs to (Caller.tenant, T-4/D-73). # Every query in the service layer filters on it — there is no # database-level RLS here, so that filter is load-bearing in Python, - # following src/domains/scribe/models.py's same note. - owner_tenant: Mapped[str] = mapped_column(String(63), nullable=False) + # following src/domains/scribe/models.py's same note. FK to + # tenants.id added by migration 007 (core-api T-8). + owner_tenant: Mapped[str] = mapped_column( + String(63), ForeignKey("tenants.id", ondelete="RESTRICT"), nullable=False + ) # A short outward id, numbered per tenant (never globally) so a tenant # can never infer another's activity from its numbers. Allocated by # BookService._allocate_number, which locks a per-tenant counter row @@ -277,7 +280,10 @@ class BookTenantCounter(Base): __tablename__ = "book_tenant_counters" - owner_tenant: Mapped[str] = mapped_column(String(63), primary_key=True) + # FK to tenants.id added by migration 007 (core-api T-8). + owner_tenant: Mapped[str] = mapped_column( + String(63), ForeignKey("tenants.id", ondelete="RESTRICT"), primary_key=True + ) next_number: Mapped[int] = mapped_column(Integer, nullable=False, default=1) def __repr__(self) -> str: diff --git a/src/domains/book/service.py b/src/domains/book/service.py index f447ea2..26c4643 100644 --- a/src/domains/book/service.py +++ b/src/domains/book/service.py @@ -38,6 +38,7 @@ from src.domains.book.models import ( BookTenantCounter, ) from src.domains.book.schemas import BookCloseRequest, BookRecordCreate, BookRecordUpdate +from src.domains.tenants.service import TenantError, get_tenants_service from src.shared.config import get_settings from src.shared.logging import get_logger @@ -60,14 +61,22 @@ class BookService: # Internal helpers # ========================================================================= - def _require_tenant(self, caller: Caller) -> str: + async def _require_tenant(self, session: AsyncSession, caller: Caller) -> str: """Every Book operation needs a tenant; a caller without one gets a clear error rather than a query that silently matches nothing. An Office caller always has one (callers.py enforces it on every call); - a user-kind caller without a `tatlock_user` claim does not.""" - if not caller.tenant: - raise BookError("tenant_required", 400) - return caller.tenant + a user-kind caller without a `tatlock_user` claim does not. + + Delegates to the tenant registry's one enforcement point (core-api + T-8 / workspace D-73), which additionally refuses an unknown + tenant, a suspended one, or an Office consumer with no grant in it. + `TenantError` is translated to `BookError` so every call site and + the controller's single `except BookError` keep working unchanged. + """ + try: + return await get_tenants_service().require_tenant(session, caller) + except TenantError as error: + raise BookError(error.code, error.status_code) from error async def _get_or_404(self, session: AsyncSession, record_id, tenant: str) -> BookRecord: """A record outside `tenant` answers exactly like one that does not @@ -160,7 +169,7 @@ class BookService: ) -> Tuple[BookRecord, List[str]]: """`add` — text-only input lands an unfiled Note (D-72). Sets `owner_tenant` from the caller and allocates `number` (core-api T-7).""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) status = self._validate_status(data.kind, data.status) number = await self._allocate_number(session, tenant) @@ -224,7 +233,7 @@ class BookService: caller: Caller, ) -> Tuple[BookRecord, List[str], List[BookRecordLink], List[BookRecordLink]]: """`show` — the record, its tags, outgoing links and backlinks.""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) record = await self._get_or_404(session, record_id, tenant) tags = await self._tags_for(session, record_id) @@ -257,7 +266,7 @@ class BookService: next round. `number=` finds the one record with that outward id in the caller's tenant (core-api T-7). """ - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) conditions = [BookRecord.owner_tenant == tenant] if view in ("inbox", "waiting", "someday", "done"): @@ -331,7 +340,7 @@ class BookService: ) -> Tuple[List[Tuple[BookRecord, List[str]]], int]: """`find` — full-text search over title and body, scoped to the caller's tenant.""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) tsquery = func.websearch_to_tsquery("simple", q) base_query = select(BookRecord).where( BookRecord.owner_tenant == tenant, BookRecord.search.op("@@")(tsquery) @@ -362,7 +371,7 @@ class BookService: caller: Caller, ) -> Tuple[BookRecord, List[str]]: """`change` — a partial update; only fields set on the request move.""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) record = await self._get_or_404(session, record_id, tenant) update_data = data.model_dump(exclude_unset=True) @@ -412,7 +421,7 @@ class BookService: in the caller's tenant the same as `link` — a target in another tenant answers 404 not_found. """ - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) record = await self._get_or_404(session, record_id, tenant) diff: dict = {} @@ -479,7 +488,7 @@ class BookService: ) -> Tuple[BookRecord, List[str]]: """`link` refuses a target in another tenant the same way a missing one is refused — both are 404 not_found (core-api T-7).""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) record = await self._get_or_404(session, record_id, tenant) await self._get_or_404(session, to_id, tenant) # the target must exist in the same tenant @@ -506,7 +515,7 @@ class BookService: relation: str, caller: Caller, ) -> Tuple[BookRecord, List[str]]: - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) record = await self._get_or_404(session, record_id, tenant) stmt = delete(BookRecordLink).where( @@ -538,7 +547,7 @@ class BookService: tag: str, caller: Caller, ) -> Tuple[BookRecord, List[str]]: - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) record = await self._get_or_404(session, record_id, tenant) existing = await session.get(BookTag, (record_id, tag)) @@ -557,7 +566,7 @@ class BookService: tag: str, caller: Caller, ) -> Tuple[BookRecord, List[str]]: - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) record = await self._get_or_404(session, record_id, tenant) stmt = delete(BookTag).where(BookTag.record_id == record_id, BookTag.tag == tag) @@ -576,7 +585,7 @@ class BookService: async def get_history( self, session: AsyncSession, record_id, caller: Caller, limit: int = 100 ) -> List[BookEvent]: - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) await self._get_or_404(session, record_id, tenant) stmt = ( diff --git a/src/domains/scribe/models.py b/src/domains/scribe/models.py index c5af4ba..c015b2e 100644 --- a/src/domains/scribe/models.py +++ b/src/domains/scribe/models.py @@ -53,7 +53,10 @@ class ScribeAddress(Base): UniqueConstraint("owner_tenant", "name", name="uq_scribe_addresses_tenant_name"), ) - owner_tenant: Mapped[str] = mapped_column(String(63), primary_key=True) + # FK to tenants.id added by migration 007 (core-api T-8). + owner_tenant: Mapped[str] = mapped_column( + String(63), ForeignKey("tenants.id", ondelete="RESTRICT"), primary_key=True + ) name: Mapped[str] = mapped_column(String(63), primary_key=True) # Nullable: a "user"-kind caller (direct OIDC, not through the Office) # carries no consumer (src/domains/auth/callers.py), so an address it @@ -82,7 +85,10 @@ class ScribeMessage(Base): __tablename__ = "scribe_messages" id: Mapped[int] = mapped_column(Integer, Identity(always=False), primary_key=True) - owner_tenant: Mapped[str] = mapped_column(String(63), nullable=False, index=True) + # FK to tenants.id added by migration 007 (core-api T-8). + owner_tenant: Mapped[str] = mapped_column( + String(63), ForeignKey("tenants.id", ondelete="RESTRICT"), nullable=False, index=True + ) topics: Mapped[List[str]] = mapped_column(ARRAY(Text), nullable=False, default=list) from_identity: Mapped[str] = mapped_column(String(63), nullable=False) from_consumer: Mapped[Optional[str]] = mapped_column(String(63), nullable=True) @@ -120,7 +126,10 @@ class ScribeMailboxEntry(Base): id: Mapped[int] = mapped_column(Integer, Identity(always=False), primary_key=True) address: Mapped[str] = mapped_column(String(63), nullable=False) - owner_tenant: Mapped[str] = mapped_column(String(63), nullable=False) + # FK to tenants.id added by migration 007 (core-api T-8). + owner_tenant: Mapped[str] = mapped_column( + String(63), ForeignKey("tenants.id", ondelete="RESTRICT"), nullable=False + ) message_id: Mapped[int] = mapped_column( Integer, ForeignKey("scribe_messages.id"), nullable=False ) diff --git a/src/domains/scribe/service.py b/src/domains/scribe/service.py index 17cd114..0e487de 100644 --- a/src/domains/scribe/service.py +++ b/src/domains/scribe/service.py @@ -24,6 +24,7 @@ from src.domains.scribe.models import ( ScribeMessage, ) from src.domains.scribe.schemas import MailboxPatchRequest, MessageSendRequest +from src.domains.tenants.service import TenantError, get_tenants_service from src.shared.logging import get_logger logger = get_logger(__name__) @@ -46,12 +47,20 @@ class ScribeService: # Internal helpers # ========================================================================= - def _require_tenant(self, caller: Caller) -> str: + async def _require_tenant(self, session: AsyncSession, caller: Caller) -> str: """Every Scribe operation needs a tenant; a caller without one gets - a clear error rather than a query that silently matches nothing.""" - if not caller.tenant: - raise ScribeError("tenant_required", 400) - return caller.tenant + a clear error rather than a query that silently matches nothing. + + Delegates to the tenant registry's one enforcement point (core-api + T-8 / workspace D-73), which additionally refuses an unknown + tenant, a suspended one, or an Office consumer with no grant in it. + `TenantError` is translated to `ScribeError` so every call site and + the controller's single `except ScribeError` keep working + unchanged.""" + try: + return await get_tenants_service().require_tenant(session, caller) + except TenantError as error: + raise ScribeError(error.code, error.status_code) from error def _validate_address_name(self, name: str) -> None: """Beyond the schema's regex (shape only): not `group-*`, not reserved. @@ -90,7 +99,7 @@ class ScribeService: ) -> Tuple[ScribeAddress, bool]: """`register` — idempotent for the same consumer; 409 address_taken for another consumer in the tenant (D-66).""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) self._validate_address_name(name) existing = await self._get_address(session, tenant, name) @@ -116,7 +125,7 @@ class ScribeService: self, session: AsyncSession, caller: Caller ) -> List[Tuple[ScribeAddress, bool]]: """`list` — the caller's tenant; mark which the caller's consumer owns.""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) stmt = ( select(ScribeAddress) @@ -134,7 +143,7 @@ class ScribeService: self, session: AsyncSession, data: MessageSendRequest, caller: Caller ) -> ScribeMessage: """`send` — `from` is the caller's own identity, never the body.""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) from_address = await self._get_address(session, tenant, caller.identity) if from_address is None or from_address.owner_consumer != caller.consumer: @@ -196,7 +205,7 @@ class ScribeService: before_id: Optional[int] = None, ) -> Tuple[List[Tuple[ScribeMailboxEntry, ScribeMessage]], int]: """`mailbox` — the caller's identity's entries, unread first.""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) await self._require_owned_identity(session, caller, tenant) conditions = [ @@ -228,7 +237,7 @@ class ScribeService: self, session: AsyncSession, caller: Caller, q: str, limit: int = 50 ) -> Tuple[List[Tuple[ScribeMailboxEntry, ScribeMessage]], int]: """`find` — full text over messages in the caller identity's mailbox.""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) await self._require_owned_identity(session, caller, tenant) tsquery = func.websearch_to_tsquery("simple", q) @@ -265,7 +274,7 @@ class ScribeService: """`history` — the retained messages sent to that address, oldest first. Reading never consumes or marks: this reads scribe_messages directly, never scribe_mailbox_entries.""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) address = await self._get_address(session, tenant, name) if address is None: @@ -298,7 +307,7 @@ class ScribeService: an identity the caller is not reading as, answers 404 not_found — indistinguishable from a nonexistent entry, so an id never confirms someone else's mail exists.""" - tenant = self._require_tenant(caller) + tenant = await self._require_tenant(session, caller) entry = await session.get(ScribeMailboxEntry, entry_id) if entry is None or entry.owner_tenant != tenant or entry.address != caller.identity: diff --git a/src/domains/tenants/__init__.py b/src/domains/tenants/__init__.py new file mode 100644 index 0000000..0435760 --- /dev/null +++ b/src/domains/tenants/__init__.py @@ -0,0 +1,10 @@ +""" +Tenants Domain + +The tenant registry (core-api T-8 / workspace D-73): tenants, consumer +grants, and the one enforcement point every tenant-scoped domain's own +`_require_tenant` calls through to (Book, Scribe). Served under /tenants/*. +""" +from src.domains.tenants.controller import tenants_controller + +__all__ = ["tenants_controller"] diff --git a/src/domains/tenants/controller.py b/src/domains/tenants/controller.py new file mode 100644 index 0000000..b7b1cff --- /dev/null +++ b/src/domains/tenants/controller.py @@ -0,0 +1,92 @@ +""" +Tenants Controller + +The tenant registry's read API (core-api T-8 / workspace D-73): `GET +/tenants/me` and `GET /tenants`. No write routes this round — provisioning +is later (workspace T-195). +""" +from fastapi import APIRouter, Depends, HTTPException +from sqlalchemy.ext.asyncio import AsyncSession + +from src.domains.auth.callers import Caller, require_caller +from src.domains.tenants.schemas import TenantListResponse, TenantMeResponse, TenantSummary +from src.domains.tenants.service import TenantError, get_tenants_service +from src.shared.base import BaseController +from src.shared.database import get_async_session +from src.shared.logging import get_logger + +logger = get_logger(__name__) + +require_tenants_reader = require_caller("office", "user") +require_tenants_lister = require_caller("office", "user", "tatlock", "scheduler") + + +def _summary(tenant) -> TenantSummary: + return TenantSummary( + id=tenant.id, display_name=tenant.display_name, kind=tenant.kind, status=tenant.status + ) + + +class TenantsController(BaseController): + """Controller for the tenant registry's read API.""" + + def __init__(self): + super().__init__(prefix="/tenants", tags=["Tenants"]) + + def create_router(self) -> APIRouter: + router = APIRouter(prefix=self.prefix, tags=self.tags) + service = get_tenants_service() + + # ===================================================================== + # me + # ===================================================================== + + @router.get( + "/me", + response_model=TenantMeResponse, + summary="The caller's own tenant and the consumers granted in it", + ) + async def get_me( + caller: Caller = Depends(require_tenants_reader), + session: AsyncSession = Depends(get_async_session), + ): + try: + tenant, consumers = await service.get_me(session, caller) + except TenantError as error: + raise HTTPException( + status_code=error.status_code, detail={"code": error.code} + ) from error + return TenantMeResponse( + id=tenant.id, + display_name=tenant.display_name, + kind=tenant.kind, + status=tenant.status, + consumers=consumers, + ) + + # ===================================================================== + # list + # ===================================================================== + + @router.get( + "", + response_model=TenantListResponse, + summary="Every tenant (tatlock/scheduler) or the caller's own, as a list of one", + ) + async def list_tenants( + caller: Caller = Depends(require_tenants_lister), + session: AsyncSession = Depends(get_async_session), + ): + try: + tenants = await service.list_tenants(session, caller) + except TenantError as error: + raise HTTPException( + status_code=error.status_code, detail={"code": error.code} + ) from error + return TenantListResponse(tenants=[_summary(tenant) for tenant in tenants]) + + return router + + +# Create controller instance +tenants_controller = TenantsController() diff --git a/src/domains/tenants/models.py b/src/domains/tenants/models.py new file mode 100644 index 0000000..de536cb --- /dev/null +++ b/src/domains/tenants/models.py @@ -0,0 +1,71 @@ +""" +Tenants Domain Models + +The tenant registry (core-api T-8 / workspace D-73): every Book and Scribe +`owner_tenant` column carries a foreign key to `tenants.id` (migration 007), +and `TenantsService.require_tenant` is the one place a caller's tenant is +checked known, active, and — for an Office caller — granted to the calling +consumer. SQLAlchemy 2.0 Mapped/mapped_column style, timestamptz, following +src/domains/book/models.py. +""" +from datetime import datetime + +from sqlalchemy import CheckConstraint, DateTime, ForeignKey, String, Text, func +from sqlalchemy.orm import Mapped, mapped_column + +from src.shared.database import Base + +TENANT_KINDS = ("household", "person", "test") +TENANT_STATUSES = ("active", "suspended") + +# The slug shape a tenant id carries, identical to +# src/domains/auth/callers.py's `_TENANT_RE` (tatlock's own tenant ids carry +# underscores, e.g. `llm_tester`) — not imported from there because that +# module's constants are private to it, following +# src/domains/scribe/models.py's same note about ADDRESS_NAME_PATTERN. +# Asserted equal in tests/test_tenants.py. +TENANT_ID_PATTERN = r"^[a-z0-9_][a-z0-9_.-]{0,62}$" + + +class Tenant(Base): + """A tenant: tatlock's own per-household/person/test namespace.""" + + __tablename__ = "tenants" + __table_args__ = ( + CheckConstraint("kind IN ('household', 'person', 'test')", name="ck_tenants_kind"), + CheckConstraint("status IN ('active', 'suspended')", name="ck_tenants_status"), + ) + + id: Mapped[str] = mapped_column(String(63), primary_key=True) + display_name: Mapped[str] = mapped_column(Text, nullable=False) + kind: Mapped[str] = mapped_column(String(20), nullable=False) + status: Mapped[str] = mapped_column(String(20), nullable=False, default="active") + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now(), nullable=False + ) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + server_default=func.now(), + onupdate=func.now(), + nullable=False, + ) + + def __repr__(self) -> str: + return f"" + + +class TenantConsumer(Base): + """A grant: consumer `consumer` may act in tenant `tenant_id`.""" + + __tablename__ = "tenant_consumers" + + tenant_id: Mapped[str] = mapped_column( + String(63), ForeignKey("tenants.id"), primary_key=True + ) + consumer: Mapped[str] = mapped_column(String(63), primary_key=True) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now(), nullable=False + ) + + def __repr__(self) -> str: + return f"" diff --git a/src/domains/tenants/schemas.py b/src/domains/tenants/schemas.py new file mode 100644 index 0000000..43147d7 --- /dev/null +++ b/src/domains/tenants/schemas.py @@ -0,0 +1,30 @@ +""" +Tenants Domain Schemas + +Pydantic request/response payloads for the tenant registry (core-api T-8 / +workspace D-73). No write routes this round — provisioning is later +(workspace T-195). +""" +from typing import List + +from src.shared.base import BaseSchema + + +class TenantSummary(BaseSchema): + """The shape `GET /tenants` answers with, per tenant.""" + + id: str + display_name: str + kind: str + status: str + + +class TenantMeResponse(TenantSummary): + """`GET /tenants/me` — the caller's own tenant plus the consumers + granted in it, the cheap "am I allowed here" probe (D-73).""" + + consumers: List[str] + + +class TenantListResponse(BaseSchema): + tenants: List[TenantSummary] diff --git a/src/domains/tenants/service.py b/src/domains/tenants/service.py new file mode 100644 index 0000000..74bf22b --- /dev/null +++ b/src/domains/tenants/service.py @@ -0,0 +1,126 @@ +""" +Tenants Domain Service + +The tenant registry (core-api T-8 / workspace D-73): `require_tenant` is +the one enforcement point Book's and Scribe's own `_require_tenant` now +delegate to, rather than each checking only presence (core-api T-1/T-5's +original shape, which let a typo'd claim open a fresh, empty namespace +instead of being refused). + +It takes the session, so it is one query: a LEFT JOIN against the grant +table, so presence, status and (for an Office caller) the grant are all +read in a single round trip. When `caller.consumer` is `None` (tatlock, +scheduler, or a user-kind caller — core-api T-6 carries no consumer for +those), the join condition `TenantConsumer.consumer == None` compiles to +`IS NULL`, which never matches a grant row (consumer is NOT NULL there), +so the grant side of the row is always empty for them — correct, since the +grant check below is skipped outright when `caller.consumer` is `None`. +""" +from typing import List, Optional, Tuple + +from sqlalchemy import and_, select +from sqlalchemy.ext.asyncio import AsyncSession + +from src.domains.auth.callers import Caller +from src.domains.tenants.models import Tenant, TenantConsumer +from src.shared.logging import get_logger + +logger = get_logger(__name__) + + +class TenantError(Exception): + """Raised for a tenant-registry failure; the controller maps it to a response.""" + + def __init__(self, code: str, status_code: int = 400): + self.code = code + self.status_code = status_code + super().__init__(code) + + +class TenantsService: + """The tenant registry's one enforcement point, and its read API.""" + + async def require_tenant(self, session: AsyncSession, caller: Caller) -> str: + """ + The one enforcement point (core-api T-8 / workspace D-73): + + - no tenant on the caller -> 400 `tenant_required` (unchanged) + - tenant not in the table -> 403 `tenant_unknown` + - tenant `suspended` -> 403 `tenant_suspended` + - an Office caller (carries a consumer) whose consumer holds no + grant in that tenant -> 403 `consumer_not_granted` + - a user-kind caller carries no consumer (core-api T-6, not this + round): tenant checks only. + + Returns the tenant id on success. + """ + if not caller.tenant: + raise TenantError("tenant_required", 400) + tenant = caller.tenant + + stmt = ( + select(Tenant.status, TenantConsumer.consumer) + .outerjoin( + TenantConsumer, + and_( + TenantConsumer.tenant_id == Tenant.id, + TenantConsumer.consumer == caller.consumer, + ), + ) + .where(Tenant.id == tenant) + ) + row = (await session.execute(stmt)).first() + if row is None: + raise TenantError("tenant_unknown", 403) + + status, granted_consumer = row + if status == "suspended": + raise TenantError("tenant_suspended", 403) + if caller.consumer is not None and granted_consumer is None: + raise TenantError("consumer_not_granted", 403) + + return tenant + + async def get_me(self, session: AsyncSession, caller: Caller) -> Tuple[Tenant, List[str]]: + """`GET /tenants/me` — through the same helper, so "am I allowed + here" shares its answer with every other tenant-scoped call.""" + tenant_id = await self.require_tenant(session, caller) + + tenant = await session.get(Tenant, tenant_id) + consumers_stmt = ( + select(TenantConsumer.consumer) + .where(TenantConsumer.tenant_id == tenant_id) + .order_by(TenantConsumer.consumer) + ) + consumers = list((await session.execute(consumers_stmt)).scalars().all()) + return tenant, consumers + + async def list_tenants(self, session: AsyncSession, caller: Caller) -> List[Tenant]: + """ + `GET /tenants` — `tatlock` and `scheduler` callers (no tenant) see + every tenant; a tenant-bearing caller sees only its own, as a list + of one (the hard boundary: another tenant's existence is never + revealed, so this still runs through `require_tenant`); a user + caller with no tenant is 400 `tenant_required`. + """ + if caller.tenant is None: + if caller.kind in ("tatlock", "scheduler"): + stmt = select(Tenant).order_by(Tenant.id) + return list((await session.execute(stmt)).scalars().all()) + raise TenantError("tenant_required", 400) + + tenant_id = await self.require_tenant(session, caller) + tenant = await session.get(Tenant, tenant_id) + return [tenant] + + +# Singleton instance +_tenants_service: Optional[TenantsService] = None + + +def get_tenants_service() -> TenantsService: + """Get singleton Tenants service instance""" + global _tenants_service + if _tenants_service is None: + _tenants_service = TenantsService() + return _tenants_service diff --git a/src/main.py b/src/main.py index d40a5fb..796b82e 100644 --- a/src/main.py +++ b/src/main.py @@ -21,6 +21,7 @@ from src.domains.static import static_controller from src.domains.dashboard import dashboard_controller from src.domains.book import book_controller from src.domains.scribe import scribe_controller +from src.domains.tenants import tenants_controller # Initialize settings settings = get_settings() @@ -110,6 +111,7 @@ app.include_router(static_controller.router) # /static/* app.include_router(dashboard_controller.router) # /dashboard/* app.include_router(book_controller.router) # /book/* app.include_router(scribe_controller.router) # /scribe/* +app.include_router(tenants_controller.router) # /tenants/* # Global exception handler diff --git a/tests/conftest.py b/tests/conftest.py index bb86f91..44cf32b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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() diff --git a/tests/test_book.py b/tests/test_book.py index f88dbc3..2bbb23e 100644 --- a/tests/test_book.py +++ b/tests/test_book.py @@ -721,3 +721,37 @@ class TestUserCallerTenantRequired: assert response.status_code == 201 assert response.json()["created_by"] == "jeroen" assert response.json()["number"] == 1 + + +class TestTenantRegistryEnforcement: + """core-api T-8 / workspace D-73: Book's own `_require_tenant` now + delegates to the tenant registry's one enforcement point, so a Book + route gains three refusal paths it did not have before, alongside the + control that a properly granted, active tenant still succeeds.""" + + def test_unknown_tenant_is_403(self, client): + response = client.get( + "/book/records", headers=office_headers(tenant="no-such-tenant") + ) + assert response.status_code == 403 + assert response.json()["detail"]["code"] == "tenant_unknown" + + def test_suspended_tenant_is_403(self, client, suspended_tenant): + response = client.get( + "/book/records", headers=office_headers(tenant=suspended_tenant) + ) + assert response.status_code == 403 + assert response.json()["detail"]["code"] == "tenant_suspended" + + def test_consumer_without_a_grant_is_403(self, client): + response = client.get( + "/book/records", headers=office_headers(consumer="office-ungranted") + ) + assert response.status_code == 403 + assert response.json()["detail"]["code"] == "consumer_not_granted" + + def test_granted_consumer_in_an_active_tenant_succeeds(self, client): + """The control: office-claude, granted in jpmschweitzer (active), + still reaches the route.""" + response = client.get("/book/records", headers=office_headers()) + assert response.status_code == 200 diff --git a/tests/test_scribe.py b/tests/test_scribe.py index 7379ccb..05148ca 100644 --- a/tests/test_scribe.py +++ b/tests/test_scribe.py @@ -579,3 +579,42 @@ class TestTenantIsolation: ).json() assert found["total"] == 1 assert found["entries"][0]["message"]["body"] == "shared-search-term in TENANT" + + +class TestTenantRegistryEnforcement: + """core-api T-8 / workspace D-73: Scribe's own `_require_tenant` now + delegates to the tenant registry's one enforcement point, so a Scribe + route gains three refusal paths it did not have before, alongside the + control that a properly granted, active tenant still succeeds.""" + + def test_unknown_tenant_is_403(self, client): + response = client.get( + "/scribe/addresses", + headers=office_headers("jeroen", "office-claude", tenant="no-such-tenant"), + ) + assert response.status_code == 403 + assert response.json()["detail"]["code"] == "tenant_unknown" + + def test_suspended_tenant_is_403(self, client, suspended_tenant): + response = client.get( + "/scribe/addresses", + headers=office_headers("jeroen", "office-claude", tenant=suspended_tenant), + ) + assert response.status_code == 403 + assert response.json()["detail"]["code"] == "tenant_suspended" + + def test_consumer_without_a_grant_is_403(self, client): + response = client.get( + "/scribe/addresses", + headers=office_headers("jeroen", "office-ungranted", tenant=TENANT), + ) + assert response.status_code == 403 + assert response.json()["detail"]["code"] == "consumer_not_granted" + + def test_granted_consumer_in_an_active_tenant_succeeds(self, client): + """The control: office-claude, granted in jpmschweitzer (active), + still reaches the route.""" + response = client.get( + "/scribe/addresses", headers=office_headers("jeroen", "office-claude", tenant=TENANT) + ) + assert response.status_code == 200 diff --git a/tests/test_tenants.py b/tests/test_tenants.py new file mode 100644 index 0000000..3ed2251 --- /dev/null +++ b/tests/test_tenants.py @@ -0,0 +1,205 @@ +""" +Tests for the tenant registry's read API (core-api T-8 / workspace D-73) +against `core_api_test`: `GET /tenants/me` and `GET /tenants`. + +Follows tests/test_scribe.py's TestClient pattern: `get_async_session` is +overridden to the test database for every test, building a fresh +engine/session per call because TestClient drives the ASGI app on a +different event loop than pytest-asyncio's fixture setup. +""" +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +from src.domains.auth.callers import _TENANT_RE +from src.domains.tenants.models import TENANT_ID_PATTERN +from src.main import app +from src.shared.config import get_settings +from src.shared.database import get_async_session + +settings = get_settings() + +OFFICE_KEY = "test-office-key" +TATLOCK_KEY = "test-tatlock-key" +SCHEDULER_KEY = "test-scheduler-key" +TENANT = "jpmschweitzer" +OTHER_TENANT = "llm_tester" + + +def test_tenant_id_pattern_matches_callers_tenant_regex(): + """models.py deliberately duplicates callers.py's `_TENANT_RE` shape + rather than importing its private constant (see models.py's comment) — + assert the two never drift apart instead.""" + assert TENANT_ID_PATTERN == _TENANT_RE.pattern + + +@pytest.fixture(autouse=True) +def service_keys(): + """Every test here authenticates with one of the three static keys + unless it says otherwise.""" + originals = ( + settings.office_service_key, + settings.tatlock_service_key, + settings.scheduler_service_key, + ) + settings.office_service_key = OFFICE_KEY + settings.tatlock_service_key = TATLOCK_KEY + settings.scheduler_service_key = SCHEDULER_KEY + yield + ( + settings.office_service_key, + settings.tatlock_service_key, + settings.scheduler_service_key, + ) = originals + + +@pytest.fixture +def client(tenant_registry, test_database_url): + from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine + from sqlalchemy.pool import NullPool + + asyncpg_url = test_database_url.replace("postgresql://", "postgresql+asyncpg://", 1) + + async def _override(): + engine = create_async_engine(asyncpg_url, poolclass=NullPool) + session_factory = async_sessionmaker(bind=engine, expire_on_commit=False) + async with session_factory() as session: + yield session + await engine.dispose() + + app.dependency_overrides[get_async_session] = _override + yield TestClient(app) + app.dependency_overrides.pop(get_async_session, None) + + +def office_headers( + identity: str = "jeroen", consumer: str = "office-claude", tenant: str = TENANT +) -> dict: + return { + "Authorization": f"Bearer {OFFICE_KEY}", + "X-Office-Identity": identity, + "X-Office-Consumer": consumer, + "X-Office-Tenant": tenant, + } + + +def tatlock_headers() -> dict: + return {"Authorization": f"Bearer {TATLOCK_KEY}"} + + +def scheduler_headers() -> dict: + return {"Authorization": f"Bearer {SCHEDULER_KEY}"} + + +class TestMe: + """`GET /tenants/me` — the cheap "am I allowed here" probe (D-73).""" + + def test_office_caller_sees_its_own_tenant_and_its_grants(self, client): + response = client.get("/tenants/me", headers=office_headers()) + assert response.status_code == 200 + body = response.json() + assert body["id"] == TENANT + assert body["display_name"] == "Jeroen" + assert body["kind"] == "household" + assert body["status"] == "active" + # office-claude is the grant the dossier seeds; office-gemini is a + # test-environment-only addition (tests/conftest.py's + # tenant_registry fixture) — assert membership, not the exact list, + # so this test is not coupled to that test-only extra. + assert "office-claude" in body["consumers"] + + def test_other_seeded_tenant_answers_its_own_identity(self, client): + response = client.get("/tenants/me", headers=office_headers(tenant=OTHER_TENANT)) + assert response.status_code == 200 + body = response.json() + assert body["id"] == OTHER_TENANT + assert body["kind"] == "test" + + def test_unknown_tenant_is_403(self, client): + response = client.get( + "/tenants/me", headers=office_headers(tenant="no-such-tenant") + ) + assert response.status_code == 403 + assert response.json()["detail"]["code"] == "tenant_unknown" + + def test_suspended_tenant_is_403(self, client, suspended_tenant): + response = client.get( + "/tenants/me", headers=office_headers(tenant=suspended_tenant) + ) + assert response.status_code == 403 + assert response.json()["detail"]["code"] == "tenant_suspended" + + def test_consumer_without_a_grant_is_403(self, client): + response = client.get( + "/tenants/me", headers=office_headers(consumer="office-ungranted") + ) + assert response.status_code == 403 + assert response.json()["detail"]["code"] == "consumer_not_granted" + + def test_tatlock_caller_is_403_forbidden(self, client): + """`/me` is Office/user-scoped (`require_tenants_reader`), the same + reader class as Book and Scribe — tatlock has no "own tenant" to + answer with, so it is refused at the caller-kind check, before the + service ever runs, rather than reaching a tenant_required 400.""" + response = client.get("/tenants/me", headers=tatlock_headers()) + assert response.status_code == 403 + assert response.json()["detail"]["code"] == "forbidden" + + def test_user_without_tenant_claim_is_400(self, client): + with patch("src.domains.auth.callers.get_current_user") as mock_get_current_user: + mock_get_current_user.return_value = {"preferred_username": "jeroen"} + response = client.get("/tenants/me", headers={"Authorization": "Bearer some-jwt"}) + assert response.status_code == 400 + assert response.json()["detail"]["code"] == "tenant_required" + + def test_user_with_tenant_claim_succeeds(self, client): + with patch("src.domains.auth.callers.get_current_user") as mock_get_current_user: + mock_get_current_user.return_value = { + "preferred_username": "jeroen", + "tatlock_user": TENANT, + } + response = client.get("/tenants/me", headers={"Authorization": "Bearer some-jwt"}) + assert response.status_code == 200 + assert response.json()["id"] == TENANT + + +class TestList: + """`GET /tenants` — the hard boundary: another tenant's existence is + never revealed to a tenant-bearing caller.""" + + def test_tatlock_sees_every_tenant(self, client): + response = client.get("/tenants", headers=tatlock_headers()) + assert response.status_code == 200 + ids = {t["id"] for t in response.json()["tenants"]} + assert {TENANT, OTHER_TENANT}.issubset(ids) + + def test_scheduler_sees_every_tenant(self, client): + response = client.get("/tenants", headers=scheduler_headers()) + assert response.status_code == 200 + ids = {t["id"] for t in response.json()["tenants"]} + assert {TENANT, OTHER_TENANT}.issubset(ids) + + def test_office_caller_sees_only_its_own_tenant_as_a_list_of_one(self, client): + response = client.get("/tenants", headers=office_headers(tenant=TENANT)) + assert response.status_code == 200 + tenants = response.json()["tenants"] + assert len(tenants) == 1 + assert tenants[0]["id"] == TENANT + # The hard boundary (D-73): the other seeded tenant's existence is + # not revealed, not even as an id in a list it is excluded from. + assert all(t["id"] != OTHER_TENANT for t in tenants) + + def test_other_tenant_caller_sees_only_its_own_as_a_list_of_one(self, client): + response = client.get("/tenants", headers=office_headers(tenant=OTHER_TENANT)) + assert response.status_code == 200 + tenants = response.json()["tenants"] + assert len(tenants) == 1 + assert tenants[0]["id"] == OTHER_TENANT + + def test_user_without_tenant_claim_is_400(self, client): + with patch("src.domains.auth.callers.get_current_user") as mock_get_current_user: + mock_get_current_user.return_value = {"preferred_username": "jeroen"} + response = client.get("/tenants", headers={"Authorization": "Bearer some-jwt"}) + assert response.status_code == 400 + assert response.json()["detail"]["code"] == "tenant_required" diff --git a/tests/test_tenants_migration.py b/tests/test_tenants_migration.py new file mode 100644 index 0000000..3f8f612 --- /dev/null +++ b/tests/test_tenants_migration.py @@ -0,0 +1,188 @@ +""" +Tests for the tenant registry migration 007 (core-api T-8, workspace D-73). + +Runs against `core_api_test` only (see conftest.py — skip when +TEST_DATABASE_URL is unset, hard failure if it resolves to `core_api`). +Self-contained: starts and ends at head regardless of what ran before it, +so test order never matters for the schema the rest of the suite needs. +""" +import pytest +from sqlalchemy import text +from sqlalchemy.exc import IntegrityError +from sqlalchemy.ext.asyncio import create_async_engine +from sqlalchemy.pool import NullPool + +from tests.conftest import run_alembic + + +async def _table_exists(url: str, table: str) -> bool: + asyncpg_url = url.replace("postgresql://", "postgresql+asyncpg://", 1) + engine = create_async_engine(asyncpg_url, poolclass=NullPool) + try: + async with engine.connect() as conn: + result = await conn.execute( + text("SELECT to_regclass(:name) IS NOT NULL AS present"), + {"name": table}, + ) + return bool(result.scalar()) + finally: + await engine.dispose() + + +async def _scalar(url: str, query: str, **params) -> object: + asyncpg_url = url.replace("postgresql://", "postgresql+asyncpg://", 1) + engine = create_async_engine(asyncpg_url, poolclass=NullPool) + try: + async with engine.connect() as conn: + result = await conn.execute(text(query), params) + return result.scalar() + finally: + await engine.dispose() + + +class TestMigration007RoundTrip: + """Migration 007 must upgrade and downgrade cleanly (dossier verification #2).""" + + @pytest.mark.asyncio + async def test_upgrade_creates_tenant_tables(self, test_database_url): + result = run_alembic(test_database_url, "upgrade", "head") + assert result.returncode == 0, result.stderr + + for table in ("tenants", "tenant_consumers"): + assert await _table_exists(test_database_url, table), f"{table} missing after upgrade" + + @pytest.mark.asyncio + async def test_downgrade_then_upgrade_round_trips_cleanly(self, test_database_url): + baseline = run_alembic(test_database_url, "upgrade", "head") + assert baseline.returncode == 0, baseline.stderr + + down = run_alembic(test_database_url, "downgrade", "006") + assert down.returncode == 0, down.stderr + assert not await _table_exists(test_database_url, "tenants") + assert not await _table_exists(test_database_url, "tenant_consumers") + # Book and Scribe tables, created by migrations below this one, must + # survive a downgrade of 007 — proves 007 only touches its own + # tables (and the FKs it added onto theirs). + assert await _table_exists(test_database_url, "book_records") + assert await _table_exists(test_database_url, "scribe_addresses") + + # Leave the schema at head for the rest of the suite. + up = run_alembic(test_database_url, "upgrade", "007") + assert up.returncode == 0, up.stderr + assert await _table_exists(test_database_url, "tenants") + assert await _table_exists(test_database_url, "tenant_consumers") + + +class TestMigration007Checks: + """The CHECK constraints on `tenants.kind` and `tenants.status`.""" + + @pytest.mark.asyncio + async def test_kind_check_constraint_exists(self, test_database_url): + run_alembic(test_database_url, "upgrade", "head") + found = await _scalar( + test_database_url, + "SELECT conname FROM pg_constraint WHERE conname = 'ck_tenants_kind'", + ) + assert found == "ck_tenants_kind" + + @pytest.mark.asyncio + async def test_status_check_constraint_exists(self, test_database_url): + run_alembic(test_database_url, "upgrade", "head") + found = await _scalar( + test_database_url, + "SELECT conname FROM pg_constraint WHERE conname = 'ck_tenants_status'", + ) + assert found == "ck_tenants_status" + + @pytest.mark.asyncio + async def test_kind_check_rejects_an_invalid_kind(self, test_database_url): + run_alembic(test_database_url, "upgrade", "head") + asyncpg_url = test_database_url.replace("postgresql://", "postgresql+asyncpg://", 1) + engine = create_async_engine(asyncpg_url, poolclass=NullPool) + try: + async with engine.connect() as conn: + with pytest.raises(IntegrityError): + await conn.execute( + text( + "INSERT INTO tenants (id, display_name, kind, status) " + "VALUES ('bad-kind-tenant', 'x', 'nonsense', 'active')" + ) + ) + await conn.commit() + finally: + await engine.dispose() + + +class TestMigration007Seed: + """core-api T-8: the registry seeds exactly the tenants/grants the + dossier specifies — not a third, and not a second grant.""" + + @pytest.mark.asyncio + async def test_seeds_jpmschweitzer_and_llm_tester(self, test_database_url): + run_alembic(test_database_url, "upgrade", "head") + for tenant_id, display_name, kind in ( + ("jpmschweitzer", "Jeroen", "household"), + ("llm_tester", "Playground", "test"), + ): + row_kind = await _scalar( + test_database_url, + "SELECT kind FROM tenants WHERE id = :id AND display_name = :name AND status = 'active'", + id=tenant_id, + name=display_name, + ) + assert row_kind == kind, f"{tenant_id} seed row missing or wrong" + + @pytest.mark.asyncio + async def test_both_tenants_grant_only_office_claude(self, test_database_url): + run_alembic(test_database_url, "upgrade", "head") + for tenant_id in ("jpmschweitzer", "llm_tester"): + found = await _scalar( + test_database_url, + "SELECT consumer FROM tenant_consumers WHERE tenant_id = :id AND consumer = 'office-claude'", + id=tenant_id, + ) + assert found == "office-claude", f"{tenant_id} is missing its office-claude grant" + + +class TestMigration007ForeignKeys: + """Every owner_tenant column (five of them, per the dossier) now carries + an FK to tenants.id; an unknown owner_tenant is refused at the database, + not only in the service layer (dossier verification #5).""" + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "fk_name", + [ + "fk_book_records_owner_tenant", + "fk_book_tenant_counters_owner_tenant", + "fk_scribe_addresses_owner_tenant", + "fk_scribe_messages_owner_tenant", + "fk_scribe_mailbox_entries_owner_tenant", + ], + ) + async def test_owner_tenant_fk_exists(self, test_database_url, fk_name): + run_alembic(test_database_url, "upgrade", "head") + found = await _scalar( + test_database_url, + "SELECT conname FROM pg_constraint WHERE conname = :name AND contype = 'f'", + name=fk_name, + ) + assert found == fk_name + + @pytest.mark.asyncio + async def test_fk_refuses_a_scribe_address_with_an_unknown_owner_tenant(self, test_database_url): + run_alembic(test_database_url, "upgrade", "head") + asyncpg_url = test_database_url.replace("postgresql://", "postgresql+asyncpg://", 1) + engine = create_async_engine(asyncpg_url, poolclass=NullPool) + try: + async with engine.connect() as conn: + with pytest.raises(IntegrityError): + await conn.execute( + text( + "INSERT INTO scribe_addresses (owner_tenant, name, created_by) " + "VALUES ('no-such-tenant', 'fk-probe', 'test-harness')" + ) + ) + await conn.commit() + finally: + await engine.dispose()