Skip to content

fix(coderd)!: restrict OIDC email fallback to first-time account linking (#25712)#26186

Open
jdomeracki-coder wants to merge 1 commit into
release/2.32from
cherry-pick/25712/release/2.32
Open

fix(coderd)!: restrict OIDC email fallback to first-time account linking (#25712)#26186
jdomeracki-coder wants to merge 1 commit into
release/2.32from
cherry-pick/25712/release/2.32

Conversation

@jdomeracki-coder

@jdomeracki-coder jdomeracki-coder commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Backport of #25712 to release/2.32.

Cherry-picked with git cherry-pick -x (53d287a139); the commit body references the original PR.

Generated by Coder Agents on behalf of @jdomeracki-coder.

…ing (#25712)

## Problem

`findLinkedUser` in `coderd/userauth.go` 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. An attacker who registers
the victim's email at the IdP (with a different OIDC subject) bypasses
the `linked_id` check and gets matched to the victim's Coder account.

Combined with the `email_verified` type assertion bypass (PLAT-228),
this creates a chained account-takeover vector.

## Fix

Restrict the email fallback in `findLinkedUser` 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
returns no user. This blocks account takeover while preserving:

- **First-time linking**: No existing `user_link` exists, email fallback
works as before.
- **Legacy links**: Empty `linked_id` (pre-migration), email fallback
still works.
- **Normal logins**: Matching `linked_id` resolves via the primary path,
no fallback needed.

Also adds a `UpdateUserLinkedID` query to backfill `linked_id` on legacy
links (only when currently empty) during login, gradually migrating them
to the secure path.

The `findLinkedUser` signature now accepts `loginType` explicitly
instead of relying on `user.LoginType`, ensuring the correct link is
checked in the legacy lookup.

## Breaking change

Marked `release/breaking`. An account whose `user_link` already has a
populated `linked_id` that does not match the subject the IdP presents
will now be denied login (403) instead of silently resolving via the
email fallback. The most likely trigger is changing
`CODER_OIDC_ISSUER_URL` (the `linked_id` is `issuer||subject`), or two
identities sharing one email. Accounts with an empty (legacy)
`linked_id` are unaffected and are backfilled on their next login.

Fixes: https://linear.app/codercom/issue/PLAT-229

<details><summary>Implementation details</summary>

### Files changed

- `coderd/userauth.go`: Core fix in `findLinkedUser` + backfill logic in
`oauthLogin`
- `coderd/database/queries/user_links.sql`: New `UpdateUserLinkedID`
query
- `coderd/database/dbauthz/dbauthz.go`: Authorization for new query
(`ActionUpdate` on the user object, matching `InsertUserLink`)
- `coderd/userauth_test.go`: New OIDC and GitHub tests
- Generated files: `queries.sql.go`, `querier.go`, `dbmock.go`,
`querymetrics.go`

### New tests

- `TestUserOIDC/OIDCEmailFallbackBlockedByExistingLink`: Attacker with a
different `sub` but the same email is rejected (403) when the victim has
an existing link (covers signups enabled and disabled).
- `TestUserOIDC/OIDCFirstTimeLinkByEmailAllowed`: User created via
SCIM/API (no `user_link`) can still link via email on first OIDC login,
and the `linked_id` is populated.
- `TestUserOIDC/OIDCLegacyLinkBackfill`: User with empty `linked_id` can
login and their `linked_id` is backfilled with the correct value.
- `TestUserOIDC/OIDCEmailFallbackBlockedByIssuerChange`: Existing link
recorded under a previous issuer is rejected (403) after the issuer
changes (documents the breaking behavior).
- `TestUserOAuth2Github/EmailFallbackBlockedByExistingLink`: GitHub
attacker with a different user ID but the victim's email is rejected
(403).

</details>

> [!NOTE]
> This PR was authored by Coder Agents on behalf of @f0ssel.

---------

Co-authored-by: Coder Agents <agents@coder.com>
(cherry picked from commit 53d287a)
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

👋 Hey @jdomeracki-coder!

This PR is targeting the release/2.32 release branch, but its title does not start with fix: or fix(scope):.

Only bug fixes should be cherry-picked to release branches. If this is a bug fix, please update the PR title to match the conventional commit format:

fix: description of the bug fix
fix(scope): description of the bug fix

If this is not a bug fix, it likely should not target a release branch.

@coder-tasks

coder-tasks Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Documentation Check

New Documentation Needed

  • docs/admin/users/oidc-auth/index.md - Document the account linking security behavior introduced by this change. Specifically: (1) Coder uses the OIDC sub (subject) claim to bind accounts after first login; (2) subsequent logins where the computed subject differs from the stored one are blocked with a 403 Account already linked error; (3) legacy users with an empty linked_id can still log in via email fallback and have their linked_id backfilled on first login; (4) users whose OIDC issuer URL changed will be blocked and need administrator intervention to unblock.

Note: The PR description explicitly flags this as a breaking change requiring "release notes and customer comms" before the v2.32.7 release. The original PR (#25712) also shipped no documentation changes.


Automated review via Coder Tasks

@jdomeracki-coder jdomeracki-coder requested a review from f0ssel June 9, 2026 19:01

@f0ssel f0ssel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backport of #25712 to release/2.32.

Verified as a clean version backport: identical changed files and diff stats to the sibling backports of this fix on the other release branches, with no unrelated files touched. Approving as requested.

This review was generated by Coder Agents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants