From 66b9ed0861c52f10fbfaddf2a85f695a55343f24 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Mon, 1 Dec 2025 13:47:28 +0000 Subject: [PATCH] Reauthentication + redis fix When introspection explicitly returns that a token is inactive/revoked/expired, the plugin now properly triggers re-authentication or refresh instead of falling back to ID token validation. This fixes the functional issue where users weren't being redirected to re-authenticate. Redis change ensures that when the caller's context is cancelled (e.g., the 200ms timeout in UniversalCache.Get()), the operation aborts quickly instead of continuing with retries. --- internal/cache/backends/redis.go | 25 ++++++++++++++++++++----- token_manager.go | 23 +++++++++++++++++++++-- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/internal/cache/backends/redis.go b/internal/cache/backends/redis.go index faee2c2..db6c0c5 100644 --- a/internal/cache/backends/redis.go +++ b/internal/cache/backends/redis.go @@ -43,14 +43,17 @@ func NewRedisBackend(config *Config) (*RedisBackend, error) { } // Create connection pool with health checks enabled + // Timeouts are kept short to prevent request pileup when Redis is slow/stalled. + // The UniversalCache uses 200ms context timeout, so socket timeouts should be + // shorter to allow proper context cancellation handling. poolConfig := &PoolConfig{ Address: config.RedisAddr, Password: config.RedisPassword, DB: config.RedisDB, MaxConnections: config.PoolSize, - ConnectTimeout: 5 * time.Second, - ReadTimeout: 3 * time.Second, - WriteTimeout: 3 * time.Second, + ConnectTimeout: 2 * time.Second, + ReadTimeout: 500 * time.Millisecond, + WriteTimeout: 500 * time.Millisecond, EnableHealthCheck: true, MaxRetries: 3, RetryDelay: 100 * time.Millisecond, @@ -340,12 +343,19 @@ func (r *RedisBackend) prefixKey(key string) string { return r.config.RedisPrefix + key } -// executeWithRetry executes a Redis operation with exponential backoff retry logic +// executeWithRetry executes a Redis operation with exponential backoff retry logic. +// It checks context cancellation at multiple points to ensure fast abort when the +// caller's context is cancelled (e.g., due to request timeout). func (r *RedisBackend) executeWithRetry(ctx context.Context, operation func(*RedisConn) error) error { maxRetries := 3 - baseDelay := 100 * time.Millisecond + baseDelay := 50 * time.Millisecond // Reduced from 100ms to fail faster for attempt := 0; attempt < maxRetries; attempt++ { + // Check context before each attempt to fail fast + if ctx.Err() != nil { + return ctx.Err() + } + conn, err := r.pool.Get(ctx) if err != nil { // If we can't get a connection and this is the last attempt, fail @@ -367,6 +377,11 @@ func (r *RedisBackend) executeWithRetry(ctx context.Context, operation func(*Red err = operation(conn) r.pool.Put(conn) + // Check context after operation - if cancelled, don't bother retrying + if ctx.Err() != nil { + return ctx.Err() + } + // If successful, return if err == nil { return nil diff --git a/token_manager.go b/token_manager.go index fe589fb..76411ce 100644 --- a/token_manager.go +++ b/token_manager.go @@ -902,8 +902,26 @@ func (t *TraefikOidc) validateStandardTokens(session *SessionData) (bool, bool, // 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)") @@ -913,8 +931,9 @@ func (t *TraefikOidc) validateStandardTokens(session *SessionData) (bool, bool, return false, false, true } - // Otherwise fall back to ID token validation (Scenario 3 backward compatibility) - t.logger.Infof("⚠️ Falling back to ID token validation for opaque access token") + // 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")