Skip to content

Add OIDC session builder from token claims #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alan-hacktron wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add OIDC session builder from token claims #35

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions providers/session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package providers

import (
"fmt"
)

type SessionClaims struct {
Email string
Verified *bool
}

type ProviderData struct {
EmailClaim string
AllowUnverifiedEmail bool
}

const OIDCEmailClaim = "email"

// buildSessionFromClaims constructs a session from OIDC token claims.
// BUG: if the IDP omits email_verified entirely, claims.Verified is nil
// and the check below is skipped — unverified emails are trusted.
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
}
Comment on lines +22 to +31

@hacktron-app-stg hacktron-app-stg Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL 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

Open in Cursor Open in Claude

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.

View finding in Hacktron