From c1afef9686187849e19f1df919667c4dba221d76 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Mon, 26 Oct 2026 00:15:33 -0400 Subject: [PATCH 1/5] Add scoped email login to the external gateway --- .github/workflows/ci.yml | 4 +- AGENTS.md | 5 +- README.md | 6 +- docs/ARCHITECTURE.md | 28 +- docs/DEVELOPMENT.md | 8 +- docs/RAILWAY-DEPLOYMENT.md | 6 +- packages/auth/README.md | 5 +- services/gateway/.env.example | 7 +- services/gateway/README.md | 50 ++- services/gateway/contracts/directory.py | 2 + services/gateway/contracts/login.py | 77 ++++ services/gateway/contracts/storage.py | 33 ++ services/gateway/docs/API.md | 112 +++++- services/gateway/docs/ARCHITECTURE.md | 64 +++- services/gateway/docs/CHANGE-AUDIT.md | 67 ++++ services/gateway/docs/CONTRIBUTING.md | 8 +- services/gateway/docs/DEPLOYMENT.md | 64 +++- .../migrations/versions/002_email_login.py | 43 +++ services/gateway/src/api/app.py | 25 +- services/gateway/src/api/deps.py | 19 + services/gateway/src/api/middleware.py | 46 +++ .../gateway/src/api/routers/email_login.py | 132 +++++++ services/gateway/src/cli.py | 7 +- services/gateway/src/config.py | 9 + services/gateway/src/directory/http_client.py | 6 + services/gateway/src/login_policy.py | 45 +++ services/gateway/src/storage/in_memory.py | 89 ++++- services/gateway/src/storage/postgres.py | 140 ++++++- services/gateway/src/storage/schema.py | 24 +- services/gateway/src/verification/__init__.py | 0 .../gateway/src/verification/http_client.py | 43 +++ services/gateway/tests/login_storage_cases.py | 108 ++++++ services/gateway/tests/smoke_stack.py | 129 ++++++- services/gateway/tests/test_config.py | 12 + services/gateway/tests/test_email_login.py | 342 ++++++++++++++++++ services/gateway/tests/test_login_storage.py | 13 + .../gateway/tests/test_postgres_adapter.py | 17 + services/gateway/tests/test_verification.py | 71 ++++ services/verification/.env.example | 2 + services/verification/README.md | 30 +- services/verification/docs/API.md | 10 +- services/verification/docs/ARCHITECTURE.md | 18 +- services/verification/docs/DEPLOYMENT.md | 31 +- services/verification/src/api/app.py | 2 + services/verification/src/api/deps.py | 12 +- services/verification/src/config.py | 1 + .../verification/src/storage/null_keys.py | 22 -- services/verification/tests/conftest.py | 3 + .../verification/tests/test_consumer_auth.py | 40 ++ 49 files changed, 1901 insertions(+), 136 deletions(-) create mode 100644 services/gateway/contracts/login.py create mode 100644 services/gateway/migrations/versions/002_email_login.py create mode 100644 services/gateway/src/api/routers/email_login.py create mode 100644 services/gateway/src/login_policy.py create mode 100644 services/gateway/src/verification/__init__.py create mode 100644 services/gateway/src/verification/http_client.py create mode 100644 services/gateway/tests/login_storage_cases.py create mode 100644 services/gateway/tests/test_email_login.py create mode 100644 services/gateway/tests/test_login_storage.py create mode 100644 services/gateway/tests/test_verification.py delete mode 100644 services/verification/src/storage/null_keys.py create mode 100644 services/verification/tests/test_consumer_auth.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 77bdc8e5..4330476b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -327,8 +327,6 @@ jobs: run: docker build -f services/gateway/Dockerfile -t gateway:ci . - name: Smoke-test gateway image (imports incl. contracts/ resolve at boot) run: docker run --rm gateway:ci python -c "import src.api.app" - - name: Verify gateway against the current directory over HTTP - run: python3 services/gateway/tests/smoke_stack.py - name: Build discord-bot image run: docker build -t discord-bot:ci ./discord-bot - name: Smoke-test discord-bot image (source parses) @@ -337,6 +335,8 @@ jobs: run: docker build -f services/verification/Dockerfile -t verification:ci . - name: Smoke-test verification image (imports incl. contracts/ resolve at boot) run: docker run --rm verification:ci python -c "import src.api.app" + - name: Verify gateway against directory and verification over HTTP + run: python3 services/gateway/tests/smoke_stack.py - name: Build meeting image run: docker build -f services/meeting/Dockerfile -t meeting:ci . - name: Smoke-test meeting image (imports incl. PyAV/ffmpeg resolve at boot) diff --git a/AGENTS.md b/AGENTS.md index 27431524..d7d7eeea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,7 +59,7 @@ Violating any of these is a bug even if tests pass. - **`down_revision` points at the current head**, and `downgrade()` must actually reverse `upgrade()` except for the explicit retirement exceptions below. Verify the round trip: `upgrade head` → `downgrade -1` → `upgrade head`. - **Retirement exceptions:** documentation-system [009](services/documentation-system/docs/DEPLOYMENT.md#registry-cleanup-migration-009) and team-tracking [011](services/team-tracking/docs/DEPLOYMENT.md#registry-cleanup-migration-011) intentionally preserve their data cleanup on downgrade; only the registry entries are restored. Follow the linked backup/recovery guidance and verify the round trip with existing rows, checking that unrelated data survives. These exceptions do not waive reversibility for other migrations. - **New columns on existing tables are nullable or have a `server_default`**, mirrored in the Pydantic model and both adapters. -- Current heads: team-tracking **011**, documentation-system **018**, verification **001**, gateway **001**. `llm`, `meeting`, `connectors` have no schema. (Verify before relying on these — `ls services//migrations/versions/` is authoritative, this line is not.) +- Current heads: team-tracking **011**, documentation-system **018**, verification **001**, gateway **002**. `llm`, `meeting`, `connectors` have no schema. (Verify before relying on these — `ls services//migrations/versions/` is authoritative, this line is not.) ### Configuration @@ -115,7 +115,8 @@ Run what CI runs for that service — `.github/workflows/ci.yml` is authoritativ - **Cross-worktree tests need source-path verification.** Pytest can prepend a historical tests checkout to `sys.path` and import its `contracts` instead of the edited code. Pin the target `src` and `contracts` packages before collection and verify imported modules' `__file__` paths; passing against the wrong checkout is not verification. - **`uv --project` is mandatory for the key CLIs.** Both `services/team-tracking` and `services/documentation-system` declare a top-level `src` package with a console script at `src.cli:main`. In the shared workspace venv they collide, so a bare `team-tracking-keys …` can resolve documentation-system's CLI and mint a `doc_`-envelope key that team-tracking rejects. Always `uv --project services/ run -keys …`, and verify the token's prefix. The same applies to every other team-tracking console script (`team-tracking-seed`, `team-tracking-groups`). - **Gateway provisioning must pin the import path too.** After installing all workspace members, `uv --project` alone can still import another service's `src.cli`. From the repo root use `uv --directory services/gateway --project . run python -m src.cli ...` (and the team-tracking directory for its internal key). The isolated containers can use their installed key CLI directly. -- **Gateway is the public entry point.** It accepts only issued `gw_` keys, has no env-bootstrap `API_KEY`, and owns only its key database. Compute-platform data access must use explicit scoped HTTP operations; see `services/gateway/docs/ARCHITECTURE.md`. Run one replica/worker for its process-local rate limits. +- **Gateway is the public entry point.** It accepts only issued `gw_` keys, has no env-bootstrap `API_KEY`, and owns its key registry plus durable login challenges/abuse counters. Compute-platform data access must use explicit scoped HTTP operations; see `services/gateway/docs/ARCHITECTURE.md`. Run one replica/worker for its process-local rate limits. +- **Gateway email login stays on Misty.** Verification owns code generation/send/check; gateway owns eligibility, consumer-bound challenges, durable quotas, and atomic single-use redemption. Preserve Discord’s existing correct-code replay. The compute backend attests `client_ip` from trusted ingress and must bind challenges to browser sessions; it never gets a verification key. See `services/gateway/docs/API.md#email-login`. - **Gateway proxy trust is specific to `X-Real-IP`.** Railway's public edge overwrites it; `X-Forwarded-For` may include a shared proxy hop and is ignored. Health probes skip the IP quota, so preserve the per-app readiness cache and nonblocking probe lock; see `services/gateway/docs/API.md#health`. - **documentation-system and verification both bind host port 5434.** They cannot run locally at the same time. Remap one to an unused port (gateway uses 5435 by default) and update its `DATABASE_URL`. If Alembic reports an unknown revision, you're almost certainly pointed at the other service's database. - **Discovery imports; refresh repairs.** `doc-discover` enumerates new Google file identities over authenticated HTTP, with a persistent checkpoint volume and `discover` connector scope. It never grants access from listing metadata, reactivates retired docs, or refetches existing docs. See [discovery operations](services/documentation-system/docs/DISCOVERY.md); keep #292 refresh separate. diff --git a/README.md b/README.md index 2de555ce..1ed7c101 100644 --- a/README.md +++ b/README.md @@ -59,10 +59,10 @@ The other four are internal-facing: **[llm](services/llm/README.md)** (`POST /ch - **Deploy is `git push`.** Merge to `staging` → Railway rebuilds and deploys staging automatically. Promote via a `staging → main` PR for production. No separate CD system. - **Roll back is `git revert`** + push. For migrations, follow the [migration-specific recovery guidance](docs/DEPLOYMENT-HISTORY.md#redeploy-and-rollback); downgrade does not necessarily restore cleaned-up data. -- **Manage API keys.** Three different storage models, by design: +- **Manage API keys.** Two storage models, by design: - **team-tracking, documentation-system, gateway** — issued into separate `api_keys` tables via each service’s key CLI. Gateway accepts issued `gw_` keys only; it has no bootstrap env key. Scoped, revocable, per-consumer, argon2-hashed at rest. - **llm, meeting, connectors** — no key table. `llm-keys` / `meeting-keys` / `connectors-keys` _print_ a key plus a JSON entry you paste into that service's `CONSUMER_KEYS` variable; adding or revoking one is a redeploy. - - **verification** — no per-consumer keys at all. Only the bootstrap `API_KEY` env var authenticates, since its single consumer is the bot. + - **verification** — config-backed scoped `vf_` keys in `CONSUMER_KEYS`, with the existing bot bootstrap key retained. See [provisioning](services/verification/docs/DEPLOYMENT.md#scoped-consumer-keys). --- @@ -213,7 +213,7 @@ All seven services are built the same way on purpose — learning one gives you - **`contracts/` Protocol boundary.** Each service has a `contracts/` package of Pydantic domain types plus `Protocol` interfaces. Application code depends on the Protocols, never on a concrete implementation. - **Swappable storage adapters.** `InMemoryStorageAdapter` for fast tests, `PostgresStorageAdapter` for real runs — both satisfy the same Protocol. Tests use in-memory; a small integration test suite gates the Postgres adapter too. -- **Scoped API-key auth.** Protected HTTP endpoints use `X-API-Key`; health and API schema/UI routes remain unauthenticated within the service’s network exposure. Keys are argon2-hashed with a set of per-resource scopes (`people:read`, `teams:write`, `chat`, `meetings`, `fetch`, etc.). This machinery is implemented once in the shared [`packages/auth`](packages/auth) (`platform_auth`) library and consumed by all seven services through a thin shim (`src/api/auth.py`, plus `hashing.py` for the services that mint their own keys) that binds its own key prefix and config. Anything the library exposes ready-to-use — `AuditLogMiddleware`, for one — is imported from `platform_auth` directly; a per-service file earns its place only by binding something. team-tracking, documentation-system, and gateway store consumer keys in their own `api_keys` tables; `llm`, `meeting`, and `connectors` seed them from a `CONSUMER_KEYS` JSON env var instead, so rotating one there is a redeploy. +- **Scoped API-key auth.** Protected HTTP endpoints use `X-API-Key`; health and API schema/UI routes remain unauthenticated within the service’s network exposure. Keys are argon2-hashed with a set of per-resource scopes (`people:read`, `teams:write`, `chat`, `meetings`, `fetch`, etc.). This machinery is implemented once in the shared [`packages/auth`](packages/auth) (`platform_auth`) library and consumed by all seven services through a thin shim (`src/api/auth.py`, plus `hashing.py` for the services that mint their own keys) that binds its own key prefix and config. Anything the library exposes ready-to-use — `AuditLogMiddleware`, for one — is imported from `platform_auth` directly; a per-service file earns its place only by binding something. team-tracking, documentation-system, and gateway store consumer keys in their own `api_keys` tables; `llm`, `meeting`, `connectors`, and `verification` seed them from a `CONSUMER_KEYS` JSON env var instead, so rotating one there is a redeploy. - **Credentials are `SecretStr`, never `str`.** Every credential field in a service's `Settings` is `pydantic.SecretStr`, so it renders as `**********` in any repr, log line, traceback, or failing assertion diff — a plain `str` once printed a real Google private key into a transcript. Unwrap with `.get_secret_value()` at the boundary; `platform_auth` still takes plain `str`. See [`packages/auth/README.md` → Credential config convention](packages/auth/README.md#credential-config-convention) for the one way forgetting to unwrap fails _silently_ rather than loudly. - **Attested actor.** The `created_by`/`updated_by` on every audit field is the authenticated key's own name — a caller can't claim to be someone else. - **Per-request audit log.** Middleware emits one JSON line per request with the resolved actor, endpoint, status, and duration. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cf20b5db..ba5fadfc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -206,19 +206,17 @@ describes how it applies them concretely. current heads rather than pinning a deployment to an old revision. For cleanup/rollback exceptions, follow the service deployment guide. `llm`, `meeting`, and `connectors` have no schema. -- **Three auth storage models.** The convention is scoped, argon2-hashed keys +- **Two auth storage models.** The convention is scoped, argon2-hashed keys behind `platform_auth`'s `ApiKeyStore` protocol — but where they *live* varies with how many consumers a service has: - **team-tracking, documentation-system, gateway** — separate `api_keys` tables, minted/revoked with each service’s CLI. Gateway accepts only issued `gw_` keys and has no inbound bootstrap env key. - - **`llm`, `meeting`, `connectors`** — no table. Keys are seeded at boot from - a `CONSUMER_KEYS` JSON array env var, so their "mint" CLIs only *print* a key - and its JSON entry. Adding or revoking one is a variable edit plus a + - **`llm`, `meeting`, `connectors`, `verification`** — no key table. Keys are seeded at boot from + a `CONSUMER_KEYS` JSON array env var, with argon2 hashes and scoped consumer identities. Services with "mint" CLIs + only print a key and its JSON entry; verification uses the shared generator. Adding or revoking one is a variable edit plus a redeploy, and a malformed `CONSUMER_KEYS` fails the deploy at boot by design. - - **`verification`** — a `NullApiKeyStore`: no per-consumer keys exist, and - only the bootstrap `API_KEY` env var authenticates. It has exactly one - consumer (the bot), so issuing keys would be ceremony without benefit. + The protocol is what makes this vary cleanly — each service picks a store, and no route code changes. @@ -245,8 +243,9 @@ callers go through a gateway at all. Callers outside the org's trust boundary (a GitHub Action, a future third-party integration) never get a team-tracking key. Instead they hold a key issued by the gateway's own external registry (`gateway-keys`, scoped e.g. `resolve:discord`), and - the gateway holds exactly **one** internal team-tracking key - (`identifiers:read`) for its own outbound calls. It composes and curates — it never + the gateway holds its own internal team-tracking key + (`identifiers:read`, plus `people:read` for login) and a separate private + verification key when email login is enabled. It composes and curates — it never passes an internal response straight through — and adds the protections an externally-facing surface needs that internal services don't: a public Railway domain, per-key rate limiting behind a per-IP flood guard, and audit logging of @@ -262,7 +261,7 @@ callers go through a gateway at all. every caller must present an issued, scoped key. When adding an externally-reachable service, copy that, not the internal shim. -The gateway's first (and so far only) endpoint is the resolver, +The gateway's first endpoint is the resolver, `GET /v1/resolve/discord/{github_login}` — it turns a GitHub login into the Discord id linked to the same person in the directory, and nothing else. It is a working example for future gateway operations; no current integration consumes it. @@ -271,10 +270,11 @@ but do not call the resolver or ping reviewers' Discord accounts. New external use cases get new narrow endpoints on the gateway, not broader access to team-tracking itself. -The compute platform will use the same per-consumer gateway contract when its -required operations are defined. The [gateway architecture](../services/gateway/docs/ARCHITECTURE.md#future-compute-platform-access) -describes how to add access while preserving service-owned databases and access -checks; the current permission grants only the Discord resolver. +The compute platform can use the [email-login contract](../services/gateway/docs/API.md#email-login) +with `auth:email` and `auth:status`. Misty handles eligibility, delivery, code +verification, and abuse limits; compute owns its sessions and permissions. The +[gateway architecture](../services/gateway/docs/ARCHITECTURE.md#future-compute-platform-access) +describes adding further operations without sharing databases or internal keys. ## Why the directory is built first diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 93e1dd9b..75cb9262 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -287,10 +287,10 @@ their code: seeded from a `CONSUMER_KEYS` JSON env var and parsed at boot into an in-memory store, not an `api_keys` table. Rotating a key means editing the variable and redeploying; there is no `revoke` command. -- **`verification` has a database but no key table.** It uses a - `NullApiKeyStore`, so only the bootstrap `API_KEY` authenticates — there are no - per-consumer keys to mint. Its database retains verification request state - and code hashes; expiry does not delete rows. +- **`verification` has a database but no key table.** Scoped `vf_` consumers + are configured in `CONSUMER_KEYS`; the bot bootstrap key remains supported. + Its database retains verification request state and code hashes; expiry does + not delete rows. Follow its [provisioning and retention guide](../services/verification/docs/DEPLOYMENT.md). - **`meeting` is stateful.** It holds one in-memory session per live meeting. It cannot be horizontally scaled without sticky routing on `session_id`, and a restart drops every in-flight meeting. See diff --git a/docs/RAILWAY-DEPLOYMENT.md b/docs/RAILWAY-DEPLOYMENT.md index f16aebe6..6f2b652b 100644 --- a/docs/RAILWAY-DEPLOYMENT.md +++ b/docs/RAILWAY-DEPLOYMENT.md @@ -14,13 +14,17 @@ Railway + Neon dashboards / CLIs. | `llm` | `/` | **none** | — | Stateless OpenAI chat/embedding API with optional Bedrock chat; keys from `CONSUMER_KEYS`. | | `meeting` | `/` | **none** | — | **Stateful in-memory**; keys from `CONSUMER_KEYS`. See the single-replica warning in step 2, and the WebSocket keepalive note in the Notes section — its `startCommand` is the one that isn't the plain template. | | `connectors` | `/` | **none** | — | Stateless outbound adapter (Google Drive/Docs); keys from `CONSUMER_KEYS`. Recommended to deploy before `documentation-system` (not required) — see its `CONNECTORS_API_KEY` note in step 3. | -| `gateway` | `/` | Neon (own project) | `alembic upgrade head` | Public scoped reads; one replica. See [gateway setup](../services/gateway/docs/DEPLOYMENT.md). | +| `gateway` | `/` | Neon (own project) | `alembic upgrade head` | Public scoped reads and optional email login; one replica. See [gateway setup](../services/gateway/docs/DEPLOYMENT.md). | | `discord-bot` | `discord-bot` | none | `node src/registerCommands.js` | Node Discord frontend. Pre-deploy registers the slash commands with Discord (step 5). | The gateway receives a public HTTPS domain. All other services remain **private** and reach each other as `.railway.internal:`. Gateway credentials, consumer keys, proxy settings, and verification are covered in its [deployment runbook](../services/gateway/docs/DEPLOYMENT.md). +Email login requires gateway migration002, `EMAIL_LOGIN_ENABLED`, private +`VERIFICATION_BASE_URL`/`VERIFICATION_API_KEY`, a scoped `vf_` entry in verification +`CONSUMER_KEYS`, directory `people:read`, and compute `auth:email`/`auth:status`. +Follow [enablement](../services/gateway/docs/DEPLOYMENT.md#enable-email-login) in order. For the optional persistent `dev` environment used to test one PR at a time with a dedicated Discord bot, see [PR previews](pr-previews.md). That diff --git a/packages/auth/README.md b/packages/auth/README.md index 7685f0d4..c11e9239 100644 --- a/packages/auth/README.md +++ b/packages/auth/README.md @@ -19,7 +19,7 @@ Shared API-key auth (keys, scopes, audit) for UTMIST platform services. A pure l | `ADMIN_SCOPE` | `models.py` | The wildcard scope (`"admin"`) that satisfies any `require_scope` check. | | `DEV_SPOOF_SCOPE` | `models.py` | The `"dev:spoof"` scope; its [production guard](#development-scope-guard) is opt-in and uses the service's `is_prod` predicate. | | `ConsumerKeyRow` | `memory_store.py` | Frozen dataclass row shape backing `InMemoryKeyStore` — `id`, `name`, `scopes`, `active`, `revoked_at`. | -| `InMemoryKeyStore` | `memory_store.py` | DB-free `ApiKeyStore` implementation. Used by services with no `api_keys` table (`llm`, `meeting`, `connectors`) — keys are seeded from config at boot rather than persisted. | +| `InMemoryKeyStore` | `memory_store.py` | DB-free `ApiKeyStore` implementation. Used by services with no `api_keys` table (`llm`, `meeting`, `connectors`, `verification`) — keys are seeded from config at boot rather than persisted. | | `key_store_from_config` | `memory_store.py` | Parses a `CONSUMER_KEYS` JSON array (`[{"name","prefix","key_hash","scopes"}]`) into a populated `InMemoryKeyStore`. Raises `RuntimeError` on a malformed value so a bad env var fails a service's boot, not its first request. | ## Credential config convention @@ -55,8 +55,7 @@ New service? Write the credentials as `SecretStr` from the start, and give the t ## Key-store wiring - **DB-backed** (`team-tracking`, `documentation-system`, `gateway`): the service's own `StorageAdapter` satisfies `ApiKeyStore` structurally. Gateway accepts only issued `gw_` keys and disables bootstrap authentication with `get_env_key=lambda: None`. -- **Config-backed** (`llm`, `meeting`, `connectors`): `key_store_from_config(settings.consumer_keys.get_secret_value())` builds an `InMemoryKeyStore` at boot from `CONSUMER_KEYS`. There is no revoke call — revocation is dropping the entry from `CONSUMER_KEYS` and redeploying. -- **Env-key-only** (`verification`): its `NullApiKeyStore` rejects all stored-key lookups; only the bootstrap `API_KEY` authenticates. See its [auth model](../../services/verification/README.md#auth-model). +- **Config-backed** (`llm`, `meeting`, `connectors`, `verification`): `key_store_from_config(settings.consumer_keys.get_secret_value())` builds an `InMemoryKeyStore` at boot from `CONSUMER_KEYS`. There is no revoke call — revocation is dropping the entry from `CONSUMER_KEYS` and redeploying. ## Development scope guard diff --git a/services/gateway/.env.example b/services/gateway/.env.example index 5daeb18d..1f99f1da 100644 --- a/services/gateway/.env.example +++ b/services/gateway/.env.example @@ -4,7 +4,7 @@ DATABASE_URL=postgresql+psycopg://gateway:dev_password@localhost:5435/gateway # admin key — inbound callers must present a scoped key issued by `gateway-keys` # and stored in the api_keys table. See src/api/auth.py. -# Outbound: the gateway's own team-tracking key, scoped identifiers:read. +# Outbound: the gateway's own team-tracking key, scoped identifiers:read; add people:read for login. DIRECTORY_BASE_URL=http://localhost:8000 DIRECTORY_API_KEY=dev-api-key-change-me @@ -14,3 +14,8 @@ GATEWAY_ENV=local # for direct/private-network callers or proxies that preserve client-supplied # X-Real-IP. X-Forwarded-For is ignored; missing/invalid X-Real-IP uses the peer. TRUST_PROXY_HEADERS=false + +# Optional server-to-server email login. Configure a distinct scoped vf_ key first. +EMAIL_LOGIN_ENABLED=false +VERIFICATION_BASE_URL=http://localhost:8003 +VERIFICATION_API_KEY=dev-api-key-change-me diff --git a/services/gateway/README.md b/services/gateway/README.md index 73df5a81..0de5fed1 100644 --- a/services/gateway/README.md +++ b/services/gateway/README.md @@ -17,13 +17,12 @@ reviewer logins, but do not call the gateway or mention reviewers on Discord. The gateway exists for exactly that shape of consumer: -- It holds **one internal team-tracking key** (scoped `identifiers:read`) for its own +- It holds **one internal team-tracking key** (scoped `identifiers:read`, plus `people:read` for email login) for its own outbound calls — external callers never see it. - It issues and manages its **own, separate registry of external API keys** (via the `gateway-keys` CLI), scoped to gateway-specific permissions like `resolve:discord`. -- It exposes a **narrow, curated surface** — today, one endpoint — that returns only - what the consumer needs (a Discord id), never a raw pass-through of team-tracking's - response. +- It exposes a **narrow, curated surface**: Discord resolution and optional email + login for active Misty members. Responses contain only the fields each operation needs. - It rate-limits and audit-logs every request, since (unlike the internal services) its callers are outside UTMIST's control. @@ -50,7 +49,7 @@ docker compose up -d postgres # 2. Install dependencies (including dev tools) uv sync --extra dev -# 3. Apply database migrations (creates the api_keys table) +# 3. Apply database migrations (key registry and login state) uv run alembic upgrade head # 4. Start the API server @@ -67,9 +66,12 @@ schema is at `http://localhost:8006/openapi.json`. | Variable | Purpose | |---|---| -| `DATABASE_URL` | The gateway's **own** Postgres — it holds only `api_keys` (its external key registry), never a copy of team-tracking's data. | +| `DATABASE_URL` | The gateway's **own** Postgres — it holds external keys, short-lived login challenges, and abuse counters. Directory records remain in team-tracking. | | `DIRECTORY_BASE_URL` | team-tracking's base URL — where the gateway makes its **outbound** call. | -| `DIRECTORY_API_KEY` | The gateway's **one internal** team-tracking key, scoped `identifiers:read`. Follow the [directory-pinned provisioning procedure](docs/DEPLOYMENT.md#database-and-internal-access). | +| `DIRECTORY_API_KEY` | The gateway's **one internal** team-tracking key, scoped `identifiers:read`, plus `people:read` for email login. Follow the [directory-pinned provisioning procedure](docs/DEPLOYMENT.md#database-and-internal-access). | +| `EMAIL_LOGIN_ENABLED` | `false` by default. Enable after provisioning private verification and directory access. | +| `VERIFICATION_BASE_URL` | Private verification URL; local default `http://localhost:8003`. | +| `VERIFICATION_API_KEY` | A distinct scoped `vf_` key; blank/dev values are refused outside local mode when login is enabled. | | `GATEWAY_ENV` | `local` / `staging` / `production`. In non-`local` envs, startup refuses to boot if `DIRECTORY_API_KEY` is blank or still the built-in dev default. | | `TRUST_PROXY_HEADERS` | Trust a single valid `X-Real-IP` supplied by an ingress that overwrites that header. **Set this to `true` behind Railway's public edge.** `X-Forwarded-For` is ignored. Missing/invalid/duplicate `X-Real-IP` falls back to the socket peer. Leave `false` for direct/private-network callers. | @@ -83,6 +85,14 @@ has neither justification: its keys are issued straight into the database by `API_KEY` here does nothing — `src/api/auth.py` passes `get_env_key=lambda: None`, and `tests/test_auth.py` pins that it stays that way. +## Email login + +The compute backend can request and confirm codes for an active person’s registered +primary email. Misty owns code delivery, verification, challenges, and abuse limits. +The compute backend owns its sessions and must check current eligibility. See the +[API and integration contract](docs/API.md#email-login) and +[enablement steps](docs/DEPLOYMENT.md#enable-email-login). + ## The resolver endpoint ```http @@ -143,6 +153,9 @@ id itself. ### Rate limiting +This section describes the resolver and shared flood guard; email login has +[additional durable quotas](docs/API.md#limits-and-failures). + Two layers, because they defend different things: | Layer | Where | Limit | Keyed on | @@ -197,7 +210,9 @@ for recording an operator name with `--actor` and preserving revocation history. Scopes recognized today: -- `resolve:discord` — the only external-facing scope, and the only one to issue. +- `resolve:discord` — Discord resolution. +- `auth:email` — start and confirm email login. +- `auth:status` — active-person status for compute sessions. `platform_auth` also treats `admin` as a wildcard that satisfies every scope check. The CLI rejects `admin`, unknown scopes, and missing scopes. There is no bootstrap path that @@ -226,7 +241,7 @@ gateway/ │ │ └── routers/resolve.py `GET /v1/resolve/discord/{github_login}` │ │ │ ├── directory/http_client.py HTTP DirectoryClient — calls team-tracking with DIRECTORY_API_KEY -│ ├── storage/ StorageAdapter implementations (in-memory + Postgres) for the gateway's own api_keys table +│ ├── storage/ StorageAdapter implementations (in-memory + Postgres) for keys, login challenges, and abuse counters │ ├── cli.py gateway-keys CLI (issue / list / revoke external keys) │ └── config.py Settings (DATABASE_URL, DIRECTORY_*, GATEWAY_ENV, TRUST_PROXY_HEADERS) │ @@ -245,7 +260,7 @@ uv run pytest --ignore=tests/test_postgres_adapter.py ``` For the full suite, start the local Compose Postgres and create a separate, -disposable `gateway_test` database. The adapter tests clear its key table: +disposable `gateway_test` database. The adapter tests clear its key and login tables: ```bash docker compose up -d postgres @@ -257,11 +272,12 @@ DATABASE_URL="$GATEWAY_TEST_DATABASE_URL" uv run alembic upgrade head RUN_PG_TESTS=1 uv run pytest ``` -From the repo root, verify the actual HTTP integration with the current directory: +From the repo root, verify the actual HTTP integration with the current directory and verification service: ```bash docker build -f services/team-tracking/Dockerfile -t team-tracking:ci . docker build -f services/gateway/Dockerfile -t gateway:ci . +docker build -f services/verification/Dockerfile -t verification:ci . python3 services/gateway/tests/smoke_stack.py ``` @@ -285,12 +301,12 @@ The health endpoints are documented in [API.md](docs/API.md#health). Railway use ## Status -Public gateway with its own `api_keys` registry (migration 001), one internal -team-tracking key for outbound calls, and one endpoint: -`GET /v1/resolve/discord/{github_login}`. Rate-limited (60 req/min/key, plus a -per-IP flood guard) and audit-logged. Every caller must present a key issued by -`gateway-keys` — there is no env-bootstrap admin path. Built on `packages/auth` -(`platform_auth`) — no auth logic is reimplemented here. +Public gateway with external keys and durable login state (migration002). +The resolver and optional active-member email login use explicit scopes, private +HTTP, quotas, and audit logging. Every caller presents an issued `gw_` key; +there is no bootstrap admin path. Code delivery and checking remain in Misty's +private verification service. Enabling and connecting compute requires the +[deployment procedure](docs/DEPLOYMENT.md#enable-email-login). **Not implemented (by design):** diff --git a/services/gateway/contracts/directory.py b/services/gateway/contracts/directory.py index 2325245d..311823d6 100644 --- a/services/gateway/contracts/directory.py +++ b/services/gateway/contracts/directory.py @@ -6,6 +6,8 @@ class DirectoryUnavailable(Exception): class DirectoryClient(Protocol): + def get_person_by_email(self, email: str) -> dict | None: ... + def get_person(self, person_id: str) -> dict | None: ... def get_person_by_github(self, github_login: str) -> dict | None: ... def list_identifiers(self, person_id: str) -> list[dict]: ... def close(self) -> None: ... diff --git a/services/gateway/contracts/login.py b/services/gateway/contracts/login.py new file mode 100644 index 00000000..b19e0e0b --- /dev/null +++ b/services/gateway/contracts/login.py @@ -0,0 +1,77 @@ +"""Login state belongs to the gateway; code secrets belong to verification.""" + +import re +from datetime import datetime +from typing import Protocol +from uuid import UUID + +from pydantic import BaseModel, ConfigDict, Field, IPvAnyAddress, SecretStr, field_validator + + +class VerificationUnavailable(Exception): + """Private verification failed; no upstream payload may escape.""" + + +class VerificationClient(Protocol): + def request_code(self, subject: str, email: str) -> None: ... + def confirm_code(self, subject: str, code: str) -> str | None: ... + def close(self) -> None: ... + + +class LoginChallenge(BaseModel): + model_config = ConfigDict(extra="forbid") + id: UUID + owner: str + email: str + created_at: datetime + expires_at: datetime + person_id: UUID | None = None + attempts: int = 0 + redeemed: bool = False + + +class EmailStartIn(BaseModel): + model_config = ConfigDict(extra="forbid") + email: str = Field(max_length=254) + # An attestation from the authenticated backend, never a public browser header. + client_ip: IPvAnyAddress + + @field_validator("email") + @classmethod + def normalize_email(cls, value: str) -> str: + value = value.strip().lower() + if not re.fullmatch(r"[^@\s]+@[^@\s]+\.[^@\s]+", value): + raise ValueError("invalid email") + return value + + +class EmailConfirmIn(BaseModel): + model_config = ConfigDict(extra="forbid") + challenge_id: UUID + code: SecretStr + client_ip: IPvAnyAddress + + @field_validator("code") + @classmethod + def valid_code(cls, value: SecretStr) -> SecretStr: + if not re.fullmatch(r"[0-9]{6}", value.get_secret_value()): + raise ValueError("expected six digits") + return value + + +class EmailStartOut(BaseModel): + model_config = ConfigDict(extra="forbid") + challenge_id: UUID + expires_in: int = 600 + message: str = "If an active Misty account matches, a code will be sent." + + +class EmailConfirmOut(BaseModel): + model_config = ConfigDict(extra="forbid") + person_id: UUID + email: str + + +class PersonStatusOut(BaseModel): + model_config = ConfigDict(extra="forbid") + active: bool diff --git a/services/gateway/contracts/storage.py b/services/gateway/contracts/storage.py index a8e54eda..62b41b61 100644 --- a/services/gateway/contracts/storage.py +++ b/services/gateway/contracts/storage.py @@ -1,10 +1,43 @@ +from datetime import datetime from typing import Protocol from uuid import UUID from contracts.types import ApiKey +from contracts.login import LoginChallenge class StorageAdapter(Protocol): + def allow_login_request( + self, owner: str, operation: str, client_ip: str | None, now: datetime + ) -> bool: + """Atomically spend app and attested-IP quotas; prune expired login state.""" + ... + + def reserve_login(self, challenge: LoginChallenge) -> bool: + """Atomically reserve mailbox send quotas and persist a pending challenge. + + Quotas apply equally to unknown/inactive mailboxes and are never refunded. + Older challenges remain usable until replacement delivery succeeds. + """ + ... + + def activate_login(self, challenge_id: UUID, person_id: UUID, now: datetime) -> bool: + """Bind after delivery and invalidate older challenges for this app/email. + + A late activation cannot supersede a newer successfully delivered code. + """ + ... + + def admit_login_attempt( + self, challenge_id: UUID, owner: str, now: datetime + ) -> LoginChallenge | None: + """Atomically spend challenge and mailbox guess budgets before verification.""" + ... + + def redeem_login(self, challenge_id: UUID, owner: str, now: datetime) -> bool: + """Single-use compare-and-set; exactly one concurrent redemption can succeed.""" + ... + def is_ready(self) -> bool: ... def create_api_key( self, *, name: str, prefix: str, key_hash: str, scopes: list[str], actor: str diff --git a/services/gateway/docs/API.md b/services/gateway/docs/API.md index df5f39f1..335d7704 100644 --- a/services/gateway/docs/API.md +++ b/services/gateway/docs/API.md @@ -11,7 +11,7 @@ documents its one-field response, errors, rate limits, and case sensitivity. The scope permits lookup of any linked GitHub login; it does not restrict a consumer to a particular person or team. -There are no public person, membership, document, database-query, or write routes. +There are no general person, membership, document, or database-query routes. External identity headers do not grant delegated user access to private services. ## Health @@ -19,7 +19,7 @@ External identity headers do not grant delegated user access to private services | Route | Authentication | Behavior | |---|---|---| | `GET /health` | None | Process liveness; `200 {"status":"ok"}` even if a dependency is unavailable. | -| `GET /health/ready` | None | Checks the migrated key table; caches either result for five seconds. Returns 200 when ready, otherwise `503 {"detail":"gateway database unavailable"}`. | +| `GET /health/ready` | None | Checks all migrated gateway tables; caches either result for five seconds. Returns 200 when ready, otherwise `503 {"detail":"gateway database unavailable"}`. | Both health routes are exempt from the IP limit. Readiness permits only one database probe at a time per app, with at least five seconds between a completed @@ -30,3 +30,111 @@ the response to change. `/health` does no database work. Readiness checks only the gateway's own database, not team-tracking. A resolver smoke request separately verifies the internal URL, internal key, and directory data. + +## Email login + +These are server-to-server routes for the compute backend. They require an issued +`gw_` key and `EMAIL_LOGIN_ENABLED=true`. Use `auth:email` for both POSTs and +`auth:status` for status checks. A resolver-only key cannot use them. No auto-signup +occurs. Only `Person.primary_email` is accepted (trimmed and lowercased); secondary +identifiers are not login addresses. Eligibility requires `active=true`. + +All responses, including errors, use `Cache-Control: no-store`. Input models +reject extra fields. Login bodies are limited to 4096 bytes; errors never echo +submitted codes. Codes must be exactly six ASCII digits. + +### Start + +`POST /v1/auth/email/start` + +```json +{"email":"member@example.com","client_ip":"192.0.2.1"} +``` + +Returns **202**, with an opaque UUID `challenge_id`, `expires_in: 600`, and +`message: "If an active Misty account matches, a code will be sent."` +The same shape is returned for unknown/inactive emails and mailbox throttling. +A throttled request returns a nonredeemable ID and leaves the previous challenge +usable. A resend invalidates previous challenges for this app/email only after successful +delivery and activation. Failed replacement delivery preserves the old code. + +Eligibility lookup and code delivery run after the response is sent, to keep +account-dependent provider latency out of the public response. Delivery is best +effort: an upstream failure or restart may leave an unusable challenge. There is +no automatic resend after ambiguous delivery. The user may request another code +after the cooldown; 202 means accepted, not proof of delivery or membership. + +### Confirm + +`POST /v1/auth/email/confirm` + +```json +{"challenge_id":"","code":"123456","client_ip":"192.0.2.1"} +``` + +On success, **200** returns only `{"person_id":"","email":"member@example.com"}`. +The code must belong to this challenge, and the authenticated application must +match the starter. Misty rechecks the original person UUID, active flag, and +primary email immediately before atomic redemption. Retiring the person, +changing the email, or reassigning it to a different person prevents login. +A challenge can succeed only once, including concurrent confirms. A lost success +response cannot be replayed: request a new code. Verification outages fail closed. + +Unknown, pending, expired, replaced, exhausted, already-used, wrong-app, and +incorrect-code challenges all return **400** with `invalid or expired login challenge`. +Neither a person ID nor email is returned on failure. + +### Current eligibility + +`GET /v1/auth/people/{person_id}/status` returns **200** with only +`{"active":true}` or `{"active":false}`. Missing and retired people both return +false. Directory failures return 503, never an assumed active result. This scope +permits status checks by UUID; it is not a public email lookup. + +### Limits and failures + +Login limits are persistent and atomic across concurrent requests and restarts. +Windows start on the first admitted request and expire after the duration below. + +| Dimension | Limit | +|---|---| +| App start requests | 30/minute, 300/hour | +| App confirm requests | 60/minute | +| App status requests | Separate 60/minute | +| Backend-attested client IP | 5 starts/minute, 30 confirms/minute, shared across apps | +| Mailbox sends | 1/minute, 5/hour, 20/day, shared across apps | +| Challenge guesses | 5, reserved before calling verification | +| Mailbox guesses | 15/hour across all challenges/apps | +| Challenge lifetime | 10 minutes from start, including delivery time | + +IPv4-mapped IPv6 addresses normalize to IPv4; native IPv6 addresses share a /64 +bucket. Mailbox quotas include unknown/inactive addresses. Delivery failure does +not refund them. App/IP exhaustion returns **429** with `Retry-After: 60` (a retry +may still hit the hourly limit). Mailbox limits remain hidden behind generic +202/400 responses. Status requests have a separate app quota, but all endpoints +still share the existing pre-authentication edge-IP flood guard (120/minute). + +Missing/invalid key → 401; missing scope → 403; oversized body → 413; +invalid input → 422; disabled login or unavailable upstream → 503. + +### Compute integration responsibilities + +1. Keep the `gw_` key exclusively on the backend. Serve the login page over HTTPS. + Never provide internal service keys or database credentials to compute. +2. Derive `client_ip` from the backend's trusted ingress/socket configuration; + discard browser-supplied values and forwarded headers from untrusted peers. + This is an attestation by the authenticated backend, not a gateway header + override. Gateway edge-IP and app/mailbox limits still apply independently. +3. Bind each returned challenge to the initiating browser session, protect POSTs + against CSRF, and forward the submitted code to confirm. Do not create a session + based on the browser's claim that confirmation succeeded. +4. On confirmed success, map the stable Misty UUID to the existing compute account + and establish/rotate a server-side session with Secure, HttpOnly cookies. Keep + existing ownership IDs for jobs/files. Do not automatically merge accounts from + a browser-supplied email or silently grant compute roles/admin rights. +5. Check status on protected compute requests (or adopt a documented short cache + window). Fail closed on false, 429, timeout, or 5xx. An active Misty identity is + eligibility; compute still owns its authorization and resource permissions. + +This is an email-code backchannel contract, not OIDC and not a bearer-token issuer. +There are no refresh tokens, public browser callbacks, or compute sessions here. diff --git a/services/gateway/docs/ARCHITECTURE.md b/services/gateway/docs/ARCHITECTURE.md index 5ffbd342..2f57d7dc 100644 --- a/services/gateway/docs/ARCHITECTURE.md +++ b/services/gateway/docs/ARCHITECTURE.md @@ -5,14 +5,16 @@ private services over authenticated HTTP and returns explicitly selected fields. Each private service remains responsible for its own database and business rules. See the platform's [access architecture](../../../docs/ARCHITECTURE.md#access-architecture-two-doors-not-one-gateway). -The gateway's Postgres database contains only its external key registry. Its -outbound `DIRECTORY_API_KEY` is issued by team-tracking with `identifiers:read`. +The gateway's Postgres database contains its external key registry, short-lived +login challenges, and abuse counters. Its outbound `DIRECTORY_API_KEY` needs +`identifiers:read` for resolution and `people:read` for email login/status. Inbound keys use `gw_`; the internal key uses `tt_`. The shared auth library verifies the envelope, hash, revocation state, and scope. No environment bootstrap key is accepted, and the CLI allows only explicitly supported external scopes. -The request path is audit middleware → IP limit → authentication → consumer quota -→ endpoint scope → upstream client → curated response. `deps.py` supplies the +The resolver path is audit middleware → IP limit → authentication → consumer quota +→ endpoint scope → upstream client → curated response. Login adds a bounded body +reader and no-store headers, then scope checks and durable login quotas. `deps.py` supplies the pooled HTTP client through the `DirectoryClient` Protocol. Each app lifespan owns its client, constructed lazily after scope checks under a lock so concurrent first requests share one pool. Shutdown closes that pool after requests drain; an unused @@ -43,16 +45,48 @@ This keeps probes accessible after quota exhaustion without letting a public readiness flood occupy the database pool or fill worker threads with lock waiters. The [health contract](API.md#health) describes the resulting detection delay. +## Email login ownership and concurrency + +The [email-login contract](API.md#email-login) implements compute's sign-in +requirements. The gateway owns eligibility orchestration and app-bound challenges; +verification owns code generation, HMAC storage, email delivery, and code checking. +Directory records remain in team-tracking. Communication is scoped private HTTP. + +A start atomically reserves mailbox quotas and a pending challenge before any +lookup or delivery. A background task looks up the active primary email, requests +verification for the opaque `gateway-login:` subject, then binds +the original person ID. Background failures log only a static message; they never +change the already-sent generic response. Tasks are not a durable delivery queue. +On process loss, the user retries after the cooldown; budgets are retained and no +ambiguous send is retried automatically. Monitor delivery-failure logs. + +Before confirmation reaches verification, the gateway atomically reserves a +challenge attempt and a mailbox attempt. It then rechecks the original person's +active flag and email. A final compare-and-set permits exactly one redemption. +Private verification's existing correct-code replay remains available for the +Discord flow; the public login boundary is strictly single-use. There is no +cross-service transaction with directory changes: status checks on compute requests +close the eligibility gap after authentication. + +Both adapters implement the same storage protocol. Postgres serializes short +login-state transactions with a transaction-scoped advisory lock, never held during +HTTP. This intentionally simple design fits the one-worker, low-volume deployment. +Expiry indexes support pruning on each authenticated login/status request. Gateway +challenges retain email/UUID until expiry and the next request; rate buckets retain +hashes of email/IP (pseudonymous, not anonymous) for at most their one-day window +and the next request. Verification retains separate HMAC/email rows; follow its +[data-retention procedure](../../verification/docs/DEPLOYMENT.md#data-retention). + +The authenticated compute backend attests `client_ip`; it must derive that value +from trusted ingress. This does not alter edge `X-Real-IP` handling. Mailbox and +application quotas do not depend on the attestation. Keep the existing one-replica, +one-worker limit because the pre-authentication flood guard and resolver quotas +are still process-local. Persistent login quotas alone do not authorize scaling. + ## Future compute-platform access -The compute platform can become another external consumer with its own key. -Add specific operations as its requirements become concrete: define returned -fields, endpoint scopes, and any per-user or per-team authorization first. Then -call the owning service's API. No database connection string or internal key is -handed to the compute platform. - -`resolve:discord` is the only implemented data permission. Issuing a key with that -scope does not grant general directory or document access. Application identity -also does not establish an end user's identity: a future user-specific endpoint -must implement an explicit delegation contract and preserve the owning service's -access checks. Add new endpoints via the [contribution guide](CONTRIBUTING.md). +Compute receives only the scopes it uses (`auth:email`, `auth:status`). Login +proves identity and current eligibility, not compute roles or general directory +access. Additional data access still requires explicit operations and permissions, +following the [contribution guide](CONTRIBUTING.md). Never expose database access +or allow a claimed user identity to bypass a private service's authorization. diff --git a/services/gateway/docs/CHANGE-AUDIT.md b/services/gateway/docs/CHANGE-AUDIT.md index 96c4470c..dfae7ac9 100644 --- a/services/gateway/docs/CHANGE-AUDIT.md +++ b/services/gateway/docs/CHANGE-AUDIT.md @@ -1,5 +1,8 @@ # Gateway integration audit +The original PR65 record below is historical. The email-login extension and its +current file rationale are recorded at the end of this document. + [PR #65](https://github.com/UTMIST/Misty/pull/65) adds the external HTTP entry point and its first curated read operation. The PR records the tested staging revision and verification results. @@ -93,3 +96,67 @@ Railway inspection found no existing gateway service in the Misty project. The [deployment runbook](DEPLOYMENT.md) specifies its separate database, scoped internal key, public HTTPS domain, and consumer-key provisioning. This audit records local verification; it does not assert a live deployment. + + +## Email-login extension + +Misty now provides scoped server-to-server email login for active directory +members. This intentionally touches gateway, private verification authentication, +and the platform documentation/CI that describe their boundary. Compute sessions +and deployment are separate integration steps. Existing Discord code replay is +preserved; gateway login redemption is atomic and single-use. + +This change is opt-in (`EMAIL_LOGIN_ENABLED=false`). Gateway migration002 adds +login challenges and durable quota windows without modifying existing key rows. +Delivery happens after generic202; failed replacement delivery preserves the old +challenge. Confirmation checks code ownership, the original person/email, current +active state, consumer binding, expiry, and atomic redemption. Unknown/inactive +challenges take the same verification call path to avoid an outage-status oracle. + +### File rationale for this extension + +Paths are repository-relative; grouped paths all share the stated purpose. + +| Files | Necessity | +|---|---| +| `services/gateway/contracts/login.py` | Strict request/response models, secret-wrapped submitted codes, challenge state, private verification protocol/errors. | +| `services/gateway/contracts/directory.py`, `services/gateway/contracts/storage.py` | Add scoped lookup and atomic login-state contracts before adapter implementations. | +| `services/gateway/src/login_policy.py` | Centralize send/guess/request windows and normalized email/IP bucket identities. | +| `services/gateway/src/storage/schema.py`, `services/gateway/migrations/versions/002_email_login.py` | Define and reversibly migrate gateway-owned challenge and quota tables/indexes. | +| `services/gateway/src/storage/in_memory.py`, `services/gateway/src/storage/postgres.py` | Equivalent synchronized admission, activation, cleanup and single-use redemption; readiness includes the new schema. | +| `services/gateway/src/api/routers/email_login.py` | Implement start, confirm, status, active-member checks and post-response private delivery. | +| `services/gateway/src/api/app.py`, `services/gateway/src/api/middleware.py` | Register routes, sanitize validation/upstream errors, bound body size, prevent response caching. | +| `services/gateway/src/api/deps.py` | Own and close a lazily constructed private verification HTTP pool. | +| `services/gateway/src/directory/http_client.py` | Use existing authenticated directory email/person lookup routes. | +| `services/gateway/src/verification/__init__.py`, `services/gateway/src/verification/http_client.py` | Package the private verification client and sanitize provider/transport failures. | +| `services/gateway/src/config.py`, `services/gateway/src/cli.py` | Add the opt-in feature, redacted internal credential/boot validation, and two external scopes. | +| `services/gateway/tests/test_email_login.py`, `services/gateway/tests/test_verification.py` | Prove HTTP eligibility, replay/concurrency, generic responses, scope ordering, failure masking, resend recovery and private client contract. | +| `services/gateway/tests/login_storage_cases.py`, `services/gateway/tests/test_login_storage.py`, `services/gateway/tests/test_postgres_adapter.py` | Exercise the same quotas, expiry, races, replacement and redemption semantics against both adapters, plus persistence across adapter recreation. | +| `services/gateway/tests/test_config.py` | Check the new secret is redacted and required when non-local login is enabled. | +| `services/gateway/tests/smoke_stack.py`, `.github/workflows/ci.yml` | Verify real gateway → directory/verification HTTP using disposable DBs, scoped keys, a fake sender and test-container-only deterministic codes; build verification before this check. | +| `services/verification/src/config.py`, `services/verification/src/api/deps.py`, `services/verification/src/api/app.py` | Support a separate scoped gateway identity through config-backed keys and boot validation. | +| `services/verification/src/storage/null_keys.py` (removed) | Remove the unused adapter that rejected all scoped keys. | +| `services/verification/tests/conftest.py`, `services/verification/tests/test_consumer_auth.py` | Isolate cached key configuration and prove scoped auth, denial without scope, and malformed-config boot rejection. | +| `services/gateway/.env.example`, `services/gateway/README.md`, `services/gateway/docs/API.md`, `services/gateway/docs/ARCHITECTURE.md`, `services/gateway/docs/CONTRIBUTING.md`, `services/gateway/docs/DEPLOYMENT.md` | Document new settings, API/session handoff, storage/trust decisions, tests, provisioning and rollback. | +| `services/verification/.env.example`, `services/verification/README.md`, `services/verification/docs/API.md`, `services/verification/docs/ARCHITECTURE.md`, `services/verification/docs/DEPLOYMENT.md` | Replace obsolete bootstrap-only descriptions and document scoped keys plus retention needs of per-login subjects. | +| `AGENTS.md`, `README.md`, `docs/ARCHITECTURE.md`, `docs/DEVELOPMENT.md`, `docs/RAILWAY-DEPLOYMENT.md`, `packages/auth/README.md` | Keep schema heads, service ownership, key-storage descriptions and platform enablement instructions consistent. | +| `services/gateway/docs/CHANGE-AUDIT.md` | Record the intentional cross-zone scope, file rationale, verification and integration boundary. | + +### Verification boundary + +Run gateway and verification full suites with separate disposable databases, +Ruff lint/format checks, migration002 downgrade/upgrade with existing key rows, +the three-service Docker HTTP smoke, and the isolated gateway wheel import check. +No live mail provider is used. These checks do not establish live Railway +configuration, real mailbox delivery, or compute session integration. + +Verified locally: both full service suites with disposable Postgres, Ruff lint +and format checks, migration002 round trip preserving external keys, all three +container builds and the HTTP smoke, and isolated gateway wheel/CLI imports. +Independent review findings were fixed and retested. The test runner reports an +existing Starlette/httpx deprecation warning; no checks failed. A separate local +browser check of the companion compute integration also passed against the real +gateway, directory and verification containers with disposable databases and a +fake mail sender. It covered login, rejection, replay, preserved compute identity, +and retirement after the status cache expires. No live deployment or real email +delivery was performed as part of these checks. diff --git a/services/gateway/docs/CONTRIBUTING.md b/services/gateway/docs/CONTRIBUTING.md index acd4cf1c..a408b430 100644 --- a/services/gateway/docs/CONTRIBUTING.md +++ b/services/gateway/docs/CONTRIBUTING.md @@ -10,8 +10,8 @@ and keep the internal services' data ownership intact. explicit response model; do not return an upstream response wholesale. 2. Add upstream operations to a `contracts/` Protocol and implement them in an HTTP client. Wire concrete clients only in `src/api/deps.py`. -3. Require the new scope before constructing an upstream dependency. Mount the - route under the `/v1` router so it inherits the authenticated key quota. +3. Require the new scope before constructing an upstream dependency. Mount resolver routes under their quota dependency; auth routes use the durable + login quotas documented in the API. Every public operation needs a quota. 4. Add the scope to the CLI's allowed choices and document it in the API and deployment guides. Grant only the needed internal scopes to the gateway. 5. Test authentication, scope rejection before upstream work, response fields, @@ -21,9 +21,9 @@ Storage methods belong in the Protocol and both adapters. Shared behavior tests live in `tests/storage_cases.py`; Postgres tests require a dedicated disposable database and explicitly opt in. See [testing](../README.md#testing). -The Docker HTTP smoke check runs the current team-tracking and gateway images +The Docker HTTP smoke check runs the current team-tracking, verification, and gateway images with separate disposable databases. It never imports another service's code or -uses live credentials. CI runs it after building both images. +uses live credentials. CI runs it after building all three images. ## Before pushing diff --git a/services/gateway/docs/DEPLOYMENT.md b/services/gateway/docs/DEPLOYMENT.md index 40a6bf61..fa76c203 100644 --- a/services/gateway/docs/DEPLOYMENT.md +++ b/services/gateway/docs/DEPLOYMENT.md @@ -6,7 +6,7 @@ The steps below provision a new service; the repository alone does not create it ## Database and internal access 1. Provision a separate Neon project for `gateway`, with independent staging and - production branches. It holds only external API keys. Use the matching branch's + production branches. It holds external API keys, login challenges, and abuse counters. Use the matching branch's `postgresql+psycopg://` URL with `sslmode=require` as `DATABASE_URL`. Keep any URL-encoded credentials as supplied (for example `%40` for `@`). The migration runner handles Alembic's interpolation escaping internally. @@ -42,7 +42,7 @@ Add `gateway` to the existing Misty project. Configure each environment: Ensure team-tracking explicitly sets `PORT=8000` so the variable reference resolves. Deploy in the same region as the directory. `railway.json` runs migrations before starting and checks `/health/ready`. Empty/default internal keys fail startup; -an unavailable or unmigrated gateway key database fails readiness. +an unavailable or unmigrated gateway database fails readiness. The image runs the API, migrations, and key CLI as the unprivileged `gateway` user. Generate a public HTTPS domain for **gateway only**, targeting port 8000. Private @@ -80,9 +80,8 @@ next request and preserves the original revocation audit metadata on repeat call To rotate, issue a new uniquely named key, update the consumer, then revoke the old key. The CLI rejects missing scopes, unknown scopes, and `admin`. -The compute platform should receive its own key when its first operation is -defined. The currently available `resolve:discord` scope grants only the resolver; -see [future compute access](ARCHITECTURE.md#future-compute-platform-access). +The compute backend receives `auth:email` and `auth:status` for email login. +`resolve:discord` remains independent; never grant unused scopes. ## Verify the deployment @@ -98,3 +97,58 @@ Readiness verifies only the gateway DB. A resolver 503 means the private URL, internal key, directory availability, or upstream response needs investigation. Never downgrade a live key database as a deployment check; the migration round trip belongs to disposable test databases only. + +## Enable email login + +The feature is off by default; existing resolver deployments continue to work. +Use the same environment for all three services and keep verification private. + +1. Deploy verification with a dedicated `vf_` consumer key named `gateway`, scoped + only `verification:write`, using its [provisioning guide](../../verification/docs/DEPLOYMENT.md#scoped-consumer-keys). + Keep the bot's existing bootstrap credential until it is separately migrated. +2. Issue a new team-tracking key with `people:read` and `identifiers:read`, using a + fresh name (for example `gateway-email-login`). Update `DIRECTORY_API_KEY` and + revoke the replaced key after verification. Never broaden an external key into + a team-tracking key or pass it to compute. +3. Apply gateway migration **002** via the normal pre-deploy step. It adds two + tables without changing existing external keys. Readiness checks all tables. +4. Set `VERIFICATION_BASE_URL=http://${{verification.RAILWAY_PRIVATE_DOMAIN}}:${{verification.PORT}}` + and set gateway `VERIFICATION_API_KEY` to the dedicated plaintext `vf_` key. + Ensure verification explicitly sets `PORT=8000`. Then set `EMAIL_LOGIN_ENABLED=true`. + Non-local startup rejects blank/dev verification keys when enabled. +5. Issue the compute backend's external key: + + ```bash + uv --directory services/gateway --project . run python -m src.cli issue \ + --name compute --scopes auth:email auth:status + ``` + +6. Store the `gw_` key only in compute's backend secret store. Implement the + [compute integration responsibilities](API.md#compute-integration-responsibilities), + including trusted client-IP derivation, browser-session challenge binding, + secure sessions, and fail-closed active-status checks. Keep one gateway replica + and worker. Login counters are durable; the edge flood guard still is not. +7. Verify with an approved active test member: request a code, confirm once, + reject replay, retire/reactivate the test member and check status. Verify that + inactive/unknown emails deliver no code and that a resolver-only key is refused. + Do not log codes or enable an inbox endpoint. The local Docker smoke check uses + a fake sender and a deterministic generator only inside its disposable container. + +Monitor static `email login delivery failed` messages and confirm/status 503s. +Generic start responses deliberately cannot diagnose mail failures. A timed-out +send is not retried automatically. Cooldown and delivery failures retain quotas; +replacement failure leaves the previous code usable until expiry. + +The verification table gets a new opaque subject per delivered login. Schedule +its documented retention cleanup before enabling sustained traffic; gateway +pruning does not delete another service's rows. Gateway prunes expired login +records/counters on authenticated auth requests, so idle environments retain them +until activity resumes. + +To disable new authentication, set `EMAIL_LOGIN_ENABLED=false` and redeploy; all +three routes then fail closed. Revoke the compute key for immediate API access +removal. Compute must end/reject its own sessions when status checks fail. Do not +reset counters during routine deploys. For rollback, disable login and stop +in-flight requests first; migration002 can be downgraded on a backup-protected +maintenance window, discarding pending challenges/counters but preserving keys. +Do not downgrade a live database merely to test reversibility. diff --git a/services/gateway/migrations/versions/002_email_login.py b/services/gateway/migrations/versions/002_email_login.py new file mode 100644 index 00000000..04d3d5ca --- /dev/null +++ b/services/gateway/migrations/versions/002_email_login.py @@ -0,0 +1,43 @@ +"""Durable email login challenges and abuse limits. + +Revision ID: 002 +Revises: 001 +Create Date: 2026-10-26 +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects.postgresql import UUID + +revision = "002" +down_revision = "001" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "login_challenges", + sa.Column("id", UUID(as_uuid=True), primary_key=True), + sa.Column("owner", sa.Text, nullable=False), + sa.Column("email", sa.Text, nullable=False), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("expires_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("person_id", UUID(as_uuid=True), nullable=True), + sa.Column("attempts", sa.Integer, nullable=False, server_default=sa.text("0")), + sa.Column("redeemed", sa.Boolean, nullable=False, server_default=sa.text("false")), + ) + op.create_index("ix_login_challenges_expires_at", "login_challenges", ["expires_at"]) + op.create_index("ix_login_challenges_owner_email", "login_challenges", ["owner", "email"]) + op.create_table( + "login_limits", + sa.Column("bucket", sa.Text, primary_key=True), + sa.Column("count", sa.Integer, nullable=False), + sa.Column("expires_at", sa.DateTime(timezone=True), nullable=False), + ) + op.create_index("ix_login_limits_expires_at", "login_limits", ["expires_at"]) + + +def downgrade() -> None: + op.drop_table("login_limits") + op.drop_table("login_challenges") diff --git a/services/gateway/src/api/app.py b/services/gateway/src/api/app.py index 84f4e6e7..b5b48ba3 100644 --- a/services/gateway/src/api/app.py +++ b/services/gateway/src/api/app.py @@ -2,14 +2,17 @@ from fastapi import Depends, FastAPI, HTTPException, Request from fastapi.responses import JSONResponse +from fastapi.exceptions import RequestValidationError +from fastapi.exception_handlers import request_validation_exception_handler from contracts.directory import DirectoryUnavailable from contracts.storage import StorageAdapter +from contracts.login import VerificationUnavailable from src.api.deps import get_storage, lifespan -from src.api.middleware import AuditLogMiddleware +from src.api.middleware import AuditLogMiddleware, LoginBoundaryMiddleware from src.api.ratelimit import ClientRateLimitMiddleware from src.api.readiness import ReadinessProbe -from src.api.routers import resolve +from src.api.routers import resolve, email_login from src.config import get_settings, verify_production_secrets logger = logging.getLogger("gateway") @@ -33,6 +36,7 @@ def create_app() -> FastAPI: ClientRateLimitMiddleware, trust_proxy=get_settings().trust_proxy_headers, ) + app.add_middleware(LoginBoundaryMiddleware) app.add_middleware(AuditLogMiddleware, logger_name="gateway.audit") readiness_probe = ReadinessProbe() @@ -47,6 +51,23 @@ def readiness(storage: StorageAdapter = Depends(get_storage)) -> dict[str, str]: return {"status": "ok"} app.include_router(resolve.router) + app.include_router(email_login.router) + + @app.exception_handler(RequestValidationError) + async def _validation_error(request: Request, exc: RequestValidationError): + if request.url.path.startswith("/v1/auth/"): + # Default Pydantic errors echo the input, including mistyped codes. + return JSONResponse( + status_code=422, + content={"detail": "invalid login request"}, + headers={"Cache-Control": "no-store"}, + ) + return await request_validation_exception_handler(request, exc) + + @app.exception_handler(VerificationUnavailable) + async def _verification_unavailable(request: Request, exc: VerificationUnavailable): + logger.warning("verification unavailable") + return JSONResponse(status_code=503, content={"detail": "login temporarily unavailable"}) @app.exception_handler(DirectoryUnavailable) async def _directory_unavailable(request: Request, exc: DirectoryUnavailable): diff --git a/services/gateway/src/api/deps.py b/services/gateway/src/api/deps.py index 8b0c01bb..1cef5a67 100644 --- a/services/gateway/src/api/deps.py +++ b/services/gateway/src/api/deps.py @@ -9,6 +9,7 @@ from contracts.directory import DirectoryClient from contracts.storage import StorageAdapter +from contracts.login import VerificationClient from src.config import get_settings @@ -38,12 +39,17 @@ def get_storage() -> StorageAdapter: async def lifespan(app: FastAPI): app.state.directory = None app.state.directory_lock = Lock() + app.state.verification = None + app.state.verification_lock = Lock() try: yield finally: if app.state.directory is not None: await run_in_threadpool(app.state.directory.close) app.state.directory = None + if app.state.verification is not None: + await run_in_threadpool(app.state.verification.close) + app.state.verification = None def get_directory(request: Request) -> DirectoryClient: @@ -53,3 +59,16 @@ def get_directory(request: Request) -> DirectoryClient: if request.app.state.directory is None: request.app.state.directory = _build_directory() return request.app.state.directory + + +def get_verification(request: Request) -> VerificationClient: + from src.verification.http_client import HttpVerificationClient + + with request.app.state.verification_lock: + if request.app.state.verification is None: + settings = get_settings() + request.app.state.verification = HttpVerificationClient( + settings.verification_base_url, + settings.verification_api_key.get_secret_value(), + ) + return request.app.state.verification diff --git a/services/gateway/src/api/middleware.py b/services/gateway/src/api/middleware.py index ef1de7d9..7c3b9adb 100644 --- a/services/gateway/src/api/middleware.py +++ b/services/gateway/src/api/middleware.py @@ -1,3 +1,49 @@ """Thin shim re-exporting the shared audit middleware.""" from platform_auth import AuditLogMiddleware # noqa: F401 + +from starlette.responses import JSONResponse + + +class LoginBoundaryMiddleware: + """Bound login bodies before JSON parsing and prevent caching, including errors.""" + + def __init__(self, app): + self.app = app + + async def __call__(self, scope, receive, send): + if scope["type"] != "http" or not scope["path"].startswith("/v1/auth/"): + return await self.app(scope, receive, send) + + async def private_send(message): + if message["type"] == "http.response.start": + message["headers"] = [ + (key, value) + for key, value in message["headers"] + if key.lower() != b"cache-control" + ] + [(b"cache-control", b"no-store")] + await send(message) + + if scope["method"] == "POST": + chunks = [] + size = 0 + while True: + message = await receive() + if message["type"] == "http.disconnect": + return + size += len(message.get("body", b"")) + if size > 4096: + response = JSONResponse( + status_code=413, content={"detail": "request too large"} + ) + return await response(scope, receive, private_send) + chunks.append(message) + if not message.get("more_body", False): + break + pending = iter(chunks) + + async def bounded_receive(): + return next(pending, None) or await receive() + + return await self.app(scope, bounded_receive, private_send) + return await self.app(scope, receive, private_send) diff --git a/services/gateway/src/api/routers/email_login.py b/services/gateway/src/api/routers/email_login.py new file mode 100644 index 00000000..9bed7a41 --- /dev/null +++ b/services/gateway/src/api/routers/email_login.py @@ -0,0 +1,132 @@ +import logging +from uuid import UUID, uuid4 + +from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Response + +from contracts.directory import DirectoryClient +from contracts.login import ( + EmailConfirmIn, + EmailConfirmOut, + EmailStartIn, + EmailStartOut, + LoginChallenge, + PersonStatusOut, + VerificationClient, +) +from contracts.storage import StorageAdapter +from src import login_policy as policy +from src.api.auth import AuthedKey, require_scope +from src.api.deps import get_directory, get_storage, get_verification +from src.config import get_settings + +logger = logging.getLogger("gateway.login") +router = APIRouter(prefix="/v1/auth", tags=["email login"]) + + +def _enabled(): + if not get_settings().email_login_enabled: + raise HTTPException(503, "email login unavailable") + + +def _quota(storage, key, operation, client_ip=None): + _enabled() + if not storage.allow_login_request(key.name, operation, client_ip, policy.now()): + raise HTTPException(429, "rate limit exceeded", headers={"Retry-After": "60"}) + + +def _eligible(person: dict | None, email: str | None = None) -> UUID | None: + if person is None: + return None + # Reject malformed upstream data, including truthy strings for active. + from contracts.directory import DirectoryUnavailable + + try: + if not isinstance(person, dict) or not isinstance(person.get("id"), str): + raise ValueError() + person_id = UUID(person["id"]) + if type(person["active"]) is not bool or not isinstance(person["primary_email"], str): + raise ValueError() + except (KeyError, TypeError, ValueError): + raise DirectoryUnavailable("invalid directory person response") from None + if not person["active"] or (email is not None and person["primary_email"].lower() != email): + return None + return person_id + + +def _deliver(challenge, storage, directory, verification): + # Runs after the generic response has been sent. Upstream latency and failures + # must not disclose account existence. No retries after ambiguous delivery. + try: + person_id = _eligible(directory.get_person_by_email(challenge.email), challenge.email) + if person_id is None: + return + verification.request_code(f"gateway-login:{challenge.id}", challenge.email) + storage.activate_login(challenge.id, person_id, policy.now()) + except Exception: + # Exceptions may carry URLs, SQL parameters or provider response bodies. + logger.warning("email login delivery failed") + + +@router.post("/email/start", response_model=EmailStartOut, status_code=202) +def start( + payload: EmailStartIn, + background: BackgroundTasks, + response: Response, + key: AuthedKey = Depends(require_scope("auth:email")), + storage: StorageAdapter = Depends(get_storage), + directory: DirectoryClient = Depends(get_directory), + verification: VerificationClient = Depends(get_verification), +) -> EmailStartOut: + _quota(storage, key, "start", str(payload.client_ip)) + response.headers["Cache-Control"] = "no-store" + now = policy.now() + challenge = LoginChallenge( + id=uuid4(), owner=key.name, email=payload.email, created_at=now, expires_at=now + policy.TTL + ) + if storage.reserve_login(challenge): + background.add_task(_deliver, challenge, storage, directory, verification) + # Mailbox throttling returns an indistinguishable, nonredeemable challenge. + return EmailStartOut(challenge_id=challenge.id) + + +@router.post("/email/confirm", response_model=EmailConfirmOut) +def confirm( + payload: EmailConfirmIn, + response: Response, + key: AuthedKey = Depends(require_scope("auth:email")), + storage: StorageAdapter = Depends(get_storage), + directory: DirectoryClient = Depends(get_directory), + verification: VerificationClient = Depends(get_verification), +) -> EmailConfirmOut: + _quota(storage, key, "confirm", str(payload.client_ip)) + response.headers["Cache-Control"] = "no-store" + invalid = HTTPException(400, "invalid or expired login challenge") + challenge = storage.admit_login_attempt(payload.challenge_id, key.name, policy.now()) + if challenge is None: + raise invalid + # Unknown/inactive/pending challenges take the same private verification + # path: otherwise latency or an outage's 400/503 difference reveals members. + email = verification.confirm_code( + f"gateway-login:{challenge.id}", payload.code.get_secret_value() + ) + if challenge.person_id is None or email != challenge.email: + raise invalid + person_id = _eligible(directory.get_person(str(challenge.person_id)), challenge.email) + if person_id != challenge.person_id: + raise invalid + if not storage.redeem_login(challenge.id, key.name, policy.now()): + raise invalid + return EmailConfirmOut(person_id=person_id, email=challenge.email) + + +@router.get("/people/{person_id}/status", response_model=PersonStatusOut) +def status( + person_id: UUID, + response: Response, + key: AuthedKey = Depends(require_scope("auth:status")), + storage: StorageAdapter = Depends(get_storage), + directory: DirectoryClient = Depends(get_directory), +) -> PersonStatusOut: + _quota(storage, key, "status") + response.headers["Cache-Control"] = "no-store" + return PersonStatusOut(active=_eligible(directory.get_person(str(person_id))) == person_id) diff --git a/services/gateway/src/cli.py b/services/gateway/src/cli.py index f0f3935b..3d0fbe38 100644 --- a/services/gateway/src/cli.py +++ b/services/gateway/src/cli.py @@ -81,7 +81,12 @@ def build_parser() -> argparse.ArgumentParser: subs = p.add_subparsers(dest="cmd", required=True) pi = subs.add_parser("issue") pi.add_argument("--name", required=True) - pi.add_argument("--scopes", nargs="+", required=True, choices=["resolve:discord"]) + pi.add_argument( + "--scopes", + nargs="+", + required=True, + choices=["resolve:discord", "auth:email", "auth:status"], + ) pi.set_defaults(func=cmd_issue) pl = subs.add_parser("list") pl.add_argument("--active-only", action="store_true") diff --git a/services/gateway/src/config.py b/services/gateway/src/config.py index 58f326ec..53032276 100644 --- a/services/gateway/src/config.py +++ b/services/gateway/src/config.py @@ -29,6 +29,9 @@ class Settings(BaseSettings): # src/api/deps.py building the outbound header) call .get_secret_value(). directory_base_url: str = "http://localhost:8000" directory_api_key: SecretStr = SecretStr(DEFAULT_DEV_API_KEY) + email_login_enabled: bool = False + verification_base_url: str = "http://localhost:8003" + verification_api_key: SecretStr = SecretStr(DEFAULT_DEV_API_KEY) gateway_env: Literal["local", "staging", "production"] = "local" @@ -60,6 +63,12 @@ def verify_production_secrets(settings: Settings | None = None) -> None: secret = settings.directory_api_key.get_secret_value().strip() if not secret or secret == DEFAULT_DEV_API_KEY: insecure.append("DIRECTORY_API_KEY") + if settings.email_login_enabled: + verification_secret = settings.verification_api_key.get_secret_value().strip() + if not verification_secret or verification_secret == DEFAULT_DEV_API_KEY: + insecure.append("VERIFICATION_API_KEY") + if not settings.verification_base_url.strip(): + insecure.append("VERIFICATION_BASE_URL") if insecure: raise RuntimeError( f"Refusing to start in gateway_env={settings.gateway_env!r}: " diff --git a/services/gateway/src/directory/http_client.py b/services/gateway/src/directory/http_client.py index 0e83c570..a77387ca 100644 --- a/services/gateway/src/directory/http_client.py +++ b/services/gateway/src/directory/http_client.py @@ -56,6 +56,12 @@ def get_person_by_github(self, github_login: str) -> dict | None: raise DirectoryUnavailable("invalid directory person response") return result + def get_person_by_email(self, email: str) -> dict | None: + return self._get(f"/people/by-email/{quote(email, safe='')}") + + def get_person(self, person_id: str) -> dict | None: + return self._get(f"/people/{quote(person_id, safe='')}") + def list_identifiers(self, person_id: str) -> list[dict]: result = self._get(f"/people/{quote(person_id, safe='')}/identifiers") if result is None: diff --git a/services/gateway/src/login_policy.py b/services/gateway/src/login_policy.py new file mode 100644 index 00000000..29eb3fa3 --- /dev/null +++ b/services/gateway/src/login_policy.py @@ -0,0 +1,45 @@ +"""Durable fixed windows start at the first admitted action, not clock boundaries.""" + +from datetime import datetime, timedelta, timezone +from hashlib import sha256 +from ipaddress import ip_address, ip_network + +TTL = timedelta(minutes=10) +MAX_ATTEMPTS = 5 +# (bucket, maximum admissions, window seconds) +Limit = tuple[str, int, int] + + +def now() -> datetime: + return datetime.now(timezone.utc) + + +def email_key(email: str) -> str: + return sha256(email.encode()).hexdigest() + + +def request_limits(owner: str, operation: str, client_ip: str | None) -> list[Limit]: + counts = {"start": 30, "confirm": 60, "status": 60} + limits = [(f"app:{owner}:{operation}", counts[operation], 60)] + if operation == "start": + limits.append((f"app:{owner}:send-hour", 300, 3600)) + if client_ip is not None: + ip = ip_address(client_ip) + ip = getattr(ip, "ipv4_mapped", None) or ip + # Group IPv6 privacy addresses so rotating the host part does not reset limits. + network = str(ip_network(f"{ip}/64", strict=False)) if ip.version == 6 else str(ip) + digest = sha256(network.encode()).hexdigest() + limits.append((f"ip:{digest}:{operation}", 5 if operation == "start" else 30, 60)) + return limits + + +def send_limits(email: str) -> list[Limit]: + digest = email_key(email) + return [ + (f"email:{digest}:send:{seconds}", count, seconds) + for count, seconds in [(1, 60), (5, 3600), (20, 86400)] + ] + + +def guess_limits(email: str) -> list[Limit]: + return [(f"email:{email_key(email)}:guess", 15, 3600)] diff --git a/services/gateway/src/storage/in_memory.py b/services/gateway/src/storage/in_memory.py index 558abba4..c8e7f7d7 100644 --- a/services/gateway/src/storage/in_memory.py +++ b/services/gateway/src/storage/in_memory.py @@ -1,7 +1,10 @@ -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone +from threading import Lock from uuid import UUID, uuid4 from contracts.types import ApiKey +from contracts.login import LoginChallenge +from src import login_policy as policy def _now() -> datetime: @@ -12,9 +15,93 @@ class InMemoryStorageAdapter: """In-process adapter for tests. Not persistent, not thread-safe.""" def __init__(self) -> None: + self._login_lock = Lock() + self._logins: dict[UUID, LoginChallenge] = {} + self._limits: dict[str, tuple[int, datetime]] = {} self._api_keys: dict[UUID, ApiKey] = {} self._hashes: dict[UUID, str] = {} + def _spend(self, limits: list[policy.Limit], now: datetime) -> bool: + for key, maximum, _ in limits: + count, expires = self._limits.get(key, (0, now)) + if expires > now and count >= maximum: + return False + for key, _, seconds in limits: + count, expires = self._limits.get(key, (0, now)) + if expires <= now: + count, expires = 0, now + timedelta(seconds=seconds) + self._limits[key] = (count + 1, expires) + return True + + def allow_login_request(self, owner, operation, client_ip, now): + with self._login_lock: + self._logins = {k: v for k, v in self._logins.items() if v.expires_at > now} + self._limits = {k: v for k, v in self._limits.items() if v[1] > now} + return self._spend(policy.request_limits(owner, operation, client_ip), now) + + def reserve_login(self, challenge: LoginChallenge) -> bool: + with self._login_lock: + if not self._spend(policy.send_limits(challenge.email), challenge.created_at): + return False + self._logins[challenge.id] = challenge.model_copy() + return True + + def activate_login(self, challenge_id, person_id, now): + with self._login_lock: + item = self._logins.get(challenge_id) + if item is None or item.expires_at <= now or item.redeemed: + return False + if any( + v.owner == item.owner + and v.email == item.email + and v.person_id is not None + and v.created_at > item.created_at + for v in self._logins.values() + ): + return False + self._logins = { + k: v + for k, v in self._logins.items() + if not ( + v.owner == item.owner + and v.email == item.email + and v.created_at < item.created_at + ) + } + self._logins[challenge_id] = item.model_copy(update={"person_id": person_id}) + return True + + def admit_login_attempt(self, challenge_id, owner, now): + with self._login_lock: + item = self._logins.get(challenge_id) + if ( + item is None + or item.owner != owner + or item.expires_at <= now + or item.redeemed + or item.attempts >= policy.MAX_ATTEMPTS + ): + return None + if not self._spend(policy.guess_limits(item.email), now): + return None + item = item.model_copy(update={"attempts": item.attempts + 1}) + self._logins[challenge_id] = item + return item.model_copy() + + def redeem_login(self, challenge_id, owner, now): + with self._login_lock: + item = self._logins.get(challenge_id) + if ( + item is None + or item.owner != owner + or item.expires_at <= now + or item.redeemed + or item.person_id is None + ): + return False + self._logins[challenge_id] = item.model_copy(update={"redeemed": True}) + return True + def is_ready(self) -> bool: return True diff --git a/services/gateway/src/storage/postgres.py b/services/gateway/src/storage/postgres.py index 5757d5eb..da7533ef 100644 --- a/services/gateway/src/storage/postgres.py +++ b/services/gateway/src/storage/postgres.py @@ -1,13 +1,17 @@ import logging -from datetime import datetime, timezone +from contextlib import contextmanager +from datetime import datetime, timedelta, timezone from uuid import UUID -from sqlalchemy import insert, select, update +from sqlalchemy import delete, insert, select, text, update +from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.engine import Engine from sqlalchemy.exc import IntegrityError, SQLAlchemyError from contracts.types import ApiKey -from src.storage.schema import api_keys +from contracts.login import LoginChallenge +from src import login_policy as policy +from src.storage.schema import api_keys, login_challenges, login_limits logger = logging.getLogger("gateway.storage") @@ -43,11 +47,141 @@ class PostgresStorageAdapter: def __init__(self, engine: Engine) -> None: self._engine = engine + @contextmanager + def _login_transaction(self): + # One short cross-process lock for this low-volume public auth boundary. + # Never hold it across HTTP. All login writes go through this lock. + with self._engine.begin() as conn: + conn.execute(text("SELECT pg_advisory_xact_lock(71329002)")) + yield conn + + def _spend(self, conn, limits: list[policy.Limit], now: datetime) -> bool: + rows = { + r.bucket: r + for r in conn.execute( + select(login_limits).where(login_limits.c.bucket.in_([x[0] for x in limits])) + ) + } + for bucket, maximum, _ in limits: + row = rows.get(bucket) + if row is not None and row.expires_at > now and row.count >= maximum: + return False + for bucket, _, seconds in limits: + row = rows.get(bucket) + live = row is not None and row.expires_at > now + values = dict( + bucket=bucket, + count=row.count + 1 if live else 1, + expires_at=row.expires_at if live else now + timedelta(seconds=seconds), + ) + conn.execute( + pg_insert(login_limits) + .values(**values) + .on_conflict_do_update(index_elements=[login_limits.c.bucket], set_=values) + ) + return True + + def allow_login_request(self, owner, operation, client_ip, now): + with self._login_transaction() as conn: + conn.execute(delete(login_challenges).where(login_challenges.c.expires_at <= now)) + conn.execute(delete(login_limits).where(login_limits.c.expires_at <= now)) + return self._spend(conn, policy.request_limits(owner, operation, client_ip), now) + + def reserve_login(self, challenge: LoginChallenge) -> bool: + with self._login_transaction() as conn: + if not self._spend(conn, policy.send_limits(challenge.email), challenge.created_at): + return False + conn.execute(insert(login_challenges).values(**challenge.model_dump())) + return True + + def activate_login(self, challenge_id, person_id, now): + with self._login_transaction() as conn: + item = conn.execute( + select(login_challenges).where( + login_challenges.c.id == challenge_id, + login_challenges.c.expires_at > now, + login_challenges.c.redeemed.is_(False), + ) + ).one_or_none() + if item is None: + return False + newer = conn.execute( + select(login_challenges.c.id) + .where( + login_challenges.c.owner == item.owner, + login_challenges.c.email == item.email, + login_challenges.c.created_at > item.created_at, + login_challenges.c.person_id.is_not(None), + ) + .limit(1) + ).first() + if newer is not None: + return False + conn.execute( + delete(login_challenges).where( + login_challenges.c.owner == item.owner, + login_challenges.c.email == item.email, + login_challenges.c.created_at < item.created_at, + ) + ) + return ( + conn.execute( + update(login_challenges) + .where( + login_challenges.c.id == challenge_id, + login_challenges.c.expires_at > now, + login_challenges.c.redeemed.is_(False), + ) + .values(person_id=person_id) + ).rowcount + == 1 + ) + + def admit_login_attempt(self, challenge_id, owner, now): + with self._login_transaction() as conn: + row = conn.execute( + select(login_challenges).where( + login_challenges.c.id == challenge_id, + login_challenges.c.owner == owner, + login_challenges.c.expires_at > now, + login_challenges.c.redeemed.is_(False), + login_challenges.c.attempts < policy.MAX_ATTEMPTS, + ) + ).one_or_none() + if row is None or not self._spend(conn, policy.guess_limits(row.email), now): + return None + updated = conn.execute( + update(login_challenges) + .where(login_challenges.c.id == challenge_id) + .values(attempts=login_challenges.c.attempts + 1) + .returning(login_challenges) + ).one() + return LoginChallenge.model_validate(updated._mapping) + + def redeem_login(self, challenge_id, owner, now): + with self._login_transaction() as conn: + return ( + conn.execute( + update(login_challenges) + .where( + login_challenges.c.id == challenge_id, + login_challenges.c.owner == owner, + login_challenges.c.expires_at > now, + login_challenges.c.redeemed.is_(False), + login_challenges.c.person_id.is_not(None), + ) + .values(redeemed=True) + ).rowcount + == 1 + ) + def is_ready(self) -> bool: try: with self._engine.connect() as conn: # Authentication needs the migrated key table, not just a live server. conn.execute(select(api_keys.c.id).limit(1)) + conn.execute(select(login_challenges.c.id).limit(1)) + conn.execute(select(login_limits.c.bucket).limit(1)) except SQLAlchemyError: return False return True diff --git a/services/gateway/src/storage/schema.py b/services/gateway/src/storage/schema.py index 0b052512..8753c8b3 100644 --- a/services/gateway/src/storage/schema.py +++ b/services/gateway/src/storage/schema.py @@ -4,7 +4,7 @@ PostgresStorageAdapter use core-style expressions against these tables. """ -from sqlalchemy import Boolean, Column, DateTime, MetaData, Table, Text, text +from sqlalchemy import Boolean, Column, DateTime, Index, Integer, MetaData, Table, Text, text from sqlalchemy.dialects.postgresql import ARRAY, UUID metadata = MetaData() @@ -25,3 +25,25 @@ Column("revoked_at", DateTime(timezone=True), nullable=True), Column("last_used_at", DateTime(timezone=True), nullable=True), ) + +login_challenges = Table( + "login_challenges", + metadata, + Column("id", UUID(as_uuid=True), primary_key=True), + Column("owner", Text, nullable=False), + Column("email", Text, nullable=False), + Column("created_at", DateTime(timezone=True), nullable=False), + Column("expires_at", DateTime(timezone=True), nullable=False, index=True), + Column("person_id", UUID(as_uuid=True), nullable=True), + Column("attempts", Integer, nullable=False, server_default=text("0")), + Column("redeemed", Boolean, nullable=False, server_default=text("false")), + Index("ix_login_challenges_owner_email", "owner", "email"), +) + +login_limits = Table( + "login_limits", + metadata, + Column("bucket", Text, primary_key=True), + Column("count", Integer, nullable=False), + Column("expires_at", DateTime(timezone=True), nullable=False, index=True), +) diff --git a/services/gateway/src/verification/__init__.py b/services/gateway/src/verification/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/services/gateway/src/verification/http_client.py b/services/gateway/src/verification/http_client.py new file mode 100644 index 00000000..dd6302e9 --- /dev/null +++ b/services/gateway/src/verification/http_client.py @@ -0,0 +1,43 @@ +import httpx + +from contracts.login import VerificationUnavailable + + +class HttpVerificationClient: + def __init__(self, base_url: str, api_key: str, client: httpx.Client | None = None): + self._base_url = base_url.rstrip("/") + self._client = client or httpx.Client(timeout=httpx.Timeout(10.0)) + self._client.headers["X-API-Key"] = api_key + + def close(self) -> None: + self._client.close() + + def _post(self, path: str, payload: dict) -> httpx.Response: + try: + return self._client.post(f"{self._base_url}/verification/{path}", json=payload) + except httpx.HTTPError: + raise VerificationUnavailable("verification unreachable") from None + + def request_code(self, subject: str, email: str) -> None: + response = self._post("request-code", {"subject": subject, "email": email}) + if response.status_code != 202: + raise VerificationUnavailable("code delivery unavailable") + + def confirm_code(self, subject: str, code: str) -> str | None: + response = self._post("confirm-code", {"subject": subject, "code": code}) + if response.status_code in {400, 404, 410, 429}: + return None + if response.status_code != 200: + raise VerificationUnavailable("verification unavailable") + try: + data = response.json() + except ValueError: + raise VerificationUnavailable("invalid verification response") from None + if ( + not isinstance(data, dict) + or data.get("verified") is not True + or data.get("subject") != subject + or not isinstance(data.get("email"), str) + ): + raise VerificationUnavailable("invalid verification response") + return data["email"] diff --git a/services/gateway/tests/login_storage_cases.py b/services/gateway/tests/login_storage_cases.py new file mode 100644 index 00000000..016a9706 --- /dev/null +++ b/services/gateway/tests/login_storage_cases.py @@ -0,0 +1,108 @@ +from concurrent.futures import ThreadPoolExecutor +from datetime import datetime, timedelta, timezone +from uuid import uuid4 + +from contracts.login import LoginChallenge + +NOW = datetime(2026, 10, 26, tzinfo=timezone.utc) +PERSON_ID = uuid4() + + +def challenge(at=NOW, *, owner="compute", email="member@example.com"): + return LoginChallenge( + id=uuid4(), owner=owner, email=email, created_at=at, expires_at=at + timedelta(minutes=10) + ) + + +class LoginStorageCases: + def test_concurrent_reservations_send_once(self, adapter): + with ThreadPoolExecutor(max_workers=8) as pool: + results = list(pool.map(lambda _: adapter.reserve_login(challenge()), range(8))) + assert results.count(True) == 1 + + def test_limits_survive_replacement_and_are_global_per_email(self, adapter): + for minute in range(3): + item = challenge(NOW + timedelta(minutes=minute)) + assert adapter.reserve_login(item) + assert adapter.activate_login(item.id, PERSON_ID, item.created_at) + for _ in range(5): + assert adapter.admit_login_attempt(item.id, item.owner, item.created_at) + assert adapter.admit_login_attempt(item.id, item.owner, item.created_at) is None + item = challenge(NOW + timedelta(minutes=3), owner="another-app") + assert adapter.reserve_login(item) + assert adapter.activate_login(item.id, PERSON_ID, item.created_at) + assert adapter.admit_login_attempt(item.id, item.owner, item.created_at) is None + + def test_send_hour_and_day_budgets(self, adapter): + for hour in range(4): + for minute in range(5): + item = challenge(NOW + timedelta(hours=hour, minutes=minute)) + assert adapter.reserve_login(item) + assert not adapter.reserve_login(challenge(NOW + timedelta(hours=hour, minutes=6))) + assert not adapter.reserve_login(challenge(NOW + timedelta(hours=4))) + assert adapter.reserve_login(challenge(NOW + timedelta(days=1))) + + def test_expiry_and_resend_invalidate_challenges(self, adapter): + first = challenge() + assert adapter.reserve_login(first) + assert adapter.activate_login(first.id, PERSON_ID, NOW) + second = challenge(NOW + timedelta(minutes=1)) + assert adapter.reserve_login(second) + assert adapter.admit_login_attempt(first.id, first.owner, second.created_at) + assert adapter.activate_login(second.id, PERSON_ID, second.created_at) + assert adapter.admit_login_attempt(first.id, first.owner, second.created_at) is None + assert not adapter.redeem_login(first.id, first.owner, second.created_at) + assert not adapter.activate_login(second.id, PERSON_ID, second.expires_at) + assert adapter.admit_login_attempt(second.id, second.owner, second.expires_at) is None + + def test_late_delivery_cannot_replace_a_newer_active_code(self, adapter): + first = challenge() + second = challenge(NOW + timedelta(minutes=1)) + assert adapter.reserve_login(first) + assert adapter.reserve_login(second) + assert adapter.activate_login(second.id, PERSON_ID, second.created_at) + assert not adapter.activate_login(first.id, PERSON_ID, second.created_at) + assert adapter.admit_login_attempt(second.id, second.owner, second.created_at) + + def test_concurrent_attempts_and_single_redemption(self, adapter): + item = challenge() + assert adapter.reserve_login(item) + assert adapter.activate_login(item.id, PERSON_ID, NOW) + with ThreadPoolExecutor(max_workers=10) as pool: + attempts = list( + pool.map(lambda _: adapter.admit_login_attempt(item.id, item.owner, NOW), range(10)) + ) + assert sum(x is not None for x in attempts) == 5 + assert not adapter.redeem_login(item.id, "other-app", NOW) + with ThreadPoolExecutor(max_workers=10) as pool: + results = list( + pool.map(lambda _: adapter.redeem_login(item.id, item.owner, NOW), range(10)) + ) + assert results.count(True) == 1 + + def test_app_and_ip_limits_are_separate_from_status(self, adapter): + for n in range(30): + assert adapter.allow_login_request("compute", "start", f"192.0.2.{n}", NOW) + assert not adapter.allow_login_request("compute", "start", "192.0.2.99", NOW) + assert adapter.allow_login_request("compute", "status", None, NOW) + assert adapter.allow_login_request("compute", "confirm", "192.0.2.99", NOW) + assert adapter.allow_login_request( + "compute", "start", "192.0.2.99", NOW + timedelta(seconds=60) + ) + + def test_ipv6_rotation_and_mapped_ipv4_do_not_reset_quota(self, adapter): + for n in range(5): + assert adapter.allow_login_request("compute", "start", f"2001:db8::{n}", NOW) + assert not adapter.allow_login_request("other", "start", "2001:db8::99", NOW) + for _ in range(5): + assert adapter.allow_login_request("compute", "start", "192.0.2.1", NOW) + assert not adapter.allow_login_request("compute", "start", "::ffff:192.0.2.1", NOW) + + def test_expired_rows_are_pruned_without_resetting_live_email_budget(self, adapter): + item = challenge() + assert adapter.reserve_login(item) + assert adapter.allow_login_request("compute", "status", None, NOW + timedelta(minutes=11)) + assert adapter.admit_login_attempt(item.id, item.owner, NOW + timedelta(minutes=11)) is None + for minute in range(11, 15): + assert adapter.reserve_login(challenge(NOW + timedelta(minutes=minute))) + assert not adapter.reserve_login(challenge(NOW + timedelta(minutes=15))) diff --git a/services/gateway/tests/smoke_stack.py b/services/gateway/tests/smoke_stack.py index 0dc4d543..865d513e 100644 --- a/services/gateway/tests/smoke_stack.py +++ b/services/gateway/tests/smoke_stack.py @@ -1,6 +1,6 @@ """Exercise the built gateway and current directory over HTTP with disposable databases. -Run from the repo root after building gateway:ci and team-tracking:ci. Requires +Run after building gateway:ci, team-tracking:ci, and verification:ci. Requires Docker and Python's standard library; never reads .env or contacts a live service. """ @@ -56,14 +56,14 @@ def main(): containers = [] docker("network", "create", network) - def start(name, image, env, *, public=False): + def start(name, image, env, *, public=False, command=()): containers.append(name) args = ["run", "-d", "--name", name, "--network", network] if public: args += ["-p", "127.0.0.1::8000"] for field in env: args += ["-e", field] - docker(*args, image, env=env) + docker(*args, image, *command, env=env) if public: port = json.loads(docker("inspect", name))[0]["NetworkSettings"]["Ports"]["8000/tcp"][ 0 @@ -85,8 +85,58 @@ def start(name, image, env, *, public=False): ) ) docker("exec", pg, "createdb", "-U", "smoke", "gateway") + docker("exec", pg, "createdb", "-U", "smoke", "verification") directory_db = f"postgresql+psycopg://smoke:{password}@{pg}:5432/directory" gateway_db = f"postgresql+psycopg://smoke:{password}@{pg}:5432/gateway" + verification_db = f"postgresql+psycopg://smoke:{password}@{pg}:5432/verification" + verification = network + "-verification" + credential = json.loads( + docker( + "run", + "--rm", + "verification:ci", + "python", + "-c", + "import json; from platform_auth import generate_key; " + "key, prefix, digest = generate_key('vf_'); " + "print(json.dumps({'key':key,'entry':{'name':'gateway','prefix':prefix," + "'key_hash':digest,'scopes':['verification:write']}}))", + ) + ) + docker( + "run", + "--rm", + "--network", + network, + "-e", + "DATABASE_URL", + "verification:ci", + "alembic", + "upgrade", + "head", + env={"DATABASE_URL": verification_db}, + ) + verification_url = start( + verification, + "verification:ci", + { + "DATABASE_URL": verification_db, + "VF_ENV": "local", + "EMAIL_BACKEND": "fake", + "CONSUMER_KEYS": json.dumps([credential["entry"]]), + "PORT": "8000", + }, + public=True, + # Test-container-only deterministic generator, with an in-memory fake + # sender. No production test hook, code inbox, or real mail call. + command=( + "python", + "-c", + "import src.codes; src.codes.generate_code=lambda:'123456'; " + "import uvicorn; uvicorn.run('src.api.app:app',host='0.0.0.0',port=8000)", + ), + ) + wait_for(lambda: http(verification_url, "/health")[0] == 200) bootstrap = secrets.token_urlsafe(32) docker( "run", @@ -117,6 +167,7 @@ def start(name, image, env, *, public=False): "gateway", "--scopes", "identifiers:read", + "people:read", ) assert internal_key.startswith("tt_") gateway_url = start( @@ -127,6 +178,9 @@ def start(name, image, env, *, public=False): "DIRECTORY_BASE_URL": f"http://{directory}:8000", "DIRECTORY_API_KEY": internal_key, "GATEWAY_ENV": "staging", + "EMAIL_LOGIN_ENABLED": "true", + "VERIFICATION_BASE_URL": f"http://{verification}:8000", + "VERIFICATION_API_KEY": credential["key"], }, public=True, ) @@ -196,12 +250,79 @@ def start(name, image, env, *, public=False): assert missing == unlinked assert missing[0] == 404 + compute_key = docker( + "exec", + gateway, + "gateway-keys", + "issue", + "--name", + "compute", + "--scopes", + "auth:email", + "auth:status", + ) + status, challenge = http( + gateway_url, + "/v1/auth/email/start", + key=compute_key, + payload={"email": "smoke@example.test", "client_ip": "192.0.2.1"}, + ) + assert status == 202 + challenge_id = challenge["challenge_id"] + wait_for( + lambda: ( + docker( + "exec", + pg, + "psql", + "-U", + "smoke", + "-d", + "gateway", + "-Atc", + f"SELECT count(*) FROM login_challenges WHERE id='{challenge_id}' AND person_id IS NOT NULL", + ) + == "1" + ) + ) + payload = {"challenge_id": challenge_id, "code": "123456", "client_ip": "192.0.2.1"} + assert http(gateway_url, "/v1/auth/email/confirm", key=compute_key, payload=payload) == ( + 200, + {"person_id": person["id"], "email": "smoke@example.test"}, + ) + assert ( + http(gateway_url, "/v1/auth/email/confirm", key=compute_key, payload=payload)[0] == 400 + ) + status_path = f"/v1/auth/people/{person['id']}/status" + assert http(gateway_url, status_path, key=compute_key) == (200, {"active": True}) + assert ( + http( + directory_url, + f"/people/{person['id']}", + key=bootstrap, + payload={"active": False}, + method="PATCH", + )[0] + == 200 + ) + assert http(gateway_url, status_path, key=compute_key) == (200, {"active": False}) + print( + "PASS: scoped email login over private HTTP, single redemption, and retirement", + flush=True, + ) + docker("stop", directory) assert http(gateway_url, path, key=external_key) == ( 503, {"detail": "directory temporarily unavailable"}, ) - key_id = docker("exec", gateway, "gateway-keys", "list", "--active-only").split()[0] + key_id = next( + line.split()[0] + for line in docker( + "exec", gateway, "gateway-keys", "list", "--active-only" + ).splitlines() + if "smoke-consumer" in line + ) docker("exec", gateway, "gateway-keys", "revoke", key_id) assert http(gateway_url, path, key=external_key)[0] == 401 print("PASS: identical misses, upstream outage, and immediate key revocation", flush=True) diff --git a/services/gateway/tests/test_config.py b/services/gateway/tests/test_config.py index 7a130eb9..b0d7ac6a 100644 --- a/services/gateway/tests/test_config.py +++ b/services/gateway/tests/test_config.py @@ -62,3 +62,15 @@ def test_directory_key_is_redacted_in_repr(): assert "super-secret-value" not in repr(s) assert "super-secret-value" not in str(s.directory_api_key) assert s.directory_api_key.get_secret_value() == "super-secret-value" + + +@pytest.mark.parametrize("value", ["", " ", DEFAULT_DEV_API_KEY]) +def test_enabled_email_login_needs_real_verification_key(value): + with pytest.raises(RuntimeError, match="VERIFICATION_API_KEY"): + verify_production_secrets(_settings(email_login_enabled=True, verification_api_key=value)) + + +def test_verification_key_is_redacted(): + settings = _settings(verification_api_key="test-private-key") + assert "test-private-key" not in repr(settings) + assert settings.verification_api_key.get_secret_value() == "test-private-key" diff --git a/services/gateway/tests/test_email_login.py b/services/gateway/tests/test_email_login.py new file mode 100644 index 00000000..5783124b --- /dev/null +++ b/services/gateway/tests/test_email_login.py @@ -0,0 +1,342 @@ +from concurrent.futures import ThreadPoolExecutor +from uuid import uuid4 + +import pytest +from fastapi.testclient import TestClient + +from src.api import deps +from src.api.app import create_app +from src.api.hashing import generate_key +from src.config import Settings, get_settings +from src.storage.in_memory import InMemoryStorageAdapter + +EMAIL = "member@example.com" +PERSON_ID = str(uuid4()) +PERSON = {"id": PERSON_ID, "primary_email": EMAIL, "active": True} + + +class Directory: + def __init__(self): + self.person = dict(PERSON) + + def get_person_by_email(self, email): + if self.person and email == self.person["primary_email"]: + return self.person + return None + + def get_person(self, person_id): + return self.person if self.person and self.person["id"] == person_id else None + + +class Verification: + """Only replace private HTTP; retain gateway state and authentication.""" + + def __init__(self): + self.sent = {} + self.guesses = [] + + def request_code(self, subject, email): + self.sent[subject] = email + + def confirm_code(self, subject, code): + self.guesses.append((subject, code)) + return self.sent.get(subject) if code == "123456" else None + + +@pytest.fixture +def login(monkeypatch): + monkeypatch.setitem(Settings.model_config, "env_file", None) + monkeypatch.setenv("GATEWAY_ENV", "local") + monkeypatch.setenv("EMAIL_LOGIN_ENABLED", "true") + get_settings.cache_clear() + store, directory, verification = InMemoryStorageAdapter(), Directory(), Verification() + token, prefix, key_hash = generate_key() + store.create_api_key( + name="compute", + prefix=prefix, + key_hash=key_hash, + scopes=["auth:email", "auth:status"], + actor="test", + ) + app = create_app() + app.dependency_overrides[deps.get_storage] = lambda: store + app.dependency_overrides[deps.get_directory] = lambda: directory + app.dependency_overrides[deps.get_verification] = lambda: verification + with TestClient(app, headers={"X-API-Key": token}) as client: + yield client, store, directory, verification + get_settings.cache_clear() + + +def start(client, email=EMAIL, **extra): + return client.post( + "/v1/auth/email/start", json={"email": email, "client_ip": "192.0.2.1", **extra} + ) + + +def confirm(client, challenge, code="123456", **extra): + return client.post( + "/v1/auth/email/confirm", + json={"challenge_id": challenge, "code": code, "client_ip": "192.0.2.1", **extra}, + ) + + +@pytest.mark.parametrize("eligible", ["active", "missing", "inactive"]) +def test_start_is_generic_and_only_sends_to_active_people(login, eligible): + client, _, directory, verification = login + if eligible == "missing": + directory.person = None + elif eligible == "inactive": + directory.person["active"] = False + response = start(client, " MEMBER@EXAMPLE.COM ") + assert response.status_code == 202 + assert set(response.json()) == {"challenge_id", "expires_in", "message"} + assert response.json()["expires_in"] == 600 + assert EMAIL not in response.text + assert len(verification.sent) == (1 if eligible == "active" else 0) + result = confirm(client, response.json()["challenge_id"]) + assert result.status_code == (200 if eligible == "active" else 400) + if eligible == "active": + assert result.json() == {"person_id": PERSON_ID, "email": EMAIL} + + +def test_success_is_single_use_even_under_concurrent_requests(login): + client, _, _, _ = login + challenge = start(client).json()["challenge_id"] + with ThreadPoolExecutor(max_workers=8) as pool: + responses = list(pool.map(lambda _: confirm(client, challenge), range(8))) + assert sum(r.status_code == 200 for r in responses) == 1 + assert all(r.status_code in {200, 400} for r in responses) + + +@pytest.mark.parametrize("change", ["retired", "email_changed", "reassigned"]) +def test_confirmation_rechecks_original_person_and_email(login, change): + client, _, directory, _ = login + challenge = start(client).json()["challenge_id"] + if change == "retired": + directory.person["active"] = False + elif change == "email_changed": + directory.person["primary_email"] = "changed@example.com" + else: + directory.person["id"] = str(uuid4()) + response = confirm(client, challenge) + assert response.status_code == 400 + assert EMAIL not in response.text + + +def test_concurrent_wrong_guesses_cannot_exceed_five_attempts(login): + client, _, _, verification = login + challenge = start(client).json()["challenge_id"] + with ThreadPoolExecutor(max_workers=8) as pool: + responses = list(pool.map(lambda _: confirm(client, challenge, "999999"), range(8))) + assert all(r.status_code == 400 for r in responses) + assert len(verification.guesses) == 5 + assert confirm(client, challenge).status_code == 400 + + +def test_repeated_send_is_generic_and_does_not_send_again(login): + client, _, _, verification = login + first, second = start(client), start(client) + assert first.status_code == second.status_code == 202 + assert first.json()["message"] == second.json()["message"] + assert len(verification.sent) == 1 + assert confirm(client, second.json()["challenge_id"]).status_code == 400 + assert confirm(client, first.json()["challenge_id"]).status_code == 200 + + +def test_confirm_is_bound_to_the_issued_consumer(login): + client, store, _, _ = login + challenge = start(client).json()["challenge_id"] + token, prefix, key_hash = generate_key() + store.create_api_key( + name="another-app", + prefix=prefix, + key_hash=key_hash, + scopes=["auth:email"], + actor="test", + ) + response = client.post( + "/v1/auth/email/confirm", + headers={"X-API-Key": token}, + json={"challenge_id": challenge, "code": "123456", "client_ip": "192.0.2.1"}, + ) + assert response.status_code == 400 + assert confirm(client, challenge).status_code == 200 + + +@pytest.mark.parametrize( + "person,active", [(PERSON, True), ({**PERSON, "active": False}, False), (None, False)] +) +def test_status_is_minimal_and_checks_current_state(login, person, active): + client, _, directory, _ = login + directory.person = person + response = client.get(f"/v1/auth/people/{PERSON_ID}/status") + assert response.status_code == 200 + assert response.json() == {"active": active} + assert response.headers["cache-control"] == "no-store" + + +@pytest.mark.parametrize( + "endpoint,body", + [ + ("/v1/auth/email/start", {"email": EMAIL, "client_ip": "192.0.2.1"}), + ( + "/v1/auth/email/confirm", + {"challenge_id": str(uuid4()), "code": "123456", "client_ip": "192.0.2.1"}, + ), + ], +) +def test_wrong_scope_stops_before_upstream_construction(login, endpoint, body): + client, store, _, _ = login + token, prefix, key_hash = generate_key() + store.create_api_key( + name="resolve-only", + prefix=prefix, + key_hash=key_hash, + scopes=["resolve:discord"], + actor="test", + ) + + def unexpected(): + raise AssertionError("upstream was constructed before scope validation") + + client.app.dependency_overrides[deps.get_directory] = unexpected + client.app.dependency_overrides[deps.get_verification] = unexpected + assert client.post(endpoint, headers={"X-API-Key": token}, json=body).status_code == 403 + + +def test_strict_inputs_and_no_input_echo_in_errors(login): + client, _, _, verification = login + assert start(client, subject="attacker-chosen").status_code == 422 + response = confirm(client, str(uuid4()), "secret-typed-by-mistake") + assert response.status_code == 422 + assert "secret-typed-by-mistake" not in response.text + assert not verification.sent + + +def test_disabled_login_fails_closed(login, monkeypatch): + client, _, _, verification = login + monkeypatch.setenv("EMAIL_LOGIN_ENABLED", "false") + get_settings.cache_clear() + assert start(client).status_code == 503 + assert not verification.sent + + +def test_large_body_rejected_before_dependencies(login): + client, _, _, _ = login + + def unexpected(): + raise AssertionError("large bodies must be rejected before upstream work") + + client.app.dependency_overrides[deps.get_directory] = unexpected + response = client.post( + "/v1/auth/email/start", content=b"x" * 4097, headers={"Content-Type": "application/json"} + ) + assert response.status_code == 413 + + +def test_upstream_failures_do_not_leak_details(login, caplog): + from contracts.directory import DirectoryUnavailable + from contracts.login import VerificationUnavailable + + client, _, directory, verification = login + + def unavailable(*args): + raise VerificationUnavailable("private-secret-provider-response") + + verification.request_code = unavailable + result = start(client) + assert result.status_code == 202 + assert "private-secret-provider-response" not in caplog.text + + def down(*args): + raise DirectoryUnavailable("directory unavailable") + + directory.get_person = down + response = client.get(f"/v1/auth/people/{PERSON_ID}/status") + assert response.status_code == 503 + assert response.headers["cache-control"] == "no-store" + + +def test_wrong_verification_email_cannot_authenticate(login): + client, _, _, verification = login + challenge = start(client).json()["challenge_id"] + verification.confirm_code = lambda subject, code: "other@example.com" + assert confirm(client, challenge).status_code == 400 + + +def test_bad_directory_shape_fails_closed(login): + client, _, directory, verification = login + directory.person["active"] = "true" + assert start(client).status_code == 202 + assert not verification.sent + assert client.get(f"/v1/auth/people/{PERSON_ID}/status").status_code == 503 + + +def test_login_start_response_is_sent_before_mail_delivery(login): + # Observe ASGI send order; TestClient itself waits for background tasks. + client, _, _, verification = login + app = client.app + response_sent = [] + + async def observed(scope, receive, send): + async def record(message): + if message["type"] == "http.response.body" and not message.get("more_body", False): + response_sent.append(True) + await send(message) + + await app(scope, receive, record) + + original = verification.request_code + + def deliver(subject, email): + assert response_sent + original(subject, email) + + verification.request_code = deliver + with TestClient(observed, headers=client.headers) as other: + assert start(other).status_code == 202 + assert len(verification.sent) == 1 + + +def test_failed_resend_keeps_previous_code_usable(login, monkeypatch): + from datetime import timedelta + from contracts.login import VerificationUnavailable + from src import login_policy + + client, _, _, verification = login + first = start(client).json()["challenge_id"] + later = login_policy.now() + timedelta(seconds=61) + monkeypatch.setattr(login_policy, "now", lambda: later) + + def failed(*args): + raise VerificationUnavailable("delivery rejected") + + verification.request_code = failed + second = start(client).json()["challenge_id"] + assert confirm(client, second).status_code == 400 + assert confirm(client, first).status_code == 200 + + +@pytest.mark.parametrize("bad_id", [1, True, [], {}]) +def test_invalid_upstream_id_returns_sanitized_503(login, bad_id): + client, _, directory, _ = login + directory.get_person = lambda _: {**PERSON, "id": bad_id} + assert client.get(f"/v1/auth/people/{PERSON_ID}/status").status_code == 503 + + +def test_missing_and_active_challenges_share_verification_failure_behavior(login): + from contracts.login import VerificationUnavailable + + client, _, directory, verification = login + active = start(client).json()["challenge_id"] + directory.person = None + missing = start(client, "missing@example.com").json()["challenge_id"] + + def unavailable(*args): + raise VerificationUnavailable("upstream down") + + verification.confirm_code = unavailable + first = confirm(client, active, "999999") + second = confirm(client, missing, "999999") + assert first.status_code == second.status_code == 503 + assert first.json() == second.json() diff --git a/services/gateway/tests/test_login_storage.py b/services/gateway/tests/test_login_storage.py new file mode 100644 index 00000000..d2822ebc --- /dev/null +++ b/services/gateway/tests/test_login_storage.py @@ -0,0 +1,13 @@ +import pytest + +from login_storage_cases import LoginStorageCases +from src.storage.in_memory import InMemoryStorageAdapter + + +@pytest.fixture +def adapter(): + return InMemoryStorageAdapter() + + +class TestLoginStorage(LoginStorageCases): + pass diff --git a/services/gateway/tests/test_postgres_adapter.py b/services/gateway/tests/test_postgres_adapter.py index c711b0ea..7501a0ef 100644 --- a/services/gateway/tests/test_postgres_adapter.py +++ b/services/gateway/tests/test_postgres_adapter.py @@ -10,6 +10,7 @@ from src.storage.postgres import PostgresStorageAdapter from src.storage.schema import api_keys from storage_cases import StorageCases +from login_storage_cases import LoginStorageCases pytestmark = pytest.mark.skipif( os.environ.get("RUN_PG_TESTS") != "1", reason="set RUN_PG_TESTS=1 to run Postgres tests" @@ -26,8 +27,12 @@ def engine(): @pytest.fixture def adapter(engine): + from src.storage.schema import login_challenges, login_limits + with engine.begin() as conn: conn.execute(delete(api_keys)) + conn.execute(delete(login_challenges)) + conn.execute(delete(login_limits)) return PostgresStorageAdapter(engine) @@ -35,6 +40,18 @@ class TestPostgresStorage(StorageCases): pass +class TestPostgresLoginStorage(LoginStorageCases): + pass + + +def test_login_quotas_survive_a_new_adapter(engine, adapter): + from login_storage_cases import challenge + + assert adapter.reserve_login(challenge()) + restarted = PostgresStorageAdapter(engine) + assert not restarted.reserve_login(challenge()) + + def test_repeated_revocation_preserves_audit_metadata(adapter, engine): key = adapter.create_api_key( name="consumer", prefix="prefix", key_hash="HASH", scopes=[], actor="issuer" diff --git a/services/gateway/tests/test_verification.py b/services/gateway/tests/test_verification.py new file mode 100644 index 00000000..c0ccfcc4 --- /dev/null +++ b/services/gateway/tests/test_verification.py @@ -0,0 +1,71 @@ +import json + +import httpx +import pytest + +from contracts.login import VerificationUnavailable +from src.verification.http_client import HttpVerificationClient + + +def client(handler): + return HttpVerificationClient( + "http://verification.test", + "private-test-key", + client=httpx.Client(transport=httpx.MockTransport(handler)), + ) + + +def test_private_request_contract_and_secret_unwrap(): + calls = [] + + def handle(request): + assert request.headers["X-API-Key"] == "private-test-key" + calls.append((request.url.path, json.loads(request.content))) + if request.url.path.endswith("request-code"): + return httpx.Response(202, json={"status": "sent"}) + return httpx.Response(200, json={"verified": True, "subject": "opaque", "email": "a@b.com"}) + + upstream = client(handle) + upstream.request_code("opaque", "a@b.com") + assert upstream.confirm_code("opaque", "123456") == "a@b.com" + assert calls == [ + ("/verification/request-code", {"subject": "opaque", "email": "a@b.com"}), + ("/verification/confirm-code", {"subject": "opaque", "code": "123456"}), + ] + upstream.close() + + +@pytest.mark.parametrize("status", [400, 404, 410, 429]) +def test_invalid_code_status_is_not_an_identity(status): + upstream = client(lambda _: httpx.Response(status, text="private-response")) + assert upstream.confirm_code("opaque", "123456") is None + upstream.close() + + +@pytest.mark.parametrize( + "status,data", + [ + (200, {"verified": True, "subject": "other", "email": "a@b.com"}), + (200, {"verified": "true", "subject": "opaque", "email": "a@b.com"}), + (200, None), + (401, {"detail": "private-response"}), + (500, {"detail": "private-response"}), + ], +) +def test_malformed_or_failed_verification_is_sanitized(status, data): + upstream = client(lambda _: httpx.Response(status, json=data)) + with pytest.raises(VerificationUnavailable) as exc: + upstream.confirm_code("opaque", "123456") + assert "private-response" not in str(exc.value) + upstream.close() + + +def test_transport_failures_do_not_leak_request_data(): + def broken(request): + raise httpx.ConnectError("private-request-data", request=request) + + upstream = client(broken) + with pytest.raises(VerificationUnavailable) as exc: + upstream.request_code("opaque", "a@b.com") + assert "private-request-data" not in str(exc.value) + upstream.close() diff --git a/services/verification/.env.example b/services/verification/.env.example index a1d68f4c..86a95b49 100644 --- a/services/verification/.env.example +++ b/services/verification/.env.example @@ -1,5 +1,7 @@ DATABASE_URL=postgresql+psycopg://verification:dev_password@localhost:5434/verification API_KEY=dev-api-key-change-me +# Optional scoped vf_ consumers; JSON entries hold only argon2 hashes. +CONSUMER_KEYS=[] CODE_HMAC_SECRET=dev-hmac-secret-change-me VF_ENV=local diff --git a/services/verification/README.md b/services/verification/README.md index 6578a232..ac22d7b8 100644 --- a/services/verification/README.md +++ b/services/verification/README.md @@ -65,12 +65,25 @@ injected sender (see [local setup](docs/CONTRIBUTING.md#local-setup)). ## Auth model -Auth is provided by the shared `platform_auth` package (`build_auth` in `src/api/auth.py`), configured **env-key-only**: - -- Both verification POST endpoints require `X-API-Key`. The only accepted key is the shared bootstrap env key (`API_KEY`), which carries the `admin` scope. There is **no key-issuing CLI and no DB-backed keys** here — `NullApiKeyStore` (`src/storage/null_keys.py`) short-circuits all key lookups, so the `vf__` envelope is reserved but unused. The env key must therefore be a plain opaque string, not shaped like that envelope. -- `require_scope("verification:write")` gates both verification endpoints. The scopes recognized are **`verification:write`** and **`admin`** (a wildcard that satisfies any check); the env key's `admin` scope covers both. -- `/health`, `/openapi.json` and the interactive API documentation routes are unauthenticated. -- `AuditLogMiddleware` (from `platform_auth`, logger `verification.audit`) logs one line per request. Authenticated requests have `key_name=env-bootstrap`; `X-Actor` does not change attribution on these routes. See the [authentication caveat](docs/API.md#authentication). `dev:spoof` is disabled. +Auth uses shared `platform_auth` with two supported credential paths: + +- Scoped `vf_` keys come from `CONSUMER_KEYS` (a SecretStr JSON array of + `{name,prefix,key_hash,scopes}`). Store argon2 hashes, never plaintext keys in + that array. The gateway gets its own key scoped `verification:write`. +- The existing bootstrap `API_KEY` still carries `admin`, preserving the bot flow. + It must be a plain opaque string, not a `vf_` envelope. +- Both POSTs require `verification:write`; `/health` and API docs are unauthenticated. +- Audit records the authenticated key's name, never codes or request bodies. + `dev:spoof` is disabled and `X-Actor` does not alter endpoint attribution. + +| Additional setting | Default | Purpose | +|---|---|---| +| `CONSUMER_KEYS` | Empty | Scoped private consumers; malformed JSON fails at boot. | + +See [key provisioning and rotation](docs/DEPLOYMENT.md#scoped-consumer-keys). +There is no key table or dedicated key CLI. The existing verification state +machine, including correct-code replay for Discord, is unchanged; the gateway +adds single-use login redemption and eligibility checks around private HTTP calls. ## API at a glance @@ -138,7 +151,7 @@ verification/ ├── src/ │ ├── api/ FastAPI application │ │ ├── app.py create_app(): verify_production_secrets, audit middleware, /health, router -│ │ ├── auth.py build_auth() over platform_auth: require_scope, env-key-only (envelope "vf_") +│ │ ├── auth.py build_auth() over platform_auth: require_scope, scoped + bootstrap (envelope "vf_") │ │ ├── deps.py get_storage / get_email_sender / get_key_store wiring │ │ └── routers/ │ │ └── verification.py request-code + confirm-code endpoints @@ -146,8 +159,7 @@ verification/ │ ├── storage/ VerificationStore implementations │ │ ├── schema.py verification_codes table (SQLAlchemy Core) │ │ ├── in_memory.py InMemoryVerificationStore — tests -│ │ ├── postgres.py PostgresVerificationStore — production (atomic upsert + increment) -│ │ └── null_keys.py NullApiKeyStore — env-key-only auth (no DB keys) +│ │ └── postgres.py PostgresVerificationStore — production (atomic upsert + increment) │ │ │ ├── email/ EmailSender implementations + registry │ │ ├── base.py EmailSender Protocol + EmailSendError diff --git a/services/verification/docs/API.md b/services/verification/docs/API.md index b4050db0..db0af87f 100644 --- a/services/verification/docs/API.md +++ b/services/verification/docs/API.md @@ -11,11 +11,15 @@ Base URL (local): `http://localhost:8003` · Swagger UI: `/docs` · Schema: `/op ## Authentication ``` -X-API-Key: +X-API-Key: ``` -verification is **env-key-only**. There is no key-issuing CLI and no DB-backed keys — `NullApiKeyStore` short-circuits key lookups, so the only accepted credential is the bootstrap `API_KEY`, which carries the `admin` scope. The `vf_` envelope is reserved but unused, so **the env key must be a plain opaque string, not shaped like `vf__`.** - +Private consumers may authenticate with scoped `vf_` keys configured through +`CONSUMER_KEYS`. The gateway uses a dedicated `verification:write` credential. +The existing bootstrap `API_KEY` remains supported with `admin`; it must be a +plain opaque string, not shaped like `vf__`. See +[provisioning](DEPLOYMENT.md#scoped-consumer-keys). No keys are stored in this +service's database. Both endpoints are gated by `require_scope("verification:write")`; `admin` satisfies it. `/health` is unauthenticated. `dev:spoof` is disabled. diff --git a/services/verification/docs/ARCHITECTURE.md b/services/verification/docs/ARCHITECTURE.md index 912854e7..e3a29aab 100644 --- a/services/verification/docs/ARCHITECTURE.md +++ b/services/verification/docs/ARCHITECTURE.md @@ -110,11 +110,19 @@ Two caveats worth knowing rather than assuming: - **No test actually exercises concurrency.** The increment tests in both adapter suites are sequential, so a read-modify-write regression in `postgres.py` would pass them. The atomicity here rests on the single `UPDATE ... RETURNING` statement, not on test coverage — which is exactly why the statement shape matters and shouldn't be "simplified". - **`InMemoryVerificationStore` is a read → `model_copy` → write**, i.e. the shape this section calls disqualifying. That's acceptable because it backs single-threaded tests, never production — but don't read it as the reference implementation of this invariant. `postgres.py` is. -## Auth: env-key-only, on purpose - -verification is the one service with a database but **no key table**. `NullApiKeyStore` short-circuits every key lookup, so only the bootstrap `API_KEY` authenticates. The `vf_` envelope is reserved but unused — which means the env key must be a plain opaque string, *not* shaped like that envelope. - -The reasoning: there is exactly one consumer (the Discord bot). Issuing per-consumer keys would be ceremony without benefit — a CLI, a table, a migration, and a revoke path, all to manage one key. If a second consumer ever appears, a real `ApiKeyStore` drops in behind the same protocol with no route changes. +## Auth: scoped consumers and bootstrap compatibility + +verification has a database but no key table. Its `CONSUMER_KEYS` configuration +seeds an `InMemoryKeyStore` with argon2-hashed, scoped `vf_` keys at boot. This +supports a distinct gateway identity without sharing the bot's bootstrap key. +Malformed configuration prevents startup. Removing a consumer and redeploying +revokes it; adding a second key supports an overlap window during rotation. + +The bootstrap `API_KEY` remains for existing bot compatibility. It carries `admin` +and must not resemble the `vf_` envelope. Both endpoints still require +`verification:write`. The gateway is responsible for login-specific eligibility, +concurrency admission, and single-use redemption; this service's confirm state +machine remains unchanged for existing callers. ## Email backends diff --git a/services/verification/docs/DEPLOYMENT.md b/services/verification/docs/DEPLOYMENT.md index f0be45ea..2f210180 100644 --- a/services/verification/docs/DEPLOYMENT.md +++ b/services/verification/docs/DEPLOYMENT.md @@ -12,7 +12,7 @@ Service-specific deploy notes. The full platform runbook is [`docs/RAILWAY-DEPLO | **Install** | `uv sync --frozen --no-dev --package verification` | | **Healthcheck** | `/health` | | **Port** | Binds Railway's injected `${PORT}`; 8003 locally | -| **Exposure** | Private. Reached by the bot as `verification.railway.internal:${PORT}`. | +| **Exposure** | Private. Reached by the bot and gateway as `verification.railway.internal:${PORT}`. | | **State** | One table of persistent rows containing short-lived codes. See [Data retention](#data-retention). | ## Variables @@ -22,6 +22,7 @@ Service-specific deploy notes. The full platform runbook is [`docs/RAILWAY-DEPLO | `VF_ENV` | `staging` / `production` | Anything but `local` turns on the boot guard | | `DATABASE_URL` | Neon connection string | Must not be the `dev_password@localhost` default | | `API_KEY` | a real random string | **Plain opaque string** — not shaped like `vf__` | +| `CONSUMER_KEYS` | optional scoped `vf_` entries | JSON with `name`, `prefix`, `key_hash`, `scopes`; validated at boot | | `CODE_HMAC_SECRET` | a real random string | Must differ from the dev default | | `EMAIL_BACKEND` | `resend` or `gmail` | **`fake` is refused outside `local`** | | `EMAIL_FROM` | the sending address | Required by `resend` | @@ -45,16 +46,26 @@ delivery prevents normal user completion. Real-backend checks validate presence of credentials, not their contents or provider access; see [request-code errors](API.md#errors). -## No key provisioning +## Scoped consumer keys -verification is the only service with **nothing to provision**. There is no `api_keys` table, no CLI, no `CONSUMER_KEYS` array. `NullApiKeyStore` short-circuits key lookups and only the bootstrap `API_KEY` authenticates. +Set `CONSUMER_KEYS` to a JSON array of `{name,prefix,key_hash,scopes}` entries. +For the gateway, generate a `vf_` token using the shared library's +`generate_key("vf_")` from the verification service environment. Store the returned +plaintext only in gateway `VERIFICATION_API_KEY`; add the returned prefix/hash +with `name: gateway` and `scopes: ["verification:write"]` to verification's array. +Follow the [shared config-backed key procedure](../../../packages/auth/README.md#key-store-wiring) +for the format. Use a secure operator script/secret store; do not paste token or +hash material into chat, logs, or source control. -So wiring the bot to verification is just: +There is no key database or CLI here. Rotation is: add a uniquely named second +entry, redeploy verification, update gateway, verify, then remove the old entry +and redeploy verification. This allows overlap. Keep keys separate per environment. -1. Set a real random `API_KEY` on the verification service. -2. Set the same value as `VERIFICATION_API_KEY` on the discord-bot, plus `VERIFICATION_BASE_URL=http://${{verification.RAILWAY_PRIVATE_DOMAIN}}:${{verification.PORT}}` — the `${{service.VAR}}` reference form, since a literal `${PORT}` would resolve to the *bot's* own port. - -**Rotating it is a two-service, ordered operation.** Both services must be updated, and there's no overlap window — the old key stops working the moment verification restarts. Do it when `/link` traffic is quiet, and update the bot first or expect a brief window of failed link attempts. +The existing bot bootstrap path remains: verification `API_KEY` equals the bot's +`VERIFICATION_API_KEY`, and the bot's base URL is +`http://${{verification.RAILWAY_PRIVATE_DOMAIN}}:${{verification.PORT}}`. +Do not give this bootstrap key to new consumers. Its rotation still requires +coordinated updates because it has no overlap window. ## Migrations @@ -75,7 +86,9 @@ request, whether confirmed or not. Rows are superseded on reissue (unique `subject` + upsert), so the table grows with distinct subjects that have had stored requests. Code expiry does not remove the email/subject association. -**Nothing prunes consumed or expired rows.** There is no cleanup job. Each row holds an email address and an HMAC — no plaintext code — but it is still personal data sitting there indefinitely. If retention becomes a concern, a periodic `DELETE FROM verification_codes WHERE expires_at < now() - interval '30 days'` is safe: the confirm path treats a missing row and an expired-consumed row identically (both 404 `no_pending_code`). +**Nothing automatically prunes consumed or expired rows.** Gateway login uses a +new subject per delivered challenge, so schedule the cleanup below before sustained +external login traffic. There is no cleanup job. Each row holds an email address and an HMAC — no plaintext code — but it is still personal data sitting there indefinitely. If retention becomes a concern, a periodic `DELETE FROM verification_codes WHERE expires_at < now() - interval '30 days'` is safe: the confirm path treats a missing row and an expired-consumed row identically (both 404 `no_pending_code`). `CODE_HMAC_SECRET` rotation invalidates every in-flight code. With a 10-minute TTL the blast radius is 10 minutes, so it's safe to rotate whenever you like — no coordination needed. diff --git a/services/verification/src/api/app.py b/services/verification/src/api/app.py index 441295b9..3e324be5 100644 --- a/services/verification/src/api/app.py +++ b/services/verification/src/api/app.py @@ -3,10 +3,12 @@ from platform_auth import AuditLogMiddleware from src.config import verify_production_secrets +from src.api.deps import get_key_store def create_app() -> FastAPI: verify_production_secrets() + get_key_store() # Validate scoped credentials at boot, before serving requests. app = FastAPI( title="UTMIST verification", diff --git a/services/verification/src/api/deps.py b/services/verification/src/api/deps.py index 952d7e20..87852a9c 100644 --- a/services/verification/src/api/deps.py +++ b/services/verification/src/api/deps.py @@ -1,5 +1,7 @@ from functools import lru_cache +from platform_auth import InMemoryKeyStore, key_store_from_config + from sqlalchemy import create_engine from sqlalchemy.engine import Engine @@ -7,7 +9,6 @@ from src.config import get_settings from src.email.base import EmailSender from src.email.fake import FakeSender -from src.storage.null_keys import NullApiKeyStore @lru_cache(maxsize=1) @@ -22,8 +23,13 @@ def get_storage() -> VerificationStore: return PostgresVerificationStore(_default_engine()) -def get_key_store() -> NullApiKeyStore: - return NullApiKeyStore() +@lru_cache(maxsize=1) +def _key_store() -> InMemoryKeyStore: + return key_store_from_config(get_settings().consumer_keys.get_secret_value()) + + +def get_key_store() -> InMemoryKeyStore: + return _key_store() def get_email_sender() -> EmailSender: diff --git a/services/verification/src/config.py b/services/verification/src/config.py index a1ae2acc..cc343b93 100644 --- a/services/verification/src/config.py +++ b/services/verification/src/config.py @@ -16,6 +16,7 @@ class Settings(BaseSettings): database_url: str = "postgresql+psycopg://verification:dev_password@localhost:5434/verification" api_key: SecretStr = SecretStr(DEFAULT_DEV_API_KEY) + consumer_keys: SecretStr = SecretStr("") code_hmac_secret: SecretStr = SecretStr(DEFAULT_DEV_HMAC_SECRET) vf_env: Literal["local", "staging", "production"] = "local" email_backend: Literal["fake", "resend", "gmail"] = "fake" diff --git a/services/verification/src/storage/null_keys.py b/services/verification/src/storage/null_keys.py deleted file mode 100644 index e7128fda..00000000 --- a/services/verification/src/storage/null_keys.py +++ /dev/null @@ -1,22 +0,0 @@ -from uuid import UUID - -from platform_auth import ApiKeyRow - - -class NullApiKeyStore: - """Satisfies platform_auth's ApiKeyStore for env-key-only auth. - - The shared env `API_KEY` must be a plain opaque string, NOT shaped like the - `vf__` DB-key envelope — the env-key path only matches - non-envelope keys. DB-backed keys are unsupported here, so all lookups - below return None. - """ - - def get_api_key_hash(self, prefix: str) -> str | None: - return None - - def get_api_key_by_prefix(self, prefix: str) -> ApiKeyRow | None: - return None - - def touch_api_key_last_used(self, api_key_id: UUID) -> None: - return None diff --git a/services/verification/tests/conftest.py b/services/verification/tests/conftest.py index b1a7edb8..7d476bf9 100644 --- a/services/verification/tests/conftest.py +++ b/services/verification/tests/conftest.py @@ -41,10 +41,13 @@ def _clear_settings_cache(_no_dotenv): # get_settings() is lru_cached, so a Settings built by an earlier test (or # at app-import time, before _no_dotenv could run) would otherwise persist. from src.config import get_settings + from src.api.deps import _key_store get_settings.cache_clear() + _key_store.cache_clear() yield get_settings.cache_clear() + _key_store.cache_clear() @pytest.fixture diff --git a/services/verification/tests/test_consumer_auth.py b/services/verification/tests/test_consumer_auth.py new file mode 100644 index 00000000..a7f7ed2d --- /dev/null +++ b/services/verification/tests/test_consumer_auth.py @@ -0,0 +1,40 @@ +import json + +import pytest +from fastapi.testclient import TestClient +from platform_auth import generate_key + +from src.api.app import create_app +from src.api.deps import get_email_sender, get_storage +from src.config import get_settings +from src.email.fake import FakeSender +from src.storage.in_memory import InMemoryVerificationStore + + +@pytest.mark.parametrize("scopes,expected", [(["verification:write"], 202), ([], 403)]) +def test_scoped_gateway_key_can_send_without_bootstrap(monkeypatch, scopes, expected): + token, prefix, key_hash = generate_key("vf_") + monkeypatch.setenv( + "CONSUMER_KEYS", + json.dumps([{"name": "gateway", "prefix": prefix, "key_hash": key_hash, "scopes": scopes}]), + ) + get_settings.cache_clear() + sender, store = FakeSender(), InMemoryVerificationStore() + app = create_app() + app.dependency_overrides[get_storage] = lambda: store + app.dependency_overrides[get_email_sender] = lambda: sender + with TestClient(app) as client: + result = client.post( + "/verification/request-code", + headers={"X-API-Key": token}, + json={"subject": "gateway-login:test", "email": "member@example.com"}, + ) + assert result.status_code == expected + assert (store.get_code("gateway-login:test") is not None) == (expected == 202) + + +def test_bad_consumer_config_is_rejected_at_boot(monkeypatch): + monkeypatch.setenv("CONSUMER_KEYS", "{bad-json") + get_settings.cache_clear() + with pytest.raises(RuntimeError, match="CONSUMER_KEYS"): + create_app() From 15dbfe95d5aac4b2fbf13999f82ed9aedec62976 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Mon, 26 Oct 2026 01:04:03 -0400 Subject: [PATCH 2/5] fix(gateway): isolate login challenges and attest environment --- AGENTS.md | 1 + docs/RAILWAY-DEPLOYMENT.md | 2 + services/gateway/README.md | 4 +- services/gateway/contracts/login.py | 12 +++-- services/gateway/contracts/storage.py | 9 ++-- services/gateway/docs/API.md | 45 ++++++++++++----- services/gateway/docs/ARCHITECTURE.md | 11 ++++- services/gateway/docs/CHANGE-AUDIT.md | 24 ++++++++-- services/gateway/docs/DEPLOYMENT.md | 9 +++- .../gateway/src/api/routers/email_login.py | 24 ++++++++-- services/gateway/src/login_policy.py | 8 ++-- services/gateway/src/storage/in_memory.py | 19 -------- services/gateway/src/storage/postgres.py | 21 +------- services/gateway/tests/login_storage_cases.py | 31 ++++++++---- services/gateway/tests/smoke_stack.py | 12 +++-- services/gateway/tests/test_email_login.py | 48 +++++++++++++++++-- 16 files changed, 184 insertions(+), 96 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d7d7eeea..080d8c9d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,6 +117,7 @@ Run what CI runs for that service — `.github/workflows/ci.yml` is authoritativ - **Gateway provisioning must pin the import path too.** After installing all workspace members, `uv --project` alone can still import another service's `src.cli`. From the repo root use `uv --directory services/gateway --project . run python -m src.cli ...` (and the team-tracking directory for its internal key). The isolated containers can use their installed key CLI directly. - **Gateway is the public entry point.** It accepts only issued `gw_` keys, has no env-bootstrap `API_KEY`, and owns its key registry plus durable login challenges/abuse counters. Compute-platform data access must use explicit scoped HTTP operations; see `services/gateway/docs/ARCHITECTURE.md`. Run one replica/worker for its process-local rate limits. - **Gateway email login stays on Misty.** Verification owns code generation/send/check; gateway owns eligibility, consumer-bound challenges, durable quotas, and atomic single-use redemption. Preserve Discord’s existing correct-code replay. The compute backend attests `client_ip` from trusted ingress and must bind challenges to browser sessions; it never gets a verification key. See `services/gateway/docs/API.md#email-login`. +- **Misty login issuers are environment-bound.** Successful auth responses carry `misty:`. Compute verifies and pins issuer plus gateway origin to its database; existing compute accounts need explicit UUID mappings. Do not restore email-only account claiming or a shared mailbox guess lockout; the gateway uses three guesses per independent challenge and five deliveries per mailbox window. - **Gateway proxy trust is specific to `X-Real-IP`.** Railway's public edge overwrites it; `X-Forwarded-For` may include a shared proxy hop and is ignored. Health probes skip the IP quota, so preserve the per-app readiness cache and nonblocking probe lock; see `services/gateway/docs/API.md#health`. - **documentation-system and verification both bind host port 5434.** They cannot run locally at the same time. Remap one to an unused port (gateway uses 5435 by default) and update its `DATABASE_URL`. If Alembic reports an unknown revision, you're almost certainly pointed at the other service's database. - **Discovery imports; refresh repairs.** `doc-discover` enumerates new Google file identities over authenticated HTTP, with a persistent checkpoint volume and `discover` connector scope. It never grants access from listing metadata, reactivates retired docs, or refetches existing docs. See [discovery operations](services/documentation-system/docs/DISCOVERY.md); keep #292 refresh separate. diff --git a/docs/RAILWAY-DEPLOYMENT.md b/docs/RAILWAY-DEPLOYMENT.md index 6f2b652b..87732d1a 100644 --- a/docs/RAILWAY-DEPLOYMENT.md +++ b/docs/RAILWAY-DEPLOYMENT.md @@ -24,6 +24,8 @@ consumer keys, proxy settings, and verification are covered in its Email login requires gateway migration002, `EMAIL_LOGIN_ENABLED`, private `VERIFICATION_BASE_URL`/`VERIFICATION_API_KEY`, a scoped `vf_` entry in verification `CONSUMER_KEYS`, directory `people:read`, and compute `auth:email`/`auth:status`. +Set gateway `GATEWAY_ENV` explicitly; compute verifies the matching issuer and +pins its gateway origin/environment to a separate database per environment. Follow [enablement](../services/gateway/docs/DEPLOYMENT.md#enable-email-login) in order. For the optional persistent `dev` environment used to test one PR at a diff --git a/services/gateway/README.md b/services/gateway/README.md index 0de5fed1..bffb15df 100644 --- a/services/gateway/README.md +++ b/services/gateway/README.md @@ -72,7 +72,7 @@ schema is at `http://localhost:8006/openapi.json`. | `EMAIL_LOGIN_ENABLED` | `false` by default. Enable after provisioning private verification and directory access. | | `VERIFICATION_BASE_URL` | Private verification URL; local default `http://localhost:8003`. | | `VERIFICATION_API_KEY` | A distinct scoped `vf_` key; blank/dev values are refused outside local mode when login is enabled. | -| `GATEWAY_ENV` | `local` / `staging` / `production`. In non-`local` envs, startup refuses to boot if `DIRECTORY_API_KEY` is blank or still the built-in dev default. | +| `GATEWAY_ENV` | `local` / `staging` / `production`; determines the attested login issuer. In non-`local` envs, startup refuses to boot if `DIRECTORY_API_KEY` is blank or still the built-in dev default. | | `TRUST_PROXY_HEADERS` | Trust a single valid `X-Real-IP` supplied by an ingress that overwrites that header. **Set this to `true` behind Railway's public edge.** `X-Forwarded-For` is ignored. Missing/invalid/duplicate `X-Real-IP` falls back to the socket peer. Leave `false` for direct/private-network callers. | **There is deliberately no `API_KEY`.** Every other service in this repo carries @@ -212,7 +212,7 @@ Scopes recognized today: - `resolve:discord` — Discord resolution. - `auth:email` — start and confirm email login. -- `auth:status` — active-person status for compute sessions. +- `auth:status` — environment issuer and active-person status for compute sessions. `platform_auth` also treats `admin` as a wildcard that satisfies every scope check. The CLI rejects `admin`, unknown scopes, and missing scopes. There is no bootstrap path that diff --git a/services/gateway/contracts/login.py b/services/gateway/contracts/login.py index b19e0e0b..4b150291 100644 --- a/services/gateway/contracts/login.py +++ b/services/gateway/contracts/login.py @@ -59,19 +59,21 @@ def valid_code(cls, value: SecretStr) -> SecretStr: return value -class EmailStartOut(BaseModel): +class IssuerOut(BaseModel): model_config = ConfigDict(extra="forbid") + issuer: str + + +class EmailStartOut(IssuerOut): challenge_id: UUID expires_in: int = 600 message: str = "If an active Misty account matches, a code will be sent." -class EmailConfirmOut(BaseModel): - model_config = ConfigDict(extra="forbid") +class EmailConfirmOut(IssuerOut): person_id: UUID email: str -class PersonStatusOut(BaseModel): - model_config = ConfigDict(extra="forbid") +class PersonStatusOut(IssuerOut): active: bool diff --git a/services/gateway/contracts/storage.py b/services/gateway/contracts/storage.py index 62b41b61..ec65cde0 100644 --- a/services/gateway/contracts/storage.py +++ b/services/gateway/contracts/storage.py @@ -17,21 +17,18 @@ def reserve_login(self, challenge: LoginChallenge) -> bool: """Atomically reserve mailbox send quotas and persist a pending challenge. Quotas apply equally to unknown/inactive mailboxes and are never refunded. - Older challenges remain usable until replacement delivery succeeds. + Other challenges remain independently usable until expiry or redemption. """ ... def activate_login(self, challenge_id: UUID, person_id: UUID, now: datetime) -> bool: - """Bind after delivery and invalidate older challenges for this app/email. - - A late activation cannot supersede a newer successfully delivered code. - """ + """Bind after delivery without changing any other challenge's validity.""" ... def admit_login_attempt( self, challenge_id: UUID, owner: str, now: datetime ) -> LoginChallenge | None: - """Atomically spend challenge and mailbox guess budgets before verification.""" + """Atomically spend this challenge's attempt budget before verification.""" ... def redeem_login(self, challenge_id: UUID, owner: str, now: datetime) -> bool: diff --git a/services/gateway/docs/API.md b/services/gateway/docs/API.md index 335d7704..6c785931 100644 --- a/services/gateway/docs/API.md +++ b/services/gateway/docs/API.md @@ -35,13 +35,22 @@ smoke request separately verifies the internal URL, internal key, and directory These are server-to-server routes for the compute backend. They require an issued `gw_` key and `EMAIL_LOGIN_ENABLED=true`. Use `auth:email` for both POSTs and -`auth:status` for status checks. A resolver-only key cannot use them. No auto-signup +`auth:status` for issuer and status checks. A resolver-only key cannot use them. No auto-signup occurs. Only `Person.primary_email` is accepted (trimmed and lowercased); secondary identifiers are not login addresses. Eligibility requires `active=true`. All responses, including errors, use `Cache-Control: no-store`. Input models reject extra fields. Login bodies are limited to 4096 bytes; errors never echo -submitted codes. Codes must be exactly six ASCII digits. +submitted codes. Codes must be exactly six ASCII digits. Successful auth responses include an +`issuer` derived from `GATEWAY_ENV`: `misty:local`, `misty:staging`, or +`misty:production`. Consumers must check the expected issuer on every success. + +### Issuer + +`GET /v1/auth/issuer` returns `{"issuer":"misty:production"}` in production. +It requires `auth:status`, shares the status quota, and fails closed when login +is disabled. Compute checks this at startup and pins both issuer and gateway +origin to its database; staging and production need separate databases and keys. ### Start @@ -52,11 +61,12 @@ submitted codes. Codes must be exactly six ASCII digits. ``` Returns **202**, with an opaque UUID `challenge_id`, `expires_in: 600`, and -`message: "If an active Misty account matches, a code will be sent."` +`message: "If an active Misty account matches, a code will be sent."`, plus `issuer`. The same shape is returned for unknown/inactive emails and mailbox throttling. A throttled request returns a nonredeemable ID and leaves the previous challenge -usable. A resend invalidates previous challenges for this app/email only after successful -delivery and activation. Failed replacement delivery preserves the old code. +usable. Every delivered challenge remains independently valid until its expiry, +attempt exhaustion, or redemption. A new request cannot invalidate another +browser's code; the UI must retain the corresponding request ID. Eligibility lookup and code delivery run after the response is sent, to keep account-dependent provider latency out of the public response. Delivery is best @@ -72,7 +82,7 @@ after the cooldown; 202 means accepted, not proof of delivery or membership. {"challenge_id":"","code":"123456","client_ip":"192.0.2.1"} ``` -On success, **200** returns only `{"person_id":"","email":"member@example.com"}`. +On success, **200** returns only `{"person_id":"","email":"member@example.com","issuer":"misty:production"}`. The code must belong to this challenge, and the authenticated application must match the starter. Misty rechecks the original person UUID, active flag, and primary email immediately before atomic redemption. Retiring the person, @@ -80,14 +90,14 @@ changing the email, or reassigning it to a different person prevents login. A challenge can succeed only once, including concurrent confirms. A lost success response cannot be replayed: request a new code. Verification outages fail closed. -Unknown, pending, expired, replaced, exhausted, already-used, wrong-app, and +Unknown, pending, expired, exhausted, already-used, wrong-app, and incorrect-code challenges all return **400** with `invalid or expired login challenge`. Neither a person ID nor email is returned on failure. ### Current eligibility -`GET /v1/auth/people/{person_id}/status` returns **200** with only -`{"active":true}` or `{"active":false}`. Missing and retired people both return +`GET /v1/auth/people/{person_id}/status` returns **200** with +`{"active":true,"issuer":"misty:production"}` or the same shape with `active:false`. Missing and retired people both return false. Directory failures return 503, never an assumed active result. This scope permits status checks by UUID; it is not a public email lookup. @@ -100,13 +110,20 @@ Windows start on the first admitted request and expire after the duration below. |---|---| | App start requests | 30/minute, 300/hour | | App confirm requests | 60/minute | -| App status requests | Separate 60/minute | +| App status/issuer requests | Separate 60/minute | | Backend-attested client IP | 5 starts/minute, 30 confirms/minute, shared across apps | | Mailbox sends | 1/minute, 5/hour, 20/day, shared across apps | -| Challenge guesses | 5, reserved before calling verification | -| Mailbox guesses | 15/hour across all challenges/apps | +| Challenge guesses | 3, reserved before calling verification | | Challenge lifetime | 10 minutes from start, including delivery time | +Five deliveries per mailbox window with three guesses each bound guesses against +the codes issued in that window to fifteen, across all apps. There is no shared +mailbox guess lockout: exhausting one challenge never spends another's budget. +Codes issued near a window boundary may remain valid into the next window for +the remainder of their ten-minute TTL. Send quotas still limit delivery abuse; +an attacker can exhaust new-send capacity, so edge abuse protection and monitoring +remain necessary. An already-delivered code remains usable through send throttling. + IPv4-mapped IPv6 addresses normalize to IPv4; native IPv6 addresses share a /64 bucket. Mailbox quotas include unknown/inactive addresses. Delivery failure does not refund them. App/IP exhaustion returns **429** with `Retry-After: 60` (a retry @@ -128,7 +145,9 @@ invalid input → 422; disabled login or unavailable upstream → 503. 3. Bind each returned challenge to the initiating browser session, protect POSTs against CSRF, and forward the submitted code to confirm. Do not create a session based on the browser's claim that confirmation succeeded. -4. On confirmed success, map the stable Misty UUID to the existing compute account +4. Explicitly map existing compute accounts to reviewed Misty UUIDs before + cutover; email possession alone must not claim an existing account. On + confirmed success, map the stable Misty UUID to the existing compute account and establish/rotate a server-side session with Secure, HttpOnly cookies. Keep existing ownership IDs for jobs/files. Do not automatically merge accounts from a browser-supplied email or silently grant compute roles/admin rights. diff --git a/services/gateway/docs/ARCHITECTURE.md b/services/gateway/docs/ARCHITECTURE.md index 2f57d7dc..4a81fb4d 100644 --- a/services/gateway/docs/ARCHITECTURE.md +++ b/services/gateway/docs/ARCHITECTURE.md @@ -61,7 +61,10 @@ On process loss, the user retries after the cooldown; budgets are retained and n ambiguous send is retried automatically. Monitor delivery-failure logs. Before confirmation reaches verification, the gateway atomically reserves a -challenge attempt and a mailbox attempt. It then rechecks the original person's +challenge attempt. Three attempts per challenge and five deliveries per mailbox +window bound guesses against codes issued in that window to fifteen; no shared +guess counter lets an attacker block a different challenge. New delivery does +not invalidate an earlier challenge. It then rechecks the original person's active flag and email. A final compare-and-set permits exactly one redemption. Private verification's existing correct-code replay remains available for the Discord flow; the public login boundary is strictly single-use. There is no @@ -83,6 +86,12 @@ application quotas do not depend on the attestation. Keep the existing one-repli one-worker limit because the pre-authentication flood guard and resolver quotas are still process-local. Persistent login quotas alone do not authorize scaling. +Successful auth responses attest `misty:`. The authenticated issuer +endpoint supports compute's startup check; compute also checks each successful +response and durably pins issuer plus gateway origin to its database. Explicit +operator mappings are required before existing compute accounts can be claimed; +email equality alone does not establish continuity of account ownership. + ## Future compute-platform access Compute receives only the scopes it uses (`auth:email`, `auth:status`). Login diff --git a/services/gateway/docs/CHANGE-AUDIT.md b/services/gateway/docs/CHANGE-AUDIT.md index dfae7ac9..ddd60ff2 100644 --- a/services/gateway/docs/CHANGE-AUDIT.md +++ b/services/gateway/docs/CHANGE-AUDIT.md @@ -124,14 +124,14 @@ Paths are repository-relative; grouped paths all share the stated purpose. | `services/gateway/src/login_policy.py` | Centralize send/guess/request windows and normalized email/IP bucket identities. | | `services/gateway/src/storage/schema.py`, `services/gateway/migrations/versions/002_email_login.py` | Define and reversibly migrate gateway-owned challenge and quota tables/indexes. | | `services/gateway/src/storage/in_memory.py`, `services/gateway/src/storage/postgres.py` | Equivalent synchronized admission, activation, cleanup and single-use redemption; readiness includes the new schema. | -| `services/gateway/src/api/routers/email_login.py` | Implement start, confirm, status, active-member checks and post-response private delivery. | +| `services/gateway/src/api/routers/email_login.py` | Implement start, confirm, issuer/status, active-member checks and post-response private delivery. | | `services/gateway/src/api/app.py`, `services/gateway/src/api/middleware.py` | Register routes, sanitize validation/upstream errors, bound body size, prevent response caching. | | `services/gateway/src/api/deps.py` | Own and close a lazily constructed private verification HTTP pool. | | `services/gateway/src/directory/http_client.py` | Use existing authenticated directory email/person lookup routes. | | `services/gateway/src/verification/__init__.py`, `services/gateway/src/verification/http_client.py` | Package the private verification client and sanitize provider/transport failures. | | `services/gateway/src/config.py`, `services/gateway/src/cli.py` | Add the opt-in feature, redacted internal credential/boot validation, and two external scopes. | -| `services/gateway/tests/test_email_login.py`, `services/gateway/tests/test_verification.py` | Prove HTTP eligibility, replay/concurrency, generic responses, scope ordering, failure masking, resend recovery and private client contract. | -| `services/gateway/tests/login_storage_cases.py`, `services/gateway/tests/test_login_storage.py`, `services/gateway/tests/test_postgres_adapter.py` | Exercise the same quotas, expiry, races, replacement and redemption semantics against both adapters, plus persistence across adapter recreation. | +| `services/gateway/tests/test_email_login.py`, `services/gateway/tests/test_verification.py` | Prove HTTP eligibility, replay/concurrency, generic responses, scope ordering, failure masking, independent challenge budgets, issuer attestation and private client contract. | +| `services/gateway/tests/login_storage_cases.py`, `services/gateway/tests/test_login_storage.py`, `services/gateway/tests/test_postgres_adapter.py` | Exercise the same quotas, expiry, races, independent challenge and redemption semantics against both adapters, plus persistence across adapter recreation. | | `services/gateway/tests/test_config.py` | Check the new secret is redacted and required when non-local login is enabled. | | `services/gateway/tests/smoke_stack.py`, `.github/workflows/ci.yml` | Verify real gateway → directory/verification HTTP using disposable DBs, scoped keys, a fake sender and test-container-only deterministic codes; build verification before this check. | | `services/verification/src/config.py`, `services/verification/src/api/deps.py`, `services/verification/src/api/app.py` | Support a separate scoped gateway identity through config-backed keys and boot validation. | @@ -160,3 +160,21 @@ gateway, directory and verification containers with disposable databases and a fake mail sender. It covered login, rejection, replay, preserved compute identity, and retirement after the status cache expires. No live deployment or real email delivery was performed as part of these checks. + +### Security hardening verification + +The gateway uses three attempts per independent challenge and five deliveries +per mailbox issuance window. A different challenge cannot exhaust or invalidate +a member's delivered code. Send quotas remain shared to bound mail abuse; see +[the limits and window trade-off](API.md#limits-and-failures). Successful responses +include the environment issuer, and the scoped issuer endpoint supports compute's +startup and database pinning checks. + +The mailbox-lockout and resend regressions failed on the previous implementation +and passed after the fix. Shared storage tests verify the three-attempt cap, +mailbox-wide delivery budget across applications, expiry and atomic redemption +in both memory and disposable Postgres. Repository checks, migration round trip, +Ruff gates, rebuilt gateway image and real private-HTTP/browser integration passed. +No live configuration was changed. The companion compute PR contains the CLI +origin/redirect and revocation fixes, explicit account migration, issuer checks, +and dependency/runtime updates. diff --git a/services/gateway/docs/DEPLOYMENT.md b/services/gateway/docs/DEPLOYMENT.md index fa76c203..859d7df5 100644 --- a/services/gateway/docs/DEPLOYMENT.md +++ b/services/gateway/docs/DEPLOYMENT.md @@ -102,6 +102,9 @@ belongs to disposable test databases only. The feature is off by default; existing resolver deployments continue to work. Use the same environment for all three services and keep verification private. +Set `GATEWAY_ENV=staging` or `production` explicitly: successful auth responses +attest this value, and compute requires the matching `MISTY_ENVIRONMENT`. +Use separate gateway keys and compute databases for staging and production. 1. Deploy verification with a dedicated `vf_` consumer key named `gateway`, scoped only `verification:write`, using its [provisioning guide](../../verification/docs/DEPLOYMENT.md#scoped-consumer-keys). @@ -125,7 +128,9 @@ Use the same environment for all three services and keep verification private. 6. Store the `gw_` key only in compute's backend secret store. Implement the [compute integration responsibilities](API.md#compute-integration-responsibilities), - including trusted client-IP derivation, browser-session challenge binding, + including an authenticated `/v1/auth/issuer` startup check and durable issuer/origin + binding, explicit UUID mappings for existing accounts, trusted client-IP + derivation, browser-session challenge binding, secure sessions, and fail-closed active-status checks. Keep one gateway replica and worker. Login counters are durable; the edge flood guard still is not. 7. Verify with an approved active test member: request a code, confirm once, @@ -146,7 +151,7 @@ records/counters on authenticated auth requests, so idle environments retain the until activity resumes. To disable new authentication, set `EMAIL_LOGIN_ENABLED=false` and redeploy; all -three routes then fail closed. Revoke the compute key for immediate API access +auth routes then fail closed. Revoke the compute key for immediate API access removal. Compute must end/reject its own sessions when status checks fail. Do not reset counters during routine deploys. For rollback, disable login and stop in-flight requests first; migration002 can be downgraded on a backup-protected diff --git a/services/gateway/src/api/routers/email_login.py b/services/gateway/src/api/routers/email_login.py index 9bed7a41..b8485422 100644 --- a/services/gateway/src/api/routers/email_login.py +++ b/services/gateway/src/api/routers/email_login.py @@ -9,6 +9,7 @@ EmailConfirmOut, EmailStartIn, EmailStartOut, + IssuerOut, LoginChallenge, PersonStatusOut, VerificationClient, @@ -23,6 +24,10 @@ router = APIRouter(prefix="/v1/auth", tags=["email login"]) +def _issuer() -> str: + return f"misty:{get_settings().gateway_env}" + + def _enabled(): if not get_settings().email_login_enabled: raise HTTPException(503, "email login unavailable") @@ -86,7 +91,7 @@ def start( if storage.reserve_login(challenge): background.add_task(_deliver, challenge, storage, directory, verification) # Mailbox throttling returns an indistinguishable, nonredeemable challenge. - return EmailStartOut(challenge_id=challenge.id) + return EmailStartOut(challenge_id=challenge.id, issuer=_issuer()) @router.post("/email/confirm", response_model=EmailConfirmOut) @@ -116,7 +121,18 @@ def confirm( raise invalid if not storage.redeem_login(challenge.id, key.name, policy.now()): raise invalid - return EmailConfirmOut(person_id=person_id, email=challenge.email) + return EmailConfirmOut(person_id=person_id, email=challenge.email, issuer=_issuer()) + + +@router.get("/issuer", response_model=IssuerOut) +def issuer( + response: Response, + key: AuthedKey = Depends(require_scope("auth:status")), + storage: StorageAdapter = Depends(get_storage), +) -> IssuerOut: + _quota(storage, key, "status") + response.headers["Cache-Control"] = "no-store" + return IssuerOut(issuer=_issuer()) @router.get("/people/{person_id}/status", response_model=PersonStatusOut) @@ -129,4 +145,6 @@ def status( ) -> PersonStatusOut: _quota(storage, key, "status") response.headers["Cache-Control"] = "no-store" - return PersonStatusOut(active=_eligible(directory.get_person(str(person_id))) == person_id) + return PersonStatusOut( + active=_eligible(directory.get_person(str(person_id))) == person_id, issuer=_issuer() + ) diff --git a/services/gateway/src/login_policy.py b/services/gateway/src/login_policy.py index 29eb3fa3..c42e8f75 100644 --- a/services/gateway/src/login_policy.py +++ b/services/gateway/src/login_policy.py @@ -5,7 +5,9 @@ from ipaddress import ip_address, ip_network TTL = timedelta(minutes=10) -MAX_ATTEMPTS = 5 +# Five deliveries/hour * three guesses/challenge = at most fifteen guesses for +# codes issued per mailbox window. Independent budgets prevent targeted lockout. +MAX_ATTEMPTS = 3 # (bucket, maximum admissions, window seconds) Limit = tuple[str, int, int] @@ -39,7 +41,3 @@ def send_limits(email: str) -> list[Limit]: (f"email:{digest}:send:{seconds}", count, seconds) for count, seconds in [(1, 60), (5, 3600), (20, 86400)] ] - - -def guess_limits(email: str) -> list[Limit]: - return [(f"email:{email_key(email)}:guess", 15, 3600)] diff --git a/services/gateway/src/storage/in_memory.py b/services/gateway/src/storage/in_memory.py index c8e7f7d7..d81b9075 100644 --- a/services/gateway/src/storage/in_memory.py +++ b/services/gateway/src/storage/in_memory.py @@ -51,23 +51,6 @@ def activate_login(self, challenge_id, person_id, now): item = self._logins.get(challenge_id) if item is None or item.expires_at <= now or item.redeemed: return False - if any( - v.owner == item.owner - and v.email == item.email - and v.person_id is not None - and v.created_at > item.created_at - for v in self._logins.values() - ): - return False - self._logins = { - k: v - for k, v in self._logins.items() - if not ( - v.owner == item.owner - and v.email == item.email - and v.created_at < item.created_at - ) - } self._logins[challenge_id] = item.model_copy(update={"person_id": person_id}) return True @@ -82,8 +65,6 @@ def admit_login_attempt(self, challenge_id, owner, now): or item.attempts >= policy.MAX_ATTEMPTS ): return None - if not self._spend(policy.guess_limits(item.email), now): - return None item = item.model_copy(update={"attempts": item.attempts + 1}) self._logins[challenge_id] = item return item.model_copy() diff --git a/services/gateway/src/storage/postgres.py b/services/gateway/src/storage/postgres.py index da7533ef..6ec957c9 100644 --- a/services/gateway/src/storage/postgres.py +++ b/services/gateway/src/storage/postgres.py @@ -105,25 +105,6 @@ def activate_login(self, challenge_id, person_id, now): ).one_or_none() if item is None: return False - newer = conn.execute( - select(login_challenges.c.id) - .where( - login_challenges.c.owner == item.owner, - login_challenges.c.email == item.email, - login_challenges.c.created_at > item.created_at, - login_challenges.c.person_id.is_not(None), - ) - .limit(1) - ).first() - if newer is not None: - return False - conn.execute( - delete(login_challenges).where( - login_challenges.c.owner == item.owner, - login_challenges.c.email == item.email, - login_challenges.c.created_at < item.created_at, - ) - ) return ( conn.execute( update(login_challenges) @@ -148,7 +129,7 @@ def admit_login_attempt(self, challenge_id, owner, now): login_challenges.c.attempts < policy.MAX_ATTEMPTS, ) ).one_or_none() - if row is None or not self._spend(conn, policy.guess_limits(row.email), now): + if row is None: return None updated = conn.execute( update(login_challenges) diff --git a/services/gateway/tests/login_storage_cases.py b/services/gateway/tests/login_storage_cases.py index 016a9706..0fecf4e6 100644 --- a/services/gateway/tests/login_storage_cases.py +++ b/services/gateway/tests/login_storage_cases.py @@ -15,23 +15,36 @@ def challenge(at=NOW, *, owner="compute", email="member@example.com"): class LoginStorageCases: + def test_delivery_quotas_bound_guesses_across_apps(self, adapter): + admitted = 0 + for minute in range(5): + item = challenge(NOW + timedelta(minutes=minute), owner=f"app-{minute}") + assert adapter.reserve_login(item) + assert adapter.activate_login(item.id, PERSON_ID, item.created_at) + for _ in range(3): + assert adapter.admit_login_attempt(item.id, item.owner, item.created_at) + admitted += 1 + assert adapter.admit_login_attempt(item.id, item.owner, item.created_at) is None + assert admitted == 15 + assert not adapter.reserve_login(challenge(NOW + timedelta(minutes=5), owner="other")) + def test_concurrent_reservations_send_once(self, adapter): with ThreadPoolExecutor(max_workers=8) as pool: results = list(pool.map(lambda _: adapter.reserve_login(challenge()), range(8))) assert results.count(True) == 1 - def test_limits_survive_replacement_and_are_global_per_email(self, adapter): + def test_guess_budgets_are_independent_and_bounded_by_delivery_quotas(self, adapter): for minute in range(3): item = challenge(NOW + timedelta(minutes=minute)) assert adapter.reserve_login(item) assert adapter.activate_login(item.id, PERSON_ID, item.created_at) - for _ in range(5): + for _ in range(3): assert adapter.admit_login_attempt(item.id, item.owner, item.created_at) assert adapter.admit_login_attempt(item.id, item.owner, item.created_at) is None item = challenge(NOW + timedelta(minutes=3), owner="another-app") assert adapter.reserve_login(item) assert adapter.activate_login(item.id, PERSON_ID, item.created_at) - assert adapter.admit_login_attempt(item.id, item.owner, item.created_at) is None + assert adapter.admit_login_attempt(item.id, item.owner, item.created_at) is not None def test_send_hour_and_day_budgets(self, adapter): for hour in range(4): @@ -42,7 +55,7 @@ def test_send_hour_and_day_budgets(self, adapter): assert not adapter.reserve_login(challenge(NOW + timedelta(hours=4))) assert adapter.reserve_login(challenge(NOW + timedelta(days=1))) - def test_expiry_and_resend_invalidate_challenges(self, adapter): + def test_resend_preserves_other_challenges_until_expiry(self, adapter): first = challenge() assert adapter.reserve_login(first) assert adapter.activate_login(first.id, PERSON_ID, NOW) @@ -50,18 +63,18 @@ def test_expiry_and_resend_invalidate_challenges(self, adapter): assert adapter.reserve_login(second) assert adapter.admit_login_attempt(first.id, first.owner, second.created_at) assert adapter.activate_login(second.id, PERSON_ID, second.created_at) - assert adapter.admit_login_attempt(first.id, first.owner, second.created_at) is None - assert not adapter.redeem_login(first.id, first.owner, second.created_at) + assert adapter.admit_login_attempt(first.id, first.owner, second.created_at) is not None + assert adapter.redeem_login(first.id, first.owner, second.created_at) assert not adapter.activate_login(second.id, PERSON_ID, second.expires_at) assert adapter.admit_login_attempt(second.id, second.owner, second.expires_at) is None - def test_late_delivery_cannot_replace_a_newer_active_code(self, adapter): + def test_late_delivery_preserves_both_active_codes(self, adapter): first = challenge() second = challenge(NOW + timedelta(minutes=1)) assert adapter.reserve_login(first) assert adapter.reserve_login(second) assert adapter.activate_login(second.id, PERSON_ID, second.created_at) - assert not adapter.activate_login(first.id, PERSON_ID, second.created_at) + assert adapter.activate_login(first.id, PERSON_ID, second.created_at) assert adapter.admit_login_attempt(second.id, second.owner, second.created_at) def test_concurrent_attempts_and_single_redemption(self, adapter): @@ -72,7 +85,7 @@ def test_concurrent_attempts_and_single_redemption(self, adapter): attempts = list( pool.map(lambda _: adapter.admit_login_attempt(item.id, item.owner, NOW), range(10)) ) - assert sum(x is not None for x in attempts) == 5 + assert sum(x is not None for x in attempts) == 3 assert not adapter.redeem_login(item.id, "other-app", NOW) with ThreadPoolExecutor(max_workers=10) as pool: results = list( diff --git a/services/gateway/tests/smoke_stack.py b/services/gateway/tests/smoke_stack.py index 865d513e..b7efe69f 100644 --- a/services/gateway/tests/smoke_stack.py +++ b/services/gateway/tests/smoke_stack.py @@ -288,13 +288,16 @@ def start(name, image, env, *, public=False, command=()): payload = {"challenge_id": challenge_id, "code": "123456", "client_ip": "192.0.2.1"} assert http(gateway_url, "/v1/auth/email/confirm", key=compute_key, payload=payload) == ( 200, - {"person_id": person["id"], "email": "smoke@example.test"}, + {"person_id": person["id"], "email": "smoke@example.test", "issuer": "misty:staging"}, ) assert ( http(gateway_url, "/v1/auth/email/confirm", key=compute_key, payload=payload)[0] == 400 ) status_path = f"/v1/auth/people/{person['id']}/status" - assert http(gateway_url, status_path, key=compute_key) == (200, {"active": True}) + assert http(gateway_url, status_path, key=compute_key) == ( + 200, + {"active": True, "issuer": "misty:staging"}, + ) assert ( http( directory_url, @@ -305,7 +308,10 @@ def start(name, image, env, *, public=False, command=()): )[0] == 200 ) - assert http(gateway_url, status_path, key=compute_key) == (200, {"active": False}) + assert http(gateway_url, status_path, key=compute_key) == ( + 200, + {"active": False, "issuer": "misty:staging"}, + ) print( "PASS: scoped email login over private HTTP, single redemption, and retirement", flush=True, diff --git a/services/gateway/tests/test_email_login.py b/services/gateway/tests/test_email_login.py index 5783124b..bed1db1d 100644 --- a/services/gateway/tests/test_email_login.py +++ b/services/gateway/tests/test_email_login.py @@ -1,4 +1,5 @@ from concurrent.futures import ThreadPoolExecutor +from datetime import datetime, timedelta, timezone from uuid import uuid4 import pytest @@ -9,6 +10,7 @@ from src.api.hashing import generate_key from src.config import Settings, get_settings from src.storage.in_memory import InMemoryStorageAdapter +from src import login_policy as policy EMAIL = "member@example.com" PERSON_ID = str(uuid4()) @@ -80,6 +82,42 @@ def confirm(client, challenge, code="123456", **extra): ) +def test_other_challenges_cannot_exhaust_a_valid_code(login, monkeypatch): + client, _, _, verification = login + clock = [datetime(2026, 10, 26, tzinfo=timezone.utc)] + monkeypatch.setattr(policy, "now", lambda: clock[0]) + for _ in range(3): + challenge = start(client).json()["challenge_id"] + for _ in range(5): + assert confirm(client, challenge, "000000").status_code == 400 + clock[0] += timedelta(seconds=61) + victim = start(client, client_ip="192.0.2.99").json()["challenge_id"] + assert f"gateway-login:{victim}" in verification.sent + assert confirm(client, victim, client_ip="192.0.2.99").status_code == 200 + + +def test_new_delivery_does_not_invalidate_another_browser_code(login, monkeypatch): + client, _, _, _ = login + now = datetime(2026, 10, 26, tzinfo=timezone.utc) + monkeypatch.setattr(policy, "now", lambda: now) + first = start(client).json()["challenge_id"] + now += timedelta(seconds=61) + second = start(client, client_ip="192.0.2.99").json()["challenge_id"] + assert confirm(client, first).status_code == 200 + assert confirm(client, second, client_ip="192.0.2.99").status_code == 200 + + +def test_issuer_is_authenticated_and_matches_environment(login): + client, store, _, _ = login + assert client.get("/v1/auth/issuer").json() == {"issuer": "misty:local"} + assert client.get("/v1/auth/issuer", headers={"X-API-Key": "invalid"}).status_code == 401 + token, prefix, key_hash = generate_key() + store.create_api_key( + name="resolver", prefix=prefix, key_hash=key_hash, scopes=["resolve:discord"], actor="test" + ) + assert client.get("/v1/auth/issuer", headers={"X-API-Key": token}).status_code == 403 + + @pytest.mark.parametrize("eligible", ["active", "missing", "inactive"]) def test_start_is_generic_and_only_sends_to_active_people(login, eligible): client, _, directory, verification = login @@ -89,14 +127,14 @@ def test_start_is_generic_and_only_sends_to_active_people(login, eligible): directory.person["active"] = False response = start(client, " MEMBER@EXAMPLE.COM ") assert response.status_code == 202 - assert set(response.json()) == {"challenge_id", "expires_in", "message"} + assert set(response.json()) == {"challenge_id", "expires_in", "message", "issuer"} assert response.json()["expires_in"] == 600 assert EMAIL not in response.text assert len(verification.sent) == (1 if eligible == "active" else 0) result = confirm(client, response.json()["challenge_id"]) assert result.status_code == (200 if eligible == "active" else 400) if eligible == "active": - assert result.json() == {"person_id": PERSON_ID, "email": EMAIL} + assert result.json() == {"person_id": PERSON_ID, "email": EMAIL, "issuer": "misty:local"} def test_success_is_single_use_even_under_concurrent_requests(login): @@ -123,13 +161,13 @@ def test_confirmation_rechecks_original_person_and_email(login, change): assert EMAIL not in response.text -def test_concurrent_wrong_guesses_cannot_exceed_five_attempts(login): +def test_concurrent_wrong_guesses_cannot_exceed_three_attempts(login): client, _, _, verification = login challenge = start(client).json()["challenge_id"] with ThreadPoolExecutor(max_workers=8) as pool: responses = list(pool.map(lambda _: confirm(client, challenge, "999999"), range(8))) assert all(r.status_code == 400 for r in responses) - assert len(verification.guesses) == 5 + assert len(verification.guesses) == 3 assert confirm(client, challenge).status_code == 400 @@ -171,7 +209,7 @@ def test_status_is_minimal_and_checks_current_state(login, person, active): directory.person = person response = client.get(f"/v1/auth/people/{PERSON_ID}/status") assert response.status_code == 200 - assert response.json() == {"active": active} + assert response.json() == {"active": active, "issuer": "misty:local"} assert response.headers["cache-control"] == "no-store" From 5fc9ae0919c3db13c0db6a4bf8434b2b458c4abf Mon Sep 17 00:00:00 2001 From: Eeetan Date: Mon, 26 Oct 2026 12:11:27 -0400 Subject: [PATCH 3/5] fix(gateway): recover throttled delivery and configure status capacity --- AGENTS.md | 2 +- docs/RAILWAY-DEPLOYMENT.md | 5 +- services/gateway/.env.example | 3 + services/gateway/README.md | 4 +- services/gateway/contracts/login.py | 8 ++ services/gateway/contracts/storage.py | 8 +- services/gateway/docs/API.md | 16 ++- services/gateway/docs/ARCHITECTURE.md | 13 ++- services/gateway/docs/CHANGE-AUDIT.md | 43 ++++++++ services/gateway/docs/DEPLOYMENT.md | 12 ++- services/gateway/src/api/app.py | 1 + .../gateway/src/api/routers/email_login.py | 47 ++++++-- services/gateway/src/config.py | 14 ++- services/gateway/src/login_policy.py | 6 +- services/gateway/src/storage/in_memory.py | 6 +- services/gateway/src/storage/postgres.py | 8 +- .../gateway/src/verification/http_client.py | 12 ++- services/gateway/tests/login_storage_cases.py | 7 ++ services/gateway/tests/smoke_stack.py | 100 ++++++++++++++++++ services/gateway/tests/test_config.py | 21 ++++ services/gateway/tests/test_email_login.py | 89 ++++++++++++++++ services/gateway/tests/test_health.py | 7 +- services/gateway/tests/test_verification.py | 27 +++++ services/verification/README.md | 2 +- services/verification/docs/API.md | 2 +- services/verification/docs/ARCHITECTURE.md | 3 + .../src/api/routers/verification.py | 8 +- .../verification/tests/test_request_code.py | 25 +++++ 28 files changed, 465 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 080d8c9d..84f3744c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -115,7 +115,7 @@ Run what CI runs for that service — `.github/workflows/ci.yml` is authoritativ - **Cross-worktree tests need source-path verification.** Pytest can prepend a historical tests checkout to `sys.path` and import its `contracts` instead of the edited code. Pin the target `src` and `contracts` packages before collection and verify imported modules' `__file__` paths; passing against the wrong checkout is not verification. - **`uv --project` is mandatory for the key CLIs.** Both `services/team-tracking` and `services/documentation-system` declare a top-level `src` package with a console script at `src.cli:main`. In the shared workspace venv they collide, so a bare `team-tracking-keys …` can resolve documentation-system's CLI and mint a `doc_`-envelope key that team-tracking rejects. Always `uv --project services/ run -keys …`, and verify the token's prefix. The same applies to every other team-tracking console script (`team-tracking-seed`, `team-tracking-groups`). - **Gateway provisioning must pin the import path too.** After installing all workspace members, `uv --project` alone can still import another service's `src.cli`. From the repo root use `uv --directory services/gateway --project . run python -m src.cli ...` (and the team-tracking directory for its internal key). The isolated containers can use their installed key CLI directly. -- **Gateway is the public entry point.** It accepts only issued `gw_` keys, has no env-bootstrap `API_KEY`, and owns its key registry plus durable login challenges/abuse counters. Compute-platform data access must use explicit scoped HTTP operations; see `services/gateway/docs/ARCHITECTURE.md`. Run one replica/worker for its process-local rate limits. +- **Gateway is the public entry point.** It accepts only issued `gw_` keys, has no env-bootstrap `API_KEY`, and owns its key registry plus durable login challenges/abuse counters. Compute-platform data access must use explicit scoped HTTP operations; see `services/gateway/docs/ARCHITECTURE.md`. Run one replica/worker for its process-local rate limits. Size `LOGIN_STATUS_PER_MINUTE` and `EDGE_REQUESTS_PER_MINUTE` together. Only verification’s explicit pre-send cooldown permits a bounded delivery retry; never retry an ambiguous send. - **Gateway email login stays on Misty.** Verification owns code generation/send/check; gateway owns eligibility, consumer-bound challenges, durable quotas, and atomic single-use redemption. Preserve Discord’s existing correct-code replay. The compute backend attests `client_ip` from trusted ingress and must bind challenges to browser sessions; it never gets a verification key. See `services/gateway/docs/API.md#email-login`. - **Misty login issuers are environment-bound.** Successful auth responses carry `misty:`. Compute verifies and pins issuer plus gateway origin to its database; existing compute accounts need explicit UUID mappings. Do not restore email-only account claiming or a shared mailbox guess lockout; the gateway uses three guesses per independent challenge and five deliveries per mailbox window. - **Gateway proxy trust is specific to `X-Real-IP`.** Railway's public edge overwrites it; `X-Forwarded-For` may include a shared proxy hop and is ignored. Health probes skip the IP quota, so preserve the per-app readiness cache and nonblocking probe lock; see `services/gateway/docs/API.md#health`. diff --git a/docs/RAILWAY-DEPLOYMENT.md b/docs/RAILWAY-DEPLOYMENT.md index 87732d1a..c96ca7ad 100644 --- a/docs/RAILWAY-DEPLOYMENT.md +++ b/docs/RAILWAY-DEPLOYMENT.md @@ -26,7 +26,10 @@ Email login requires gateway migration002, `EMAIL_LOGIN_ENABLED`, private `CONSUMER_KEYS`, directory `people:read`, and compute `auth:email`/`auth:status`. Set gateway `GATEWAY_ENV` explicitly; compute verifies the matching issuer and pins its gateway origin/environment to a separate database per environment. -Follow [enablement](../services/gateway/docs/DEPLOYMENT.md#enable-email-login) in order. +Deploy verification’s cooldown `Retry-After` support before gateway retry support. +Size `LOGIN_STATUS_PER_MINUTE` and `EDGE_REQUESTS_PER_MINUTE` together using +[enablement](../services/gateway/docs/DEPLOYMENT.md#enable-email-login); keep the +one-worker limit and the compute eligibility cache unchanged. For the optional persistent `dev` environment used to test one PR at a time with a dedicated Discord bot, see [PR previews](pr-previews.md). That diff --git a/services/gateway/.env.example b/services/gateway/.env.example index 1f99f1da..6774ec42 100644 --- a/services/gateway/.env.example +++ b/services/gateway/.env.example @@ -17,5 +17,8 @@ TRUST_PROXY_HEADERS=false # Optional server-to-server email login. Configure a distinct scoped vf_ key first. EMAIL_LOGIN_ENABLED=false +# Size both for status refreshes plus other traffic; keep edge headroom >=120. +LOGIN_STATUS_PER_MINUTE=600 +EDGE_REQUESTS_PER_MINUTE=1200 VERIFICATION_BASE_URL=http://localhost:8003 VERIFICATION_API_KEY=dev-api-key-change-me diff --git a/services/gateway/README.md b/services/gateway/README.md index bffb15df..e67ee0aa 100644 --- a/services/gateway/README.md +++ b/services/gateway/README.md @@ -69,6 +69,8 @@ schema is at `http://localhost:8006/openapi.json`. | `DATABASE_URL` | The gateway's **own** Postgres — it holds external keys, short-lived login challenges, and abuse counters. Directory records remain in team-tracking. | | `DIRECTORY_BASE_URL` | team-tracking's base URL — where the gateway makes its **outbound** call. | | `DIRECTORY_API_KEY` | The gateway's **one internal** team-tracking key, scoped `identifiers:read`, plus `people:read` for email login. Follow the [directory-pinned provisioning procedure](docs/DEPLOYMENT.md#database-and-internal-access). | +| `LOGIN_STATUS_PER_MINUTE` | Status/issuer requests per consumer per minute; default 600, range 1–10000. | +| `EDGE_REQUESTS_PER_MINUTE` | Pre-authentication requests per IP per minute; default 1200, range 120–20000. Must exceed the status quota by at least 120. | | `EMAIL_LOGIN_ENABLED` | `false` by default. Enable after provisioning private verification and directory access. | | `VERIFICATION_BASE_URL` | Private verification URL; local default `http://localhost:8003`. | | `VERIFICATION_API_KEY` | A distinct scoped `vf_` key; blank/dev values are refused outside local mode when login is enabled. | @@ -161,7 +163,7 @@ Two layers, because they defend different things: | Layer | Where | Limit | Keyed on | |---|---|---|---| | Per-consumer quota | Router dependency, **after** auth | 60 / 60s | `AuthedKey.name` — an issued key | -| Flood guard | Middleware, **in front of** auth | 120 / 60s | Client IP (health routes exempt) | +| Flood guard | Middleware, **in front of** auth | `EDGE_REQUESTS_PER_MINUTE` (default 1200 / 60s) | Client IP (health routes exempt) | The quota is the one a consumer notices. It runs after `require_api_key`, so it can only ever be keyed on the name of a key we issued — the set is bounded by diff --git a/services/gateway/contracts/login.py b/services/gateway/contracts/login.py index 4b150291..0bb3c842 100644 --- a/services/gateway/contracts/login.py +++ b/services/gateway/contracts/login.py @@ -12,6 +12,14 @@ class VerificationUnavailable(Exception): """Private verification failed; no upstream payload may escape.""" +class VerificationRateLimited(VerificationUnavailable): + """Verification explicitly refused delivery before sending any email.""" + + def __init__(self, retry_after: int): + super().__init__("code delivery cooling down") + self.retry_after = retry_after + + class VerificationClient(Protocol): def request_code(self, subject: str, email: str) -> None: ... def confirm_code(self, subject: str, code: str) -> str | None: ... diff --git a/services/gateway/contracts/storage.py b/services/gateway/contracts/storage.py index ec65cde0..1c6fda50 100644 --- a/services/gateway/contracts/storage.py +++ b/services/gateway/contracts/storage.py @@ -8,7 +8,13 @@ class StorageAdapter(Protocol): def allow_login_request( - self, owner: str, operation: str, client_ip: str | None, now: datetime + self, + owner: str, + operation: str, + client_ip: str | None, + now: datetime, + *, + status_limit: int = 600, ) -> bool: """Atomically spend app and attested-IP quotas; prune expired login state.""" ... diff --git a/services/gateway/docs/API.md b/services/gateway/docs/API.md index 6c785931..6072acf8 100644 --- a/services/gateway/docs/API.md +++ b/services/gateway/docs/API.md @@ -71,7 +71,11 @@ browser's code; the UI must retain the corresponding request ID. Eligibility lookup and code delivery run after the response is sent, to keep account-dependent provider latency out of the public response. Delivery is best effort: an upstream failure or restart may leave an unusable challenge. There is -no automatic resend after ambiguous delivery. The user may request another code +no automatic resend after ambiguous delivery. An explicit verification cooldown +that guarantees no email was sent permits one deferred retry, at most 60 seconds +later, on the same challenge and reservation. Eligibility and expiry are checked +again before sending; waiting does not occupy an HTTP worker thread. Repeated +throttling or process loss can still prevent delivery. The user may request another code after the cooldown; 202 means accepted, not proof of delivery or membership. ### Confirm @@ -110,7 +114,7 @@ Windows start on the first admitted request and expire after the duration below. |---|---| | App start requests | 30/minute, 300/hour | | App confirm requests | 60/minute | -| App status/issuer requests | Separate 60/minute | +| App status/issuer requests | `LOGIN_STATUS_PER_MINUTE` (default 600/minute), separate from login admission | | Backend-attested client IP | 5 starts/minute, 30 confirms/minute, shared across apps | | Mailbox sends | 1/minute, 5/hour, 20/day, shared across apps | | Challenge guesses | 3, reserved before calling verification | @@ -129,7 +133,13 @@ bucket. Mailbox quotas include unknown/inactive addresses. Delivery failure does not refund them. App/IP exhaustion returns **429** with `Retry-After: 60` (a retry may still hit the hourly limit). Mailbox limits remain hidden behind generic 202/400 responses. Status requests have a separate app quota, but all endpoints -still share the existing pre-authentication edge-IP flood guard (120/minute). +still share the pre-authentication edge-IP flood guard (`EDGE_REQUESTS_PER_MINUTE`, +default 1200/minute). Configure the two limits together: startup rejects edge +capacity below the status budget plus 120 requests/minute. With 30-second compute +status caching, the default status budget covers roughly 300 continuously active +identities; leave room for issuer checks and size shared-IP traffic separately. +This is a request budget, not a measured throughput guarantee. Increasing it does +not extend how long a retired member can retain access. Missing/invalid key → 401; missing scope → 403; oversized body → 413; invalid input → 422; disabled login or unavailable upstream → 503. diff --git a/services/gateway/docs/ARCHITECTURE.md b/services/gateway/docs/ARCHITECTURE.md index 4a81fb4d..176148d0 100644 --- a/services/gateway/docs/ARCHITECTURE.md +++ b/services/gateway/docs/ARCHITECTURE.md @@ -58,7 +58,18 @@ verification for the opaque `gateway-login:` subject, then binds the original person ID. Background failures log only a static message; they never change the already-sent generic response. Tasks are not a durable delivery queue. On process loss, the user retries after the cooldown; budgets are retained and no -ambiguous send is retried automatically. Monitor delivery-failure logs. +ambiguous send is retried automatically. The one safe exception is verification +429 `rate_limited` with a valid 1–60 second `Retry-After`: no send occurred. The +gateway waits asynchronously and retries once on the existing challenge, without +spending another mailbox reservation. It rechecks eligibility and challenge +expiry first. The shared mailbox throttle remains intact across Discord and +compute, and a second throttle is terminal. Monitor delivery-failure logs. + +Status capacity and the pre-authentication flood guard are configured together; +see [capacity and limits](API.md#limits-and-failures). Compute retains its +30-second eligibility cache. Larger status quotas increase the allowed auth and +directory workload, so size the one-worker deployment and monitor latency/errors +before raising them; do not remove the pre-authentication guard. Before confirmation reaches verification, the gateway atomically reserves a challenge attempt. Three attempts per challenge and five deliveries per mailbox diff --git a/services/gateway/docs/CHANGE-AUDIT.md b/services/gateway/docs/CHANGE-AUDIT.md index ddd60ff2..c6f97791 100644 --- a/services/gateway/docs/CHANGE-AUDIT.md +++ b/services/gateway/docs/CHANGE-AUDIT.md @@ -178,3 +178,46 @@ Ruff gates, rebuilt gateway image and real private-HTTP/browser integration pass No live configuration was changed. The companion compute PR contains the CLI origin/redirect and revocation fixes, explicit account migration, issuer checks, and dependency/runtime updates. + + +### Independent audit follow-up: delivery and capacity + +Addresses the reproduced cross-consumer cooldown and resend-boundary failures, +and the fixed status-capacity ceiling. Verification preserves its shared mailbox +throttle and now tells authenticated callers the remaining cooldown. Gateway +retries only that explicit pre-send refusal, once, asynchronously, using the same +challenge and reservation. It rechecks eligibility/expiry and never retries an +ambiguous failure. Repeated throttling or process loss may still prevent delivery. + +Status and pre-authentication IP budgets are now configured together, with bounded +values and mandatory headroom. The defaults budget roughly 300 continuously active +compute identities with the existing 30-second cache; this is quota sizing, not a +throughput benchmark. Increasing the edge budget also permits more authentication +work, so operators must monitor load. Invalid settings omit input values from +error text to protect configured credentials. + +File rationale for the follow-up (paths relative to `services/gateway`): + +| Files | Reason | +|---|---| +| `contracts/login.py`, `src/verification/http_client.py`, `src/api/routers/email_login.py` | Represent and validate a known pre-send throttle; defer one retry without occupying a worker thread or charging again. | +| `contracts/storage.py`, `src/login_policy.py`, `src/storage/in_memory.py`, `src/storage/postgres.py` | Apply the configured status budget with the same durable semantics in both adapters. | +| `src/config.py`, `src/api/app.py`, `.env.example` | Validate and wire paired capacity settings and hide configuration inputs in startup errors. | +| `tests/test_email_login.py`, `tests/test_verification.py`, `tests/test_config.py`, `tests/login_storage_cases.py` | Regress delayed delivery, bounded retries, expiry/retirement, unsafe retry responses, capacity enforcement and credential-safe errors. | +| `tests/test_health.py` | Verify probe availability after exhausting the configured edge budget. | +| `tests/smoke_stack.py` | Reproduce both cooldown collisions through real gateway/directory/verification HTTP and disposable Postgres, retaining fake mail only. | +| `README.md`, `docs/API.md`, `docs/ARCHITECTURE.md`, `docs/DEPLOYMENT.md`, `docs/CHANGE-AUDIT.md` | Document the retry contract, limits, rollout order, file rationale and verification. | +| `/services/verification/src/api/routers/verification.py`, `/services/verification/tests/test_request_code.py` | Return a rounded-up pre-send `Retry-After` and verify cross-consumer suppression followed by successful delivery. | +| `/services/verification/README.md`, `/services/verification/docs/API.md`, `/services/verification/docs/ARCHITECTURE.md` | Explain the new response header while retaining the shared mailbox protection and unchanged confirm state machine. | +| `/AGENTS.md`, `/docs/RAILWAY-DEPLOYMENT.md` | Record paired quota sizing and verification-before-gateway rollout. | + +The delivery and capacity regressions failed before the changes and pass after +them. Both complete service suites passed against disposable Postgres, including +migration round trips and source-path assertions. The Docker HTTP regression +passed both previously failing delivery scenarios and verified one reservation +per challenge, independent redemption and retirement. Repository `make check` +and service Ruff gates passed. No real emails or live configuration were used. + +Other audit observations about quota exhaustion, shared NATs, credential blast +radius and live ingress are operational risks, not claims removed by these fixes. +The companion compute change separately revokes Misty sessions on local rollback. diff --git a/services/gateway/docs/DEPLOYMENT.md b/services/gateway/docs/DEPLOYMENT.md index 859d7df5..ddc808c6 100644 --- a/services/gateway/docs/DEPLOYMENT.md +++ b/services/gateway/docs/DEPLOYMENT.md @@ -105,8 +105,16 @@ Use the same environment for all three services and keep verification private. Set `GATEWAY_ENV=staging` or `production` explicitly: successful auth responses attest this value, and compute requires the matching `MISTY_ENVIRONMENT`. Use separate gateway keys and compute databases for staging and production. - -1. Deploy verification with a dedicated `vf_` consumer key named `gateway`, scoped +Set `LOGIN_STATUS_PER_MINUTE` (default 600) and `EDGE_REQUESTS_PER_MINUTE` +(default 1200) for expected status refreshes and total traffic from each backend +IP. Keep at least 120 requests/minute of edge headroom above the status budget. +The default status budget accommodates roughly 300 continuously active members +with a 30-second cache; allow margin for issuer checks and monitor upstream load. +See [the capacity contract](API.md#limits-and-failures). + +1. Deploy verification with the cooldown `Retry-After` response before rolling + out the gateway retry. Older verification deployments are safe but cannot + recover a deferred send. Provision a dedicated `vf_` consumer key named `gateway`, scoped only `verification:write`, using its [provisioning guide](../../verification/docs/DEPLOYMENT.md#scoped-consumer-keys). Keep the bot's existing bootstrap credential until it is separately migrated. 2. Issue a new team-tracking key with `people:read` and `identifiers:read`, using a diff --git a/services/gateway/src/api/app.py b/services/gateway/src/api/app.py index b5b48ba3..990dfb36 100644 --- a/services/gateway/src/api/app.py +++ b/services/gateway/src/api/app.py @@ -34,6 +34,7 @@ def create_app() -> FastAPI: # auth has resolved the key (see src/api/ratelimit.py). app.add_middleware( ClientRateLimitMiddleware, + limit=get_settings().edge_requests_per_minute, trust_proxy=get_settings().trust_proxy_headers, ) app.add_middleware(LoginBoundaryMiddleware) diff --git a/services/gateway/src/api/routers/email_login.py b/services/gateway/src/api/routers/email_login.py index b8485422..2e8f2a0e 100644 --- a/services/gateway/src/api/routers/email_login.py +++ b/services/gateway/src/api/routers/email_login.py @@ -1,7 +1,10 @@ import logging +from asyncio import sleep +from datetime import timedelta from uuid import UUID, uuid4 from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Response +from starlette.concurrency import run_in_threadpool from contracts.directory import DirectoryClient from contracts.login import ( @@ -13,6 +16,7 @@ LoginChallenge, PersonStatusOut, VerificationClient, + VerificationRateLimited, ) from contracts.storage import StorageAdapter from src import login_policy as policy @@ -35,7 +39,13 @@ def _enabled(): def _quota(storage, key, operation, client_ip=None): _enabled() - if not storage.allow_login_request(key.name, operation, client_ip, policy.now()): + if not storage.allow_login_request( + key.name, + operation, + client_ip, + policy.now(), + status_limit=get_settings().login_status_per_minute, + ): raise HTTPException(429, "rate limit exceeded", headers={"Retry-After": "60"}) @@ -58,18 +68,37 @@ def _eligible(person: dict | None, email: str | None = None) -> UUID | None: return person_id -def _deliver(challenge, storage, directory, verification): +def _send_and_activate(challenge, storage, directory, verification): + if policy.now() >= challenge.expires_at: + return + person_id = _eligible(directory.get_person_by_email(challenge.email), challenge.email) + if person_id is None or policy.now() >= challenge.expires_at: + return + verification.request_code(f"gateway-login:{challenge.id}", challenge.email) + storage.activate_login(challenge.id, person_id, policy.now()) + + +async def _deliver(challenge, storage, directory, verification): # Runs after the generic response has been sent. Upstream latency and failures # must not disclose account existence. No retries after ambiguous delivery. - try: - person_id = _eligible(directory.get_person_by_email(challenge.email), challenge.email) - if person_id is None: + for attempt in range(2): + try: + await run_in_threadpool(_send_and_activate, challenge, storage, directory, verification) return - verification.request_code(f"gateway-login:{challenge.id}", challenge.email) - storage.activate_login(challenge.id, person_id, policy.now()) - except Exception: - # Exceptions may carry URLs, SQL parameters or provider response bodies. + except VerificationRateLimited as exc: + # Only an explicit pre-send throttle is safe to retry. Keep the same + # challenge and quota reservation; never wait in a worker thread. + if ( + attempt == 0 + and policy.now() + timedelta(seconds=exc.retry_after) < challenge.expires_at + ): + await sleep(exc.retry_after) + continue + except Exception: + # Exceptions may carry URLs, SQL parameters or provider response bodies. + pass logger.warning("email login delivery failed") + return @router.post("/email/start", response_model=EmailStartOut, status_code=202) diff --git a/services/gateway/src/config.py b/services/gateway/src/config.py index 53032276..ff350a3d 100644 --- a/services/gateway/src/config.py +++ b/services/gateway/src/config.py @@ -1,7 +1,7 @@ from functools import lru_cache from typing import Literal -from pydantic import SecretStr +from pydantic import Field, SecretStr, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict # The built-in dev secret for the gateway's OUTBOUND team-tracking key. Only @@ -11,7 +11,7 @@ class Settings(BaseSettings): - model_config = SettingsConfigDict(env_file=".env", extra="ignore") + model_config = SettingsConfigDict(env_file=".env", extra="ignore", hide_input_in_errors=True) database_url: str = "postgresql+psycopg://gateway:dev_password@localhost:5435/gateway" @@ -39,6 +39,16 @@ class Settings(BaseSettings): # edge. X-Forwarded-For is ignored. Keep false for direct access or internal # callers, whose headers are not sanitized by Railway's public edge. trust_proxy_headers: bool = False + login_status_per_minute: int = Field(default=600, ge=1, le=10000) + edge_requests_per_minute: int = Field(default=1200, ge=120, le=20000) + + @model_validator(mode="after") + def status_capacity_has_edge_headroom(self): + if self.edge_requests_per_minute < self.login_status_per_minute + 120: + raise ValueError( + "EDGE_REQUESTS_PER_MINUTE must exceed LOGIN_STATUS_PER_MINUTE by at least 120" + ) + return self @lru_cache(maxsize=1) diff --git a/services/gateway/src/login_policy.py b/services/gateway/src/login_policy.py index c42e8f75..84790c29 100644 --- a/services/gateway/src/login_policy.py +++ b/services/gateway/src/login_policy.py @@ -20,8 +20,10 @@ def email_key(email: str) -> str: return sha256(email.encode()).hexdigest() -def request_limits(owner: str, operation: str, client_ip: str | None) -> list[Limit]: - counts = {"start": 30, "confirm": 60, "status": 60} +def request_limits( + owner: str, operation: str, client_ip: str | None, *, status_limit: int = 600 +) -> list[Limit]: + counts = {"start": 30, "confirm": 60, "status": status_limit} limits = [(f"app:{owner}:{operation}", counts[operation], 60)] if operation == "start": limits.append((f"app:{owner}:send-hour", 300, 3600)) diff --git a/services/gateway/src/storage/in_memory.py b/services/gateway/src/storage/in_memory.py index d81b9075..5b0d2823 100644 --- a/services/gateway/src/storage/in_memory.py +++ b/services/gateway/src/storage/in_memory.py @@ -33,11 +33,13 @@ def _spend(self, limits: list[policy.Limit], now: datetime) -> bool: self._limits[key] = (count + 1, expires) return True - def allow_login_request(self, owner, operation, client_ip, now): + def allow_login_request(self, owner, operation, client_ip, now, *, status_limit=600): with self._login_lock: self._logins = {k: v for k, v in self._logins.items() if v.expires_at > now} self._limits = {k: v for k, v in self._limits.items() if v[1] > now} - return self._spend(policy.request_limits(owner, operation, client_ip), now) + return self._spend( + policy.request_limits(owner, operation, client_ip, status_limit=status_limit), now + ) def reserve_login(self, challenge: LoginChallenge) -> bool: with self._login_lock: diff --git a/services/gateway/src/storage/postgres.py b/services/gateway/src/storage/postgres.py index 6ec957c9..b0599169 100644 --- a/services/gateway/src/storage/postgres.py +++ b/services/gateway/src/storage/postgres.py @@ -81,11 +81,15 @@ def _spend(self, conn, limits: list[policy.Limit], now: datetime) -> bool: ) return True - def allow_login_request(self, owner, operation, client_ip, now): + def allow_login_request(self, owner, operation, client_ip, now, *, status_limit=600): with self._login_transaction() as conn: conn.execute(delete(login_challenges).where(login_challenges.c.expires_at <= now)) conn.execute(delete(login_limits).where(login_limits.c.expires_at <= now)) - return self._spend(conn, policy.request_limits(owner, operation, client_ip), now) + return self._spend( + conn, + policy.request_limits(owner, operation, client_ip, status_limit=status_limit), + now, + ) def reserve_login(self, challenge: LoginChallenge) -> bool: with self._login_transaction() as conn: diff --git a/services/gateway/src/verification/http_client.py b/services/gateway/src/verification/http_client.py index dd6302e9..651a7058 100644 --- a/services/gateway/src/verification/http_client.py +++ b/services/gateway/src/verification/http_client.py @@ -1,6 +1,6 @@ import httpx -from contracts.login import VerificationUnavailable +from contracts.login import VerificationRateLimited, VerificationUnavailable class HttpVerificationClient: @@ -20,6 +20,16 @@ def _post(self, path: str, payload: dict) -> httpx.Response: def request_code(self, subject: str, email: str) -> None: response = self._post("request-code", {"subject": subject, "email": email}) + if response.status_code == 429: + delay = response.headers.get("Retry-After", "") + try: + known_cooldown = response.json() == {"detail": "rate_limited"} + except ValueError: + known_cooldown = False + if known_cooldown and len(delay) <= 2 and delay.isascii() and delay.isdecimal(): + seconds = int(delay) + if 1 <= seconds <= 60: + raise VerificationRateLimited(seconds) if response.status_code != 202: raise VerificationUnavailable("code delivery unavailable") diff --git a/services/gateway/tests/login_storage_cases.py b/services/gateway/tests/login_storage_cases.py index 0fecf4e6..43b0331e 100644 --- a/services/gateway/tests/login_storage_cases.py +++ b/services/gateway/tests/login_storage_cases.py @@ -119,3 +119,10 @@ def test_expired_rows_are_pruned_without_resetting_live_email_budget(self, adapt for minute in range(11, 15): assert adapter.reserve_login(challenge(NOW + timedelta(minutes=minute))) assert not adapter.reserve_login(challenge(NOW + timedelta(minutes=15))) + + def test_configured_status_budget_remains_bounded(self, adapter): + for _ in range(75): + assert adapter.allow_login_request("compute", "status", None, NOW, status_limit=75) + assert not adapter.allow_login_request("compute", "status", None, NOW, status_limit=75) + assert adapter.allow_login_request("other", "status", None, NOW, status_limit=75) + assert adapter.allow_login_request("compute", "start", "192.0.2.1", NOW) diff --git a/services/gateway/tests/smoke_stack.py b/services/gateway/tests/smoke_stack.py index b7efe69f..b9d50ead 100644 --- a/services/gateway/tests/smoke_stack.py +++ b/services/gateway/tests/smoke_stack.py @@ -90,6 +90,7 @@ def start(name, image, env, *, public=False, command=()): gateway_db = f"postgresql+psycopg://smoke:{password}@{pg}:5432/gateway" verification_db = f"postgresql+psycopg://smoke:{password}@{pg}:5432/verification" verification = network + "-verification" + verification_bootstrap = secrets.token_urlsafe(32) credential = json.loads( docker( "run", @@ -121,6 +122,7 @@ def start(name, image, env, *, public=False, command=()): "verification:ci", { "DATABASE_URL": verification_db, + "API_KEY": verification_bootstrap, "VF_ENV": "local", "EMAIL_BACKEND": "fake", "CONSUMER_KEYS": json.dumps([credential["entry"]]), @@ -261,6 +263,29 @@ def start(name, image, env, *, public=False, command=()): "auth:email", "auth:status", ) + # A separate private consumer has just sent a Discord-link code. Age the + # fixture's timestamp to avoid spending a minute in CI, retaining the real + # verification cooldown and HTTP response that previously lost this login. + assert ( + http( + verification_url, + "/verification/request-code", + key=verification_bootstrap, + payload={"subject": "discord:smoke", "email": "smoke@example.test"}, + )[0] + == 202 + ) + docker( + "exec", + pg, + "psql", + "-U", + "smoke", + "-d", + "verification", + "-c", + "UPDATE verification_codes SET created_at=now()-interval '55 seconds' WHERE subject='discord:smoke'", + ) status, challenge = http( gateway_url, "/v1/auth/email/start", @@ -285,6 +310,81 @@ def start(name, image, env, *, public=False, command=()): == "1" ) ) + # Simulate the gateway minute expiring before verification's minute. The + # resend must eventually activate without superseding the first code. + docker( + "exec", + pg, + "psql", + "-U", + "smoke", + "-d", + "gateway", + "-c", + "UPDATE login_limits SET expires_at=now()-interval '1 second' WHERE bucket LIKE 'email:%:send:60'", + ) + docker( + "exec", + pg, + "psql", + "-U", + "smoke", + "-d", + "verification", + "-c", + f"UPDATE verification_codes SET created_at=now()-interval '55 seconds' WHERE subject='gateway-login:{challenge_id}'", + ) + resend_status, resend = http( + gateway_url, + "/v1/auth/email/start", + key=compute_key, + payload={"email": "smoke@example.test", "client_ip": "192.0.2.2"}, + ) + assert resend_status == 202 + resend_id = resend["challenge_id"] + wait_for( + lambda: ( + docker( + "exec", + pg, + "psql", + "-U", + "smoke", + "-d", + "gateway", + "-Atc", + f"SELECT count(*) FROM login_challenges WHERE id='{resend_id}' AND person_id IS NOT NULL", + ) + == "1" + ) + ) + assert ( + http( + gateway_url, + "/v1/auth/email/confirm", + key=compute_key, + payload={"challenge_id": resend_id, "code": "123456", "client_ip": "192.0.2.2"}, + )[0] + == 200 + ) + assert ( + docker( + "exec", + pg, + "psql", + "-U", + "smoke", + "-d", + "gateway", + "-Atc", + "SELECT count FROM login_limits WHERE bucket LIKE 'email:%:send:3600'", + ) + == "2" + ) + print( + "PASS: shared-mailbox cooldown and resend timing recover over real private HTTP", + flush=True, + ) payload = {"challenge_id": challenge_id, "code": "123456", "client_ip": "192.0.2.1"} assert http(gateway_url, "/v1/auth/email/confirm", key=compute_key, payload=payload) == ( 200, diff --git a/services/gateway/tests/test_config.py b/services/gateway/tests/test_config.py index b0d7ac6a..7d763331 100644 --- a/services/gateway/tests/test_config.py +++ b/services/gateway/tests/test_config.py @@ -74,3 +74,24 @@ def test_verification_key_is_redacted(): settings = _settings(verification_api_key="test-private-key") assert "test-private-key" not in repr(settings) assert settings.verification_api_key.get_secret_value() == "test-private-key" + + +@pytest.mark.parametrize( + "overrides", + [ + {"login_status_per_minute": 0}, + {"edge_requests_per_minute": 0}, + {"login_status_per_minute": 600, "edge_requests_per_minute": 600}, + {"login_status_per_minute": 10001, "edge_requests_per_minute": 20000}, + ], +) +def test_status_capacity_requires_bounded_limits_and_edge_headroom(overrides): + with pytest.raises(ValueError): + _settings(**overrides) + + +def test_invalid_capacity_does_not_print_configured_credentials(): + with pytest.raises(ValueError) as exc: + Settings(_env_file=None, directory_api_key="Z", edge_requests_per_minute=120) + assert "input_value=" not in str(exc.value) + assert "Z" not in str(exc.value) diff --git a/services/gateway/tests/test_email_login.py b/services/gateway/tests/test_email_login.py index bed1db1d..3400b6c4 100644 --- a/services/gateway/tests/test_email_login.py +++ b/services/gateway/tests/test_email_login.py @@ -378,3 +378,92 @@ def unavailable(*args): second = confirm(client, missing, "999999") assert first.status_code == second.status_code == 503 assert first.json() == second.json() + + +def test_status_capacity_exceeds_thirty_active_members(login): + client, _, directory, _ = login + directory.get_person = lambda person_id: {**PERSON, "id": person_id} + people = [str(uuid4()) for _ in range(100)] + for _ in range(2): + for person_id in people: + response = client.get(f"/v1/auth/people/{person_id}/status") + assert response.status_code == 200 + assert response.json()["active"] is True + + +@pytest.mark.parametrize("retry_outcome", ["sent", "retired", "expired", "throttled"]) +def test_explicit_delivery_cooldown_is_retried_once(login, monkeypatch, retry_outcome): + import json + import httpx + from src.api.routers import email_login + from src.verification.http_client import HttpVerificationClient + + client, _, directory, verification = login + original = verification.request_code + calls = [] + waits = [] + clock = [policy.now()] + monkeypatch.setattr(policy, "now", lambda: clock[0]) + + async def wait(seconds): + waits.append(seconds) + clock[0] += timedelta(seconds=seconds) + if retry_outcome == "retired": + directory.person["active"] = False + if retry_outcome == "expired": + clock[0] += timedelta(minutes=10) + + monkeypatch.setattr(email_login, "sleep", wait, raising=False) + + def handle(request): + data = json.loads(request.content) + calls.append(data) + if len(calls) == 1 or retry_outcome == "throttled": + return httpx.Response( + 429, json={"detail": "rate_limited"}, headers={"Retry-After": "1"} + ) + original(data["subject"], data["email"]) + return httpx.Response(202) + + upstream = HttpVerificationClient( + "http://verification.test", + "test-key", + client=httpx.Client(transport=httpx.MockTransport(handle)), + ) + verification.request_code = upstream.request_code + try: + response = start(client) + assert response.status_code == 202 + assert waits == [1] + assert len(calls) == (1 if retry_outcome in {"retired", "expired"} else 2) + if retry_outcome == "sent": + assert confirm(client, response.json()["challenge_id"]).status_code == 200 + else: + assert not verification.sent + finally: + upstream.close() + + +@pytest.mark.parametrize("status", [429, 500, 502]) +def test_ambiguous_delivery_failure_never_retries(login, status): + import httpx + from src.verification.http_client import HttpVerificationClient + + client, _, _, verification = login + calls = [] + + def handle(request): + calls.append(request) + return httpx.Response(status, json={"detail": "unknown"}) + + upstream = HttpVerificationClient( + "http://verification.test", + "test-key", + client=httpx.Client(transport=httpx.MockTransport(handle)), + ) + verification.request_code = upstream.request_code + try: + assert start(client).status_code == 202 + assert len(calls) == 1 + finally: + upstream.close() diff --git a/services/gateway/tests/test_health.py b/services/gateway/tests/test_health.py index ddb4f8fd..66b1f150 100644 --- a/services/gateway/tests/test_health.py +++ b/services/gateway/tests/test_health.py @@ -7,7 +7,7 @@ from src.api.app import create_app from src.api.deps import get_storage -from src.api.ratelimit import IP_LIMIT +from src.config import get_settings from src.api.readiness import READINESS_CACHE_SECONDS, ReadinessProbe from src.storage.in_memory import InMemoryStorageAdapter from src.storage.postgres import PostgresStorageAdapter @@ -63,11 +63,12 @@ def test_public_readiness_requests_do_not_repeat_database_work_after_ip_limit(cl store = CountingStore() app = create_app() app.dependency_overrides[get_storage] = lambda: store + limit = get_settings().edge_requests_per_minute with TestClient(app) as client: - for _ in range(IP_LIMIT): + for _ in range(limit): assert client.get("/v1/resolve/discord/example").status_code == 401 assert client.get("/v1/resolve/discord/example").status_code == 429 - for _ in range(IP_LIMIT + 1): + for _ in range(limit + 1): assert client.get("/health/ready").status_code == 200 assert store.calls == 1 diff --git a/services/gateway/tests/test_verification.py b/services/gateway/tests/test_verification.py index c0ccfcc4..af575fa5 100644 --- a/services/gateway/tests/test_verification.py +++ b/services/gateway/tests/test_verification.py @@ -69,3 +69,30 @@ def broken(request): upstream.request_code("opaque", "a@b.com") assert "private-request-data" not in str(exc.value) upstream.close() + + +@pytest.mark.parametrize( + "header,body", + [ + (None, {"detail": "rate_limited"}), + ("0", {"detail": "rate_limited"}), + ("61", {"detail": "rate_limited"}), + ("-1", {"detail": "rate_limited"}), + ("1.5", {"detail": "rate_limited"}), + ("NaN", {"detail": "rate_limited"}), + ("9" * 100, {"detail": "rate_limited"}), + ("1", {"detail": "different_failure"}), + ("1", None), + ], +) +def test_unknown_throttles_never_authorize_retry(header, body): + from contracts.login import VerificationRateLimited + + headers = {"Retry-After": header} if header is not None else {} + upstream = client(lambda _: httpx.Response(429, json=body, headers=headers)) + try: + with pytest.raises(VerificationUnavailable) as exc: + upstream.request_code("opaque", "a@b.com") + assert not isinstance(exc.value, VerificationRateLimited) + finally: + upstream.close() diff --git a/services/verification/README.md b/services/verification/README.md index ac22d7b8..b883fd51 100644 --- a/services/verification/README.md +++ b/services/verification/README.md @@ -89,7 +89,7 @@ adds single-use login redemption and eligibility checks around private HTTP call | Method | Path | Scope | Description | |--------|------|-------|-------------| -| POST | `/verification/request-code` | `verification:write` | Generate + email a one-time code for `{subject, email}`. **202 Accepted** on success | +| POST | `/verification/request-code` | `verification:write` | Generate + email a one-time code for `{subject, email}`. **202 Accepted** on success; pre-send cooldown returns **429** with `Retry-After`. | | POST | `/verification/confirm-code` | `verification:write` | Verify `{subject, code}`; **200** → `{verified, subject, email}` | | GET | `/health` | — (none) | Liveness probe → `{"status": "ok"}` | diff --git a/services/verification/docs/API.md b/services/verification/docs/API.md index db0af87f..8dc3ee2f 100644 --- a/services/verification/docs/API.md +++ b/services/verification/docs/API.md @@ -64,7 +64,7 @@ curl -sS -X POST http://localhost:8003/verification/request-code \ ### Behavior -- **Rate limited per email.** If an unconsumed code for that *email* was created within `RATE_LIMIT_WINDOW` (60 s), the request is rejected with **429 `rate_limited`**. Note the limit keys on email, not subject — it protects the mailbox regardless of how many subjects a caller invents. +- **Rate limited per email.** If an unconsumed code for that *email* was created within `RATE_LIMIT_WINDOW` (60 s), the request is rejected with **429 `rate_limited`**. The response includes `Retry-After` with the remaining window rounded up to whole seconds; this 429 occurs before any email send. Note the limit keys on email, not subject — it protects the mailbox regardless of how many subjects a caller invents. - **Send before persist.** The email goes out first. On `EmailSendError` the endpoint returns **502 `email_send_failed`** and stores nothing, so the caller isn't rate-limited out of an immediate retry. - **Supersede on reissue.** A new request for an existing subject atomically replaces that subject's row. Only the newest code is ever live. - The code is 6 digits, generated with `secrets`, expires after `CODE_TTL` (10 min), and is stored only as an HMAC-SHA256 digest. diff --git a/services/verification/docs/ARCHITECTURE.md b/services/verification/docs/ARCHITECTURE.md index e3a29aab..3e2e05b4 100644 --- a/services/verification/docs/ARCHITECTURE.md +++ b/services/verification/docs/ARCHITECTURE.md @@ -72,6 +72,9 @@ The cost is a genuine (small) race: mail delivered, then the write fails. The us So only the newest code is ever live: reissuing invalidates the previous one implicitly, with no cleanup pass and no way for two valid codes to coexist for one subject. Note the asymmetry: the **rate limit is keyed by email**, the **row is keyed by subject**. That's on purpose — the limit protects the *mailbox* from being flooded across whatever subjects an attacker invents, while the row models "the one proof in flight for this subject." +A throttled request returns `Retry-After` rounded up to the remaining whole +seconds before any send. Gateway can defer that specific failure without +bypassing the mailbox limit; unknown errors must not authorize a retry. ## The confirm state machine diff --git a/services/verification/src/api/routers/verification.py b/services/verification/src/api/routers/verification.py index dbab9704..48352345 100644 --- a/services/verification/src/api/routers/verification.py +++ b/services/verification/src/api/routers/verification.py @@ -1,4 +1,5 @@ from datetime import datetime, timezone +from math import ceil from fastapi import APIRouter, Depends, HTTPException, status @@ -38,7 +39,12 @@ def request_code( now = _now() recent = storage.latest_unconsumed_for_email(payload.email) if recent is not None and (now - recent.created_at) < RATE_LIMIT_WINDOW: - raise HTTPException(status_code=status.HTTP_429_TOO_MANY_REQUESTS, detail="rate_limited") + remaining = RATE_LIMIT_WINDOW - (now - recent.created_at) + raise HTTPException( + status_code=status.HTTP_429_TOO_MANY_REQUESTS, + detail="rate_limited", + headers={"Retry-After": str(max(1, ceil(remaining.total_seconds())))}, + ) code = generate_code() # Send before persisting: if delivery fails we neither store an unsendable diff --git a/services/verification/tests/test_request_code.py b/services/verification/tests/test_request_code.py index 7a9606b3..8f4bab1d 100644 --- a/services/verification/tests/test_request_code.py +++ b/services/verification/tests/test_request_code.py @@ -43,3 +43,28 @@ def send(self, *, to, subject, body): assert r.json()["detail"] == "email_send_failed" # A failed send must not leave a code behind (else the user is rate-limited). assert store.get_code("discord:9") is None + + +def test_cross_consumer_cooldown_reports_safe_retry_delay(client, email, monkeypatch): + from datetime import datetime, timedelta, timezone + from src.api.routers import verification + + clock = [datetime(2026, 10, 26, tzinfo=timezone.utc)] + monkeypatch.setattr(verification, "_now", lambda: clock[0]) + assert ( + client.post( + "/verification/request-code", + headers=AUTH, + json={"subject": "discord:1", "email": "a@b.com"}, + ).status_code + == 202 + ) + clock[0] += timedelta(seconds=59.2) + body = {"subject": "gateway-login:independent", "email": "a@b.com"} + response = client.post("/verification/request-code", headers=AUTH, json=body) + assert response.status_code == 429 + assert response.headers.get("retry-after") == "1" + assert len(email.sent) == 1 + clock[0] += timedelta(seconds=1) + assert client.post("/verification/request-code", headers=AUTH, json=body).status_code == 202 + assert len(email.sent) == 2 From 88ccb9b00ff9a94ffef3828713fa315356520763 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Mon, 9 Nov 2026 15:54:05 -0500 Subject: [PATCH 4/5] fix(gateway): harden email login admission and request boundaries --- services/gateway/contracts/login.py | 2 + services/gateway/docs/API.md | 22 ++- services/gateway/docs/ARCHITECTURE.md | 19 ++- services/gateway/src/api/app.py | 6 +- services/gateway/src/api/middleware.py | 87 ++++++++---- services/gateway/src/login_policy.py | 3 + services/gateway/tests/login_storage_cases.py | 34 +++++ services/gateway/tests/test_email_login.py | 29 ++++ services/gateway/tests/test_login_boundary.py | 129 ++++++++++++++++++ .../gateway/tests/test_postgres_adapter.py | 58 +++++++- 10 files changed, 350 insertions(+), 39 deletions(-) create mode 100644 services/gateway/tests/test_login_boundary.py diff --git a/services/gateway/contracts/login.py b/services/gateway/contracts/login.py index 0bb3c842..e3864ff5 100644 --- a/services/gateway/contracts/login.py +++ b/services/gateway/contracts/login.py @@ -47,6 +47,8 @@ class EmailStartIn(BaseModel): @field_validator("email") @classmethod def normalize_email(cls, value: str) -> str: + if not value.isprintable(): + raise ValueError("invalid email") value = value.strip().lower() if not re.fullmatch(r"[^@\s]+@[^@\s]+\.[^@\s]+", value): raise ValueError("invalid email") diff --git a/services/gateway/docs/API.md b/services/gateway/docs/API.md index 6072acf8..e520c30b 100644 --- a/services/gateway/docs/API.md +++ b/services/gateway/docs/API.md @@ -40,8 +40,12 @@ occurs. Only `Person.primary_email` is accepted (trimmed and lowercased); second identifiers are not login addresses. Eligibility requires `active=true`. All responses, including errors, use `Cache-Control: no-store`. Input models -reject extra fields. Login bodies are limited to 4096 bytes; errors never echo -submitted codes. Codes must be exactly six ASCII digits. Successful auth responses include an +reject extra fields. Login bodies are limited to 4096 bytes and must arrive within +five seconds total, including chunked uploads. HTTP/1 connections close on login +408, 413, and 429 responses so rejected uploads cannot linger. The edge-IP limit +runs before the body reader, even when login is disabled. Email addresses containing control or +other non-printable characters are rejected before login admission. Errors never +echo submitted codes. Codes must be exactly six ASCII digits. Successful auth responses include an `issuer` derived from `GATEWAY_ENV`: `misty:local`, `misty:staging`, or `misty:production`. Consumers must check the expected issuer on every success. @@ -115,7 +119,7 @@ Windows start on the first admitted request and expire after the duration below. | App start requests | 30/minute, 300/hour | | App confirm requests | 60/minute | | App status/issuer requests | `LOGIN_STATUS_PER_MINUTE` (default 600/minute), separate from login admission | -| Backend-attested client IP | 5 starts/minute, 30 confirms/minute, shared across apps | +| Backend-attested client IP | 5 starts/minute, 20 starts/hour, 30 confirms/minute, shared across apps | | Mailbox sends | 1/minute, 5/hour, 20/day, shared across apps | | Challenge guesses | 3, reserved before calling verification | | Challenge lifetime | 10 minutes from start, including delivery time | @@ -129,8 +133,12 @@ an attacker can exhaust new-send capacity, so edge abuse protection and monitori remain necessary. An already-delivered code remains usable through send throttling. IPv4-mapped IPv6 addresses normalize to IPv4; native IPv6 addresses share a /64 -bucket. Mailbox quotas include unknown/inactive addresses. Delivery failure does -not refund them. App/IP exhaustion returns **429** with `Retry-After: 60` (a retry +bucket. The hourly client limit prevents a few clients from spending the app's +entire hourly start budget. Users behind the same NAT or IPv6 /64 share this +budget; sufficiently distributed traffic can still exhaust an app's capacity. +Rejected app/IP admissions spend none of the other request buckets. Mailbox +quotas include unknown/inactive addresses. Delivery failure does not refund them. +App/IP exhaustion returns **429** with `Retry-After: 60` (a retry may still hit the hourly limit). Mailbox limits remain hidden behind generic 202/400 responses. Status requests have a separate app quota, but all endpoints still share the pre-authentication edge-IP flood guard (`EDGE_REQUESTS_PER_MINUTE`, @@ -141,8 +149,8 @@ identities; leave room for issuer checks and size shared-IP traffic separately. This is a request budget, not a measured throughput guarantee. Increasing it does not extend how long a retired member can retain access. -Missing/invalid key → 401; missing scope → 403; oversized body → 413; -invalid input → 422; disabled login or unavailable upstream → 503. +Missing/invalid key → 401; missing scope → 403; body receive deadline → 408; +oversized body → 413; invalid input → 422; disabled login or unavailable upstream → 503. ### Compute integration responsibilities diff --git a/services/gateway/docs/ARCHITECTURE.md b/services/gateway/docs/ARCHITECTURE.md index 176148d0..33c8d37e 100644 --- a/services/gateway/docs/ARCHITECTURE.md +++ b/services/gateway/docs/ARCHITECTURE.md @@ -13,9 +13,14 @@ verifies the envelope, hash, revocation state, and scope. No environment bootstr key is accepted, and the CLI allows only explicitly supported external scopes. The resolver path is audit middleware → IP limit → authentication → consumer quota -→ endpoint scope → upstream client → curated response. Login adds a bounded body -reader and no-store headers, then scope checks and durable login quotas. `deps.py` supplies the -pooled HTTP client through the `DirectoryClient` Protocol. Each app lifespan owns +→ endpoint scope → upstream client → curated response. Login wraps the IP limit +with no-store headers, then reads at most 4096 body bytes within one five-second +deadline before JSON parsing, scope checks, and durable login quotas. Throttled +requests are rejected before any body read; incoming chunks cannot extend the +deadline. Login 408/413/429 responses close HTTP/1 connections so unfinished +uploads cannot outlive rejection by cancelling the server's idle timeout. +`deps.py` supplies the pooled HTTP client through the `DirectoryClient` Protocol. +Each app lifespan owns its client, constructed lazily after scope checks under a lock so concurrent first requests share one pool. Shutdown closes that pool after requests drain; an unused app creates no client, and separate apps never share or close each other's pool. @@ -93,8 +98,12 @@ and the next request. Verification retains separate HMAC/email rows; follow its The authenticated compute backend attests `client_ip`; it must derive that value from trusted ingress. This does not alter edge `X-Real-IP` handling. Mailbox and -application quotas do not depend on the attestation. Keep the existing one-replica, -one-worker limit because the pre-authentication flood guard and resolver quotas +application quotas do not depend on the attestation. A persistent hourly start +limit per client bounds how much of an app's hourly capacity a few clients can +consume; it is shared across apps and by users behind the same NAT or IPv6 /64. +See the [limits and remaining abuse trade-offs](API.md#limits-and-failures). +Keep the existing one-replica, one-worker limit because the pre-authentication +flood guard and resolver quotas are still process-local. Persistent login quotas alone do not authorize scaling. Successful auth responses attest `misty:`. The authenticated issuer diff --git a/services/gateway/src/api/app.py b/services/gateway/src/api/app.py index 990dfb36..d9f5b478 100644 --- a/services/gateway/src/api/app.py +++ b/services/gateway/src/api/app.py @@ -9,7 +9,7 @@ from contracts.storage import StorageAdapter from contracts.login import VerificationUnavailable from src.api.deps import get_storage, lifespan -from src.api.middleware import AuditLogMiddleware, LoginBoundaryMiddleware +from src.api.middleware import AuditLogMiddleware, LoginBodyLimitMiddleware, LoginBoundaryMiddleware from src.api.ratelimit import ClientRateLimitMiddleware from src.api.readiness import ReadinessProbe from src.api.routers import resolve, email_login @@ -31,7 +31,9 @@ def create_app() -> FastAPI: # outermost. Audit must be outermost so it still records the requests the # flood guard short-circuits — a 429 storm is exactly what you want in the # log. The per-key quota is not here; it runs as a router dependency, after - # auth has resolved the key (see src/api/ratelimit.py). + # auth has resolved the key (see src/api/ratelimit.py). Login no-store wraps + # the IP limit, which must reject floods before reading any request body. + app.add_middleware(LoginBodyLimitMiddleware) app.add_middleware( ClientRateLimitMiddleware, limit=get_settings().edge_requests_per_minute, diff --git a/services/gateway/src/api/middleware.py b/services/gateway/src/api/middleware.py index 7c3b9adb..bdaed987 100644 --- a/services/gateway/src/api/middleware.py +++ b/services/gateway/src/api/middleware.py @@ -1,12 +1,16 @@ -"""Thin shim re-exporting the shared audit middleware.""" +"""Login request boundaries and the shared audit middleware.""" + +from asyncio import timeout from platform_auth import AuditLogMiddleware # noqa: F401 from starlette.responses import JSONResponse +LOGIN_BODY_TIMEOUT_SECONDS = 5 + class LoginBoundaryMiddleware: - """Bound login bodies before JSON parsing and prevent caching, including errors.""" + """Prevent caching and close rejected HTTP/1 login connections.""" def __init__(self, app): self.app = app @@ -22,28 +26,63 @@ async def private_send(message): for key, value in message["headers"] if key.lower() != b"cache-control" ] + [(b"cache-control", b"no-store")] + if message["status"] in {408, 413, 429} and scope["http_version"] in {"1.0", "1.1"}: + # Uvicorn's idle timeout can be cancelled by another body byte + # after an early response. Explicitly release rejected uploads. + message["headers"] = [ + (key, value) + for key, value in message["headers"] + if key.lower() != b"connection" + ] + [(b"connection", b"close")] await send(message) - if scope["method"] == "POST": - chunks = [] - size = 0 - while True: - message = await receive() - if message["type"] == "http.disconnect": - return - size += len(message.get("body", b"")) - if size > 4096: - response = JSONResponse( - status_code=413, content={"detail": "request too large"} - ) - return await response(scope, receive, private_send) - chunks.append(message) - if not message.get("more_body", False): - break - pending = iter(chunks) - - async def bounded_receive(): - return next(pending, None) or await receive() - - return await self.app(scope, bounded_receive, private_send) return await self.app(scope, receive, private_send) + + +class LoginBodyLimitMiddleware: + """Bound body size and total receive time after the pre-authentication IP limit.""" + + def __init__(self, app): + self.app = app + + async def __call__(self, scope, receive, send): + if ( + scope["type"] != "http" + or not scope["path"].startswith("/v1/auth/") + or scope["method"] != "POST" + ): + return await self.app(scope, receive, send) + + body = bytearray() + try: + # One deadline for the entire body; small periodic chunks cannot extend it. + async with timeout(LOGIN_BODY_TIMEOUT_SECONDS): + while True: + message = await receive() + if message["type"] == "http.disconnect": + return + chunk = message.get("body", b"") + if len(body) + len(chunk) > 4096: + response = JSONResponse( + status_code=413, content={"detail": "request too large"} + ) + break + body.extend(chunk) + if not message.get("more_body", False): + response = None + break + except TimeoutError: + response = JSONResponse(status_code=408, content={"detail": "request body timed out"}) + if response is not None: + return await response(scope, receive, send) + + pending = True + + async def bounded_receive(): + nonlocal pending + if pending: + pending = False + return {"type": "http.request", "body": bytes(body), "more_body": False} + return await receive() + + return await self.app(scope, bounded_receive, send) diff --git a/services/gateway/src/login_policy.py b/services/gateway/src/login_policy.py index 84790c29..ab28de0d 100644 --- a/services/gateway/src/login_policy.py +++ b/services/gateway/src/login_policy.py @@ -34,6 +34,9 @@ def request_limits( network = str(ip_network(f"{ip}/64", strict=False)) if ip.version == 6 else str(ip) digest = sha256(network.encode()).hexdigest() limits.append((f"ip:{digest}:{operation}", 5 if operation == "start" else 30, 60)) + if operation == "start": + # A few clients must not consume the entire app's hourly start budget. + limits.append((f"ip:{digest}:start-hour", 20, 3600)) return limits diff --git a/services/gateway/tests/login_storage_cases.py b/services/gateway/tests/login_storage_cases.py index 43b0331e..daa74ff1 100644 --- a/services/gateway/tests/login_storage_cases.py +++ b/services/gateway/tests/login_storage_cases.py @@ -2,6 +2,8 @@ from datetime import datetime, timedelta, timezone from uuid import uuid4 +import pytest + from contracts.login import LoginChallenge NOW = datetime(2026, 10, 26, tzinfo=timezone.utc) @@ -111,6 +113,38 @@ def test_ipv6_rotation_and_mapped_ipv4_do_not_reset_quota(self, adapter): assert adapter.allow_login_request("compute", "start", "192.0.2.1", NOW) assert not adapter.allow_login_request("compute", "start", "::ffff:192.0.2.1", NOW) + def test_two_clients_cannot_exhaust_app_hourly_start_budget(self, adapter): + admitted = 0 + for minute in range(30): + at = NOW + timedelta(minutes=minute) + for ip in ("192.0.2.1", "192.0.2.2"): + for _ in range(5): + admitted += adapter.allow_login_request("compute", "start", ip, at) + assert admitted == 40 + assert adapter.allow_login_request( + "compute", "start", "192.0.2.99", NOW + timedelta(minutes=30) + ) + assert adapter.allow_login_request( + "compute", "start", "192.0.2.1", NOW + timedelta(hours=1) + ) + + @pytest.mark.parametrize( + "first,second", + [("192.0.2.1", "::ffff:192.0.2.1"), ("2001:db8::1", "2001:db8::99")], + ) + def test_hourly_client_quota_is_shared_across_apps_and_address_forms( + self, adapter, first, second + ): + for minute in range(4): + for _ in range(5): + assert adapter.allow_login_request( + f"app-{minute}", "start", first, NOW + timedelta(minutes=minute) + ) + at = NOW + timedelta(minutes=5) + assert not adapter.allow_login_request("new-app", "start", second, at) + assert adapter.allow_login_request("new-app", "confirm", second, at) + assert adapter.allow_login_request("new-app", "status", None, at) + def test_expired_rows_are_pruned_without_resetting_live_email_budget(self, adapter): item = challenge() assert adapter.reserve_login(item) diff --git a/services/gateway/tests/test_email_login.py b/services/gateway/tests/test_email_login.py index 3400b6c4..5a01222f 100644 --- a/services/gateway/tests/test_email_login.py +++ b/services/gateway/tests/test_email_login.py @@ -82,6 +82,35 @@ def confirm(client, challenge, code="123456", **extra): ) +@pytest.mark.parametrize( + "email", + [ + "member\x00@example.com", + "member\x1f@example.com", + "member\x7f@example.com", + "member\u200b@example.com", + "member\u202e@example.com", + "\tmember@example.com", + ], +) +def test_nonprintable_email_is_rejected_before_login_work(login, monkeypatch, email): + client, store, directory, verification = login + calls = [] + + def unexpected(*args, **kwargs): + calls.append(True) + return False + + monkeypatch.setattr(store, "allow_login_request", unexpected) + monkeypatch.setattr(directory, "get_person_by_email", unexpected) + response = start(client, email) + assert response.status_code == 422 + assert response.json() == {"detail": "invalid login request"} + assert response.headers["cache-control"] == "no-store" + assert not calls + assert not verification.sent + + def test_other_challenges_cannot_exhaust_a_valid_code(login, monkeypatch): client, _, _, verification = login clock = [datetime(2026, 10, 26, tzinfo=timezone.utc)] diff --git a/services/gateway/tests/test_login_boundary.py b/services/gateway/tests/test_login_boundary.py new file mode 100644 index 00000000..c12b490d --- /dev/null +++ b/services/gateway/tests/test_login_boundary.py @@ -0,0 +1,129 @@ +import asyncio + +import pytest +from starlette.requests import Request +from starlette.responses import Response + +from src.api import middleware +from src.api.app import create_app +from src.config import Settings, get_settings + + +@pytest.fixture +def app(monkeypatch): + monkeypatch.setitem(Settings.model_config, "env_file", None) + monkeypatch.setenv("GATEWAY_ENV", "local") + monkeypatch.setenv("EDGE_REQUESTS_PER_MINUTE", "121") + monkeypatch.setenv("LOGIN_STATUS_PER_MINUTE", "1") + get_settings.cache_clear() + yield create_app() + get_settings.cache_clear() + + +async def request(app, receive, *, method="POST", path="/v1/auth/probe", http_version="1.1"): + messages = [] + + async def send(message): + messages.append(message) + + scope = { + "type": "http", + "asgi": {"version": "3.0", "spec_version": "2.4"}, + "http_version": http_version, + "method": method, + "scheme": "http", + "path": path, + "query_string": b"", + "headers": [], + "client": ("192.0.2.1", 12345), + "server": ("test", 80), + } + await asyncio.wait_for(app(scope, receive, send), timeout=1) + return messages + + +def assert_private_status(messages, status, *, http_version="1.1"): + response = next(m for m in messages if m["type"] == "http.response.start") + assert response["status"] == status + assert dict(response["headers"])[b"cache-control"] == b"no-store" + headers = dict(response["headers"]) + if status in {408, 413, 429} and http_version in {"1.0", "1.1"}: + assert headers.get(b"connection") == b"close" + else: + assert b"connection" not in headers + + +@pytest.mark.parametrize("enabled", ["true", "false"]) +def test_throttled_login_is_rejected_without_reading_body(app, monkeypatch, enabled): + monkeypatch.setenv("EMAIL_LOGIN_ENABLED", enabled) + get_settings.cache_clear() + + async def never_read(): + pytest.fail("a throttled request must not wait for its body") + + async def exercise(): + for _ in range(121): + assert_private_status(await request(app, never_read, method="GET"), 404) + assert_private_status(await request(app, never_read, path="/v1/auth/email/start"), 429) + + asyncio.run(exercise()) + + +@pytest.mark.parametrize("chunk", [None, b"x", b""]) +def test_body_deadline_is_not_extended_by_trickled_chunks(app, monkeypatch, chunk): + # Shorten the production deadline so stalled and trickling streams are cheap to test. + monkeypatch.setattr(middleware, "LOGIN_BODY_TIMEOUT_SECONDS", 0.05) + + @app.post("/v1/auth/probe") + async def unreachable(): + pytest.fail("an incomplete body must not reach the router") + + async def slow_receive(): + if chunk is None: + await asyncio.Future() + await asyncio.sleep(0.01) + return {"type": "http.request", "body": chunk, "more_body": True} + + async def exercise(): + assert_private_status(await request(app, slow_receive), 408) + + asyncio.run(exercise()) + + +def test_chunked_body_is_preserved(app): + body = b'{"example":"' + b"x" * 4082 + b'"}' + assert len(body) == 4096 + + @app.post("/v1/auth/probe") + async def echo(request: Request): + return Response(await request.body()) + + async def exercise(): + chunks = iter([b"", body[:1000], b"", body[1000:]]) + + async def receive(): + chunk = next(chunks) + return {"type": "http.request", "body": chunk, "more_body": chunk != body[1000:]} + + messages = await request(app, receive) + assert_private_status(messages, 200) + assert b"".join(m.get("body", b"") for m in messages) == body + + asyncio.run(exercise()) + + +@pytest.mark.parametrize("http_version", ["1.0", "1.1", "2"]) +def test_chunked_oversized_body_is_rejected_before_router(app, http_version): + @app.post("/v1/auth/probe") + async def unreachable(): + pytest.fail("an oversized body must not reach the router") + + async def receive(): + return {"type": "http.request", "body": b"x" * 2049, "more_body": True} + + async def exercise(): + assert_private_status( + await request(app, receive, http_version=http_version), 413, http_version=http_version + ) + + asyncio.run(exercise()) diff --git a/services/gateway/tests/test_postgres_adapter.py b/services/gateway/tests/test_postgres_adapter.py index 7501a0ef..0abe578e 100644 --- a/services/gateway/tests/test_postgres_adapter.py +++ b/services/gateway/tests/test_postgres_adapter.py @@ -1,4 +1,5 @@ import os +from datetime import timedelta import pytest from fastapi.testclient import TestClient @@ -6,7 +7,9 @@ from src import cli from src.api.app import create_app -from src.api.deps import get_directory, get_storage +from src.api.deps import get_directory, get_storage, get_verification +from src.api.hashing import generate_key +from src.config import Settings, get_settings from src.storage.postgres import PostgresStorageAdapter from src.storage.schema import api_keys from storage_cases import StorageCases @@ -52,6 +55,59 @@ def test_login_quotas_survive_a_new_adapter(engine, adapter): assert not restarted.reserve_login(challenge()) +def test_hourly_client_quota_survives_a_new_adapter(engine, adapter): + from login_storage_cases import NOW + + for minute in range(4): + for _ in range(5): + assert adapter.allow_login_request( + "compute", "start", "192.0.2.1", NOW + timedelta(minutes=minute) + ) + restarted = PostgresStorageAdapter(engine) + at = NOW + timedelta(minutes=5) + assert not restarted.allow_login_request("compute", "start", "192.0.2.1", at) + assert restarted.allow_login_request("compute", "start", "192.0.2.99", at) + + +def test_nul_email_returns_422_without_persisting_login_state(engine, adapter, monkeypatch): + from src.storage.schema import login_challenges, login_limits + + monkeypatch.setitem(Settings.model_config, "env_file", None) + monkeypatch.setenv("GATEWAY_ENV", "local") + monkeypatch.setenv("EMAIL_LOGIN_ENABLED", "true") + get_settings.cache_clear() + token, prefix, key_hash = generate_key() + adapter.create_api_key( + name="compute", prefix=prefix, key_hash=key_hash, scopes=["auth:email"], actor="test" + ) + + class UnexpectedUpstream: + def get_person_by_email(self, email): + pytest.fail("invalid email must not reach the directory") + + def request_code(self, subject, email): + pytest.fail("invalid email must not reach verification") + + app = create_app() + app.dependency_overrides[get_storage] = lambda: adapter + app.dependency_overrides[get_directory] = UnexpectedUpstream + app.dependency_overrides[get_verification] = UnexpectedUpstream + try: + with TestClient(app, headers={"X-API-Key": token}) as client: + response = client.post( + "/v1/auth/email/start", + json={"email": "member\x00@example.com", "client_ip": "192.0.2.1"}, + ) + assert response.status_code == 422 + assert response.json() == {"detail": "invalid login request"} + assert response.headers["cache-control"] == "no-store" + with engine.connect() as conn: + assert conn.execute(select(login_challenges)).first() is None + assert conn.execute(select(login_limits)).first() is None + finally: + get_settings.cache_clear() + + def test_repeated_revocation_preserves_audit_metadata(adapter, engine): key = adapter.create_api_key( name="consumer", prefix="prefix", key_hash="HASH", scopes=[], actor="issuer" From dc8a5cd2cda2ce30b6f7d8537e7364f7b98a4dfd Mon Sep 17 00:00:00 2001 From: Eeetan Date: Mon, 9 Nov 2026 16:35:13 -0500 Subject: [PATCH 5/5] fix(gateway): enforce login attempt eligibility in atomic update --- services/gateway/docs/ARCHITECTURE.md | 3 ++ services/gateway/src/storage/postgres.py | 13 ++---- services/gateway/tests/login_storage_cases.py | 18 ++++++++ .../gateway/tests/test_postgres_adapter.py | 43 ++++++++++++++++++- 4 files changed, 67 insertions(+), 10 deletions(-) diff --git a/services/gateway/docs/ARCHITECTURE.md b/services/gateway/docs/ARCHITECTURE.md index 33c8d37e..98b9afca 100644 --- a/services/gateway/docs/ARCHITECTURE.md +++ b/services/gateway/docs/ARCHITECTURE.md @@ -90,6 +90,9 @@ close the eligibility gap after authentication. Both adapters implement the same storage protocol. Postgres serializes short login-state transactions with a transaction-scoped advisory lock, never held during HTTP. This intentionally simple design fits the one-worker, low-volume deployment. +Attempt admission also checks owner, expiry, redemption, and the remaining guess +budget in a single conditional update, enforcing the cap in SQL independently of +the advisory lock. Expiry indexes support pruning on each authenticated login/status request. Gateway challenges retain email/UUID until expiry and the next request; rate buckets retain hashes of email/IP (pseudonymous, not anonymous) for at most their one-day window diff --git a/services/gateway/src/storage/postgres.py b/services/gateway/src/storage/postgres.py index b0599169..c3ced4d6 100644 --- a/services/gateway/src/storage/postgres.py +++ b/services/gateway/src/storage/postgres.py @@ -125,23 +125,18 @@ def activate_login(self, challenge_id, person_id, now): def admit_login_attempt(self, challenge_id, owner, now): with self._login_transaction() as conn: row = conn.execute( - select(login_challenges).where( + update(login_challenges) + .where( login_challenges.c.id == challenge_id, login_challenges.c.owner == owner, login_challenges.c.expires_at > now, login_challenges.c.redeemed.is_(False), login_challenges.c.attempts < policy.MAX_ATTEMPTS, ) - ).one_or_none() - if row is None: - return None - updated = conn.execute( - update(login_challenges) - .where(login_challenges.c.id == challenge_id) .values(attempts=login_challenges.c.attempts + 1) .returning(login_challenges) - ).one() - return LoginChallenge.model_validate(updated._mapping) + ).one_or_none() + return LoginChallenge.model_validate(row._mapping) if row is not None else None def redeem_login(self, challenge_id, owner, now): with self._login_transaction() as conn: diff --git a/services/gateway/tests/login_storage_cases.py b/services/gateway/tests/login_storage_cases.py index daa74ff1..810630ae 100644 --- a/services/gateway/tests/login_storage_cases.py +++ b/services/gateway/tests/login_storage_cases.py @@ -1,5 +1,6 @@ from concurrent.futures import ThreadPoolExecutor from datetime import datetime, timedelta, timezone +from threading import Barrier from uuid import uuid4 import pytest @@ -95,6 +96,23 @@ def test_concurrent_attempts_and_single_redemption(self, adapter): ) assert results.count(True) == 1 + def test_concurrent_confirmations_share_the_final_attempt(self, adapter): + item = challenge() + assert adapter.reserve_login(item) + assert adapter.activate_login(item.id, PERSON_ID, NOW) + for _ in range(2): + assert adapter.admit_login_attempt(item.id, item.owner, NOW) + ready = Barrier(8, timeout=10) + + def admit(_): + ready.wait() + return adapter.admit_login_attempt(item.id, item.owner, NOW) + + with ThreadPoolExecutor(max_workers=8) as pool: + attempts = list(pool.map(admit, range(8))) + assert [item.attempts for item in attempts if item is not None] == [3] + assert adapter.admit_login_attempt(item.id, item.owner, NOW) is None + def test_app_and_ip_limits_are_separate_from_status(self, adapter): for n in range(30): assert adapter.allow_login_request("compute", "start", f"192.0.2.{n}", NOW) diff --git a/services/gateway/tests/test_postgres_adapter.py b/services/gateway/tests/test_postgres_adapter.py index 0abe578e..3e830491 100644 --- a/services/gateway/tests/test_postgres_adapter.py +++ b/services/gateway/tests/test_postgres_adapter.py @@ -1,9 +1,11 @@ import os +from concurrent.futures import ThreadPoolExecutor from datetime import timedelta +from threading import Barrier import pytest from fastapi.testclient import TestClient -from sqlalchemy import create_engine, delete, select +from sqlalchemy import create_engine, delete, event, select from src import cli from src.api.app import create_app @@ -47,6 +49,45 @@ class TestPostgresLoginStorage(LoginStorageCases): pass +def test_final_attempt_is_atomic_without_advisory_lock(engine, adapter, monkeypatch): + from login_storage_cases import NOW, PERSON_ID, challenge + from src.storage.schema import login_challenges + + item = challenge() + assert adapter.reserve_login(item) + assert adapter.activate_login(item.id, PERSON_ID, NOW) + for _ in range(2): + assert adapter.admit_login_attempt(item.id, item.owner, NOW) + + # Exercise the SQL guard independently of the production advisory lock. + # Both callers reach their UPDATE before either executes it; a separate + # eligibility SELECT would therefore let both observe two spent attempts. + monkeypatch.setattr(adapter, "_login_transaction", engine.begin) + ready = Barrier(2, timeout=10) + + def before_update(conn, cursor, statement, parameters, context, executemany): + if context.compiled is not None and context.compiled.statement.is_update: + ready.wait() + + event.listen(engine, "before_cursor_execute", before_update) + try: + with ThreadPoolExecutor(max_workers=2) as pool: + attempts = list( + pool.map(lambda _: adapter.admit_login_attempt(item.id, item.owner, NOW), range(2)) + ) + finally: + event.remove(engine, "before_cursor_execute", before_update) + + assert [result.attempts for result in attempts if result is not None] == [3] + with engine.connect() as conn: + assert ( + conn.execute( + select(login_challenges.c.attempts).where(login_challenges.c.id == item.id) + ).scalar_one() + == 3 + ) + + def test_login_quotas_survive_a_new_adapter(engine, adapter): from login_storage_cases import challenge