From fa4e9857c7cc2b727af3c8749541ab047d32592a Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 20 Oct 2026 17:21:43 -0400 Subject: [PATCH 1/2] docs: correct the security posture and document per-instance limits Closes #434. Session lifetime, lockout, the owner grant, key retirement and the roles route now match the code, and production-operations lists what is held per process when more than one instance runs. --- .github/workflows/ci.yml | 4 +-- CLAUDE.md | 2 +- README.md | 3 +- docs/admin-operations.md | 2 +- docs/configuration.md | 22 +++++++++----- docs/fido-conformance.md | 4 ++- docs/production-operations.md | 22 ++++++++++++-- docs/security-posture.md | 53 +++++++++++++++++++++++++++++---- src/controllers/systemConfig.ts | 7 ++--- src/lib/ownerAdmin.ts | 16 +++++----- 10 files changed, 99 insertions(+), 36 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 60757dd3..11d7e97f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,9 +62,7 @@ jobs: - name: Install dependencies run: npm ci - # Fails on a new high or critical in anything that ships. Two moderates - # remain from sequelize, whose only offered fix is a downgrade to v3, so - # the gate sits above them rather than being disabled outright. + # Fails on a high or critical in anything that ships. - name: Audit production dependencies run: npm audit --omit=dev --audit-level=high diff --git a/CLAUDE.md b/CLAUDE.md index 0fd972e8..45479a00 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,7 +20,7 @@ report real output — never claim a change works without running them. | Types | `npm run typecheck` | `tsc --noEmit`, ~2s | | Lint | `npm run lint` | eslint, ~2s. Enforces license headers + import sort | | Tests | `npm run test:run` | vitest, ~2s. **Uses a mock DB by default — no Postgres needed** | -| Coverage | `npm run coverage` | thresholds: lines/functions/statements 70%, branches 65% | +| Coverage | `npm run coverage` | thresholds: lines/functions/statements 98%, branches 95% | Tests default to `TEST_DB=mock`. Only set `TEST_DB=postgres` (with a running Postgres) when specifically exercising real DB behavior. diff --git a/README.md b/README.md index 05d553f5..2a501933 100644 --- a/README.md +++ b/README.md @@ -349,7 +349,8 @@ curl http://localhost:5312/health/status ``` To get an admin account, sign up at `/console` with `owner@example.com`. That address is the compose -file's `OWNER_EMAIL`, so the account is granted the `admin:write` and `admin` roles on creation. +file's `OWNER_EMAIL`, so the account is granted `admin:write` on creation (or `admin` on an instance +whose `AVAILABLE_ROLES` lists only that). Override it before first boot to use your own: ```bash diff --git a/docs/admin-operations.md b/docs/admin-operations.md index 133d0c70..ab910440 100644 --- a/docs/admin-operations.md +++ b/docs/admin-operations.md @@ -16,7 +16,7 @@ The legacy `admin` role remains broad for backwards compatibility. ### Making scoped roles assignable Enforcement understands scoped roles, but a role can only be handed out if it appears in -`available_roles`, which is what `GET /roles` returns and what the console's role picker offers. +`available_roles`, which is what `GET /system-config/roles` returns and what the console's role picker offers. Ship `admin:read` and `admin:write` there so read-only admin can be granted without typing a raw role string: diff --git a/docs/configuration.md b/docs/configuration.md index b6ea56ed..a6d0f48d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -197,14 +197,20 @@ RDS global bundle. ### First admin -| Variable | Required | Default | Notes | -| ------------- | -------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `OWNER_EMAIL` | No | - | Comma separated list of tenant owner emails. A user who signs up with one of these is granted `admin` on account creation, and auth events about them carry `owner = true`. | - -The grant is safe because both signup paths establish control of the email before the account is -created (email OTP verification, or a verified OAuth profile), so only someone who actually receives -mail at the owner address can claim it. When `OWNER_EMAIL` is unset the grant is a no-op and the -instance has no admin until one is assigned another way. +| Variable | Required | Default | Notes | +| ------------- | -------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | +| `OWNER_EMAIL` | No | - | Comma separated list of owner emails. An account created for one of these is granted one admin role, and auth events about it carry `owner = true`. | + +The role is `admin:write` when `available_roles` includes it, otherwise `admin`, and nothing when +neither is available ([`src/lib/ownerAdmin.ts`](../src/lib/ownerAdmin.ts)). It is granted only when +the account is created: setting `OWNER_EMAIL` after the owner already has an account promotes +nobody. + +Email OTP registration creates the account, with the role, at `POST /registration/register`, before +the address is verified. An unverified account cannot sign in (`/login` answers it as an unknown +identifier), so only someone who receives the code at the owner address can use it. OAuth signup +creates the account from a verified provider email. When `OWNER_EMAIL` is unset the grant is a +no-op and the instance has no admin until one is assigned another way. ### Admin dashboard (optional) diff --git a/docs/fido-conformance.md b/docs/fido-conformance.md index d86bda86..c3246969 100644 --- a/docs/fido-conformance.md +++ b/docs/fido-conformance.md @@ -25,7 +25,9 @@ the options payload merged in alongside on the two options endpoints. The interface is mounted only when `FIDO_CONFORMANCE_MODE=true`, and the flag is refused outright under `NODE_ENV=production`. With the flag unset the routes are never registered: they do not reach Express, they do not reach the OpenAPI document, -and the paths answer the ordinary 404. That is enforced by +and the paths answer the ordinary 404. The committed `openapi.json` and `adapter-manifest.json` +are built with the flag unset, so `/conformance/*` is deliberately absent from both; with the +flag set, the live `/openapi.json` of that process does list them. That is enforced by [the mounting tests](../tests/integration/conformance/conformance.spec.ts) rather than left to this page. diff --git a/docs/production-operations.md b/docs/production-operations.md index 8b8b9117..f268846e 100644 --- a/docs/production-operations.md +++ b/docs/production-operations.md @@ -60,8 +60,10 @@ mechanism. 2. **Flip.** Point `SEAMLESS_JWKS_ACTIVE_KID` at the new key id, inject its `SEAMLESS_JWKS_KEY__PRIVATE`, and deploy. The new key signs. Tokens already issued keep verifying, because the outgoing public key is still published. -3. **Retire.** Once `refresh_token_ttl` has passed, so no token signed by the outgoing key can - still be live, remove it from the document and drop its private key. Deploy. +3. **Retire.** Once `access_token_ttl` has passed, so no token signed by the outgoing key can still + be live, remove it from the document and drop its private key. Deploy. Access tokens are the + longest-lived signed tokens (ephemeral tokens last five minutes). Refresh tokens are opaque and + not signed, so rotation never affects them. Each step needs a restart to take effect, because the values arrive as environment variables. The server re-reads the document every five minutes within a process, which covers a secret updated in @@ -120,6 +122,22 @@ OAuth provider client secrets live in environment variables referenced by `clien Direct email/SMS delivery requires provider credentials. If you use an external trusted server adapter for delivery, keep delivery tokens on that server and avoid returning delivery payloads to browsers. +## Running More Than One Instance + +Everything that has to agree between instances lives in Postgres: sessions and refresh rotation +(the rotation claim is a single statement the database serialises), the lockout counter +(`auth_failures`), OTP codes, magic links, WebAuthn challenges, `system_config` and the audit trail. +Several instances behind a load balancer share all of it. + +Some state is held in each process, and with more than one instance it behaves differently: + +| State | Where | With N instances | +| --------------------------------- | ----------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Rate limit and slow-down counters | `express-rate-limit` and `express-slow-down` in-memory stores ([`src/middleware`](../src/middleware)) | Each instance counts on its own, so a client whose requests are spread across instances gets up to N times the configured budget. Route by client address at the load balancer, or enforce a limit at the proxy as well. | +| `system_config` cache | Five minutes per process ([`getSystemConfig.ts`](../src/config/getSystemConfig.ts)) | A config write clears the cache only on the instance that served it. The others keep the old values (roles, origins, login methods, lockout policy, token lifetimes) for up to five minutes. Restart the instances when a change must apply at once. | +| OAuth `state` replay cache | Consumed states, for ten minutes ([`oauthService.ts`](../src/services/oauthService.ts)) | A state is refused a second time only by the instance that consumed it, so another instance could accept a replayed callback within those ten minutes. The state is still HMAC signed, bound to its provider and expires after ten minutes, and the provider's authorization code is single use. | +| Audit degraded flag | Five minutes after a failed audit write ([`auditHealth.ts`](../src/services/auditHealth.ts)) | `/health/status` reports `degraded` only on the instance whose write failed. Monitor every instance, not one. | + ## Redaction Logs and auth events redact sensitive metadata by default. Sensitive values include tokens, OTPs, magic-link URLs, OAuth state/codes, PRF salts and output, TOTP secrets, email/phone snapshots, private keys, and configured provider secrets. diff --git a/docs/security-posture.md b/docs/security-posture.md index 3ce74807..b8e0d8e8 100644 --- a/docs/security-posture.md +++ b/docs/security-posture.md @@ -159,10 +159,12 @@ key there means a coordinated release across both SDKs for an operational tuning ### What is still observable -- **Account lockout** answers `423` with `retryAfterSeconds`. Only a real account can be - locked, so this still discloses existence. It requires prior failed attempts against - that specific account, and suppressing it would leave a locked-out user with no way to - understand what happened. Accepted, unchanged. +- **Account lockout.** A locked account with a passkey continues from `/login` with + `loginMethods: ["passkey"]`; one without a passkey is answered `423` with + `retryAfterSeconds`. Only a real account can be locked, so either answer discloses + existence. It requires prior failed attempts against that specific account, and + suppressing it would leave a locked-out user with no way to understand what happened. + Accepted. See [Account lockout](#account-lockout). - **A malformed identifier** answers `400`. This does not depend on whether any account exists, so it is not an enumeration signal. - **External delivery mode** answers a real OTP or magic link request with a `delivery` @@ -416,8 +418,18 @@ of one token signs its user out, which is the direction this has to fail: the al is that whoever wins the race keeps a session, and the winner is not always the client that should have it. -The absolute lifetime that a rotation resets is separate, and is -[issue #185](https://github.com/fells-code/seamless-auth-api/issues/185). +### Absolute and idle lifetime + +Rotation does not extend a session. Every session in a chain carries `chainStartedAt`, the time of +the sign-in that began it, and a rotated session's `expiresAt` is `chainStartedAt` plus +`refresh_token_ttl` (`REFRESH_TOKEN_TTL`, default `1d`), whatever happened in between +([`src/controllers/authentication.ts`](../src/controllers/authentication.ts)). A client that +refreshes constantly is still signed out at the absolute limit. Shortening `refresh_token_ttl` +applies to chains already running: the next refresh past the new limit revokes the session. + +Separately, `session_idle_ttl` (`SESSION_IDLE_TTL`, default `8h`) ends a session nobody has used +for that long. A refusal for either reason is recorded with `absolute_lifetime_reached` or +`idle_timeout`, so the trail says which limit ended the session. ## Concurrent sessions per user @@ -448,6 +460,35 @@ than briefly exceeding the cap. This is NIST 800-53 AC-10. +## Account lockout + +**Posture: repeated wrong codes lock the code factors, never the passkeys.** + +The policy is `lockout_policy` (`LOCKOUT_POLICY`), default +`{ "enabled": true, "maxFailures": 10, "windowSeconds": 900, "lockoutSeconds": 900 }` +([`src/services/lockoutPolicyService.ts`](../src/services/lockoutPolicyService.ts)). + +- **When it locks.** When `maxFailures` failed attempts fall inside one `windowSeconds` window. It + is not a rolling count that unlocks as old failures age out. +- **How long.** `lockoutSeconds`, measured from the failure that crossed the threshold. + `retryAfterSeconds` in the `423` answer is the time actually left. +- **What counts.** Failed logins, OTP codes, TOTP codes (sign-in and step-up) and magic links + against the account, recorded in `auth_failures` (`LOCKOUT_FAILURE_TYPES` in + [`src/services/authFailureCounter.ts`](../src/services/authFailureCounter.ts)). Failed passkey + assertions do not count, because an assertion cannot be guessed. +- **What it blocks.** The code factors. Passkey sign-in is not subject to lockout, so `/login` for a + locked account with a passkey continues with `loginMethods: ["passkey"]`, and answers + `423 account_locked` only for an account with no passkey. + +The reason passkeys are spared is that `/login` mints an ephemeral token from an identifier alone. +Anyone who knows an address can submit wrong codes until the account locks, and before 0.19.0 that +locked the owner out of every method, passkeys included (GHSA-66gq-4pwg-33jq). An account with no +passkey can still be locked out this way, which is the cost of lockout protecting a guessable +factor. Enrolling a passkey keeps a user able to sign in through a lock someone else triggered. + +The counter lives in Postgres, so every instance of a deployment sees the same count. When it cannot +be read, the account is treated as locked (see [An unknown count means locked](#an-unknown-count-means-locked)). + ## Audit write failure **Posture: the trail may be lost, no security control may be.** diff --git a/src/controllers/systemConfig.ts b/src/controllers/systemConfig.ts index 4cad35d2..92f70307 100644 --- a/src/controllers/systemConfig.ts +++ b/src/controllers/systemConfig.ts @@ -146,10 +146,9 @@ export const getAvailableRoles = async (_req: Request, res: Response) => { /** * The configuration a signed-out client is allowed to read. * - * Served through getLoginPolicy rather than off the raw config so a tainted or - * partially written config still answers with the defaults. The sign-in screens - * call this before anyone has a session, so failing here would leave a client - * with no methods to offer at all. + * Served through getLoginPolicy, which fills defaults for login method values it + * does not recognise. It does not survive a config that fails schema validation: + * getSystemConfig throws, and this answers 500 like every other route. * * Only add a key to this response when a signed-out client genuinely cannot work * without it. Everything else belongs on the admin routes. diff --git a/src/lib/ownerAdmin.ts b/src/lib/ownerAdmin.ts index a911e691..8d8701f5 100644 --- a/src/lib/ownerAdmin.ts +++ b/src/lib/ownerAdmin.ts @@ -5,16 +5,14 @@ */ /** - * Managed instances are provisioned with an `OWNER_EMAIL` (the tenant owner, - * optionally a comma separated list). A user who signs up with an owner email is - * granted the admin role on account creation, so a freshly provisioned instance - * has a working `/console` admin as soon as the owner signs up. + * `OWNER_EMAIL` names the instance owner (optionally a comma separated list). An + * account created for an owner email gets one admin role at creation, so a new + * instance has a working `/console` admin as soon as the owner signs up. * - * The grant is safe because both signup paths establish control of the email - * before the account is created (email OTP verification, or a verified OAuth - * profile), so only someone who actually receives mail at the owner address can - * claim it. When `OWNER_EMAIL` is unset the helpers are no-ops, so non-managed - * deployments are unaffected. + * Email OTP registration creates the account, role included, before the address is + * verified, but an unverified account cannot sign in, so only someone who receives + * the code at the owner address can use it. OAuth signup creates it from a verified + * provider email. When `OWNER_EMAIL` is unset the helpers are no-ops. */ /** From 4d01399758d73b3972c99388c9d60df9e79bf54d Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 20 Oct 2026 17:33:21 -0400 Subject: [PATCH 2/2] docs(posture): fix the decoy endpoint count, the floor scope and session wording Fourteen endpoints accept an ephemeral token, register/start has needed an access session since 0.11.0, the login floor covers the 200 pre-auth answer, refresh_token_ttl has no server default, only a refresh extends the idle limit, and the lockout duration is measured as the code measures it. --- docs/security-posture.md | 52 +++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/docs/security-posture.md b/docs/security-posture.md index b8e0d8e8..f4393303 100644 --- a/docs/security-posture.md +++ b/docs/security-posture.md @@ -27,18 +27,18 @@ The reason is still recorded in the `login_failed` auth event metadata, now with Returning `200` for an unknown identifier is worth nothing on its own. If the next request distinguished the decoy, the oracle would simply have moved one step later. All -thirteen endpoints that accept an ephemeral token therefore answer for a decoy the way +fourteen endpoints that accept an ephemeral token therefore answer for a decoy the way they answer for a real account: -| Endpoint group | A decoy gets | -| --------------------- | --------------------------------------------------------------- | -| OTP send (4) | `200 { message: 'success', token }`, with nothing sent | -| OTP verify (4) | `401 { error: 'Not allowed' }`, the body a wrong code gets | -| Magic link request | `200`, the same "if an account exists" body a real request gets | -| Magic link poll | `204`, the state a real account sits in until someone clicks | -| WebAuthn login start | An assertion challenge over a fabricated credential id | -| WebAuthn login finish | `401 { error: 'Authentication failed.' }` | -| TOTP login verify | `401 { error: 'totp_verification_failed' }` | +| Endpoint group | A decoy gets | +| ---------------------- | --------------------------------------------------------------- | +| OTP send (4) | `200 { message: 'success', token }`, with nothing sent | +| OTP verify (4) | `401 { error: 'Not allowed' }`, the body a wrong code gets | +| Magic link request (2) | `200`, the same "if an account exists" body a real request gets | +| Magic link poll | `204`, the state a real account sits in until someone clicks | +| WebAuthn login start | An assertion challenge over a fabricated credential id | +| WebAuthn login finish | `401 { error: 'Authentication failed.' }` | +| TOTP login verify | `401 { error: 'totp_verification_failed' }` | Policy-dependent branches are reproduced rather than skipped. A deployment with `email_otp` disabled answers `403 login_method_disabled` for every identifier, so a decoy @@ -120,20 +120,14 @@ Neither validation writes anything, so reproducing them costs a decoy nothing. ### What this still does not cover -`/webauthn/register/start` echoes the account's email as `user.name` in the options it -returns, because that is what an authenticator displays. A decoy has no real address, so -it echoes its synthetic one, and a caller that reads `user.name` sees an `@example.invalid` -address where a real account shows the identifier the caller typed. That is a complete -oracle, one request past `/login`. +`/webauthn/register/start` used to be reachable with a pre-auth token, and its options echo +the account's email as `user.name`. A decoy has no real address, so it echoed a synthetic +`@example.invalid` one, which separated decoys from real accounts one request past `/login`. +Passkey enrollment has required an access session since 0.11.0 +([`src/routes/webauthn.routes.ts`](../src/routes/webauthn.routes.ts)), so neither a decoy nor a +real pre-auth token reaches that endpoint any more. -Closing it means the decoy echoing the identifier that was supplied, which it cannot do: -a decoy is rebuilt from its subject alone, and the subject is a one-way HMAC. Carrying the -identifier would mean putting it in the ephemeral token, and putting it only in decoy -tokens would be the `decoy` claim by another name, so every ephemeral token would have to -carry it. That is a second change to the token contract and a second coordinated release, -so it is tracked separately rather than folded in here. - -For the same reason, a decoy's shape is derived from its subject and cannot depend on +A decoy is rebuilt from its subject alone, which is a one-way HMAC, so its shape cannot depend on which kind of identifier was looked up. `/login` finds a phone account by its number, so such an account always has a phone and is always offered `phone_otp`, while only about half of decoys are. Where `phone_otp` is an enabled method, a phone identifier whose @@ -151,7 +145,8 @@ Identical bodies that arrive at measurably different times still answer the ques real path reads the users table, the lockout counter, the credentials table and the login policy; the decoy path reads only the policy. -`LOGIN_RESPONSE_FLOOR_MS` (default `250`) holds every `/login` answer to a minimum, which +`LOGIN_RESPONSE_FLOOR_MS` (default `250`) holds every `200` pre-auth answer from `/login`, real or +decoy, to a minimum, which removes the difference as long as both paths finish under it. Set it above the slowest real login the deployment sees. It is an environment variable rather than a `system_config` key because that schema is shared through `@seamless-auth/types`, and a @@ -422,13 +417,13 @@ should have it. Rotation does not extend a session. Every session in a chain carries `chainStartedAt`, the time of the sign-in that began it, and a rotated session's `expiresAt` is `chainStartedAt` plus -`refresh_token_ttl` (`REFRESH_TOKEN_TTL`, default `1d`), whatever happened in between +`refresh_token_ttl` (`REFRESH_TOKEN_TTL`, which must be set), whatever happened in between ([`src/controllers/authentication.ts`](../src/controllers/authentication.ts)). A client that refreshes constantly is still signed out at the absolute limit. Shortening `refresh_token_ttl` applies to chains already running: the next refresh past the new limit revokes the session. -Separately, `session_idle_ttl` (`SESSION_IDLE_TTL`, default `8h`) ends a session nobody has used -for that long. A refusal for either reason is recorded with `absolute_lifetime_reached` or +Separately, `session_idle_ttl` (`SESSION_IDLE_TTL`, default `8h`) ends a session that has not been +refreshed for that long. Using an access token does not extend it; only a refresh does. A refusal for either reason is recorded with `absolute_lifetime_reached` or `idle_timeout`, so the trail says which limit ended the session. ## Concurrent sessions per user @@ -470,7 +465,8 @@ The policy is `lockout_policy` (`LOCKOUT_POLICY`), default - **When it locks.** When `maxFailures` failed attempts fall inside one `windowSeconds` window. It is not a rolling count that unlocks as old failures age out. -- **How long.** `lockoutSeconds`, measured from the failure that crossed the threshold. +- **How long.** `lockoutSeconds`, measured from the most recent of the last `maxFailures` failures, + which is the one that crossed the threshold unless more were recorded after it. `retryAfterSeconds` in the `423` answer is the time actually left. - **What counts.** Failed logins, OTP codes, TOTP codes (sign-in and step-up) and magic links against the account, recorded in `auth_failures` (`LOCKOUT_FAILURE_TYPES` in