Skip to content

Add OIDC provider methods and ID token validation#3580

Merged
jhrozek merged 4 commits intomainfrom
auth-proxy-pr-7-1-oidc-methods
Feb 5, 2026
Merged

Add OIDC provider methods and ID token validation#3580
jhrozek merged 4 commits intomainfrom
auth-proxy-pr-7-1-oidc-methods

Conversation

@jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Feb 3, 2026

This implements the remaining OIDC provider functionality building on the OIDCProvider type as well as implements OIDC ID token validation.

This PR will be followed by integration tests that verify the end-to-end OIDC flow, and then config pipeline changes that preserve the OIDC provider type through the authserver configuration so that OIDC-specific features like ID token validation are available at runtime.

@github-actions github-actions bot added the size/M Medium PR: 300-599 lines changed label Feb 3, 2026
@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 80.95238% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.76%. Comparing base (42bc940) to head (96fa490).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
pkg/authserver/upstream/oidc.go 80.72% 8 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3580      +/-   ##
==========================================
- Coverage   65.77%   65.76%   -0.02%     
==========================================
  Files         411      411              
  Lines       40727    40805      +78     
==========================================
+ Hits        26790    26834      +44     
- Misses      11856    11881      +25     
- Partials     2081     2090       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@JAORMX JAORMX left a comment

Choose a reason for hiding this comment

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

Good work on the OIDC validation. A few thoughts on the security flow.

@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/M Medium PR: 300-599 lines changed labels Feb 4, 2026
tgrunnagle
tgrunnagle previously approved these changes Feb 5, 2026
jhrozek and others added 3 commits February 5, 2026 16:03
This implements the remaining OIDC provider functionality building on the OIDCProvider type. The changes add WithNonce authorization option for replay attack prevention, AuthorizationURL with OIDC-specific parameters (nonce, prompt), ExchangeCode with mandatory ID token validation per OIDC Core spec section 3.1.3.3, and RefreshTokens with optional ID token validation per section 12.2. The validateIDToken function uses the go-oidc library verifier to ensure proper token validation. Comprehensive tests cover all new methods using table-driven patterns.

This is part of the larger auth-proxy effort to add proper OIDC upstream support to the embedded auth server. It follows the initial OIDC provider type addition and will be followed by integration tests that verify the end-to-end OIDC flow, and then config pipeline changes that preserve the OIDC provider type through the authserver configuration so that OIDC-specific features like ID token validation are available at runtime.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Wrap validateIDToken error for debugging context (reviewer feedback)
- Clarify ExchangeCode nonce deferral with accurate comments
- Use validateIDToken consistently in ExchangeCode and RefreshTokens
  instead of calling p.verifier.Verify directly
- Document intentional nonce omission in RefreshTokens per Section 12.2
- Add sub claim validation to RefreshTokens per OIDC Core Section 12.2
  (sub MUST match original on refresh) with ErrSubjectMismatch sentinel
- Update OAuth2Provider interface to accept expectedSubject parameter

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Address review feedback from tgrunnagle:

- Add ErrNonceMissing sentinel error for consistency with ErrNonceMismatch
  and ErrSubjectMismatch (nit about inconsistent error handling)
- Validate that openid scope is present in NewOIDCProvider, failing fast
  with a clear error message instead of cryptic runtime failures when
  the IDP doesn't return an ID token
@jhrozek jhrozek force-pushed the auth-proxy-pr-7-1-oidc-methods branch from aa6eb82 to 244a344 Compare February 5, 2026 16:03
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Feb 5, 2026
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Feb 5, 2026
@jhrozek jhrozek merged commit 788057b into main Feb 5, 2026
36 checks passed
@jhrozek jhrozek deleted the auth-proxy-pr-7-1-oidc-methods branch February 5, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR: 600-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants