Skip to content

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

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

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

Conversation

@jdomeracki-coder

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

Copy link
Copy Markdown
Contributor

Backport of #25712 to release/2.34.

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 added the release/breaking This label is applied to PRs to detect breaking changes as part of the release process label Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

👋 Hey @jdomeracki-coder!

This PR is targeting the release/2.34 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

Updates Needed

  • docs/admin/users/oidc-auth/index.md - Document that OIDC email-based fallback is now restricted to first-time account linking and legacy (empty linked_id) links only. Existing users whose IdP subject (linked_id) changes (e.g., due to an issuer URL migration) will be blocked with a 403. Admins need to know how to remediate affected users.
  • docs/admin/users/github-auth.md - Same behavioral change applies to GitHub OAuth: email fallback is blocked when the GitHub user ID differs from the stored linked_id. Document the new error and admin remediation steps.

New Documentation Needed

  • Consider a section in the OIDC or authentication docs explaining the linked_id backfill behavior for legacy accounts and the security rationale (prevents account takeover via IdP email reuse). This is especially important since the PR is labeled release/breaking and references security advisory GHSA-9r87-mvcw-x35f.

Automated review via Coder Agents

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

@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.34.

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

Labels

cherry-pick/v2.34 release/breaking This label is applied to PRs to detect breaking changes as part of the release process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants