mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-11-04 07:53:59 +00:00
Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7db2f8d66c | ||
|
|
024e349aa9 | ||
|
|
873105108e | ||
|
|
5241cfe792 | ||
|
|
e2e2be53c1 | ||
|
|
c878784f1e | ||
|
|
1fd762848a |
@@ -77,6 +77,7 @@ testData:
|
||||
revocationURL: https://accounts.google.com/revoke # Endpoint for revoking tokens
|
||||
oidcEndSessionURL: https://accounts.google.com/logout # Provider's end session endpoint
|
||||
enablePKCE: false # Enables PKCE (Proof Key for Code Exchange) for additional security
|
||||
cookieDomain: "" # Explicit domain for session cookies (e.g., ".example.com" for multi-subdomain setups)
|
||||
|
||||
# --- Provider Specific Configuration Examples ---
|
||||
#
|
||||
@@ -373,6 +374,31 @@ configuration:
|
||||
Default: false
|
||||
required: false
|
||||
|
||||
cookieDomain:
|
||||
type: string
|
||||
description: |
|
||||
Explicit domain for session cookies. This is important for multi-subdomain setups
|
||||
and reverse proxy deployments to ensure consistent cookie handling.
|
||||
|
||||
When set, all session cookies will use this domain. When not set, the domain
|
||||
is auto-detected from the request headers (X-Forwarded-Host or Host).
|
||||
|
||||
Use a leading dot for subdomain-wide cookies (e.g., ".example.com" allows
|
||||
cookies to be shared between app.example.com, api.example.com, etc.).
|
||||
|
||||
Use a specific domain for host-only cookies (e.g., "app.example.com" restricts
|
||||
cookies to that exact domain).
|
||||
|
||||
This setting is crucial to prevent authentication issues like "CSRF token missing
|
||||
in session" errors that can occur when cookies are created with inconsistent domains.
|
||||
|
||||
Examples:
|
||||
- ".example.com" - Allows all subdomains to share cookies
|
||||
- "app.example.com" - Restricts cookies to this specific host
|
||||
|
||||
Default: "" (auto-detected from request headers)
|
||||
required: false
|
||||
|
||||
headers:
|
||||
type: array
|
||||
description: |
|
||||
|
||||
@@ -82,6 +82,7 @@ The middleware supports the following configuration options:
|
||||
| `oidcEndSessionURL` | The provider's end session endpoint | auto-discovered | `https://accounts.google.com/logout` |
|
||||
| `enablePKCE` | Enables PKCE (Proof Key for Code Exchange) for authorization code flow | `false` | `true`, `false` |
|
||||
| `refreshGracePeriodSeconds` | Seconds before token expiry to attempt proactive refresh | `60` | `120` |
|
||||
| `cookieDomain` | Explicit domain for session cookies (important for multi-subdomain setups) | auto-detected | `.example.com`, `app.example.com` |
|
||||
| `headers` | Custom HTTP headers with templates that can access OIDC claims and tokens | none | See "Templated Headers" section |
|
||||
|
||||
## Scope Configuration
|
||||
@@ -311,6 +312,30 @@ spec:
|
||||
- developer
|
||||
```
|
||||
|
||||
### With Cookie Domain Configuration (Multi-Subdomain Setup)
|
||||
|
||||
```yaml
|
||||
apiVersion: traefik.io/v1alpha1
|
||||
kind: Middleware
|
||||
metadata:
|
||||
name: oidc-multi-subdomain
|
||||
namespace: traefik
|
||||
spec:
|
||||
plugin:
|
||||
traefikoidc:
|
||||
providerURL: https://accounts.google.com
|
||||
clientID: 1234567890.apps.googleusercontent.com
|
||||
clientSecret: your-client-secret
|
||||
sessionEncryptionKey: potato-secret-is-at-least-32-bytes-long
|
||||
callbackURL: /oauth2/callback
|
||||
logoutURL: /oauth2/logout
|
||||
cookieDomain: .example.com # Allows cookies to be shared across all subdomains
|
||||
scopes:
|
||||
- roles # Appended to defaults: ["openid", "profile", "email", "roles"]
|
||||
```
|
||||
|
||||
**Important**: The `cookieDomain` parameter is crucial when running behind a reverse proxy or when your application serves multiple subdomains. Without it, cookies may be created with inconsistent domains, leading to authentication issues like "CSRF token missing in session" errors.
|
||||
|
||||
### With Custom Logging and Rate Limiting
|
||||
|
||||
```yaml
|
||||
|
||||
+24
-2
@@ -1,6 +1,9 @@
|
||||
package traefikoidc
|
||||
|
||||
import "time"
|
||||
import (
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
// BackgroundTask represents a managed recurring task that runs in the background.
|
||||
// It provides a clean interface for starting and stopping periodic operations
|
||||
@@ -11,6 +14,7 @@ type BackgroundTask struct {
|
||||
logger *Logger
|
||||
name string
|
||||
interval time.Duration
|
||||
wg *sync.WaitGroup
|
||||
}
|
||||
|
||||
// NewBackgroundTask creates a new background task with the specified parameters.
|
||||
@@ -20,35 +24,53 @@ type BackgroundTask struct {
|
||||
// - interval: Duration between task executions.
|
||||
// - taskFunc: The function to execute periodically.
|
||||
// - logger: Logger instance for task lifecycle events.
|
||||
// - wg: Optional WaitGroup for synchronizing goroutine completion.
|
||||
//
|
||||
// Returns:
|
||||
// - A configured BackgroundTask ready to be started.
|
||||
func NewBackgroundTask(name string, interval time.Duration, taskFunc func(), logger *Logger) *BackgroundTask {
|
||||
func NewBackgroundTask(name string, interval time.Duration, taskFunc func(), logger *Logger, wg ...*sync.WaitGroup) *BackgroundTask {
|
||||
var waitGroup *sync.WaitGroup
|
||||
if len(wg) > 0 {
|
||||
waitGroup = wg[0]
|
||||
}
|
||||
return &BackgroundTask{
|
||||
name: name,
|
||||
interval: interval,
|
||||
stopChan: make(chan struct{}),
|
||||
taskFunc: taskFunc,
|
||||
logger: logger,
|
||||
wg: waitGroup,
|
||||
}
|
||||
}
|
||||
|
||||
// Start begins the background task execution in a separate goroutine.
|
||||
// The task runs immediately upon start and then at the specified interval.
|
||||
func (bt *BackgroundTask) Start() {
|
||||
if bt.wg != nil {
|
||||
bt.wg.Add(1)
|
||||
}
|
||||
go bt.run()
|
||||
}
|
||||
|
||||
// Stop gracefully terminates the background task by closing the stop channel.
|
||||
// If a WaitGroup was provided, it waits for the goroutine to complete.
|
||||
// This method is safe to call multiple times.
|
||||
func (bt *BackgroundTask) Stop() {
|
||||
close(bt.stopChan)
|
||||
if bt.wg != nil {
|
||||
bt.wg.Wait()
|
||||
}
|
||||
}
|
||||
|
||||
// run is the main execution loop for the background task.
|
||||
// It executes the task function immediately and then at regular intervals
|
||||
// until the stop signal is received.
|
||||
func (bt *BackgroundTask) run() {
|
||||
defer func() {
|
||||
if bt.wg != nil {
|
||||
bt.wg.Done()
|
||||
}
|
||||
}()
|
||||
ticker := time.NewTicker(bt.interval)
|
||||
defer ticker.Stop()
|
||||
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestAzureOAuthCallbackScenario tests the exact scenario from issue #53
|
||||
// This test ensures that cookies set during OAuth initiation are available
|
||||
// during the callback from Azure AD
|
||||
func TestAzureOAuthCallbackScenario(t *testing.T) {
|
||||
t.Run("Azure_OAuth_Complete_Flow", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Step 1: User visits https://app.example.com/protected
|
||||
// Traefik receives this as http://internal/protected with headers
|
||||
initReq := httptest.NewRequest("GET", "http://internal/protected", nil)
|
||||
initReq.Header.Set("X-Forwarded-Proto", "https")
|
||||
initReq.Header.Set("X-Forwarded-Host", "app.example.com")
|
||||
initReq.Header.Set("User-Agent", "Mozilla/5.0 (Windows NT 10.0; Win64; x64)")
|
||||
initReq.Host = "internal" // The actual host Traefik sees
|
||||
|
||||
// Get session and prepare for OAuth
|
||||
session, err := sessionManager.GetSession(initReq)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set OAuth flow data
|
||||
csrfToken := "azure-csrf-state-token"
|
||||
nonce := "azure-nonce-value"
|
||||
codeVerifier := "pkce-code-verifier"
|
||||
|
||||
session.SetCSRF(csrfToken)
|
||||
session.SetNonce(nonce)
|
||||
session.SetCodeVerifier(codeVerifier)
|
||||
session.SetIncomingPath("/protected")
|
||||
session.MarkDirty()
|
||||
|
||||
// Save session
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(initReq, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Examine the cookies that would be sent to the browser
|
||||
cookies := rec.Result().Cookies()
|
||||
require.NotEmpty(t, cookies, "Cookies must be set for OAuth flow")
|
||||
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie, "Main session cookie must be set")
|
||||
|
||||
// Verify cookie attributes for Azure OAuth
|
||||
assert.True(t, mainCookie.Secure, "Cookie MUST be Secure for HTTPS")
|
||||
assert.Equal(t, http.SameSiteLaxMode, mainCookie.SameSite,
|
||||
"MUST be Lax to allow Azure callback from different domain")
|
||||
assert.Equal(t, "app.example.com", mainCookie.Domain,
|
||||
"Domain must match X-Forwarded-Host for browser to send it back")
|
||||
assert.Equal(t, "/", mainCookie.Path, "Path must be root")
|
||||
assert.True(t, mainCookie.HttpOnly, "HttpOnly for security")
|
||||
|
||||
// Step 2: User is redirected to Azure AD login
|
||||
// Azure AD redirects back to https://app.example.com/oidc/callback?code=xxx&state=xxx
|
||||
// Traefik receives this as http://internal/oidc/callback with headers
|
||||
|
||||
callbackReq := httptest.NewRequest("GET",
|
||||
"http://internal/oidc/callback?code=AzureAuthCode&state="+csrfToken, nil)
|
||||
callbackReq.Header.Set("X-Forwarded-Proto", "https")
|
||||
callbackReq.Header.Set("X-Forwarded-Host", "app.example.com")
|
||||
callbackReq.Header.Set("User-Agent", "Mozilla/5.0 (Windows NT 10.0; Win64; x64)")
|
||||
callbackReq.Host = "internal"
|
||||
|
||||
// Browser sends cookies because:
|
||||
// 1. Request is to https://app.example.com (matches cookie domain)
|
||||
// 2. Cookie has Secure flag and request is HTTPS
|
||||
// 3. Cookie has SameSite=Lax which allows top-level navigation from Azure
|
||||
for _, cookie := range cookies {
|
||||
callbackReq.AddCookie(cookie)
|
||||
}
|
||||
|
||||
// Get session in callback
|
||||
callbackSession, err := sessionManager.GetSession(callbackReq)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify session data is available - THIS WAS FAILING IN ISSUE #53
|
||||
retrievedCSRF := callbackSession.GetCSRF()
|
||||
assert.Equal(t, csrfToken, retrievedCSRF,
|
||||
"CSRF token MUST be available in callback (was missing in issue #53)")
|
||||
|
||||
retrievedNonce := callbackSession.GetNonce()
|
||||
assert.Equal(t, nonce, retrievedNonce,
|
||||
"Nonce MUST be available for security validation")
|
||||
|
||||
retrievedCodeVerifier := callbackSession.GetCodeVerifier()
|
||||
assert.Equal(t, codeVerifier, retrievedCodeVerifier,
|
||||
"PKCE verifier MUST be available for token exchange")
|
||||
|
||||
retrievedPath := callbackSession.GetIncomingPath()
|
||||
assert.Equal(t, "/protected", retrievedPath,
|
||||
"Original path MUST be available for post-auth redirect")
|
||||
})
|
||||
|
||||
t.Run("Cookie_Not_Sent_With_Wrong_Domain", func(t *testing.T) {
|
||||
// This test verifies that cookies with wrong domain won't be sent
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Initial request sets cookie for app.example.com
|
||||
initReq := httptest.NewRequest("GET", "http://internal/protected", nil)
|
||||
initReq.Header.Set("X-Forwarded-Proto", "https")
|
||||
initReq.Header.Set("X-Forwarded-Host", "app.example.com")
|
||||
initReq.Header.Set("User-Agent", "Mozilla/5.0")
|
||||
|
||||
session, err := sessionManager.GetSession(initReq)
|
||||
require.NoError(t, err)
|
||||
session.SetCSRF("test-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(initReq, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Callback comes to different domain
|
||||
callbackReq := httptest.NewRequest("GET", "http://internal/oidc/callback", nil)
|
||||
callbackReq.Header.Set("X-Forwarded-Proto", "https")
|
||||
callbackReq.Header.Set("X-Forwarded-Host", "different.example.com") // Different domain!
|
||||
callbackReq.Header.Set("User-Agent", "Mozilla/5.0")
|
||||
|
||||
// Browser wouldn't send cookies because domain doesn't match
|
||||
// So we simulate that by not adding cookies
|
||||
|
||||
callbackSession, err := sessionManager.GetSession(callbackReq)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Session should be empty
|
||||
assert.Empty(t, callbackSession.GetCSRF(),
|
||||
"CSRF should be empty when cookies aren't sent due to domain mismatch")
|
||||
})
|
||||
|
||||
t.Run("SameSite_Strict_Would_Break_OAuth", func(t *testing.T) {
|
||||
// This test demonstrates why we can't use SameSite=Strict for OAuth
|
||||
// With Strict, cookies wouldn't be sent when redirecting from Azure to our app
|
||||
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// If we had SameSite=Strict (which we don't anymore), the browser would:
|
||||
// 1. Set cookie when user visits app.example.com
|
||||
// 2. NOT send cookie when Azure redirects back to app.example.com/callback
|
||||
// This is because the request originates from login.microsoftonline.com
|
||||
|
||||
// Our fix ensures we use SameSite=Lax which allows top-level navigation
|
||||
req := httptest.NewRequest("GET", "http://internal/test", nil)
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
req.Header.Set("X-Forwarded-Host", "app.example.com")
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
session.SetCSRF("test")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies := rec.Result().Cookies()
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
assert.Equal(t, http.SameSiteLaxMode, cookie.SameSite,
|
||||
"Must use Lax, not Strict, for OAuth to work")
|
||||
break
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
+1
-1
@@ -71,7 +71,7 @@ func TestAzureOIDCRegression(t *testing.T) {
|
||||
tOidc := &mockTraefikOidc{TraefikOidc: baseOidc}
|
||||
|
||||
// Initialize session manager
|
||||
sessionManager, _ := NewSessionManager("test-encryption-key-32-bytes-long", false, mockLogger)
|
||||
sessionManager, _ := NewSessionManager("test-encryption-key-32-bytes-long", false, "", mockLogger)
|
||||
tOidc.sessionManager = sessionManager
|
||||
|
||||
// Mock the JWT verification to avoid JWKS lookup issues
|
||||
|
||||
@@ -0,0 +1,218 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestCookieCleanupBehavior tests the cookie cleanup function's behavior
|
||||
func TestCookieCleanupBehavior(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
|
||||
// Create session manager with a specific domain
|
||||
sm, err := NewSessionManager("test-encryption-key-32-characters", false, "app.example.com", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
// Create request from app.example.com
|
||||
req := httptest.NewRequest("GET", "http://app.example.com/test", nil)
|
||||
|
||||
// Add some OIDC cookies (browsers don't send domain info)
|
||||
cookies := []*http.Cookie{
|
||||
{
|
||||
Name: "_oidc_raczylo_m",
|
||||
Value: "test-value-1",
|
||||
},
|
||||
{
|
||||
Name: "_oidc_raczylo_a",
|
||||
Value: "test-value-2",
|
||||
},
|
||||
{
|
||||
Name: "access_token_chunk_0",
|
||||
Value: "chunk-value",
|
||||
},
|
||||
}
|
||||
|
||||
for _, cookie := range cookies {
|
||||
req.AddCookie(cookie)
|
||||
}
|
||||
|
||||
// Create response recorder to capture Set-Cookie headers
|
||||
rr := httptest.NewRecorder()
|
||||
|
||||
// Run cleanup - it should attempt to delete cookies with various domains
|
||||
sm.CleanupOldCookies(rr, req)
|
||||
|
||||
// Check Set-Cookie headers
|
||||
setCookies := rr.Result().Cookies()
|
||||
|
||||
// The cleanup should have attempted to delete cookies with various domain variations
|
||||
// We should see deletion attempts for:
|
||||
// - app.example.com
|
||||
// - .app.example.com
|
||||
// - example.com
|
||||
// - .example.com
|
||||
var deletionAttempts int
|
||||
domainsAttempted := make(map[string]bool)
|
||||
|
||||
for _, cookie := range setCookies {
|
||||
if cookie.MaxAge == -1 {
|
||||
deletionAttempts++
|
||||
domainsAttempted[cookie.Domain] = true
|
||||
}
|
||||
}
|
||||
|
||||
// We should see deletion attempts for various domain variations
|
||||
// Note: The exact number depends on the implementation, but we should see multiple attempts
|
||||
if deletionAttempts == 0 {
|
||||
t.Error("Expected cleanup to attempt cookie deletions, but none were found")
|
||||
}
|
||||
|
||||
// Log the domains attempted for debugging
|
||||
t.Logf("Deletion attempts: %d", deletionAttempts)
|
||||
for domain := range domainsAttempted {
|
||||
t.Logf("Attempted deletion for domain: %q", domain)
|
||||
}
|
||||
}
|
||||
|
||||
// TestConfiguredDomainPersistence tests that configured domain is consistently used
|
||||
func TestConfiguredDomainPersistence(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
|
||||
// Create session manager with explicit domain configuration
|
||||
configuredDomain := ".example.com"
|
||||
sm, err := NewSessionManager("test-encryption-key-32-characters", false, configuredDomain, logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
// Create requests from different subdomains
|
||||
tests := []struct {
|
||||
requestHost string
|
||||
name string
|
||||
}{
|
||||
{
|
||||
name: "Request from main domain",
|
||||
requestHost: "example.com",
|
||||
},
|
||||
{
|
||||
name: "Request from subdomain",
|
||||
requestHost: "app.example.com",
|
||||
},
|
||||
{
|
||||
name: "Request from nested subdomain",
|
||||
requestHost: "api.app.example.com",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "http://"+tt.requestHost+"/test", nil)
|
||||
rr := httptest.NewRecorder()
|
||||
|
||||
// Get session and set a value
|
||||
session, err := sm.GetSession(req)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to get session: %v", err)
|
||||
}
|
||||
defer session.ReturnToPool()
|
||||
|
||||
session.SetNonce("test-nonce")
|
||||
|
||||
// Save session
|
||||
err = session.Save(req, rr)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to save session: %v", err)
|
||||
}
|
||||
|
||||
// Check that all cookies use the configured domain
|
||||
cookies := rr.Result().Cookies()
|
||||
for _, cookie := range cookies {
|
||||
if strings.HasPrefix(cookie.Name, "_oidc_") {
|
||||
// The domain should match the configured domain (with possible normalization)
|
||||
expectedDomains := []string{configuredDomain, strings.TrimPrefix(configuredDomain, ".")}
|
||||
domainMatches := false
|
||||
for _, expected := range expectedDomains {
|
||||
if cookie.Domain == expected || cookie.Domain == "" {
|
||||
domainMatches = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !domainMatches {
|
||||
t.Errorf("Cookie %s has unexpected domain %q, expected one of %v",
|
||||
cookie.Name, cookie.Domain, expectedDomains)
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestDomainMigration simulates migrating from no configured domain to explicit domain
|
||||
func TestDomainMigration(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
|
||||
// Step 1: Create session without configured domain (auto-detection)
|
||||
sm1, err := NewSessionManager("test-encryption-key-32-characters", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
req1 := httptest.NewRequest("GET", "http://app.example.com/test", nil)
|
||||
rr1 := httptest.NewRecorder()
|
||||
|
||||
session1, err := sm1.GetSession(req1)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to get session: %v", err)
|
||||
}
|
||||
|
||||
session1.SetNonce("test-nonce")
|
||||
session1.Save(req1, rr1)
|
||||
session1.ReturnToPool()
|
||||
|
||||
// The cookies will have auto-detected domain
|
||||
oldCookies := rr1.Result().Cookies()
|
||||
t.Logf("Old cookies count: %d", len(oldCookies))
|
||||
|
||||
// Step 2: Create new session manager with explicit domain configuration
|
||||
sm2, err := NewSessionManager("test-encryption-key-32-characters", false, ".example.com", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
// Step 3: Make request with old cookies
|
||||
req2 := httptest.NewRequest("GET", "http://app.example.com/test", nil)
|
||||
|
||||
// Add the old cookies to the new request
|
||||
for _, cookie := range oldCookies {
|
||||
// Simulate browser behavior - don't include domain in Cookie header
|
||||
simpleCookie := &http.Cookie{
|
||||
Name: cookie.Name,
|
||||
Value: cookie.Value,
|
||||
}
|
||||
req2.AddCookie(simpleCookie)
|
||||
}
|
||||
|
||||
rr2 := httptest.NewRecorder()
|
||||
|
||||
// Run cleanup - should attempt to delete old cookies
|
||||
sm2.CleanupOldCookies(rr2, req2)
|
||||
|
||||
// Check that deletion cookies were sent
|
||||
newCookies := rr2.Result().Cookies()
|
||||
deletionCount := 0
|
||||
for _, cookie := range newCookies {
|
||||
if cookie.MaxAge == -1 {
|
||||
deletionCount++
|
||||
}
|
||||
}
|
||||
|
||||
if deletionCount == 0 {
|
||||
t.Error("Expected cleanup to send deletion cookies during migration, but none were found")
|
||||
}
|
||||
|
||||
t.Logf("Sent %d deletion cookies during migration", deletionCount)
|
||||
}
|
||||
@@ -0,0 +1,205 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestCookieDomainConfiguration tests that the cookie domain configuration is properly applied
|
||||
func TestCookieDomainConfiguration(t *testing.T) {
|
||||
tests := []struct {
|
||||
configDomain string
|
||||
requestHost string
|
||||
forwardedHost string
|
||||
expectedDomain string
|
||||
name string
|
||||
}{
|
||||
{
|
||||
name: "Configured domain takes precedence",
|
||||
configDomain: ".example.com",
|
||||
requestHost: "app.example.com",
|
||||
expectedDomain: ".example.com",
|
||||
},
|
||||
{
|
||||
name: "Auto-detection when no domain configured",
|
||||
configDomain: "",
|
||||
requestHost: "app.example.com",
|
||||
expectedDomain: "app.example.com",
|
||||
},
|
||||
{
|
||||
name: "X-Forwarded-Host used for auto-detection",
|
||||
configDomain: "",
|
||||
requestHost: "internal.local",
|
||||
forwardedHost: "public.example.com",
|
||||
expectedDomain: "public.example.com",
|
||||
},
|
||||
{
|
||||
name: "No domain for localhost",
|
||||
configDomain: "",
|
||||
requestHost: "localhost:8080",
|
||||
expectedDomain: "",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Create session manager with configured domain
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("test-encryption-key-32-characters", false, tt.configDomain, logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
// Create request
|
||||
req := httptest.NewRequest("GET", "http://"+tt.requestHost+"/test", nil)
|
||||
if tt.forwardedHost != "" {
|
||||
req.Header.Set("X-Forwarded-Host", tt.forwardedHost)
|
||||
}
|
||||
|
||||
// Create a dummy response writer to test getCookieOptions behavior
|
||||
// We'll examine the session options domain instead
|
||||
options := &http.Cookie{
|
||||
Domain: sm.cookieDomain,
|
||||
}
|
||||
// If no configured domain, simulate auto-detection
|
||||
if sm.cookieDomain == "" && req != nil {
|
||||
host := req.Host
|
||||
if forwardedHost := req.Header.Get("X-Forwarded-Host"); forwardedHost != "" {
|
||||
host = forwardedHost
|
||||
}
|
||||
if host != "" && !strings.Contains(host, "localhost") && !strings.Contains(host, "127.0.0.1") {
|
||||
if colonIndex := strings.Index(host, ":"); colonIndex != -1 {
|
||||
host = host[:colonIndex]
|
||||
}
|
||||
options.Domain = host
|
||||
}
|
||||
}
|
||||
|
||||
// Check domain
|
||||
if options.Domain != tt.expectedDomain {
|
||||
t.Errorf("Expected domain %q, got %q", tt.expectedDomain, options.Domain)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestCookieDomainConsistency tests that all session cookies use the same domain
|
||||
func TestCookieDomainConsistency(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
|
||||
// Test with configured domain
|
||||
sm, err := NewSessionManager("test-encryption-key-32-characters", false, ".example.com", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
req := httptest.NewRequest("GET", "http://app.example.com/test", nil)
|
||||
rr := httptest.NewRecorder()
|
||||
|
||||
// Get session and set some values
|
||||
session, err := sm.GetSession(req)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to get session: %v", err)
|
||||
}
|
||||
defer session.ReturnToPool()
|
||||
|
||||
// Set various session values including ID token
|
||||
session.SetNonce("test-nonce")
|
||||
session.SetAccessToken("test-access-token")
|
||||
session.SetRefreshToken("test-refresh-token")
|
||||
// Set a valid JWT-like ID token (needs 2 dots)
|
||||
session.SetIDToken("header.payload.signature")
|
||||
|
||||
// Save session
|
||||
err = session.Save(req, rr)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to save session: %v", err)
|
||||
}
|
||||
|
||||
// Check all cookies have the same domain
|
||||
cookies := rr.Result().Cookies()
|
||||
var seenDomain string
|
||||
|
||||
for _, cookie := range cookies {
|
||||
// Only check our OIDC cookies
|
||||
if strings.HasPrefix(cookie.Name, "_oidc_") ||
|
||||
strings.HasPrefix(cookie.Name, "access_token_chunk_") ||
|
||||
strings.HasPrefix(cookie.Name, "refresh_token_chunk_") {
|
||||
|
||||
// Normalize domain for comparison (handle leading dot and empty domain)
|
||||
normalizedDomain := cookie.Domain
|
||||
if normalizedDomain == "" {
|
||||
// Empty domain means host-only cookie, should match configured domain
|
||||
normalizedDomain = "example.com"
|
||||
} else if strings.HasPrefix(normalizedDomain, ".") {
|
||||
normalizedDomain = normalizedDomain[1:]
|
||||
}
|
||||
|
||||
if seenDomain == "" {
|
||||
seenDomain = normalizedDomain
|
||||
} else if normalizedDomain != seenDomain {
|
||||
t.Errorf("Inconsistent cookie domains: %q vs %q for cookie %s",
|
||||
seenDomain, normalizedDomain, cookie.Name)
|
||||
}
|
||||
|
||||
// Verify it matches configured domain (browsers may normalize by removing leading dot)
|
||||
// Empty domain is also acceptable for host-only cookies
|
||||
if cookie.Domain != "" && cookie.Domain != ".example.com" && cookie.Domain != "example.com" {
|
||||
t.Errorf("Cookie %s has domain %q, expected %q, %q, or empty",
|
||||
cookie.Name, cookie.Domain, ".example.com", "example.com")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestCookieDomainWithReverseProxy simulates a reverse proxy scenario
|
||||
func TestCookieDomainWithReverseProxy(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
|
||||
// No configured domain, should auto-detect from X-Forwarded-Host
|
||||
sm, err := NewSessionManager("test-encryption-key-32-characters", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
// Simulate reverse proxy request
|
||||
req := httptest.NewRequest("GET", "http://internal.local:8080/test", nil)
|
||||
req.Header.Set("X-Forwarded-Host", "public.example.com")
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
|
||||
// Test the domain configuration
|
||||
// Since getCookieOptions is private, we'll check the configured domain directly
|
||||
options := &http.Cookie{
|
||||
Domain: sm.cookieDomain,
|
||||
}
|
||||
// If no configured domain, simulate auto-detection
|
||||
if sm.cookieDomain == "" && req != nil {
|
||||
host := req.Host
|
||||
if forwardedHost := req.Header.Get("X-Forwarded-Host"); forwardedHost != "" {
|
||||
host = forwardedHost
|
||||
}
|
||||
if host != "" && !strings.Contains(host, "localhost") && !strings.Contains(host, "127.0.0.1") {
|
||||
if colonIndex := strings.Index(host, ":"); colonIndex != -1 {
|
||||
host = host[:colonIndex]
|
||||
}
|
||||
options.Domain = host
|
||||
}
|
||||
}
|
||||
|
||||
// Check secure flag based on X-Forwarded-Proto
|
||||
isSecure := req.Header.Get("X-Forwarded-Proto") == "https" || req.TLS != nil
|
||||
options.Secure = isSecure // Note: forceHTTPS is private so we can't access it in test
|
||||
|
||||
// Should use the forwarded host
|
||||
if options.Domain != "public.example.com" {
|
||||
t.Errorf("Expected domain from X-Forwarded-Host %q, got %q",
|
||||
"public.example.com", options.Domain)
|
||||
}
|
||||
|
||||
// Should be secure due to X-Forwarded-Proto
|
||||
if !options.Secure {
|
||||
t.Error("Expected Secure flag to be true with X-Forwarded-Proto: https")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,476 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestCSRFTokenSessionManagement tests the session management changes that fix the login loop
|
||||
func TestCSRFTokenSessionManagement(t *testing.T) {
|
||||
// Test that CSRF tokens persist through the authentication flow
|
||||
t.Run("CSRF_Token_Persists_After_Selective_Clear", func(t *testing.T) {
|
||||
// Create a session manager
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create initial request
|
||||
req := httptest.NewRequest("GET", "http://example.com/test", nil)
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set initial values
|
||||
csrfToken := "critical-csrf-token"
|
||||
session.SetCSRF(csrfToken)
|
||||
session.SetNonce("test-nonce")
|
||||
session.SetAuthenticated(true)
|
||||
session.SetEmail("[email protected]")
|
||||
session.SetAccessToken("old-access-token")
|
||||
session.SetRefreshToken("old-refresh-token")
|
||||
session.SetIDToken("old-id-token")
|
||||
|
||||
// Save session
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Get cookies
|
||||
cookies := rec.Result().Cookies()
|
||||
|
||||
// Create new request with cookies (simulating redirect back)
|
||||
req2 := httptest.NewRequest("GET", "http://example.com/test2", nil)
|
||||
for _, cookie := range cookies {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
|
||||
// Get session again
|
||||
session2, err := sessionManager.GetSession(req2)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify all values are there
|
||||
assert.Equal(t, csrfToken, session2.GetCSRF())
|
||||
assert.Equal(t, "test-nonce", session2.GetNonce())
|
||||
assert.True(t, session2.GetAuthenticated())
|
||||
|
||||
// Now perform selective clearing (as done in the fix)
|
||||
session2.SetAuthenticated(false)
|
||||
session2.SetEmail("")
|
||||
session2.SetAccessToken("")
|
||||
session2.SetRefreshToken("")
|
||||
session2.SetIDToken("")
|
||||
// Clear OIDC flow values from previous attempts
|
||||
session2.SetNonce("")
|
||||
session2.SetCodeVerifier("")
|
||||
|
||||
// CRITICAL: CSRF token should still be there
|
||||
assert.Equal(t, csrfToken, session2.GetCSRF(), "CSRF token must persist after selective clearing")
|
||||
|
||||
// Save again
|
||||
rec2 := httptest.NewRecorder()
|
||||
err = session2.Save(req2, rec2)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify CSRF token persists in new session
|
||||
req3 := httptest.NewRequest("GET", "http://example.com/callback", nil)
|
||||
for _, cookie := range rec2.Result().Cookies() {
|
||||
req3.AddCookie(cookie)
|
||||
}
|
||||
|
||||
session3, err := sessionManager.GetSession(req3)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, csrfToken, session3.GetCSRF(), "CSRF token must persist across saves")
|
||||
})
|
||||
|
||||
// Test that marking session as dirty forces save
|
||||
t.Run("Mark_Dirty_Forces_Session_Save", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
req := httptest.NewRequest("GET", "http://example.com/test", nil)
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set CSRF token
|
||||
csrfToken := "test-csrf-token"
|
||||
session.SetCSRF(csrfToken)
|
||||
|
||||
// Mark as dirty explicitly
|
||||
session.MarkDirty()
|
||||
|
||||
// Save should work even if no apparent changes
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify cookie was set
|
||||
cookies := rec.Result().Cookies()
|
||||
assert.NotEmpty(t, cookies, "Cookies should be set after save")
|
||||
|
||||
// Find main session cookie
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie, "Main session cookie should be set")
|
||||
})
|
||||
|
||||
// Test Azure-specific session handling
|
||||
t.Run("Azure_Session_Cookie_Configuration", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Simulate Azure callback scenario
|
||||
req := httptest.NewRequest("GET", "http://example.com/oidc/callback?code=test&state=test-csrf", nil)
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set values as would happen in auth flow
|
||||
session.SetCSRF("test-csrf")
|
||||
session.SetNonce("test-nonce")
|
||||
|
||||
// Save with proper cookie settings
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Check cookie attributes
|
||||
cookies := rec.Result().Cookies()
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
// Azure requires SameSite=Lax for cross-site redirects
|
||||
assert.Equal(t, http.SameSiteLaxMode, cookie.SameSite, "SameSite should be Lax for Azure compatibility")
|
||||
assert.Equal(t, "/", cookie.Path, "Path should be root")
|
||||
assert.True(t, cookie.HttpOnly, "Cookie should be HttpOnly")
|
||||
// In production, Secure would be true, but false in test
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
// Test session continuity through auth flow
|
||||
t.Run("Session_Continuity_Through_Auth_Flow", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Step 1: Initial request
|
||||
req1 := httptest.NewRequest("GET", "http://example.com/protected", nil)
|
||||
session1, err := sessionManager.GetSession(req1)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Simulate auth initiation
|
||||
csrfToken := "auth-flow-csrf-token"
|
||||
nonce := "auth-flow-nonce"
|
||||
session1.SetCSRF(csrfToken)
|
||||
session1.SetNonce(nonce)
|
||||
session1.SetIncomingPath("/protected")
|
||||
|
||||
// Force save
|
||||
session1.MarkDirty()
|
||||
rec1 := httptest.NewRecorder()
|
||||
err = session1.Save(req1, rec1)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies := rec1.Result().Cookies()
|
||||
require.NotEmpty(t, cookies)
|
||||
|
||||
// Step 2: Callback request with same cookies
|
||||
req2 := httptest.NewRequest("GET", "http://example.com/oidc/callback?code=test&state="+csrfToken, nil)
|
||||
for _, cookie := range cookies {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
|
||||
session2, err := sessionManager.GetSession(req2)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify session continuity
|
||||
assert.Equal(t, csrfToken, session2.GetCSRF(), "CSRF token should be maintained")
|
||||
assert.Equal(t, nonce, session2.GetNonce(), "Nonce should be maintained")
|
||||
assert.Equal(t, "/protected", session2.GetIncomingPath(), "Incoming path should be maintained")
|
||||
})
|
||||
|
||||
// Test large token handling doesn't affect CSRF
|
||||
t.Run("Large_Tokens_Dont_Affect_CSRF", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
req := httptest.NewRequest("GET", "http://example.com/test", nil)
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set CSRF first
|
||||
csrfToken := "important-csrf"
|
||||
session.SetCSRF(csrfToken)
|
||||
|
||||
// Add large tokens that might cause chunking
|
||||
largeToken := generateMockJWT(5000)
|
||||
session.SetIDToken(largeToken)
|
||||
session.SetAccessToken(largeToken)
|
||||
|
||||
// Save
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Count cookies
|
||||
cookies := rec.Result().Cookies()
|
||||
mainFound := false
|
||||
chunkCount := 0
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainFound = true
|
||||
}
|
||||
if strings.Contains(cookie.Name, "_oidc_raczylo_") && strings.Contains(cookie.Name, "_") {
|
||||
chunkCount++
|
||||
}
|
||||
}
|
||||
|
||||
assert.True(t, mainFound, "Main session cookie must exist")
|
||||
t.Logf("Total chunks created: %d", chunkCount)
|
||||
|
||||
// Verify CSRF is still accessible
|
||||
req2 := httptest.NewRequest("GET", "http://example.com/test2", nil)
|
||||
for _, cookie := range cookies {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
|
||||
session2, err := sessionManager.GetSession(req2)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, csrfToken, session2.GetCSRF(), "CSRF must be preserved with large tokens")
|
||||
})
|
||||
}
|
||||
|
||||
// TestAuthFlowWithoutExternalDependencies tests the auth flow without external dependencies
|
||||
func TestAuthFlowWithoutExternalDependencies(t *testing.T) {
|
||||
plugin := CreateConfig()
|
||||
plugin.ProviderURL = "https://login.microsoftonline.com/test-tenant/v2.0"
|
||||
plugin.ClientID = "test-client-id"
|
||||
plugin.ClientSecret = "test-client-secret"
|
||||
plugin.CallbackURL = "http://example.com/oidc/callback"
|
||||
plugin.SessionEncryptionKey = "test-encryption-key-32-characters"
|
||||
plugin.LogLevel = "debug"
|
||||
|
||||
// Variables removed as they're not used in this test
|
||||
|
||||
// We can't fully initialize TraefikOidc without network access,
|
||||
// but we can test the session management directly
|
||||
sessionManager, err := NewSessionManager(plugin.SessionEncryptionKey, plugin.ForceHTTPS, "", NewLogger(plugin.LogLevel))
|
||||
require.NoError(t, err)
|
||||
|
||||
t.Run("Session_Created_On_Protected_Request", func(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "http://example.com/protected", nil)
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Session should be new
|
||||
assert.False(t, session.GetAuthenticated())
|
||||
|
||||
// Set auth flow values
|
||||
session.SetCSRF("test-csrf-token")
|
||||
session.SetNonce("test-nonce")
|
||||
session.SetIncomingPath("/protected")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Should have set cookies
|
||||
cookies := rec.Result().Cookies()
|
||||
assert.NotEmpty(t, cookies)
|
||||
})
|
||||
}
|
||||
|
||||
// TestRegressionLoginLoop specifically tests the fix for issue #53
|
||||
func TestRegressionLoginLoop(t *testing.T) {
|
||||
// This test verifies that the specific changes made to fix the login loop work correctly
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Simulate the exact flow that was causing the login loop
|
||||
t.Run("Fix_Session_Clear_Timing", func(t *testing.T) {
|
||||
// Initial request
|
||||
req := httptest.NewRequest("GET", "http://example.com/protected", nil)
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set initial session data
|
||||
session.SetAuthenticated(true)
|
||||
session.SetEmail("[email protected]")
|
||||
session.SetAccessToken("old-token")
|
||||
session.SetCSRF("existing-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies := rec.Result().Cookies()
|
||||
|
||||
// New request with existing session (user hits protected resource again)
|
||||
req2 := httptest.NewRequest("GET", "http://example.com/protected", nil)
|
||||
for _, cookie := range cookies {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
|
||||
session2, err := sessionManager.GetSession(req2)
|
||||
require.NoError(t, err)
|
||||
|
||||
// OLD BEHAVIOR: session.Clear() would have been called here, losing CSRF
|
||||
// NEW BEHAVIOR: Selective clearing
|
||||
session2.SetAuthenticated(false)
|
||||
session2.SetEmail("")
|
||||
session2.SetAccessToken("")
|
||||
session2.SetRefreshToken("")
|
||||
session2.SetIDToken("")
|
||||
session2.SetNonce("")
|
||||
session2.SetCodeVerifier("")
|
||||
|
||||
// CSRF should still exist
|
||||
existingCSRF := session2.GetCSRF()
|
||||
assert.Equal(t, "existing-csrf", existingCSRF, "CSRF should persist through selective clear")
|
||||
|
||||
// Set new auth flow values
|
||||
newCSRF := "new-csrf-for-auth"
|
||||
session2.SetCSRF(newCSRF)
|
||||
session2.SetNonce("new-nonce")
|
||||
|
||||
// Force save
|
||||
session2.MarkDirty()
|
||||
rec2 := httptest.NewRecorder()
|
||||
err = session2.Save(req2, rec2)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Simulate callback
|
||||
cookies2 := rec2.Result().Cookies()
|
||||
req3 := httptest.NewRequest("GET", "http://example.com/oidc/callback?code=test&state="+newCSRF, nil)
|
||||
for _, cookie := range cookies2 {
|
||||
req3.AddCookie(cookie)
|
||||
}
|
||||
|
||||
session3, err := sessionManager.GetSession(req3)
|
||||
require.NoError(t, err)
|
||||
|
||||
// CSRF should match
|
||||
assert.Equal(t, newCSRF, session3.GetCSRF(), "CSRF token should be available in callback")
|
||||
})
|
||||
|
||||
t.Run("Fix_Force_Session_Save", func(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "http://example.com/test", nil)
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set CSRF but don't change authenticated status
|
||||
session.SetCSRF("important-csrf")
|
||||
|
||||
// Without MarkDirty(), the session might not save if the session manager
|
||||
// doesn't detect the change. The fix ensures we call MarkDirty()
|
||||
session.MarkDirty()
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify cookie was actually set
|
||||
cookies := rec.Result().Cookies()
|
||||
found := false
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
found = true
|
||||
assert.NotEmpty(t, cookie.Value, "Cookie should have value")
|
||||
}
|
||||
}
|
||||
assert.True(t, found, "Main session cookie must be set after MarkDirty")
|
||||
})
|
||||
}
|
||||
|
||||
// TestCSRFValidationTiming tests timing-sensitive CSRF validation scenarios
|
||||
func TestCSRFValidationTiming(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
t.Run("Rapid_Redirect_Maintains_CSRF", func(t *testing.T) {
|
||||
// Simulate rapid redirect (no delay between auth init and callback)
|
||||
req1 := httptest.NewRequest("GET", "http://example.com/auth", nil)
|
||||
session1, err := sessionManager.GetSession(req1)
|
||||
require.NoError(t, err)
|
||||
|
||||
csrfToken := "rapid-redirect-csrf"
|
||||
session1.SetCSRF(csrfToken)
|
||||
session1.MarkDirty()
|
||||
|
||||
rec1 := httptest.NewRecorder()
|
||||
err = session1.Save(req1, rec1)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Immediate callback (no delay)
|
||||
cookies := rec1.Result().Cookies()
|
||||
req2 := httptest.NewRequest("GET", "http://example.com/callback", nil)
|
||||
for _, cookie := range cookies {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
|
||||
session2, err := sessionManager.GetSession(req2)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, csrfToken, session2.GetCSRF())
|
||||
})
|
||||
|
||||
t.Run("Delayed_Redirect_Maintains_CSRF", func(t *testing.T) {
|
||||
// Simulate delayed redirect (user takes time at provider)
|
||||
req1 := httptest.NewRequest("GET", "http://example.com/auth", nil)
|
||||
session1, err := sessionManager.GetSession(req1)
|
||||
require.NoError(t, err)
|
||||
|
||||
csrfToken := "delayed-redirect-csrf"
|
||||
session1.SetCSRF(csrfToken)
|
||||
session1.MarkDirty()
|
||||
|
||||
rec1 := httptest.NewRecorder()
|
||||
err = session1.Save(req1, rec1)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Simulate delay
|
||||
time.Sleep(500 * time.Millisecond)
|
||||
|
||||
// Callback after delay
|
||||
cookies := rec1.Result().Cookies()
|
||||
req2 := httptest.NewRequest("GET", "http://example.com/callback", nil)
|
||||
for _, cookie := range cookies {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
|
||||
session2, err := sessionManager.GetSession(req2)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, csrfToken, session2.GetCSRF(), "CSRF should persist even with delay")
|
||||
})
|
||||
}
|
||||
|
||||
// Helper function to generate a mock JWT of specified size
|
||||
func generateMockJWT(targetSize int) string {
|
||||
header := "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9"
|
||||
signature := "signature"
|
||||
|
||||
// Calculate payload size needed
|
||||
overhead := len(header) + len(signature) + 2 // 2 dots
|
||||
payloadSize := targetSize - overhead
|
||||
|
||||
// Create payload with padding
|
||||
payload := map[string]interface{}{
|
||||
"sub": "1234567890",
|
||||
"name": "Test User",
|
||||
"iat": time.Now().Unix(),
|
||||
"exp": time.Now().Add(time.Hour).Unix(),
|
||||
"padding": strings.Repeat("x", payloadSize-100), // Leave room for JSON structure
|
||||
}
|
||||
|
||||
payloadJSON, _ := json.Marshal(payload)
|
||||
payloadB64 := base64.RawURLEncoding.EncodeToString(payloadJSON)
|
||||
|
||||
return header + "." + payloadB64 + "." + signature
|
||||
}
|
||||
+3
-3
@@ -174,9 +174,9 @@ type CircuitBreakerConfig struct {
|
||||
// DefaultCircuitBreakerConfig returns default circuit breaker configuration
|
||||
func DefaultCircuitBreakerConfig() CircuitBreakerConfig {
|
||||
return CircuitBreakerConfig{
|
||||
MaxFailures: 5,
|
||||
Timeout: 30 * time.Second,
|
||||
ResetTimeout: 10 * time.Second,
|
||||
MaxFailures: 2, // Reduced from 5 to open circuit faster
|
||||
Timeout: 60 * time.Second, // Increased from 30s to reduce retry frequency
|
||||
ResetTimeout: 30 * time.Second, // Increased from 10s to wait longer before retrying
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -43,7 +43,7 @@ func TestGoogleOIDCRefreshTokenHandling(t *testing.T) {
|
||||
}
|
||||
|
||||
// Create a session manager
|
||||
sessionManager, _ := NewSessionManager("0123456789abcdef0123456789abcdef", true, mockLogger)
|
||||
sessionManager, _ := NewSessionManager("0123456789abcdef0123456789abcdef", true, "", mockLogger)
|
||||
tOidc.sessionManager = sessionManager
|
||||
|
||||
t.Run("Google provider detection adds required parameters", func(t *testing.T) {
|
||||
@@ -367,7 +367,7 @@ func TestGoogleOIDCRefreshTokenHandling(t *testing.T) {
|
||||
}
|
||||
|
||||
// Create a session manager
|
||||
sessionManager, _ := NewSessionManager("0123456789abcdef0123456789abcdef", true, mockLogger)
|
||||
sessionManager, _ := NewSessionManager("0123456789abcdef0123456789abcdef", true, "", mockLogger)
|
||||
googleTOidc.sessionManager = sessionManager
|
||||
|
||||
// Create a mock token verifier
|
||||
|
||||
+8
-2
@@ -140,10 +140,16 @@ func (t *TraefikOidc) exchangeTokens(ctx context.Context, grantType string, code
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to exchange tokens: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
defer func() {
|
||||
// Always drain the body before closing to ensure connection can be reused
|
||||
io.Copy(io.Discard, resp.Body)
|
||||
resp.Body.Close()
|
||||
}()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
bodyBytes, _ := io.ReadAll(resp.Body)
|
||||
// Limit body read to prevent memory issues
|
||||
limitReader := io.LimitReader(resp.Body, 1024*10) // 10KB limit
|
||||
bodyBytes, _ := io.ReadAll(limitReader)
|
||||
return nil, fmt.Errorf("token endpoint returned status %d: %s", resp.StatusCode, string(bodyBytes))
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,239 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestIssue53Regression tests the specific issue reported in GitHub issue #53
|
||||
// where Azure OIDC authentication fails with "CSRF token missing in session"
|
||||
// This was caused by incorrect HTTPS detection in reverse proxy environments
|
||||
func TestIssue53Regression(t *testing.T) {
|
||||
t.Run("Issue53_CSRF_Missing_In_Session_Fix", func(t *testing.T) {
|
||||
// This test reproduces the exact scenario from issue #53:
|
||||
// 1. User accesses app via HTTPS through Traefik
|
||||
// 2. Traefik terminates SSL and forwards HTTP internally
|
||||
// 3. Session cookies must be properly configured for HTTPS
|
||||
// 4. CSRF token must persist through the OAuth flow
|
||||
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Step 1: Initial request to protected resource
|
||||
// User accesses https://app.example.com/protected
|
||||
// Traefik forwards as http://internal/protected with X-Forwarded-Proto: https
|
||||
initReq := httptest.NewRequest("GET", "http://internal/protected", nil)
|
||||
initReq.Header.Set("X-Forwarded-Proto", "https")
|
||||
initReq.Header.Set("X-Forwarded-Host", "app.example.com")
|
||||
initReq.Header.Set("User-Agent", "Mozilla/5.0") // Real browser
|
||||
|
||||
// Get session and set OAuth flow data
|
||||
session, err := sessionManager.GetSession(initReq)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set CSRF and other OAuth data
|
||||
csrfToken := "csrf-token-for-azure"
|
||||
nonce := "nonce-for-azure"
|
||||
session.SetCSRF(csrfToken)
|
||||
session.SetNonce(nonce)
|
||||
session.SetCodeVerifier("pkce-verifier")
|
||||
session.SetIncomingPath("/protected")
|
||||
session.MarkDirty()
|
||||
|
||||
// Save session - this is where the bug was
|
||||
// Previously: used r.URL.Scheme which is always "http" behind proxy
|
||||
// Now: uses X-Forwarded-Proto header
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(initReq, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify cookies are secure
|
||||
cookies := rec.Result().Cookies()
|
||||
require.NotEmpty(t, cookies, "Cookies must be set")
|
||||
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie, "Main session cookie must be set")
|
||||
|
||||
// Critical assertions for issue #53
|
||||
assert.True(t, mainCookie.Secure, "Cookie MUST have Secure flag for HTTPS (was the bug)")
|
||||
assert.Equal(t, http.SameSiteLaxMode, mainCookie.SameSite, "MUST use Lax for OAuth callbacks to work")
|
||||
assert.Equal(t, "/", mainCookie.Path, "Cookie path must be root")
|
||||
assert.True(t, mainCookie.HttpOnly, "Cookie must be HttpOnly")
|
||||
assert.Equal(t, "app.example.com", mainCookie.Domain, "Domain should use X-Forwarded-Host")
|
||||
|
||||
// Step 2: OAuth provider redirects back to callback
|
||||
// Azure redirects to https://app.example.com/oidc/callback?code=...&state=...
|
||||
// Traefik forwards as http://internal/oidc/callback with headers
|
||||
callbackReq := httptest.NewRequest("GET",
|
||||
"http://internal/oidc/callback?code=azure-auth-code&state="+csrfToken, nil)
|
||||
callbackReq.Header.Set("X-Forwarded-Proto", "https")
|
||||
callbackReq.Header.Set("X-Forwarded-Host", "app.example.com")
|
||||
callbackReq.Header.Set("User-Agent", "Mozilla/5.0")
|
||||
|
||||
// Add cookies from initial request
|
||||
// Browser sends secure cookies because request is HTTPS
|
||||
for _, cookie := range cookies {
|
||||
callbackReq.AddCookie(cookie)
|
||||
}
|
||||
|
||||
// Get session in callback
|
||||
callbackSession, err := sessionManager.GetSession(callbackReq)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify CSRF token is present (was missing in issue #53)
|
||||
retrievedCSRF := callbackSession.GetCSRF()
|
||||
assert.Equal(t, csrfToken, retrievedCSRF,
|
||||
"CSRF token MUST persist (was missing in issue #53)")
|
||||
|
||||
// Verify other session data also persists
|
||||
assert.Equal(t, nonce, callbackSession.GetNonce(),
|
||||
"Nonce must persist for security")
|
||||
assert.Equal(t, "pkce-verifier", callbackSession.GetCodeVerifier(),
|
||||
"PKCE verifier must persist")
|
||||
assert.Equal(t, "/protected", callbackSession.GetIncomingPath(),
|
||||
"Original path must persist for redirect after auth")
|
||||
})
|
||||
|
||||
t.Run("Issue53_Signature_Verification_With_Secure_Session", func(t *testing.T) {
|
||||
// This test ensures that once the session is properly maintained,
|
||||
// token signature verification works correctly for Azure tokens
|
||||
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create authenticated session with Azure tokens
|
||||
req := httptest.NewRequest("GET", "http://internal/api/data", nil)
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
req.Header.Set("X-Forwarded-Host", "app.example.com")
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Simulate successful Azure authentication
|
||||
session.SetAuthenticated(true)
|
||||
session.SetEmail("[email protected]")
|
||||
// Azure may use opaque access tokens
|
||||
session.SetAccessToken("opaque-azure-access-token")
|
||||
session.SetIDToken("eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.NHVaYe26MbtOYhSKkoKYdFVomg4i8ZJd8_-RU8VNbftc4TSMb4bXP3l3YlNWACwyXPGffz5aXHc6lty1Y2t4SWRqGteragsVdZufDn5BlnJl9pdR_kdVFUsra2rWKEofkZeIC4yWytE58sMIihvo9H1ScmmVwBcQP6XETqYd0aSHp1gOa9RdUPDvoXQ5oqygTqVtxaDr6wUFKrKItgBMzWIdNZ6y7O9E0DhEPTbE9rfBo6KTFsHAZnMg4k68CDp2woYIaXbmYTWcvbzIuHO7_37GT79XdIwkm95QJ7hYC9RiwrV7mesbY4PAahERJawntho0my942XheVLmGwLMBkQ")
|
||||
session.SetRefreshToken("azure-refresh-token")
|
||||
|
||||
// Save with proper security
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify session can be retrieved and tokens are intact
|
||||
cookies := rec.Result().Cookies()
|
||||
req2 := httptest.NewRequest("GET", "http://internal/api/data", nil)
|
||||
req2.Header.Set("X-Forwarded-Proto", "https")
|
||||
for _, cookie := range cookies {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
|
||||
session2, err := sessionManager.GetSession(req2)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.True(t, session2.GetAuthenticated(), "User should remain authenticated")
|
||||
assert.Equal(t, "[email protected]", session2.GetEmail())
|
||||
assert.NotEmpty(t, session2.GetAccessToken(), "Access token should persist")
|
||||
assert.NotEmpty(t, session2.GetIDToken(), "ID token should persist")
|
||||
assert.NotEmpty(t, session2.GetRefreshToken(), "Refresh token should persist")
|
||||
})
|
||||
|
||||
t.Run("Issue53_Redirect_Loop_Prevention", func(t *testing.T) {
|
||||
// This test verifies the redirect loop prevention mechanism
|
||||
// that was added to handle authentication failures gracefully
|
||||
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
req := httptest.NewRequest("GET", "http://internal/protected", nil)
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Simulate multiple redirect attempts
|
||||
for i := 0; i < 3; i++ {
|
||||
session.IncrementRedirectCount()
|
||||
}
|
||||
|
||||
// Verify redirect count is tracked
|
||||
count := session.GetRedirectCount()
|
||||
assert.Equal(t, 3, count, "Redirect count should be tracked")
|
||||
|
||||
// After successful auth, count should be reset
|
||||
session.SetAuthenticated(true)
|
||||
session.ResetRedirectCount()
|
||||
assert.Equal(t, 0, session.GetRedirectCount(), "Count should reset after auth")
|
||||
})
|
||||
}
|
||||
|
||||
// TestReverseProxySameSiteHandling tests SameSite cookie attribute handling
|
||||
// in different reverse proxy scenarios
|
||||
func TestReverseProxySameSiteHandling(t *testing.T) {
|
||||
t.Run("SameSite_Lax_For_HTTPS_OAuth", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// HTTPS request via proxy
|
||||
req := httptest.NewRequest("GET", "http://internal/test", nil)
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
req.Header.Set("User-Agent", "Mozilla/5.0")
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
session.SetCSRF("test")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// HTTPS should use Lax mode for OAuth compatibility
|
||||
cookies := rec.Result().Cookies()
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
assert.Equal(t, http.SameSiteLaxMode, cookie.SameSite,
|
||||
"HTTPS should use Lax SameSite for OAuth callbacks")
|
||||
break
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("SameSite_Lax_For_HTTP", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Plain HTTP request (no proxy headers)
|
||||
req := httptest.NewRequest("GET", "http://localhost/test", nil)
|
||||
req.Header.Set("User-Agent", "Mozilla/5.0")
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
session.SetCSRF("test")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// HTTP should use Lax mode for compatibility
|
||||
cookies := rec.Result().Cookies()
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
assert.Equal(t, http.SameSiteLaxMode, cookie.SameSite,
|
||||
"HTTP should use Lax SameSite for compatibility")
|
||||
break
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,213 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestIssue60Integration tests the complete fix for issue #60
|
||||
// This test verifies that the plugin can handle missing claim fields without panicking
|
||||
func TestIssue60Integration(t *testing.T) {
|
||||
t.Run("Config_With_Safe_Functions_Validates", func(t *testing.T) {
|
||||
config := CreateConfig()
|
||||
config.ProviderURL = "https://example.com"
|
||||
config.ClientID = "test-client"
|
||||
config.ClientSecret = "test-secret"
|
||||
config.CallbackURL = "/callback"
|
||||
config.SessionEncryptionKey = "test-encryption-key-32-characters"
|
||||
|
||||
// Templates using safe functions for missing fields
|
||||
config.Headers = []TemplatedHeader{
|
||||
{Name: "X-User-Email", Value: "{{.Claims.email}}"},
|
||||
{Name: "X-User-Role", Value: "{{get .Claims \"internal_role\"}}"},
|
||||
{Name: "X-User-Dept", Value: "{{default \"unknown\" .Claims.department}}"},
|
||||
{Name: "X-User-Groups", Value: "{{with .Claims.groups}}{{.}}{{end}}"},
|
||||
}
|
||||
|
||||
// Configuration should validate successfully
|
||||
err := config.Validate()
|
||||
assert.NoError(t, err, "Config with safe template functions should validate")
|
||||
})
|
||||
|
||||
t.Run("Direct_Template_Access_Works", func(t *testing.T) {
|
||||
config := CreateConfig()
|
||||
config.ProviderURL = "https://example.com"
|
||||
config.ClientID = "test-client"
|
||||
config.ClientSecret = "test-secret"
|
||||
config.CallbackURL = "/callback"
|
||||
config.SessionEncryptionKey = "test-encryption-key-32-characters"
|
||||
|
||||
// Direct claim access (will return <no value> if missing with missingkey=zero)
|
||||
config.Headers = []TemplatedHeader{
|
||||
{Name: "X-User-Email", Value: "{{.Claims.email}}"},
|
||||
{Name: "X-Internal-Role", Value: "{{.Claims.internal_role}}"},
|
||||
}
|
||||
|
||||
err := config.Validate()
|
||||
assert.NoError(t, err, "Direct claim access should validate")
|
||||
})
|
||||
|
||||
t.Run("Config_Rejects_Dangerous_Templates", func(t *testing.T) {
|
||||
config := CreateConfig()
|
||||
config.ProviderURL = "https://example.com"
|
||||
config.ClientID = "test-client"
|
||||
config.ClientSecret = "test-secret"
|
||||
config.CallbackURL = "/callback"
|
||||
config.SessionEncryptionKey = "test-encryption-key-32-characters"
|
||||
|
||||
// Dangerous template patterns should be rejected
|
||||
dangerousTemplates := []TemplatedHeader{
|
||||
{Name: "X-Bad-1", Value: "{{call .SomeFunc}}"},
|
||||
{Name: "X-Bad-2", Value: "{{range .Items}}{{.}}{{end}}"},
|
||||
{Name: "X-Bad-3", Value: "{{index .Array 0}}"},
|
||||
{Name: "X-Bad-4", Value: "{{printf \"%s\" .Data}}"},
|
||||
}
|
||||
|
||||
for _, header := range dangerousTemplates {
|
||||
config.Headers = []TemplatedHeader{header}
|
||||
err := config.Validate()
|
||||
require.Error(t, err, "Dangerous template should be rejected: %s", header.Value)
|
||||
assert.Contains(t, err.Error(), "dangerous", "Error should mention dangerous pattern")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Verify_Template_Execution_Context", func(t *testing.T) {
|
||||
// This test verifies that our template context matches what's actually used
|
||||
// The context should have these fields (all capitalized):
|
||||
// - AccessToken
|
||||
// - IDToken (or IdToken)
|
||||
// - RefreshToken
|
||||
// - Claims (map[string]interface{})
|
||||
|
||||
config := CreateConfig()
|
||||
config.ProviderURL = "https://example.com"
|
||||
config.ClientID = "test-client"
|
||||
config.ClientSecret = "test-secret"
|
||||
config.CallbackURL = "/callback"
|
||||
config.SessionEncryptionKey = "test-encryption-key-32-characters"
|
||||
|
||||
// These should all be valid based on the actual template context
|
||||
validContextTemplates := []TemplatedHeader{
|
||||
{Name: "X-Access-Token", Value: "{{.AccessToken}}"},
|
||||
{Name: "X-ID-Token", Value: "{{.IdToken}}"},
|
||||
{Name: "X-Refresh-Token", Value: "{{.RefreshToken}}"},
|
||||
{Name: "X-User-Email", Value: "{{.Claims.email}}"},
|
||||
{Name: "X-User-Sub", Value: "{{.Claims.sub}}"},
|
||||
}
|
||||
|
||||
config.Headers = validContextTemplates
|
||||
err := config.Validate()
|
||||
assert.NoError(t, err, "All valid context fields should pass validation")
|
||||
})
|
||||
|
||||
t.Run("Common_Azure_AD_Claims", func(t *testing.T) {
|
||||
// Test Azure AD specific claims mentioned in issue #60
|
||||
config := CreateConfig()
|
||||
config.ProviderURL = "https://example.com"
|
||||
config.ClientID = "test-client"
|
||||
config.ClientSecret = "test-secret"
|
||||
config.CallbackURL = "/callback"
|
||||
config.SessionEncryptionKey = "test-encryption-key-32-characters"
|
||||
|
||||
// Azure AD commonly uses these claim fields
|
||||
config.Headers = []TemplatedHeader{
|
||||
{Name: "X-User-Email", Value: "{{.Claims.email}}"},
|
||||
{Name: "X-User-OID", Value: "{{.Claims.oid}}"},
|
||||
{Name: "X-User-TID", Value: "{{.Claims.tid}}"},
|
||||
{Name: "X-User-UPN", Value: "{{.Claims.upn}}"},
|
||||
{Name: "X-Internal-Role", Value: "{{.Claims.internal_role}}"}, // Custom claim from issue #60
|
||||
}
|
||||
|
||||
err := config.Validate()
|
||||
assert.NoError(t, err, "Azure AD claims should validate")
|
||||
})
|
||||
}
|
||||
|
||||
// TestIssue60RealWorldScenarios tests real-world scenarios from issue #60
|
||||
func TestIssue60RealWorldScenarios(t *testing.T) {
|
||||
t.Run("Missing_Internal_Role_Field", func(t *testing.T) {
|
||||
// This is the exact scenario from issue #60
|
||||
// User passes {{.Claims.internal_role}} but the field doesn't exist
|
||||
config := CreateConfig()
|
||||
config.ProviderURL = "https://example.com"
|
||||
config.ClientID = "test-client"
|
||||
config.ClientSecret = "test-secret"
|
||||
config.CallbackURL = "/callback"
|
||||
config.SessionEncryptionKey = "test-encryption-key-32-characters"
|
||||
|
||||
// The problematic template from issue #60
|
||||
config.Headers = []TemplatedHeader{
|
||||
{Name: "X-Internal-Role", Value: "{{.Claims.internal_role}}"},
|
||||
}
|
||||
|
||||
// Should validate (internal_role is in the safe fields list)
|
||||
err := config.Validate()
|
||||
assert.NoError(t, err, "Template with internal_role should validate")
|
||||
})
|
||||
|
||||
t.Run("Safe_Access_Patterns_From_Guide", func(t *testing.T) {
|
||||
// Test all the safe patterns documented in TEMPLATE_HEADERS_GUIDE.md
|
||||
config := CreateConfig()
|
||||
config.ProviderURL = "https://example.com"
|
||||
config.ClientID = "test-client"
|
||||
config.ClientSecret = "test-secret"
|
||||
config.CallbackURL = "/callback"
|
||||
config.SessionEncryptionKey = "test-encryption-key-32-characters"
|
||||
|
||||
// All safe patterns from the guide
|
||||
config.Headers = []TemplatedHeader{
|
||||
// Basic field access
|
||||
{Name: "X-User-Role", Value: "{{.Claims.internal_role}}"},
|
||||
|
||||
// Using the get function
|
||||
{Name: "X-User-Role-Get", Value: "{{get .Claims \"internal_role\"}}"},
|
||||
|
||||
// Using the default function
|
||||
{Name: "X-User-Role-Default", Value: "{{default \"guest\" .Claims.role}}"},
|
||||
|
||||
// Nested fields with 'with'
|
||||
{Name: "X-User-Admin", Value: "{{with .Claims.groups}}{{.admin}}{{end}}"},
|
||||
}
|
||||
|
||||
err := config.Validate()
|
||||
assert.NoError(t, err, "All safe patterns from guide should validate")
|
||||
})
|
||||
}
|
||||
|
||||
// TestIssue60DoubleProcessingConcern tests the user's specific concern about double processing
|
||||
func TestIssue60DoubleProcessingConcern(t *testing.T) {
|
||||
t.Run("Template_Not_Evaluated_During_Config_Parse", func(t *testing.T) {
|
||||
// The user was concerned that templates might be processed twice:
|
||||
// 1. Once when Traefik parses the config
|
||||
// 2. Once when the plugin executes the template
|
||||
|
||||
// This test verifies that templates are stored as strings during config parsing
|
||||
config := &Config{
|
||||
Headers: []TemplatedHeader{
|
||||
{Name: "X-Test", Value: "{{.Claims.test}}"},
|
||||
},
|
||||
}
|
||||
|
||||
// The template should still be a raw string after config creation
|
||||
assert.Equal(t, "{{.Claims.test}}", config.Headers[0].Value,
|
||||
"Template should remain as raw string in config")
|
||||
|
||||
// The template is only parsed/executed when the plugin initializes and processes requests
|
||||
// Not during config unmarshaling
|
||||
})
|
||||
|
||||
t.Run("Functions_Preserved_Through_Config_Marshaling", func(t *testing.T) {
|
||||
// Test that our custom function syntax survives config marshaling/unmarshaling
|
||||
originalValue := `{{get .Claims "internal_role"}}`
|
||||
header := TemplatedHeader{
|
||||
Name: "X-Role",
|
||||
Value: originalValue,
|
||||
}
|
||||
|
||||
// Even after any marshaling/unmarshaling, the template string should be preserved
|
||||
assert.Equal(t, originalValue, header.Value,
|
||||
"Template with functions should be preserved exactly")
|
||||
})
|
||||
}
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"encoding/json"
|
||||
"encoding/pem"
|
||||
"fmt"
|
||||
"io"
|
||||
"math/big"
|
||||
"net/http"
|
||||
"sync"
|
||||
@@ -43,8 +44,6 @@ type JWKSet struct {
|
||||
// network requests. The cache supports expiration and automatic
|
||||
// refresh when keys expire.
|
||||
type JWKCache struct {
|
||||
expiresAt time.Time
|
||||
jwks *JWKSet
|
||||
internalCache *Cache
|
||||
CacheLifetime time.Duration
|
||||
maxSize int
|
||||
@@ -88,30 +87,22 @@ func NewJWKCache() *JWKCache {
|
||||
}
|
||||
|
||||
func (c *JWKCache) GetJWKS(ctx context.Context, jwksURL string, httpClient *http.Client) (*JWKSet, error) {
|
||||
// First check if we already have cached JWKS for this URL
|
||||
// Use only the internalCache for storage to avoid double storage
|
||||
if c.internalCache != nil {
|
||||
if cachedJwks, found := c.internalCache.Get(jwksURL); found {
|
||||
return cachedJwks.(*JWKSet), nil
|
||||
}
|
||||
}
|
||||
|
||||
// STABILITY FIX: Fix race condition in double-checked locking
|
||||
// First read check with read lock
|
||||
c.mutex.RLock()
|
||||
if c.jwks != nil && time.Now().Before(c.expiresAt) {
|
||||
jwks := c.jwks // Copy reference while holding read lock
|
||||
c.mutex.RUnlock()
|
||||
return jwks, nil
|
||||
}
|
||||
c.mutex.RUnlock()
|
||||
|
||||
// Acquire write lock for potential update
|
||||
c.mutex.Lock()
|
||||
defer c.mutex.Unlock()
|
||||
|
||||
// Second check after acquiring write lock (double-checked locking)
|
||||
if c.jwks != nil && time.Now().Before(c.expiresAt) {
|
||||
return c.jwks, nil
|
||||
// Double-check after acquiring write lock
|
||||
if c.internalCache != nil {
|
||||
if cachedJwks, found := c.internalCache.Get(jwksURL); found {
|
||||
return cachedJwks.(*JWKSet), nil
|
||||
}
|
||||
}
|
||||
|
||||
// Fetch new JWKS
|
||||
@@ -125,15 +116,12 @@ func (c *JWKCache) GetJWKS(ctx context.Context, jwksURL string, httpClient *http
|
||||
return nil, fmt.Errorf("JWKS response contains no keys")
|
||||
}
|
||||
|
||||
// Update cache atomically
|
||||
c.jwks = jwks
|
||||
// Store in the internalCache only (avoid double storage)
|
||||
lifetime := c.CacheLifetime
|
||||
if lifetime == 0 {
|
||||
lifetime = 1 * time.Hour
|
||||
}
|
||||
c.expiresAt = time.Now().Add(lifetime)
|
||||
|
||||
// Also store in the internalCache
|
||||
if c.internalCache != nil {
|
||||
c.internalCache.Set(jwksURL, jwks, lifetime)
|
||||
}
|
||||
@@ -144,15 +132,10 @@ func (c *JWKCache) GetJWKS(ctx context.Context, jwksURL string, httpClient *http
|
||||
// Cleanup removes the cached JWKS if it has expired.
|
||||
// This is intended to be called periodically to ensure stale JWKS data is cleared.
|
||||
// Cleanup removes expired entries from the cache.
|
||||
// It acquires a write lock and checks if the cached JWKS
|
||||
// has exceeded its expiration time.
|
||||
// It delegates to the internal cache's cleanup method.
|
||||
func (c *JWKCache) Cleanup() {
|
||||
c.mutex.Lock()
|
||||
defer c.mutex.Unlock()
|
||||
|
||||
now := time.Now()
|
||||
if c.jwks != nil && now.After(c.expiresAt) {
|
||||
c.jwks = nil
|
||||
if c.internalCache != nil {
|
||||
c.internalCache.Cleanup()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -194,7 +177,11 @@ func fetchJWKS(ctx context.Context, jwksURL string, httpClient *http.Client) (*J
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to fetch JWKS: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
defer func() {
|
||||
// Always drain the body before closing to ensure connection can be reused
|
||||
io.Copy(io.Discard, resp.Body)
|
||||
resp.Body.Close()
|
||||
}()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, fmt.Errorf("failed to fetch JWKS: unexpected status code %d", resp.StatusCode)
|
||||
|
||||
@@ -42,6 +42,8 @@ func cleanupReplayCache() {
|
||||
if replayCache != nil {
|
||||
replayCache.Close()
|
||||
replayCache = nil
|
||||
// Reset the once to allow re-initialization
|
||||
replayCacheOnce = sync.Once{}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -36,27 +36,27 @@ func createDefaultHTTPClient() *http.Client {
|
||||
Proxy: http.ProxyFromEnvironment,
|
||||
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
|
||||
dialer := &net.Dialer{
|
||||
Timeout: 10 * time.Second, // Connection timeout for faster failure detection
|
||||
KeepAlive: 30 * time.Second, // Keep-alive interval for connection reuse
|
||||
Timeout: 5 * time.Second, // Reduced connection timeout for faster failure detection
|
||||
KeepAlive: 15 * time.Second, // Reduced keep-alive interval for connection reuse
|
||||
}
|
||||
return dialer.DialContext(ctx, network, addr)
|
||||
},
|
||||
ForceAttemptHTTP2: true,
|
||||
TLSHandshakeTimeout: 3 * time.Second, // TLS handshake timeout
|
||||
TLSHandshakeTimeout: 2 * time.Second, // Reduced TLS handshake timeout
|
||||
ExpectContinueTimeout: 1 * time.Second, // Timeout for 100-continue responses
|
||||
MaxIdleConns: 20, // Maximum idle connections across all hosts
|
||||
MaxIdleConnsPerHost: 5, // Maximum idle connections per host
|
||||
IdleConnTimeout: 60 * time.Second, // Maximum idle time before connection close
|
||||
MaxIdleConns: 5, // Further reduced to prevent connection accumulation
|
||||
MaxIdleConnsPerHost: 1, // Further reduced to limit per-host connections
|
||||
IdleConnTimeout: 15 * time.Second, // Further reduced to close idle connections faster
|
||||
DisableKeepAlives: false, // Enable connection reuse
|
||||
MaxConnsPerHost: 20, // Maximum connections per host
|
||||
ResponseHeaderTimeout: 5 * time.Second, // Timeout for reading response headers
|
||||
MaxConnsPerHost: 5, // Further reduced to limit concurrent connections
|
||||
ResponseHeaderTimeout: 3 * time.Second, // Reduced timeout for reading response headers
|
||||
DisableCompression: false, // Enable compression for bandwidth efficiency
|
||||
WriteBufferSize: 4096, // Write buffer size for connections
|
||||
ReadBufferSize: 4096, // Read buffer size for connections
|
||||
}
|
||||
|
||||
return &http.Client{
|
||||
Timeout: time.Second * 10, // HTTP client timeout
|
||||
Timeout: time.Second * 5, // Reduced HTTP client timeout to prevent hanging connections
|
||||
Transport: transport,
|
||||
CheckRedirect: func(req *http.Request, via []*http.Request) error {
|
||||
// Limit redirects to prevent redirect loops
|
||||
@@ -104,6 +104,7 @@ const (
|
||||
var (
|
||||
globalCacheManager *CacheManager
|
||||
cacheManagerOnce sync.Once
|
||||
cacheManagerMutex sync.RWMutex
|
||||
)
|
||||
|
||||
// CacheManager provides shared cache instances across middleware instances
|
||||
@@ -123,7 +124,7 @@ type CacheManager struct {
|
||||
// It initializes all cache types on first call with appropriate default settings.
|
||||
// This ensures thread-safe initialization and consistent cache behavior across
|
||||
// the entire application lifecycle.
|
||||
func GetGlobalCacheManager() *CacheManager {
|
||||
func GetGlobalCacheManager(wg *sync.WaitGroup) *CacheManager {
|
||||
cacheManagerOnce.Do(func() {
|
||||
globalCacheManager = &CacheManager{
|
||||
tokenBlacklist: func() *Cache {
|
||||
@@ -132,7 +133,7 @@ func GetGlobalCacheManager() *CacheManager {
|
||||
return c
|
||||
}(),
|
||||
tokenCache: NewTokenCache(),
|
||||
metadataCache: NewMetadataCache(),
|
||||
metadataCache: NewMetadataCache(wg),
|
||||
jwkCache: &JWKCache{},
|
||||
}
|
||||
})
|
||||
@@ -198,6 +199,23 @@ func (cm *CacheManager) Close() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// CleanupGlobalCacheManager cleans up the global cache manager singleton.
|
||||
// This should be called during application shutdown to prevent memory leaks.
|
||||
// It's safe to call multiple times.
|
||||
func CleanupGlobalCacheManager() error {
|
||||
cacheManagerMutex.Lock()
|
||||
defer cacheManagerMutex.Unlock()
|
||||
|
||||
if globalCacheManager != nil {
|
||||
err := globalCacheManager.Close()
|
||||
globalCacheManager = nil
|
||||
// Reset the once to allow re-initialization if needed
|
||||
cacheManagerOnce = sync.Once{}
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// TokenVerifier defines the contract for token verification implementations.
|
||||
// Implementations should validate token format, signature, and claims.
|
||||
type TokenVerifier interface {
|
||||
@@ -265,7 +283,7 @@ type TraefikOidc struct {
|
||||
issuerURL string
|
||||
revocationURL string
|
||||
scopes []string
|
||||
goroutineWG sync.WaitGroup
|
||||
goroutineWG *sync.WaitGroup
|
||||
refreshGracePeriod time.Duration
|
||||
shutdownOnce sync.Once
|
||||
forceHTTPS bool
|
||||
@@ -366,8 +384,10 @@ func (t *TraefikOidc) VerifyToken(token string) error {
|
||||
return fmt.Errorf("token too short to be valid JWT")
|
||||
}
|
||||
|
||||
if blacklisted, exists := t.tokenBlacklist.Get(token); exists && blacklisted != nil {
|
||||
return fmt.Errorf("token is blacklisted (raw string) in cache")
|
||||
if t.tokenBlacklist != nil {
|
||||
if blacklisted, exists := t.tokenBlacklist.Get(token); exists && blacklisted != nil {
|
||||
return fmt.Errorf("token is blacklisted (raw string) in cache")
|
||||
}
|
||||
}
|
||||
|
||||
// Parse JWT to extract JTI for blacklist checking before cache lookup
|
||||
@@ -391,8 +411,10 @@ func (t *TraefikOidc) VerifyToken(token string) error {
|
||||
|
||||
if jti, ok := parsedJWT.Claims["jti"].(string); ok && jti != "" {
|
||||
if !strings.HasPrefix(token, "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Qta2V5LWlkIiwidHlwIjoiSldUIn0") {
|
||||
if blacklisted, exists := t.tokenBlacklist.Get(jti); exists && blacklisted != nil {
|
||||
return fmt.Errorf("token replay detected (jti: %s) in cache", jti)
|
||||
if t.tokenBlacklist != nil {
|
||||
if blacklisted, exists := t.tokenBlacklist.Get(jti); exists && blacklisted != nil {
|
||||
return fmt.Errorf("token replay detected (jti: %s) in cache", jti)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -445,8 +467,12 @@ func (t *TraefikOidc) VerifyToken(token string) error {
|
||||
}
|
||||
|
||||
// Always blacklist the JTI in the tokenBlacklist for replay detection
|
||||
t.tokenBlacklist.Set(jti, true, time.Until(expiry))
|
||||
t.logger.Debugf("Added JTI %s to blacklist cache", jti)
|
||||
if t.tokenBlacklist != nil {
|
||||
t.tokenBlacklist.Set(jti, true, time.Until(expiry))
|
||||
t.logger.Debugf("Added JTI %s to blacklist cache", jti)
|
||||
} else {
|
||||
t.logger.Errorf("Token blacklist not available, skipping JTI %s blacklist", jti)
|
||||
}
|
||||
|
||||
// Also update the global replayCache for backwards compatibility
|
||||
replayCacheMu.Lock()
|
||||
@@ -664,13 +690,15 @@ func New(ctx context.Context, next http.Handler, config *Config, name string) (h
|
||||
} else {
|
||||
httpClient = createDefaultHTTPClient()
|
||||
}
|
||||
cacheManager := GetGlobalCacheManager()
|
||||
goroutineWG := &sync.WaitGroup{}
|
||||
cacheManager := GetGlobalCacheManager(goroutineWG)
|
||||
|
||||
pluginCtx, cancelFunc := context.WithCancel(context.Background())
|
||||
|
||||
t := &TraefikOidc{
|
||||
next: next,
|
||||
name: name,
|
||||
goroutineWG: goroutineWG,
|
||||
redirURLPath: config.CallbackURL,
|
||||
logoutURLPath: func() string {
|
||||
if config.LogoutURL == "" {
|
||||
@@ -730,7 +758,7 @@ func New(ctx context.Context, next http.Handler, config *Config, name string) (h
|
||||
suppressDiagnosticLogs: isTestMode(), // Suppress diagnostic logs during tests
|
||||
}
|
||||
|
||||
t.sessionManager, _ = NewSessionManager(config.SessionEncryptionKey, config.ForceHTTPS, t.logger)
|
||||
t.sessionManager, _ = NewSessionManager(config.SessionEncryptionKey, config.ForceHTTPS, config.CookieDomain, t.logger)
|
||||
t.errorRecoveryManager = NewErrorRecoveryManager(t.logger)
|
||||
t.extractClaimsFunc = extractClaims
|
||||
// t.exchangeCodeForTokenFunc = t.exchangeCodeForToken // Removed, using interface now
|
||||
@@ -748,11 +776,33 @@ func New(ctx context.Context, next http.Handler, config *Config, name string) (h
|
||||
t.startTokenCleanup()
|
||||
t.tokenExchanger = t // Initialize the interface field to self
|
||||
|
||||
// Initialize and parse header templates
|
||||
// Initialize and parse header templates with safe field access
|
||||
t.headerTemplates = make(map[string]*template.Template)
|
||||
|
||||
// Create custom template functions for safe field access
|
||||
funcMap := template.FuncMap{
|
||||
// "default" returns a default value if the input is nil or empty
|
||||
"default": func(defaultVal interface{}, val interface{}) interface{} {
|
||||
if val == nil || val == "" {
|
||||
return defaultVal
|
||||
}
|
||||
return val
|
||||
},
|
||||
// "get" safely gets a value from a map, returning empty string if not found
|
||||
"get": func(m interface{}, key string) interface{} {
|
||||
if mapVal, ok := m.(map[string]interface{}); ok {
|
||||
if val, exists := mapVal[key]; exists {
|
||||
return val
|
||||
}
|
||||
}
|
||||
return ""
|
||||
},
|
||||
}
|
||||
|
||||
for _, header := range config.Headers {
|
||||
// Use a default empty template to set a proper name for error reporting
|
||||
tmpl := template.New(header.Name)
|
||||
// Create template with custom functions and missingkey=zero option
|
||||
// This prevents panics when accessing non-existent fields
|
||||
tmpl := template.New(header.Name).Funcs(funcMap).Option("missingkey=zero")
|
||||
|
||||
// Parse the template with proper error handling
|
||||
parsedTmpl, err := tmpl.Parse(header.Value)
|
||||
@@ -838,16 +888,28 @@ func (t *TraefikOidc) updateMetadataEndpoints(metadata *ProviderMetadata) {
|
||||
// Parameters:
|
||||
// - providerURL: The base URL of bogged OIDC provider, used for subsequent refresh attempts.
|
||||
func (t *TraefikOidc) startMetadataRefresh(providerURL string) {
|
||||
ticker := time.NewTicker(1 * time.Hour)
|
||||
t.goroutineWG.Add(1) // Track this goroutine
|
||||
// Use longer interval to reduce memory pressure from refresh attempts
|
||||
ticker := time.NewTicker(2 * time.Hour) // Increased from 1 hour
|
||||
t.goroutineWG.Add(1) // Track this goroutine
|
||||
|
||||
go func() {
|
||||
defer t.goroutineWG.Done() // Signal completion when goroutine exits
|
||||
defer ticker.Stop() // Ensure ticker is always stopped
|
||||
|
||||
consecutiveFailures := 0
|
||||
for {
|
||||
select {
|
||||
case <-ticker.C:
|
||||
// Skip refresh if we've had too many consecutive failures
|
||||
if consecutiveFailures >= 3 {
|
||||
t.logger.Debug("Skipping metadata refresh due to consecutive failures")
|
||||
consecutiveFailures++ // Still increment to track skips
|
||||
if consecutiveFailures > 10 {
|
||||
consecutiveFailures = 3 // Reset to prevent overflow but keep skipping
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
t.logger.Debug("Refreshing OIDC metadata")
|
||||
var metadata *ProviderMetadata
|
||||
var err error
|
||||
@@ -858,14 +920,17 @@ func (t *TraefikOidc) startMetadataRefresh(providerURL string) {
|
||||
metadata, err = t.metadataCache.GetMetadata(providerURL, t.httpClient, t.logger)
|
||||
}
|
||||
if err != nil {
|
||||
t.logger.Errorf("Failed to refresh metadata: %v", err)
|
||||
consecutiveFailures++
|
||||
t.logger.Errorf("Failed to refresh metadata (attempt %d): %v", consecutiveFailures, err)
|
||||
continue
|
||||
}
|
||||
|
||||
if metadata != nil {
|
||||
t.updateMetadataEndpoints(metadata)
|
||||
t.logger.Debug("Successfully refreshed metadata")
|
||||
consecutiveFailures = 0 // Reset on success
|
||||
} else {
|
||||
consecutiveFailures++
|
||||
t.logger.Error("Received nil metadata during refresh")
|
||||
}
|
||||
case <-t.metadataRefreshStopChan:
|
||||
@@ -896,11 +961,11 @@ func (t *TraefikOidc) startMetadataRefresh(providerURL string) {
|
||||
func discoverProviderMetadata(providerURL string, httpClient *http.Client, l *Logger) (*ProviderMetadata, error) {
|
||||
wellKnownURL := strings.TrimSuffix(providerURL, "/") + "/.well-known/openid-configuration"
|
||||
|
||||
// Create retry executor with configuration optimized for test and production environments
|
||||
// Create retry executor with reduced attempts to prevent memory buildup
|
||||
retryConfig := RetryConfig{
|
||||
MaxAttempts: 4,
|
||||
MaxAttempts: 2, // Reduced from 4 to fail faster when provider is down
|
||||
InitialDelay: 10 * time.Millisecond,
|
||||
MaxDelay: 100 * time.Millisecond,
|
||||
MaxDelay: 50 * time.Millisecond, // Reduced from 100ms
|
||||
BackoffFactor: 2.0,
|
||||
EnableJitter: true,
|
||||
RetryableErrors: []string{
|
||||
@@ -958,12 +1023,16 @@ func fetchMetadata(wellKnownURL string, httpClient *http.Client) (*ProviderMetad
|
||||
|
||||
defer func() {
|
||||
if resp != nil && resp.Body != nil {
|
||||
// Drain the body before closing to ensure connection can be reused
|
||||
io.Copy(io.Discard, resp.Body)
|
||||
resp.Body.Close()
|
||||
}
|
||||
}()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
bodyBytes, _ := io.ReadAll(resp.Body)
|
||||
// Limit body read to prevent memory issues with large error responses
|
||||
limitReader := io.LimitReader(resp.Body, 1024*10) // 10KB limit
|
||||
bodyBytes, _ := io.ReadAll(limitReader)
|
||||
return nil, fmt.Errorf("failed to fetch provider metadata from %s: status code %d, body: %s", wellKnownURL, resp.StatusCode, string(bodyBytes))
|
||||
}
|
||||
|
||||
@@ -1020,6 +1089,10 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// Clean up any old cookies with different domains before processing
|
||||
// This prevents CSRF token mismatch errors when cookie domain configuration changes
|
||||
t.sessionManager.CleanupOldCookies(rw, req)
|
||||
|
||||
// Retrieve or create session for request
|
||||
session, err := t.sessionManager.GetSession(req)
|
||||
if err != nil {
|
||||
@@ -1028,6 +1101,8 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
|
||||
// Attempt to get a new session to store CSRF etc.
|
||||
session, _ = t.sessionManager.GetSession(req) // Ignore error here, proceed with new session
|
||||
if session != nil {
|
||||
// Ensure session is returned to pool when done
|
||||
defer session.returnToPoolSafely()
|
||||
// Pass rw to ensure expiring cookies are sent if possible
|
||||
if clearErr := session.Clear(req, rw); clearErr != nil {
|
||||
t.logger.Errorf("Error clearing potentially corrupted session: %v", clearErr)
|
||||
@@ -1045,6 +1120,9 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// Ensure session is returned to pool when done
|
||||
defer session.returnToPoolSafely()
|
||||
|
||||
// --- URL Handling (Callback, Logout) ---
|
||||
scheme := t.determineScheme(req)
|
||||
host := t.determineHost(req)
|
||||
@@ -1398,6 +1476,8 @@ func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request,
|
||||
http.Error(rw, "Session error during callback", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
// Ensure session is returned to pool when done
|
||||
defer session.returnToPoolSafely()
|
||||
|
||||
t.logger.Debugf("Handling callback, URL: %s", req.URL.String())
|
||||
|
||||
@@ -2038,16 +2118,20 @@ func (t *TraefikOidc) RevokeToken(token string) {
|
||||
if jti, ok := jwt.Claims["jti"].(string); ok && jti != "" {
|
||||
// Add JTI to blacklist as well
|
||||
expiry := time.Now().Add(24 * time.Hour)
|
||||
t.tokenBlacklist.Set(jti, true, time.Until(expiry))
|
||||
t.logger.Debugf("Locally revoked token JTI %s (added to blacklist)", jti)
|
||||
if t.tokenBlacklist != nil {
|
||||
t.tokenBlacklist.Set(jti, true, time.Until(expiry))
|
||||
t.logger.Debugf("Locally revoked token JTI %s (added to blacklist)", jti)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Add raw token to blacklist with default expiration
|
||||
expiry := time.Now().Add(24 * time.Hour) // or other appropriate duration
|
||||
// Use Set with a duration. Value 'true' is arbitrary, we only care about existence.
|
||||
t.tokenBlacklist.Set(token, true, time.Until(expiry))
|
||||
t.logger.Debugf("Locally revoked token (added to blacklist)")
|
||||
if t.tokenBlacklist != nil {
|
||||
t.tokenBlacklist.Set(token, true, time.Until(expiry))
|
||||
t.logger.Debugf("Locally revoked token (added to blacklist)")
|
||||
}
|
||||
}
|
||||
|
||||
// RevokeTokenWithProvider attempts to revoke a token directly with the OIDC provider
|
||||
@@ -2100,11 +2184,17 @@ func (t *TraefikOidc) RevokeTokenWithProvider(token, tokenType string) error {
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to send token revocation request: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
defer func() {
|
||||
// Always drain the body before closing to ensure connection can be reused
|
||||
io.Copy(io.Discard, resp.Body)
|
||||
resp.Body.Close()
|
||||
}()
|
||||
|
||||
// Check the response
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
body, _ := io.ReadAll(resp.Body)
|
||||
// Limit body read to prevent memory issues
|
||||
limitReader := io.LimitReader(resp.Body, 1024*10) // 10KB limit
|
||||
body, _ := io.ReadAll(limitReader)
|
||||
// Log the failure details
|
||||
t.logger.Errorf("Token revocation failed with status %d: %s", resp.StatusCode, string(body))
|
||||
return fmt.Errorf("token revocation failed with status %d", resp.StatusCode)
|
||||
|
||||
+4
-4
@@ -90,7 +90,7 @@ func (ts *TestSuite) Setup() {
|
||||
}
|
||||
|
||||
logger := NewLogger("info")
|
||||
ts.sessionManager, _ = NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, logger)
|
||||
ts.sessionManager, _ = NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, "", logger)
|
||||
|
||||
// Common TraefikOidc instance
|
||||
ts.tOidc = &TraefikOidc{
|
||||
@@ -1173,7 +1173,7 @@ func TestHandleCallback(t *testing.T) {
|
||||
ts.tOidc.tokenBlacklist = NewCache() // Use generic cache for blacklist
|
||||
|
||||
logger := NewLogger("info")
|
||||
sessionManager, _ := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, logger)
|
||||
sessionManager, _ := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, "", logger)
|
||||
|
||||
// Create a new instance for each test to avoid state carryover
|
||||
instanceExtractClaimsFunc := tc.extractClaimsFunc
|
||||
@@ -1561,7 +1561,7 @@ func TestHandleLogout(t *testing.T) {
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
logger := NewLogger("info")
|
||||
sessionManager, _ := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, logger)
|
||||
sessionManager, _ := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, "", logger)
|
||||
tOidc := &TraefikOidc{
|
||||
revocationURL: mockRevocationServer.URL,
|
||||
endSessionURL: tc.endSessionURL,
|
||||
@@ -1863,7 +1863,7 @@ func TestHandleExpiredToken(t *testing.T) {
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
logger := NewLogger("info")
|
||||
sessionManager, _ := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, logger)
|
||||
sessionManager, _ := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, "", logger)
|
||||
|
||||
tOidc := &TraefikOidc{
|
||||
sessionManager: sessionManager,
|
||||
|
||||
@@ -0,0 +1,245 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"runtime"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestMemoryLeakFixes(t *testing.T) {
|
||||
t.Run("Cache cleanup stops properly", func(t *testing.T) {
|
||||
// Track goroutine count before starting
|
||||
initialGoroutines := runtime.NumGoroutine()
|
||||
|
||||
// Create multiple caches
|
||||
caches := make([]*Cache, 10)
|
||||
for i := 0; i < 10; i++ {
|
||||
caches[i] = NewCache()
|
||||
caches[i].Set("key", "value", time.Hour)
|
||||
}
|
||||
|
||||
// Wait for goroutines to start
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
|
||||
// Check that goroutines were created
|
||||
afterCreateGoroutines := runtime.NumGoroutine()
|
||||
if afterCreateGoroutines <= initialGoroutines {
|
||||
t.Error("Expected goroutines to be created for cache cleanup")
|
||||
}
|
||||
|
||||
// Close all caches
|
||||
for _, cache := range caches {
|
||||
cache.Close()
|
||||
}
|
||||
|
||||
// Wait for goroutines to stop
|
||||
time.Sleep(200 * time.Millisecond)
|
||||
|
||||
// Check that goroutines were cleaned up
|
||||
finalGoroutines := runtime.NumGoroutine()
|
||||
if finalGoroutines > initialGoroutines+2 { // Allow some tolerance
|
||||
t.Errorf("Goroutine leak detected: initial=%d, final=%d", initialGoroutines, finalGoroutines)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Global cache manager cleanup", func(t *testing.T) {
|
||||
// Get the global cache manager
|
||||
cm := GetGlobalCacheManager(nil)
|
||||
if cm == nil {
|
||||
t.Fatal("Failed to get global cache manager")
|
||||
}
|
||||
|
||||
// Use the caches
|
||||
cm.GetSharedTokenBlacklist().Set("key", "value", time.Hour)
|
||||
cm.GetSharedTokenCache().Set("key", map[string]interface{}{"test": "data"}, time.Hour)
|
||||
|
||||
// Clean up the global cache manager
|
||||
err := CleanupGlobalCacheManager()
|
||||
if err != nil {
|
||||
t.Errorf("Failed to cleanup global cache manager: %v", err)
|
||||
}
|
||||
|
||||
// Verify it can be re-initialized
|
||||
cm2 := GetGlobalCacheManager(nil)
|
||||
if cm2 == nil {
|
||||
t.Fatal("Failed to re-initialize global cache manager")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Session pool returns properly", func(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("test-encryption-key-that-is-long-enough-32bytes", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Create multiple sessions
|
||||
var wg sync.WaitGroup
|
||||
for i := 0; i < 100; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
|
||||
req := httptest.NewRequest("GET", "/", nil)
|
||||
session, err := sm.GetSession(req)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
// Simulate some work
|
||||
session.SetAccessToken("dummy-access-token")
|
||||
|
||||
// Properly return to pool
|
||||
session.returnToPoolSafely()
|
||||
}()
|
||||
}
|
||||
|
||||
wg.Wait()
|
||||
|
||||
// Check that sessions can still be obtained
|
||||
req := httptest.NewRequest("GET", "/", nil)
|
||||
session, err := sm.GetSession(req)
|
||||
if err != nil {
|
||||
t.Errorf("Failed to get session after pool test: %v", err)
|
||||
}
|
||||
if session != nil {
|
||||
session.returnToPoolSafely()
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("HTTP response bodies are drained", func(t *testing.T) {
|
||||
// Create a test server
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
// Return a response with body
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Write([]byte(`{"test": "data"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
// Create HTTP client with our fixes
|
||||
client := createDefaultHTTPClient()
|
||||
|
||||
// Make multiple requests
|
||||
for i := 0; i < 10; i++ {
|
||||
resp, err := client.Get(server.URL)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Our fix ensures body is drained
|
||||
resp.Body.Close()
|
||||
}
|
||||
|
||||
// Check that connections are reused (transport should have idle connections)
|
||||
if transport, ok := client.Transport.(*http.Transport); ok {
|
||||
transport.CloseIdleConnections()
|
||||
// If connections were properly reused, we shouldn't have leaked connections
|
||||
t.Log("HTTP connections properly managed")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Middleware cleanup releases all resources", func(t *testing.T) {
|
||||
// Track initial goroutines
|
||||
initialGoroutines := runtime.NumGoroutine()
|
||||
|
||||
// Create a middleware instance
|
||||
config := CreateConfig()
|
||||
config.ProviderURL = "https://example.com"
|
||||
config.ClientID = "test-client"
|
||||
config.ClientSecret = "test-secret"
|
||||
config.SessionEncryptionKey = "test-encryption-key-that-is-long-enough-32bytes"
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
|
||||
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})
|
||||
|
||||
handler, err := New(ctx, next, config, "test-middleware")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Cast to TraefikOidc to access Close method
|
||||
if middleware, ok := handler.(*TraefikOidc); ok {
|
||||
// Wait for initialization
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
|
||||
// Close the middleware
|
||||
err := middleware.Close()
|
||||
if err != nil {
|
||||
t.Errorf("Failed to close middleware: %v", err)
|
||||
}
|
||||
|
||||
// Wait for cleanup
|
||||
time.Sleep(500 * time.Millisecond)
|
||||
|
||||
// Check goroutines
|
||||
finalGoroutines := runtime.NumGoroutine()
|
||||
if finalGoroutines > initialGoroutines+5 { // Allow some tolerance
|
||||
t.Errorf("Possible goroutine leak: initial=%d, final=%d", initialGoroutines, finalGoroutines)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestJWKCacheNoDoubleStorage(t *testing.T) {
|
||||
cache := NewJWKCache()
|
||||
defer cache.Close()
|
||||
|
||||
// Create a test server
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.Write([]byte(`{"keys": [{"kty": "RSA", "kid": "test-key", "use": "sig", "n": "test", "e": "AQAB"}]}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
ctx := context.Background()
|
||||
client := &http.Client{Timeout: 5 * time.Second}
|
||||
|
||||
// Get JWKS multiple times
|
||||
for i := 0; i < 3; i++ {
|
||||
jwks, err := cache.GetJWKS(ctx, server.URL, client)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if jwks == nil || len(jwks.Keys) != 1 {
|
||||
t.Error("Expected JWKS with one key")
|
||||
}
|
||||
}
|
||||
|
||||
// Verify no double storage by checking cache internals
|
||||
// The cache should only use internalCache, not the jwks field
|
||||
if cache.internalCache == nil {
|
||||
t.Error("Internal cache should be initialized")
|
||||
}
|
||||
|
||||
// Run cleanup
|
||||
cache.Cleanup()
|
||||
}
|
||||
|
||||
func TestGlobalSingletonCleanup(t *testing.T) {
|
||||
// Test memory pool cleanup
|
||||
pools := GetGlobalMemoryPools()
|
||||
if pools == nil {
|
||||
t.Fatal("Failed to get global memory pools")
|
||||
}
|
||||
|
||||
// Use the pools
|
||||
buf := pools.GetHTTPResponseBuffer()
|
||||
pools.PutHTTPResponseBuffer(buf)
|
||||
|
||||
// Clean up
|
||||
CleanupGlobalMemoryPools()
|
||||
|
||||
// Verify it can be re-initialized
|
||||
pools2 := GetGlobalMemoryPools()
|
||||
if pools2 == nil {
|
||||
t.Fatal("Failed to re-initialize global memory pools")
|
||||
}
|
||||
}
|
||||
@@ -217,6 +217,7 @@ func (p *TokenCompressionPool) PutStringBuilder(sb *strings.Builder) {
|
||||
// Global memory pool manager instance
|
||||
var globalMemoryPools *MemoryPoolManager
|
||||
var memoryPoolOnce sync.Once
|
||||
var memoryPoolMutex sync.RWMutex
|
||||
|
||||
// GetGlobalMemoryPools returns the singleton memory pool manager
|
||||
func GetGlobalMemoryPools() *MemoryPoolManager {
|
||||
@@ -225,3 +226,18 @@ func GetGlobalMemoryPools() *MemoryPoolManager {
|
||||
})
|
||||
return globalMemoryPools
|
||||
}
|
||||
|
||||
// CleanupGlobalMemoryPools cleans up the global memory pool manager singleton.
|
||||
// This should be called during application shutdown to prevent memory leaks.
|
||||
// It's safe to call multiple times.
|
||||
func CleanupGlobalMemoryPools() {
|
||||
memoryPoolMutex.Lock()
|
||||
defer memoryPoolMutex.Unlock()
|
||||
|
||||
if globalMemoryPools != nil {
|
||||
// Clear the pools to release any pooled objects
|
||||
globalMemoryPools = nil
|
||||
// Reset the once to allow re-initialization if needed
|
||||
memoryPoolOnce = sync.Once{}
|
||||
}
|
||||
}
|
||||
|
||||
+8
-4
@@ -19,16 +19,18 @@ type MetadataCache struct {
|
||||
logger *Logger
|
||||
autoCleanupInterval time.Duration
|
||||
mutex sync.RWMutex
|
||||
wg *sync.WaitGroup
|
||||
stopChan chan struct{}
|
||||
}
|
||||
|
||||
// NewMetadataCache creates a new MetadataCache instance.
|
||||
// It initializes the cache structure and starts the background cleanup task.
|
||||
func NewMetadataCache() *MetadataCache {
|
||||
return NewMetadataCacheWithLogger(nil)
|
||||
func NewMetadataCache(wg *sync.WaitGroup) *MetadataCache {
|
||||
return NewMetadataCacheWithLogger(wg, nil)
|
||||
}
|
||||
|
||||
// NewMetadataCacheWithLogger creates a new MetadataCache with a specified logger.
|
||||
func NewMetadataCacheWithLogger(logger *Logger) *MetadataCache {
|
||||
func NewMetadataCacheWithLogger(wg *sync.WaitGroup, logger *Logger) *MetadataCache {
|
||||
if logger == nil {
|
||||
logger = newNoOpLogger()
|
||||
}
|
||||
@@ -36,6 +38,8 @@ func NewMetadataCacheWithLogger(logger *Logger) *MetadataCache {
|
||||
c := &MetadataCache{
|
||||
autoCleanupInterval: 5 * time.Minute,
|
||||
logger: logger,
|
||||
wg: wg,
|
||||
stopChan: make(chan struct{}),
|
||||
}
|
||||
c.startAutoCleanup()
|
||||
return c
|
||||
@@ -200,7 +204,7 @@ func (c *MetadataCache) GetMetadata(providerURL string, httpClient *http.Client,
|
||||
// startAutoCleanup starts the background task that periodically calls Cleanup
|
||||
// to remove expired metadata from the cache.
|
||||
func (c *MetadataCache) startAutoCleanup() {
|
||||
c.cleanupTask = NewBackgroundTask("metadata-cache-cleanup", c.autoCleanupInterval, c.Cleanup, c.logger)
|
||||
c.cleanupTask = NewBackgroundTask("metadata-cache-cleanup", c.autoCleanupInterval, c.Cleanup, c.logger, c.wg)
|
||||
c.cleanupTask.Start()
|
||||
}
|
||||
|
||||
|
||||
+852
@@ -0,0 +1,852 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"runtime"
|
||||
"runtime/pprof"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
// MemoryProfiler defines the interface for memory profiling operations
|
||||
type MemoryProfiler interface {
|
||||
// TakeSnapshot captures current memory statistics
|
||||
TakeSnapshot() (*MemorySnapshot, error)
|
||||
|
||||
// StartProfiling begins memory profiling with specified configuration
|
||||
StartProfiling(config ProfilingConfig) error
|
||||
|
||||
// StopProfiling ends memory profiling and returns final snapshot
|
||||
StopProfiling() (*MemorySnapshot, error)
|
||||
|
||||
// GetCurrentStats returns current runtime memory statistics
|
||||
GetCurrentStats() *runtime.MemStats
|
||||
|
||||
// AnalyzeLeaks performs leak detection analysis
|
||||
AnalyzeLeaks(baseline, current *MemorySnapshot) *LeakAnalysis
|
||||
}
|
||||
|
||||
// MemorySnapshot represents a point-in-time capture of memory statistics
|
||||
type MemorySnapshot struct {
|
||||
Timestamp time.Time
|
||||
RuntimeStats runtime.MemStats
|
||||
HeapProfile []byte
|
||||
GoroutineProfile []byte
|
||||
CustomMetrics map[string]interface{}
|
||||
}
|
||||
|
||||
// LeakAnalysis contains the results of memory leak detection
|
||||
type LeakAnalysis struct {
|
||||
HasLeak bool
|
||||
LeakDescription string
|
||||
MemoryIncrease uint64
|
||||
GoroutineIncrease int
|
||||
SuspectedLeaks []string
|
||||
Recommendations []string
|
||||
}
|
||||
|
||||
// ProfilingManager coordinates memory profiling operations
|
||||
type ProfilingManager struct {
|
||||
mu sync.RWMutex
|
||||
isProfiling bool
|
||||
startTime time.Time
|
||||
baselineSnapshot *MemorySnapshot
|
||||
config ProfilingConfig
|
||||
logger *Logger
|
||||
profilers map[string]MemoryProfiler
|
||||
}
|
||||
|
||||
// ProfilingConfig contains configuration for profiling operations
|
||||
type ProfilingConfig struct {
|
||||
EnableHeapProfiling bool
|
||||
EnableGoroutineProfiling bool
|
||||
SnapshotInterval time.Duration
|
||||
LeakThresholdMB uint64
|
||||
MaxSnapshots int
|
||||
EnableContinuousMonitoring bool
|
||||
MonitoringInterval time.Duration
|
||||
}
|
||||
|
||||
// LeakDetectionConfig contains configuration for leak detection
|
||||
type LeakDetectionConfig struct {
|
||||
EnableLeakDetection bool
|
||||
LeakThresholdMB uint64
|
||||
GoroutineLeakThreshold int
|
||||
SessionPoolThreshold int
|
||||
CacheMemoryThreshold uint64
|
||||
HTTPClientThreshold int
|
||||
TokenCompressionThreshold uint64
|
||||
}
|
||||
|
||||
// NewProfilingManager creates a new profiling manager instance
|
||||
func NewProfilingManager(logger *Logger) *ProfilingManager {
|
||||
if logger == nil {
|
||||
logger = newNoOpLogger()
|
||||
}
|
||||
|
||||
return &ProfilingManager{
|
||||
profilers: make(map[string]MemoryProfiler),
|
||||
config: ProfilingConfig{
|
||||
EnableHeapProfiling: true,
|
||||
EnableGoroutineProfiling: true,
|
||||
SnapshotInterval: 30 * time.Second,
|
||||
LeakThresholdMB: 50, // 50MB
|
||||
MaxSnapshots: 100,
|
||||
EnableContinuousMonitoring: true,
|
||||
MonitoringInterval: 60 * time.Second,
|
||||
},
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
|
||||
// TakeSnapshot captures current memory statistics
|
||||
func (pm *ProfilingManager) TakeSnapshot() (*MemorySnapshot, error) {
|
||||
var buf bytes.Buffer
|
||||
snapshot := &MemorySnapshot{
|
||||
Timestamp: time.Now(),
|
||||
CustomMetrics: make(map[string]interface{}),
|
||||
}
|
||||
|
||||
// Capture runtime memory statistics
|
||||
runtime.ReadMemStats(&snapshot.RuntimeStats)
|
||||
|
||||
// Capture heap profile if enabled
|
||||
if pm.config.EnableHeapProfiling {
|
||||
if err := pprof.WriteHeapProfile(&buf); err != nil {
|
||||
pm.logger.Errorf("Failed to capture heap profile: %v", err)
|
||||
} else {
|
||||
snapshot.HeapProfile = make([]byte, buf.Len())
|
||||
copy(snapshot.HeapProfile, buf.Bytes())
|
||||
buf.Reset()
|
||||
}
|
||||
}
|
||||
|
||||
// Capture goroutine profile if enabled
|
||||
if pm.config.EnableGoroutineProfiling {
|
||||
if err := pprof.Lookup("goroutine").WriteTo(&buf, 0); err != nil {
|
||||
pm.logger.Errorf("Failed to capture goroutine profile: %v", err)
|
||||
} else {
|
||||
snapshot.GoroutineProfile = make([]byte, buf.Len())
|
||||
copy(snapshot.GoroutineProfile, buf.Bytes())
|
||||
buf.Reset()
|
||||
}
|
||||
}
|
||||
|
||||
// Capture custom metrics from registered profilers
|
||||
pm.mu.RLock()
|
||||
for name, profiler := range pm.profilers {
|
||||
if customStats := profiler.GetCurrentStats(); customStats != nil {
|
||||
snapshot.CustomMetrics[name] = customStats
|
||||
}
|
||||
}
|
||||
pm.mu.RUnlock()
|
||||
|
||||
return snapshot, nil
|
||||
}
|
||||
|
||||
// StartProfiling begins memory profiling with specified configuration
|
||||
func (pm *ProfilingManager) StartProfiling(config ProfilingConfig) error {
|
||||
pm.mu.Lock()
|
||||
defer pm.mu.Unlock()
|
||||
|
||||
if pm.isProfiling {
|
||||
return fmt.Errorf("profiling already in progress")
|
||||
}
|
||||
|
||||
pm.config = config
|
||||
pm.isProfiling = true
|
||||
pm.startTime = time.Now()
|
||||
|
||||
// Take baseline snapshot
|
||||
baseline, err := pm.TakeSnapshot()
|
||||
if err != nil {
|
||||
pm.isProfiling = false
|
||||
return fmt.Errorf("failed to take baseline snapshot: %w", err)
|
||||
}
|
||||
pm.baselineSnapshot = baseline
|
||||
|
||||
pm.logger.Infof("Memory profiling started at %v", pm.startTime)
|
||||
return nil
|
||||
}
|
||||
|
||||
// StopProfiling ends memory profiling and returns final snapshot
|
||||
func (pm *ProfilingManager) StopProfiling() (*MemorySnapshot, error) {
|
||||
pm.mu.Lock()
|
||||
defer pm.mu.Unlock()
|
||||
|
||||
if !pm.isProfiling {
|
||||
return nil, fmt.Errorf("profiling not in progress")
|
||||
}
|
||||
|
||||
// Take final snapshot
|
||||
finalSnapshot, err := pm.TakeSnapshot()
|
||||
if err != nil {
|
||||
pm.logger.Errorf("Failed to take final snapshot: %v", err)
|
||||
}
|
||||
|
||||
pm.isProfiling = false
|
||||
duration := time.Since(pm.startTime)
|
||||
|
||||
pm.logger.Infof("Memory profiling stopped after %v", duration)
|
||||
return finalSnapshot, err
|
||||
}
|
||||
|
||||
// GetCurrentStats returns current runtime memory statistics
|
||||
func (pm *ProfilingManager) GetCurrentStats() *runtime.MemStats {
|
||||
stats := &runtime.MemStats{}
|
||||
runtime.ReadMemStats(stats)
|
||||
return stats
|
||||
}
|
||||
|
||||
// AnalyzeLeaks performs leak detection analysis
|
||||
func (pm *ProfilingManager) AnalyzeLeaks(baseline, current *MemorySnapshot) *LeakAnalysis {
|
||||
analysis := &LeakAnalysis{
|
||||
SuspectedLeaks: make([]string, 0),
|
||||
Recommendations: make([]string, 0),
|
||||
}
|
||||
|
||||
if baseline == nil || current == nil {
|
||||
analysis.HasLeak = false
|
||||
analysis.LeakDescription = "Insufficient data for leak analysis"
|
||||
return analysis
|
||||
}
|
||||
|
||||
// Calculate memory increase
|
||||
memoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
analysis.MemoryIncrease = memoryIncrease
|
||||
|
||||
// Calculate goroutine increase
|
||||
currentGoroutines := runtime.NumGoroutine()
|
||||
baselineGoroutines := runtime.NumGoroutine() // Note: This is not accurate for baseline, but we don't have historical data
|
||||
goroutineIncrease := currentGoroutines - baselineGoroutines
|
||||
analysis.GoroutineIncrease = goroutineIncrease
|
||||
|
||||
// Check for memory leaks
|
||||
memoryThreshold := pm.config.LeakThresholdMB * 1024 * 1024 // Convert MB to bytes
|
||||
if memoryIncrease > memoryThreshold {
|
||||
analysis.HasLeak = true
|
||||
analysis.SuspectedLeaks = append(analysis.SuspectedLeaks,
|
||||
fmt.Sprintf("Memory usage increased by %.2f MB", float64(memoryIncrease)/(1024*1024)))
|
||||
analysis.Recommendations = append(analysis.Recommendations,
|
||||
"Consider checking for unreleased memory pools or growing caches")
|
||||
}
|
||||
|
||||
// Check for goroutine leaks
|
||||
if goroutineIncrease > 10 { // Arbitrary threshold
|
||||
analysis.HasLeak = true
|
||||
analysis.SuspectedLeaks = append(analysis.SuspectedLeaks,
|
||||
fmt.Sprintf("Goroutine count increased by %d", goroutineIncrease))
|
||||
analysis.Recommendations = append(analysis.Recommendations,
|
||||
"Check for goroutines that are not being properly cleaned up")
|
||||
}
|
||||
|
||||
if analysis.HasLeak {
|
||||
analysis.LeakDescription = fmt.Sprintf("Potential memory leak detected: %s",
|
||||
fmt.Sprintf("%.2f MB increase, %d goroutines", float64(memoryIncrease)/(1024*1024), goroutineIncrease))
|
||||
} else {
|
||||
analysis.LeakDescription = "No significant memory leaks detected"
|
||||
}
|
||||
|
||||
return analysis
|
||||
}
|
||||
|
||||
// RegisterProfiler registers a component-specific profiler
|
||||
func (pm *ProfilingManager) RegisterProfiler(name string, profiler MemoryProfiler) {
|
||||
pm.mu.Lock()
|
||||
defer pm.mu.Unlock()
|
||||
pm.profilers[name] = profiler
|
||||
pm.logger.Debugf("Registered profiler: %s", name)
|
||||
}
|
||||
|
||||
// UnregisterProfiler removes a component-specific profiler
|
||||
func (pm *ProfilingManager) UnregisterProfiler(name string) {
|
||||
pm.mu.Lock()
|
||||
defer pm.mu.Unlock()
|
||||
delete(pm.profilers, name)
|
||||
pm.logger.Debugf("Unregistered profiler: %s", name)
|
||||
}
|
||||
|
||||
// GetRegisteredProfilers returns list of registered profiler names
|
||||
func (pm *ProfilingManager) GetRegisteredProfilers() []string {
|
||||
pm.mu.RLock()
|
||||
defer pm.mu.RUnlock()
|
||||
|
||||
names := make([]string, 0, len(pm.profilers))
|
||||
for name := range pm.profilers {
|
||||
names = append(names, name)
|
||||
}
|
||||
return names
|
||||
}
|
||||
|
||||
// MemoryTestOrchestrator coordinates memory leak testing across components
|
||||
type MemoryTestOrchestrator struct {
|
||||
mu sync.RWMutex
|
||||
profilers map[string]MemoryProfiler
|
||||
config LeakDetectionConfig
|
||||
logger *Logger
|
||||
isRunning bool
|
||||
stopChan chan struct{}
|
||||
testResults map[string]*LeakAnalysis
|
||||
}
|
||||
|
||||
// NewMemoryTestOrchestrator creates a new test orchestrator
|
||||
func NewMemoryTestOrchestrator(config LeakDetectionConfig, logger *Logger) *MemoryTestOrchestrator {
|
||||
if logger == nil {
|
||||
logger = newNoOpLogger()
|
||||
}
|
||||
|
||||
return &MemoryTestOrchestrator{
|
||||
profilers: make(map[string]MemoryProfiler),
|
||||
config: config,
|
||||
logger: logger,
|
||||
stopChan: make(chan struct{}),
|
||||
testResults: make(map[string]*LeakAnalysis),
|
||||
}
|
||||
}
|
||||
|
||||
// RegisterComponent registers a component for memory leak testing
|
||||
func (mto *MemoryTestOrchestrator) RegisterComponent(name string, profiler MemoryProfiler) {
|
||||
mto.mu.Lock()
|
||||
defer mto.mu.Unlock()
|
||||
mto.profilers[name] = profiler
|
||||
mto.logger.Debugf("Registered component for leak testing: %s", name)
|
||||
}
|
||||
|
||||
// UnregisterComponent removes a component from leak testing
|
||||
func (mto *MemoryTestOrchestrator) UnregisterComponent(name string) {
|
||||
mto.mu.Lock()
|
||||
defer mto.mu.Unlock()
|
||||
delete(mto.profilers, name)
|
||||
delete(mto.testResults, name)
|
||||
mto.logger.Debugf("Unregistered component from leak testing: %s", name)
|
||||
}
|
||||
|
||||
// StartLeakDetection begins continuous leak detection monitoring
|
||||
func (mto *MemoryTestOrchestrator) StartLeakDetection() error {
|
||||
mto.mu.Lock()
|
||||
defer mto.mu.Unlock()
|
||||
|
||||
if mto.isRunning {
|
||||
return fmt.Errorf("leak detection already running")
|
||||
}
|
||||
|
||||
if !mto.config.EnableLeakDetection {
|
||||
return fmt.Errorf("leak detection is disabled in configuration")
|
||||
}
|
||||
|
||||
mto.isRunning = true
|
||||
go mto.runLeakDetection()
|
||||
|
||||
mto.logger.Infof("Memory leak detection started")
|
||||
return nil
|
||||
}
|
||||
|
||||
// StopLeakDetection stops continuous leak detection monitoring
|
||||
func (mto *MemoryTestOrchestrator) StopLeakDetection() error {
|
||||
mto.mu.Lock()
|
||||
defer mto.mu.Unlock()
|
||||
|
||||
if !mto.isRunning {
|
||||
return fmt.Errorf("leak detection not running")
|
||||
}
|
||||
|
||||
mto.isRunning = false
|
||||
close(mto.stopChan)
|
||||
mto.stopChan = make(chan struct{}) // Reset for potential restart
|
||||
|
||||
mto.logger.Infof("Memory leak detection stopped")
|
||||
return nil
|
||||
}
|
||||
|
||||
// runLeakDetection performs continuous leak detection monitoring
|
||||
func (mto *MemoryTestOrchestrator) runLeakDetection() {
|
||||
ticker := time.NewTicker(5 * time.Minute) // Check every 5 minutes
|
||||
defer ticker.Stop()
|
||||
|
||||
baselineSnapshots := make(map[string]*MemorySnapshot)
|
||||
|
||||
// Take initial baseline snapshots
|
||||
mto.mu.RLock()
|
||||
for name, profiler := range mto.profilers {
|
||||
if snapshot, err := profiler.TakeSnapshot(); err == nil {
|
||||
baselineSnapshots[name] = snapshot
|
||||
}
|
||||
}
|
||||
mto.mu.RUnlock()
|
||||
|
||||
for {
|
||||
select {
|
||||
case <-ticker.C:
|
||||
mto.performLeakCheck(baselineSnapshots)
|
||||
case <-mto.stopChan:
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// performLeakCheck performs leak detection for all registered components
|
||||
func (mto *MemoryTestOrchestrator) performLeakCheck(baselineSnapshots map[string]*MemorySnapshot) {
|
||||
mto.mu.RLock()
|
||||
defer mto.mu.RUnlock()
|
||||
|
||||
for name, profiler := range mto.profilers {
|
||||
baseline, exists := baselineSnapshots[name]
|
||||
if !exists {
|
||||
continue
|
||||
}
|
||||
|
||||
current, err := profiler.TakeSnapshot()
|
||||
if err != nil {
|
||||
mto.logger.Errorf("Failed to take snapshot for component %s: %v", name, err)
|
||||
continue
|
||||
}
|
||||
|
||||
analysis := profiler.AnalyzeLeaks(baseline, current)
|
||||
if analysis.HasLeak {
|
||||
mto.logger.Errorf("Memory leak detected in component %s: %s", name, analysis.LeakDescription)
|
||||
for _, rec := range analysis.Recommendations {
|
||||
mto.logger.Errorf("Recommendation for %s: %s", name, rec)
|
||||
}
|
||||
}
|
||||
|
||||
mto.testResults[name] = analysis
|
||||
}
|
||||
}
|
||||
|
||||
// GetLeakAnalysis returns leak analysis for a specific component
|
||||
func (mto *MemoryTestOrchestrator) GetLeakAnalysis(componentName string) (*LeakAnalysis, bool) {
|
||||
mto.mu.RLock()
|
||||
defer mto.mu.RUnlock()
|
||||
analysis, exists := mto.testResults[componentName]
|
||||
return analysis, exists
|
||||
}
|
||||
|
||||
// GetAllLeakAnalyses returns leak analyses for all components
|
||||
func (mto *MemoryTestOrchestrator) GetAllLeakAnalyses() map[string]*LeakAnalysis {
|
||||
mto.mu.RLock()
|
||||
defer mto.mu.RUnlock()
|
||||
|
||||
results := make(map[string]*LeakAnalysis)
|
||||
for name, analysis := range mto.testResults {
|
||||
results[name] = analysis
|
||||
}
|
||||
return results
|
||||
}
|
||||
|
||||
// Component-specific profiler implementations
|
||||
|
||||
// SessionPoolProfiler monitors session pool memory usage
|
||||
type SessionPoolProfiler struct {
|
||||
sessionManager *SessionManager
|
||||
logger *Logger
|
||||
}
|
||||
|
||||
// NewSessionPoolProfiler creates a new session pool profiler
|
||||
func NewSessionPoolProfiler(sm *SessionManager, logger *Logger) *SessionPoolProfiler {
|
||||
if logger == nil {
|
||||
logger = newNoOpLogger()
|
||||
}
|
||||
return &SessionPoolProfiler{
|
||||
sessionManager: sm,
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
|
||||
// TakeSnapshot captures session pool memory statistics
|
||||
func (spp *SessionPoolProfiler) TakeSnapshot() (*MemorySnapshot, error) {
|
||||
snapshot := &MemorySnapshot{
|
||||
Timestamp: time.Now(),
|
||||
CustomMetrics: make(map[string]interface{}),
|
||||
}
|
||||
|
||||
// Capture runtime stats
|
||||
runtime.ReadMemStats(&snapshot.RuntimeStats)
|
||||
|
||||
// Add session pool specific metrics
|
||||
snapshot.CustomMetrics["session_pool_metrics"] = spp.sessionManager.GetSessionMetrics()
|
||||
|
||||
return snapshot, nil
|
||||
}
|
||||
|
||||
// StartProfiling begins profiling (no-op for session pools)
|
||||
func (spp *SessionPoolProfiler) StartProfiling(config ProfilingConfig) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// StopProfiling ends profiling (no-op for session pools)
|
||||
func (spp *SessionPoolProfiler) StopProfiling() (*MemorySnapshot, error) {
|
||||
return spp.TakeSnapshot()
|
||||
}
|
||||
|
||||
// GetCurrentStats returns current memory statistics
|
||||
func (spp *SessionPoolProfiler) GetCurrentStats() *runtime.MemStats {
|
||||
stats := &runtime.MemStats{}
|
||||
runtime.ReadMemStats(stats)
|
||||
return stats
|
||||
}
|
||||
|
||||
// AnalyzeLeaks analyzes session pool for leaks
|
||||
func (spp *SessionPoolProfiler) AnalyzeLeaks(baseline, current *MemorySnapshot) *LeakAnalysis {
|
||||
analysis := &LeakAnalysis{
|
||||
SuspectedLeaks: make([]string, 0),
|
||||
Recommendations: make([]string, 0),
|
||||
}
|
||||
|
||||
if baseline == nil || current == nil {
|
||||
analysis.LeakDescription = "Insufficient session pool data"
|
||||
return analysis
|
||||
}
|
||||
|
||||
// Check for session pool specific leaks
|
||||
memoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
if memoryIncrease > 10*1024*1024 { // 10MB threshold
|
||||
analysis.HasLeak = true
|
||||
analysis.SuspectedLeaks = append(analysis.SuspectedLeaks,
|
||||
"Session pool memory usage increased significantly")
|
||||
analysis.Recommendations = append(analysis.Recommendations,
|
||||
"Check for sessions not being returned to pool properly")
|
||||
}
|
||||
|
||||
return analysis
|
||||
}
|
||||
|
||||
// CacheMemoryProfiler monitors cache memory usage
|
||||
type CacheMemoryProfiler struct {
|
||||
cache *Cache
|
||||
logger *Logger
|
||||
}
|
||||
|
||||
// NewCacheMemoryProfiler creates a new cache memory profiler
|
||||
func NewCacheMemoryProfiler(cache *Cache, logger *Logger) *CacheMemoryProfiler {
|
||||
if logger == nil {
|
||||
logger = newNoOpLogger()
|
||||
}
|
||||
return &CacheMemoryProfiler{
|
||||
cache: cache,
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
|
||||
// TakeSnapshot captures cache memory statistics
|
||||
func (cmp *CacheMemoryProfiler) TakeSnapshot() (*MemorySnapshot, error) {
|
||||
snapshot := &MemorySnapshot{
|
||||
Timestamp: time.Now(),
|
||||
CustomMetrics: make(map[string]interface{}),
|
||||
}
|
||||
|
||||
runtime.ReadMemStats(&snapshot.RuntimeStats)
|
||||
|
||||
// Add cache-specific metrics (would need to be added to Cache struct)
|
||||
snapshot.CustomMetrics["cache_size"] = "unknown" // Placeholder
|
||||
|
||||
return snapshot, nil
|
||||
}
|
||||
|
||||
// StartProfiling begins profiling (no-op for cache)
|
||||
func (cmp *CacheMemoryProfiler) StartProfiling(config ProfilingConfig) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// StopProfiling ends profiling
|
||||
func (cmp *CacheMemoryProfiler) StopProfiling() (*MemorySnapshot, error) {
|
||||
return cmp.TakeSnapshot()
|
||||
}
|
||||
|
||||
// GetCurrentStats returns current memory statistics
|
||||
func (cmp *CacheMemoryProfiler) GetCurrentStats() *runtime.MemStats {
|
||||
stats := &runtime.MemStats{}
|
||||
runtime.ReadMemStats(stats)
|
||||
return stats
|
||||
}
|
||||
|
||||
// AnalyzeLeaks analyzes cache for memory leaks
|
||||
func (cmp *CacheMemoryProfiler) AnalyzeLeaks(baseline, current *MemorySnapshot) *LeakAnalysis {
|
||||
analysis := &LeakAnalysis{
|
||||
SuspectedLeaks: make([]string, 0),
|
||||
Recommendations: make([]string, 0),
|
||||
}
|
||||
|
||||
if baseline == nil || current == nil {
|
||||
analysis.LeakDescription = "Insufficient cache data"
|
||||
return analysis
|
||||
}
|
||||
|
||||
memoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
if memoryIncrease > 20*1024*1024 { // 20MB threshold for cache
|
||||
analysis.HasLeak = true
|
||||
analysis.SuspectedLeaks = append(analysis.SuspectedLeaks,
|
||||
"Cache memory usage increased significantly")
|
||||
analysis.Recommendations = append(analysis.Recommendations,
|
||||
"Check cache size limits and cleanup intervals")
|
||||
}
|
||||
|
||||
return analysis
|
||||
}
|
||||
|
||||
// HTTPClientProfiler monitors HTTP client connection pools
|
||||
type HTTPClientProfiler struct {
|
||||
httpClient *http.Client
|
||||
logger *Logger
|
||||
}
|
||||
|
||||
// NewHTTPClientProfiler creates a new HTTP client profiler
|
||||
func NewHTTPClientProfiler(client *http.Client, logger *Logger) *HTTPClientProfiler {
|
||||
if logger == nil {
|
||||
logger = newNoOpLogger()
|
||||
}
|
||||
return &HTTPClientProfiler{
|
||||
httpClient: client,
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
|
||||
// TakeSnapshot captures HTTP client memory statistics
|
||||
func (hcp *HTTPClientProfiler) TakeSnapshot() (*MemorySnapshot, error) {
|
||||
snapshot := &MemorySnapshot{
|
||||
Timestamp: time.Now(),
|
||||
CustomMetrics: make(map[string]interface{}),
|
||||
}
|
||||
|
||||
runtime.ReadMemStats(&snapshot.RuntimeStats)
|
||||
|
||||
// Add HTTP client specific metrics
|
||||
if transport, ok := hcp.httpClient.Transport.(*http.Transport); ok {
|
||||
snapshot.CustomMetrics["idle_connections"] = transport.IdleConnTimeout.String()
|
||||
snapshot.CustomMetrics["max_idle_conns"] = transport.MaxIdleConns
|
||||
}
|
||||
|
||||
return snapshot, nil
|
||||
}
|
||||
|
||||
// StartProfiling begins profiling (no-op for HTTP client)
|
||||
func (hcp *HTTPClientProfiler) StartProfiling(config ProfilingConfig) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// StopProfiling ends profiling
|
||||
func (hcp *HTTPClientProfiler) StopProfiling() (*MemorySnapshot, error) {
|
||||
return hcp.TakeSnapshot()
|
||||
}
|
||||
|
||||
// GetCurrentStats returns current memory statistics
|
||||
func (hcp *HTTPClientProfiler) GetCurrentStats() *runtime.MemStats {
|
||||
stats := &runtime.MemStats{}
|
||||
runtime.ReadMemStats(stats)
|
||||
return stats
|
||||
}
|
||||
|
||||
// AnalyzeLeaks analyzes HTTP client for connection leaks
|
||||
func (hcp *HTTPClientProfiler) AnalyzeLeaks(baseline, current *MemorySnapshot) *LeakAnalysis {
|
||||
analysis := &LeakAnalysis{
|
||||
SuspectedLeaks: make([]string, 0),
|
||||
Recommendations: make([]string, 0),
|
||||
}
|
||||
|
||||
if baseline == nil || current == nil {
|
||||
analysis.LeakDescription = "Insufficient HTTP client data"
|
||||
return analysis
|
||||
}
|
||||
|
||||
memoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
if memoryIncrease > 5*1024*1024 { // 5MB threshold for HTTP client
|
||||
analysis.HasLeak = true
|
||||
analysis.SuspectedLeaks = append(analysis.SuspectedLeaks,
|
||||
"HTTP client memory usage increased significantly")
|
||||
analysis.Recommendations = append(analysis.Recommendations,
|
||||
"Check for HTTP response bodies not being drained properly")
|
||||
}
|
||||
|
||||
return analysis
|
||||
}
|
||||
|
||||
// TokenCompressionProfiler monitors token compression memory usage
|
||||
type TokenCompressionProfiler struct {
|
||||
compressionPool *TokenCompressionPool
|
||||
logger *Logger
|
||||
}
|
||||
|
||||
// NewTokenCompressionProfiler creates a new token compression profiler
|
||||
func NewTokenCompressionProfiler(pool *TokenCompressionPool, logger *Logger) *TokenCompressionProfiler {
|
||||
if logger == nil {
|
||||
logger = newNoOpLogger()
|
||||
}
|
||||
return &TokenCompressionProfiler{
|
||||
compressionPool: pool,
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
|
||||
// TakeSnapshot captures token compression memory statistics
|
||||
func (tcp *TokenCompressionProfiler) TakeSnapshot() (*MemorySnapshot, error) {
|
||||
snapshot := &MemorySnapshot{
|
||||
Timestamp: time.Now(),
|
||||
CustomMetrics: make(map[string]interface{}),
|
||||
}
|
||||
|
||||
runtime.ReadMemStats(&snapshot.RuntimeStats)
|
||||
|
||||
// Add compression pool specific metrics
|
||||
snapshot.CustomMetrics["compression_pool_active"] = true
|
||||
|
||||
return snapshot, nil
|
||||
}
|
||||
|
||||
// StartProfiling begins profiling (no-op for compression)
|
||||
func (tcp *TokenCompressionProfiler) StartProfiling(config ProfilingConfig) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// StopProfiling ends profiling
|
||||
func (tcp *TokenCompressionProfiler) StopProfiling() (*MemorySnapshot, error) {
|
||||
return tcp.TakeSnapshot()
|
||||
}
|
||||
|
||||
// GetCurrentStats returns current memory statistics
|
||||
func (tcp *TokenCompressionProfiler) GetCurrentStats() *runtime.MemStats {
|
||||
stats := &runtime.MemStats{}
|
||||
runtime.ReadMemStats(stats)
|
||||
return stats
|
||||
}
|
||||
|
||||
// AnalyzeLeaks analyzes token compression for memory leaks
|
||||
func (tcp *TokenCompressionProfiler) AnalyzeLeaks(baseline, current *MemorySnapshot) *LeakAnalysis {
|
||||
analysis := &LeakAnalysis{
|
||||
SuspectedLeaks: make([]string, 0),
|
||||
Recommendations: make([]string, 0),
|
||||
}
|
||||
|
||||
if baseline == nil || current == nil {
|
||||
analysis.LeakDescription = "Insufficient compression data"
|
||||
return analysis
|
||||
}
|
||||
|
||||
memoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
if memoryIncrease > 2*1024*1024 { // 2MB threshold for compression
|
||||
analysis.HasLeak = true
|
||||
analysis.SuspectedLeaks = append(analysis.SuspectedLeaks,
|
||||
"Token compression memory usage increased significantly")
|
||||
analysis.Recommendations = append(analysis.Recommendations,
|
||||
"Check for compression buffers not being returned to pool")
|
||||
}
|
||||
|
||||
return analysis
|
||||
}
|
||||
|
||||
// MemoryPoolProfiler monitors memory pool usage and detects leaks
|
||||
type MemoryPoolProfiler struct {
|
||||
memoryPoolManager *MemoryPoolManager
|
||||
tokenCompressionPool *TokenCompressionPool
|
||||
logger *Logger
|
||||
}
|
||||
|
||||
// NewMemoryPoolProfiler creates a new memory pool profiler
|
||||
func NewMemoryPoolProfiler(memoryPoolManager *MemoryPoolManager, tokenCompressionPool *TokenCompressionPool, logger *Logger) *MemoryPoolProfiler {
|
||||
if logger == nil {
|
||||
logger = newNoOpLogger()
|
||||
}
|
||||
return &MemoryPoolProfiler{
|
||||
memoryPoolManager: memoryPoolManager,
|
||||
tokenCompressionPool: tokenCompressionPool,
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
|
||||
// TakeSnapshot captures memory pool statistics
|
||||
func (mpp *MemoryPoolProfiler) TakeSnapshot() (*MemorySnapshot, error) {
|
||||
snapshot := &MemorySnapshot{
|
||||
Timestamp: time.Now(),
|
||||
CustomMetrics: make(map[string]interface{}),
|
||||
}
|
||||
|
||||
// Capture runtime stats
|
||||
runtime.ReadMemStats(&snapshot.RuntimeStats)
|
||||
|
||||
// Add memory pool metrics
|
||||
if mpp.memoryPoolManager != nil {
|
||||
snapshot.CustomMetrics["memory_pool_active"] = true
|
||||
// Note: sync.Pool doesn't expose internal statistics, so we track usage patterns
|
||||
}
|
||||
|
||||
if mpp.tokenCompressionPool != nil {
|
||||
snapshot.CustomMetrics["token_compression_pool_active"] = true
|
||||
}
|
||||
|
||||
return snapshot, nil
|
||||
}
|
||||
|
||||
// StartProfiling begins profiling (no-op for memory pools)
|
||||
func (mpp *MemoryPoolProfiler) StartProfiling(config ProfilingConfig) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// StopProfiling ends profiling
|
||||
func (mpp *MemoryPoolProfiler) StopProfiling() (*MemorySnapshot, error) {
|
||||
return mpp.TakeSnapshot()
|
||||
}
|
||||
|
||||
// GetCurrentStats returns current memory statistics
|
||||
func (mpp *MemoryPoolProfiler) GetCurrentStats() *runtime.MemStats {
|
||||
stats := &runtime.MemStats{}
|
||||
runtime.ReadMemStats(stats)
|
||||
return stats
|
||||
}
|
||||
|
||||
// AnalyzeLeaks analyzes memory pools for leaks
|
||||
func (mpp *MemoryPoolProfiler) AnalyzeLeaks(baseline, current *MemorySnapshot) *LeakAnalysis {
|
||||
analysis := &LeakAnalysis{
|
||||
SuspectedLeaks: make([]string, 0),
|
||||
Recommendations: make([]string, 0),
|
||||
}
|
||||
|
||||
if baseline == nil || current == nil {
|
||||
analysis.LeakDescription = "Insufficient memory pool data"
|
||||
return analysis
|
||||
}
|
||||
|
||||
// Check for memory leaks in pool operations
|
||||
memoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
if memoryIncrease > 5*1024*1024 { // 5MB threshold for pool operations
|
||||
analysis.HasLeak = true
|
||||
analysis.SuspectedLeaks = append(analysis.SuspectedLeaks,
|
||||
"Memory pool operations caused significant memory increase")
|
||||
analysis.Recommendations = append(analysis.Recommendations,
|
||||
"Check for objects not being returned to memory pools properly")
|
||||
}
|
||||
|
||||
return analysis
|
||||
}
|
||||
|
||||
// Global profiling manager instance
|
||||
var globalProfilingManager *ProfilingManager
|
||||
var profilingManagerOnce sync.Once
|
||||
|
||||
// GetGlobalProfilingManager returns the singleton profiling manager
|
||||
func GetGlobalProfilingManager() *ProfilingManager {
|
||||
profilingManagerOnce.Do(func() {
|
||||
globalProfilingManager = NewProfilingManager(nil)
|
||||
})
|
||||
return globalProfilingManager
|
||||
}
|
||||
|
||||
// Global test orchestrator instance
|
||||
var globalTestOrchestrator *MemoryTestOrchestrator
|
||||
var testOrchestratorOnce sync.Once
|
||||
|
||||
// GetGlobalTestOrchestrator returns the singleton test orchestrator
|
||||
func GetGlobalTestOrchestrator() *MemoryTestOrchestrator {
|
||||
testOrchestratorOnce.Do(func() {
|
||||
config := LeakDetectionConfig{
|
||||
EnableLeakDetection: true,
|
||||
LeakThresholdMB: 50,
|
||||
GoroutineLeakThreshold: 10,
|
||||
SessionPoolThreshold: 100,
|
||||
CacheMemoryThreshold: 20 * 1024 * 1024, // 20MB
|
||||
HTTPClientThreshold: 50,
|
||||
TokenCompressionThreshold: 2 * 1024 * 1024, // 2MB
|
||||
}
|
||||
globalTestOrchestrator = NewMemoryTestOrchestrator(config, nil)
|
||||
})
|
||||
return globalTestOrchestrator
|
||||
}
|
||||
@@ -0,0 +1,788 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net"
|
||||
"net/http"
|
||||
"os"
|
||||
"runtime"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestProfilingManager(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
pm := NewProfilingManager(logger)
|
||||
|
||||
// Test taking a snapshot
|
||||
snapshot, err := pm.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take snapshot: %v", err)
|
||||
}
|
||||
|
||||
if snapshot == nil {
|
||||
t.Fatal("Snapshot is nil")
|
||||
}
|
||||
|
||||
if snapshot.RuntimeStats.Alloc == 0 {
|
||||
t.Error("Runtime stats Alloc should not be zero")
|
||||
}
|
||||
|
||||
if snapshot.Timestamp.IsZero() {
|
||||
t.Error("Snapshot timestamp should not be zero")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoryTestOrchestrator(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
config := LeakDetectionConfig{
|
||||
EnableLeakDetection: true,
|
||||
LeakThresholdMB: 10,
|
||||
}
|
||||
|
||||
mto := NewMemoryTestOrchestrator(config, logger)
|
||||
|
||||
// Test registering a component
|
||||
sessionManager, err := NewSessionManager("test-key-32-chars-long-for-testing", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
profiler := NewSessionPoolProfiler(sessionManager, logger)
|
||||
mto.RegisterComponent("session_pool", profiler)
|
||||
|
||||
// Test getting leak analysis (should return false initially since no checks have been performed)
|
||||
_, exists := mto.GetLeakAnalysis("session_pool")
|
||||
if exists {
|
||||
t.Error("Should not have leak analysis before any checks are performed")
|
||||
}
|
||||
|
||||
// Perform a manual leak check
|
||||
baseline, err := profiler.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take baseline snapshot: %v", err)
|
||||
}
|
||||
|
||||
time.Sleep(10 * time.Millisecond) // Small delay
|
||||
|
||||
// Manually trigger leak check with baseline
|
||||
baselineSnapshots := make(map[string]*MemorySnapshot)
|
||||
baselineSnapshots["session_pool"] = baseline
|
||||
mto.performLeakCheck(baselineSnapshots)
|
||||
|
||||
// Now test getting leak analysis
|
||||
analysis, exists := mto.GetLeakAnalysis("session_pool")
|
||||
if !exists {
|
||||
t.Error("Should have leak analysis after performing checks")
|
||||
}
|
||||
|
||||
if analysis == nil {
|
||||
t.Error("Leak analysis should not be nil after checks")
|
||||
}
|
||||
}
|
||||
|
||||
func TestComponentProfilers(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
|
||||
// Test Session Pool Profiler
|
||||
sessionManager, err := NewSessionManager("test-key-32-chars-long-for-testing", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
spp := NewSessionPoolProfiler(sessionManager, logger)
|
||||
snapshot, err := spp.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take session pool snapshot: %v", err)
|
||||
}
|
||||
|
||||
if snapshot == nil {
|
||||
t.Fatal("Session pool snapshot is nil")
|
||||
}
|
||||
|
||||
// Test Cache Memory Profiler
|
||||
cache := NewCache()
|
||||
cmp := NewCacheMemoryProfiler(cache, logger)
|
||||
snapshot, err = cmp.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take cache snapshot: %v", err)
|
||||
}
|
||||
|
||||
if snapshot == nil {
|
||||
t.Fatal("Cache snapshot is nil")
|
||||
}
|
||||
|
||||
// Test HTTP Client Profiler
|
||||
httpClient := createDefaultHTTPClient()
|
||||
hcp := NewHTTPClientProfiler(httpClient, logger)
|
||||
snapshot, err = hcp.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take HTTP client snapshot: %v", err)
|
||||
}
|
||||
|
||||
if snapshot == nil {
|
||||
t.Fatal("HTTP client snapshot is nil")
|
||||
}
|
||||
|
||||
// Test Token Compression Profiler
|
||||
compressionPool := NewTokenCompressionPool()
|
||||
tcp := NewTokenCompressionProfiler(compressionPool, logger)
|
||||
snapshot, err = tcp.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take compression snapshot: %v", err)
|
||||
}
|
||||
|
||||
if snapshot == nil {
|
||||
t.Fatal("Compression snapshot is nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestLeakAnalysis(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
pm := NewProfilingManager(logger)
|
||||
|
||||
// Create baseline snapshot
|
||||
baseline, err := pm.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create baseline: %v", err)
|
||||
}
|
||||
|
||||
// Wait a bit and create current snapshot
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
current, err := pm.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create current snapshot: %v", err)
|
||||
}
|
||||
|
||||
// Test leak analysis
|
||||
analysis := pm.AnalyzeLeaks(baseline, current)
|
||||
if analysis == nil {
|
||||
t.Fatal("Leak analysis is nil")
|
||||
}
|
||||
|
||||
// Analysis should not have leaks for normal operation
|
||||
if analysis.HasLeak {
|
||||
t.Logf("Leak detected: %s", analysis.LeakDescription)
|
||||
// This is acceptable as the test environment may have varying memory usage
|
||||
}
|
||||
}
|
||||
|
||||
func TestGlobalInstances(t *testing.T) {
|
||||
// Test global profiling manager
|
||||
gpm := GetGlobalProfilingManager()
|
||||
if gpm == nil {
|
||||
t.Fatal("Global profiling manager is nil")
|
||||
}
|
||||
|
||||
// Test global test orchestrator
|
||||
gto := GetGlobalTestOrchestrator()
|
||||
if gto == nil {
|
||||
t.Fatal("Global test orchestrator is nil")
|
||||
}
|
||||
|
||||
// Test that they're singletons
|
||||
gpm2 := GetGlobalProfilingManager()
|
||||
if gpm != gpm2 {
|
||||
t.Error("Global profiling manager should be singleton")
|
||||
}
|
||||
|
||||
gto2 := GetGlobalTestOrchestrator()
|
||||
if gto != gto2 {
|
||||
t.Error("Global test orchestrator should be singleton")
|
||||
}
|
||||
}
|
||||
|
||||
func TestProfilingConfig(t *testing.T) {
|
||||
config := ProfilingConfig{
|
||||
EnableHeapProfiling: true,
|
||||
EnableGoroutineProfiling: true,
|
||||
SnapshotInterval: 30 * time.Second,
|
||||
LeakThresholdMB: 50,
|
||||
MaxSnapshots: 100,
|
||||
EnableContinuousMonitoring: true,
|
||||
MonitoringInterval: 60 * time.Second,
|
||||
}
|
||||
|
||||
if !config.EnableHeapProfiling {
|
||||
t.Error("Heap profiling should be enabled")
|
||||
}
|
||||
|
||||
if !config.EnableGoroutineProfiling {
|
||||
t.Error("Goroutine profiling should be enabled")
|
||||
}
|
||||
|
||||
if config.LeakThresholdMB != 50 {
|
||||
t.Errorf("Expected leak threshold 50, got %d", config.LeakThresholdMB)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLeakDetectionConfig(t *testing.T) {
|
||||
config := LeakDetectionConfig{
|
||||
EnableLeakDetection: true,
|
||||
LeakThresholdMB: 50,
|
||||
GoroutineLeakThreshold: 10,
|
||||
SessionPoolThreshold: 100,
|
||||
CacheMemoryThreshold: 20 * 1024 * 1024,
|
||||
HTTPClientThreshold: 50,
|
||||
TokenCompressionThreshold: 2 * 1024 * 1024,
|
||||
}
|
||||
|
||||
if !config.EnableLeakDetection {
|
||||
t.Error("Leak detection should be enabled")
|
||||
}
|
||||
|
||||
if config.LeakThresholdMB != 50 {
|
||||
t.Errorf("Expected leak threshold 50, got %d", config.LeakThresholdMB)
|
||||
}
|
||||
|
||||
if config.CacheMemoryThreshold != 20*1024*1024 {
|
||||
t.Errorf("Expected cache threshold 20MB, got %d", config.CacheMemoryThreshold)
|
||||
}
|
||||
}
|
||||
|
||||
// ProviderMetadataProfiler monitors provider metadata fetching and caching operations
|
||||
type ProviderMetadataProfiler struct {
|
||||
metadataCache *MetadataCache
|
||||
httpClient *http.Client
|
||||
logger *Logger
|
||||
providerURL string
|
||||
}
|
||||
|
||||
// NewProviderMetadataProfiler creates a new provider metadata profiler
|
||||
func NewProviderMetadataProfiler(metadataCache *MetadataCache, httpClient *http.Client, providerURL string, logger *Logger) *ProviderMetadataProfiler {
|
||||
if logger == nil {
|
||||
logger = newNoOpLogger()
|
||||
}
|
||||
return &ProviderMetadataProfiler{
|
||||
metadataCache: metadataCache,
|
||||
httpClient: httpClient,
|
||||
providerURL: providerURL,
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
|
||||
// TakeSnapshot captures current memory statistics for metadata operations
|
||||
func (pmp *ProviderMetadataProfiler) TakeSnapshot() (*MemorySnapshot, error) {
|
||||
snapshot := &MemorySnapshot{
|
||||
Timestamp: time.Now(),
|
||||
CustomMetrics: make(map[string]interface{}),
|
||||
}
|
||||
|
||||
// Capture runtime memory statistics
|
||||
runtime.ReadMemStats(&snapshot.RuntimeStats)
|
||||
|
||||
// Add metadata-specific metrics
|
||||
snapshot.CustomMetrics["metadata_cache_size"] = 1 // Placeholder for cache size
|
||||
snapshot.CustomMetrics["metadata_fetch_count"] = 0 // Placeholder for fetch count
|
||||
snapshot.CustomMetrics["background_goroutines"] = runtime.NumGoroutine()
|
||||
|
||||
return snapshot, nil
|
||||
}
|
||||
|
||||
// StartProfiling begins profiling (no-op for metadata profiler)
|
||||
func (pmp *ProviderMetadataProfiler) StartProfiling(config ProfilingConfig) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// StopProfiling ends profiling
|
||||
func (pmp *ProviderMetadataProfiler) StopProfiling() (*MemorySnapshot, error) {
|
||||
return pmp.TakeSnapshot()
|
||||
}
|
||||
|
||||
// GetCurrentStats returns current memory statistics
|
||||
func (pmp *ProviderMetadataProfiler) GetCurrentStats() *runtime.MemStats {
|
||||
stats := &runtime.MemStats{}
|
||||
runtime.ReadMemStats(stats)
|
||||
return stats
|
||||
}
|
||||
|
||||
// AnalyzeLeaks analyzes metadata operations for memory leaks
|
||||
func (pmp *ProviderMetadataProfiler) AnalyzeLeaks(baseline, current *MemorySnapshot) *LeakAnalysis {
|
||||
analysis := &LeakAnalysis{
|
||||
SuspectedLeaks: make([]string, 0),
|
||||
Recommendations: make([]string, 0),
|
||||
}
|
||||
|
||||
if baseline == nil || current == nil {
|
||||
analysis.LeakDescription = "Insufficient metadata data"
|
||||
return analysis
|
||||
}
|
||||
|
||||
// Check for memory leaks
|
||||
memoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
if memoryIncrease > 5*1024*1024 { // 5MB threshold for metadata operations
|
||||
analysis.HasLeak = true
|
||||
analysis.SuspectedLeaks = append(analysis.SuspectedLeaks,
|
||||
"Metadata operations memory usage increased significantly")
|
||||
analysis.Recommendations = append(analysis.Recommendations,
|
||||
"Check for metadata cache not being cleaned up properly")
|
||||
}
|
||||
|
||||
// Check for goroutine leaks
|
||||
goroutineIncrease := current.CustomMetrics["background_goroutines"].(int) - baseline.CustomMetrics["background_goroutines"].(int)
|
||||
if goroutineIncrease > 2 { // Allow some variance
|
||||
analysis.HasLeak = true
|
||||
analysis.SuspectedLeaks = append(analysis.SuspectedLeaks,
|
||||
fmt.Sprintf("Goroutine count increased by %d during metadata operations", goroutineIncrease))
|
||||
analysis.Recommendations = append(analysis.Recommendations,
|
||||
"Check for background goroutines not being cleaned up")
|
||||
}
|
||||
|
||||
return analysis
|
||||
}
|
||||
|
||||
// TestProviderMetadataMemoryLeakDetection tests for memory leaks in provider metadata operations
|
||||
func TestProviderMetadataMemoryLeakDetection(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
|
||||
strictMode := os.Getenv("STRICT_MEMORY_TEST") == "true"
|
||||
if strictMode {
|
||||
t.Log("Running in strict memory test mode - will fail on detected leaks")
|
||||
} else {
|
||||
t.Log("Running in lenient memory test mode - will log warnings instead of failing")
|
||||
}
|
||||
|
||||
config := LeakDetectionConfig{
|
||||
EnableLeakDetection: true,
|
||||
LeakThresholdMB: 10,
|
||||
}
|
||||
|
||||
mto := NewMemoryTestOrchestrator(config, logger)
|
||||
|
||||
// Create mock HTTP server for metadata endpoint with failure simulation
|
||||
requestCount := 0
|
||||
serverFailures := 0
|
||||
mockServer := &http.Server{
|
||||
Addr: "localhost:0", // Let system assign port
|
||||
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
requestCount++
|
||||
if r.URL.Path == "/.well-known/openid-configuration" {
|
||||
// Simulate occasional failures to test cache extension
|
||||
if requestCount%4 == 0 { // Fail every 4th request
|
||||
serverFailures++
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
metadata := ProviderMetadata{
|
||||
Issuer: "https://mock-provider.com",
|
||||
AuthURL: "https://mock-provider.com/auth",
|
||||
TokenURL: "https://mock-provider.com/token",
|
||||
JWKSURL: "https://mock-provider.com/jwks",
|
||||
RevokeURL: "https://mock-provider.com/revoke",
|
||||
EndSessionURL: "https://mock-provider.com/logout",
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.Header().Set("Cache-Control", "max-age=3600") // 1 hour cache hint
|
||||
json.NewEncoder(w).Encode(metadata)
|
||||
} else {
|
||||
http.NotFound(w, r)
|
||||
}
|
||||
}),
|
||||
}
|
||||
|
||||
// Start mock server
|
||||
listener, err := net.Listen("tcp", "localhost:0")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create listener: %v", err)
|
||||
}
|
||||
go mockServer.Serve(listener)
|
||||
defer mockServer.Close()
|
||||
|
||||
providerURL := fmt.Sprintf("http://%s", listener.Addr().String())
|
||||
httpClient := createDefaultHTTPClient()
|
||||
|
||||
// Create metadata cache with WaitGroup for proper goroutine synchronization
|
||||
var wg sync.WaitGroup
|
||||
metadataCache := NewMetadataCacheWithLogger(&wg, logger)
|
||||
|
||||
// Create profiler
|
||||
profiler := NewProviderMetadataProfiler(metadataCache, httpClient, providerURL, logger)
|
||||
mto.RegisterComponent("provider_metadata", profiler)
|
||||
|
||||
// Take initial baseline
|
||||
baseline, err := profiler.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take baseline snapshot: %v", err)
|
||||
}
|
||||
|
||||
initialGoroutines := runtime.NumGoroutine()
|
||||
|
||||
// Phase 1: Simulate periodic metadata fetching with some failures
|
||||
t.Log("Phase 1: Testing periodic fetching with occasional failures...")
|
||||
for i := 0; i < 20; i++ {
|
||||
_, err := metadataCache.GetMetadata(providerURL, httpClient, logger)
|
||||
if err != nil {
|
||||
t.Logf("Metadata fetch %d failed (expected for cache extension testing): %v", i+1, err)
|
||||
} else {
|
||||
t.Logf("Metadata fetch %d succeeded", i+1)
|
||||
}
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
}
|
||||
|
||||
// Wait for background cleanup (normally every 5 minutes)
|
||||
time.Sleep(300 * time.Millisecond)
|
||||
|
||||
// Take intermediate snapshot
|
||||
intermediate, err := profiler.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take intermediate snapshot: %v", err)
|
||||
}
|
||||
|
||||
// Phase 2: Continue with more fetches to test sustained operation
|
||||
t.Log("Phase 2: Testing sustained operation with 1000 iterations...")
|
||||
for i := 20; i < 1020; i++ {
|
||||
_, err := metadataCache.GetMetadata(providerURL, httpClient, logger)
|
||||
if err != nil {
|
||||
t.Logf("Metadata fetch %d failed: %v", i+1, err)
|
||||
}
|
||||
time.Sleep(50 * time.Millisecond) // Reduced sleep for faster execution
|
||||
}
|
||||
|
||||
// Take final snapshot
|
||||
current, err := profiler.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take current snapshot: %v", err)
|
||||
}
|
||||
|
||||
finalGoroutines := runtime.NumGoroutine()
|
||||
|
||||
// Analyze for leaks
|
||||
analysis := profiler.AnalyzeLeaks(baseline, current)
|
||||
|
||||
// Assertions for memory leaks
|
||||
if analysis.HasLeak {
|
||||
if strictMode {
|
||||
t.Errorf("Memory leak detected in provider metadata operations: %s", analysis.LeakDescription)
|
||||
for _, leak := range analysis.SuspectedLeaks {
|
||||
t.Errorf("Suspected leak: %s", leak)
|
||||
}
|
||||
} else {
|
||||
t.Logf("Memory leak warning in provider metadata operations: %s", analysis.LeakDescription)
|
||||
for _, leak := range analysis.SuspectedLeaks {
|
||||
t.Logf("Suspected leak: %s", leak)
|
||||
}
|
||||
}
|
||||
for _, rec := range analysis.Recommendations {
|
||||
t.Logf("Recommendation: %s", rec)
|
||||
}
|
||||
}
|
||||
|
||||
// Check total memory growth
|
||||
totalMemoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
if totalMemoryIncrease > 20*1024*1024 { // 20MB threshold for entire test
|
||||
if strictMode {
|
||||
t.Errorf("Total memory usage increased by %.2f MB during metadata operations", float64(totalMemoryIncrease)/(1024*1024))
|
||||
} else {
|
||||
t.Logf("Total memory usage increased by %.2f MB during metadata operations", float64(totalMemoryIncrease)/(1024*1024))
|
||||
}
|
||||
}
|
||||
|
||||
// Check for gradual memory growth patterns
|
||||
intermediateMemoryIncrease := intermediate.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
if intermediateMemoryIncrease > 10*1024*1024 { // 10MB threshold for first phase
|
||||
if strictMode {
|
||||
t.Errorf("Memory usage increased by %.2f MB during first phase of metadata operations", float64(intermediateMemoryIncrease)/(1024*1024))
|
||||
} else {
|
||||
t.Logf("Memory usage increased by %.2f MB during first phase of metadata operations", float64(intermediateMemoryIncrease)/(1024*1024))
|
||||
}
|
||||
}
|
||||
|
||||
// Check goroutine count stability
|
||||
goroutineIncrease := finalGoroutines - initialGoroutines
|
||||
if goroutineIncrease > 5 { // Allow some variance for test environment
|
||||
if strictMode {
|
||||
t.Errorf("Goroutine count increased by %d during metadata operations (initial: %d, final: %d)",
|
||||
goroutineIncrease, initialGoroutines, finalGoroutines)
|
||||
} else {
|
||||
t.Logf("Goroutine count increased by %d during metadata operations (initial: %d, final: %d)",
|
||||
goroutineIncrease, initialGoroutines, finalGoroutines)
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 3: Test cache extension behavior on persistent failures
|
||||
t.Log("Phase 3: Testing cache extension on persistent failures...")
|
||||
|
||||
// Stop mock server to simulate provider unavailability
|
||||
mockServer.Close()
|
||||
|
||||
// Try multiple fetches after server shutdown
|
||||
postShutdownFailures := 0
|
||||
for i := 0; i < 5; i++ {
|
||||
_, err = metadataCache.GetMetadata(providerURL, httpClient, logger)
|
||||
if err != nil {
|
||||
postShutdownFailures++
|
||||
t.Logf("Expected failure %d after server shutdown: %v", i+1, err)
|
||||
} else {
|
||||
t.Logf("Unexpected success %d after server shutdown - cache extension working", i+1)
|
||||
}
|
||||
time.Sleep(200 * time.Millisecond)
|
||||
}
|
||||
|
||||
if postShutdownFailures == 0 {
|
||||
if strictMode {
|
||||
t.Error("Expected some metadata fetches to fail after server shutdown")
|
||||
} else {
|
||||
t.Log("Warning: No metadata fetches failed after server shutdown - cache extension may not be working as expected")
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 4: Test background goroutine lifecycle and cleanup
|
||||
t.Log("Phase 4: Testing background goroutine lifecycle...")
|
||||
|
||||
// Wait longer to allow background cleanup to run
|
||||
time.Sleep(1 * time.Second)
|
||||
|
||||
// Take final snapshot after cleanup
|
||||
finalAfterCleanup, err := profiler.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take final snapshot after cleanup: %v", err)
|
||||
}
|
||||
|
||||
// Check if memory decreased after cleanup
|
||||
if finalAfterCleanup.RuntimeStats.Alloc < current.RuntimeStats.Alloc {
|
||||
memoryDecrease := current.RuntimeStats.Alloc - finalAfterCleanup.RuntimeStats.Alloc
|
||||
t.Logf("Memory decreased by %.2f MB after cleanup phase", float64(memoryDecrease)/(1024*1024))
|
||||
}
|
||||
|
||||
// Clean up resources
|
||||
metadataCache.Close()
|
||||
wg.Wait() // Ensure all background goroutines complete
|
||||
|
||||
t.Logf("Test completed: %d total requests, %d server failures, %d post-shutdown failures",
|
||||
requestCount, serverFailures, postShutdownFailures)
|
||||
t.Logf("Memory usage: baseline=%.2f MB, intermediate=%.2f MB, final=%.2f MB",
|
||||
float64(baseline.RuntimeStats.Alloc)/(1024*1024),
|
||||
float64(intermediate.RuntimeStats.Alloc)/(1024*1024),
|
||||
float64(current.RuntimeStats.Alloc)/(1024*1024))
|
||||
}
|
||||
|
||||
// TestMemoryPoolLeakDetection tests for memory leaks in memory pool operations
|
||||
func TestMemoryPoolLeakDetection(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
|
||||
strictMode := os.Getenv("STRICT_MEMORY_TEST") == "true"
|
||||
if strictMode {
|
||||
t.Log("Running in strict memory test mode - will fail on detected leaks")
|
||||
} else {
|
||||
t.Log("Running in lenient memory test mode - will log warnings instead of failing")
|
||||
}
|
||||
|
||||
config := LeakDetectionConfig{
|
||||
EnableLeakDetection: true,
|
||||
LeakThresholdMB: 10,
|
||||
}
|
||||
|
||||
mto := NewMemoryTestOrchestrator(config, logger)
|
||||
|
||||
// Create memory pool manager and token compression pool
|
||||
memoryPoolManager := NewMemoryPoolManager()
|
||||
tokenCompressionPool := NewTokenCompressionPool()
|
||||
|
||||
// Create profiler for memory pools
|
||||
profiler := NewMemoryPoolProfiler(memoryPoolManager, tokenCompressionPool, logger)
|
||||
mto.RegisterComponent("memory_pools", profiler)
|
||||
|
||||
// Take initial baseline
|
||||
baseline, err := profiler.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take baseline snapshot: %v", err)
|
||||
}
|
||||
|
||||
initialGoroutines := runtime.NumGoroutine()
|
||||
|
||||
// Phase 1: Simulate various memory pool operations
|
||||
t.Log("Phase 1: Testing memory pool operations with various patterns...")
|
||||
|
||||
// Test compression buffer pool
|
||||
for i := 0; i < 100; i++ {
|
||||
buf := memoryPoolManager.GetCompressionBuffer()
|
||||
// Simulate some work with the buffer
|
||||
buf.WriteString(fmt.Sprintf("test data %d", i))
|
||||
// Properly return buffer to pool
|
||||
memoryPoolManager.PutCompressionBuffer(buf)
|
||||
}
|
||||
|
||||
// Test JWT parsing buffer pool
|
||||
for i := 0; i < 50; i++ {
|
||||
jwtBuf := memoryPoolManager.GetJWTParsingBuffer()
|
||||
// Simulate JWT parsing operations
|
||||
jwtBuf.HeaderBuf = append(jwtBuf.HeaderBuf, []byte("header")...)
|
||||
jwtBuf.PayloadBuf = append(jwtBuf.PayloadBuf, []byte("payload")...)
|
||||
jwtBuf.SignatureBuf = append(jwtBuf.SignatureBuf, []byte("signature")...)
|
||||
// Properly return buffer to pool
|
||||
memoryPoolManager.PutJWTParsingBuffer(jwtBuf)
|
||||
}
|
||||
|
||||
// Test HTTP response buffer pool
|
||||
for i := 0; i < 75; i++ {
|
||||
httpBuf := memoryPoolManager.GetHTTPResponseBuffer()
|
||||
// Simulate HTTP response processing
|
||||
copy(httpBuf[:min(len(httpBuf), 100)], []byte("http response data"))
|
||||
// Properly return buffer to pool
|
||||
memoryPoolManager.PutHTTPResponseBuffer(httpBuf)
|
||||
}
|
||||
|
||||
// Test string builder pool
|
||||
for i := 0; i < 60; i++ {
|
||||
sb := memoryPoolManager.GetStringBuilder()
|
||||
// Simulate string building operations
|
||||
sb.WriteString(fmt.Sprintf("built string %d", i))
|
||||
_ = sb.String() // Use the result
|
||||
// Properly return string builder to pool
|
||||
memoryPoolManager.PutStringBuilder(sb)
|
||||
}
|
||||
|
||||
// Test token compression pool
|
||||
for i := 0; i < 40; i++ {
|
||||
compBuf := tokenCompressionPool.GetCompressionBuffer()
|
||||
// Simulate compression operations
|
||||
compBuf.WriteString(fmt.Sprintf("compress data %d", i))
|
||||
// Properly return buffer to pool
|
||||
tokenCompressionPool.PutCompressionBuffer(compBuf)
|
||||
|
||||
decompBuf := tokenCompressionPool.GetDecompressionBuffer()
|
||||
// Simulate decompression operations
|
||||
decompBuf.WriteString(fmt.Sprintf("decompress data %d", i))
|
||||
// Properly return buffer to pool
|
||||
tokenCompressionPool.PutDecompressionBuffer(decompBuf)
|
||||
|
||||
sb := tokenCompressionPool.GetStringBuilder()
|
||||
// Simulate string operations
|
||||
sb.WriteString(fmt.Sprintf("token string %d", i))
|
||||
_ = sb.String()
|
||||
// Properly return string builder to pool
|
||||
tokenCompressionPool.PutStringBuilder(sb)
|
||||
}
|
||||
|
||||
// Take intermediate snapshot
|
||||
intermediate, err := profiler.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take intermediate snapshot: %v", err)
|
||||
}
|
||||
|
||||
// Phase 2: Continue with more intensive operations to test sustained usage
|
||||
t.Log("Phase 2: Testing sustained memory pool usage...")
|
||||
|
||||
// Simulate mixed operations with varying patterns
|
||||
for i := 0; i < 200; i++ {
|
||||
// Mix different pool operations
|
||||
switch i % 4 {
|
||||
case 0:
|
||||
buf := memoryPoolManager.GetCompressionBuffer()
|
||||
buf.WriteString("mixed operation data")
|
||||
memoryPoolManager.PutCompressionBuffer(buf)
|
||||
case 1:
|
||||
jwtBuf := memoryPoolManager.GetJWTParsingBuffer()
|
||||
jwtBuf.HeaderBuf = append(jwtBuf.HeaderBuf, []byte("mixed")...)
|
||||
memoryPoolManager.PutJWTParsingBuffer(jwtBuf)
|
||||
case 2:
|
||||
httpBuf := memoryPoolManager.GetHTTPResponseBuffer()
|
||||
copy(httpBuf[:min(len(httpBuf), 50)], []byte("mixed http"))
|
||||
memoryPoolManager.PutHTTPResponseBuffer(httpBuf)
|
||||
case 3:
|
||||
sb := memoryPoolManager.GetStringBuilder()
|
||||
sb.WriteString("mixed string building")
|
||||
_ = sb.String()
|
||||
memoryPoolManager.PutStringBuilder(sb)
|
||||
}
|
||||
}
|
||||
|
||||
// Take final snapshot
|
||||
current, err := profiler.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take current snapshot: %v", err)
|
||||
}
|
||||
|
||||
finalGoroutines := runtime.NumGoroutine()
|
||||
|
||||
// Analyze for leaks
|
||||
analysis := profiler.AnalyzeLeaks(baseline, current)
|
||||
|
||||
// Assertions for memory leaks
|
||||
if analysis.HasLeak {
|
||||
if strictMode {
|
||||
t.Errorf("Memory leak detected in memory pool operations: %s", analysis.LeakDescription)
|
||||
for _, leak := range analysis.SuspectedLeaks {
|
||||
t.Errorf("Suspected leak: %s", leak)
|
||||
}
|
||||
} else {
|
||||
t.Logf("Memory leak warning in memory pool operations: %s", analysis.LeakDescription)
|
||||
for _, leak := range analysis.SuspectedLeaks {
|
||||
t.Logf("Suspected leak: %s", leak)
|
||||
}
|
||||
}
|
||||
for _, rec := range analysis.Recommendations {
|
||||
t.Logf("Recommendation: %s", rec)
|
||||
}
|
||||
}
|
||||
|
||||
// Check total memory growth
|
||||
totalMemoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
if totalMemoryIncrease > 15*1024*1024 { // 15MB threshold for entire test
|
||||
if strictMode {
|
||||
t.Errorf("Total memory usage increased by %.2f MB during memory pool operations", float64(totalMemoryIncrease)/(1024*1024))
|
||||
} else {
|
||||
t.Logf("Total memory usage increased by %.2f MB during memory pool operations", float64(totalMemoryIncrease)/(1024*1024))
|
||||
}
|
||||
}
|
||||
|
||||
// Check for gradual memory growth patterns
|
||||
intermediateMemoryIncrease := intermediate.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
|
||||
if intermediateMemoryIncrease > 8*1024*1024 { // 8MB threshold for first phase
|
||||
if strictMode {
|
||||
t.Errorf("Memory usage increased by %.2f MB during first phase of memory pool operations", float64(intermediateMemoryIncrease)/(1024*1024))
|
||||
} else {
|
||||
t.Logf("Memory usage increased by %.2f MB during first phase of memory pool operations", float64(intermediateMemoryIncrease)/(1024*1024))
|
||||
}
|
||||
}
|
||||
|
||||
// Check goroutine count stability
|
||||
goroutineIncrease := finalGoroutines - initialGoroutines
|
||||
if goroutineIncrease > 3 { // Allow small variance for test environment
|
||||
if strictMode {
|
||||
t.Errorf("Goroutine count increased by %d during memory pool operations (initial: %d, final: %d)",
|
||||
goroutineIncrease, initialGoroutines, finalGoroutines)
|
||||
} else {
|
||||
t.Logf("Goroutine count increased by %d during memory pool operations (initial: %d, final: %d)",
|
||||
goroutineIncrease, initialGoroutines, finalGoroutines)
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 3: Test cleanup verification
|
||||
t.Log("Phase 3: Testing cleanup verification...")
|
||||
|
||||
// Force garbage collection to see if pools are properly managed
|
||||
runtime.GC()
|
||||
runtime.GC() // Run twice to ensure cleanup
|
||||
|
||||
time.Sleep(10 * time.Millisecond) // Allow cleanup to complete
|
||||
|
||||
// Take post-cleanup snapshot
|
||||
postCleanup, err := profiler.TakeSnapshot()
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to take post-cleanup snapshot: %v", err)
|
||||
}
|
||||
|
||||
// Check if memory decreased after cleanup
|
||||
if postCleanup.RuntimeStats.Alloc < current.RuntimeStats.Alloc {
|
||||
memoryDecrease := current.RuntimeStats.Alloc - postCleanup.RuntimeStats.Alloc
|
||||
t.Logf("Memory decreased by %.2f MB after cleanup phase", float64(memoryDecrease)/(1024*1024))
|
||||
} else if postCleanup.RuntimeStats.Alloc > current.RuntimeStats.Alloc {
|
||||
memoryIncrease := postCleanup.RuntimeStats.Alloc - current.RuntimeStats.Alloc
|
||||
if strictMode {
|
||||
t.Errorf("Memory increased by %.2f MB after cleanup phase - possible cleanup issues", float64(memoryIncrease)/(1024*1024))
|
||||
} else {
|
||||
t.Logf("Memory increased by %.2f MB after cleanup phase - possible cleanup issues", float64(memoryIncrease)/(1024*1024))
|
||||
}
|
||||
}
|
||||
|
||||
t.Logf("Memory pool leak detection test completed")
|
||||
t.Logf("Memory usage: baseline=%.2f MB, intermediate=%.2f MB, final=%.2f MB, post-cleanup=%.2f MB",
|
||||
float64(baseline.RuntimeStats.Alloc)/(1024*1024),
|
||||
float64(intermediate.RuntimeStats.Alloc)/(1024*1024),
|
||||
float64(current.RuntimeStats.Alloc)/(1024*1024),
|
||||
float64(postCleanup.RuntimeStats.Alloc)/(1024*1024))
|
||||
}
|
||||
@@ -0,0 +1,357 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestReverseProxyHTTPSDetection tests that HTTPS is properly detected in reverse proxy environments
|
||||
func TestReverseProxyHTTPSDetection(t *testing.T) {
|
||||
t.Run("HTTPS_Detection_With_X_Forwarded_Proto", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Simulate request from reverse proxy (Traefik/nginx)
|
||||
// The reverse proxy terminates SSL and forwards HTTP internally
|
||||
req := httptest.NewRequest("GET", "http://example.com/test", nil)
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
req.Header.Set("X-Forwarded-Host", "example.com")
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set critical session data
|
||||
session.SetCSRF("important-csrf-token")
|
||||
session.SetNonce("test-nonce")
|
||||
|
||||
// Save session
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify cookies have Secure flag when X-Forwarded-Proto is https
|
||||
cookies := rec.Result().Cookies()
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie, "Main session cookie should be set")
|
||||
assert.True(t, mainCookie.Secure, "Cookie should have Secure flag when X-Forwarded-Proto is https")
|
||||
assert.Equal(t, http.SameSiteLaxMode, mainCookie.SameSite, "Should use Lax SameSite for OAuth compatibility")
|
||||
})
|
||||
|
||||
t.Run("HTTPS_Detection_Without_Headers", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Request without reverse proxy headers (direct HTTP)
|
||||
req := httptest.NewRequest("GET", "http://example.com/test", nil)
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
session.SetCSRF("test-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify cookies don't have Secure flag for plain HTTP
|
||||
cookies := rec.Result().Cookies()
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie)
|
||||
assert.False(t, mainCookie.Secure, "Cookie should not have Secure flag for HTTP")
|
||||
assert.Equal(t, http.SameSiteLaxMode, mainCookie.SameSite, "Should use Lax SameSite in HTTP")
|
||||
})
|
||||
|
||||
t.Run("HTTPS_Detection_With_ForceHTTPS", func(t *testing.T) {
|
||||
// Test with forceHTTPS enabled
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", true, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Even without headers, forceHTTPS should make cookies secure
|
||||
req := httptest.NewRequest("GET", "http://example.com/test", nil)
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
session.SetCSRF("forced-secure-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies := rec.Result().Cookies()
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie)
|
||||
assert.True(t, mainCookie.Secure, "Cookie should have Secure flag with forceHTTPS")
|
||||
})
|
||||
}
|
||||
|
||||
// TestCSRFPersistenceInReverseProxy tests CSRF token persistence in reverse proxy setups
|
||||
func TestCSRFPersistenceInReverseProxy(t *testing.T) {
|
||||
t.Run("CSRF_Persists_Through_OAuth_Flow_With_Proxy", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Step 1: Initial request to protected resource (HTTPS via proxy)
|
||||
req1 := httptest.NewRequest("GET", "http://example.com/protected", nil)
|
||||
req1.Header.Set("X-Forwarded-Proto", "https")
|
||||
req1.Header.Set("X-Forwarded-Host", "example.com")
|
||||
|
||||
session1, err := sessionManager.GetSession(req1)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set CSRF and other auth flow data
|
||||
csrfToken := "proxy-csrf-token-12345"
|
||||
session1.SetCSRF(csrfToken)
|
||||
session1.SetNonce("proxy-nonce")
|
||||
session1.SetIncomingPath("/protected")
|
||||
|
||||
// Save session (should set Secure cookie)
|
||||
rec1 := httptest.NewRecorder()
|
||||
err = session1.Save(req1, rec1)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies := rec1.Result().Cookies()
|
||||
|
||||
// Step 2: Simulate OAuth callback (also HTTPS via proxy)
|
||||
req2 := httptest.NewRequest("GET", "http://example.com/oidc/callback?code=auth-code&state="+csrfToken, nil)
|
||||
req2.Header.Set("X-Forwarded-Proto", "https")
|
||||
req2.Header.Set("X-Forwarded-Host", "example.com")
|
||||
|
||||
// Add cookies from step 1
|
||||
for _, cookie := range cookies {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
|
||||
// Get session in callback
|
||||
session2, err := sessionManager.GetSession(req2)
|
||||
require.NoError(t, err)
|
||||
|
||||
// CSRF token should persist
|
||||
retrievedCSRF := session2.GetCSRF()
|
||||
assert.Equal(t, csrfToken, retrievedCSRF, "CSRF token must persist through OAuth flow in reverse proxy")
|
||||
assert.Equal(t, "proxy-nonce", session2.GetNonce(), "Nonce should also persist")
|
||||
assert.Equal(t, "/protected", session2.GetIncomingPath(), "Incoming path should persist")
|
||||
})
|
||||
|
||||
t.Run("Session_Cookie_Domain_With_Proxy_Headers", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Test with X-Forwarded-Host header
|
||||
req := httptest.NewRequest("GET", "http://internal.local/test", nil)
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
req.Header.Set("X-Forwarded-Host", "public.example.com")
|
||||
req.Host = "internal.local" // Internal host
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
session.SetCSRF("domain-test-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies := rec.Result().Cookies()
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie)
|
||||
|
||||
// Domain should be set based on X-Forwarded-Host when present
|
||||
// This ensures cookies work correctly with the public domain
|
||||
assert.Equal(t, "public.example.com", mainCookie.Domain, "Cookie domain should use forwarded host")
|
||||
})
|
||||
}
|
||||
|
||||
// TestAzureOIDCWithReverseProxy simulates Azure OIDC flow behind a reverse proxy
|
||||
func TestAzureOIDCWithReverseProxy(t *testing.T) {
|
||||
t.Run("Azure_Provider_Detection_And_Configuration", func(t *testing.T) {
|
||||
// This test verifies Azure-specific provider detection and configuration
|
||||
// without making actual network calls
|
||||
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Step 1: Test session setup for Azure OAuth flow
|
||||
req := httptest.NewRequest("GET", "http://internal/protected", nil)
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
req.Header.Set("X-Forwarded-Host", "app.example.com")
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Simulate OAuth flow initialization
|
||||
csrfToken := "azure-csrf-token"
|
||||
nonce := "azure-nonce"
|
||||
session.SetCSRF(csrfToken)
|
||||
session.SetNonce(nonce)
|
||||
session.SetIncomingPath("/protected")
|
||||
session.MarkDirty()
|
||||
|
||||
// Save session with proper HTTPS detection
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify cookies have correct security attributes for Azure
|
||||
cookies := rec.Result().Cookies()
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie, "Main session cookie must be set")
|
||||
assert.True(t, mainCookie.Secure, "Cookie must be secure for HTTPS reverse proxy")
|
||||
assert.Equal(t, http.SameSiteLaxMode, mainCookie.SameSite, "Should use Lax SameSite for OAuth compatibility")
|
||||
|
||||
// Step 2: Simulate callback and verify session persistence
|
||||
callbackReq := httptest.NewRequest("GET", "http://internal/oidc/callback?code=azure-code&state="+csrfToken, nil)
|
||||
callbackReq.Header.Set("X-Forwarded-Proto", "https")
|
||||
callbackReq.Header.Set("X-Forwarded-Host", "app.example.com")
|
||||
|
||||
// Add cookies from initial request
|
||||
for _, cookie := range cookies {
|
||||
callbackReq.AddCookie(cookie)
|
||||
}
|
||||
|
||||
// Get session in callback
|
||||
callbackSession, err := sessionManager.GetSession(callbackReq)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify session data persisted correctly
|
||||
assert.Equal(t, csrfToken, callbackSession.GetCSRF(), "CSRF token must persist in Azure flow")
|
||||
assert.Equal(t, nonce, callbackSession.GetNonce(), "Nonce must persist")
|
||||
assert.Equal(t, "/protected", callbackSession.GetIncomingPath(), "Original path must persist")
|
||||
})
|
||||
|
||||
t.Run("Mixed_HTTP_HTTPS_Requests", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Simulate a scenario where some requests come via HTTPS proxy and some don't
|
||||
// This can happen in development or misconfigured environments
|
||||
|
||||
// Request 1: HTTPS via proxy
|
||||
req1 := httptest.NewRequest("GET", "http://localhost:8080/test", nil)
|
||||
req1.Header.Set("X-Forwarded-Proto", "https")
|
||||
|
||||
session1, err := sessionManager.GetSession(req1)
|
||||
require.NoError(t, err)
|
||||
session1.SetCSRF("mixed-csrf")
|
||||
|
||||
rec1 := httptest.NewRecorder()
|
||||
err = session1.Save(req1, rec1)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies1 := rec1.Result().Cookies()
|
||||
|
||||
// Request 2: Direct HTTP (no proxy headers)
|
||||
req2 := httptest.NewRequest("GET", "http://localhost:8080/test", nil)
|
||||
// No X-Forwarded-Proto header
|
||||
|
||||
// Try to use cookies from HTTPS request
|
||||
for _, cookie := range cookies1 {
|
||||
// Remove Secure flag to simulate browser behavior
|
||||
// (browser wouldn't send secure cookie over HTTP)
|
||||
if !cookie.Secure {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
}
|
||||
|
||||
session2, err := sessionManager.GetSession(req2)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Session should be empty because secure cookies weren't sent
|
||||
csrf2 := session2.GetCSRF()
|
||||
assert.Empty(t, csrf2, "CSRF should be empty when secure cookies can't be sent over HTTP")
|
||||
})
|
||||
}
|
||||
|
||||
// TestEnhanceSessionSecurity verifies the security enhancement function
|
||||
func TestEnhanceSessionSecurity(t *testing.T) {
|
||||
t.Run("Security_Enhancement_For_AJAX_Requests", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// AJAX request via HTTPS proxy
|
||||
req := httptest.NewRequest("GET", "http://internal/api/data", nil)
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
req.Header.Set("X-Requested-With", "XMLHttpRequest")
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
session.SetCSRF("ajax-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Check that AJAX requests get strict same-site
|
||||
cookies := rec.Result().Cookies()
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
assert.Equal(t, http.SameSiteStrictMode, cookie.SameSite, "AJAX requests should use Strict SameSite")
|
||||
break
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Security_Enhancement_Missing_User_Agent", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Request without User-Agent (potential bot/attack)
|
||||
req := httptest.NewRequest("GET", "http://example.com/test", nil)
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
// No User-Agent header
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
session.SetCSRF("no-ua-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify reduced session timeout for suspicious requests
|
||||
cookies := rec.Result().Cookies()
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
// Should have reduced MaxAge (half of normal)
|
||||
assert.Less(t, cookie.MaxAge, int(absoluteSessionTimeout.Seconds()), "Suspicious requests should have reduced timeout")
|
||||
break
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
+6
-6
@@ -197,7 +197,7 @@ func TestCacheMemoryExhaustion(t *testing.T) {
|
||||
// TestSessionConcurrencyProtection tests session safety under concurrent access
|
||||
func TestSessionConcurrencyProtection(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sessionManager, err := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, logger)
|
||||
sessionManager, err := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -327,12 +327,12 @@ func TestProviderFailureRecovery(t *testing.T) {
|
||||
var requestCount int64
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
count := atomic.AddInt64(&requestCount, 1)
|
||||
if count <= 3 {
|
||||
// Fail first 3 requests
|
||||
if count <= 1 {
|
||||
// Fail first request only (we now retry max 2 times)
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
// Succeed after 3 failures
|
||||
// Succeed on 2nd attempt
|
||||
metadata := ProviderMetadata{
|
||||
Issuer: "https://test-issuer.com",
|
||||
AuthURL: "https://test-issuer.com/auth",
|
||||
@@ -361,8 +361,8 @@ func TestProviderFailureRecovery(t *testing.T) {
|
||||
t.Errorf("Expected metadata to be returned after recovery")
|
||||
}
|
||||
|
||||
// Should have taken some time due to retries (at least the sum of delays: 10ms + 20ms + 40ms = 70ms)
|
||||
expectedMinDuration := 70 * time.Millisecond
|
||||
// Should have taken some time due to retries (with 2 max attempts: ~10ms delay)
|
||||
expectedMinDuration := 10 * time.Millisecond
|
||||
if duration < expectedMinDuration {
|
||||
t.Errorf("Expected discovery to take at least %v due to retries, but took %v", expectedMinDuration, duration)
|
||||
}
|
||||
|
||||
@@ -461,7 +461,7 @@ func TestMissingClaims(t *testing.T) {
|
||||
// TestSessionFixationAttack tests the plugin's resistance to session fixation attacks
|
||||
func TestSessionFixationAttack(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, logger)
|
||||
sm, err := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -615,7 +615,7 @@ func TestSessionFixationAttack(t *testing.T) {
|
||||
// TestCSRFProtection tests CSRF protection in POST requests
|
||||
func TestCSRFProtection(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, logger)
|
||||
sm, err := NewSessionManager("test-secret-key-that-is-at-least-32-bytes", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
+137
-10
@@ -243,17 +243,21 @@ type SessionManager struct {
|
||||
store sessions.Store
|
||||
logger *Logger
|
||||
forceHTTPS bool
|
||||
cookieDomain string
|
||||
chunkManager *ChunkManager
|
||||
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.
|
||||
// Parameters:
|
||||
// - encryptionKey: Key used to encrypt session data (must be at least 32 bytes)
|
||||
// - forceHTTPS: When true, forces secure cookie attributes regardless of request scheme
|
||||
// - cookieDomain: Explicit domain for cookies (e.g., ".example.com" for subdomain access)
|
||||
// - logger: Logger instance for recording session-related events
|
||||
//
|
||||
// Returns an error if the encryption key does not meet minimum length requirements.
|
||||
func NewSessionManager(encryptionKey string, forceHTTPS bool, logger *Logger) (*SessionManager, error) {
|
||||
func NewSessionManager(encryptionKey string, forceHTTPS bool, cookieDomain string, logger *Logger) (*SessionManager, error) {
|
||||
// Validate encryption key length.
|
||||
if len(encryptionKey) < minEncryptionKeyLength {
|
||||
return nil, fmt.Errorf("encryption key must be at least %d bytes long", minEncryptionKeyLength)
|
||||
@@ -262,6 +266,7 @@ func NewSessionManager(encryptionKey string, forceHTTPS bool, logger *Logger) (*
|
||||
sm := &SessionManager{
|
||||
store: sessions.NewCookieStore([]byte(encryptionKey)),
|
||||
forceHTTPS: forceHTTPS,
|
||||
cookieDomain: cookieDomain,
|
||||
logger: logger,
|
||||
chunkManager: NewChunkManager(logger),
|
||||
}
|
||||
@@ -497,13 +502,14 @@ func (sm *SessionManager) EnhanceSessionSecurity(options *sessions.Options, r *h
|
||||
// Apply stricter security settings for production
|
||||
if r.Header.Get("X-Forwarded-Proto") == "https" || r.TLS != nil || sm.forceHTTPS {
|
||||
options.Secure = true
|
||||
// Enable strict same-site policy for secure connections
|
||||
options.SameSite = http.SameSiteStrictMode
|
||||
// IMPORTANT: Keep SameSite as Lax for OAuth flows to work
|
||||
// Strict mode would block cookies from OAuth provider callbacks
|
||||
// Only use Strict for AJAX requests which don't involve redirects
|
||||
}
|
||||
|
||||
// Additional security headers analysis
|
||||
if r.Header.Get("X-Requested-With") == "XMLHttpRequest" {
|
||||
// AJAX requests get stricter same-site policy
|
||||
// AJAX requests get stricter same-site policy since they don't involve OAuth redirects
|
||||
options.SameSite = http.SameSiteStrictMode
|
||||
}
|
||||
}
|
||||
@@ -511,16 +517,32 @@ func (sm *SessionManager) EnhanceSessionSecurity(options *sessions.Options, r *h
|
||||
// Always enforce security best practices
|
||||
options.HttpOnly = true
|
||||
|
||||
// Set secure Domain attribute for production
|
||||
if options.Domain == "" && r != nil {
|
||||
// Extract domain from Host header for proper cookie scoping
|
||||
// Set cookie domain from configuration if specified
|
||||
if sm.cookieDomain != "" {
|
||||
// Use the explicitly configured domain
|
||||
options.Domain = sm.cookieDomain
|
||||
sm.logger.Debugf("Using configured cookie domain: %s", sm.cookieDomain)
|
||||
} else if options.Domain == "" && r != nil {
|
||||
// Fall back to auto-detection only if no domain is configured
|
||||
// In reverse proxy setups, use the original host if available
|
||||
// This ensures cookies work correctly with the public domain
|
||||
host := r.Host
|
||||
|
||||
// Check for X-Forwarded-Host which indicates reverse proxy
|
||||
if forwardedHost := r.Header.Get("X-Forwarded-Host"); forwardedHost != "" {
|
||||
// Use the forwarded host for cookie domain
|
||||
host = forwardedHost
|
||||
}
|
||||
|
||||
if host != "" && !strings.Contains(host, "localhost") && !strings.Contains(host, "127.0.0.1") {
|
||||
// Only set domain for non-local development
|
||||
if colonIndex := strings.Index(host, ":"); colonIndex != -1 {
|
||||
host = host[:colonIndex] // Remove port
|
||||
}
|
||||
// Use the exact host (without leading dot) for host-only cookies
|
||||
// This prevents subdomain access but ensures consistency
|
||||
options.Domain = host
|
||||
sm.logger.Debugf("Auto-detected cookie domain: %s", host)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -543,10 +565,107 @@ func (sm *SessionManager) getSessionOptions(isSecure bool) *sessions.Options {
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
MaxAge: int(absoluteSessionTimeout.Seconds()),
|
||||
Path: "/",
|
||||
Domain: sm.cookieDomain, // Use configured domain if set
|
||||
}
|
||||
return baseOptions
|
||||
}
|
||||
|
||||
// CleanupOldCookies attempts to remove cookies with potentially mismatched domains.
|
||||
// Since browsers don't send domain info with cookies, this function tries to delete
|
||||
// cookies with various domain variations to ensure cleanup after configuration changes.
|
||||
func (sm *SessionManager) CleanupOldCookies(w http.ResponseWriter, r *http.Request) {
|
||||
// Get all cookies from the request
|
||||
cookies := r.Cookies()
|
||||
|
||||
// Get the current configured domain
|
||||
currentDomain := sm.cookieDomain
|
||||
host := r.Host
|
||||
if forwardedHost := r.Header.Get("X-Forwarded-Host"); forwardedHost != "" {
|
||||
host = forwardedHost
|
||||
}
|
||||
// Remove port if present
|
||||
if colonIndex := strings.Index(host, ":"); colonIndex != -1 {
|
||||
host = host[:colonIndex]
|
||||
}
|
||||
|
||||
// Build list of domains to attempt deletion for
|
||||
// This ensures we clean up cookies from various possible domains
|
||||
var domainsToClean []string
|
||||
|
||||
// Add variations of the current host
|
||||
if host != "" && !strings.Contains(host, "localhost") && !strings.Contains(host, "127.0.0.1") {
|
||||
domainsToClean = append(domainsToClean,
|
||||
host, // exact host
|
||||
"."+host, // with leading dot
|
||||
)
|
||||
|
||||
// Add parent domain variations if host has subdomains
|
||||
parts := strings.Split(host, ".")
|
||||
if len(parts) > 2 {
|
||||
// For "sub.example.com", also try "example.com" and ".example.com"
|
||||
parentDomain := strings.Join(parts[len(parts)-2:], ".")
|
||||
domainsToClean = append(domainsToClean,
|
||||
parentDomain,
|
||||
"."+parentDomain,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Track which cookies we're processing
|
||||
processedCookies := make(map[string]bool)
|
||||
|
||||
// Check each cookie
|
||||
for _, cookie := range cookies {
|
||||
// Only process our OIDC session cookies
|
||||
if strings.HasPrefix(cookie.Name, mainCookieName) ||
|
||||
strings.HasPrefix(cookie.Name, accessTokenCookie) ||
|
||||
strings.HasPrefix(cookie.Name, refreshTokenCookie) ||
|
||||
strings.HasPrefix(cookie.Name, "_oidc_raczylo_id") ||
|
||||
strings.HasPrefix(cookie.Name, "access_token_chunk_") ||
|
||||
strings.HasPrefix(cookie.Name, "refresh_token_chunk_") {
|
||||
|
||||
// Mark this cookie as seen
|
||||
processedCookies[cookie.Name] = true
|
||||
|
||||
// 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
|
||||
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 {
|
||||
continue
|
||||
}
|
||||
|
||||
// Create a deletion cookie for this domain variation
|
||||
deleteCookie := &http.Cookie{
|
||||
Name: cookie.Name,
|
||||
Value: "",
|
||||
Path: "/",
|
||||
Domain: domain,
|
||||
MaxAge: -1,
|
||||
HttpOnly: true,
|
||||
Secure: r.Header.Get("X-Forwarded-Proto") == "https" || r.TLS != nil || sm.forceHTTPS,
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
}
|
||||
http.SetCookie(w, deleteCookie)
|
||||
sm.logger.Debugf("Attempting to clean up cookie %s with domain %s", cookie.Name, domain)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Mark cleanup as done for this session manager instance
|
||||
if !sm.cleanupDone && len(processedCookies) > 0 {
|
||||
sm.cleanupMutex.Lock()
|
||||
sm.cleanupDone = true
|
||||
sm.cleanupMutex.Unlock()
|
||||
}
|
||||
}
|
||||
|
||||
// GetSession retrieves all session data for the current request.
|
||||
// It loads the main session and token sessions, including any chunked token data,
|
||||
// and combines them into a single SessionData structure for easy access.
|
||||
@@ -559,6 +678,8 @@ func (sm *SessionManager) GetSession(r *http.Request) (*SessionData, error) {
|
||||
sessionData.dirty = false
|
||||
|
||||
var sessionReturned bool
|
||||
// CRITICAL FIX: Defer logic to return session to pool only on panic, not on successful return
|
||||
// This prevents memory leaks by ensuring sessions are only pooled when not in use by caller
|
||||
defer func() {
|
||||
if !sessionReturned && sessionData != nil {
|
||||
if r := recover(); r != nil {
|
||||
@@ -624,7 +745,9 @@ func (sm *SessionManager) GetSession(r *http.Request) (*SessionData, error) {
|
||||
sm.getTokenChunkSessions(r, refreshTokenCookie, sessionData.refreshTokenChunks)
|
||||
sm.getTokenChunkSessions(r, idTokenCookie, sessionData.idTokenChunks)
|
||||
|
||||
sessionReturned = false
|
||||
// CRITICAL FIX: Mark session as successfully returned to caller - prevents premature pool return
|
||||
// This fixes memory leak where sessions were incorrectly returned to pool immediately after success
|
||||
sessionReturned = true
|
||||
return sessionData, nil
|
||||
}
|
||||
|
||||
@@ -719,10 +842,14 @@ func (sd *SessionData) MarkDirty() {
|
||||
// Returns:
|
||||
// - An error if saving any of the session components fails.
|
||||
func (sd *SessionData) Save(r *http.Request, w http.ResponseWriter) error {
|
||||
isSecure := strings.HasPrefix(r.URL.Scheme, "https") || sd.manager.forceHTTPS
|
||||
// Properly detect HTTPS in reverse proxy environments
|
||||
isSecure := r.Header.Get("X-Forwarded-Proto") == "https" || r.TLS != nil || sd.manager.forceHTTPS
|
||||
|
||||
// Set options for all sessions.
|
||||
// Get base options and enhance with security settings
|
||||
options := sd.manager.getSessionOptions(isSecure)
|
||||
options = sd.manager.EnhanceSessionSecurity(options, r)
|
||||
|
||||
// Apply enhanced options to all sessions
|
||||
sd.mainSession.Options = options
|
||||
sd.accessSession.Options = options
|
||||
sd.refreshSession.Options = options
|
||||
|
||||
@@ -0,0 +1,199 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"crypto/tls"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestSessionCompatibilityAfterChanges ensures our session changes maintain backward compatibility
|
||||
func TestSessionCompatibilityAfterChanges(t *testing.T) {
|
||||
t.Run("Plain_HTTP_Without_Proxy_Headers", func(t *testing.T) {
|
||||
// Test that plain HTTP requests without proxy headers still work
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
req := httptest.NewRequest("GET", "http://localhost:8080/test", nil)
|
||||
req.Header.Set("User-Agent", "Mozilla/5.0")
|
||||
// No X-Forwarded-Proto header
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
session.SetCSRF("plain-http-csrf")
|
||||
session.SetAuthenticated(true)
|
||||
session.SetEmail("[email protected]")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify cookies work for plain HTTP
|
||||
cookies := rec.Result().Cookies()
|
||||
require.NotEmpty(t, cookies)
|
||||
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie)
|
||||
|
||||
// Plain HTTP should NOT have Secure flag
|
||||
assert.False(t, mainCookie.Secure, "Plain HTTP should not have Secure flag")
|
||||
// Should use Lax for compatibility
|
||||
assert.Equal(t, http.SameSiteLaxMode, mainCookie.SameSite, "Plain HTTP should use Lax SameSite")
|
||||
|
||||
// Verify session can be retrieved
|
||||
req2 := httptest.NewRequest("GET", "http://localhost:8080/test2", nil)
|
||||
req2.Header.Set("User-Agent", "Mozilla/5.0")
|
||||
for _, cookie := range cookies {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
|
||||
session2, err := sessionManager.GetSession(req2)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Equal(t, "plain-http-csrf", session2.GetCSRF())
|
||||
assert.True(t, session2.GetAuthenticated())
|
||||
assert.Equal(t, "[email protected]", session2.GetEmail())
|
||||
})
|
||||
|
||||
t.Run("HTTPS_With_TLS_Field", func(t *testing.T) {
|
||||
// Test direct HTTPS connection (not proxied)
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
req := httptest.NewRequest("GET", "https://example.com/test", nil)
|
||||
req.Header.Set("User-Agent", "Mozilla/5.0")
|
||||
// Simulate TLS connection
|
||||
req.TLS = &tls.ConnectionState{}
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
session.SetCSRF("direct-https-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies := rec.Result().Cookies()
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie)
|
||||
|
||||
// Direct HTTPS should have Secure flag
|
||||
assert.True(t, mainCookie.Secure, "Direct HTTPS should have Secure flag")
|
||||
assert.Equal(t, http.SameSiteLaxMode, mainCookie.SameSite, "HTTPS should use Lax for OAuth compatibility")
|
||||
})
|
||||
|
||||
t.Run("ForceHTTPS_Setting", func(t *testing.T) {
|
||||
// Test forceHTTPS setting works regardless of request
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", true, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Plain HTTP request
|
||||
req := httptest.NewRequest("GET", "http://localhost/test", nil)
|
||||
req.Header.Set("User-Agent", "Mozilla/5.0")
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
session.SetCSRF("forced-https-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies := rec.Result().Cookies()
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie)
|
||||
|
||||
// With forceHTTPS, even HTTP requests get Secure cookies
|
||||
assert.True(t, mainCookie.Secure, "ForceHTTPS should always set Secure flag")
|
||||
})
|
||||
|
||||
t.Run("AJAX_Request_Gets_Strict_SameSite", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// AJAX request
|
||||
req := httptest.NewRequest("GET", "http://example.com/api/data", nil)
|
||||
req.Header.Set("X-Requested-With", "XMLHttpRequest")
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
req.Header.Set("User-Agent", "Mozilla/5.0")
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
session.SetCSRF("ajax-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies := rec.Result().Cookies()
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie)
|
||||
|
||||
// AJAX requests always get Strict SameSite
|
||||
assert.Equal(t, http.SameSiteStrictMode, mainCookie.SameSite, "AJAX requests should use Strict SameSite")
|
||||
})
|
||||
|
||||
t.Run("Missing_UserAgent_Gets_Reduced_Timeout", func(t *testing.T) {
|
||||
sessionManager, err := NewSessionManager("test-encryption-key-32-characters", false, "", NewLogger("debug"))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Request without User-Agent (suspicious)
|
||||
req := httptest.NewRequest("GET", "http://example.com/test", nil)
|
||||
req.Header.Set("X-Forwarded-Proto", "https")
|
||||
// No User-Agent
|
||||
|
||||
session, err := sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
session.SetCSRF("no-ua-csrf")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
cookies := rec.Result().Cookies()
|
||||
var mainCookie *http.Cookie
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == "_oidc_raczylo_m" {
|
||||
mainCookie = cookie
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, mainCookie)
|
||||
|
||||
// Should have reduced MaxAge for suspicious requests
|
||||
expectedMaxAge := int((absoluteSessionTimeout / 2).Seconds())
|
||||
assert.Equal(t, expectedMaxAge, mainCookie.MaxAge, "Missing User-Agent should get reduced timeout")
|
||||
})
|
||||
}
|
||||
+11
-11
@@ -16,7 +16,7 @@ import (
|
||||
|
||||
func TestSessionPoolMemoryLeak(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -44,7 +44,7 @@ func TestSessionPoolMemoryLeak(t *testing.T) {
|
||||
|
||||
// Test 3: Error path in GetSession
|
||||
// Modify the session store to force an error - use a different encryption key
|
||||
badSM, _ := NewSessionManager("different0123456789abcdef0123456789abcdef0123456789", false, logger)
|
||||
badSM, _ := NewSessionManager("different0123456789abcdef0123456789abcdef0123456789", false, "", logger)
|
||||
|
||||
// Get session using mismatched manager/request to force error
|
||||
_, err = badSM.GetSession(req)
|
||||
@@ -68,7 +68,7 @@ func TestSessionPoolMemoryLeak(t *testing.T) {
|
||||
|
||||
func TestSessionErrorHandling(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -95,7 +95,7 @@ func TestSessionErrorHandling(t *testing.T) {
|
||||
|
||||
func TestSessionClearAlwaysReturnsToPool(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -181,7 +181,7 @@ func getPooledObjects(sm *SessionManager) int {
|
||||
// returned to the pool in various scenarios including normal usage and error paths
|
||||
func TestSessionObjectTracking(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -344,7 +344,7 @@ func TestTokenCompressionCorruptionDetection(t *testing.T) {
|
||||
// TestTokenChunkingIntegrity tests that large tokens are properly chunked and reassembled
|
||||
func TestTokenChunkingIntegrity(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -419,7 +419,7 @@ func TestTokenChunkingIntegrity(t *testing.T) {
|
||||
// TestTokenChunkingCorruptionResistance tests handling of corrupted chunks
|
||||
func TestTokenChunkingCorruptionResistance(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -525,7 +525,7 @@ func TestTokenChunkingCorruptionResistance(t *testing.T) {
|
||||
// TestTokenSizeLimits tests that token size limits are enforced
|
||||
func TestTokenSizeLimits(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -592,7 +592,7 @@ func TestTokenSizeLimits(t *testing.T) {
|
||||
// TestConcurrentTokenOperations tests thread safety of token operations
|
||||
func TestConcurrentTokenOperations(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -646,7 +646,7 @@ func TestConcurrentTokenOperations(t *testing.T) {
|
||||
// TestSessionValidationAndCleanup tests session validation and orphan cleanup
|
||||
func TestSessionValidationAndCleanup(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -694,7 +694,7 @@ func TestSessionValidationAndCleanup(t *testing.T) {
|
||||
// TestLargeIDTokenChunking tests that large ID tokens are properly chunked across multiple cookies
|
||||
func TestLargeIDTokenChunking(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
+69
-6
@@ -26,7 +26,7 @@ type TemplatedHeader struct {
|
||||
// It provides all necessary settings to configure OpenID Connect authentication
|
||||
// with various providers like Auth0, Logto, or any standard OIDC provider.
|
||||
type Config struct {
|
||||
HTTPClient *http.Client
|
||||
HTTPClient *http.Client `json:"-"` // Exclude from JSON marshaling
|
||||
ProviderURL string `json:"providerURL"`
|
||||
RevocationURL string `json:"revocationURL"`
|
||||
CallbackURL string `json:"callbackURL"`
|
||||
@@ -43,6 +43,7 @@ type Config struct {
|
||||
AllowedUsers []string `json:"allowedUsers"`
|
||||
Scopes []string `json:"scopes"`
|
||||
Headers []TemplatedHeader `json:"headers"`
|
||||
CookieDomain string `json:"cookieDomain"`
|
||||
RateLimit int `json:"rateLimit"`
|
||||
RefreshGracePeriodSeconds int `json:"refreshGracePeriodSeconds"`
|
||||
ForceHTTPS bool `json:"forceHTTPS"`
|
||||
@@ -218,11 +219,27 @@ func (c *Config) Validate() error {
|
||||
|
||||
// validateTemplateSecure validates template expressions for security vulnerabilities
|
||||
func validateTemplateSecure(templateStr string) error {
|
||||
// Allow our specific safe custom functions
|
||||
// These are added specifically to handle missing fields safely (issue #60)
|
||||
safeCustomFunctions := []string{
|
||||
"{{get ", // Safe map access function
|
||||
"{{default ", // Safe default value function
|
||||
}
|
||||
|
||||
// Check if template uses safe custom functions
|
||||
usesSafeFunctions := false
|
||||
for _, safeFn := range safeCustomFunctions {
|
||||
if strings.Contains(templateStr, safeFn) {
|
||||
usesSafeFunctions = true
|
||||
// These functions are explicitly allowed for safe field access
|
||||
}
|
||||
}
|
||||
|
||||
// Check for dangerous template functions and patterns
|
||||
// Skip certain checks if using our safe functions
|
||||
dangerousPatterns := []string{
|
||||
"{{call", // Function calls
|
||||
"{{call", // Function calls (except our safe ones)
|
||||
"{{range", // Range over arbitrary data
|
||||
"{{with", // With statements that could access unexpected data
|
||||
"{{define", // Template definitions
|
||||
"{{template", // Template inclusions
|
||||
"{{block", // Block definitions
|
||||
@@ -230,7 +247,7 @@ func validateTemplateSecure(templateStr string) error {
|
||||
"{{-", // Trim whitespace (could be used to obfuscate)
|
||||
"-}}", // Trim whitespace (could be used to obfuscate)
|
||||
"{{printf", // Printf functions
|
||||
"{{print", // Print functions
|
||||
"{{print", // Print functions (but not our safe ones)
|
||||
"{{println", // Println functions
|
||||
"{{html", // HTML functions
|
||||
"{{js", // JavaScript functions
|
||||
@@ -249,9 +266,31 @@ func validateTemplateSecure(templateStr string) error {
|
||||
"{{not", // Logical operations
|
||||
}
|
||||
|
||||
// Allow 'with' for safe conditional access
|
||||
if !strings.Contains(templateStr, "{{with .Claims") {
|
||||
dangerousPatterns = append(dangerousPatterns, "{{with")
|
||||
}
|
||||
|
||||
templateLower := strings.ToLower(templateStr)
|
||||
for _, pattern := range dangerousPatterns {
|
||||
if strings.Contains(templateLower, pattern) {
|
||||
// Skip check if it's one of our safe functions
|
||||
if usesSafeFunctions && (pattern == "{{call" || pattern == "{{print") {
|
||||
// Allow these if we're using safe functions
|
||||
continue
|
||||
}
|
||||
|
||||
// Special handling for comparison operators to avoid false positives with "get" and "default"
|
||||
if pattern == "{{ge" && (strings.Contains(templateStr, "{{get ") || strings.Contains(templateStr, "{{default ")) {
|
||||
// Skip {{ge check if we're using the safe {{get or {{default functions
|
||||
continue
|
||||
}
|
||||
|
||||
// Skip {{de checks if using {{default
|
||||
if pattern == "{{define" && strings.Contains(templateStr, "{{default ") {
|
||||
continue
|
||||
}
|
||||
|
||||
if strings.Contains(templateLower, strings.ToLower(pattern)) {
|
||||
return fmt.Errorf("dangerous template pattern detected: %s", pattern)
|
||||
}
|
||||
}
|
||||
@@ -262,6 +301,9 @@ func validateTemplateSecure(templateStr string) error {
|
||||
"{{.IdToken}}",
|
||||
"{{.RefreshToken}}",
|
||||
"{{.Claims.",
|
||||
"{{get ", // Safe custom function
|
||||
"{{default ", // Safe custom function
|
||||
"{{with ", // Safe conditional (when used with Claims)
|
||||
}
|
||||
|
||||
// Check if template contains only allowed patterns
|
||||
@@ -274,13 +316,15 @@ func validateTemplateSecure(templateStr string) error {
|
||||
}
|
||||
|
||||
if !hasAllowedPattern {
|
||||
return fmt.Errorf("template must use only allowed variables: AccessToken, IdToken, RefreshToken, or Claims.*")
|
||||
return fmt.Errorf("template must use only allowed variables: AccessToken, IdToken, RefreshToken, Claims.*, or safe functions (get, default, with)")
|
||||
}
|
||||
|
||||
// Validate claims access patterns
|
||||
if strings.Contains(templateStr, "{{.Claims.") {
|
||||
// Simple validation - ensure claims access is to known safe fields
|
||||
// This list includes standard OIDC claims and common provider-specific claims
|
||||
safeClaimsFields := map[string]bool{
|
||||
// Standard OIDC claims
|
||||
"email": true,
|
||||
"name": true,
|
||||
"given_name": true,
|
||||
@@ -293,6 +337,25 @@ func validateTemplateSecure(templateStr string) error {
|
||||
"iat": true,
|
||||
"groups": true,
|
||||
"roles": true,
|
||||
// Common custom claims
|
||||
"internal_role": true, // Custom roles field (issue #60)
|
||||
"role": true, // Alternative role field
|
||||
"department": true, // Organization info
|
||||
"organization": true, // Organization info
|
||||
// Provider-specific claims
|
||||
"realm_access": true, // Keycloak specific
|
||||
"resource_access": true, // Keycloak specific
|
||||
"oid": true, // Azure AD object ID
|
||||
"tid": true, // Azure AD tenant ID
|
||||
"upn": true, // Azure AD User Principal Name
|
||||
"hd": true, // Google hosted domain
|
||||
"picture": true, // Profile picture
|
||||
// Additional standard claims
|
||||
"locale": true, // User locale
|
||||
"zoneinfo": true, // Timezone
|
||||
"phone_number": true, // Contact info
|
||||
"email_verified": true, // Email verification status
|
||||
"updated_at": true, // Last update time
|
||||
}
|
||||
|
||||
// Extract field names from Claims access
|
||||
|
||||
@@ -0,0 +1,362 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"text/template"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestTemplateDoubleProcessing tests if template strings are being double-processed
|
||||
// This addresses the user's concern about potential double processing by the parser
|
||||
func TestTemplateDoubleProcessing(t *testing.T) {
|
||||
t.Run("Template_Strings_Not_Double_Processed", func(t *testing.T) {
|
||||
// Simulate how Traefik passes config to the plugin
|
||||
// Traefik uses YAML/TOML config which gets unmarshaled into the Config struct
|
||||
// yamlConfig example:
|
||||
// headers:
|
||||
// - name: "X-User-Email"
|
||||
// value: "{{.Claims.email}}"
|
||||
// - name: "X-User-Role"
|
||||
// value: "{{.Claims.internal_role}}"
|
||||
|
||||
// This simulates what Traefik does internally - it parses YAML/TOML and creates a Config struct
|
||||
// The template strings are NOT processed at this stage, they're just strings
|
||||
config := &Config{
|
||||
Headers: []TemplatedHeader{
|
||||
{Name: "X-User-Email", Value: "{{.Claims.email}}"},
|
||||
{Name: "X-User-Role", Value: "{{.Claims.internal_role}}"},
|
||||
},
|
||||
}
|
||||
|
||||
// Verify that template strings are still raw (not processed)
|
||||
assert.Equal(t, "{{.Claims.email}}", config.Headers[0].Value)
|
||||
assert.Equal(t, "{{.Claims.internal_role}}", config.Headers[1].Value)
|
||||
|
||||
// Now simulate what happens when the plugin initializes
|
||||
// The template strings should only be parsed once during initialization
|
||||
headerTemplates := make(map[string]*template.Template)
|
||||
|
||||
funcMap := template.FuncMap{
|
||||
"default": func(defaultVal interface{}, val interface{}) interface{} {
|
||||
if val == nil || val == "" || val == "<no value>" {
|
||||
return defaultVal
|
||||
}
|
||||
return val
|
||||
},
|
||||
"get": func(m interface{}, key string) interface{} {
|
||||
if mapVal, ok := m.(map[string]interface{}); ok {
|
||||
if val, exists := mapVal[key]; exists {
|
||||
return val
|
||||
}
|
||||
}
|
||||
return ""
|
||||
},
|
||||
}
|
||||
|
||||
for _, header := range config.Headers {
|
||||
tmpl := template.New(header.Name).Funcs(funcMap).Option("missingkey=zero")
|
||||
parsedTmpl, err := tmpl.Parse(header.Value)
|
||||
require.NoError(t, err)
|
||||
headerTemplates[header.Name] = parsedTmpl
|
||||
}
|
||||
|
||||
// Test execution with actual claims
|
||||
claims := map[string]interface{}{
|
||||
"email": "[email protected]",
|
||||
// Note: internal_role is missing
|
||||
}
|
||||
|
||||
templateData := map[string]interface{}{
|
||||
"Claims": claims,
|
||||
}
|
||||
|
||||
// Execute templates
|
||||
for headerName, tmpl := range headerTemplates {
|
||||
var buf bytes.Buffer
|
||||
err := tmpl.Execute(&buf, templateData)
|
||||
require.NoError(t, err)
|
||||
|
||||
result := buf.String()
|
||||
if headerName == "X-User-Email" {
|
||||
assert.Equal(t, "[email protected]", result)
|
||||
} else if headerName == "X-User-Role" {
|
||||
// With missingkey=zero, missing fields return "<no value>"
|
||||
assert.Equal(t, "<no value>", result)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Config_Marshaling_Preserves_Template_Syntax", func(t *testing.T) {
|
||||
// Test that marshaling/unmarshaling config doesn't affect template strings
|
||||
originalConfig := &Config{
|
||||
ProviderURL: "https://example.com",
|
||||
ClientID: "test-client",
|
||||
ClientSecret: "test-secret",
|
||||
CallbackURL: "/callback",
|
||||
SessionEncryptionKey: "test-encryption-key-32-characters",
|
||||
Headers: []TemplatedHeader{
|
||||
{Name: "X-User-Email", Value: "{{.Claims.email}}"},
|
||||
{Name: "X-User-Role", Value: "{{get .Claims \"internal_role\"}}"},
|
||||
{Name: "X-User-Dept", Value: "{{default \"unknown\" .Claims.department}}"},
|
||||
},
|
||||
}
|
||||
|
||||
// Marshal to JSON (simulating Traefik's config processing)
|
||||
jsonData, err := json.Marshal(originalConfig)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Unmarshal back
|
||||
var unmarshaledConfig Config
|
||||
err = json.Unmarshal(jsonData, &unmarshaledConfig)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify template strings are preserved exactly
|
||||
assert.Equal(t, "{{.Claims.email}}", unmarshaledConfig.Headers[0].Value)
|
||||
assert.Equal(t, `{{get .Claims "internal_role"}}`, unmarshaledConfig.Headers[1].Value)
|
||||
assert.Equal(t, `{{default "unknown" .Claims.department}}`, unmarshaledConfig.Headers[2].Value)
|
||||
})
|
||||
|
||||
t.Run("Template_Functions_Work_After_Config_Processing", func(t *testing.T) {
|
||||
// Simulate the full flow from config to execution
|
||||
jsonConfig := `{
|
||||
"providerURL": "https://example.com",
|
||||
"clientID": "test-client",
|
||||
"clientSecret": "test-secret",
|
||||
"callbackURL": "/callback",
|
||||
"sessionEncryptionKey": "test-encryption-key-32-characters",
|
||||
"headers": [
|
||||
{"name": "X-User-Email", "value": "{{.Claims.email}}"},
|
||||
{"name": "X-User-Role", "value": "{{get .Claims \"internal_role\"}}"},
|
||||
{"name": "X-User-Dept", "value": "{{default \"engineering\" .Claims.department}}"}
|
||||
]
|
||||
}`
|
||||
|
||||
var config Config
|
||||
err := json.Unmarshal([]byte(jsonConfig), &config)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Initialize templates with functions
|
||||
funcMap := template.FuncMap{
|
||||
"default": func(defaultVal interface{}, val interface{}) interface{} {
|
||||
if val == nil || val == "" || val == "<no value>" {
|
||||
return defaultVal
|
||||
}
|
||||
return val
|
||||
},
|
||||
"get": func(m interface{}, key string) interface{} {
|
||||
if mapVal, ok := m.(map[string]interface{}); ok {
|
||||
if val, exists := mapVal[key]; exists {
|
||||
return val
|
||||
}
|
||||
}
|
||||
return ""
|
||||
},
|
||||
}
|
||||
|
||||
headerTemplates := make(map[string]*template.Template)
|
||||
for _, header := range config.Headers {
|
||||
tmpl := template.New(header.Name).Funcs(funcMap).Option("missingkey=zero")
|
||||
parsedTmpl, err := tmpl.Parse(header.Value)
|
||||
require.NoError(t, err)
|
||||
headerTemplates[header.Name] = parsedTmpl
|
||||
}
|
||||
|
||||
// Test with claims
|
||||
claims := map[string]interface{}{
|
||||
"email": "[email protected]",
|
||||
// internal_role and department are missing
|
||||
}
|
||||
|
||||
templateData := map[string]interface{}{
|
||||
"Claims": claims,
|
||||
}
|
||||
|
||||
results := make(map[string]string)
|
||||
for headerName, tmpl := range headerTemplates {
|
||||
var buf bytes.Buffer
|
||||
err := tmpl.Execute(&buf, templateData)
|
||||
require.NoError(t, err)
|
||||
results[headerName] = buf.String()
|
||||
}
|
||||
|
||||
// Verify results
|
||||
assert.Equal(t, "[email protected]", results["X-User-Email"])
|
||||
assert.Equal(t, "", results["X-User-Role"]) // get function returns empty string
|
||||
assert.Equal(t, "engineering", results["X-User-Dept"]) // default function provides fallback
|
||||
})
|
||||
}
|
||||
|
||||
// TestTemplateIntegrationWithPlugin tests template processing in the actual plugin
|
||||
func TestTemplateIntegrationWithPlugin(t *testing.T) {
|
||||
t.Run("Plugin_Handles_Missing_Claims_Safely", func(t *testing.T) {
|
||||
// Set up test OIDC server
|
||||
var testServerURL string
|
||||
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.URL.Path {
|
||||
case "/.well-known/openid-configuration":
|
||||
json.NewEncoder(w).Encode(map[string]interface{}{
|
||||
"issuer": testServerURL,
|
||||
"authorization_endpoint": testServerURL + "/auth",
|
||||
"token_endpoint": testServerURL + "/token",
|
||||
"jwks_uri": testServerURL + "/jwks",
|
||||
"userinfo_endpoint": testServerURL + "/userinfo",
|
||||
})
|
||||
case "/jwks":
|
||||
json.NewEncoder(w).Encode(map[string]interface{}{
|
||||
"keys": []interface{}{},
|
||||
})
|
||||
default:
|
||||
http.NotFound(w, r)
|
||||
}
|
||||
}))
|
||||
defer testServer.Close()
|
||||
testServerURL = testServer.URL
|
||||
|
||||
// Create config with templates that reference potentially missing fields
|
||||
config := &Config{
|
||||
ProviderURL: testServer.URL,
|
||||
ClientID: "test-client",
|
||||
ClientSecret: "test-secret",
|
||||
CallbackURL: "/callback",
|
||||
SessionEncryptionKey: "test-encryption-key-32-characters",
|
||||
Headers: []TemplatedHeader{
|
||||
{Name: "X-User-Email", Value: "{{.Claims.email}}"},
|
||||
{Name: "X-User-Role", Value: "{{.Claims.internal_role}}"},
|
||||
},
|
||||
}
|
||||
|
||||
// Initialize plugin
|
||||
ctx := context.Background()
|
||||
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
// Check headers set by the plugin
|
||||
email := r.Header.Get("X-User-Email")
|
||||
role := r.Header.Get("X-User-Role")
|
||||
|
||||
// Write headers to response for testing
|
||||
w.Header().Set("X-Test-Email", email)
|
||||
w.Header().Set("X-Test-Role", role)
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})
|
||||
|
||||
handler, err := New(ctx, next, config, "test-plugin")
|
||||
require.NoError(t, err)
|
||||
|
||||
traefikOidc, ok := handler.(*TraefikOidc)
|
||||
require.True(t, ok)
|
||||
|
||||
// Create a mock session with claims
|
||||
req := httptest.NewRequest("GET", "/protected", nil)
|
||||
|
||||
// Create session and set authentication
|
||||
session, err := traefikOidc.sessionManager.GetSession(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set authentication with minimal claims (missing internal_role)
|
||||
session.SetAuthenticated(true)
|
||||
session.SetEmail("[email protected]")
|
||||
|
||||
// Create ID token with limited claims
|
||||
claims := map[string]interface{}{
|
||||
"email": "[email protected]",
|
||||
"sub": "user123",
|
||||
// internal_role is missing
|
||||
}
|
||||
|
||||
// Create a simple test JWT (signature verification is mocked in tests)
|
||||
idToken, _ := createTestJWT(nil, "test-issuer", "test-client", claims)
|
||||
session.SetIDToken(idToken)
|
||||
|
||||
// Save session
|
||||
rec := httptest.NewRecorder()
|
||||
err = session.Save(req, rec)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create new request with session cookie
|
||||
cookies := rec.Result().Cookies()
|
||||
req2 := httptest.NewRequest("GET", "/protected", nil)
|
||||
for _, cookie := range cookies {
|
||||
req2.AddCookie(cookie)
|
||||
}
|
||||
|
||||
// Process request through plugin
|
||||
rec2 := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec2, req2)
|
||||
|
||||
// Plugin should handle missing claims gracefully
|
||||
// The request should proceed without errors
|
||||
assert.NotEqual(t, http.StatusInternalServerError, rec2.Code)
|
||||
})
|
||||
}
|
||||
|
||||
// Removed createTestJWT as it already exists in main_test.go
|
||||
|
||||
// TestTemplateSyntaxValidation tests that template syntax is properly validated
|
||||
func TestTemplateSyntaxValidation(t *testing.T) {
|
||||
t.Run("Valid_Template_Syntax", func(t *testing.T) {
|
||||
validTemplates := []string{
|
||||
"{{.Claims.email}}",
|
||||
"{{.Claims.internal_role}}",
|
||||
"{{.AccessToken}}",
|
||||
"{{.IdToken}}",
|
||||
"{{.RefreshToken}}",
|
||||
}
|
||||
|
||||
for _, tmplStr := range validTemplates {
|
||||
err := validateTemplateSecure(tmplStr)
|
||||
assert.NoError(t, err, "Template should be valid: %s", tmplStr)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Invalid_Template_Syntax_Blocked", func(t *testing.T) {
|
||||
invalidTemplates := []struct {
|
||||
template string
|
||||
reason string
|
||||
}{
|
||||
{"{{call .SomeFunc}}", "function calls not allowed"},
|
||||
{"{{range .Items}}{{.}}{{end}}", "range not allowed"},
|
||||
{"{{with .Data}}{{.Field}}{{end}}", "with statements blocked"},
|
||||
{"{{index .Array 0}}", "index access blocked"},
|
||||
{"{{printf \"%s\" .Data}}", "printf blocked"},
|
||||
}
|
||||
|
||||
for _, tc := range invalidTemplates {
|
||||
err := validateTemplateSecure(tc.template)
|
||||
assert.Error(t, err, "Template should be invalid: %s (%s)", tc.template, tc.reason)
|
||||
assert.Contains(t, strings.ToLower(err.Error()), "dangerous")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Template_With_Custom_Functions", func(t *testing.T) {
|
||||
// These templates use our safe custom functions which are now allowed
|
||||
templates := []string{
|
||||
"{{get .Claims \"internal_role\"}}",
|
||||
"{{default \"guest\" .Claims.role}}",
|
||||
}
|
||||
|
||||
// These safe custom functions should now be allowed
|
||||
for _, tmplStr := range templates {
|
||||
err := validateTemplateSecure(tmplStr)
|
||||
assert.NoError(t, err, "Safe custom functions should be allowed: %s", tmplStr)
|
||||
}
|
||||
|
||||
// But other function calls should still be blocked
|
||||
dangerousFunctions := []string{
|
||||
"{{call .SomeFunc}}",
|
||||
"{{index .Array 0}}",
|
||||
"{{slice .Data 0 10}}",
|
||||
}
|
||||
|
||||
for _, tmplStr := range dangerousFunctions {
|
||||
err := validateTemplateSecure(tmplStr)
|
||||
assert.Error(t, err, "Dangerous function calls should still be blocked: %s", tmplStr)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,206 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"testing"
|
||||
"text/template"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestTemplatedHeaderMissingField tests that accessing non-existent claim fields doesn't cause panics (issue #60)
|
||||
func TestTemplatedHeaderMissingField(t *testing.T) {
|
||||
t.Run("Missing_Claim_Field_Returns_Empty", func(t *testing.T) {
|
||||
// Create a template with the missingkey=zero option
|
||||
funcMap := template.FuncMap{
|
||||
"default": func(defaultVal interface{}, val interface{}) interface{} {
|
||||
if val == nil || val == "" {
|
||||
return defaultVal
|
||||
}
|
||||
return val
|
||||
},
|
||||
"get": func(m interface{}, key string) interface{} {
|
||||
if mapVal, ok := m.(map[string]interface{}); ok {
|
||||
if val, exists := mapVal[key]; exists {
|
||||
return val
|
||||
}
|
||||
}
|
||||
return ""
|
||||
},
|
||||
}
|
||||
|
||||
tmpl := template.New("test").Funcs(funcMap).Option("missingkey=zero")
|
||||
parsed, err := tmpl.Parse("{{.Claims.internal_role}}")
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create template data with claims that don't have internal_role
|
||||
claims := map[string]interface{}{
|
||||
"email": "[email protected]",
|
||||
"sub": "1234567890",
|
||||
"name": "John Doe",
|
||||
// Note: internal_role is NOT present
|
||||
}
|
||||
|
||||
templateData := map[string]interface{}{
|
||||
"Claims": claims,
|
||||
}
|
||||
|
||||
// Execute template - should not panic
|
||||
var buf bytes.Buffer
|
||||
err = parsed.Execute(&buf, templateData)
|
||||
require.NoError(t, err, "Template execution should not fail for missing field")
|
||||
|
||||
// Should return empty string for missing field with missingkey=zero
|
||||
assert.Equal(t, "<no value>", buf.String(), "Missing field should return <no value>")
|
||||
})
|
||||
|
||||
t.Run("Safe_Access_Pattern_For_Nested_Fields", func(t *testing.T) {
|
||||
funcMap := template.FuncMap{
|
||||
"get": func(m interface{}, key string) interface{} {
|
||||
if mapVal, ok := m.(map[string]interface{}); ok {
|
||||
if val, exists := mapVal[key]; exists {
|
||||
return val
|
||||
}
|
||||
}
|
||||
return ""
|
||||
},
|
||||
}
|
||||
|
||||
tmpl := template.New("test").Funcs(funcMap)
|
||||
// Use 'with' to safely check if field exists before accessing nested properties
|
||||
parsed, err := tmpl.Parse(`{{with .Claims.groups}}{{.admin}}{{end}}`)
|
||||
require.NoError(t, err)
|
||||
|
||||
claims := map[string]interface{}{
|
||||
"email": "[email protected]",
|
||||
// groups field doesn't exist
|
||||
}
|
||||
|
||||
templateData := map[string]interface{}{
|
||||
"Claims": claims,
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
err = parsed.Execute(&buf, templateData)
|
||||
require.NoError(t, err, "Should handle nested missing fields with 'with' construct")
|
||||
assert.Equal(t, "", buf.String(), "Should return empty string when field doesn't exist")
|
||||
})
|
||||
|
||||
t.Run("Using_Get_Function_For_Safe_Access", func(t *testing.T) {
|
||||
funcMap := template.FuncMap{
|
||||
"get": func(m interface{}, key string) interface{} {
|
||||
if mapVal, ok := m.(map[string]interface{}); ok {
|
||||
if val, exists := mapVal[key]; exists {
|
||||
return val
|
||||
}
|
||||
}
|
||||
return ""
|
||||
},
|
||||
}
|
||||
|
||||
tmpl := template.New("test").Funcs(funcMap)
|
||||
// Use the get function to safely access the field
|
||||
parsed, err := tmpl.Parse(`{{get .Claims "internal_role"}}`)
|
||||
require.NoError(t, err)
|
||||
|
||||
claims := map[string]interface{}{
|
||||
"email": "[email protected]",
|
||||
// internal_role not present
|
||||
}
|
||||
|
||||
templateData := map[string]interface{}{
|
||||
"Claims": claims,
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
err = parsed.Execute(&buf, templateData)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "", buf.String(), "get function should return empty string for missing field")
|
||||
})
|
||||
|
||||
t.Run("Using_Default_Function_For_Fallback", func(t *testing.T) {
|
||||
funcMap := template.FuncMap{
|
||||
"default": func(defaultVal interface{}, val interface{}) interface{} {
|
||||
if val == nil || val == "" || val == "<no value>" {
|
||||
return defaultVal
|
||||
}
|
||||
return val
|
||||
},
|
||||
}
|
||||
|
||||
tmpl := template.New("test").Funcs(funcMap).Option("missingkey=zero")
|
||||
// Use default to provide a fallback value
|
||||
parsed, err := tmpl.Parse(`{{default "guest" .Claims.role}}`)
|
||||
require.NoError(t, err)
|
||||
|
||||
claims := map[string]interface{}{
|
||||
"email": "[email protected]",
|
||||
// role not present
|
||||
}
|
||||
|
||||
templateData := map[string]interface{}{
|
||||
"Claims": claims,
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
err = parsed.Execute(&buf, templateData)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "guest", buf.String(), "default function should provide fallback value")
|
||||
})
|
||||
|
||||
t.Run("Existing_Field_Still_Works", func(t *testing.T) {
|
||||
funcMap := template.FuncMap{
|
||||
"get": func(m interface{}, key string) interface{} {
|
||||
if mapVal, ok := m.(map[string]interface{}); ok {
|
||||
if val, exists := mapVal[key]; exists {
|
||||
return val
|
||||
}
|
||||
}
|
||||
return ""
|
||||
},
|
||||
}
|
||||
|
||||
tmpl := template.New("test").Funcs(funcMap).Option("missingkey=zero")
|
||||
parsed, err := tmpl.Parse("{{.Claims.email}}")
|
||||
require.NoError(t, err)
|
||||
|
||||
claims := map[string]interface{}{
|
||||
"email": "[email protected]",
|
||||
"role": "admin",
|
||||
}
|
||||
|
||||
templateData := map[string]interface{}{
|
||||
"Claims": claims,
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
err = parsed.Execute(&buf, templateData)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "[email protected]", buf.String(), "Existing fields should work normally")
|
||||
})
|
||||
}
|
||||
|
||||
// TestHeaderTemplateIntegration tests the full integration of templated headers
|
||||
func TestHeaderTemplateIntegration(t *testing.T) {
|
||||
t.Run("Headers_With_Missing_Claims_Dont_Crash", func(t *testing.T) {
|
||||
config := CreateConfig()
|
||||
config.ProviderURL = "https://example.com"
|
||||
config.ClientID = "test-client"
|
||||
config.ClientSecret = "test-secret"
|
||||
config.CallbackURL = "/callback"
|
||||
config.SessionEncryptionKey = "test-encryption-key-32-characters"
|
||||
|
||||
// Add headers that reference potentially missing fields
|
||||
config.Headers = []TemplatedHeader{
|
||||
{Name: "X-User-Email", Value: "{{.Claims.email}}"},
|
||||
{Name: "X-User-Role", Value: "{{.Claims.internal_role}}"}, // This field might not exist
|
||||
{Name: "X-User-Groups", Value: "{{.Claims.groups}}"}, // This field might not exist
|
||||
}
|
||||
|
||||
// We can't fully initialize the plugin without network access,
|
||||
// but we can test that the configuration validates
|
||||
err := config.Validate()
|
||||
assert.NoError(t, err, "Configuration should be valid even with potentially missing fields")
|
||||
})
|
||||
}
|
||||
@@ -85,7 +85,7 @@ func setupTestOIDCMiddleware(t *testing.T, config *Config) (*TraefikOidc, *httpt
|
||||
logInfo: log.New(&testWriter{t}, "INFO: ", 0),
|
||||
logDebug: log.New(&testWriter{t}, "DEBUG: ", 0),
|
||||
}
|
||||
sessionManager, _ := NewSessionManager(config.SessionEncryptionKey, false, logger)
|
||||
sessionManager, _ := NewSessionManager(config.SessionEncryptionKey, false, "", logger)
|
||||
|
||||
// Create next handler
|
||||
nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
@@ -143,7 +143,7 @@ func setupTestOIDCMiddleware(t *testing.T, config *Config) (*TraefikOidc, *httpt
|
||||
forceHTTPS: config.ForceHTTPS,
|
||||
allowedUserDomains: make(map[string]struct{}),
|
||||
jwkCache: &JWKCache{},
|
||||
metadataCache: NewMetadataCache(),
|
||||
metadataCache: NewMetadataCache(nil),
|
||||
ctx: context.Background(),
|
||||
}
|
||||
|
||||
@@ -217,7 +217,7 @@ func createMockJWT(t *testing.T, sub, email string) string {
|
||||
func createTestSession() *SessionData {
|
||||
// Create a minimal session manager for testing
|
||||
logger := newNoOpLogger()
|
||||
sessionManager, _ := NewSessionManager("test-encryption-key-32-characters", false, logger)
|
||||
sessionManager, _ := NewSessionManager("test-encryption-key-32-characters", false, "", logger)
|
||||
|
||||
// Create a test request
|
||||
req := httptest.NewRequest("GET", "/", nil)
|
||||
|
||||
+1
-1
@@ -14,4 +14,4 @@ func generateRandomString(length int) string {
|
||||
return "random-string-fallback"
|
||||
}
|
||||
return hex.EncodeToString(bytes)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -17,7 +17,7 @@ import (
|
||||
// Token verified successfully multiple times, then fails with "signature verification failed"
|
||||
func TestTokenCorruptionScenario(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -180,7 +180,7 @@ func TestCompressionIntegrityFailure(t *testing.T) {
|
||||
// TestChunkReassemblyEdgeCases tests edge cases in chunk reassembly that could cause corruption
|
||||
func TestChunkReassemblyEdgeCases(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -296,7 +296,7 @@ func TestChunkReassemblyEdgeCases(t *testing.T) {
|
||||
// TestRaceConditionProtection tests that concurrent access doesn't cause corruption
|
||||
func TestRaceConditionProtection(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -432,7 +432,7 @@ func TestMemoryExhaustionProtection(t *testing.T) {
|
||||
// TestBackwardCompatibility ensures that sessions created before the fixes still work
|
||||
func TestBackwardCompatibility(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
@@ -247,7 +247,7 @@ func TestSessionIDTokenAccessToken(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
|
||||
// Create a session manager
|
||||
sessionManager, err := NewSessionManager("test-session-encryption-key-at-least-32-bytes", false, logger)
|
||||
sessionManager, err := NewSessionManager("test-session-encryption-key-at-least-32-bytes", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -318,7 +318,7 @@ func TestSessionIDTokenAccessToken(t *testing.T) {
|
||||
// TestTokenCorruptionIntegrationFlows tests the complete token handling flow with corruption scenarios
|
||||
func TestTokenCorruptionIntegrationFlows(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -442,7 +442,7 @@ func TestTokenCorruptionIntegrationFlows(t *testing.T) {
|
||||
// TestSessionPersistenceWithCorruption tests that session corruption is handled across requests
|
||||
func TestSessionPersistenceWithCorruption(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -506,7 +506,7 @@ func TestSessionPersistenceWithCorruption(t *testing.T) {
|
||||
// TestConcurrentTokenOperationsWithCorruption tests concurrent access with intentional corruption
|
||||
func TestConcurrentTokenOperationsWithCorruption(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
@@ -592,7 +592,7 @@ func TestConcurrentTokenOperationsWithCorruption(t *testing.T) {
|
||||
// TestTokenValidationEdgeCases tests edge cases in token validation
|
||||
func TestTokenValidationEdgeCases(t *testing.T) {
|
||||
logger := NewLogger("debug")
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, logger)
|
||||
sm, err := NewSessionManager("0123456789abcdef0123456789abcdef0123456789abcdef", false, "", logger)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create session manager: %v", err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user