diff --git a/audience_test.go b/audience_test.go index 5310df1..5f4c314 100644 --- a/audience_test.go +++ b/audience_test.go @@ -484,7 +484,8 @@ func TestAuth0Scenario3OpaqueAccessToken(t *testing.T) { session.SetAccessToken(opaqueAccessToken) session.SetIDToken(idToken) - authenticated, needsRefresh, expired := ts.tOidc.validateStandardTokens(session) + rs := (&requestState{}).captureSession(session) + authenticated, needsRefresh, expired := ts.tOidc.validateStandardTokensRS(rs) if !authenticated || needsRefresh || expired { t.Errorf("Session with opaque access token and valid ID token should be authenticated. Got: auth=%v, refresh=%v, expired=%v", authenticated, needsRefresh, expired) @@ -623,7 +624,8 @@ func TestAuth0Scenario2StrictMode(t *testing.T) { session.SetRefreshToken("test-refresh-token") // Add refresh token so it can attempt refresh // In strict mode, this should FAIL (no fallback to ID token) - authenticated, needsRefresh, expired := ts.tOidc.validateStandardTokens(session) + rs := (&requestState{}).captureSession(session) + authenticated, needsRefresh, expired := ts.tOidc.validateStandardTokensRS(rs) if authenticated { t.Errorf("Strict mode: Session with wrong access token audience should be rejected, but got authenticated=true") } diff --git a/auth_flow.go b/auth_flow.go index 39d577a..df96bd0 100644 --- a/auth_flow.go +++ b/auth_flow.go @@ -305,28 +305,6 @@ func (t *TraefikOidc) handleExpiredToken(rw http.ResponseWriter, req *http.Reque t.defaultInitiateAuthentication(rw, req, session, redirectURL) } -// isUserAuthenticated determines the authentication status and refresh requirements. -// It delegates to provider-specific validation methods that handle different token types -// and expiration behaviors. -// Parameters: -// - session: The session data containing authentication tokens. -// -// Returns: -// - authenticated (bool): True if the user has valid tokens. -// - needsRefresh (bool): True if tokens are valid but nearing expiration. -// - expired (bool): True if the session is unauthenticated, the token is missing, -// or the token verification failed for reasons other than nearing/actual expiration. -func (t *TraefikOidc) isUserAuthenticated(session *SessionData) (bool, bool, bool) { - if t.isAzureProvider() { - return t.validateAzureTokens(session) - } else if t.isGoogleProvider() { - return t.validateGoogleTokens(session) - } - // Auth0 and other providers can now use standard validation - // which handles opaque tokens generically - return t.validateStandardTokens(session) -} - // isAjaxRequest determines if this is an AJAX request that should receive 401 instead of redirect func (t *TraefikOidc) isAjaxRequest(req *http.Request) bool { xhr := req.Header.Get("X-Requested-With") diff --git a/azure_oidc_test.go b/azure_oidc_test.go index d7106fc..c89a5d1 100644 --- a/azure_oidc_test.go +++ b/azure_oidc_test.go @@ -262,7 +262,8 @@ func TestAzureOIDCRegression(t *testing.T) { defer func() { tOidc.tokenVerifier = originalTokenVerifier }() // Test that CSRF is preserved during Azure validation failures - authenticated, needsRefresh, expired := tOidc.validateAzureTokens(session) + rs := (&requestState{}).captureSession(session) + authenticated, needsRefresh, expired := tOidc.validateAzureTokensRS(rs) // Should not be authenticated due to validation failure if authenticated { @@ -453,7 +454,8 @@ func TestValidateGoogleTokens(t *testing.T) { t.Run(tt.name, func(t *testing.T) { session := tt.setupSession() - auth, refresh, expired := ts.tOidc.validateGoogleTokens(session) + rs := (&requestState{}).captureSession(session) + auth, refresh, expired := ts.tOidc.validateGoogleTokensRS(rs) if auth != tt.expectedAuth { t.Errorf("Expected authenticated=%v, got %v. %s", tt.expectedAuth, auth, tt.description) @@ -637,7 +639,8 @@ func TestIsUserAuthenticated(t *testing.T) { defer func() { ts.tOidc.issuerURL = originalIssuer }() session := tt.setupSession() - auth, refresh, expired := ts.tOidc.isUserAuthenticated(session) + rs := (&requestState{}).captureSession(session) + auth, refresh, expired := ts.tOidc.isUserAuthenticatedRS(rs) if auth != tt.expectedAuth { t.Errorf("Expected authenticated=%v, got %v. %s", tt.expectedAuth, auth, tt.description) @@ -762,7 +765,8 @@ func TestValidateAzureTokensEdgeCases(t *testing.T) { t.Run(tt.name, func(t *testing.T) { session := tt.setupSession() - auth, refresh, expired := ts.tOidc.validateAzureTokens(session) + rs := (&requestState{}).captureSession(session) + auth, refresh, expired := ts.tOidc.validateAzureTokensRS(rs) if auth != tt.expectedAuth { t.Errorf("Expected authenticated=%v, got %v. %s", tt.expectedAuth, auth, tt.description) diff --git a/issue134_followup_graph_test.go b/issue134_followup_graph_test.go index 2b1694d..74ee512 100644 --- a/issue134_followup_graph_test.go +++ b/issue134_followup_graph_test.go @@ -234,7 +234,8 @@ func TestIssue134_Followup_ValidateAzureTokensSkipsGraphAccessToken(t *testing.T oidc, errBuf := newAzureFollowupOIDC(t, jwks) session := authedSessionWithTokens(t, graphAccessToken, idToken) - authenticated, needsRefresh, expired := oidc.validateAzureTokens(session) + rs := (&requestState{}).captureSession(session) + authenticated, needsRefresh, expired := oidc.validateAzureTokensRS(rs) output := errBuf.String() assert.NotContains(t, output, "crypto/rsa: verification error", @@ -344,7 +345,8 @@ func TestIssue134_Followup_StandardAzureAccessTokenStillVerifies(t *testing.T) { oidc, errBuf := newAzureFollowupOIDC(t, jwks) session := authedSessionWithTokens(t, accessToken, idToken) - authenticated, needsRefresh, expired := oidc.validateAzureTokens(session) + rs := (&requestState{}).captureSession(session) + authenticated, needsRefresh, expired := oidc.validateAzureTokensRS(rs) assert.True(t, authenticated, "standard Azure access token must verify and authenticate") assert.False(t, needsRefresh) @@ -381,7 +383,8 @@ func TestIssue134_Followup_GraphAccessTokenWithoutIDToken(t *testing.T) { oidc, errBuf := newAzureFollowupOIDC(t, jwks) session := authedSessionWithTokens(t, graphAccessToken, "") - authenticated, needsRefresh, expired := oidc.validateAzureTokens(session) + rs := (&requestState{}).captureSession(session) + authenticated, needsRefresh, expired := oidc.validateAzureTokensRS(rs) assert.True(t, authenticated, "Graph token without ID token must remain authenticated (matches existing opaque-token semantics)") assert.False(t, needsRefresh) @@ -443,7 +446,8 @@ func TestIssue134_Followup_ConfusedDeputyAttackDoesNotBypassVerification(t *test oidc, _ := newAzureFollowupOIDC(t, jwks) session := authedSessionWithTokens(t, forgedAccessToken, forgedIDToken) - authenticated, _, _ := oidc.validateAzureTokens(session) + rs := (&requestState{}).captureSession(session) + authenticated, _, _ := oidc.validateAzureTokensRS(rs) assert.False(t, authenticated, "attacker's forged tokens must not authenticate even when the access token has a nonce header — ID token verification rejects the wrong-key signature") } diff --git a/token_manager.go b/token_manager.go index b4a7596..bdd0b77 100644 --- a/token_manager.go +++ b/token_manager.go @@ -5,8 +5,6 @@ package traefikoidc import ( "context" - "encoding/base64" - "encoding/json" "fmt" "io" "net/http" @@ -860,437 +858,6 @@ func (t *TraefikOidc) isAzureProvider() bool { strings.Contains(issuerURL, "login.windows.net") } -// validateAzureTokens validates tokens with Azure AD-specific logic. -// Azure tokens may be opaque access tokens that cannot be verified as JWTs, -// so this method handles both JWT and opaque token scenarios. -// Parameters: -// - session: The session data containing tokens to validate. -// -// Returns: -// - authenticated: Whether the user has valid authentication. -// - needsRefresh: Whether tokens need to be refreshed. -// - expired: Whether tokens have expired and cannot be refreshed. -// -//nolint:gocognit // Azure-specific validation requires multiple token type checks -func (t *TraefikOidc) validateAzureTokens(session *SessionData) (bool, bool, bool) { - if !session.GetAuthenticated() { - t.logger.Debug("Azure user is not authenticated according to session flag") - if session.GetRefreshToken() != "" { - t.logger.Debug("Azure session not authenticated, but refresh token exists. Signaling need for refresh.") - return false, true, false - } - return false, true, false - } - - accessToken := session.GetAccessToken() - idToken := session.GetIDToken() - - if accessToken != "" { - if strings.Count(accessToken, ".") == 2 { - // Microsoft documents that client apps cannot validate access - // tokens issued for Microsoft-owned APIs (Graph, Azure Mgmt) due - // to their proprietary signing format (nonce in JWT header is - // the marker — signed bytes hash the nonce, wire bytes ship the - // raw value, so rsa verification always fails). Treat such - // tokens as opaque, matching Microsoft's guidance and avoiding - // per-request signature-error log spam (issue #134 followup). - // - // https://learn.microsoft.com/en-us/entra/identity-platform/access-tokens - // "you can't validate tokens for Microsoft Graph according to - // these rules due to their proprietary format" - if t.isUnverifiableAzureAccessToken(accessToken) { - t.logger.Debug("Azure access token is Microsoft-proprietary (Graph/Mgmt) — treating as opaque per Microsoft guidance") - if idToken != "" { - if err := t.verifyToken(idToken); err != nil { - t.logger.Debugf("Azure: ID token validation failed while access token was opaque: %v", err) - if session.GetRefreshToken() != "" { - return false, true, false - } - return false, false, true - } - return t.validateTokenExpiry(session, idToken) - } - return true, false, false - } - if err := t.verifyToken(accessToken); err != nil { - if idToken != "" { - if err := t.verifyToken(idToken); err != nil { - t.logger.Debugf("Azure: Both access and ID token validation failed: %v", err) - if session.GetRefreshToken() != "" { - return false, true, false - } - return false, false, true - } - return t.validateTokenExpiry(session, idToken) - } - if session.GetRefreshToken() != "" { - return false, true, false - } - return false, false, true - } - return t.validateTokenExpiry(session, accessToken) - } - t.logger.Debug("Azure access token appears opaque, treating as valid") - if idToken != "" { - return t.validateTokenExpiry(session, idToken) - } - return true, false, false - } - - if idToken != "" { - if err := t.verifyToken(idToken); err != nil { - if strings.Contains(err.Error(), "token has expired") { - if session.GetRefreshToken() != "" { - return false, true, false - } - return false, false, true - } - if session.GetRefreshToken() != "" { - return false, true, false - } - return false, false, true - } - return t.validateTokenExpiry(session, idToken) - } - - if session.GetRefreshToken() != "" { - return false, true, false - } - return false, false, true -} - -// validateGoogleTokens handles Google-specific token validation logic. -// Currently delegates to standard token validation but provides a hook -// for Google-specific validation requirements in the future. -// Parameters: -// - session: The session data containing tokens to validate. -// -// Returns: -// - authenticated: Whether the user has valid authentication. -// - needsRefresh: Whether tokens need to be refreshed. -// - expired: Whether tokens have expired and cannot be refreshed. -func (t *TraefikOidc) validateGoogleTokens(session *SessionData) (bool, bool, bool) { - return t.validateStandardTokens(session) -} - -// validateStandardTokens handles standard OIDC token validation logic. -// This is the default validation method for generic OIDC providers. -// It verifies ID tokens and handles access tokens appropriately. -// Parameters: -// - session: The session data containing tokens to validate. -// -// Returns: -// - authenticated: Whether the user has valid authentication. -// - needsRefresh: Whether tokens need to be refreshed. -// - expired: Whether tokens have expired and cannot be refreshed. -// -//nolint:gocognit,gocyclo // Complex validation logic handles multiple token scenarios and edge cases -func (t *TraefikOidc) validateStandardTokens(session *SessionData) (bool, bool, bool) { - authenticated := session.GetAuthenticated() - // Removed debug output - if !authenticated { - t.logger.Debug("User is not authenticated according to session flag") - if session.GetRefreshToken() != "" { - t.logger.Debug("Session not authenticated, but refresh token exists. Signaling need for refresh.") - return false, true, false - } - return false, false, false - } - - accessToken := session.GetAccessToken() - // Removed debug output - if accessToken == "" { - t.logger.Debug("Authenticated flag set, but no access token found in session") - if session.GetRefreshToken() != "" { - // Check if we have an ID token to determine if we're beyond grace period - // When access token is missing, check ID token expiry to determine if refresh is viable - idToken := session.GetIDToken() - t.logger.Debugf("Checking ID token for grace period: ID token present: %v", idToken != "") - if idToken != "" { - // Try to parse the ID token to check its expiry - parts := strings.Split(idToken, ".") - if len(parts) == 3 { - // Decode the claims part - claimsData, err := base64.RawURLEncoding.DecodeString(parts[1]) - if err == nil { - var claims map[string]interface{} - if err := json.Unmarshal(claimsData, &claims); err == nil { - if expClaim, ok := claims["exp"].(float64); ok { - expTime := time.Unix(int64(expClaim), 0) - if time.Now().After(expTime) { - expiredDuration := time.Since(expTime) - if expiredDuration > t.refreshGracePeriod { - t.logger.Debugf("ID token expired beyond grace period (%v > %v), must re-authenticate", - expiredDuration, t.refreshGracePeriod) - return false, false, true // expired, cannot refresh - } - t.logger.Debugf("ID token expired %v ago, within grace period %v, allowing refresh", - expiredDuration, t.refreshGracePeriod) - } - } - } - } - } - } - t.logger.Debug("Access token missing, but refresh token exists. Signaling need for refresh.") - return false, true, false - } - return false, false, true - } - - // Check if access token is opaque (doesn't have JWT structure) - dotCount := strings.Count(accessToken, ".") - isOpaqueToken := dotCount != 2 - - // For opaque access tokens, use introspection if available (RFC 7662 - Option C: Scenario 3) - if isOpaqueToken { - t.logger.Debugf("Access token appears to be opaque (dots: %d)", dotCount) - - // Try introspection first if opaque tokens are allowed - if t.allowOpaqueTokens { - if err := t.validateOpaqueToken(accessToken); err != nil { - errMsg := err.Error() - t.logger.Infof("⚠️ Opaque access token validation via introspection failed: %v", err) - - // Check if the token was explicitly marked as inactive/revoked/expired by the provider - // In these cases, we should NOT fall back to ID token - the provider has explicitly - // told us this token is no longer valid. We must refresh or re-authenticate. - isTokenInvalid := strings.Contains(errMsg, "token is not active") || - strings.Contains(errMsg, "revoked") || - strings.Contains(errMsg, "token has expired") - - if isTokenInvalid { - t.logger.Infof("⚠️ Token explicitly marked as invalid by provider, cannot fall back to ID token") - if session.GetRefreshToken() != "" { - t.logger.Debug("Refresh token available, attempting refresh") - return false, true, false - } - t.logger.Debug("No refresh token available, must re-authenticate") - return false, false, true - } - - // If introspection required, reject the session - if t.requireTokenIntrospection { - t.logger.Errorf("❌ SECURITY: Opaque token rejected (introspection required but failed)") - if session.GetRefreshToken() != "" { - return false, true, false - } - return false, false, true - } - - // Only fall back to ID token validation for transient errors (network issues, etc.) - // where the introspection endpoint couldn't be reached - t.logger.Infof("⚠️ Falling back to ID token validation for opaque access token (transient error)") - } else { - // Introspection successful - t.logger.Debugf("✓ Opaque access token validated via introspection") - // Still need to check ID token for session expiry - idToken := session.GetIDToken() - if idToken != "" { - return t.validateTokenExpiry(session, idToken) - } - return true, false, false - } - } else { - // Opaque tokens not allowed - log warning and reject or fall back - t.logger.Infof("⚠️ Opaque access token detected but allowOpaqueTokens=false") - } - - // Fall back to ID token validation - idToken := session.GetIDToken() - if idToken == "" { - t.logger.Debug("Opaque access token present but no ID token found") - if session.GetRefreshToken() != "" { - t.logger.Debug("ID token missing but refresh token exists. Signaling need for refresh.") - return false, true, false - } - // Accept session with opaque access token even without ID token - // The OAuth provider validated it when issued - t.logger.Debug("Accepting session with opaque access token") - return true, false, false - } - - // Validate ID token if present - if err := t.verifyToken(idToken); err != nil { - if strings.Contains(err.Error(), "token has expired") { - t.logger.Debugf("ID token expired with opaque access token, needs refresh") - if session.GetRefreshToken() != "" { - return false, true, false - } - return false, false, true - } - - t.logger.Errorf("ID token verification failed with opaque access token: %v", err) - if session.GetRefreshToken() != "" { - return false, true, false - } - return false, false, true - } - - // Use ID token for expiry validation - return t.validateTokenExpiry(session, idToken) - } - - // JWT access token present - validate it explicitly to detect Scenario 2 - // (Option C: Scenario 2 detection and strict mode) - accessTokenValid := false - accessTokenError := "" - - if err := t.verifyToken(accessToken); err != nil { - // Access token validation failed - accessTokenError = err.Error() - - // Check if it's an audience validation failure (Scenario 2) - if strings.Contains(accessTokenError, "invalid audience") || strings.Contains(accessTokenError, "audience") { - // SCENARIO 2 DETECTED: Access token has wrong audience - t.logger.Infof("⚠️ SCENARIO 2 DETECTED: Access token validation failed due to audience mismatch: %v", err) - - if t.strictAudienceValidation { - // Strict mode: Reject the session (don't fall back to ID token) - t.logger.Errorf("❌ SECURITY: Session rejected due to access token audience mismatch (strictAudienceValidation=true)") - t.logger.Errorf("❌ This prevents potential cross-API token confusion attacks (Auth0 Scenario 2)") - if session.GetRefreshToken() != "" { - return false, true, false // try refresh - } - return false, false, true // must re-authenticate - } - // Backward compatibility mode: Log loud warning but allow fallback to ID token - t.logger.Infof("⚠️⚠️⚠️ SECURITY WARNING: Falling back to ID token validation despite access token audience mismatch!") - t.logger.Infof("⚠️ This could allow tokens intended for different APIs to grant access") - t.logger.Infof("⚠️ Set strictAudienceValidation=true to enforce proper audience validation") - t.logger.Infof("⚠️ See: https://github.com/lukaszraczylo/traefikoidc/issues/74") - } else if !strings.Contains(accessTokenError, "token has expired") { - // Other validation errors (not expiration, not audience) - t.logger.Debugf("Access token validation failed (non-expiration, non-audience): %v", err) - } - } else { - // Access token is valid - accessTokenValid = true - } - - idToken := session.GetIDToken() - if idToken == "" { - if accessTokenValid { - // Access token is valid, no ID token needed - t.logger.Debug("Access token valid, no ID token present") - return t.validateTokenExpiry(session, accessToken) - } - - t.logger.Debug("Authenticated flag set with access token, but no ID token found in session") - if session.GetRefreshToken() != "" { - t.logger.Debug("ID token missing but refresh token exists. Signaling conditional refresh to obtain ID token.") - return true, true, false - } - return true, false, false - } - - // Validate ID token - if err := t.verifyToken(idToken); err != nil { - if strings.Contains(err.Error(), "token has expired") { - t.logger.Debugf("ID token signature/claims valid but token expired, needs refresh") - if session.GetRefreshToken() != "" { - return false, true, false - } - return false, false, true - } - - t.logger.Errorf("ID token verification failed (non-expiration): %v", err) - if session.GetRefreshToken() != "" { - t.logger.Debug("ID token verification failed, but refresh token exists. Signaling need for refresh.") - return false, true, false - } - return false, false, true - } - - // If access token was valid, use it for expiry; otherwise use ID token - if accessTokenValid { - return t.validateTokenExpiry(session, accessToken) - } - - return t.validateTokenExpiry(session, idToken) -} - -// validateTokenExpiry checks if a token is nearing expiration and needs refresh. -// It uses the configured grace period to determine when proactive refresh should occur. -// Parameters: -// - session: The session data for refresh token availability. -// - token: The token to check expiry for. -// -// Returns: -// - authenticated: Whether the token is currently valid. -// - needsRefresh: Whether the token is nearing expiration and should be refreshed. -// - expired: Whether the token is invalid or verification failed. -func (t *TraefikOidc) validateTokenExpiry(session *SessionData, token string) (bool, bool, bool) { - cachedClaims, found := t.tokenCache.Get(token) - if !found { - t.logger.Debug("Claims not found in cache after successful token verification") - if session.GetRefreshToken() != "" { - t.logger.Debug("Claims missing post-verification, attempting refresh to recover.") - return false, true, false - } - return false, false, true - } - - expClaim, ok := cachedClaims["exp"].(float64) - if !ok { - t.logger.Error("Failed to get expiration time ('exp' claim) from verified token") - if session.GetRefreshToken() != "" { - t.logger.Debug("Token missing 'exp' claim, but refresh token exists. Signaling need for refresh.") - return false, true, false - } - return false, false, true - } - - expTime := int64(expClaim) - expTimeObj := time.Unix(expTime, 0) - nowObj := time.Now() - - // Check if token has already expired - if expTimeObj.Before(nowObj) { - // Token has expired - expiredDuration := nowObj.Sub(expTimeObj) - - t.logger.Debugf("Token expired %v ago, grace period is %v", - expiredDuration, t.refreshGracePeriod) - - // If we have a refresh token, always attempt to use it regardless of grace period - // The refresh token has its own expiry and the provider will reject it if invalid - if session.GetRefreshToken() != "" { - t.logger.Debugf("Token expired, attempting refresh with available refresh token") - return false, true, false // needs refresh - } - - // No refresh token available - must re-authenticate - t.logger.Debugf("Token expired and no refresh token available, must re-authenticate") - return false, false, true // expired, cannot refresh - } - - // Token not yet expired - check if nearing expiration - refreshThreshold := nowObj.Add(t.refreshGracePeriod) - - t.logger.Debugf("Token expires at %v, now is %v, refresh threshold is %v", - expTimeObj.Format(time.RFC3339), - nowObj.Format(time.RFC3339), - refreshThreshold.Format(time.RFC3339)) - - if expTimeObj.Before(refreshThreshold) { - remainingSeconds := int64(time.Until(expTimeObj).Seconds()) - t.logger.Debugf("Token nearing expiration (expires in %d seconds, grace period %s), scheduling proactive refresh", - remainingSeconds, t.refreshGracePeriod) - - if session.GetRefreshToken() != "" { - return true, true, false - } - - t.logger.Debugf("Token nearing expiration but no refresh token available, cannot proactively refresh.") - return true, false, false - } - - t.logger.Debugf("Token is valid and not nearing expiration (expires in %d seconds, outside %s grace period)", - int64(time.Until(expTimeObj).Seconds()), t.refreshGracePeriod) - - return true, false, false -} // startTokenCleanup starts background cleanup goroutines for cache maintenance. // It runs periodic cleanup of token cache, JWK cache, and session chunks. diff --git a/token_validation_rs.go b/token_validation_rs.go index ce291cf..c2aa6d0 100644 --- a/token_validation_rs.go +++ b/token_validation_rs.go @@ -26,11 +26,18 @@ func (t *TraefikOidc) isUserAuthenticatedRS(rs *requestState) (bool, bool, bool) if t.isAzureProvider() { return t.validateAzureTokensRS(rs) } else if t.isGoogleProvider() { - return t.validateStandardTokensRS(rs) + return t.validateGoogleTokensRS(rs) } return t.validateStandardTokensRS(rs) } +// validateGoogleTokensRS handles Google-specific token validation. Currently +// delegates to standard token validation; retained as a hook for any future +// Google-specific behavior (matches the v1.0.20 layout of the non-RS variant). +func (t *TraefikOidc) validateGoogleTokensRS(rs *requestState) (bool, bool, bool) { + return t.validateStandardTokensRS(rs) +} + // validateTokenExpiryRS is the requestState-aware variant of validateTokenExpiry. // Reads rs.refreshToken instead of session.GetRefreshToken() (4 RLocks avoided). func (t *TraefikOidc) validateTokenExpiryRS(rs *requestState, token string) (bool, bool, bool) {