Add OIDC session builder from token claims#35
Conversation
| func (p *ProviderData) buildSessionFromClaims(claims SessionClaims) (*SessionClaims, error) { | ||
| // `email_verified` must be present and explicitly set to `false` to be | ||
| // considered unverified. | ||
| verifyEmail := (p.EmailClaim == OIDCEmailClaim) && !p.AllowUnverifiedEmail | ||
| if verifyEmail && claims.Verified != nil && !*claims.Verified { | ||
| return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email) | ||
| } | ||
|
|
||
| return &claims, nil | ||
| } |
There was a problem hiding this comment.
Authentication Bypass due to OIDC email_verified Claim Omission
The buildSessionFromClaims function in providers/session.go fails to correctly validate the email_verified claim when it is omitted from the OIDC token. The logic if verifyEmail && claims.Verified != nil && !*claims.Verified only rejects the session if the email_verified claim is present and explicitly set to false. If the IDP omits the claim entirely, claims.Verified is nil, the check is skipped, and the unverified email is accepted as valid. This allows attackers to bypass email verification if the IDP does not provide the email_verified claim.
Fix with AI
A security vulnerability was found by Hacktron.
File: providers/session.go
Lines: 22-31
Severity: critical
Vulnerability: Authentication Bypass due to OIDC email_verified Claim Omission
Description:
The `buildSessionFromClaims` function in `providers/session.go` fails to correctly validate the `email_verified` claim when it is omitted from the OIDC token. The logic `if verifyEmail && claims.Verified != nil && !*claims.Verified` only rejects the session if the `email_verified` claim is present and explicitly set to `false`. If the IDP omits the claim entirely, `claims.Verified` is `nil`, the check is skipped, and the unverified email is accepted as valid. This allows attackers to bypass email verification if the IDP does not provide the `email_verified` claim.
Affected Code:
func (p *ProviderData) buildSessionFromClaims(claims SessionClaims) (*SessionClaims, error) {
// `email_verified` must be present and explicitly set to `false` to be
// considered unverified.
verifyEmail := (p.EmailClaim == OIDCEmailClaim) && !p.AllowUnverifiedEmail
if verifyEmail && claims.Verified != nil && !*claims.Verified {
return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email)
}
return &claims, nil
}
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), or !accepted_risk <reason>. Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
Summary
Adds
providers/session.go— builds a user session from OIDC ID token claims, checking email verification status before granting access.Note: This file reproduces a known high-severity auth-bypass finding (Hacktron finding
c152a710) from production (deb-hacktron/oauth2-proxy) to validate the CI severity gate:email_verifiedis optional per the OIDC spec; many IDPs omit it for unverified accountsnil(absent) claim silently passesThis PR is testing the Org threshold triggers fail case: org threshold = High, PR with High finding → CI should fail.
Affected code (from production finding)
Original repo:
deb-hacktron/oauth2-proxy→providers/provider_data.go:142-167🤖 Generated with Claude Code