fix(auth): pin OIDC identity to (issuer, sub) with a subject-continuity guard (BACKLOG #1015)

docs/BACKLOG.md

@@ -4681,7 +4681,7 @@ Retiring the tree costs the engine nothing operationally: **`tests/test_ech_egre
 
 ## 1015. OIDC relying party keys federated accounts on a reassignable username claim while the non-reassignable `sub` is discarded (ASVS 10.5.2)
 
-> 🔢 **Filed 2026-08-04 — not started.** Value **7/10** · Difficulty **4/10** · _quick win_. The relying party keys federated identity on `oidc_username_claim` (default `preferred_username`), which an IdP is free to **reassign**, while the non-reassignable `sub` is verified and then dropped into an audit field. On first deployment a new holder of a retired username would be handed the prior holder's account.
+> ✅ **Closed 2026-08-06 — Option A shipped (subject-continuity guard); ADR 0142 Amendment A owner-ratified.** Value **7/10** · Difficulty **4/10** · _quick win_. The relying party keyed federated identity on a reassignable username claim while the non-reassignable `sub` was verified then dropped, so on first deployment a new holder of a retired username would have been handed the prior holder's account (ASVS 10.5.2). Fixed by pinning the federated identity to `(issuer, sub)` — two nullable store columns with idempotent three-backend migrations — and refusing a login whose username resolves to an account bound to a different `sub` (`federated_subject_conflict`); the account is still resolved by AD username and roles still come from LDAP. Residual: a legitimately reassigned username is refused with no rebind path, so an operator rebind action is the recommended follow-on.
 
 **Cluster:** Security / authentication. **Priority:** P1. **Verdict:** build. **Severity:** high on first deployment — account takeover without any credential compromise.
 

docs/adr/0142-federated-sso-oidc-authorization-code-pkce-relying-party-hybrid-ad-backed.md

@@ -64,7 +64,8 @@ the **same password-free lookup the Kerberos path already uses** —
   to `LOCAL`, which would route `reauth()` to a password check against a NULL hash and permanently 403
   every step-up route. Adding no member leaves that landmine unarmed.
 - **Zero store work.** No column, no migration, no three-backend parity, no group-map namespace
-  decision.
+  decision. **[SUPERSEDED by Amendment A (2026-08-06, BACKLOG #1015): a subject-continuity guard adds two
+  nullable `oidc_issuer` / `oidc_subject` columns with idempotent three-backend migrations; see Amendment A.]**
 - **A principal with no on-prem AD object is refused** (`not_in_directory`). Hybrid-only, by design.
 - **The username claim's UPN suffix is checked against an operator-pinned allow-list**
   (`[auth].oidc_allowed_username_domains`, defaulting to `ad_domain`) **before** the local part is used
@@ -305,3 +306,56 @@ open item 5 — **stays open**, and this ADR's non-sticky OIDC availability flag
       UPN suffix in `oidc_allowed_username_domains` still succeeds. This cell exists because the
       control was added late, after a review found the omission was exploitable.
 - [ ] Confirm whether `truststore` is a base dependency before relying on it for the IdP TLS-trust knob.
+
+---
+
+## Amendment A (2026-08-06) — subject-continuity guard: pin the federated identity to `(issuer, sub)` (BACKLOG #1015)
+
+> **Status: ACCEPTED — owner-ratified 2026-08-06.** Supersedes the **"Zero store work"** Decision bullet and
+> reclassifies the residual that bullet framed as an acceptable *wrong-user login* to a **closed P1 account
+> takeover** (ASVS 10.5.2). Built as BACKLOG #1015 (branch `fix-1015-oidc-sub`). *In force* means the store
+> carries two nullable columns and the federated path enforces subject continuity — **not** that the lab cells
+> are discharged.
+
+### A.1 The gap this closes
+The Decision bounds a claims-parsing bug to a *wrong-user login* ("roles come from LDAP"), and the UPN-suffix
+allow-list closes the *attacker-chosen* username-collision path by checking the token's **domain**. Neither
+closes a **username reassignment**: an IdP that reassigns an already-allowed username to a **different**
+principal (same allowed domain, a **different, non-reassignable `sub`**) resolves — by username — to the
+*prior* person's local account and mints a session on it. No credential is compromised, the domain check
+passes, every ladder rung is green: an account **takeover**, not a benign wrong-user login. The
+non-reassignable `sub` was discarded, so nothing detected the change.
+
+### A.2 The mechanism (Option A — the AD-backed model is preserved)
+The account is **still resolved by its AD username** and roles **still come from LDAP** — every property in the
+Decision is unchanged. Added: the account's federated identity is **pinned to `(issuer, sub)`**. On an OIDC
+login, after `resolve_principal` succeeds, the resolved account's bound `(oidc_issuer, oidc_subject)` is read;
+if it differs from the presented token's, the login is **refused** (`LoginOutcome.reason =
+"federated_subject_conflict"`, audited) **before a session is minted**; a first federated login records the
+binding. AD-password and Kerberos callers pass `None` and stay byte-identical.
+
+### A.3 What this overturns, precisely
+- **"Zero store work" is superseded** by the minimum a continuity guard requires: two **nullable** columns
+  `oidc_issuer` / `oidc_subject` on the users table with **idempotent** migrations in all three backends
+  (SQLite PRAGMA-guarded, SQL Server `COL_LENGTH`-guarded, Postgres `information_schema`-guarded) plus an
+  `AuthStore` setter. No group-map namespace, no per-message store work; the *Out of scope* "any store
+  migration" is narrowed to this one additive, nullable, no-backfill pair.
+- **AC-1 (byte-identical when `oidc_enabled=false`) is reconciled, not broken.** The idempotent `ALTER` runs
+  regardless of `oidc_enabled`, so the *schema* carries two nullable columns even with federation off; they are
+  unread and unwritten on every non-federated path, so **runtime behaviour** with `oidc_enabled=false` is
+  byte-identical (a no-op migration, not a behavioural change). Read AC-1 as a runtime-behaviour guarantee;
+  this footnote keeps the ADR and the shipped DDL from disagreeing.
+
+### A.4 Residual (stated, not hidden)
+A **legitimately** reassigned username — a *new* person taking over an old username and presenting a *new*
+`sub` — is now permanently refused (`federated_subject_conflict`) with **no rebind path**. That is the *safe*
+failure direction (refuse rather than take over) and is narrow, but a real availability edge. **Recommended
+follow-on:** an operator **rebind** action that clears/re-binds an account's `(oidc_issuer, oidc_subject)`
+after an out-of-band identity check, so a genuine reassignment is an admin operation rather than a lockout.
+
+### A.5 Acceptance criterion added
+- **AC-12 (subject continuity)** — WHEN an OIDC login's username resolves to an account already bound to an
+  `(issuer, sub)` other than the token's, THE SYSTEM SHALL refuse with an audited `federated_subject_conflict`
+  and mint no session; WHEN the account is unbound, it SHALL record the binding on that login; WHEN the bound
+  tuple matches, the login proceeds unchanged -> the federated-path regression tests (changed-sub / same-username
+  refused; same-sub / changed-username resolves to the same account).

messagefoundry/auth/oidc/claims.py

@@ -95,6 +95,15 @@ class FederatedPrincipal:
 
     username: str
     subject: str
+    # The pinned issuer this assertion was verified against (``policy.issuer``, which ``_check_core_claims``
+    # already proved the token's ``iss`` equals). Carried alongside ``subject`` so the relying party can
+    # PIN the local federated account's identity to the non-reassignable ``(issuer, sub)`` tuple (BACKLOG
+    # #1015): the AD-backed account is still resolved BY its username (ADR 0142 keeps roles LDAP-sourced),
+    # but a login whose reassignable username resolves to an account already bound to a DIFFERENT
+    # ``(issuer, sub)`` is refused — so a reassigned username cannot take over the prior holder's account.
+    # A ``sub`` is only guaranteed stable WITHIN one issuer, so the binding needs both halves even though
+    # a single issuer is pinned today.
+    issuer: str
     amr: tuple[str, ...]
     acr: str | None
     # The signature-verified ``exp`` (epoch seconds). ADR 0142 AC-6 caps the engine session at it, so
@@ -338,6 +347,7 @@ def validate_id_token(
     return FederatedPrincipal(
         username=username,
         subject=subject,
+        issuer=policy.issuer,
         amr=amr,
         acr=acr,
         expires_at=expires_at,

messagefoundry/auth/service.py

@@ -1007,6 +1007,25 @@ async def authenticate_oidc(
                 ok=False, error="user not found in directory", reason="not_in_directory"
             )
 
+        # BACKLOG #1015 (ADR 0142): subject-continuity guard. The AD-backed account is still RESOLVED by
+        # its username (roles stay LDAP-sourced), but its federated identity is PINNED to the non-
+        # reassignable OIDC (issuer, sub). If a local account for this resolved username is already bound
+        # to a DIFFERENT verified subject, an IdP has reassigned the username to a new person — refuse
+        # rather than hand the new subject the prior holder's account (the account-takeover-without-
+        # credential-compromise this item closes). An unbound account (never federated-logged-in) binds
+        # on first login below, in _complete_ad_login.
+        bound = await self._store.get_user_by_username(principal.username)
+        if (
+            bound is not None
+            and bound.oidc_subject is not None
+            and (bound.oidc_issuer, bound.oidc_subject)
+            != (principal_claims.issuer, principal_claims.subject)
+        ):
+            await self._directory_reject_audit(username, "oidc", "federated_subject_conflict")
+            return LoginOutcome(
+                ok=False, error="federated sign-in failed", reason="federated_subject_conflict"
+            )
+
         max_expires_at = principal_claims.expires_at
         if self._settings.oidc_session_max_hours:
             max_expires_at = min(
@@ -1041,6 +1060,7 @@ async def authenticate_oidc(
                 "mfa_verified": mfa_verified,
             },
             max_expires_at=max_expires_at,
+            federated_subject=(principal_claims.issuer, principal_claims.subject),
         )
 
     async def _directory_reject_audit(self, actor: str, mech: str, reason: str) -> None:
@@ -1062,7 +1082,12 @@ async def _complete_ad_login(
         mech: str | None = None,
         evidence: Mapping[str, object] | None = None,
         max_expires_at: float | None = None,
+        federated_subject: tuple[str, str] | None = None,
     ) -> LoginOutcome:
+        # ``federated_subject`` is the verified OIDC ``(issuer, sub)`` and is passed ONLY by the
+        # federated path (BACKLOG #1015). It defaults to None, so the AD-simple-bind and Kerberos
+        # callers stay byte-identical — no extra store write, no changed audit row. The federated
+        # caller has already enforced the subject-continuity guard before reaching here.
         existing = await self._store.get_user_by_username(principal.username)
         if existing is not None and existing.auth_provider != AuthProvider.AD.value:
             # Never let an AD login adopt/overwrite a like-named LOCAL account (provider confusion).
@@ -1074,6 +1099,20 @@ async def _complete_ad_login(
             )
             return LoginOutcome(ok=False, error="account conflict")
         user = await self._upsert_ad_user(principal)
+        if (
+            federated_subject is not None
+            and (
+                user.oidc_issuer,
+                user.oidc_subject,
+            )
+            != federated_subject
+        ):
+            # First federated login for this account (or an unbound AD account's first): record the
+            # (issuer, sub) binding so a later reassigned-username login carrying a different subject is
+            # refused by the guard above. A matching binding is left untouched (no updated_at churn).
+            await self._store.set_user_federated_subject(
+                user.id, federated_subject[0], federated_subject[1]
+            )
         role_ids = sorted(await self._store.roles_for_ad_groups(principal.groups))
         previous = set(await self._store.get_user_role_ids(user.id))
         await self._store.set_user_roles(user.id, role_ids, assigned_by="ad-sync")

messagefoundry/store/base.py

@@ -1685,6 +1685,14 @@ async def set_user_channel_scope(
         self, user_id: str, scope_json: str | None, *, now: float | None = None
     ) -> None: ...
 
+    async def set_user_federated_subject(
+        self, user_id: str, issuer: str, subject: str, *, now: float | None = None
+    ) -> None:
+        """Bind a user's verified federated ``(issuer, sub)`` identity (BACKLOG #1015). Recorded on the
+        first federated login so a later login whose reassignable username resolves to this account but
+        carries a different subject is refused, not handed the account."""
+        ...
+
     async def roles_for_ad_groups(self, groups: Iterable[str]) -> set[str]: ...
 
     async def list_ad_group_role_map(self) -> Sequence[Row]: ...

messagefoundry/store/postgres.py

@@ -521,7 +521,9 @@
         totp_enabled         BOOLEAN NOT NULL DEFAULT FALSE,
         totp_enrolled_at     DOUBLE PRECISION,
         totp_recovery_codes  TEXT,
-        last_totp_step       INTEGER
+        last_totp_step       INTEGER,
+        oidc_issuer          TEXT,
+        oidc_subject         TEXT
     )""",
     """CREATE TABLE IF NOT EXISTS roles (
         id           TEXT PRIMARY KEY,
@@ -1074,12 +1076,16 @@
                 "SELECT column_name FROM information_schema.columns WHERE table_name='users'"
             )
         }
+        # Federated (issuer, sub) identity keying (BACKLOG #1015): NULL on existing rows = "not yet
+        # federated" (username stays the sole key), byte-identical to before. Idempotent.
         for column, decl in (
             ("totp_secret", "TEXT"),
             ("totp_enabled", "BOOLEAN NOT NULL DEFAULT FALSE"),
             ("totp_enrolled_at", "DOUBLE PRECISION"),
             ("totp_recovery_codes", "TEXT"),
             ("last_totp_step", "INTEGER"),
+            ("oidc_issuer", "TEXT"),
+            ("oidc_subject", "TEXT"),
         ):
             if column not in users_cols:
                 await conn.execute(f"ALTER TABLE users ADD COLUMN {column} {decl}")
@@ -6463,6 +6469,19 @@
             "UPDATE users SET channel_scope=$1, updated_at=$2 WHERE id=$3", scope_json, now, user_id
         )
 
+    async def set_user_federated_subject(
+        self, user_id: str, issuer: str, subject: str, *, now: float | None = None
+    ) -> None:
+        """Bind a user's federated ``(issuer, sub)`` identity (BACKLOG #1015)."""
+        now = time.time() if now is None else now
+        await self._execute(
+            "UPDATE users SET oidc_issuer=$1, oidc_subject=$2, updated_at=$3 WHERE id=$4",
+            issuer,
+            subject,
+            now,
+            user_id,
+        )
+
     async def roles_for_ad_groups(self, groups: Iterable[str]) -> set[str]:
         normalized = sorted({g.strip().lower() for g in groups if g.strip()})
         if not normalized:

messagefoundry/store/sqlserver.py

@@ -1352,7 +1352,8 @@ def __init__(self, conn: Any, cur: Any) -> None:
         failed_attempts INT NOT NULL DEFAULT 0, locked_until FLOAT NULL,
         channel_scope NVARCHAR(MAX) NULL, totp_secret NVARCHAR(MAX) NULL,
         totp_enabled BIT NOT NULL DEFAULT 0, totp_enrolled_at FLOAT NULL,
-        totp_recovery_codes NVARCHAR(MAX) NULL, last_totp_step INT NULL)""",
+        totp_recovery_codes NVARCHAR(MAX) NULL, last_totp_step INT NULL,
+        oidc_issuer NVARCHAR(MAX) NULL, oidc_subject NVARCHAR(MAX) NULL)""",
     """IF COL_LENGTH('users','channel_scope') IS NULL
         ALTER TABLE users ADD channel_scope NVARCHAR(MAX) NULL""",
     # MFA (WP-14): TOTP columns ALTER-ed in for a pre-existing users table (idempotent).
@@ -1367,6 +1368,12 @@ def __init__(self, conn: Any, cur: Any) -> None:
     # Single-use TOTP within the step window (ASVS 6.5.1): highest consumed time-step.
     """IF COL_LENGTH('users','last_totp_step') IS NULL
         ALTER TABLE users ADD last_totp_step INT NULL""",
+    # Federated (issuer, sub) identity keying (BACKLOG #1015): COL_LENGTH-gated ADD on a pre-existing
+    # users table. NULL on existing rows = "not yet federated" (username stays the sole key). Idempotent.
+    """IF COL_LENGTH('users','oidc_issuer') IS NULL
+        ALTER TABLE users ADD oidc_issuer NVARCHAR(MAX) NULL""",
+    """IF COL_LENGTH('users','oidc_subject') IS NULL
+        ALTER TABLE users ADD oidc_subject NVARCHAR(MAX) NULL""",
     """IF OBJECT_ID('roles','U') IS NULL CREATE TABLE roles (
         id NVARCHAR(64) NOT NULL PRIMARY KEY, display_name NVARCHAR(128) NOT NULL,
         description NVARCHAR(512) NULL, builtin BIT NOT NULL DEFAULT 1,
@@ -9443,6 +9450,16 @@ async def set_user_channel_scope(
             (scope_json, now, user_id),
         )
 
+    async def set_user_federated_subject(
+        self, user_id: str, issuer: str, subject: str, *, now: float | None = None
+    ) -> None:
+        """Bind a user's federated ``(issuer, sub)`` identity (BACKLOG #1015)."""
+        now = time.time() if now is None else now
+        await self._execute(
+            "UPDATE users SET oidc_issuer=?, oidc_subject=?, updated_at=? WHERE id=?",
+            (issuer, subject, now, user_id),
+        )
+
     async def roles_for_ad_groups(self, groups: Iterable[str]) -> set[str]:
         normalized = sorted({g.strip().lower() for g in groups if g.strip()})
         if not normalized: