diff --git a/CHANGELOG.md b/CHANGELOG.md index 20a8f67f..cb4904c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go index 5b063c3f..a1f807ab 100644 --- a/pkg/apis/sessions/session_state.go +++ b/pkg/apis/sessions/session_state.go @@ -29,8 +29,9 @@ type SessionState struct { PreferredUsername string `msgpack:"pu,omitempty"` // Internal helpers, not serialized - Clock func() time.Time `msgpack:"-"` // override for time.Now, for testing - Lock Lock `msgpack:"-"` + 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 { diff --git a/providers/oidc.go b/providers/oidc.go index 15598aba..eeac4073 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -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 }