fix: validation of refreshed sessions using the access_token in the OIDC provider (#1933)

Signed-off-by: Jan Larwig <jan@larwig.com>
This commit is contained in:
Michi Gysel 2025-11-08 13:49:48 +01:00 committed by GitHub
parent f3f30fa976
commit 22053dcade
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 25 additions and 8 deletions

View File

@ -13,6 +13,7 @@
- [#3238](https://github.com/oauth2-proxy/oauth2-proxy/pull/3238) chore: Replace pkg/clock with narrowly targeted stub clocks (@dsymonds)
- [#3237](https://github.com/oauth2-proxy/oauth2-proxy/pull/3237) - feat: add option to use organization id for preferred username in Google Provider (@pixeldrew)
- [GHSA-vjrc-mh2v-45x6](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-vjrc-mh2v-45x6) fix: request header smuggling by stripping all normalized header variants (@tuunit)
- [#1933](https://github.com/oauth2-proxy/oauth2-proxy/pull/1933) fix: validation of refreshed sessions using the access_token in the OIDC provider (@gysel / @tuunit)
# V7.12.0

View File

@ -31,6 +31,7 @@ type SessionState struct {
// Internal helpers, not serialized
Clock func() time.Time `msgpack:"-"` // override for time.Now, for testing
Lock Lock `msgpack:"-"`
Refreshed bool `msgpack:"-"` // indicates whether the session was refreshed
}
func (s *SessionState) now() time.Time {

View File

@ -110,11 +110,24 @@ func (p *OIDCProvider) EnrichSession(_ context.Context, s *sessions.SessionState
return nil
}
// ValidateSession checks that the session's IDToken is still valid
// ValidateSession checks that the session's id_token or access_token (when a ValidateURL is configured) is still valid
func (p *OIDCProvider) ValidateSession(ctx context.Context, s *sessions.SessionState) bool {
ctx = oidc.ClientContext(ctx, requests.DefaultHTTPClient)
_, err := p.Verifier.Verify(ctx, s.IDToken)
if err != nil {
// https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse
// The ID Token is optional in the Refresh Token Response
// TODO: @tuunit remove dependency on refreshed flag and only rely on presence of access_token
// in accordance with the spec. For now, keep existing behavior.
if s.Refreshed {
if !validateToken(ctx, p, s.AccessToken, makeOIDCHeader(s.AccessToken)) {
logger.Errorf("access_token validation failed")
return false
}
return true
}
if _, err := p.Verifier.Verify(ctx, s.IDToken); err != nil {
logger.Errorf("id_token verification failed: %v", err)
return false
}
@ -122,8 +135,8 @@ func (p *OIDCProvider) ValidateSession(ctx context.Context, s *sessions.SessionS
if p.SkipNonce {
return true
}
err = p.checkNonce(s)
if err != nil {
if err := p.checkNonce(s); err != nil {
logger.Errorf("nonce verification failed: %v", err)
return false
}
@ -147,7 +160,8 @@ func (p *OIDCProvider) RefreshSession(ctx context.Context, s *sessions.SessionSt
}
// redeemRefreshToken uses a RefreshToken with the RedeemURL to refresh the
// Access Token and (probably) the ID Token.
// Access Token and (optionally) the ID Token.
// https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse
func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.SessionState) error {
clientSecret, err := p.GetClientSecret()
if err != nil {
@ -250,6 +264,7 @@ func (p *OIDCProvider) createSession(ctx context.Context, token *oauth2.Token, r
ss.CreatedAtNow()
ss.SetExpiresOn(token.Expiry)
ss.Refreshed = refresh
return ss, nil
}