diff --git a/internal/providers/registry.go b/internal/providers/registry.go index 2a0abfd..bfd5496 100644 --- a/internal/providers/registry.go +++ b/internal/providers/registry.go @@ -64,15 +64,37 @@ func (r *ProviderRegistry) ClearCache() { // DetectProvider determines the most appropriate provider for a given issuer URL. // It iterates through the registered providers and returns the first one that matches. // Detection is based on URL patterns and other provider-specific criteria. +// Uses double-checked locking pattern to avoid race conditions while caching results. func (r *ProviderRegistry) DetectProvider(issuerURL string) OIDCProvider { + // First check: read lock for cache lookup r.mu.RLock() - defer r.mu.RUnlock() + if provider, found := r.cache[issuerURL]; found { + r.mu.RUnlock() + return provider + } + r.mu.RUnlock() - // Check cache first for performance + // Cache miss - acquire write lock for detection and caching + r.mu.Lock() + defer r.mu.Unlock() + + // Second check: another goroutine might have cached the result while we waited for write lock if provider, found := r.cache[issuerURL]; found { return provider } + // Perform detection under write lock + detectedProvider := r.detectProviderUnsafe(issuerURL) + + // Cache the result (even if nil to avoid repeated expensive operations) + r.cache[issuerURL] = detectedProvider + + return detectedProvider +} + +// detectProviderUnsafe performs the actual provider detection logic. +// This method assumes the caller holds the appropriate lock and should not be called directly. +func (r *ProviderRegistry) detectProviderUnsafe(issuerURL string) OIDCProvider { // Normalize issuer URL for consistent matching normalizedURL, err := url.Parse(issuerURL) if err != nil { @@ -86,12 +108,10 @@ func (r *ProviderRegistry) DetectProvider(issuerURL string) OIDCProvider { switch p.GetType() { case ProviderTypeGoogle: if strings.Contains(host, "accounts.google.com") { - r.cache[issuerURL] = p return p } case ProviderTypeAzure: if strings.Contains(host, "login.microsoftonline.com") || strings.Contains(host, "sts.windows.net") { - r.cache[issuerURL] = p return p } } @@ -100,7 +120,6 @@ func (r *ProviderRegistry) DetectProvider(issuerURL string) OIDCProvider { // Fallback to the generic provider if no specific provider is detected for _, p := range r.providers { if p.GetType() == ProviderTypeGeneric { - r.cache[issuerURL] = p return p } } diff --git a/session.go b/session.go index 5019c5e..39c29dd 100644 --- a/session.go +++ b/session.go @@ -245,7 +245,8 @@ type SessionManager struct { forceHTTPS bool cookieDomain string chunkManager *ChunkManager - cleanupDone bool // Track if we've attempted cookie cleanup + cleanupMutex sync.RWMutex // Protects cleanup operations + cleanupDone bool // Track if we've attempted cookie cleanup } // NewSessionManager creates a new session manager with the specified configuration. @@ -628,7 +629,11 @@ func (sm *SessionManager) CleanupOldCookies(w http.ResponseWriter, r *http.Reque // If we have a configured domain and this is the first request after config change, // attempt to delete the cookie with various domain variations to ensure cleanup - if currentDomain != "" && !sm.cleanupDone { + sm.cleanupMutex.RLock() + shouldCleanup := currentDomain != "" && !sm.cleanupDone + sm.cleanupMutex.RUnlock() + + if shouldCleanup { for _, domain := range domainsToClean { // Skip the current configured domain if domain == currentDomain || domain == "."+currentDomain || "."+domain == currentDomain { @@ -655,7 +660,9 @@ func (sm *SessionManager) CleanupOldCookies(w http.ResponseWriter, r *http.Reque // Mark cleanup as done for this session manager instance if !sm.cleanupDone && len(processedCookies) > 0 { + sm.cleanupMutex.Lock() sm.cleanupDone = true + sm.cleanupMutex.Unlock() } } diff --git a/test_utils_test.go b/test_utils_test.go index a7e5428..84d6ae2 100644 --- a/test_utils_test.go +++ b/test_utils_test.go @@ -14,4 +14,4 @@ func generateRandomString(length int) string { return "random-string-fallback" } return hex.EncodeToString(bytes) -} \ No newline at end of file +}