diff --git a/docs/requirements/identity-lifecycle.md b/docs/requirements/identity-lifecycle.md index 3ce62e96..c196c82e 100644 --- a/docs/requirements/identity-lifecycle.md +++ b/docs/requirements/identity-lifecycle.md @@ -7,9 +7,20 @@ enforceable policy. It also carries the bootstrap/first-admin invariant from issue #1430, folded in here after PR #1431's independent review showed the quick-fix approach was insufficient. +Revision 2: addresses the 9 findings of the independent review +(`fleet/lanes/webui-audit/findings/pr1433-review.md`) — epoch enforcement +tightened (§3), canonical email split from provider claims (§5), external +principal keyed by issuer+subject with DB uniqueness and link step-up (§6), +JIT default precedence and first-admin SSO path defined (§2, §4), +deactivation made measurable (§7.1), deletion kept in scope and the existing +hard-delete endpoint required to fail closed (§7.3), workspace identity +reconciled with the native-kanban SOT (§1.4), verification matrix expanded +(§8), factual labels corrected (§7.3, §8.1). + Scope: account creation, bootstrap, federated login, account linking, claim -mapping, and deactivation. Out of scope: RBAC grant semantics (contract 2), -wizard UX flow (contract 3), hierarchy schema (contract 1). +mapping, deactivation, and (minimally) deletion gating. Out of scope: RBAC +grant semantics (contract 2), wizard UX flow (contract 3), hierarchy schema +(contract 1), sensitive-data custody (contract 7 / D14). ## 1. System of record @@ -21,6 +32,13 @@ wizard UX flow (contract 3), hierarchy schema (contract 1). 3. The forward-auth perimeter shim is a deployment measure. Once in-app OIDC is configured for a deployment, the shim is demoted: it may stay as network perimeter, but no application code may read identity from its headers. +4. **Account ≠ workspace membership.** Creating an account (by any path: + bootstrap, sign-up, invite, JIT, admin creation) creates no workspace, no + hierarchy grant, and no workspace-scoped authority (native-kanban SOT + REQ-TEN-001 / REQ-ID-001). The better-auth `role` field is a platform/auth + role (`member` | `admin`), not workspace membership. Workspace grants are + defined by contract 2; until then a fresh account can authenticate and + holds no workspace authority. ## 2. Registration gating @@ -33,10 +51,14 @@ Contract: 1. A single server-side setting `registration_mode` with values `open | invite | closed`. It lives in the database (admin-mutable at runtime), not in env config. -2. Default after bootstrap: `closed`. The wizard (contract 3) may set it - during setup; Enterprise mode defaults to `closed` with OIDC JIT enabled. +2. Default after bootstrap: `closed`. The wizard (contract 3) may set a + different mode during setup, recorded as an explicit operator choice. + While the bootstrap epoch is open (§3), the effective mode is `closed` + regardless of any stored value: the setting takes effect only after the + epoch completes. 3. `closed` blocks self-service email/password sign-up. It does not block - admin-created users or OIDC JIT (§4), each gated separately. + admin-created users or OIDC JIT (§4); each of those is gated by its own + switch, and all of them are closed while the bootstrap epoch is open (§3). 4. `invite` requires a single-use, expiring invite token bound to an email address. Invite issuance is an admin operation and is audit-logged. 5. Enforcement point: a better-auth hook (or equivalent middleware executed @@ -52,92 +74,186 @@ path writes users.** Constraints any implementation MUST satisfy (each traces to a verified defect in PR #1431's review, `fleet/lanes/webui-audit/findings/pr1431-review.md`): -1. **All-writer coverage or database enforcement.** The invariant must hold - against every user-creating path: bootstrap setup, better-auth sign-up, - OIDC JIT, and admin creation. An advisory lock taken only in the bootstrap - controller does not satisfy this; a concurrent sign-up can commit between - the locked count and the admin insert. Acceptable mechanisms: a partial - unique index / constraint-backed one-row `bootstrap_state` table whose - transition insert races safely at the database level, or registration_mode - enforcement (§2) that provably closes every non-bootstrap writer while the - epoch is open. -2. **Pool safety.** No design may hold a pooled connection inside a +1. **Durable fail-closed epoch state, obeyed by every writer.** The epoch + lives in a constraint-backed one-row `bootstrap_state` table. While the + epoch is open, every non-bootstrap user-creating writer — better-auth + sign-up, OIDC JIT, admin creation — refuses, fail-closed, enforced inside + the writer's own path (better-auth hook for the raw handler; guard for + admin routes). A partial unique index or a winning epoch-transition row is + necessary but not sufficient on its own: neither stops an untagged insert + from a writer that never consulted the epoch. Both layers are required: + database-level transition safety (the epoch-completing write races safely + and at most one wins) and writer-level refusal (no path can create a user + without reading epoch state). +2. **Atomic first-admin transition.** The admin user, its credential account, + the initial admin token, and the epoch-completed transition commit in one + database transaction or not at all. A better-auth call through + `drizzleAdapter(db)` runs on the root pool and is NOT part of any caller + transaction; it may be used inside the bootstrap transition only if the + adapter is explicitly bound to the transaction handle. Otherwise the + bootstrap writer must create the user rows itself within the transaction. +3. **Pool safety.** No design may hold a pooled connection inside a transaction while awaiting a write that acquires a second connection from - the same pool (`DB_POOL_MAX=1` is a supported configuration; better-auth's - `drizzleAdapter(db)` uses the root pool, not the caller's transaction). -3. **Re-runnability (D4).** Bootstrap is not a one-shot: after the first-admin + the same pool (`DB_POOL_MAX=1` is a supported configuration). +4. **Re-runnability (D4).** Bootstrap is not a one-shot: after the first-admin epoch completes, re-running the wizard reconfigures the system but never re-opens the zero-user transition. "Setup already completed" is a stable, testable state, and factory-reset (a future, explicitly destructive operation) is the only way to open a new epoch. -4. **Atomic outcome.** The first-admin transition either yields admin user + - initial credential/token together or nothing observable. A failure may not - strand an admin user without its token such that setup can never retry - (the pre-existing failure mode noted in the #1431 review). +5. **No stranded partial outcome.** A failure at any point in the transition + leaves nothing observable (no admin user without its token, no completed + epoch without an admin) and setup remains retryable — this follows from + §3.2 and is stated separately because it is the pre-existing failure mode + the #1431 review verified. +6. **First-admin via SSO (D4).** When the operator chooses SSO for the + initial user, the wizard executes the OIDC login as part of the bootstrap + transition itself: the bootstrap writer creates the account from the + asserted identity inside the §3.2 transaction. This path is the bootstrap + writer, not JIT — §4's JIT gate stays closed during the epoch and is not + an obstacle to D4. ## 4. JIT provisioning (OIDC first login) 1. A successful OIDC login with no matching account creates a user - just-in-time only when `jit_provisioning` is enabled (per-provider flag, - default off). -2. JIT users receive role `member`, never an elevated role, regardless of IdP - claims (§5). -3. An optional per-provider email-domain allowlist constrains JIT. Empty - allowlist with JIT on means any authenticated subject at that IdP gets an - account — permitted, but the wizard must present it as an explicit choice. -4. JIT is disabled while the bootstrap epoch is open (§3.1). + just-in-time only when `jit_provisioning` is enabled for that provider. + The flag is per-provider and defaults off, always. There is no + mode-implied default: Enterprise setup enables JIT only when the wizard + records it as an explicit operator choice for a named provider (this + replaces revision 1's "Enterprise mode defaults to closed with OIDC JIT + enabled", which contradicted the per-provider default). +2. JIT users receive platform role `member`, never an elevated role, + regardless of IdP claims (§5), and no workspace authority (§1.4). +3. An optional per-provider email-domain allowlist constrains JIT. The + allowlist matches only when the IdP asserts the email with + `email_verified: true`; an unverified address never satisfies the + allowlist. Empty allowlist with JIT on means any authenticated subject at + that IdP gets an account — permitted, but the wizard must present it as an + explicit choice. +4. JIT is disabled while the bootstrap epoch is open (§3.1). The first-admin + SSO path is §3.6, not JIT. ## 5. Claim mapping -1. Mapped at first login and refreshed at each login: `email`, - `email_verified`, display name, avatar. -2. Never mapped from IdP claims: `role` and any future authorization +1. **Two stores, not one.** Provider-observed claims (`email`, + `email_verified`, display name, avatar) are recorded per external + principal — keyed by issuer + subject (§6.1) — at first login and + refreshed at each login. The canonical account fields (`users.email`, + `users.name`, `users.image`) are set from the provider claims exactly + once, at account creation (JIT or first-admin SSO), and are never silently + overwritten by a later login. +2. **Canonical email changes only through an explicit workflow.** Either the + user-initiated email change (with verification of the new address) or an + admin edit. A provider-claim refresh never rebinds `users.email`; a + divergence between canonical email and the latest provider-observed email + is surfaced per §6.3. +3. Never mapped from IdP claims: `role` and any future authorization attribute. Authorization lives in the system of record and in the RBAC layer (contract 2). An IdP group/role claim may at most be recorded for audit; it grants nothing. -3. Email change at the IdP does not silently rebind the account (§6.3). ## 6. Account linking trust -1. Linking an OIDC identity to an existing account happens only in one of two - ways: (a) explicit link initiated by the logged-in user from settings, or - (b) automatic link when the IdP asserts a verified email exactly matching - an existing account **and** the provider is marked `trusted_for_linking` +1. **External principal identity is issuer + subject.** A linked identity is + keyed by the OIDC issuer and subject claims, not by an unqualified + provider subject id and not by email. Each configured provider binds to + exactly one issuer; that binding is immutable after creation (changing the + issuer means creating a new provider). The database enforces at most one + local account per (provider, subject) with a unique constraint — the + current non-unique `(provider_id, account_id)` index does not satisfy + this; application-level checks without a uniqueness witness lose + concurrent-callback races. +2. Linking an OIDC identity to an existing account happens only in one of two + ways: (a) explicit link initiated by the logged-in user from settings, + which requires step-up: a fresh reauthentication (password or existing + linked method) no older than a short bound the implementation defines + (≤ 10 minutes) — a session cookie alone is insufficient, so a stolen + session cannot quietly attach a durable login method; or (b) automatic + link when the IdP asserts a verified email exactly matching an existing + account **and** the provider is marked `trusted_for_linking` (per-provider flag, default off). -2. Untrusted-provider email collision produces a login error naming the +3. Untrusted-provider email collision produces a login error naming the conflict, not an auto-link and not a duplicate account. -3. A linked identity whose IdP email later diverges from the account email - keeps working (the link is by provider subject id, not email) but the - divergence is surfaced in the user's settings and audit log. -4. Unlinking the last login method that the user can actually use is refused +4. A linked identity whose IdP-observed email later diverges from the + canonical account email keeps working (the link is by issuer + subject, + §6.1) but the divergence is surfaced in the user's settings and audit log + (the per-principal claim store in §5.1 is what makes the divergence + representable). +5. Unlinking the last login method that the user can actually use is refused unless a password is set first. ## 7. Deactivation propagation -1. Deactivation (better-auth admin ban) is authoritative and immediate: all - sessions for the user are revoked at ban time, and banned users fail every - guard, including active websocket sessions, within one heartbeat interval. +1. **Deactivation (better-auth admin ban) is authoritative and bounded.** + Concretely: + - Ban and session revocation are one operation: the ban commit revokes all + better-auth sessions for the user. If revocation partially fails, the + ban itself must already be committed and every guard denies from that + point (fail closed); the operation is retryable. + - Every authenticated entry path checks banned state: HTTP session guards, + the admin bearer-token path (which today does not test `banned` — an + implementation defect this contract makes non-conformant), MCP, and + Socket.IO. + - Active socket connections are terminated or denied within 30 seconds of + the ban commit, or at the next inbound message on that socket, whichever + comes first (socket auth at connect-time only, as today, does not + satisfy this). + - The current admin ban route updates only the user row; it does not + conform to this section until revocation and guard coverage land. + - Admin tokens owned by the banned user are revoked in the same operation. 2. Deactivation at an external IdP does not propagate automatically in this contract's scope (no SCIM). Operational rule: removing a user from the IdP without banning them in Mosaic leaves any password or other linked login method usable — the admin guide must state this. SCIM/webhook-driven propagation is future work and out of scope here. -3. Deletion is not deactivation. Account deletion semantics (FK fan-out - across the 21 referencing tables) are deferred to the custody contract - (contract 7 / D14); until ratified, deactivation is the only supported - removal operation. +3. **Deletion is not deactivation, and deletion is gated here.** Account + deletion semantics (FK fan-out across the 21 foreign-key constraints to + `users.id`, spread over 19 referencing tables) require their own + deletion-and-retention contract, chartered as an addition to the S2 list — + contract 7 is the D14 sensitive-data custody contract and does not cover + account deletion. Until that deletion contract is ratified: the existing + hard-delete endpoint (`DELETE /api/admin/users/:id`) is disabled and fails + closed, and deactivation is the only supported removal operation. A + contract that merely declared deactivation "the only supported removal" + while the endpoint stayed live would be false on its face. ## 8. Verification requirements -1. The bootstrap invariant (§3) requires a real-PostgreSQL two-connection - concurrency test (pattern: +Every MUST above needs a bounded observable. The matrix: + +1. **Bootstrap invariant (§3).** Real-PostgreSQL concurrency tests using two + distinct physical connections (pattern: `apps/gateway/src/agent/connector-lease.postgres.integration.test.ts`, - runs in the `ci-postgres` CI step). Mocked-transaction specs are + which runs in the `test` CI step against the `ci-postgres` PostgreSQL + service — note that pattern multiplexes one pooled handle, so the tests + here must explicitly open separate connections). Races to cover: + setup-vs-setup, setup-vs-raw-sign-up, setup-vs-JIT, setup-vs-admin-create. + Plus: liveness under `DB_POOL_MAX=1`; fault injection after each write in + the transition (user, credential, token, epoch) proving nothing observable + leaks and setup retries; wizard re-run after completion proving the + zero-user transition never re-opens. Mocked-transaction specs are supplementary; they cannot prove serialization. -2. Registration gating (§2) requires spec coverage of all three modes against - the raw `/api/auth/` handler path, not only against Gateway controllers. -3. Linking (§6) requires specs for: trusted auto-link, untrusted collision - error, and last-usable-method unlink refusal. +2. **Registration gating (§2).** Spec coverage of all three modes against the + raw `/api/auth/` handler path, not only Gateway controllers; invite + lifecycle (single-use, expiry, email binding); effective-`closed` while + the epoch is open regardless of stored mode. +3. **JIT (§4).** Provider flag off → no account on first OIDC login; on → + account with platform role `member` and no workspace grant; domain + allowlist rejects an unverified email claim even when the domain matches; + JIT refused while the epoch is open. +4. **Claim mapping (§5).** Login refresh updates the per-principal claim + store and does not touch canonical `users.email`/name/image; explicit + email-change workflow is the only path that rebinds canonical email. +5. **Linking (§6).** Unique-constraint witness: concurrent first-login + callbacks for the same (provider, subject) yield exactly one account; + trusted auto-link; untrusted collision error; explicit link refused + without fresh step-up reauthentication; last-usable-method unlink refusal; + divergence surfaced after IdP email change. +6. **Deactivation (§7).** Ban revokes sessions atomically or fails closed + (partial-failure injection); guard denial post-ban on each transport: + HTTP session, admin bearer token, MCP, Socket.IO; active socket terminated + within the 30-second/next-message bound; banned user's admin tokens + unusable; hard-delete endpoint returns a fail-closed error while the + deletion contract is unratified. ## Ruling request