fix!: reject OIDC login when email_verified claim is non-bool or absent by f0ssel · Pull Request #25713 · coder/coder

f0ssel · 2026-05-27T00:48:55Z

Problem

The OIDC callback checks email_verified via a Go type assertion (verifiedRaw.(bool)). When an IdP returns the claim as a string ("false"), a number, or omits it entirely, the assertion fails silently and the email is implicitly treated as verified. Several real IdPs (SAML-to-OIDC bridges, certain Azure AD B2C configurations) emit string-typed booleans, making this reachable in practice.

Fix

Add coerceEmailVerified() to handle bool, string ("true"/"false"/"1"/"0" via strconv.ParseBool), float64, json.Number, and int/int64 variants. Rewrite the check to be fail-closed: an absent claim, an unrecognized type, or any non-truthy value is treated as unverified and rejected. The existing IgnoreEmailVerified config option remains as an escape hatch.

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

Generated with Coder Agents by @f0ssel

Implementation plan

Production code (`coderd/userauth.go`)

Added encoding/json import
Added coerceEmailVerified(v interface{}) (verified bool, ok bool) helper near EOF
Replaced the type-assertion block (lines ~1342-1363) with fail-closed logic that uses coerceEmailVerified

Unit tests (`coderd/userauth_internal_test.go`, new file)

Table-driven test covering: bool, string ("true", "false", "1", "0", "TRUE", "t", "f", "invalid", ""), json.Number, float64, int, int64, nil, []string{}, map[string]string{}

Integration tests (`coderd/userauth_test.go`, `coderd/users_test.go`)

Added 3 new test cases: EmailVerifiedMissingIgnored (200), EmailVerifiedAsStringTrue (200), EmailVerifiedAsStringFalse (403)
Updated existing test cases that omitted email_verified and expected success to include "email_verified": true

FakeIDP (`coderd/coderdtest/oidctest/idp.go`)

encodeClaims now defaults email_verified to true (like exp, aud, iss) so tests that don't care about the verification flow are unaffected

…or absent The email_verified claim check used a Go type assertion that silently passed when the claim was a string, number, or absent. This allowed unverified emails to bypass rejection. Add coerceEmailVerified() to handle string, numeric, and json.Number variants. Default to unverified (fail-closed) when the claim is missing or an unrecognized type. The existing IgnoreEmailVerified config option remains as an escape hatch for IdPs that never send the claim.

…tibility The FakeIDP encodeClaims now defaults email_verified to true (like exp, aud, iss) so enterprise and other OIDC tests that do not care about the verification flow continue to pass. Tests that need a specific value can set it explicitly.

f0ssel · 2026-05-27T11:48:08Z

/coder-agents-review

coder-agents-review · 2026-05-27T11:48:13Z

Review posted | Chat
Requested: 2026-05-27 11:48 UTC by @f0ssel
Spend: $2.95 / $100.00

Review history

R1 (2026-05-27), 1 Nit, 1 Note, 1 P2, 1 P3, COMMENT. Review

deep-review v0.5.0 | Round 1 | 20b50dd..384b786

Last posted: Round 1, 4 findings (1 P2, 1 P3, 1 Nit, 1 Note), COMMENT. Review

Finding inventory

Findings

#	Sev	Status	Location	Summary	Round	Reviewer	Posted
CRF-1	P2	Open	idp.go:913	encodeClaims default makes absent-claim rejection path untestable in integration tests	R1	Netero	Yes
CRF-2	P3	Open	userauth_test.go:1105	EmailVerifiedMissingIgnored duplicates EmailNotVerifiedIgnored with factually wrong name	R1	Netero	Yes
CRF-3	Nit	Open	userauth_internal_test.go:57	tc := tc unnecessary since Go 1.22	R1	Netero	Yes
CRF-4	Note	Open	userauth_test.go:1119	EmailVerifiedAsStringTrue passes on old code, does not distinguish new behavior	R1	Netero	Yes

Contested and acknowledged

(none)

Round log

Round 1

Netero-only. 1 P2, 1 P3, 1 Nit, 1 Note. Reviewed against 20b50dd..384b786.

About deep-review

CRF = Coder Review Finding (P0-P4, Nit, Note)

Reviewer	Focus
Bisky	tests
Chopper	ops/errors
Churn-guard	change verification
Ging	language modernization
Gon	naming
Hisoka	edge cases
Killua	perf
Kite	change integrity
Knov	contracts
Knuckle	SQL
Kurapika	security
Leorio	docs
Luffy	product
Mafu-san	process
Mafuuu	contracts
Melody	dispatch/pairing
Meruem	structural
Nami	frontend
Netero	mechanical checks
Pariston	premise testing
Pen-botter	product gaps
Razor	verification
Robin	duplication
Ryosuke	Go arch
Takumi	concurrency
Zoro	shape

🤖 Managed by Coder Agents.

coder-agents-review

Solid security fix. The fail-closed rewrite is the right call, coerceEmailVerified covers the realistic IdP variants cleanly, and the unit test matrix is thorough (22 cases). One structural issue worth addressing before full panel review: the FakeIDP now defaults email_verified to true, which makes the absent-claim rejection path, the core behavioral change in this PR, unreachable in integration tests.

This is a first-pass review (Netero only). The full review panel has not yet reviewed this PR. The panel will review after these findings are addressed.

1 P2, 1 P3, 1 Nit, 1 Note.

“The encodeClaims default is a convenience that hides a coverage gap. The one scenario that changed from implicit-allow to explicit-deny is the one scenario no integration test can reach.” — Netero

🤖 This review was automatically generated with Coder Agents.

- Add WithOmitEmailVerifiedDefault() FakeIDP option so integration tests can exercise the handler's absent-claim rejection branch (CRF-1). - Add EmailVerifiedMissing and EmailVerifiedMissingIgnored standalone subtests that use the opt-out to cover the absent-claim path end-to-end (CRF-1, CRF-2). - Remove duplicate EmailVerifiedMissingIgnored table entry that was functionally identical to EmailNotVerifiedIgnored (CRF-2). - Remove unnecessary tc := tc loop variable capture (CRF-3).

Emyrk

Mark this as a breaking change.

Emyrk

Mark as breaking, otherwise 👍

github-actions Bot assigned f0ssel May 27, 2026

f0ssel changed the title ~~fix(coderd): reject OIDC login when email_verified claim is non-bool or absent~~ fix: reject OIDC login when email_verified claim is non-bool or absent May 27, 2026

coder-agents-review Bot reviewed May 27, 2026

View reviewed changes

Comment thread coderd/coderdtest/oidctest/idp.go Outdated

Comment thread coderd/userauth_test.go Outdated

Comment thread coderd/userauth_internal_test.go Outdated

Comment thread coderd/userauth_test.go

f0ssel marked this pull request as ready for review May 27, 2026 14:15

f0ssel requested a review from Emyrk May 27, 2026 18:32

Emyrk reviewed Jun 4, 2026

View reviewed changes

Emyrk approved these changes Jun 4, 2026

View reviewed changes

Emyrk added the release/breaking This label is applied to PRs to detect breaking changes as part of the release process label Jun 4, 2026

github-actions Bot changed the title ~~fix: reject OIDC login when email_verified claim is non-bool or absent~~ fix!: reject OIDC login when email_verified claim is non-bool or absent Jun 4, 2026

f0ssel merged commit d5b0e93 into main Jun 4, 2026
41 checks passed

f0ssel deleted the fix/plat-228-oidc-email-verified-type-coercion branch June 4, 2026 18:37

github-actions Bot locked and limited conversation to collaborators Jun 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix!: reject OIDC login when email_verified claim is non-bool or absent#25713

fix!: reject OIDC login when email_verified claim is non-bool or absent#25713
f0ssel merged 3 commits into
mainfrom
fix/plat-228-oidc-email-verified-type-coercion

f0ssel commented May 27, 2026 •

edited

Loading

Uh oh!

f0ssel commented May 27, 2026

Uh oh!

coder-agents-review Bot commented May 27, 2026 •

edited

Loading

Findings

Contested and acknowledged

Round log

Round 1

Uh oh!

coder-agents-review Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Emyrk left a comment

Uh oh!

Emyrk left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

f0ssel commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Production code (coderd/userauth.go)

Unit tests (coderd/userauth_internal_test.go, new file)

Integration tests (coderd/userauth_test.go, coderd/users_test.go)

FakeIDP (coderd/coderdtest/oidctest/idp.go)

Uh oh!

f0ssel commented May 27, 2026

Uh oh!

coder-agents-review Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Findings

Contested and acknowledged

Round log

Round 1

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Emyrk left a comment

Choose a reason for hiding this comment

Uh oh!

Emyrk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

f0ssel commented May 27, 2026 •

edited

Loading

Production code (`coderd/userauth.go`)

Unit tests (`coderd/userauth_internal_test.go`, new file)

Integration tests (`coderd/userauth_test.go`, `coderd/users_test.go`)

FakeIDP (`coderd/coderdtest/oidctest/idp.go`)

coder-agents-review Bot commented May 27, 2026 •

edited

Loading