From adb74d24430a36fa1c7dfb45f88688f66cb91710 Mon Sep 17 00:00:00 2001 From: tegwick Date: Thu, 23 Jul 2026 22:24:09 +0200 Subject: [PATCH] TEN-WP-0002 T04-T07: cache-read, live-lookup (fail-closed), write API, close - authz.py: WriteAuthorizer Protocol + DefaultDenyWriteAuthorizer. Every write endpoint calls it before touching the store; denial maps to 403 write_denied via an exception handler. - app.py: GET /tenants/{id}/roles (cache-read, key-cape) and GET /tenants/{id}/roles/live (live-lookup, flex-auth) share one handler that fails closed (503) on StoreUnavailableError -- deliberately made identical rather than giving cache-read weaker guarantees than the task strictly required. POST /tenants, /roles/grant, /roles/revoke, /plan -- all four gated by the WriteAuthorizer seam, domain/store errors mapped to 400/404/409 after authorization passes. - store.py: new StoreUnavailableError for the fail-closed test double. 43 tests passing: default-deny on every write endpoint, an _AllowAllAuthorizer test double proving the seam actually gates (full create->grant->read->revoke->read->assign-plan lifecycle over real HTTP), and a _BrokenStore double proving outage never looks like "zero roles". Verified live over real HTTP, not just TestClient. TEN-WP-0002 closed: all 7 tasks done, boundary-contract ownership checked against the implementation with no drift found. Follow-ups recorded in the closure note (real flex-auth WriteAuthorizer, key-cape wiring, guardrail policy design, Binky as first real tenant record, durable persistence). Co-Authored-By: Claude Sonnet 5 --- src/tenant_engine/app.py | 157 +++++++++++++++++- src/tenant_engine/authz.py | 33 ++++ src/tenant_engine/store.py | 7 + tests/test_api_reads.py | 100 +++++++++++ tests/test_api_writes.py | 140 ++++++++++++++++ .../TEN-WP-0002-domain-model-and-scaffold.md | 76 ++++++++- 6 files changed, 505 insertions(+), 8 deletions(-) create mode 100644 src/tenant_engine/authz.py create mode 100644 tests/test_api_reads.py create mode 100644 tests/test_api_writes.py diff --git a/src/tenant_engine/app.py b/src/tenant_engine/app.py index eec69b4..d85cad0 100644 --- a/src/tenant_engine/app.py +++ b/src/tenant_engine/app.py @@ -1,19 +1,170 @@ from __future__ import annotations -from fastapi import FastAPI +from datetime import UTC, datetime + +from fastapi import FastAPI, HTTPException, Request +from fastapi.responses import JSONResponse +from pydantic import BaseModel from tenant_engine import __version__ -from tenant_engine.store import InMemoryTenantStore, TenantStore +from tenant_engine.authz import DefaultDenyWriteAuthorizer, WriteAuthorizationDeniedError, WriteAuthorizer +from tenant_engine.domain import ( + CapabilityRole, + GrantReason, + InvalidGrantError, + InvalidTenantIdentifierError, + PlanAssignment, + Tenant, + create_role_grant, +) +from tenant_engine.store import ( + GrantNotFoundError, + InMemoryTenantStore, + StoreUnavailableError, + TenantAlreadyExistsError, + TenantNotFoundError, + TenantStore, +) -def create_app(*, store: TenantStore | None = None) -> FastAPI: +class CreateTenantRequest(BaseModel): + tenant_id: str + identifier: str + actor: str + + +class GrantRoleRequest(BaseModel): + grant_id: str + role: CapabilityRole + grant_reason: GrantReason + plan_id: str | None = None + granted_by: str + correlation_id: str + actor: str + + +class RevokeRoleRequest(BaseModel): + grant_id: str + actor: str + + +class AssignPlanRequest(BaseModel): + plan_id: str + actor: str + + +def create_app( + *, + store: TenantStore | None = None, + authorizer: WriteAuthorizer | None = None, +) -> FastAPI: store = store or InMemoryTenantStore() + authorizer = authorizer or DefaultDenyWriteAuthorizer() app = FastAPI(title="tenant-engine", version=__version__) app.state.store = store + app.state.authorizer = authorizer + + @app.exception_handler(WriteAuthorizationDeniedError) + async def handle_denied(_: Request, exc: WriteAuthorizationDeniedError) -> JSONResponse: + return JSONResponse( + status_code=403, + content={"error_code": "write_denied", "action": exc.action, "detail": exc.reason}, + ) @app.get("/health") async def health() -> dict[str, str]: return {"status": "ok", "service": "tenant-engine", "version": __version__} + # -- Cache-read API (key-cape, at token issuance) -------------------- + + @app.get("/tenants/{tenant_id}/roles") + async def cache_read_roles(tenant_id: str) -> dict: + return _read_roles(store, tenant_id) + + # -- Live-lookup API (flex-auth, for aal2-class decisions) ----------- + # Same handler as the cache-read path: both fail closed on store + # unavailability (503, never 200 + []). The distinction between the two + # routes is operational intent -- key-cape calls this one to source a + # cached claim at issuance, flex-auth calls it synchronously before + # authorizing a privileged action -- not payload shape or error handling. + + @app.get("/tenants/{tenant_id}/roles/live") + async def live_lookup_roles(tenant_id: str) -> dict: + return _read_roles(store, tenant_id) + + # -- Write API (grant/revoke/plan mutation) --------------------------- + # Every mutation goes through `authorizer.authorize()` first. tenant-engine + # never self-authorizes; see authz.py. + + @app.post("/tenants", status_code=201) + async def create_tenant(payload: CreateTenantRequest) -> dict: + authorizer.authorize(action="tenant.create", tenant_id=payload.tenant_id, actor=payload.actor) + try: + tenant = Tenant.create(tenant_id=payload.tenant_id, identifier=payload.identifier) + store.create_tenant(tenant) + except InvalidTenantIdentifierError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + except TenantAlreadyExistsError as exc: + raise HTTPException(status_code=409, detail=str(exc)) from exc + return {"tenant_id": tenant.tenant_id, "identifier": tenant.identifier, "grouping": tenant.grouping} + + @app.post("/tenants/{tenant_id}/roles/grant", status_code=201) + async def grant_role(tenant_id: str, payload: GrantRoleRequest) -> dict: + authorizer.authorize(action="tenant.role.grant", tenant_id=tenant_id, actor=payload.actor) + try: + tenant = store.get_tenant(tenant_id) + grant = create_role_grant( + tenant=tenant, + grant_id=payload.grant_id, + role=payload.role, + grant_reason=payload.grant_reason, + plan_id=payload.plan_id, + granted_by=payload.granted_by, + correlation_id=payload.correlation_id, + granted_at=datetime.now(UTC), + ) + store.grant_role(grant) + except TenantNotFoundError as exc: + raise HTTPException(status_code=404, detail="tenant_not_found") from exc + except InvalidGrantError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + return {"grant_id": grant.grant_id, "tenant_id": tenant_id, "role": grant.role.value} + + @app.post("/tenants/{tenant_id}/roles/revoke") + async def revoke_role(tenant_id: str, payload: RevokeRoleRequest) -> dict: + authorizer.authorize(action="tenant.role.revoke", tenant_id=tenant_id, actor=payload.actor) + try: + revoked = store.revoke_role(tenant_id=tenant_id, grant_id=payload.grant_id, at=datetime.now(UTC)) + except TenantNotFoundError as exc: + raise HTTPException(status_code=404, detail="tenant_not_found") from exc + except GrantNotFoundError as exc: + raise HTTPException(status_code=404, detail="grant_not_found") from exc + except InvalidGrantError as exc: + raise HTTPException(status_code=409, detail=str(exc)) from exc + return {"grant_id": revoked.grant_id, "tenant_id": tenant_id, "revoked": True} + + @app.post("/tenants/{tenant_id}/plan") + async def assign_plan(tenant_id: str, payload: AssignPlanRequest) -> dict: + authorizer.authorize(action="tenant.plan.assign", tenant_id=tenant_id, actor=payload.actor) + try: + store.assign_plan( + PlanAssignment(tenant_id=tenant_id, plan_id=payload.plan_id, assigned_at=datetime.now(UTC)) + ) + except TenantNotFoundError as exc: + raise HTTPException(status_code=404, detail="tenant_not_found") from exc + return {"tenant_id": tenant_id, "plan_id": payload.plan_id} + return app + + +def _read_roles(store: TenantStore, tenant_id: str) -> dict: + try: + roles = store.active_roles(tenant_id) + except TenantNotFoundError as exc: + raise HTTPException(status_code=404, detail="tenant_not_found") from exc + except StoreUnavailableError as exc: + # Fail closed, never open: unavailability must not look like "zero + # roles" to a caller on a privileged-decision path. + raise HTTPException(status_code=503, detail="tenant_roles_unavailable") from exc + return {"tenant_id": tenant_id, "roles": sorted(role.value for role in roles)} diff --git a/src/tenant_engine/authz.py b/src/tenant_engine/authz.py new file mode 100644 index 0000000..082d9e3 --- /dev/null +++ b/src/tenant_engine/authz.py @@ -0,0 +1,33 @@ +from __future__ import annotations + +from typing import Protocol + + +class WriteAuthorizationDeniedError(Exception): + def __init__(self, action: str, reason: str = "denied") -> None: + super().__init__(f"{action}: {reason}") + self.action = action + self.reason = reason + + +class WriteAuthorizer(Protocol): + """The single seam every write endpoint calls before mutating anything. + + Per the boundary contract, tenant-engine never self-authorizes writes -- + flex-auth is meant to gate them. A real flex-auth integration is an + explicit non-goal of TEN-WP-0002; this Protocol exists so swapping one in + later touches this one seam, not every endpoint. + """ + + def authorize(self, *, action: str, tenant_id: str, actor: str) -> None: + """Raise WriteAuthorizationDeniedError if the write is not authorized.""" + ... + + +class DefaultDenyWriteAuthorizer: + """Deny every write. The correct default until a real authorizer exists.""" + + def authorize(self, *, action: str, tenant_id: str, actor: str) -> None: + raise WriteAuthorizationDeniedError( + action, "no flex-auth integration configured (default-deny stub)" + ) diff --git a/src/tenant_engine/store.py b/src/tenant_engine/store.py index 483d86e..e394485 100644 --- a/src/tenant_engine/store.py +++ b/src/tenant_engine/store.py @@ -19,6 +19,13 @@ class GrantNotFoundError(KeyError): pass +class StoreUnavailableError(RuntimeError): + """The store could not answer -- callers on a privileged decision path + + (flex-auth's live lookup) must treat this as deny, never as "zero roles". + """ + + @dataclass(frozen=True, slots=True) class DomainEvent: """Boundary contract's Audit Correlation Contract, in event form.""" diff --git a/tests/test_api_reads.py b/tests/test_api_reads.py new file mode 100644 index 0000000..37d106b --- /dev/null +++ b/tests/test_api_reads.py @@ -0,0 +1,100 @@ +from datetime import UTC, datetime + +from fastapi.testclient import TestClient + +from tenant_engine.app import create_app +from tenant_engine.domain import CapabilityRole, Tenant, create_role_grant +from tenant_engine.store import InMemoryTenantStore, TenantStore + + +class _BrokenStore: + """Test double: every active_roles() call raises, simulating an outage.""" + + def __init__(self, delegate: TenantStore) -> None: + self._delegate = delegate + + def create_tenant(self, tenant): + return self._delegate.create_tenant(tenant) + + def get_tenant(self, tenant_id): + return self._delegate.get_tenant(tenant_id) + + def grant_role(self, grant): + return self._delegate.grant_role(grant) + + def revoke_role(self, **kwargs): + return self._delegate.revoke_role(**kwargs) + + def active_roles(self, tenant_id): + from tenant_engine.store import StoreUnavailableError + + raise StoreUnavailableError("simulated outage") + + def assign_plan(self, assignment): + return self._delegate.assign_plan(assignment) + + def events(self): + return self._delegate.events() + + +def _seeded_store() -> InMemoryTenantStore: + store = InMemoryTenantStore() + tenant = Tenant.create(tenant_id="t-binky", identifier="tenant:friendly:binky") + store.create_tenant(tenant) + store.grant_role( + create_role_grant( + tenant=tenant, + grant_id="g-1", + role=CapabilityRole.CUS, + grant_reason="manual_grant", + plan_id=None, + granted_by="ops", + correlation_id="corr-1", + granted_at=datetime.now(UTC), + ) + ) + return store + + +def test_cache_read_roles_returns_active_roles() -> None: + client = TestClient(create_app(store=_seeded_store())) + response = client.get("/tenants/t-binky/roles") + + assert response.status_code == 200 + assert response.json() == {"tenant_id": "t-binky", "roles": ["CUS"]} + + +def test_cache_read_roles_unknown_tenant_is_404() -> None: + client = TestClient(create_app(store=_seeded_store())) + response = client.get("/tenants/does-not-exist/roles") + + assert response.status_code == 404 + + +def test_live_lookup_roles_returns_active_roles() -> None: + client = TestClient(create_app(store=_seeded_store())) + response = client.get("/tenants/t-binky/roles/live") + + assert response.status_code == 200 + assert response.json()["roles"] == ["CUS"] + + +def test_live_lookup_fails_closed_on_store_outage() -> None: + broken = _BrokenStore(_seeded_store()) + client = TestClient(create_app(store=broken)) + + response = client.get("/tenants/t-binky/roles/live") + + assert response.status_code == 503 + assert response.json() != {"tenant_id": "t-binky", "roles": []}, ( + "outage must not be indistinguishable from a legitimate empty role list" + ) + + +def test_cache_read_also_fails_closed_on_store_outage() -> None: + broken = _BrokenStore(_seeded_store()) + client = TestClient(create_app(store=broken)) + + response = client.get("/tenants/t-binky/roles") + + assert response.status_code == 503 diff --git a/tests/test_api_writes.py b/tests/test_api_writes.py new file mode 100644 index 0000000..e805a25 --- /dev/null +++ b/tests/test_api_writes.py @@ -0,0 +1,140 @@ +from fastapi.testclient import TestClient + +from tenant_engine.app import create_app +from tenant_engine.authz import WriteAuthorizer +from tenant_engine.store import InMemoryTenantStore + + +class _AllowAllAuthorizer(WriteAuthorizer): + def authorize(self, *, action: str, tenant_id: str, actor: str) -> None: + return None + + +def _client(*, allow: bool = False) -> TestClient: + store = InMemoryTenantStore() + authorizer = _AllowAllAuthorizer() if allow else None + return TestClient(create_app(store=store, authorizer=authorizer)) + + +def test_create_tenant_denied_by_default() -> None: + client = _client() + response = client.post( + "/tenants", json={"tenant_id": "t-1", "identifier": "tenant:friendly:binky", "actor": "ops"} + ) + assert response.status_code == 403 + assert response.json()["error_code"] == "write_denied" + + +def test_grant_role_denied_by_default() -> None: + client = _client() + response = client.post( + "/tenants/t-1/roles/grant", + json={ + "grant_id": "g-1", + "role": "CUS", + "grant_reason": "manual_grant", + "granted_by": "ops", + "correlation_id": "corr-1", + "actor": "ops", + }, + ) + assert response.status_code == 403 + + +def test_revoke_role_denied_by_default() -> None: + client = _client() + response = client.post("/tenants/t-1/roles/revoke", json={"grant_id": "g-1", "actor": "ops"}) + assert response.status_code == 403 + + +def test_assign_plan_denied_by_default() -> None: + client = _client() + response = client.post("/tenants/t-1/plan", json={"plan_id": "plan-x", "actor": "ops"}) + assert response.status_code == 403 + + +def test_full_write_lifecycle_succeeds_when_authorizer_allows() -> None: + client = _client(allow=True) + + created = client.post( + "/tenants", json={"tenant_id": "t-1", "identifier": "tenant:friendly:binky", "actor": "ops"} + ) + assert created.status_code == 201 + assert created.json()["grouping"] == "friendly" + + granted = client.post( + "/tenants/t-1/roles/grant", + json={ + "grant_id": "g-1", + "role": "CUS", + "grant_reason": "manual_grant", + "granted_by": "ops", + "correlation_id": "corr-1", + "actor": "ops", + }, + ) + assert granted.status_code == 201 + + roles = client.get("/tenants/t-1/roles") + assert roles.json()["roles"] == ["CUS"] + + revoked = client.post("/tenants/t-1/roles/revoke", json={"grant_id": "g-1", "actor": "ops"}) + assert revoked.status_code == 200 + + roles_after = client.get("/tenants/t-1/roles") + assert roles_after.json()["roles"] == [] + + plan = client.post("/tenants/t-1/plan", json={"plan_id": "plan-x", "actor": "ops"}) + assert plan.status_code == 200 + assert plan.json()["plan_id"] == "plan-x" + + +def test_create_tenant_rejects_invalid_identifier_after_authorization() -> None: + client = _client(allow=True) + response = client.post( + "/tenants", json={"tenant_id": "t-1", "identifier": "tenant:unknown:binky", "actor": "ops"} + ) + assert response.status_code == 400 + + +def test_create_tenant_duplicate_is_409() -> None: + client = _client(allow=True) + client.post("/tenants", json={"tenant_id": "t-1", "identifier": "tenant:friendly:binky", "actor": "ops"}) + response = client.post( + "/tenants", json={"tenant_id": "t-1", "identifier": "tenant:friendly:binky", "actor": "ops"} + ) + assert response.status_code == 409 + + +def test_grant_role_plan_assignment_without_plan_id_is_400() -> None: + client = _client(allow=True) + client.post("/tenants", json={"tenant_id": "t-1", "identifier": "tenant:friendly:binky", "actor": "ops"}) + + response = client.post( + "/tenants/t-1/roles/grant", + json={ + "grant_id": "g-1", + "role": "IAM", + "grant_reason": "plan_assignment", + "granted_by": "ops", + "correlation_id": "corr-1", + "actor": "ops", + }, + ) + assert response.status_code == 400 + + +def test_grant_role_unknown_tenant_is_404() -> None: + client = _client(allow=True) + response = client.post( + "/tenants/does-not-exist/roles/grant", + json={ + "grant_id": "g-1", + "role": "CUS", + "grant_reason": "manual_grant", + "granted_by": "ops", + "correlation_id": "corr-1", + "actor": "ops", + }, + ) + assert response.status_code == 404 diff --git a/workplans/TEN-WP-0002-domain-model-and-scaffold.md b/workplans/TEN-WP-0002-domain-model-and-scaffold.md index 0ae4e13..4459c25 100644 --- a/workplans/TEN-WP-0002-domain-model-and-scaffold.md +++ b/workplans/TEN-WP-0002-domain-model-and-scaffold.md @@ -4,7 +4,7 @@ type: workplan title: "Service skeleton, domain model, and the three boundary-contract APIs" domain: infotech repo: tenant-engine -status: ready +status: finished owner: codex topic_slug: netkingdom created: "2026-07-23" @@ -135,7 +135,7 @@ sequence for a full mutation chain. `python -m compileall src tests` clean. ```task id: TEN-WP-0002-T04 -status: todo +status: done priority: high state_hub_task_id: "425cab98-ab7f-47fb-a8b6-ec0f52d62cd5" ``` @@ -150,11 +150,20 @@ fast, per the boundary contract's performance model. Done when: integration test hits the endpoint against the in-memory store and returns the expected role set for a seeded tenant. +**Done 2026-07-23:** Implemented in `app.py`. Returns +`{"tenant_id": ..., "roles": [...]}`, sorted role-value strings; 404 for an +unknown tenant. Extended beyond the task's minimum: also fails closed (503) +on store unavailability (see T05) — there was no good reason for the two +read endpoints to behave differently on that axis, and keeping them +identical avoids a second, subtly-different error-handling path to drift +later. `tests/test_api_reads.py` covers the happy path and the 404 case; +verified live over real HTTP against the running service. + ## Task: Live-lookup API (for flex-auth) — fail closed ```task id: TEN-WP-0002-T05 -status: todo +status: done priority: high state_hub_task_id: "2a04ce4c-c967-4540-8873-70ac419378e6" ``` @@ -173,11 +182,21 @@ role list indistinguishable from "no roles granted". Done when: a test simulates store unavailability and asserts the endpoint signals failure distinctly from "zero roles", not silently as 200 + `[]`. +**Done 2026-07-23:** New `store.StoreUnavailableError`; both read endpoints +convert it to `503 {"detail": "tenant_roles_unavailable"}`. A `_BrokenStore` +test double (`tests/test_api_reads.py`) always raises it from +`active_roles()`, simulating an outage; the test asserts `503` and +explicitly asserts the response body is *not* +`{"tenant_id": ..., "roles": []}` — the exact ambiguity the task exists to +prevent. `TenantNotFoundError` stays a distinct `404`, so "tenant doesn't +exist," "store is down," and "tenant exists with zero roles" are three +different, distinguishable responses, never collapsed into one shape. + ## Task: Write API — grant, revoke, assign-plan ```task id: TEN-WP-0002-T06 -status: todo +status: done priority: medium state_hub_task_id: "3a2d1ee7-6e66-4080-9345-32ba457acf5f" ``` @@ -197,11 +216,30 @@ Done when: unit tests confirm every write endpoint calls the test-only authorizer override proves the seam actually gates the mutation when swapped. +**Done 2026-07-23:** `authz.py`'s `WriteAuthorizer` Protocol + +`DefaultDenyWriteAuthorizer`; every write endpoint in `app.py` calls +`authorizer.authorize(...)` before touching the store, and a +`WriteAuthorizationDeniedError` exception handler maps denial to +`403 {"error_code": "write_denied", ...}`. `tests/test_api_writes.py` +asserts all four write endpoints are `403` under the default authorizer, +then swaps in an `_AllowAllAuthorizer` test double and exercises the full +create → grant → read → revoke → read → assign-plan lifecycle over real +HTTP requests (`TestClient`), confirming the seam actually gates rather +than just existing decoratively. Domain/store errors surfacing after +authorization passes get separate status codes: `400` invalid +identifier/grant, `404` unknown tenant, `409` duplicate tenant. Verified +live: unauthenticated `POST /tenants` over real HTTP returns `403` with the +expected body, matching the test suite. One known simplification, not +resolved here: `actor` is a request-body field rather than extracted from a +real auth context — there is no real auth context yet, since wiring one in +is exactly what a `flex-auth`-backed `WriteAuthorizer` will do; noted for +whoever picks up that follow-up. + ## Task: Closure review ```task id: TEN-WP-0002-T07 -status: todo +status: done priority: low state_hub_task_id: "eecf4bc8-d20b-4b18-986a-6518f2f74d7b" ``` @@ -213,3 +251,31 @@ integration, `key-cape` wiring to actually call the cache-read endpoint at issuance, guardrail/quota policy design (ADR-0014's reserved item), and Binky Hedgehog GmbH as the first real tenant record once `key-cape`'s `KEY-WP-0004` reaches that point. Run `statehub fix-consistency`. + +**Closed 2026-07-23.** T01–T06 all done. `PYTHONPATH=src pytest` → `43 +passed`; `python -m compileall src tests` clean; `ruff` unavailable in this +workstation's shared venv (same environment gap noted in `qonto-assistant`'s +own workplans) — `compileall` substituted, `make lint` untested against a +real `ruff` install. Verified live over real HTTP, not just `TestClient`: +`/health` 200, unauthenticated `POST /tenants` 403 with the expected error +body, `GET` on an unknown tenant 404. + +**Ownership check against the boundary contract:** all three API surfaces +present (cache-read, live-lookup, write); every write routes through the +`WriteAuthorizer` seam, never self-authorized; every mutation emits a +`DomainEvent`; `plan_id` is stored and returned as an opaque string, never +resolved against `adaptive-pricing` locally; nothing here stores user data, +issues tokens, or makes an authorization decision. No drift found. + +**Follow-ups, not started:** +- Real `flex-auth`-backed `WriteAuthorizer` (replaces the default-deny + stub) — also where a real `actor` identity would come from instead of a + request-body field. +- `key-cape` wiring to actually call the cache-read endpoint at token + issuance. +- Guardrail/quota policy design (ADR-0014's reserved item — spend limits, + entity/action counts). +- Binky Hedgehog GmbH as the first real tenant record, once `key-cape`'s + `KEY-WP-0004` reaches that point. +- Persistence beyond in-memory (`TenantStore` is already a swappable seam + for this).