fixup! fixup! Further pursue of perfection.

This commit is contained in:
2025-09-05 00:22:13 +01:00
parent f291dcca26
commit 2c902eaafb
11 changed files with 238 additions and 63 deletions
+8
View File
@@ -144,9 +144,17 @@ func (c *Cache) Delete(key string) {
// This method is called automatically by the background cleanup task but can also // This method is called automatically by the background cleanup task but can also
// be called manually for immediate cleanup of expired entries. // be called manually for immediate cleanup of expired entries.
func (c *Cache) Cleanup() { func (c *Cache) Cleanup() {
if c == nil {
return
}
c.mutex.Lock() c.mutex.Lock()
defer c.mutex.Unlock() defer c.mutex.Unlock()
if c.items == nil {
return
}
now := time.Now() now := time.Now()
for key, item := range c.items { for key, item := range c.items {
if now.After(item.ExpiresAt) { if now.After(item.ExpiresAt) {
+13 -3
View File
@@ -172,20 +172,30 @@ func (c *OptimizedCache) Delete(key string) {
// It scans the LRU list for expired entries and enforces memory limits // It scans the LRU list for expired entries and enforces memory limits
// by evicting least recently used items if necessary. // by evicting least recently used items if necessary.
func (c *OptimizedCache) Cleanup() { func (c *OptimizedCache) Cleanup() {
if c == nil {
return
}
c.mutex.Lock() c.mutex.Lock()
defer c.mutex.Unlock() defer c.mutex.Unlock()
if c.items == nil || c.head == nil || c.tail == nil {
return
}
now := time.Now() now := time.Now()
toRemove := make([]*OptimizedCacheEntry, 0, len(c.items)/10) toRemove := make([]*OptimizedCacheEntry, 0, len(c.items)/10)
for entry := c.head.next; entry != c.tail; entry = entry.next { for entry := c.head.next; entry != nil && entry != c.tail; entry = entry.next {
if now.After(entry.ExpiresAt) { if entry != nil && now.After(entry.ExpiresAt) {
toRemove = append(toRemove, entry) toRemove = append(toRemove, entry)
} }
} }
for _, entry := range toRemove { for _, entry := range toRemove {
c.removeEntry(entry) if entry != nil {
c.removeEntry(entry)
}
} }
for c.currentMemoryBytes > c.maxMemoryBytes && len(c.items) > 0 { for c.currentMemoryBytes > c.maxMemoryBytes && len(c.items) > 0 {
+7 -16
View File
@@ -364,33 +364,24 @@ func TestReplayCacheBasic(t *testing.T) {
// Test audience verification // Test audience verification
func TestAudienceVerify(t *testing.T) { func TestAudienceVerify(t *testing.T) {
// Exact match // Exact match
claims1 := map[string]interface{}{ if err := verifyAudience("my-client", "my-client"); err != nil {
"aud": "my-client",
}
if err := verifyAudience(claims1, "my-client"); err != nil {
t.Errorf("Exact match should succeed: %v", err) t.Errorf("Exact match should succeed: %v", err)
} }
// Array contains // Array contains
claims2 := map[string]interface{}{ audArray := []interface{}{"client1", "my-client", "client2"}
"aud": []interface{}{"client1", "my-client", "client2"}, if err := verifyAudience(audArray, "my-client"); err != nil {
}
if err := verifyAudience(claims2, "my-client"); err != nil {
t.Errorf("Array contains should succeed: %v", err) t.Errorf("Array contains should succeed: %v", err)
} }
// Mismatch // Mismatch
claims3 := map[string]interface{}{ if err := verifyAudience("other-client", "my-client"); err == nil {
"aud": "other-client",
}
if err := verifyAudience(claims3, "my-client"); err == nil {
t.Error("Mismatch should fail") t.Error("Mismatch should fail")
} }
// Missing // Missing/nil audience
claims4 := map[string]interface{}{} if err := verifyAudience(nil, "my-client"); err == nil {
if err := verifyAudience(claims4, "my-client"); err == nil { t.Error("Nil aud should fail")
t.Error("Missing aud should fail")
} }
} }
+3 -1
View File
@@ -38,7 +38,9 @@ func TestInputValidatorExtended(t *testing.T) {
func TestSessionManagerCreation(t *testing.T) { func TestSessionManagerCreation(t *testing.T) {
logger := NewLogger("debug") logger := NewLogger("debug")
sm, err := NewSessionManager("test-secret", false, "Lax", logger) // Use a 32-byte encryption key as required
encryptionKey := "12345678901234567890123456789012"
sm, err := NewSessionManager(encryptionKey, false, "Lax", logger)
if err != nil { if err != nil {
t.Fatalf("NewSessionManager failed: %v", err) t.Fatalf("NewSessionManager failed: %v", err)
} }
+122
View File
@@ -589,6 +589,128 @@ func (e *HTTPError) Error() string {
return fmt.Sprintf("HTTP %d: %s", e.StatusCode, e.Message) return fmt.Sprintf("HTTP %d: %s", e.StatusCode, e.Message)
} }
// OIDCError represents OIDC-specific errors with context information.
// It provides structured error reporting for authentication and authorization failures.
type OIDCError struct {
// Code identifies the specific error type
Code string
// Message provides a human-readable description
Message string
// Context contains additional error context (e.g., provider, session details)
Context map[string]interface{}
// Cause is the underlying error that caused this error
Cause error
}
// Error returns the string representation of the OIDC error.
// Implements the error interface.
func (e *OIDCError) Error() string {
if e.Cause != nil {
return fmt.Sprintf("OIDC error [%s]: %s - caused by: %v", e.Code, e.Message, e.Cause)
}
return fmt.Sprintf("OIDC error [%s]: %s", e.Code, e.Message)
}
// Unwrap returns the underlying error for error chain unwrapping.
func (e *OIDCError) Unwrap() error {
return e.Cause
}
// SessionError represents session-related errors with context.
// Used for session management, validation, and storage errors.
type SessionError struct {
// Operation describes what session operation failed
Operation string
// Message provides a human-readable description
Message string
// SessionID identifies the session (if available)
SessionID string
// Cause is the underlying error that caused this error
Cause error
}
// Error returns the string representation of the session error.
// Implements the error interface.
func (e *SessionError) Error() string {
if e.Cause != nil {
return fmt.Sprintf("Session error in %s: %s - caused by: %v", e.Operation, e.Message, e.Cause)
}
return fmt.Sprintf("Session error in %s: %s", e.Operation, e.Message)
}
// Unwrap returns the underlying error for error chain unwrapping.
func (e *SessionError) Unwrap() error {
return e.Cause
}
// TokenError represents token-related errors with validation context.
// Used for JWT validation, token refresh, and token format errors.
type TokenError struct {
// TokenType identifies the type of token (id_token, access_token, refresh_token)
TokenType string
// Reason describes why the token is invalid
Reason string
// Message provides a human-readable description
Message string
// Cause is the underlying error that caused this error
Cause error
}
// Error returns the string representation of the token error.
// Implements the error interface.
func (e *TokenError) Error() string {
if e.Cause != nil {
return fmt.Sprintf("Token error (%s) - %s: %s - caused by: %v", e.TokenType, e.Reason, e.Message, e.Cause)
}
return fmt.Sprintf("Token error (%s) - %s: %s", e.TokenType, e.Reason, e.Message)
}
// Unwrap returns the underlying error for error chain unwrapping.
func (e *TokenError) Unwrap() error {
return e.Cause
}
// NewOIDCError creates a new OIDC error with context.
func NewOIDCError(code, message string, cause error) *OIDCError {
return &OIDCError{
Code: code,
Message: message,
Context: make(map[string]interface{}),
Cause: cause,
}
}
// WithContext adds context information to the OIDC error.
func (e *OIDCError) WithContext(key string, value interface{}) *OIDCError {
e.Context[key] = value
return e
}
// NewSessionError creates a new session error with operation context.
func NewSessionError(operation, message string, cause error) *SessionError {
return &SessionError{
Operation: operation,
Message: message,
Cause: cause,
}
}
// WithSessionID adds session ID to the session error.
func (e *SessionError) WithSessionID(sessionID string) *SessionError {
e.SessionID = sessionID
return e
}
// NewTokenError creates a new token error with type and reason.
func NewTokenError(tokenType, reason, message string, cause error) *TokenError {
return &TokenError{
TokenType: tokenType,
Reason: reason,
Message: message,
Cause: cause,
}
}
// GracefulDegradation implements graceful degradation patterns for service resilience. // GracefulDegradation implements graceful degradation patterns for service resilience.
// It provides fallback mechanisms when primary services are unavailable and monitors // It provides fallback mechanisms when primary services are unavailable and monitors
// service health to automatically recover when services become available again. // service health to automatically recover when services become available again.
+3 -1
View File
@@ -263,7 +263,9 @@ func (tc *TokenCache) Delete(token string) {
// This is typically called automatically but can be invoked manually for // This is typically called automatically but can be invoked manually for
// removing expired token entries. // removing expired token entries.
func (tc *TokenCache) Cleanup() { func (tc *TokenCache) Cleanup() {
tc.cache.Cleanup() if tc != nil && tc.cache != nil {
tc.cache.Cleanup()
}
} }
// Close stops the cleanup goroutine and releases resources. // Close stops the cleanup goroutine and releases resources.
+4 -3
View File
@@ -40,7 +40,7 @@ type HTTPClientConfig struct {
// DefaultHTTPClientConfig returns the default configuration for general use // DefaultHTTPClientConfig returns the default configuration for general use
func DefaultHTTPClientConfig() HTTPClientConfig { func DefaultHTTPClientConfig() HTTPClientConfig {
return HTTPClientConfig{ return HTTPClientConfig{
Timeout: 5 * time.Second, Timeout: 30 * time.Second,
MaxRedirects: 10, MaxRedirects: 10,
UseCookieJar: false, UseCookieJar: false,
DialTimeout: 5 * time.Second, DialTimeout: 5 * time.Second,
@@ -63,8 +63,9 @@ func DefaultHTTPClientConfig() HTTPClientConfig {
// TokenHTTPClientConfig returns configuration optimized for token operations // TokenHTTPClientConfig returns configuration optimized for token operations
func TokenHTTPClientConfig() HTTPClientConfig { func TokenHTTPClientConfig() HTTPClientConfig {
config := DefaultHTTPClientConfig() config := DefaultHTTPClientConfig()
config.MaxRedirects = 50 // Token endpoints may redirect more config.Timeout = 10 * time.Second // Shorter timeout for token operations
config.UseCookieJar = true // Enable cookie jar for token operations config.MaxRedirects = 50 // Token endpoints may redirect more
config.UseCookieJar = true // Enable cookie jar for token operations
return config return config
} }
+1 -1
View File
@@ -133,7 +133,7 @@ func (c *JWKCache) GetJWKS(ctx context.Context, jwksURL string, httpClient *http
// Cleanup removes expired entries from the JWK cache to free memory. // Cleanup removes expired entries from the JWK cache to free memory.
// It delegates to the internal cache's cleanup method. // It delegates to the internal cache's cleanup method.
func (c *JWKCache) Cleanup() { func (c *JWKCache) Cleanup() {
if c.internalCache != nil { if c != nil && c.internalCache != nil {
c.internalCache.Cleanup() c.internalCache.Cleanup()
} }
} }
+1 -1
View File
@@ -78,7 +78,7 @@ func getReplayCacheStats() (size int, maxSize int) {
defer replayCacheMu.RUnlock() defer replayCacheMu.RUnlock()
if replayCache == nil { if replayCache == nil {
return 0, 0 return 0, 10000
} }
return 0, 10000 return 0, 10000
+65 -28
View File
@@ -751,16 +751,7 @@ func (t *TraefikOidc) updateMetadataEndpoints(metadata *ProviderMetadata) {
func (t *TraefikOidc) startMetadataRefresh(providerURL string) { func (t *TraefikOidc) startMetadataRefresh(providerURL string) {
ticker := time.NewTicker(2 * time.Hour) ticker := time.NewTicker(2 * time.Hour)
if t.goroutineWG != nil {
t.goroutineWG.Add(1)
}
go func() { go func() {
defer func() {
if t.goroutineWG != nil {
t.goroutineWG.Done()
}
}()
defer ticker.Stop() defer ticker.Stop()
consecutiveFailures := 0 consecutiveFailures := 0
@@ -925,7 +916,17 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
if !t.metadataRefreshStarted && t.providerURL != "" { if !t.metadataRefreshStarted && t.providerURL != "" {
t.metadataRefreshStarted = true t.metadataRefreshStarted = true
go t.startMetadataRefresh(t.providerURL) if t.goroutineWG != nil {
t.goroutineWG.Add(1)
}
go func() {
defer func() {
if t.goroutineWG != nil {
t.goroutineWG.Done()
}
}()
t.startMetadataRefresh(t.providerURL)
}()
} }
} }
t.firstRequestMutex.Unlock() t.firstRequestMutex.Unlock()
@@ -1801,44 +1802,80 @@ func (t *TraefikOidc) validateHost(host string) error {
// It runs periodic cleanup of token cache, JWK cache, and session chunks. // It runs periodic cleanup of token cache, JWK cache, and session chunks.
// Includes panic recovery to ensure stability. // Includes panic recovery to ensure stability.
func (t *TraefikOidc) startTokenCleanup() { func (t *TraefikOidc) startTokenCleanup() {
if t == nil {
return
}
// Capture values to avoid race conditions
tokenCache := t.tokenCache
jwkCache := t.jwkCache
sessionManager := t.sessionManager
logger := t.logger
goroutineWG := t.goroutineWG
stopChan := t.tokenCleanupStopChan
ctx := t.ctx
ticker := time.NewTicker(1 * time.Minute) ticker := time.NewTicker(1 * time.Minute)
if t.goroutineWG != nil { if goroutineWG != nil {
t.goroutineWG.Add(1) goroutineWG.Add(1)
} }
go func() { go func() {
defer func() { defer func() {
if t.goroutineWG != nil { if goroutineWG != nil {
t.goroutineWG.Done() goroutineWG.Done()
} }
ticker.Stop() ticker.Stop()
if r := recover(); r != nil { if r := recover(); r != nil {
t.logger.Errorf("Token cleanup goroutine panic recovered: %v", r) if logger != nil {
logger.Errorf("Token cleanup goroutine panic recovered: %v", r)
}
} }
}() }()
for { for {
select { select {
case <-ticker.C: case <-ticker.C:
t.logger.Debug("Starting token cleanup cycle") if logger != nil {
if t.tokenCache != nil { logger.Debug("Starting token cleanup cycle")
t.tokenCache.Cleanup()
} }
if t.jwkCache != nil { if tokenCache != nil {
t.jwkCache.Cleanup() tokenCache.Cleanup()
} }
if t.sessionManager != nil { if jwkCache != nil {
t.sessionManager.PeriodicChunkCleanup() jwkCache.Cleanup()
}
t.logger.Debug("Running session health monitoring") if sessionManager != nil {
sessionManager.PeriodicChunkCleanup()
if logger != nil {
logger.Debug("Running session health monitoring")
}
} }
case <-t.tokenCleanupStopChan: case <-func() <-chan struct{} {
t.logger.Debug("Token cleanup goroutine stopped.") if stopChan != nil {
return stopChan
}
// Return a channel that never receives anything
c := make(chan struct{})
return c
}():
if logger != nil {
logger.Debug("Token cleanup goroutine stopped.")
}
return return
case <-t.ctx.Done(): case <-func() <-chan struct{} {
t.logger.Debug("Token cleanup goroutine stopped due to context cancellation.") if ctx != nil {
return ctx.Done()
}
// Return a channel that never receives anything
c := make(chan struct{})
return c
}():
if logger != nil {
logger.Debug("Token cleanup goroutine stopped due to context cancellation.")
}
return return
} }
} }
+11 -9
View File
@@ -275,21 +275,27 @@ func NewSessionManager(encryptionKey string, forceHTTPS bool, cookieDomain strin
// It cleans up orphaned token chunks, expired sessions, and unused pool objects. // It cleans up orphaned token chunks, expired sessions, and unused pool objects.
// This helps maintain performance and prevent cookie accumulation in client browsers. // This helps maintain performance and prevent cookie accumulation in client browsers.
func (sm *SessionManager) PeriodicChunkCleanup() { func (sm *SessionManager) PeriodicChunkCleanup() {
if sm == nil || sm.logger == nil {
return
}
sm.logger.Debug("Starting comprehensive session cleanup cycle") sm.logger.Debug("Starting comprehensive session cleanup cycle")
cleanupStart := time.Now() cleanupStart := time.Now()
var orphanedChunks, expiredSessions, cleanupErrors int var orphanedChunks, expiredSessions, cleanupErrors int
if cookieStore, ok := sm.store.(*sessions.CookieStore); ok { if sm.store != nil {
sm.logger.Debug("Running session store cleanup") if cookieStore, ok := sm.store.(*sessions.CookieStore); ok {
_ = cookieStore sm.logger.Debug("Running session store cleanup")
_ = cookieStore
}
} }
poolCleaned := 0 poolCleaned := 0
for i := 0; i < 10; i++ { for i := 0; i < 10; i++ {
if poolSession := sm.sessionPool.Get(); poolSession != nil { if poolSession := sm.sessionPool.Get(); poolSession != nil {
sessionData := poolSession.(*SessionData) sessionData, ok := poolSession.(*SessionData)
if sessionData != nil && !sessionData.inUse { if ok && sessionData != nil && !sessionData.inUse {
sessionData.Reset() sessionData.Reset()
poolCleaned++ poolCleaned++
} }
@@ -861,10 +867,6 @@ func (sd *SessionData) clearAllSessionData(r *http.Request, expire bool) {
// - An error if session saving fails during cleanup. // - An error if session saving fails during cleanup.
func (sd *SessionData) Clear(r *http.Request, w http.ResponseWriter) error { func (sd *SessionData) Clear(r *http.Request, w http.ResponseWriter) error {
defer func() { defer func() {
if rec := recover(); rec != nil {
sd.returnToPoolSafely()
panic(rec)
}
sd.returnToPoolSafely() sd.returnToPoolSafely()
}() }()