Add documentation for the google provider session timeouts. (#39)

2026-06-05 22:44:17 +00:00 · 2025-04-14 00:00:56 +01:00
parent 8a6e37f7fc
commit 983585e96e
3 changed files with 286 additions and 23 deletions
@@ -259,6 +259,34 @@ spec:
        - profile
 ```

+### Google OIDC Configuration Example
+
+This example shows a configuration specifically tailored for Google OIDC, including necessary scopes for session extension:
+
+```yaml
+apiVersion: traefik.io/v1alpha1
+kind: Middleware
+metadata:
+  name: oidc-google
+  namespace: traefik
+spec:
+  plugin:
+    traefikoidc:
+      providerURL: https://accounts.google.com
+      clientID: your-google-client-id.apps.googleusercontent.com # Replace with your Client ID
+      clientSecret: your-google-client-secret                     # Replace with your Client Secret
+      sessionEncryptionKey: your-secure-encryption-key-min-32-chars # Replace with your key
+      callbackURL: /oauth2/callback                             # Adjust if needed
+      logoutURL: /oauth2/logout                                 # Optional: Adjust if needed
+      scopes:
+        - openid
+        - email
+        - profile
+        - offline_access # Required for refresh tokens / long sessions with Google
+      refreshGracePeriodSeconds: 300  # Optional: Start refresh 5 min before expiry (default 60)
+      # Other optional parameters like allowedUserDomains, etc. can be added here
+```
+
 ### Keeping Secrets Secret in Kubernetes

 For Kubernetes environments, you can reference secrets instead of hardcoding sensitive values:
@@ -415,6 +443,23 @@ PKCE is recommended when:

 Note that not all OIDC providers support PKCE, so check your provider's documentation before enabling this feature.

+### Session Duration and Token Refresh
+
+This middleware aims to provide long-lived user sessions, typically up to 24 hours, by utilizing OIDC refresh tokens.
+
+**How it works:**
+- When a user authenticates, the middleware requests an access token and, if available, a refresh token from the OIDC provider.
+- The access token usually has a short lifespan (e.g., 1 hour).
+- Before the access token expires (controlled by `refreshGracePeriodSeconds`), the middleware uses the refresh token to obtain a new access token from the provider without requiring the user to log in again.
+- This process repeats, allowing the session to remain valid for as long as the refresh token is valid (often 24 hours or more, depending on the provider).
+
+**Provider-Specific Considerations (e.g., Google):**
+- Some providers, like Google, issue short-lived access tokens (e.g., 1 hour) and require specific configurations for long-term sessions.
+- To enable session extension beyond the initial token expiry with Google and similar providers, the middleware automatically includes the `offline_access` scope in the authentication request. This scope is necessary to obtain a refresh token.
+- For Google specifically, the middleware also adds the `prompt=consent` parameter to the initial authorization request. This ensures Google issues a refresh token, which is crucial for extending the session.
+- If a refresh attempt fails (e.g., the refresh token is revoked or expired), the user will be required to re-authenticate. The middleware includes enhanced error handling and logging for these scenarios.
+- Ensure your OIDC provider is configured to issue refresh tokens and allows their use for extending sessions. Check your provider's documentation for details on refresh token validity periods.
+
 ### Token Caching and Blacklisting

 The middleware automatically caches validated tokens to improve performance and maintains a blacklist of revoked tokens.
@@ -456,6 +501,10 @@ logLevel: debug
 3. **No matching public key found**: The JWKS endpoint might be unavailable or the token's key ID (kid) doesn't match any key in the JWKS.
 4. **Access denied: Your email domain is not allowed**: The user's email domain is not in the `allowedUserDomains` list.
 5. **Access denied: You do not have any of the allowed roles or groups**: The user doesn't have any of the roles or groups specified in `allowedRolesAndGroups`.
+6. **Google sessions expire after ~1 hour**: If using Google as the OIDC provider and sessions expire prematurely (around 1 hour instead of longer), ensure:
+   - The `offline_access` scope is included in your configuration (the middleware adds this automatically now, but verify if manually configured).
+   - Your Google Cloud OAuth consent screen is set to "External" and "Production" mode. "Testing" mode often limits refresh token validity.
+   - The fix involving automatic `offline_access` scope and `prompt=consent` for Google is active in your middleware version. Check the plugin version corresponds to when this fix was implemented. Enhanced logging around refresh token failures can provide more clues if issues persist.

 ## Contributing

@@ -0,0 +1,147 @@
+package traefikoidc
+
+import (
+	"fmt"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+)
+
+// MockTokenVerifier implements the TokenVerifier interface for testing
+type MockTokenVerifier struct {
+	VerifyFunc func(token string) error
+}
+
+func (m *MockTokenVerifier) VerifyToken(token string) error {
+	if m.VerifyFunc != nil {
+		return m.VerifyFunc(token)
+	}
+	return nil
+}
+
+func TestGoogleOIDCRefreshTokenHandling(t *testing.T) {
+	// Create a mocked TraefikOidc instance that simulates Google provider behavior
+	mockLogger := NewLogger("debug")
+
+	// Create a test instance with a Google-like issuer URL
+	tOidc := &TraefikOidc{
+		issuerURL:          "https://accounts.google.com",
+		clientID:           "test-client-id",
+		clientSecret:       "test-client-secret",
+		logger:             mockLogger,
+		scopes:             []string{"openid", "profile", "email"},
+		refreshGracePeriod: 60,
+	}
+
+	// Create a session manager
+	sessionManager, _ := NewSessionManager("0123456789abcdef0123456789abcdef", true, mockLogger)
+	tOidc.sessionManager = sessionManager
+
+	t.Run("Google provider detection adds required parameters", func(t *testing.T) {
+		// Test buildAuthURL to ensure it adds offline_access and prompt=consent for Google
+		authURL := tOidc.buildAuthURL("https://example.com/callback", "state123", "nonce123", "")
+
+		// Check that offline_access scope was added
+		if !strings.Contains(authURL, "scope=") || !strings.Contains(authURL, "offline_access") {
+			t.Errorf("offline_access scope not added to Google auth URL: %s", authURL)
+		}
+
+		// Check that prompt=consent was added
+		if !strings.Contains(authURL, "prompt=consent") {
+			t.Errorf("prompt=consent not added to Google auth URL: %s", authURL)
+		}
+	})
+
+	t.Run("Non-Google provider doesn't add Google-specific params", func(t *testing.T) {
+		// Create a test instance with a non-Google issuer URL
+		nonGoogleOidc := &TraefikOidc{
+			issuerURL:    "https://auth.example.com",
+			clientID:     "test-client-id",
+			clientSecret: "test-client-secret",
+			logger:       mockLogger,
+			scopes:       []string{"openid", "profile", "email"},
+		}
+
+		// Test buildAuthURL without Google-specific parameters
+		authURL := nonGoogleOidc.buildAuthURL("https://example.com/callback", "state123", "nonce123", "")
+
+		// Check that prompt=consent is not automatically added
+		if strings.Contains(authURL, "prompt=consent") {
+			t.Errorf("prompt=consent added to non-Google auth URL: %s", authURL)
+		}
+	})
+
+	t.Run("Session refresh with Google provider", func(t *testing.T) {
+		// Create a request and response recorder
+		req := httptest.NewRequest("GET", "/test", nil)
+		rw := httptest.NewRecorder()
+
+		// Create a session and set a refresh token
+		session, _ := sessionManager.GetSession(req)
+		session.SetAuthenticated(true)
+		session.SetEmail("test@example.com")
+		session.SetAccessToken("old-access-token")
+		session.SetRefreshToken("valid-refresh-token")
+
+		// Create a mock token exchanger that simulates Google's behavior
+		mockTokenExchanger := &MockTokenExchanger{
+			RefreshTokenFunc: func(refreshToken string) (*TokenResponse, error) {
+				// Check that the refresh token is passed correctly
+				if refreshToken != "valid-refresh-token" {
+					t.Errorf("Incorrect refresh token passed: %s", refreshToken)
+					return nil, fmt.Errorf("invalid token")
+				}
+
+				// Return a simulated Google token response with a new access token
+				// but without a new refresh token (Google doesn't always return a new refresh token)
+				return &TokenResponse{
+					IDToken:      "new-id-token-from-google",
+					AccessToken:  "new-access-token-from-google",
+					RefreshToken: "", // Google often doesn't return a new refresh token
+					ExpiresIn:    3600,
+				}, nil
+			},
+		}
+
+		// Set the mock token exchanger
+		tOidc.tokenExchanger = mockTokenExchanger
+
+		// Create a struct that implements the TokenVerifier interface
+		tOidc.tokenVerifier = &MockTokenVerifier{
+			VerifyFunc: func(token string) error {
+				return nil
+			},
+		}
+
+		tOidc.extractClaimsFunc = func(token string) (map[string]interface{}, error) {
+			// Return mock claims
+			return map[string]interface{}{
+				"email": "test@example.com",
+				"exp":   float64(time.Now().Add(1 * time.Hour).Unix()),
+			}, nil
+		}
+
+		// Attempt to refresh the token
+		refreshed := tOidc.refreshToken(rw, req, session)
+
+		// Verify the refresh was successful
+		if !refreshed {
+			t.Error("Token refresh failed for Google provider")
+		}
+
+		// Check that we kept the original refresh token since Google didn't provide a new one
+		if session.GetRefreshToken() != "valid-refresh-token" {
+			t.Errorf("Original refresh token not preserved: got %s, expected 'valid-refresh-token'",
+				session.GetRefreshToken())
+		}
+
+		// Check that the access token was updated
+		if session.GetAccessToken() != "new-id-token-from-google" {
+			t.Errorf("Access token not updated: got %s, expected 'new-id-token-from-google'",
+				session.GetAccessToken())
+		}
+	})
+}
+
+// No need to redefine MockTokenExchanger - it's already defined in main_test.go
@@ -17,14 +17,6 @@ import (
 	"golang.org/x/time/rate"
 )

-// min returns the smaller of x or y.
-func min(x, y int) int {
-	if x > y {
-		return y
-	}
-	return x
-}
-
 // createDefaultHTTPClient creates a new http.Client with settings optimized for OIDC communication.
 // It configures the transport with specific timeouts (dial, keepalive, TLS handshake, idle connection),
 // connection limits (max idle, max per host), enables HTTP/2, and sets a default request timeout.
@@ -1288,8 +1280,34 @@ func (t *TraefikOidc) buildAuthURL(redirectURL, state, nonce, codeChallenge stri
 		params.Set("code_challenge_method", "S256")
 	}

-	if len(t.scopes) > 0 {
-		params.Set("scope", strings.Join(t.scopes, " "))
+	// Handle scopes - ensure offline_access is included for refresh tokens
+	scopes := make([]string, len(t.scopes))
+	copy(scopes, t.scopes)
+
+	// Check if we're dealing with a Google OIDC provider
+	isGoogleProvider := strings.Contains(t.issuerURL, "google") || strings.Contains(t.issuerURL, "accounts.google.com")
+
+	// Add offline_access scope if it's missing
+	hasOfflineAccess := false
+	for _, scope := range scopes {
+		if scope == "offline_access" {
+			hasOfflineAccess = true
+			break
+		}
+	}
+
+	if !hasOfflineAccess {
+		scopes = append(scopes, "offline_access")
+	}
+
+	if len(scopes) > 0 {
+		params.Set("scope", strings.Join(scopes, " "))
+	}
+
+	// Add prompt=consent for Google to ensure refresh token is issued
+	if isGoogleProvider {
+		params.Set("prompt", "consent")
+		t.logger.Debug("Google OIDC provider detected, added prompt=consent to ensure refresh tokens")
 	}

 	// Use buildURLWithParams which handles potential relative authURL from metadata
@@ -1450,21 +1468,55 @@ func (t *TraefikOidc) refreshToken(rw http.ResponseWriter, req *http.Request, se
 		return false
 	}

-	// Store the initial token for later comparison
-	t.logger.Debugf("Attempting refresh with token starting with %s...", initialRefreshToken[:min(len(initialRefreshToken), 10)])
+	// Detect if we're using Google's OIDC provider
+	isGoogleProvider := strings.Contains(t.issuerURL, "google") || strings.Contains(t.issuerURL, "accounts.google.com")
+	if isGoogleProvider {
+		t.logger.Debug("Google OIDC provider detected for token refresh operation")
+	}

+	// Log the attempt with a truncated token for security
+	tokenPrefix := initialRefreshToken
+	if len(initialRefreshToken) > 10 {
+		tokenPrefix = initialRefreshToken[:10]
+	}
+	t.logger.Debugf("Attempting refresh with token starting with %s...", tokenPrefix)
+
+	// Attempt to refresh the token
 	newToken, err := t.tokenExchanger.GetNewTokenWithRefreshToken(initialRefreshToken)
 	if err != nil {
-		// Log the error more explicitly before returning false
-		truncatedToken := initialRefreshToken[:min(len(initialRefreshToken), 10)] // Log first 10 chars
-		t.logger.Errorf("refreshToken failed: Error from tokenExchanger.GetNewTokenWithRefreshToken for token starting with %s...: %v", truncatedToken, err)
-		// No need to clear token here, as the session might be cleared by another request anyway
+		// Log detailed error information
+		t.logger.Errorf("refreshToken failed: Error from token refresh operation: %v", err)
+
+		// Check for specific error patterns
+		errMsg := err.Error()
+		if strings.Contains(errMsg, "invalid_grant") || strings.Contains(errMsg, "token expired") {
+			t.logger.Errorf("Refresh token appears to be expired or revoked: %v", err)
+			// Don't keep trying with an invalid refresh token
+			session.SetRefreshToken("")
+			if err := session.Save(req, rw); err != nil {
+				t.logger.Errorf("Failed to remove invalid refresh token from session: %v", err)
+			}
+		} else if strings.Contains(errMsg, "invalid_client") {
+			t.logger.Errorf("Client credentials rejected: %v - check client_id and client_secret configuration", err)
+		} else if isGoogleProvider && strings.Contains(errMsg, "invalid_request") {
+			t.logger.Errorf("Google OIDC provider error: %v - check scope configuration includes 'offline_access' and prompt=consent is used during authentication", err)
+		}
+
+		return false
+	}
+
+	// Handle potentially missing tokens in the response
+	if newToken.IDToken == "" {
+		t.logger.Errorf("refreshToken failed: Provider did not return a new ID token")
 		return false
 	}

 	// Verify the new access token (ID token)
 	if err := t.verifyToken(newToken.IDToken); err != nil {
-		truncatedNewToken := newToken.IDToken[:min(len(newToken.IDToken), 10)] // Log first 10 chars
+		truncatedNewToken := newToken.IDToken
+		if len(newToken.IDToken) > 10 {
+			truncatedNewToken = newToken.IDToken[:10]
+		}
 		t.logger.Errorf("refreshToken failed: Failed to verify newly obtained ID token starting with %s...: %v", truncatedNewToken, err)
 		return false
 	}
@@ -1475,7 +1527,7 @@ func (t *TraefikOidc) refreshToken(rw http.ResponseWriter, req *http.Request, se
 	currentRefreshToken := session.GetRefreshToken() // Get token again *after* the potentially long exchange
 	if initialRefreshToken != currentRefreshToken {
 		// Use Infof as Warnf doesn't exist
-		t.logger.Infof("refreshToken aborted: Session refresh token changed concurrently during refresh attempt. Initial token prefix: %s..., Current token prefix: %s...", initialRefreshToken[:min(len(initialRefreshToken), 10)], currentRefreshToken[:min(len(currentRefreshToken), 10)])
+		t.logger.Infof("refreshToken aborted: Session refresh token changed concurrently during refresh attempt.")
 		// Do not save the new tokens, as the session state is likely invalid/cleared.
 		return false // Indicate refresh failure due to concurrency conflict
 	}
@@ -1497,24 +1549,39 @@ func (t *TraefikOidc) refreshToken(rw http.ResponseWriter, req *http.Request, se
 	}
 	session.SetEmail(email) // Update email in session

+	// Get token expiry information for logging
+	var expiryTime time.Time
+	if expClaim, ok := claims["exp"].(float64); ok {
+		expiryTime = time.Unix(int64(expClaim), 0)
+		t.logger.Debugf("New token expires at: %v (in %v)", expiryTime, time.Until(expiryTime))
+	}
+
+	// Set the new access token
 	session.SetAccessToken(newToken.IDToken)
-	// Ensure the new refresh token is actually set, even if it's the same as the old one
-	// Also handle cases where the provider might not return a new refresh token
+
+	// Handle the refresh token
 	if newToken.RefreshToken != "" {
+		t.logger.Debug("Received new refresh token from provider")
 		session.SetRefreshToken(newToken.RefreshToken)
 	} else {
 		// If no new refresh token is returned, keep the existing one
+		t.logger.Debug("Provider did not return a new refresh token, keeping the existing one")
 		session.SetRefreshToken(initialRefreshToken)
-		t.logger.Debugf("Provider did not return a new refresh token, keeping the existing one.")
+	}
+
+	// Ensure authenticated flag is set
+	if err := session.SetAuthenticated(true); err != nil {
+		t.logger.Errorf("refreshToken warning: Failed to set authenticated flag: %v", err)
+		// Continue anyway since we have valid tokens
 	}

 	// Save the session
 	if err := session.Save(req, rw); err != nil {
-		t.logger.Errorf("refreshToken failed: Failed to save session after successful token refresh and concurrency check: %v", err)
+		t.logger.Errorf("refreshToken failed: Failed to save session after successful token refresh: %v", err)
 		return false
 	}

-	t.logger.Debugf("Token refresh successful and session saved.")
+	t.logger.Debugf("Token refresh successful and session saved")
 	return true
 }