R80: req secret for introspection, pool conn on panic

internal/cache/backends/redis.go

@@ -376,9 +376,14 @@ func (r *RedisBackend) executeWithRetry(ctx context.Context, operation func(*Red
 			}
 		}
 
-		// Execute the operation
-		err = operation(conn)
-		r.pool.Put(conn)
+		// Execute the operation, guaranteeing the borrowed connection is
+		// returned to the pool even if the operation panics (previously a
+		// panic here leaked the connection and desynced the pool counters;
+		// other pool call sites use defer for this).
+		func() {
+			defer func() { r.pool.Put(conn) }()
+			err = operation(conn)
+		}()
 
 		// Check context after operation - if canceled, don't bother retrying
 		if ctx.Err() != nil {

internal/cache/backends/review_r80_test.go

@@ -0,0 +1,48 @@
+package backends
+
+import (
+	"context"
+	"net"
+	"testing"
+	"time"
+)
+
+// TestExecuteWithRetry_ReturnsConnOnPanic is a regression: executeWithRetry
+// returned the borrowed connection imperatively with no defer, so an operation
+// panic leaked the Redis connection out of the pool and desynced its counters.
+// The fix guarantees the connection is returned on all exit paths including
+// panic.
+func TestExecuteWithRetry_ReturnsConnOnPanic(t *testing.T) {
+	pool, err := NewConnectionPool(&PoolConfig{MaxConnections: 1, ConnectTimeout: 50 * time.Millisecond})
+	if err != nil {
+		t.Fatalf("NewConnectionPool: %v", err)
+	}
+
+	// Seed one available connection so executeWithRetry's Get succeeds without
+	// dialing (no Redis server needed).
+	pc, sw := net.Pipe()
+	defer sw.Close()
+	rc := &RedisConn{conn: pc, readTimeout: time.Second, writeTimeout: time.Second}
+	pool.Put(rc)
+
+	if got := len(pool.connections); got != 1 {
+		t.Fatalf("setup: expected one seeded connection, got %d", got)
+	}
+
+	r := &RedisBackend{pool: pool}
+
+	func() {
+		defer func() {
+			if v := recover(); v == nil {
+				t.Fatalf("expected operation to panic")
+			}
+		}()
+		_ = r.executeWithRetry(context.Background(), func(c *RedisConn) error {
+			panic("boom")
+		})
+	}()
+
+	if got := len(pool.connections); got != 1 {
+		t.Fatalf("expected connection returned to pool after operation panic, got %d", got)
+	}
+}

review_r80_test.go

@@ -0,0 +1,43 @@
+package traefikoidc
+
+import (
+	"strings"
+	"testing"
+)
+
+// TestIntrospectionRequiresClientSecret is a regression: Config.Validate allowed
+// requireTokenIntrospection together with clientAuthMethod=private_key_jwt and no
+// client secret, but the introspection path always authenticates via
+// client_secret_basic (SetBasicAuth in token_introspection.go) — so the dead
+// config passed validation yet every introspection sent empty Basic credentials
+// and failed. Validate must reject it.
+func TestIntrospectionRequiresClientSecret(t *testing.T) {
+	mk := func() *Config {
+		c := CreateConfig()
+		c.ProviderURL = "https://provider.example.com"
+		c.ClientID = "test-client"
+		c.SessionEncryptionKey = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+		c.CallbackURL = "/callback"
+		c.RequireTokenIntrospection = true
+		c.ClientAuthMethod = "private_key_jwt"
+		c.ClientAssertionPrivateKey = "-----BEGIN PRIVATE KEY-----\nMIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg"
+		c.ClientAssertionKeyID = "key-id-1"
+		return c
+	}
+
+	// OLD behavior: no client secret -> Validate still passed, introspection dead.
+	noSecret := mk()
+	noSecret.ClientSecret = ""
+	if err := noSecret.Validate(); err == nil {
+		t.Fatalf("expected Validate to require clientSecret when introspection is enabled")
+	} else if !strings.Contains(err.Error(), "clientSecret is required when requireTokenIntrospection") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// With a secret the config is valid.
+	withSecret := mk()
+	withSecret.ClientSecret = "secret"
+	if err := withSecret.Validate(); err != nil {
+		t.Fatalf("expected Validate to pass with a client secret, got: %v", err)
+	}
+}

settings.go

@@ -459,6 +459,15 @@ func (c *Config) Validate() error {
 		}
 	}
 
+	// The token-introspection path always authenticates with client_secret_basic
+	// (token_introspection.go SetBasicAuth), regardless of clientAuthMethod.
+	// Reject the validated-but-broken config where introspection is enabled but
+	// no client secret is supplied (e.g. private_key_jwt with no secret), which
+	// would otherwise send empty Basic credentials and fail every introspection.
+	if c.RequireTokenIntrospection && c.ClientSecret == "" {
+		return fmt.Errorf("clientSecret is required when requireTokenIntrospection is enabled (introspection authenticates via client_secret_basic)")
+	}
+
 	// Validate session encryption key
 	if c.SessionEncryptionKey == "" {
 		return fmt.Errorf("sessionEncryptionKey is required")