fix(auth): pin OIDC identity to (issuer, sub) with a subject-continuity guard (BACKLOG #1015) - #243
Open
wshallwshall wants to merge 4 commits into
Open
fix(auth): pin OIDC identity to (issuer, sub) with a subject-continuity guard (BACKLOG #1015)#243wshallwshall wants to merge 4 commits into
wshallwshall wants to merge 4 commits into
Conversation
…r, sub) (BACKLOG #1015)
The OIDC relying party resolved the AD-backed account by the reassignable
username claim (oidc_username_claim, default preferred_username) and dropped
the verified sub into an audit field. On first deployment a reassigned username
(a normal IdP lifecycle operation) would let a new subject be handed the prior
holder's account -- account takeover with no credential compromise (ASVS
10.5.2). This is wrong in the shipped code; NOT-DEPLOYED beta, so it is stated
in the conditional and there is no stored account to migrate.
Fix -- subject-continuity guard. FederatedPrincipal now carries the pinned
issuer alongside sub. The account is STILL resolved by its AD username (ADR
0142 keeps roles LDAP-sourced and the principal must exist on-prem), but its
federated login is now PINNED to the verified (issuer, sub): the binding is
recorded on the first federated login and enforced on every later one, so a
login whose username resolves to an account already bound to a DIFFERENT
subject is refused (federated_subject_conflict, a closed-set audit reason that
collapses to the generic oidc_failed on the login page) rather than adopted.
Store: adds nullable oidc_issuer/oidc_subject columns and set_user_federated_subject
across SQLite, SQL Server, and Postgres (idempotent ALTERs, byte-parallel to the
shipped totp migration; SELECT * + from_mapping reads them on all three, NULL =
never federated). Persisting the non-reassignable subject is unavoidable for any
(issuer, sub) keying, so this is store work ADR 0142 recorded as out of scope
("zero store work"). This lane does not allocate or amend an ADR by design --
ADR 0142 must be amended by the owner in the same PR (see PR body).
Tests: three regressions (changed-subject-same-username refused; same-subject-
changed-username stays the same account with the display refreshed; a username
reused across two subjects refused without collision) plus a claims test that the
principal carries the issuer. Each was falsified -- guard neutralized -> the two
takeover tests go red (audit shows login_success for actor jdoe carrying a
different sub); issuer broken -> the claims test goes red; binding write
suppressed -> all three service tests go red.
Owner decision pending (see PR body): Option A (this guard, which fits the
AD-backed model) vs Option B (drop users.username UNIQUE and key the row on
(issuer, sub), username display-only). Option A is fail-closed: a legitimately
reassigned username is refused with no automated rebind path.
…aiting owner review (BACKLOG #1015) Flip only the #1015 banner from not-started to in-progress: Option A (an OIDC subject-continuity guard) is built and committed, but #1015 is NOT closed -- it awaits two owner decisions: (1) ratify Option A vs Option B (full re-key), and (2) authorize an ADR 0142 amendment (this change adds oidc_issuer/oidc_subject store columns + three-backend migrations that ADR 0142's "zero store work" decision forecloses). Only the banner line under the 1015 heading changed; the census was NOT recomputed.
…s zero-store-work (BACKLOG #1015) Owner-ratified 2026-08-06. #1015's fix (Option A, committed earlier on this branch) adds oidc_issuer/oidc_subject store columns + a federated-path subject-continuity guard, which overturns ADR 0142's "Zero store work" Decision and reclassifies the username-reassignment residual it framed as an acceptable wrong-user login to a closed P1 account takeover. Amendment A records the actual mechanism (account still resolved by AD username, federated identity pinned to (issuer,sub)), narrows the store-migration out-of-scope to the two nullable columns, reconciles AC-1 (runtime byte-identical when oidc_enabled is false -- the idempotent ALTER runs regardless, a no-op migration), states the reassigned-username availability residual + the recommended operator rebind follow-on, and adds AC-12 (subject continuity). A superseding pointer is added at the Zero-store-work bullet. Docs only.
…Amendment A (BACKLOG #1015) Flip #1015 from in-progress to closed: Option A (the subject-continuity guard) is built and ADR 0142 Amendment A is owner-ratified, so the account-takeover is fixed and the code and its governing ADR are now self-consistent. Only #1015's banner line under its heading changed; the census was NOT recomputed.
wshallwshall
enabled auto-merge (squash)
August 6, 2026 14:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes BACKLOG #1015. auth/ across 3 backends, plus store. 4 commits.
OIDC identity is now pinned to the (issuer, sub) pair rather than to a mutable claim, with a
subject-continuity guard so a legitimately-reassigned username cannot silently inherit a prior
identity. ADR 0142 Amendment A, owner-ratified.
SEQUENCING: this is the AUTH CLUSTER BASE. #1016 and #1021 branch off main WITHOUT it and are
sequenced after it deliberately -- they take a small auth/ merge once this lands. File-disjoint
within each pair; no line conflict expected.
FOLLOW-ON TO FILE (not in this PR): an operator REBIND action for a legitimately-reassigned
username -- the availability residual, ADR 0142 A.4.
Verified in-lane: plan -> build -> 3-lens adversarial verify -> remediate. Banner gate OK.