◐ Shell
clean mode source ↗

fix(coderd)!: restrict OIDC email fallback to first-time account linking by f0ssel · Pull Request #25712 · coder/coder

coder-agents-review[bot]

@f0ssel f0ssel marked this pull request as ready for review

May 27, 2026 14:14

Emyrk

@f0ssel f0ssel changed the title fix(coderd): restrict OIDC email fallback to first-time account linking fix(coderd)!: restrict OIDC email fallback to first-time account linking

Jun 4, 2026
The findLinkedUser function falls back to email-based user lookup when
no linked_id match is found. This fallback was used for all logins, not
just first-time linking, allowing an attacker with the victim's email
at the IdP (but a different subject) to hijack an existing account.

Restrict the email fallback so that when a user found by email already
has a user_link with a non-empty linked_id that differs from the
current login's linked_id, the function does not return that user.
This blocks account takeover while preserving first-time linking
(no existing link) and legacy links (empty linked_id).

Also backfill linked_id on legacy user_links via a new
UpdateUserLinkedID query that only sets the value when currently empty.

Fixes: https://linear.app/codercom/issue/PLAT-229
The TestMethodTestSuite/Accounting test requires every database method
to have an authorization test. Add the missing test for the new
UpdateUserLinkedID query.
CRF-3 (P2): Add defense-in-depth TOCTOU check inside oauthLogin
transaction. After UpdateUserLink, verify link.LinkedID matches
params.LinkedID to catch concurrent backfill races on legacy links.

CRF-4 (P2): Return sentinel errLinkedIDAlreadyBound from findLinkedUser
instead of nil. Both OIDC and GitHub callers now render a clean 403
with a descriptive message instead of leaking DB constraint errors.

CRF-5 (P2): Save the sub, compute expected linked_id from
fake.IssuerURL(), and assert exact equality in OIDCLegacyLinkBackfill.

CRF-1 (P3): Callers now log a warn-level message when the sentinel
error is returned, providing operator visibility.

CRF-2 (Nit): Fix double-negative typo in findLinkedUser doc comment.

CRF-6 (P3): Update doc comment to accurately describe email fallback
scope (first-time linking and legacy links with empty linked_id).

CRF-8 (P3): Add AllowSignups=true subtest to OIDCEmailFallbackBlocked
to cover both signup paths.

CRF-9 (P3): Add linked_id verification to OIDCFirstTimeLinkByEmailAllowed.

CRF-10 (P3): Use fake.IssuerURL() for victim's linked_id in tests
so the issuer format is realistic (same issuer, different subject).

CRF-11, CRF-12 (Nit): Tighten comments to avoid restating guards.
- Authorize UpdateUserLinkedID with ActionUpdate on the user object,
  matching the sibling InsertUserLink, instead of ActionUpdatePersonal.
- Add a GitHub OAuth test for the errLinkedIDAlreadyBound email-fallback
  block (previously only the OIDC path was covered).
- Add an OIDC issuer-URL-change test documenting the deliberate breaking
  behavior for existing links recorded under a previous issuer.

Co-authored-by: Coder Agents <agents@coder.com>
The defense-in-depth re-check inside the oauthLogin transaction returned
a bare error, which surfaced as a 500. Return an idpsync.HTTPError with
StatusForbidden so the concurrent-backfill race renders the same clean
403 as the primary findLinkedUser guard.

Co-authored-by: Coder Agents <agents@coder.com>

@f0ssel f0ssel deleted the fix/plat-229-oidc-linked-id-email-fallback branch

June 4, 2026 18:36