R82: req discovery endpoints, single-use code retry

error_recovery.go

@@ -521,12 +521,82 @@ func (re *RetryExecutor) ExecuteWithContext(ctx context.Context, fn func() error
 }
 
 // Execute runs the given function with retry logic (for backward compatibility)
-// Execute executes a function with retry logic (backward compatibility).
+// Execute runs a function with retry logic (backward compatibility).
 // This method provides the same functionality as ExecuteWithContext.
 func (re *RetryExecutor) Execute(ctx context.Context, fn func() error) error {
 	return re.ExecuteWithContext(ctx, fn)
 }
 
+// singleUseRetryableErrors are error fragments that prove the request was NEVER
+// sent to the server (dial/connect failures). Retrying those is safe.
+var singleUseRetryableErrors = []string{
+	"connection refused",
+	"network unreachable",
+	"no route to host",
+	"temporary failure",
+}
+
+// ExecuteSingleUseWithContext retries only on errors that prove the request was
+// never sent (connect/dial failures). It intentionally does NOT retry on
+// "timeout": for a single-use operation such as the authorization-code
+// exchange, a timeout may mean the provider already consumed the one-time code,
+// so re-sending it would surface invalid_grant permanently even though the
+// first attempt succeeded.
+func (re *RetryExecutor) ExecuteSingleUseWithContext(ctx context.Context, fn func() error) error {
+	re.RecordRequest()
+	var lastErr error
+
+	for attempt := 1; attempt <= re.config.MaxAttempts; attempt++ {
+		err := fn()
+		if err == nil {
+			if attempt > 1 {
+				re.LogInfo("Operation succeeded after %d attempts", attempt)
+			}
+			re.RecordSuccess()
+			return nil
+		}
+
+		lastErr = err
+
+		if !isSubstringMatch(err.Error(), singleUseRetryableErrors) {
+			re.RecordFailure()
+			return err
+		}
+
+		if attempt == re.config.MaxAttempts {
+			re.RecordFailure()
+			break
+		}
+
+		delay := re.calculateDelay(attempt)
+		if attempt == 1 || attempt%3 == 0 {
+			re.LogDebug("Retrying single-use operation after %v (attempt %d/%d): %v",
+				delay, attempt, re.config.MaxAttempts, err)
+		}
+
+		select {
+		case <-ctx.Done():
+			re.RecordFailure()
+			return ctx.Err()
+		case <-time.After(delay):
+		}
+	}
+
+	finalErr := fmt.Errorf("operation failed after %d attempts: %w", re.config.MaxAttempts, lastErr)
+	return finalErr
+}
+
+// isSubstringMatch reports whether s contains any of the given fragments.
+func isSubstringMatch(s string, fragments []string) bool {
+	lower := strings.ToLower(s)
+	for _, frag := range fragments {
+		if strings.Contains(lower, frag) {
+			return true
+		}
+	}
+	return false
+}
+
 // isRetryableError checks if an error should trigger a retry
 // isRetryableError determines if an error should trigger a retry attempt.
 // Checks error message against configured retryable error patterns.

helpers.go

@@ -186,7 +186,9 @@ func (t *TraefikOidc) exchangeTokens(ctx context.Context, grantType string, code
 	}
 
 	var tokenResponse TokenResponse
-	if err := json.NewDecoder(resp.Body).Decode(&tokenResponse); err != nil {
+	// Cap the decoded body so a corrupted or malicious oversized 200-body cannot
+	// drive an unbounded read/allocation. Token responses are small (a few KB).
+	if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&tokenResponse); err != nil {
 		return nil, fmt.Errorf("failed to decode token response: %w", err)
 	}
 

metadata_cache.go

@@ -146,6 +146,14 @@ func (mc *MetadataCache) GetProviderMetadata(ctx context.Context, providerURL st
 		return nil, fmt.Errorf("failed to decode metadata: %w", err)
 	}
 
+	// Require the core endpoints. An incomplete discovery document (missing
+	// authorization_endpoint / token_endpoint) would otherwise be accepted,
+	// cached, and adopted as the working auth/token URL, failing at runtime
+	// (wrong authorize redirect, token-exchange failure) instead of at startup.
+	if metadata.AuthURL == "" || metadata.TokenURL == "" {
+		return nil, fmt.Errorf("discovery document missing required endpoints (authorization_endpoint or token_endpoint)")
+	}
+
 	// Pin the advertised issuer to the configured provider host. The issuer is
 	// the trust anchor for JWT issuer validation; rejecting a mismatch here
 	// ensures a poisoned discovery document advertising an attacker-chosen

review_r82_test.go

@@ -0,0 +1,94 @@
+package traefikoidc
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+)
+
+// TestMetadataDiscovery_RequiresEndpoints is a regression: an OIDC discovery
+// document missing the core endpoints (authorization_endpoint or
+// token_endpoint) was accepted, cached, and adopted as the working
+// auth/token URL, failing only at runtime (wrong authorize redirect, token
+// exchange failure). The fix validates the endpoints at startup.
+func TestMetadataDiscovery_RequiresEndpoints(t *testing.T) {
+	mc := NewMetadataCacheWithLogger(nil, newNoOpLogger())
+
+	// Discovery document with a matching issuer but no authorization/token
+	// endpoint: the issuer check passes, so only the missing-endpoint
+	// validation can reject it.
+	var serverURL string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(map[string]interface{}{"issuer": serverURL})
+	}))
+	defer server.Close()
+	serverURL = server.URL
+
+	_, err := mc.GetProviderMetadata(context.Background(), server.URL, server.Client())
+	if err == nil {
+		t.Fatalf("expected error on an incomplete discovery document")
+	}
+	if !strings.Contains(err.Error(), "missing required endpoints") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+// TestRetryExecutor_SingleUse_DoesNotRetryTimeout guards the single-use
+// (authorization-code exchange) retry policy: a timeout is NOT retried,
+// because the provider may already have consumed the one-time code; re-sending
+// would surface invalid_grant permanently even though the first attempt
+// succeeded.
+func TestRetryExecutor_SingleUse_DoesNotRetryTimeout(t *testing.T) {
+	cfg := DefaultRetryConfig()
+	cfg.MaxAttempts = 3
+	cfg.EnableJitter = false
+	cfg.InitialDelay = time.Millisecond
+	cfg.MaxDelay = time.Millisecond
+
+	re := NewRetryExecutor(cfg, newNoOpLogger())
+	calls := 0
+	err := re.ExecuteSingleUseWithContext(context.Background(), func() error {
+		calls++
+		return errors.New("Post https://idp.example.com/token: context deadline exceeded (Client.Timeout)")
+	})
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	if calls != 1 {
+		t.Fatalf("expected single-use request to NOT retry a timeout, got %d calls", calls)
+	}
+}
+
+// TestRetryExecutor_SingleUse_RetriesPreSendError guards that single-use still
+// retries errors which prove the request was never sent (connection refused),
+// so a transient dial failure of the token endpoint does not lose the
+// (unconsumed) authorization code.
+func TestRetryExecutor_SingleUse_RetriesPreSendError(t *testing.T) {
+	cfg := DefaultRetryConfig()
+	cfg.MaxAttempts = 3
+	cfg.EnableJitter = false
+	cfg.InitialDelay = time.Millisecond
+	cfg.MaxDelay = time.Millisecond
+
+	re := NewRetryExecutor(cfg, newNoOpLogger())
+	calls := 0
+	err := re.ExecuteSingleUseWithContext(context.Background(), func() error {
+		calls++
+		if calls < 3 {
+			return errors.New("dial tcp 10.0.0.1:443: connect: connection refused")
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("expected success after retrying a connection-refused error, got %v", err)
+	}
+	if calls != 3 {
+		t.Fatalf("expected 3 attempts for a pre-send error, got %d", calls)
+	}
+}

token_resilience.go

@@ -104,8 +104,11 @@ func NewTokenResilienceManager(config TokenResilienceConfig, logger *Logger) *To
 	return manager
 }
 
-// ExecuteTokenOperation executes a token operation with full resilience support
+// ExecuteTokenOperation executes a token operation with full resilience support.
-func (trm *TokenResilienceManager) ExecuteTokenOperation(ctx context.Context, operation string, fn func() error) error {
+// singleUse marks operations whose request must not be re-sent after a timeout
+// (e.g. the authorization-code exchange, where the one-time code may already
+// have been consumed server-side) — only pre-send (connect) failures are retried.
+func (trm *TokenResilienceManager) ExecuteTokenOperation(ctx context.Context, operation string, singleUse bool, fn func() error) error {
 	if trm.logger != nil {
 		trm.logger.Debugf("Executing token operation %s with resilience", operation)
 	}
@@ -129,8 +132,14 @@ func (trm *TokenResilienceManager) ExecuteTokenOperation(ctx context.Context, op
 	// Wrap with retry if enabled
 	if trm.config.RetryEnabled && trm.retryExecutor != nil {
 		originalOp := finalOperation
-		finalOperation = func() error {
+		if singleUse {
-			return trm.retryExecutor.ExecuteWithContext(ctx, originalOp)
+			finalOperation = func() error {
+				return trm.retryExecutor.ExecuteSingleUseWithContext(ctx, originalOp)
+			}
+		} else {
+			finalOperation = func() error {
+				return trm.retryExecutor.ExecuteWithContext(ctx, originalOp)
+			}
 		}
 	}
 
@@ -152,7 +161,7 @@ func (trm *TokenResilienceManager) ExecuteTokenExchange(ctx context.Context, t *
 
 	operation := fmt.Sprintf("token_exchange_%s", grantType)
 
-	err = trm.ExecuteTokenOperation(ctx, operation, func() error {
+	err = trm.ExecuteTokenOperation(ctx, operation, grantType == "authorization_code", func() error {
 		result, err = t.exchangeTokens(ctx, grantType, codeOrToken, redirectURL, codeVerifier)
 		return err
 	})
@@ -165,7 +174,7 @@ func (trm *TokenResilienceManager) ExecuteTokenRefresh(ctx context.Context, t *T
 	var result *TokenResponse
 	var err error
 
-	err = trm.ExecuteTokenOperation(ctx, "token_refresh", func() error {
+	err = trm.ExecuteTokenOperation(ctx, "token_refresh", false, func() error {
 		// Call exchangeTokens directly to avoid recursion back to getNewTokenWithRefreshToken
 		// which would call ExecuteTokenRefresh again, causing infinite loop (issue #67)
 		result, err = t.exchangeTokens(ctx, "refresh_token", refreshToken, "", "")