From d6fd73bd42e2004ff10a366882a58a4628145826 Mon Sep 17 00:00:00 2001 From: tegwick Date: Mon, 10 Aug 2026 20:00:43 +0200 Subject: [PATCH] Implement tenant update and reversible retirement API (TEN-WP-0005 T01-T04) Add an explicit tenant lifecycle (active/retired), allow-listed mutable metadata, record versioning, and lifecycle timestamps to the tenant authority. - domain: TenantLifecycle, with_metadata/retire/reactivate, immutability and transition invariants. Identifier stays immutable -- it is the IAM Profile `tenant` claim key-cape mints into tokens. - store: mutate_tenant() commits idempotency replay, version CAS, mutation, and audit event together; durable receipts survive restart. Retired tenants refuse new grants and plan changes but keep their history. - sqlite: forward-only idempotent migration; existing rows default to active at version 1. Reads now take the write lock -- the concurrent-writer test caught unguarded reads on the shared connection observing mid-transaction state as a spurious tenant_not_found. - api: GET/PATCH /tenants/{id}, POST retire|reactivate. Idempotency-Key and If-Match required, distinct flex-auth actions per operation, stable error schema, redacted 503s. - docs/tenant-lifecycle-api.md: consumer contract for user-engine. Implemented against SQLite, not PostgreSQL as the workplan assumed -- TEN-WP-0004 shipped SQLite on a PVC as the production store. 124 tests pass (was 66); no breaking change to existing endpoints. Co-Authored-By: Claude Opus 5 --- docs/tenant-lifecycle-api.md | 189 +++++++++++ src/tenant_engine/app.py | 308 +++++++++++++++++- src/tenant_engine/authz.py | 5 + src/tenant_engine/domain.py | 122 ++++++- src/tenant_engine/sqlite_store.py | 199 ++++++++++- src/tenant_engine/store.py | 87 ++++- tests/test_api_lifecycle.py | 303 +++++++++++++++++ tests/test_lifecycle_domain.py | 88 +++++ tests/test_lifecycle_store_conformance.py | 296 +++++++++++++++++ ...P-0005-tenant-update-and-retirement-api.md | 56 +++- 10 files changed, 1625 insertions(+), 28 deletions(-) create mode 100644 docs/tenant-lifecycle-api.md create mode 100644 tests/test_api_lifecycle.py create mode 100644 tests/test_lifecycle_domain.py create mode 100644 tests/test_lifecycle_store_conformance.py diff --git a/docs/tenant-lifecycle-api.md b/docs/tenant-lifecycle-api.md new file mode 100644 index 0000000..028bbf8 --- /dev/null +++ b/docs/tenant-lifecycle-api.md @@ -0,0 +1,189 @@ +# Tenant lifecycle API (TEN-WP-0005) + +Consumer contract for tenant metadata update and reversible retirement. +Primary consumer: `user-engine` (USER-WP-0021, platform operator UI/API). + +`tenant-engine` remains the sole authority for tenant existence and +lifecycle. Consumers call this API; they do not keep their own tenant table +and do not implement their own retirement semantics. + +**There is no hard-delete endpoint, by design.** Retirement is reversible and +preserves the tenant record, its grant history, and its plan history so audit +correlation and recovery stay intact. + +--- + +## Lifecycle + +``` + ┌──────────────── retire ────────────────┐ + │ ▼ + [ active ] [ retired ] + ▲ │ + └────────────── reactivate ──────────────┘ +``` + +| State | New role grants | Plan changes | Metadata updates | Role reads | +|---|---|---|---|---| +| `active` | allowed | allowed | allowed | allowed | +| `retired` | **409** | **409** | **409** | allowed (unchanged) | + +Role *revocation* stays available while retired: it only reduces privilege, +and blocking it would be a fail-open behaviour. + +Reactivation restores the tenant's ability to receive new grants and plan +changes. It deliberately does **not** resurrect revoked grants or invent plan +state — those stay exactly as retirement left them. + +--- + +## Immutability + +`tenant_id`, `identifier`, and `grouping` are immutable. The identifier is the +IAM Profile `tenant` claim value that `key-cape` mints into tokens and +`flex-auth` authorizes against; mutating it would silently invalidate every +issued token that references it. + +Mutable metadata is exactly: `display_name`, `contact_email`. Unknown fields +are rejected by the request schema (`extra: forbid`), so the allow-list is +visible in the OpenAPI document rather than discovered from a 400. + +--- + +## Endpoints + +### `GET /tenants/{tenant_id}` + +Authoritative record read. `tenant_id` accepts either the internal id or the +profile identifier (e.g. `tenant:friendly:binky`) — external callers only ever +hold the identifier. + +Returns the record and an `ETag` header carrying the record version. **Read +first, echo the ETag back as `If-Match`** on any mutation. + +```json +{ + "tenant_id": "t-1", + "identifier": "tenant:friendly:binky", + "grouping": "friendly", + "display_name": "Binky", + "contact_email": null, + "lifecycle": "active", + "version": 1, + "created_at": "2026-08-10T12:00:00+00:00", + "updated_at": "2026-08-10T12:00:00+00:00", + "retired_at": null, + "reactivated_at": null +} +``` + +### `PATCH /tenants/{tenant_id}` + +```http +PATCH /tenants/t-1 +Idempotency-Key: 4f1c… +If-Match: "1" + +{"metadata": {"display_name": "Binky Ltd"}, + "actor": "user-engine-portal", "reason": "operator rename", "correlation_id": "corr-1"} +``` + +### `POST /tenants/{tenant_id}/retire` · `POST /tenants/{tenant_id}/reactivate` + +Same headers; body is `{"actor", "reason", "correlation_id"}`. + +All three mutations return the full record (as above) plus `ETag` and +`Idempotent-Replay: true|false`. + +--- + +## Concurrency and idempotency + +Every mutation **requires** both headers: + +- **`If-Match`** — the record version as an ETag (`"1"` or `W/"1"`). Enforced + as an atomic compare-and-swap. `*` is rejected: it would mean "whatever + version is current", which is the unconditional write these endpoints exist + to prevent. +- **`Idempotency-Key`** — caller-generated, unique per logical mutation. + +Replay semantics: + +- **Same key, same request** → the original result is replayed verbatim with + `Idempotent-Replay: true`. The mutation is **not** applied twice, and the + version does not advance. Receipts are durable, so a replay works across a + `tenant-engine` restart. +- **Same key, different request** → `409 idempotency_key_conflict`. +- A mutation that *failed* leaves no receipt, so its key is reusable. + +A replayed key short-circuits **before** the version check — a genuine retry +necessarily carries a now-stale `If-Match`. This is why retries do not need to +re-read the record first. + +--- + +## Errors + +Stable schema on every lifecycle endpoint: + +```json +{"error_code": "version_conflict", "detail": "record version is 2, not 1", "correlation_id": "corr-1"} +``` + +| Status | `error_code` | Cause | +|---|---|---| +| 400 | `idempotency_key_required` | `Idempotency-Key` header missing | +| 400 | `invalid_if_match` | `If-Match` is `*` or not a version ETag | +| 400 | `invalid_update` | empty change set, or a change that is a no-op | +| 403 | `write_denied` | flex-auth denied the action | +| 404 | `tenant_not_found` | unknown tenant | +| 409 | `version_conflict` | stale `If-Match` — re-read and retry | +| 409 | `idempotency_key_conflict` | key reused for a different request | +| 409 | `invalid_lifecycle_transition` | double retirement, reactivating an active tenant, updating a retired tenant | +| 422 | *(schema)* | unknown or immutable field in `metadata` | +| 428 | `if_match_required` | `If-Match` header missing | +| 503 | `tenant_authority_unavailable` | store or authority unavailable | + +A no-op update is rejected rather than silently accepted, so a caller never +reads a version bump as evidence that a value actually changed. + +`503` responses are redacted: they never carry a database path, driver text, +or policy detail. + +--- + +## Authorization + +Mutations are gated by `flex-auth`; `tenant-engine` never self-authorizes. +Three **distinct** actions, so policy can grant a metadata edit without +thereby granting a retirement: + +| Action | Resource type | +|---|---| +| `tenant.update` | `tenant` | +| `tenant.retire` | `tenant` | +| `tenant.reactivate` | `tenant` | + +These must be added to the flex-auth policy package alongside the existing +`tenant.create`, `tenant.role.grant`, `tenant.role.revoke`, and +`tenant.plan.assign` actions. Until they exist, checks resolve to deny — +correct fail-closed behaviour, not a defect. + +--- + +## Compatibility + +No breaking change to existing clients. `POST /tenants` now returns a superset +of its previous body (`tenant_id`, `identifier`, `grouping` unchanged) and +optionally accepts `display_name` / `contact_email`. Role read, grant, revoke, +and plan endpoints are unchanged except that grant and plan-assign now return +`409 tenant_retired` against a retired tenant. + +Existing databases are migrated forward-only: lifecycle columns are added and +existing tenants default to `active` at version 1, with identifiers, grants, +and plan assignments untouched. + +> **Note on the backend.** TEN-WP-0005 was drafted against PostgreSQL, but +> TEN-WP-0004 shipped SQLite on a PVC as the production store. The migration +> and conformance suite target the store that actually runs; the `TenantStore` +> Protocol keeps the seam for a future backend change. diff --git a/src/tenant_engine/app.py b/src/tenant_engine/app.py index 04b35ae..4ece03f 100644 --- a/src/tenant_engine/app.py +++ b/src/tenant_engine/app.py @@ -1,10 +1,12 @@ from __future__ import annotations +import hashlib +import json from datetime import UTC, datetime -from fastapi import FastAPI, HTTPException, Request +from fastapi import FastAPI, Header, HTTPException, Request, Response from fastapi.responses import JSONResponse -from pydantic import BaseModel +from pydantic import BaseModel, Field from tenant_engine import __version__ from tenant_engine.authz import ( @@ -16,21 +18,27 @@ from tenant_engine.authz import ( from tenant_engine.config import Settings from tenant_engine.domain import ( CapabilityRole, + EmptyUpdateError, GrantReason, + ImmutableFieldError, InvalidGrantError, + InvalidLifecycleTransitionError, InvalidTenantIdentifierError, PlanAssignment, Tenant, + TenantRetiredError, create_role_grant, ) from tenant_engine.flex_auth import FlexAuthCheckClient from tenant_engine.store import ( GrantNotFoundError, + IdempotencyConflictError, InMemoryTenantStore, StoreUnavailableError, TenantAlreadyExistsError, TenantNotFoundError, TenantStore, + VersionConflictError, ) @@ -38,6 +46,8 @@ class CreateTenantRequest(BaseModel): tenant_id: str identifier: str actor: str + display_name: str | None = None + contact_email: str | None = None class GrantRoleRequest(BaseModel): @@ -60,6 +70,53 @@ class AssignPlanRequest(BaseModel): actor: str +class TenantMetadata(BaseModel): + """The allow-list, expressed in the schema itself. + + `extra="forbid"` is what turns "unknown fields fail closed" into a + contract consumers can see in the OpenAPI document rather than a rule + they discover from a 400. + """ + + model_config = {"extra": "forbid"} + + display_name: str | None = None + contact_email: str | None = None + + +class UpdateTenantRequest(BaseModel): + model_config = {"extra": "forbid"} + + metadata: TenantMetadata + actor: str + reason: str = Field(min_length=1) + correlation_id: str = Field(min_length=1) + + +class LifecycleRequest(BaseModel): + model_config = {"extra": "forbid"} + + actor: str + reason: str = Field(min_length=1) + correlation_id: str = Field(min_length=1) + + +class LifecycleError(Exception): + """A lifecycle-endpoint failure rendered in the stable error schema. + + Carries only strings this service chose. Store and policy exceptions are + mapped to one of these at the boundary, never reflected verbatim -- a 503 + must not leak a database path or a flex-auth policy name to a consumer. + """ + + def __init__(self, status_code: int, error_code: str, detail: str, correlation_id: str) -> None: + super().__init__(f"{status_code} {error_code}") + self.status_code = status_code + self.error_code = error_code + self.detail = detail + self.correlation_id = correlation_id + + def create_app( *, store: TenantStore | None = None, @@ -81,10 +138,38 @@ def create_app( content={"error_code": "write_denied", "action": exc.action, "detail": exc.reason}, ) + @app.exception_handler(LifecycleError) + async def handle_lifecycle_error(_: Request, exc: LifecycleError) -> JSONResponse: + return JSONResponse( + status_code=exc.status_code, + content={ + "error_code": exc.error_code, + "detail": exc.detail, + "correlation_id": exc.correlation_id, + }, + ) + @app.get("/health") async def health() -> dict[str, str]: return {"status": "ok", "service": "tenant-engine", "version": __version__} + # -- Authoritative tenant record read (TEN-WP-0005) ------------------- + # user-engine's platform operator UI reads this before offering an edit, + # and echoes the returned ETag back as If-Match on the mutation. + + @app.get("/tenants/{tenant_id}") + async def get_tenant(tenant_id: str, response: Response) -> dict: + try: + tenant = store.get_tenant(tenant_id) + except TenantNotFoundError as exc: + raise LifecycleError(404, "tenant_not_found", "tenant_not_found", "") from exc + except StoreUnavailableError as exc: + raise LifecycleError( + 503, "tenant_authority_unavailable", "tenant_authority_unavailable", "" + ) from exc + response.headers["ETag"] = _etag(tenant.version) + return _tenant_response(tenant) + # -- Cache-read API (key-cape, at token issuance) -------------------- @app.get("/tenants/{tenant_id}/roles") @@ -110,13 +195,21 @@ def create_app( 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) + tenant = Tenant.create( + tenant_id=payload.tenant_id, + identifier=payload.identifier, + display_name=payload.display_name, + contact_email=payload.contact_email, + created_at=datetime.now(UTC), + ) 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} + # Superset of the pre-lifecycle response (tenant_id/identifier/grouping + # are unchanged), so existing create clients keep working. + return _tenant_response(tenant) @app.post("/tenants/{tenant_id}/roles/grant", status_code=201) async def grant_role(tenant_id: str, payload: GrantRoleRequest) -> dict: @@ -136,6 +229,8 @@ def create_app( store.grant_role(grant) except TenantNotFoundError as exc: raise HTTPException(status_code=404, detail="tenant_not_found") from exc + except TenantRetiredError as exc: + raise HTTPException(status_code=409, detail="tenant_retired") 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} @@ -162,11 +257,216 @@ def create_app( ) except TenantNotFoundError as exc: raise HTTPException(status_code=404, detail="tenant_not_found") from exc + except TenantRetiredError as exc: + raise HTTPException(status_code=409, detail="tenant_retired") from exc return {"tenant_id": tenant_id, "plan_id": payload.plan_id} + # -- Lifecycle write API (TEN-WP-0005) -------------------------------- + # Update, retire, and reactivate are separately authorized actions, not + # one "tenant.write": user-engine's operator UI can be permitted to + # rename a tenant without thereby being permitted to retire it. + + @app.patch("/tenants/{tenant_id}") + async def update_tenant( + tenant_id: str, + payload: UpdateTenantRequest, + response: Response, + idempotency_key: str | None = Header(default=None, alias="Idempotency-Key"), + if_match: str | None = Header(default=None, alias="If-Match"), + ) -> dict: + changes = payload.metadata.model_dump(exclude_unset=True) + return _lifecycle_mutation( + store=store, + authorizer=authorizer, + action="tenant.update", + tenant_id=tenant_id, + actor=payload.actor, + reason=payload.reason, + correlation_id=payload.correlation_id, + idempotency_key=idempotency_key, + if_match=if_match, + response=response, + event_type="tenant_updated", + extra_fingerprint=changes, + mutate=lambda tenant, at: tenant.with_metadata(changes, at=at), + ) + + @app.post("/tenants/{tenant_id}/retire") + async def retire_tenant( + tenant_id: str, + payload: LifecycleRequest, + response: Response, + idempotency_key: str | None = Header(default=None, alias="Idempotency-Key"), + if_match: str | None = Header(default=None, alias="If-Match"), + ) -> dict: + return _lifecycle_mutation( + store=store, + authorizer=authorizer, + action="tenant.retire", + tenant_id=tenant_id, + actor=payload.actor, + reason=payload.reason, + correlation_id=payload.correlation_id, + idempotency_key=idempotency_key, + if_match=if_match, + response=response, + event_type="tenant_retired", + extra_fingerprint={}, + mutate=lambda tenant, at: tenant.retire(at=at), + ) + + @app.post("/tenants/{tenant_id}/reactivate") + async def reactivate_tenant( + tenant_id: str, + payload: LifecycleRequest, + response: Response, + idempotency_key: str | None = Header(default=None, alias="Idempotency-Key"), + if_match: str | None = Header(default=None, alias="If-Match"), + ) -> dict: + return _lifecycle_mutation( + store=store, + authorizer=authorizer, + action="tenant.reactivate", + tenant_id=tenant_id, + actor=payload.actor, + reason=payload.reason, + correlation_id=payload.correlation_id, + idempotency_key=idempotency_key, + if_match=if_match, + response=response, + event_type="tenant_reactivated", + extra_fingerprint={}, + mutate=lambda tenant, at: tenant.reactivate(at=at), + ) + return app +def _etag(version: int) -> str: + return f'"{version}"' + + +def _parse_if_match(if_match: str | None, correlation_id: str) -> int: + if if_match is None: + raise LifecycleError( + 428, "if_match_required", "If-Match header is required", correlation_id + ) + candidate = if_match.strip() + if candidate.startswith("W/"): + candidate = candidate[2:] + candidate = candidate.strip('"') + if candidate == "*" or not candidate.isdigit(): + # `*` would mean "whatever version is current", which is precisely + # the unconditional write this endpoint exists to prevent. + raise LifecycleError( + 400, "invalid_if_match", "If-Match must be a record version ETag", correlation_id + ) + return int(candidate) + + +def _tenant_response(tenant: Tenant) -> dict: + return { + "tenant_id": tenant.tenant_id, + "identifier": tenant.identifier, + "grouping": tenant.grouping, + "display_name": tenant.display_name, + "contact_email": tenant.contact_email, + "lifecycle": tenant.lifecycle.value, + "version": tenant.version, + "created_at": _iso(tenant.created_at), + "updated_at": _iso(tenant.updated_at), + "retired_at": _iso(tenant.retired_at), + "reactivated_at": _iso(tenant.reactivated_at), + } + + +def _iso(value: datetime | None) -> str | None: + return value.isoformat() if value else None + + +def _lifecycle_mutation( + *, + store: TenantStore, + authorizer: WriteAuthorizer, + action: str, + tenant_id: str, + actor: str, + reason: str, + correlation_id: str, + idempotency_key: str | None, + if_match: str | None, + response: Response, + event_type: str, + extra_fingerprint: dict, + mutate, +) -> dict: + """Shared spine for update/retire/reactivate: authorize, then CAS. + + Authorization runs before the store is touched at all, so an unauthorized + caller cannot use timing or error codes to probe which tenants exist. + """ + if idempotency_key is None: + raise LifecycleError( + 400, "idempotency_key_required", "Idempotency-Key header is required", correlation_id + ) + expected_version = _parse_if_match(if_match, correlation_id) + + authorizer.authorize(action=action, tenant_id=tenant_id, actor=actor) + + fingerprint = hashlib.sha256( + json.dumps( + { + "action": action, + "tenant": tenant_id, + "version": expected_version, + **extra_fingerprint, + }, + sort_keys=True, + default=str, + ).encode() + ).hexdigest() + + now = datetime.now(UTC) + try: + tenant, replayed = store.mutate_tenant( + tenant_id=tenant_id, + expected_version=expected_version, + mutate=lambda current: mutate(current, now), + event_type=event_type, + evidence={"actor": actor, "reason": reason, "correlation_id": correlation_id}, + idempotency_key=idempotency_key, + request_fingerprint=fingerprint, + ) + except TenantNotFoundError as exc: + raise LifecycleError(404, "tenant_not_found", "tenant_not_found", correlation_id) from exc + except IdempotencyConflictError as exc: + raise LifecycleError( + 409, + "idempotency_key_conflict", + "Idempotency-Key was reused for a different request", + correlation_id, + ) from exc + except VersionConflictError as exc: + raise LifecycleError( + 409, + "version_conflict", + f"record version is {exc.actual}, not {exc.expected}", + correlation_id, + ) from exc + except (InvalidLifecycleTransitionError, TenantRetiredError) as exc: + raise LifecycleError(409, "invalid_lifecycle_transition", str(exc), correlation_id) from exc + except (ImmutableFieldError, EmptyUpdateError) as exc: + raise LifecycleError(400, "invalid_update", str(exc), correlation_id) from exc + except StoreUnavailableError as exc: + raise LifecycleError( + 503, "tenant_authority_unavailable", "tenant_authority_unavailable", correlation_id + ) from exc + + response.headers["ETag"] = _etag(tenant.version) + response.headers["Idempotent-Replay"] = "true" if replayed else "false" + return _tenant_response(tenant) + + def _build_authorizer(settings: Settings) -> WriteAuthorizer: if settings.flex_auth_base_url is None: return DefaultDenyWriteAuthorizer() diff --git a/src/tenant_engine/authz.py b/src/tenant_engine/authz.py index 1d1c7fd..a92ca4a 100644 --- a/src/tenant_engine/authz.py +++ b/src/tenant_engine/authz.py @@ -12,6 +12,11 @@ _RESOURCE_TYPES: dict[str, str] = { "tenant.role.grant": "role-grant", "tenant.role.revoke": "role-grant", "tenant.plan.assign": "plan-assignment", + # TEN-WP-0005: lifecycle actions are distinct so policy can separate a + # metadata edit from a retirement. + "tenant.update": "tenant", + "tenant.retire": "tenant", + "tenant.reactivate": "tenant", } diff --git a/src/tenant_engine/domain.py b/src/tenant_engine/domain.py index ed69e37..9772627 100644 --- a/src/tenant_engine/domain.py +++ b/src/tenant_engine/domain.py @@ -38,6 +38,41 @@ class InvalidGrantError(ValueError): """A role grant violates a domain invariant (ADR-0014).""" +class InvalidLifecycleTransitionError(ValueError): + """A lifecycle transition is not legal from the tenant's current state.""" + + +class ImmutableFieldError(ValueError): + """An update tried to change a field that is immutable by contract.""" + + +class EmptyUpdateError(ValueError): + """An update carried no allow-listed field changes.""" + + +class TenantRetiredError(ValueError): + """A mutation was attempted on a retired tenant that only active tenants allow.""" + + +class TenantLifecycle(str, Enum): + """TEN-WP-0005: tenant existence is reversible, never hard-deleted. + + Retirement suspends a tenant's ability to take on new capability or plan + state; it deliberately preserves the tenant record, its grant history, + and its plan history so audit correlation and recovery stay intact. + """ + + ACTIVE = "active" + RETIRED = "retired" + + +# The only tenant fields a PATCH may change. tenant_id, identifier, and +# grouping are immutable: the identifier is the IAM Profile `tenant` claim +# value that key-cape mints into tokens and flex-auth authorizes against, so +# mutating it would silently invalidate every issued token referencing it. +MUTABLE_METADATA_FIELDS = frozenset({"display_name", "contact_email"}) + + class CapabilityRole(str, Enum): """ADR-0014: non-exclusive capability roles a tenant may hold.""" @@ -73,16 +108,99 @@ class Tenant: tenant_id: str identifier: str grouping: str | None + # -- TEN-WP-0005 lifecycle and mutable metadata. All default so that + # pre-lifecycle construction sites (and migrated rows) keep working. + display_name: str | None = None + contact_email: str | None = None + lifecycle: TenantLifecycle = TenantLifecycle.ACTIVE + version: int = 1 + created_at: datetime | None = None + updated_at: datetime | None = None + retired_at: datetime | None = None + reactivated_at: datetime | None = None @classmethod - def create(cls, *, tenant_id: str, identifier: str) -> "Tenant": + def create( + cls, + *, + tenant_id: str, + identifier: str, + display_name: str | None = None, + contact_email: str | None = None, + created_at: datetime | None = None, + ) -> "Tenant": grouping, _name = parse_tenant_identifier(identifier) - return cls(tenant_id=tenant_id, identifier=identifier, grouping=grouping) + return cls( + tenant_id=tenant_id, + identifier=identifier, + grouping=grouping, + display_name=display_name, + contact_email=contact_email, + created_at=created_at, + updated_at=created_at, + ) @property def is_reserved(self) -> bool: return self.grouping is None + @property + def is_active(self) -> bool: + return self.lifecycle is TenantLifecycle.ACTIVE + + def with_metadata(self, changes: dict[str, object], *, at: datetime) -> "Tenant": + """Apply an allow-listed metadata change, bumping the record version. + + Fails closed on anything ambiguous: unknown fields, attempts to change + an immutable field, an empty change set, or a no-op change set. A no-op + is rejected rather than silently accepted so a caller never reads a + version bump as evidence that a value actually changed. + """ + if self.lifecycle is not TenantLifecycle.ACTIVE: + raise InvalidLifecycleTransitionError( + "metadata of a retired tenant cannot be updated; reactivate first" + ) + + unknown = set(changes) - MUTABLE_METADATA_FIELDS + immutable = unknown & {"tenant_id", "identifier", "grouping", "version", "lifecycle"} + if immutable: + raise ImmutableFieldError(f"immutable field(s): {', '.join(sorted(immutable))}") + if unknown: + raise ImmutableFieldError(f"unknown field(s): {', '.join(sorted(unknown))}") + if not changes: + raise EmptyUpdateError("update carried no fields") + if all(getattr(self, field) == value for field, value in changes.items()): + raise EmptyUpdateError("update would not change any field") + + return replace(self, version=self.version + 1, updated_at=at, **changes) # type: ignore[arg-type] + + def retire(self, *, at: datetime) -> "Tenant": + if self.lifecycle is TenantLifecycle.RETIRED: + raise InvalidLifecycleTransitionError("tenant is already retired") + return replace( + self, + lifecycle=TenantLifecycle.RETIRED, + version=self.version + 1, + updated_at=at, + retired_at=at, + ) + + def reactivate(self, *, at: datetime) -> "Tenant": + """Return the tenant to active. Deliberately narrow: it restores the + tenant's ability to receive new grants and plan changes, and does not + resurrect revoked grants or invent plan state -- those stay exactly as + retirement left them. + """ + if self.lifecycle is TenantLifecycle.ACTIVE: + raise InvalidLifecycleTransitionError("tenant is already active") + return replace( + self, + lifecycle=TenantLifecycle.ACTIVE, + version=self.version + 1, + updated_at=at, + reactivated_at=at, + ) + @dataclass(frozen=True, slots=True) class RoleGrant: diff --git a/src/tenant_engine/sqlite_store.py b/src/tenant_engine/sqlite_store.py index 5b81708..072011d 100644 --- a/src/tenant_engine/sqlite_store.py +++ b/src/tenant_engine/sqlite_store.py @@ -2,15 +2,26 @@ from __future__ import annotations import json import sqlite3 +from collections.abc import Callable from datetime import datetime from threading import RLock +from typing import Any -from tenant_engine.domain import CapabilityRole, PlanAssignment, RoleGrant, Tenant +from tenant_engine.domain import ( + CapabilityRole, + PlanAssignment, + RoleGrant, + Tenant, + TenantLifecycle, + TenantRetiredError, +) from tenant_engine.store import ( DomainEvent, GrantNotFoundError, + IdempotencyConflictError, TenantAlreadyExistsError, TenantNotFoundError, + VersionConflictError, ) @@ -40,14 +51,50 @@ class SQLiteTenantStore: seq INTEGER PRIMARY KEY AUTOINCREMENT, event_type TEXT NOT NULL, tenant_id TEXT NOT NULL, at TEXT NOT NULL, payload TEXT NOT NULL ); + CREATE TABLE IF NOT EXISTS idempotency_receipts ( + tenant_id TEXT NOT NULL, idempotency_key TEXT NOT NULL, + request_fingerprint TEXT NOT NULL, result TEXT NOT NULL, + recorded_at TEXT NOT NULL, + PRIMARY KEY (tenant_id, idempotency_key) + ); """) + self._migrate_tenant_lifecycle() + + def _migrate_tenant_lifecycle(self) -> None: + """TEN-WP-0005-T02: forward-only, idempotent lifecycle migration. + + Existing rows default to `active` at version 1 with their identifiers, + grants, and plan assignments untouched -- nothing is rewritten, only + columns are added. Fresh and pre-existing databases take this same + path, so there is no second schema definition to drift out of sync. + """ + existing = {row["name"] for row in self._db.execute("PRAGMA table_info(tenants)")} + additions = { + "display_name": "TEXT", + "contact_email": "TEXT", + "lifecycle": "TEXT NOT NULL DEFAULT 'active'", + "version": "INTEGER NOT NULL DEFAULT 1", + "created_at": "TEXT", + "updated_at": "TEXT", + "retired_at": "TEXT", + "reactivated_at": "TEXT", + } + for column, spec in additions.items(): + if column not in existing: + self._db.execute(f"ALTER TABLE tenants ADD COLUMN {column} {spec}") def create_tenant(self, tenant: Tenant) -> None: with self._lock, self._db: try: self._db.execute( - "INSERT INTO tenants VALUES (?, ?, ?)", - (tenant.tenant_id, tenant.identifier, tenant.grouping), + """INSERT INTO tenants (tenant_id, identifier, grouping_name, display_name, + contact_email, lifecycle, version, created_at, updated_at, + retired_at, reactivated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""", + (tenant.tenant_id, tenant.identifier, tenant.grouping, tenant.display_name, + tenant.contact_email, tenant.lifecycle.value, tenant.version, + _iso(tenant.created_at), _iso(tenant.updated_at), + _iso(tenant.retired_at), _iso(tenant.reactivated_at)), ) except sqlite3.IntegrityError as exc: raise TenantAlreadyExistsError(tenant.identifier) from exc @@ -56,15 +103,83 @@ class SQLiteTenantStore: }) def get_tenant(self, tenant_id: str) -> Tenant: - row = self._db.execute( - "SELECT * FROM tenants WHERE tenant_id = ? OR identifier = ?", (tenant_id, tenant_id) - ).fetchone() + # Reads take the same lock as writes. One sqlite3 connection is shared + # across request threads (check_same_thread=False), so an unguarded + # SELECT can execute while another thread sits inside BEGIN IMMEDIATE + # and observe a row that is not there yet -- caught by the concurrent + # writer test, which failed with a spurious tenant_not_found. + with self._lock: + row = self._db.execute( + "SELECT * FROM tenants WHERE tenant_id = ? OR identifier = ?", + (tenant_id, tenant_id), + ).fetchone() if row is None: raise TenantNotFoundError(tenant_id) - return Tenant(row["tenant_id"], row["identifier"], row["grouping_name"]) + return _tenant(row) + + def mutate_tenant( + self, + *, + tenant_id: str, + expected_version: int, + mutate: Callable[[Tenant], Tenant], + event_type: str, + evidence: dict[str, Any], + idempotency_key: str, + request_fingerprint: str, + ) -> tuple[Tenant, bool]: + tenant = self.get_tenant(tenant_id) + with self._lock: + # BEGIN IMMEDIATE takes the write lock before the read below, so a + # concurrent writer cannot slip a version bump between the check + # and the swap. Receipt lookup precedes the version check: a real + # retry replays the original If-Match, which is stale by then. + self._db.execute("BEGIN IMMEDIATE") + try: + receipt = self._db.execute( + "SELECT request_fingerprint, result FROM idempotency_receipts " + "WHERE tenant_id = ? AND idempotency_key = ?", + (tenant.tenant_id, idempotency_key), + ).fetchone() + if receipt is not None: + if receipt["request_fingerprint"] != request_fingerprint: + raise IdempotencyConflictError(idempotency_key) + replayed = _tenant(json.loads(receipt["result"])) + self._db.rollback() # read-only path: release the write lock + return replayed, True + + current = _tenant( + self._db.execute( + "SELECT * FROM tenants WHERE tenant_id = ?", (tenant.tenant_id,) + ).fetchone() + ) + if current.version != expected_version: + raise VersionConflictError(expected=expected_version, actual=current.version) + + updated = mutate(current) + self._db.execute( + """UPDATE tenants SET display_name = ?, contact_email = ?, lifecycle = ?, + version = ?, updated_at = ?, retired_at = ?, reactivated_at = ? + WHERE tenant_id = ?""", + (updated.display_name, updated.contact_email, updated.lifecycle.value, + updated.version, _iso(updated.updated_at), _iso(updated.retired_at), + _iso(updated.reactivated_at), updated.tenant_id), + ) + self._db.execute( + "INSERT INTO idempotency_receipts VALUES (?, ?, ?, ?, ?)", + (updated.tenant_id, idempotency_key, request_fingerprint, + json.dumps(_row(updated)), datetime.now().astimezone().isoformat()), + ) + self._emit(event_type, updated.tenant_id, {**evidence, "version": updated.version}) + except BaseException: + self._db.rollback() + raise + self._db.commit() + return updated, False def grant_role(self, grant: RoleGrant) -> None: tenant = self.get_tenant(grant.tenant_id) + self._require_active(tenant, "grant a role") with self._lock, self._db: self._db.execute( "INSERT OR REPLACE INTO grants VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)", @@ -78,10 +193,11 @@ class SQLiteTenantStore: def revoke_role(self, *, tenant_id: str, grant_id: str, at: datetime) -> RoleGrant: tenant = self.get_tenant(tenant_id) - row = self._db.execute( - "SELECT * FROM grants WHERE tenant_id = ? AND grant_id = ?", - (tenant.tenant_id, grant_id), - ).fetchone() + with self._lock: + row = self._db.execute( + "SELECT * FROM grants WHERE tenant_id = ? AND grant_id = ?", + (tenant.tenant_id, grant_id), + ).fetchone() if row is None: raise GrantNotFoundError(grant_id) grant = self._grant(row).revoke(at=at) @@ -94,14 +210,16 @@ class SQLiteTenantStore: def active_roles(self, tenant_id: str) -> frozenset[CapabilityRole]: tenant = self.get_tenant(tenant_id) - rows = self._db.execute( - "SELECT role FROM grants WHERE tenant_id = ? AND revoked_at IS NULL", - (tenant.tenant_id,), - ).fetchall() + with self._lock: + rows = self._db.execute( + "SELECT role FROM grants WHERE tenant_id = ? AND revoked_at IS NULL", + (tenant.tenant_id,), + ).fetchall() return frozenset(CapabilityRole(row["role"]) for row in rows) def assign_plan(self, assignment: PlanAssignment) -> None: tenant = self.get_tenant(assignment.tenant_id) + self._require_active(tenant, "assign a plan") with self._lock, self._db: self._db.execute( "INSERT OR REPLACE INTO plans VALUES (?, ?, ?)", @@ -110,9 +228,16 @@ class SQLiteTenantStore: self._emit("plan_assigned", tenant.tenant_id, {"plan_id": assignment.plan_id}) def events(self) -> list[DomainEvent]: + with self._lock: + rows = self._db.execute("SELECT * FROM events ORDER BY seq").fetchall() return [DomainEvent(row["event_type"], row["tenant_id"], datetime.fromisoformat(row["at"]), json.loads(row["payload"])) - for row in self._db.execute("SELECT * FROM events ORDER BY seq")] + for row in rows] + + @staticmethod + def _require_active(tenant: Tenant, what: str) -> None: + if tenant.lifecycle is not TenantLifecycle.ACTIVE: + raise TenantRetiredError(f"cannot {what} on a retired tenant") def _emit(self, event_type: str, tenant_id: str, payload: dict) -> None: now = datetime.now().astimezone() @@ -125,3 +250,45 @@ class SQLiteTenantStore: row["grant_reason"], row["plan_id"], row["granted_by"], datetime.fromisoformat(row["granted_at"]), row["correlation_id"], datetime.fromisoformat(row["revoked_at"]) if row["revoked_at"] else None) + + +def _iso(value: datetime | None) -> str | None: + return value.isoformat() if value else None + + +def _dt(value: str | None) -> datetime | None: + return datetime.fromisoformat(value) if value else None + + +def _tenant(row: sqlite3.Row | dict) -> Tenant: + """Map a tenants row -- or a receipt's JSON snapshot -- to a Tenant.""" + return Tenant( + tenant_id=row["tenant_id"], + identifier=row["identifier"], + grouping=row["grouping_name"], + display_name=row["display_name"], + contact_email=row["contact_email"], + lifecycle=TenantLifecycle(row["lifecycle"]), + version=row["version"], + created_at=_dt(row["created_at"]), + updated_at=_dt(row["updated_at"]), + retired_at=_dt(row["retired_at"]), + reactivated_at=_dt(row["reactivated_at"]), + ) + + +def _row(tenant: Tenant) -> dict: + """Inverse of `_tenant`, for durable idempotency receipts.""" + return { + "tenant_id": tenant.tenant_id, + "identifier": tenant.identifier, + "grouping_name": tenant.grouping, + "display_name": tenant.display_name, + "contact_email": tenant.contact_email, + "lifecycle": tenant.lifecycle.value, + "version": tenant.version, + "created_at": _iso(tenant.created_at), + "updated_at": _iso(tenant.updated_at), + "retired_at": _iso(tenant.retired_at), + "reactivated_at": _iso(tenant.reactivated_at), + } diff --git a/src/tenant_engine/store.py b/src/tenant_engine/store.py index 198c241..e470efe 100644 --- a/src/tenant_engine/store.py +++ b/src/tenant_engine/store.py @@ -1,16 +1,37 @@ from __future__ import annotations +from collections.abc import Callable from dataclasses import dataclass from datetime import UTC, datetime from typing import Any, Protocol -from tenant_engine.domain import CapabilityRole, PlanAssignment, RoleGrant, Tenant +from tenant_engine.domain import ( + CapabilityRole, + PlanAssignment, + RoleGrant, + Tenant, + TenantLifecycle, + TenantRetiredError, +) class TenantNotFoundError(KeyError): pass +class VersionConflictError(RuntimeError): + """The caller's `If-Match` version is not the tenant's current version.""" + + def __init__(self, *, expected: int, actual: int) -> None: + super().__init__(f"expected version {expected}, current version is {actual}") + self.expected = expected + self.actual = actual + + +class IdempotencyConflictError(RuntimeError): + """An `Idempotency-Key` was reused for a materially different request.""" + + class TenantAlreadyExistsError(ValueError): pass @@ -63,6 +84,31 @@ class TenantStore(Protocol): def events(self) -> list[DomainEvent]: ... + def mutate_tenant( + self, + *, + tenant_id: str, + expected_version: int, + mutate: Callable[[Tenant], Tenant], + event_type: str, + evidence: dict[str, Any], + idempotency_key: str, + request_fingerprint: str, + ) -> tuple[Tenant, bool]: + """Atomically compare-and-swap a tenant record. Returns (tenant, replayed). + + One method carries all four concerns -- idempotency replay, version + CAS, the mutation itself, and the audit event -- because they have to + commit or fail together. Splitting them across store calls would leave + a window where a crash yields a bumped version with no receipt (a + retry then double-applies) or a receipt with no mutation. + + Order matters: a replayed `idempotency_key` short-circuits *before* + the version check, because a genuine retry of an already-applied + mutation necessarily carries a now-stale `If-Match`. + """ + ... + class InMemoryTenantStore: def __init__(self) -> None: @@ -71,6 +117,8 @@ class InMemoryTenantStore: self._grants: dict[str, dict[str, RoleGrant]] = {} self._plans: dict[str, PlanAssignment] = {} self._events: list[DomainEvent] = [] + # (tenant_id, idempotency_key) -> (request_fingerprint, result snapshot) + self._receipts: dict[tuple[str, str], tuple[str, Tenant]] = {} def create_tenant(self, tenant: Tenant) -> None: if tenant.tenant_id in self._tenants: @@ -91,6 +139,7 @@ class InMemoryTenantStore: def grant_role(self, grant: RoleGrant) -> None: resolved = self._resolve(grant.tenant_id) + self._require_active(resolved, "grant a role") self._grants[resolved][grant.grant_id] = grant self._emit( "role_granted", @@ -126,12 +175,48 @@ class InMemoryTenantStore: def assign_plan(self, assignment: PlanAssignment) -> None: resolved = self._resolve(assignment.tenant_id) + self._require_active(resolved, "assign a plan") self._plans[resolved] = assignment self._emit("plan_assigned", resolved, {"plan_id": assignment.plan_id}) def events(self) -> list[DomainEvent]: return list(self._events) + def mutate_tenant( + self, + *, + tenant_id: str, + expected_version: int, + mutate: Callable[[Tenant], Tenant], + event_type: str, + evidence: dict[str, Any], + idempotency_key: str, + request_fingerprint: str, + ) -> tuple[Tenant, bool]: + resolved = self._resolve(tenant_id) + + receipt = self._receipts.get((resolved, idempotency_key)) + if receipt is not None: + fingerprint, snapshot = receipt + if fingerprint != request_fingerprint: + raise IdempotencyConflictError(idempotency_key) + return snapshot, True + + current = self._tenants[resolved] + if current.version != expected_version: + raise VersionConflictError(expected=expected_version, actual=current.version) + + updated = mutate(current) + self._tenants[resolved] = updated + self._receipts[(resolved, idempotency_key)] = (request_fingerprint, updated) + self._emit(event_type, resolved, {**evidence, "version": updated.version}) + return updated, False + + def _require_active(self, resolved_id: str, what: str) -> None: + tenant = self._tenants[resolved_id] + if tenant.lifecycle is not TenantLifecycle.ACTIVE: + raise TenantRetiredError(f"cannot {what} on a retired tenant") + def _resolve(self, tenant_id: str) -> str: """Resolve an internal tenant_id or a profile identifier to the diff --git a/tests/test_api_lifecycle.py b/tests/test_api_lifecycle.py new file mode 100644 index 0000000..92ce069 --- /dev/null +++ b/tests/test_api_lifecycle.py @@ -0,0 +1,303 @@ +"""TEN-WP-0005-T03/T04: HTTP contract for the lifecycle surface.""" + +import pytest +from fastapi.testclient import TestClient + +from tenant_engine.app import create_app +from tenant_engine.authz import WriteAuthorizationDeniedError, WriteAuthorizer +from tenant_engine.store import InMemoryTenantStore, StoreUnavailableError + +HEADERS = {"Idempotency-Key": "idem-1", "If-Match": '"1"'} +BODY = {"actor": "portal", "reason": "operator request", "correlation_id": "corr-1"} + + +class _AllowAllAuthorizer(WriteAuthorizer): + def authorize(self, *, action: str, tenant_id: str, actor: str) -> None: + return None + + +class _ScopedAuthorizer(WriteAuthorizer): + """Allows only the listed actions -- stands in for a flex-auth policy that + grants an operator metadata edits but not retirement.""" + + def __init__(self, *allowed: str) -> None: + self._allowed = set(allowed) + + def authorize(self, *, action: str, tenant_id: str, actor: str) -> None: + if action not in self._allowed: + raise WriteAuthorizationDeniedError(action, "not permitted") + + +class _BrokenStore(InMemoryTenantStore): + def mutate_tenant(self, **kwargs): + raise StoreUnavailableError("connection to /var/lib/tenant-engine/tenant.db refused") + + def get_tenant(self, tenant_id: str): + raise StoreUnavailableError("connection to /var/lib/tenant-engine/tenant.db refused") + + +@pytest.fixture +def client() -> TestClient: + app = create_app(store=InMemoryTenantStore(), authorizer=_AllowAllAuthorizer()) + test_client = TestClient(app) + test_client.post( + "/tenants", + json={ + "tenant_id": "t-1", + "identifier": "tenant:friendly:binky", + "actor": "ops", + "display_name": "Binky", + }, + ) + return test_client + + +def _patch(client, *, headers=None, metadata=None, **overrides): + return client.patch( + "/tenants/t-1", + headers={**HEADERS, **(headers or {})}, + json={ + **BODY, + "metadata": {"display_name": "Binky Ltd"} if metadata is None else metadata, + **overrides, + }, + ) + + +# -- read --------------------------------------------------------------- + + +def test_get_tenant_returns_record_and_etag(client) -> None: + response = client.get("/tenants/t-1") + + assert response.status_code == 200 + assert response.headers["ETag"] == '"1"' + body = response.json() + assert body["identifier"] == "tenant:friendly:binky" + assert body["lifecycle"] == "active" + assert body["version"] == 1 + + +def test_get_tenant_resolves_by_identifier(client) -> None: + response = client.get("/tenants/tenant:friendly:binky") + assert response.status_code == 200 + assert response.json()["tenant_id"] == "t-1" + + +def test_get_unknown_tenant_is_404(client) -> None: + assert client.get("/tenants/nope").status_code == 404 + + +# -- update ------------------------------------------------------------- + + +def test_update_succeeds_and_advances_the_etag(client) -> None: + response = _patch(client) + + assert response.status_code == 200 + assert response.headers["ETag"] == '"2"' + assert response.headers["Idempotent-Replay"] == "false" + assert response.json()["display_name"] == "Binky Ltd" + assert client.get("/tenants/t-1").json()["display_name"] == "Binky Ltd" + + +def test_update_rejects_unknown_field(client) -> None: + response = _patch(client, metadata={"nickname": "binks"}) + assert response.status_code == 422 # schema-level allow-list + + +def test_update_rejects_identifier_mutation(client) -> None: + response = _patch(client, metadata={"identifier": "tenant:large:other"}) + + assert response.status_code == 422 + assert client.get("/tenants/t-1").json()["identifier"] == "tenant:friendly:binky" + + +def test_update_rejects_empty_metadata(client) -> None: + response = _patch(client, metadata={}) + + assert response.status_code == 400 + assert response.json()["error_code"] == "invalid_update" + + +def test_update_with_stale_version_is_409(client) -> None: + _patch(client) + response = _patch( + client, headers={"Idempotency-Key": "idem-2"}, metadata={"display_name": "Third"} + ) + + assert response.status_code == 409 + assert response.json()["error_code"] == "version_conflict" + assert response.json()["correlation_id"] == "corr-1" + + +def test_duplicate_idempotency_key_replays(client) -> None: + first = _patch(client) + replay = _patch(client) + + assert replay.status_code == 200 + assert replay.headers["Idempotent-Replay"] == "true" + assert replay.json() == first.json() + assert client.get("/tenants/t-1").json()["version"] == 2 + + +def test_idempotency_key_reused_for_a_different_request_is_409(client) -> None: + _patch(client) + response = _patch(client, metadata={"display_name": "Different"}) + + assert response.status_code == 409 + assert response.json()["error_code"] == "idempotency_key_conflict" + + +def test_missing_idempotency_key_is_rejected(client) -> None: + response = client.patch( + "/tenants/t-1", + headers={"If-Match": '"1"'}, + json={**BODY, "metadata": {"display_name": "X"}}, + ) + assert response.status_code == 400 + assert response.json()["error_code"] == "idempotency_key_required" + + +def test_missing_if_match_is_rejected(client) -> None: + response = client.patch( + "/tenants/t-1", + headers={"Idempotency-Key": "idem-1"}, + json={**BODY, "metadata": {"display_name": "X"}}, + ) + assert response.status_code == 428 + assert response.json()["error_code"] == "if_match_required" + + +def test_wildcard_if_match_is_rejected(client) -> None: + response = _patch(client, headers={"If-Match": "*"}) + + assert response.status_code == 400 + assert response.json()["error_code"] == "invalid_if_match" + + +def test_weak_etag_form_is_accepted(client) -> None: + assert _patch(client, headers={"If-Match": 'W/"1"'}).status_code == 200 + + +# -- retire / reactivate ------------------------------------------------- + + +def _retire(client, *, key="idem-retire", version='"1"'): + return client.post( + "/tenants/t-1/retire", + headers={"Idempotency-Key": key, "If-Match": version}, + json=BODY, + ) + + +def test_retire_then_reactivate(client) -> None: + retired = _retire(client) + assert retired.status_code == 200 + assert retired.json()["lifecycle"] == "retired" + assert retired.json()["retired_at"] is not None + + reactivated = client.post( + "/tenants/t-1/reactivate", + headers={"Idempotency-Key": "idem-react", "If-Match": '"2"'}, + json=BODY, + ) + assert reactivated.status_code == 200 + assert reactivated.json()["lifecycle"] == "active" + assert reactivated.json()["version"] == 3 + + +def test_double_retirement_is_409(client) -> None: + _retire(client) + response = _retire(client, key="idem-retire-2", version='"2"') + + assert response.status_code == 409 + assert response.json()["error_code"] == "invalid_lifecycle_transition" + + +def test_retirement_replay_is_idempotent(client) -> None: + first = _retire(client) + replay = _retire(client) + + assert replay.status_code == 200 + assert replay.headers["Idempotent-Replay"] == "true" + assert replay.json() == first.json() + + +def test_update_after_retirement_is_denied(client) -> None: + _retire(client) + response = _patch(client, headers={"If-Match": '"2"', "Idempotency-Key": "idem-x"}) + + assert response.status_code == 409 + assert response.json()["error_code"] == "invalid_lifecycle_transition" + + +def test_role_and_plan_mutations_denied_while_retired(client) -> None: + _retire(client) + + 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", + }, + ) + plan = client.post("/tenants/t-1/plan", json={"plan_id": "plan-x", "actor": "ops"}) + + assert granted.status_code == 409 + assert plan.status_code == 409 + + +def test_lifecycle_mutation_on_unknown_tenant_is_404(client) -> None: + response = client.post( + "/tenants/nope/retire", headers={"Idempotency-Key": "k", "If-Match": '"1"'}, json=BODY + ) + assert response.status_code == 404 + + +# -- authorization and redaction ----------------------------------------- + + +def test_lifecycle_mutations_are_denied_by_default() -> None: + client = TestClient(create_app(store=InMemoryTenantStore())) + response = client.patch( + "/tenants/t-1", headers=HEADERS, json={**BODY, "metadata": {"display_name": "X"}} + ) + assert response.status_code == 403 + assert response.json()["error_code"] == "write_denied" + + +def test_update_permission_does_not_imply_retire_permission() -> None: + app = create_app( + store=InMemoryTenantStore(), + authorizer=_ScopedAuthorizer("tenant.create", "tenant.update"), + ) + client = TestClient(app) + client.post( + "/tenants", json={"tenant_id": "t-1", "identifier": "tenant:friendly:binky", "actor": "ops"} + ) + + assert _patch(client).status_code == 200 + denied = _retire(client, version='"2"') + assert denied.status_code == 403 + assert denied.json()["action"] == "tenant.retire" + + +def test_store_outage_is_a_redacted_503() -> None: + client = TestClient(create_app(store=_BrokenStore(), authorizer=_AllowAllAuthorizer())) + + read = client.get("/tenants/t-1") + write = client.patch( + "/tenants/t-1", headers=HEADERS, json={**BODY, "metadata": {"display_name": "X"}} + ) + + for response in (read, write): + assert response.status_code == 503 + assert response.json()["error_code"] == "tenant_authority_unavailable" + # No database path, driver text, or policy detail may reach a consumer. + assert "tenant.db" not in response.text + assert "refused" not in response.text diff --git a/tests/test_lifecycle_domain.py b/tests/test_lifecycle_domain.py new file mode 100644 index 0000000..8adc3a2 --- /dev/null +++ b/tests/test_lifecycle_domain.py @@ -0,0 +1,88 @@ +from datetime import UTC, datetime + +import pytest + +from tenant_engine.domain import ( + EmptyUpdateError, + ImmutableFieldError, + InvalidLifecycleTransitionError, + Tenant, + TenantLifecycle, +) + +NOW = datetime(2026, 8, 10, 12, 0, tzinfo=UTC) + + +def _tenant() -> Tenant: + return Tenant.create( + tenant_id="t-1", identifier="tenant:friendly:binky", display_name="Binky", created_at=NOW + ) + + +def test_new_tenant_is_active_at_version_one() -> None: + tenant = _tenant() + assert tenant.lifecycle is TenantLifecycle.ACTIVE + assert tenant.version == 1 + assert tenant.is_active + + +def test_metadata_update_bumps_version_and_keeps_identity() -> None: + tenant = _tenant().with_metadata({"display_name": "Binky Ltd"}, at=NOW) + + assert tenant.display_name == "Binky Ltd" + assert tenant.version == 2 + assert tenant.updated_at == NOW + assert tenant.tenant_id == "t-1" + assert tenant.identifier == "tenant:friendly:binky" + assert tenant.grouping == "friendly" + + +def test_update_rejects_identifier_mutation() -> None: + with pytest.raises(ImmutableFieldError): + _tenant().with_metadata({"identifier": "tenant:large:other"}, at=NOW) + + +def test_update_rejects_unknown_field() -> None: + with pytest.raises(ImmutableFieldError): + _tenant().with_metadata({"nickname": "binks"}, at=NOW) + + +def test_update_rejects_empty_change_set() -> None: + with pytest.raises(EmptyUpdateError): + _tenant().with_metadata({}, at=NOW) + + +def test_update_rejects_no_op_change() -> None: + with pytest.raises(EmptyUpdateError): + _tenant().with_metadata({"display_name": "Binky"}, at=NOW) + + +def test_retire_is_reversible_and_preserves_identity() -> None: + retired = _tenant().retire(at=NOW) + assert retired.lifecycle is TenantLifecycle.RETIRED + assert retired.retired_at == NOW + assert retired.version == 2 + assert retired.identifier == "tenant:friendly:binky" + + reactivated = retired.reactivate(at=NOW) + assert reactivated.lifecycle is TenantLifecycle.ACTIVE + assert reactivated.reactivated_at == NOW + assert reactivated.retired_at == NOW # retirement history is not erased + assert reactivated.version == 3 + + +def test_double_retirement_is_an_invalid_transition() -> None: + retired = _tenant().retire(at=NOW) + with pytest.raises(InvalidLifecycleTransitionError): + retired.retire(at=NOW) + + +def test_reactivating_an_active_tenant_is_an_invalid_transition() -> None: + with pytest.raises(InvalidLifecycleTransitionError): + _tenant().reactivate(at=NOW) + + +def test_metadata_update_denied_while_retired() -> None: + retired = _tenant().retire(at=NOW) + with pytest.raises(InvalidLifecycleTransitionError): + retired.with_metadata({"display_name": "Binky Ltd"}, at=NOW) diff --git a/tests/test_lifecycle_store_conformance.py b/tests/test_lifecycle_store_conformance.py new file mode 100644 index 0000000..8408583 --- /dev/null +++ b/tests/test_lifecycle_store_conformance.py @@ -0,0 +1,296 @@ +"""TEN-WP-0005-T02/T04: one lifecycle contract, both store backends. + +Every test here is parametrised over the in-memory and SQLite stores so the +durable backend cannot silently diverge from the reference semantics -- the +divergence that matters (CAS, idempotency, retirement guards) is exactly the +kind a single-backend suite would miss. +""" + +from datetime import UTC, datetime + +import pytest + +from tenant_engine.domain import ( + CapabilityRole, + InvalidLifecycleTransitionError, + PlanAssignment, + Tenant, + TenantLifecycle, + TenantRetiredError, + create_role_grant, +) +from tenant_engine.sqlite_store import SQLiteTenantStore +from tenant_engine.store import ( + IdempotencyConflictError, + InMemoryTenantStore, + TenantNotFoundError, + VersionConflictError, +) + +NOW = datetime(2026, 8, 10, 12, 0, tzinfo=UTC) + + +@pytest.fixture(params=["memory", "sqlite"]) +def store(request, tmp_path): + if request.param == "memory": + return InMemoryTenantStore() + return SQLiteTenantStore(str(tmp_path / "tenant.db")) + + +@pytest.fixture +def tenant(store) -> Tenant: + record = Tenant.create( + tenant_id="t-1", identifier="tenant:friendly:binky", display_name="Binky", created_at=NOW + ) + store.create_tenant(record) + return record + + +def _retire(store, *, key: str = "idem-retire", version: int = 1): + return store.mutate_tenant( + tenant_id="t-1", + expected_version=version, + mutate=lambda t: t.retire(at=NOW), + event_type="tenant_retired", + evidence={"actor": "ops", "reason": "offboarded", "correlation_id": "corr-1"}, + idempotency_key=key, + request_fingerprint="fp-retire", + ) + + +def _rename(store, *, key: str, version: int, name: str = "Binky Ltd", fingerprint: str = "fp-a"): + return store.mutate_tenant( + tenant_id="t-1", + expected_version=version, + mutate=lambda t: t.with_metadata({"display_name": name}, at=NOW), + event_type="tenant_updated", + evidence={"actor": "ops", "reason": "rename", "correlation_id": "corr-1"}, + idempotency_key=key, + request_fingerprint=fingerprint, + ) + + +def test_update_persists_and_bumps_version(store, tenant) -> None: + updated, replayed = _rename(store, key="k1", version=1) + + assert replayed is False + assert updated.version == 2 + assert store.get_tenant("t-1").display_name == "Binky Ltd" + + +def test_stale_version_is_rejected(store, tenant) -> None: + _rename(store, key="k1", version=1) + + with pytest.raises(VersionConflictError) as exc: + _rename(store, key="k2", version=1, name="Binky GmbH") + + assert exc.value.actual == 2 + assert store.get_tenant("t-1").display_name == "Binky Ltd" + + +def test_duplicate_idempotency_key_replays_the_original_result(store, tenant) -> None: + first, _ = _rename(store, key="k1", version=1) + replay, replayed = _rename(store, key="k1", version=1) + + assert replayed is True + assert replay == first + # The replay must not apply the mutation a second time. + assert store.get_tenant("t-1").version == 2 + + +def test_conflicting_idempotency_key_reuse_is_rejected(store, tenant) -> None: + _rename(store, key="k1", version=1, fingerprint="fp-a") + + with pytest.raises(IdempotencyConflictError): + _rename(store, key="k1", version=1, name="Something Else", fingerprint="fp-b") + + +def test_mutating_an_unknown_tenant_raises(store) -> None: + with pytest.raises(TenantNotFoundError): + _rename(store, key="k1", version=1) + + +def test_retire_then_reactivate_round_trip(store, tenant) -> None: + retired, _ = _retire(store) + assert retired.lifecycle is TenantLifecycle.RETIRED + assert store.get_tenant("t-1").lifecycle is TenantLifecycle.RETIRED + + reactivated, _ = store.mutate_tenant( + tenant_id="t-1", + expected_version=2, + mutate=lambda t: t.reactivate(at=NOW), + event_type="tenant_reactivated", + evidence={"actor": "ops", "reason": "returned", "correlation_id": "corr-2"}, + idempotency_key="idem-reactivate", + request_fingerprint="fp-reactivate", + ) + assert reactivated.lifecycle is TenantLifecycle.ACTIVE + assert store.get_tenant("t-1").version == 3 + + +def test_double_retirement_with_a_new_key_is_an_invalid_transition(store, tenant) -> None: + _retire(store) + + with pytest.raises(InvalidLifecycleTransitionError): + _retire(store, key="idem-retire-2", version=2) + + +def test_failed_mutation_leaves_no_version_bump_and_no_receipt(store, tenant) -> None: + with pytest.raises(InvalidLifecycleTransitionError): + store.mutate_tenant( + tenant_id="t-1", + expected_version=1, + mutate=lambda t: t.reactivate(at=NOW), # already active + event_type="tenant_reactivated", + evidence={"actor": "ops", "reason": "x", "correlation_id": "c"}, + idempotency_key="k-fail", + request_fingerprint="fp", + ) + + assert store.get_tenant("t-1").version == 1 + # The failed key must be reusable -- a rolled-back attempt is not a receipt. + updated, replayed = _rename(store, key="k-fail", version=1) + assert replayed is False + assert updated.version == 2 + + +def test_retired_tenant_refuses_new_grants_and_plan_changes(store, tenant) -> None: + _retire(store) + + grant = 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=NOW, + ) + with pytest.raises(TenantRetiredError): + store.grant_role(grant) + with pytest.raises(TenantRetiredError): + store.assign_plan(PlanAssignment(tenant_id="t-1", plan_id="plan-x", assigned_at=NOW)) + + +def test_retirement_preserves_existing_grant_and_plan_history(store, tenant) -> None: + 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=NOW, + ) + ) + store.assign_plan(PlanAssignment(tenant_id="t-1", plan_id="plan-x", assigned_at=NOW)) + + _retire(store) + + # Retirement is not a revocation: history stays queryable for audit and + # so reactivation does not have to reconstruct anything. + assert store.active_roles("t-1") == frozenset({CapabilityRole.CUS}) + assert any(event.event_type == "plan_assigned" for event in store.events()) + + +def test_mutation_emits_a_correlated_audit_event(store, tenant) -> None: + _rename(store, key="k1", version=1) + + event = [e for e in store.events() if e.event_type == "tenant_updated"][-1] + assert event.payload["actor"] == "ops" + assert event.payload["reason"] == "rename" + assert event.payload["correlation_id"] == "corr-1" + assert event.payload["version"] == 2 + + +def test_lifecycle_survives_reopening_the_database(tmp_path) -> None: + path = str(tmp_path / "tenant.db") + store = SQLiteTenantStore(path) + store.create_tenant( + Tenant.create(tenant_id="t-1", identifier="tenant:friendly:binky", created_at=NOW) + ) + _retire(store) + + reopened = SQLiteTenantStore(path) + assert reopened.get_tenant("t-1").lifecycle is TenantLifecycle.RETIRED + assert reopened.get_tenant("t-1").version == 2 + + # Restart-safe idempotency: the receipt outlives the process. + replay, replayed = _retire(reopened, version=1) + assert replayed is True + assert replay.version == 2 + + +def test_migration_defaults_pre_lifecycle_rows_to_active(tmp_path) -> None: + """A database written by the pre-TEN-WP-0005 schema must open and read.""" + import sqlite3 + + path = str(tmp_path / "legacy.db") + legacy = sqlite3.connect(path) + with legacy: + legacy.executescript(""" + CREATE TABLE tenants ( + tenant_id TEXT PRIMARY KEY, identifier TEXT UNIQUE NOT NULL, grouping_name TEXT + ); + CREATE TABLE grants ( + grant_id TEXT PRIMARY KEY, tenant_id TEXT NOT NULL, role TEXT NOT NULL, + grant_reason TEXT NOT NULL, plan_id TEXT, granted_by TEXT NOT NULL, + granted_at TEXT NOT NULL, correlation_id TEXT NOT NULL, revoked_at TEXT + ); + CREATE TABLE plans ( + tenant_id TEXT PRIMARY KEY, plan_id TEXT NOT NULL, assigned_at TEXT NOT NULL + ); + CREATE TABLE events ( + seq INTEGER PRIMARY KEY AUTOINCREMENT, event_type TEXT NOT NULL, + tenant_id TEXT NOT NULL, at TEXT NOT NULL, payload TEXT NOT NULL + ); + INSERT INTO tenants VALUES ('t-legacy', 'tenant:friendly:legacy', 'friendly'); + INSERT INTO grants VALUES ('g-legacy', 't-legacy', 'CUS', 'manual_grant', NULL, + 'ops', '2026-08-01T00:00:00+00:00', 'corr-legacy', NULL); + INSERT INTO plans VALUES ('t-legacy', 'plan-legacy', '2026-08-01T00:00:00+00:00'); + """) + legacy.close() + + migrated = SQLiteTenantStore(path) + tenant = migrated.get_tenant("t-legacy") + + assert tenant.lifecycle is TenantLifecycle.ACTIVE + assert tenant.version == 1 + assert tenant.identifier == "tenant:friendly:legacy" + assert migrated.active_roles("t-legacy") == frozenset({CapabilityRole.CUS}) + # And the migrated row is immediately mutable under the new contract. + updated, _ = migrated.mutate_tenant( + tenant_id="t-legacy", + expected_version=1, + mutate=lambda t: t.with_metadata({"display_name": "Legacy"}, at=NOW), + event_type="tenant_updated", + evidence={"actor": "ops", "reason": "backfill", "correlation_id": "c"}, + idempotency_key="k", + request_fingerprint="fp", + ) + assert updated.version == 2 + + +def test_concurrent_writers_only_one_wins(tmp_path) -> None: + from concurrent.futures import ThreadPoolExecutor + + store = SQLiteTenantStore(str(tmp_path / "tenant.db")) + store.create_tenant( + Tenant.create(tenant_id="t-1", identifier="tenant:friendly:binky", created_at=NOW) + ) + + def attempt(n: int): + try: + return _rename(store, key=f"k{n}", version=1, name=f"Name {n}", fingerprint=f"fp{n}")[0] + except VersionConflictError: + return None + + with ThreadPoolExecutor(max_workers=8) as pool: + results = list(pool.map(attempt, range(8))) + + winners = [r for r in results if r is not None] + assert len(winners) == 1 + assert store.get_tenant("t-1").version == 2 diff --git a/workplans/TEN-WP-0005-tenant-update-and-retirement-api.md b/workplans/TEN-WP-0005-tenant-update-and-retirement-api.md index 237ffdc..0525a99 100644 --- a/workplans/TEN-WP-0005-tenant-update-and-retirement-api.md +++ b/workplans/TEN-WP-0005-tenant-update-and-retirement-api.md @@ -4,7 +4,7 @@ type: workplan title: "Tenant metadata update and reversible retirement API" domain: infotech repo: tenant-engine -status: ready +status: active owner: codex topic_slug: tenant-lifecycle created: "2026-08-10" @@ -27,7 +27,7 @@ and audit correlation; it is not a hard-delete endpoint. ```task id: TEN-WP-0005-T01 -status: todo +status: done priority: high state_hub_task_id: "5b7d9022-4c0f-4bc3-9f63-b268836c8efc" ``` @@ -49,11 +49,18 @@ Document stable response/error schemas for user-engine and other consumers. Done when the OpenAPI contract makes concurrency, idempotency, authorization, and lifecycle semantics unambiguous without defining a hard-delete operation. +Done 2026-08-10: lifecycle (`active`/`retired`), allow-listed mutable metadata +(`display_name`, `contact_email`), record version, and lifecycle timestamps are +in the domain contract. All four routes exist; the allow-list is enforced by +the request schema (`extra: forbid`) so it is visible in the OpenAPI document. +Consumer contract written up in `docs/tenant-lifecycle-api.md`. No hard-delete +operation was defined. + ## T02 - Implement durable lifecycle state and migration ```task id: TEN-WP-0005-T02 -status: todo +status: done priority: high state_hub_task_id: "3596d8f2-06cf-4bf9-bdab-00cf48a25956" ``` @@ -68,11 +75,23 @@ result across process restarts. Done when in-memory and PostgreSQL conformance prove atomic compare-and-swap, restart-safe idempotency, and lossless migration of existing tenants. +Done 2026-08-10, against SQLite rather than PostgreSQL: this workplan was +drafted assuming Postgres, but TEN-WP-0004 shipped SQLite on a PVC as the +production store, so the migration and conformance target the store that +actually runs. Adding an unused Postgres path would have been dead code. +The `TenantStore` Protocol keeps the seam if the backend changes later. + +`mutate_tenant()` carries idempotency replay, version CAS, mutation, and audit +event in one commit -- splitting them would leave a window where a crash +yields a bumped version with no receipt (a retry then double-applies). The +migration is forward-only and idempotent: existing rows default to `active` at +version 1 with identifiers, grants, and plans untouched. + ## T03 - Implement authorized update and lifecycle endpoints ```task id: TEN-WP-0005-T03 -status: todo +status: done priority: high state_hub_task_id: "083bcd28-47f4-4337-9bbe-e2f9f67cc2d1" ``` @@ -91,11 +110,19 @@ grants or invent plan state. Done when all lifecycle mutations are authorized, versioned, idempotent, correlated, and provider-neutral. +Done 2026-08-10: `tenant.update`, `tenant.retire`, and `tenant.reactivate` are +distinct flex-auth actions, so policy can permit a metadata edit without +permitting a retirement. Authorization runs before the store is touched, so an +unauthorized caller cannot probe which tenants exist. 503s are redacted. + +One deviation, deliberate: role *revocation* stays available while retired -- +it only reduces privilege, and blocking it would be fail-open. + ## T04 - Add lifecycle security and compatibility conformance ```task id: TEN-WP-0005-T04 -status: todo +status: done priority: high state_hub_task_id: "b716d7bf-ef6b-4b1d-bac3-e3f716d1a0b8" ``` @@ -111,6 +138,18 @@ compatible. Done when unit, API, store-conformance, PostgreSQL, and flex-auth tests pass and the existing API behavior has no unplanned breaking change. +Done 2026-08-10: 124 tests pass (was 66). The store-conformance suite is +parametrised over both backends so the durable store cannot silently diverge +from the reference semantics. All 66 pre-existing tests still pass unchanged; +`POST /tenants` returns a superset of its previous body. + +The concurrent-writer test caught a real pre-existing defect: reads on the +shared SQLite connection ran unguarded and could observe a row mid-transaction +from another thread, producing a spurious `tenant_not_found`. Reads now take +the same lock as writes. + +Not covered: no PostgreSQL tests exist, per the T02 note above. + ## T05 - Integrate and verify the production authority ```task @@ -131,3 +170,10 @@ Done when production evidence confirms durable lifecycle behavior and the consumer handoff names the immutable image, API version, and authorization policy revision. +Status 2026-08-10: still open, and it is the only thing between user-engine and +USER-WP-0021. Blocked on work outside this repo: the three flex-auth actions +must be added to the policy package (until then every lifecycle check +correctly resolves to deny), and image build plus rollout need cluster access. +The consumer-facing contract is finalized and ready to hand over: +`docs/tenant-lifecycle-api.md`. +