From ccbb98b9dd8891e9f0eb4417b8ae4b4356cb6885 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Wed, 4 Mar 2026 00:23:30 +0000 Subject: [PATCH] fix-issue-122 (#128) --- .traefik.yml | 30 ++++ README.md | 24 +++ main.go | 1 + main_servehttp_test.go | 341 +++++++++++++++++++++++++++++++++++++++++ middleware.go | 16 ++ session.go | 5 + settings.go | 1 + types.go | 1 + 8 files changed, 419 insertions(+) diff --git a/.traefik.yml b/.traefik.yml index f3f65cc..9c450f2 100644 --- a/.traefik.yml +++ b/.traefik.yml @@ -123,6 +123,7 @@ testData: disableReplayDetection: false # Disable JTI replay detection for multi-replica deployments (default: false) allowPrivateIPAddresses: false # Allow private IP addresses in provider URLs for internal networks (default: false) minimalHeaders: false # Reduce forwarded headers to prevent 431 errors (default: false) + stripAuthCookies: false # Strip OIDC session cookies before forwarding to backend (default: false) # Security Headers Configuration (enabled by default with 'default' profile) securityHeaders: @@ -1021,6 +1022,35 @@ configuration: See: https://github.com/lukaszraczylo/traefikoidc/issues/64 required: false + stripAuthCookies: + type: boolean + description: | + Strip OIDC session cookies from the request before forwarding to backend services. + + When enabled, the middleware removes all cookies with the OIDC prefix (default: _oidc_raczylo_) + from the Cookie header before the request is forwarded to the backend. The cookies remain + in the browser and are still sent to Traefik for session management — they are only removed + from the Traefik-to-backend hop. + + This prevents "431 Request Header Fields Too Large" errors caused by large OIDC session + cookies (which can reach ~28KB with token chunking) being forwarded to backend services + that have limited header buffer sizes. + + Non-OIDC cookies (application sessions, preferences, etc.) are always passed through + untouched. + + Use this option when: + - Backend services return "431 Request Header Fields Too Large" errors + - OIDC session cookies are large due to token chunking + - Backend services don't need OIDC session cookies + - You want to reduce Cookie header overhead on backend requests + + Can be combined with minimalHeaders for maximum header size reduction. + + Default: false (all cookies forwarded for backward compatibility) + See: https://github.com/lukaszraczylo/traefikoidc/issues/122 + required: false + enableBackchannelLogout: type: boolean description: | diff --git a/README.md b/README.md index bfc026d..f74e761 100644 --- a/README.md +++ b/README.md @@ -154,6 +154,7 @@ The middleware supports the following configuration options: | `disableReplayDetection` | Disable JTI-based replay attack detection for multi-replica deployments | `false` | `true` | | `allowPrivateIPAddresses` | Allow private IP addresses in provider URLs (for internal networks with Keycloak, etc.) | `false` | `true` | | `minimalHeaders` | Reduce forwarded headers to prevent "431 Request Header Fields Too Large" errors | `false` | `true` | +| `stripAuthCookies` | Strip OIDC session cookies before forwarding to backend services | `false` | `true` | | `enableBackchannelLogout` | Enable OIDC Back-Channel Logout (IdP-initiated logout via server-to-server POST) | `false` | `true` | | `backchannelLogoutURL` | The path for receiving backchannel logout tokens from the IdP | none | `/backchannel-logout` | | `enableFrontchannelLogout` | Enable OIDC Front-Channel Logout (IdP-initiated logout via iframe) | `false` | `true` | @@ -1770,6 +1771,29 @@ This is particularly useful when: See [GitHub Issue #64](https://github.com/lukaszraczylo/traefikoidc/issues/64) for details. +#### Strip Auth Cookies Mode + +If your backend services return **"431 Request Header Fields Too Large"** errors due to large OIDC session cookies (which can reach ~28KB with token chunking), you can strip them before forwarding: + +```yaml +http: + middlewares: + my-auth: + plugin: + traefikoidc: + stripAuthCookies: true + # ... other config +``` + +When `stripAuthCookies: true` is set: +- **Strips**: All OIDC session cookies (`_oidc_raczylo_*`) from the request before forwarding to the backend +- **Preserves**: All non-OIDC cookies (application sessions, preferences, etc.) +- **No browser impact**: Cookies remain in the browser and are still sent to Traefik for session management + +This can be combined with `minimalHeaders: true` for maximum header size reduction. + +See [GitHub Issue #122](https://github.com/lukaszraczylo/traefikoidc/issues/122) for details. + ### Security Headers The middleware also sets the following security headers: diff --git a/main.go b/main.go index f8a2963..654da31 100644 --- a/main.go +++ b/main.go @@ -222,6 +222,7 @@ func NewWithContext(ctx context.Context, config *Config, next http.Handler, name dcrConfig: config.DynamicClientRegistration, allowPrivateIPAddresses: config.AllowPrivateIPAddresses, minimalHeaders: config.MinimalHeaders, + stripAuthCookies: config.StripAuthCookies, enableBackchannelLogout: config.EnableBackchannelLogout, enableFrontchannelLogout: config.EnableFrontchannelLogout, backchannelLogoutPath: normalizeLogoutPath(config.BackchannelLogoutURL), diff --git a/main_servehttp_test.go b/main_servehttp_test.go index e6db0ef..cf4cace 100644 --- a/main_servehttp_test.go +++ b/main_servehttp_test.go @@ -710,3 +710,344 @@ func TestMinimalHeaders_TokenHeaderNotSet(t *testing.T) { t.Error("expected X-Auth-Request-Redirect to NOT be set with minimalHeaders=true") } } + +// TestStripAuthCookies tests the stripAuthCookies configuration option. +// This addresses GitHub issue #122 - OIDC cookies bloating backend requests. +func TestStripAuthCookies(t *testing.T) { + tests := []struct { + name string + stripAuthCookies bool + expectOIDCCookies bool + expectAppCookies bool + }{ + { + name: "stripAuthCookies=false (default) forwards all cookies", + stripAuthCookies: false, + expectOIDCCookies: true, + expectAppCookies: true, + }, + { + name: "stripAuthCookies=true strips OIDC cookies but keeps app cookies", + stripAuthCookies: true, + expectOIDCCookies: false, + expectAppCookies: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var capturedCookies []*http.Cookie + + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedCookies = r.Cookies() + w.WriteHeader(http.StatusOK) + }) + + sessionManager := createTestSessionManager(t) + cookiePrefix := sessionManager.GetCookiePrefix() + + oidc := &TraefikOidc{ + next: next, + logger: NewLogger("debug"), + initComplete: make(chan struct{}), + sessionManager: sessionManager, + firstRequestReceived: true, + metadataRefreshStarted: true, + issuerURL: "https://provider.example.com", + stripAuthCookies: tt.stripAuthCookies, + extractClaimsFunc: func(token string) (map[string]interface{}, error) { + return map[string]interface{}{ + "email": "user@example.com", + }, nil + }, + } + close(oidc.initComplete) + + req := httptest.NewRequest("GET", "/protected", nil) + rw := httptest.NewRecorder() + + // Get a valid session first (before adding fake cookies) + session, err := sessionManager.GetSession(req) + if err != nil { + t.Fatalf("Failed to get session: %v", err) + } + session.SetEmail("user@example.com") + session.SetAuthenticated(true) + + // Now add OIDC session cookies (simulating what the browser would send) + req.AddCookie(&http.Cookie{Name: cookiePrefix + "m", Value: "session-data"}) + req.AddCookie(&http.Cookie{Name: cookiePrefix + "s_0", Value: "chunk0"}) + req.AddCookie(&http.Cookie{Name: cookiePrefix + "s_1", Value: "chunk1"}) + req.AddCookie(&http.Cookie{Name: cookiePrefix + "a", Value: "access-token"}) + req.AddCookie(&http.Cookie{Name: cookiePrefix + "r", Value: "refresh-token"}) + + // Add non-OIDC application cookies (these must always pass through) + req.AddCookie(&http.Cookie{Name: "my_app_session", Value: "app-session-id"}) + req.AddCookie(&http.Cookie{Name: "theme", Value: "dark"}) + + oidc.processAuthorizedRequest(rw, req, session, "https://example.com/callback") + + // Check for OIDC cookies in captured cookies + hasOIDCCookie := false + hasAppSession := false + hasTheme := false + for _, c := range capturedCookies { + if len(c.Name) >= len(cookiePrefix) && c.Name[:len(cookiePrefix)] == cookiePrefix { + hasOIDCCookie = true + } + if c.Name == "my_app_session" { + hasAppSession = true + } + if c.Name == "theme" { + hasTheme = true + } + } + + if tt.expectOIDCCookies && !hasOIDCCookie { + t.Error("expected OIDC cookies to be forwarded to backend") + } + if !tt.expectOIDCCookies && hasOIDCCookie { + t.Error("expected OIDC cookies to be stripped before forwarding to backend") + } + + if tt.expectAppCookies && !hasAppSession { + t.Error("expected my_app_session cookie to be forwarded to backend") + } + if tt.expectAppCookies && !hasTheme { + t.Error("expected theme cookie to be forwarded to backend") + } + }) + } +} + +// TestStripAuthCookies_NoCookies verifies stripping works when the request has no cookies. +func TestStripAuthCookies_NoCookies(t *testing.T) { + var capturedCookies []*http.Cookie + + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedCookies = r.Cookies() + w.WriteHeader(http.StatusOK) + }) + + sessionManager := createTestSessionManager(t) + oidc := &TraefikOidc{ + next: next, + logger: NewLogger("debug"), + initComplete: make(chan struct{}), + sessionManager: sessionManager, + firstRequestReceived: true, + metadataRefreshStarted: true, + issuerURL: "https://provider.example.com", + stripAuthCookies: true, + extractClaimsFunc: func(token string) (map[string]interface{}, error) { + return map[string]interface{}{"email": "user@example.com"}, nil + }, + } + close(oidc.initComplete) + + req := httptest.NewRequest("GET", "/protected", nil) + rw := httptest.NewRecorder() + + session, err := sessionManager.GetSession(req) + if err != nil { + t.Fatalf("Failed to get session: %v", err) + } + session.SetEmail("user@example.com") + session.SetAuthenticated(true) + + oidc.processAuthorizedRequest(rw, req, session, "https://example.com/callback") + + if len(capturedCookies) != 0 { + t.Errorf("expected no cookies, got %d", len(capturedCookies)) + } +} + +// TestStripAuthCookies_OnlyOIDCCookies verifies that when all cookies are OIDC cookies, +// the Cookie header is empty after stripping. +func TestStripAuthCookies_OnlyOIDCCookies(t *testing.T) { + var capturedCookieHeader string + var capturedCookies []*http.Cookie + + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedCookieHeader = r.Header.Get("Cookie") + capturedCookies = r.Cookies() + w.WriteHeader(http.StatusOK) + }) + + sessionManager := createTestSessionManager(t) + cookiePrefix := sessionManager.GetCookiePrefix() + + oidc := &TraefikOidc{ + next: next, + logger: NewLogger("debug"), + initComplete: make(chan struct{}), + sessionManager: sessionManager, + firstRequestReceived: true, + metadataRefreshStarted: true, + issuerURL: "https://provider.example.com", + stripAuthCookies: true, + extractClaimsFunc: func(token string) (map[string]interface{}, error) { + return map[string]interface{}{"email": "user@example.com"}, nil + }, + } + close(oidc.initComplete) + + req := httptest.NewRequest("GET", "/protected", nil) + rw := httptest.NewRecorder() + + session, err := sessionManager.GetSession(req) + if err != nil { + t.Fatalf("Failed to get session: %v", err) + } + session.SetEmail("user@example.com") + session.SetAuthenticated(true) + + // Add only OIDC cookies + req.AddCookie(&http.Cookie{Name: cookiePrefix + "m", Value: "session-data"}) + req.AddCookie(&http.Cookie{Name: cookiePrefix + "s_0", Value: "chunk0"}) + req.AddCookie(&http.Cookie{Name: cookiePrefix + "a", Value: "access-token"}) + + oidc.processAuthorizedRequest(rw, req, session, "https://example.com/callback") + + if len(capturedCookies) != 0 { + t.Errorf("expected all cookies to be stripped, got %d", len(capturedCookies)) + } + if capturedCookieHeader != "" { + t.Errorf("expected empty Cookie header, got %q", capturedCookieHeader) + } +} + +// TestStripAuthCookies_OnlyAppCookies verifies that non-OIDC cookies pass through +// untouched when stripping is enabled. +func TestStripAuthCookies_OnlyAppCookies(t *testing.T) { + var capturedCookies []*http.Cookie + + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedCookies = r.Cookies() + w.WriteHeader(http.StatusOK) + }) + + sessionManager := createTestSessionManager(t) + oidc := &TraefikOidc{ + next: next, + logger: NewLogger("debug"), + initComplete: make(chan struct{}), + sessionManager: sessionManager, + firstRequestReceived: true, + metadataRefreshStarted: true, + issuerURL: "https://provider.example.com", + stripAuthCookies: true, + extractClaimsFunc: func(token string) (map[string]interface{}, error) { + return map[string]interface{}{"email": "user@example.com"}, nil + }, + } + close(oidc.initComplete) + + req := httptest.NewRequest("GET", "/protected", nil) + rw := httptest.NewRecorder() + + session, err := sessionManager.GetSession(req) + if err != nil { + t.Fatalf("Failed to get session: %v", err) + } + session.SetEmail("user@example.com") + session.SetAuthenticated(true) + + // Add only non-OIDC cookies + req.AddCookie(&http.Cookie{Name: "my_app_session", Value: "abc123"}) + req.AddCookie(&http.Cookie{Name: "lang", Value: "en"}) + req.AddCookie(&http.Cookie{Name: "theme", Value: "dark"}) + + oidc.processAuthorizedRequest(rw, req, session, "https://example.com/callback") + + if len(capturedCookies) != 3 { + t.Errorf("expected 3 cookies, got %d", len(capturedCookies)) + } + + cookieNames := make(map[string]bool) + for _, c := range capturedCookies { + cookieNames[c.Name] = true + } + for _, expected := range []string{"my_app_session", "lang", "theme"} { + if !cookieNames[expected] { + t.Errorf("expected cookie %q to be forwarded", expected) + } + } +} + +// TestStripAuthCookies_CustomPrefix verifies stripping works with a custom cookie prefix. +func TestStripAuthCookies_CustomPrefix(t *testing.T) { + var capturedCookies []*http.Cookie + + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedCookies = r.Cookies() + w.WriteHeader(http.StatusOK) + }) + + // Create session manager with custom prefix + sm, err := NewSessionManager("test-encryption-key-32-characters", false, "", "myapp_oidc_", 0, NewLogger("debug")) + if err != nil { + t.Fatalf("Failed to create session manager: %v", err) + } + customPrefix := sm.GetCookiePrefix() + + oidc := &TraefikOidc{ + next: next, + logger: NewLogger("debug"), + initComplete: make(chan struct{}), + sessionManager: sm, + firstRequestReceived: true, + metadataRefreshStarted: true, + issuerURL: "https://provider.example.com", + stripAuthCookies: true, + extractClaimsFunc: func(token string) (map[string]interface{}, error) { + return map[string]interface{}{"email": "user@example.com"}, nil + }, + } + close(oidc.initComplete) + + req := httptest.NewRequest("GET", "/protected", nil) + rw := httptest.NewRecorder() + + session, err := sm.GetSession(req) + if err != nil { + t.Fatalf("Failed to get session: %v", err) + } + session.SetEmail("user@example.com") + session.SetAuthenticated(true) + + // Add cookies with the custom prefix (should be stripped) + req.AddCookie(&http.Cookie{Name: customPrefix + "m", Value: "session-data"}) + req.AddCookie(&http.Cookie{Name: customPrefix + "s_0", Value: "chunk0"}) + + // Add default-prefix cookie (should NOT be stripped — different prefix) + req.AddCookie(&http.Cookie{Name: "_oidc_raczylo_m", Value: "other-session"}) + + // Add app cookie (should NOT be stripped) + req.AddCookie(&http.Cookie{Name: "my_app", Value: "val"}) + + oidc.processAuthorizedRequest(rw, req, session, "https://example.com/callback") + + cookieNames := make(map[string]bool) + for _, c := range capturedCookies { + cookieNames[c.Name] = true + } + + // Custom prefix cookies should be stripped + if cookieNames[customPrefix+"m"] { + t.Errorf("expected cookie %q to be stripped", customPrefix+"m") + } + if cookieNames[customPrefix+"s_0"] { + t.Errorf("expected cookie %q to be stripped", customPrefix+"s_0") + } + + // Default prefix cookie should pass through (different prefix) + if !cookieNames["_oidc_raczylo_m"] { + t.Error("expected _oidc_raczylo_m cookie to pass through (different prefix)") + } + + // App cookie should pass through + if !cookieNames["my_app"] { + t.Error("expected my_app cookie to pass through") + } +} diff --git a/middleware.go b/middleware.go index 4bbf4af..b7e3251 100644 --- a/middleware.go +++ b/middleware.go @@ -445,6 +445,22 @@ func (t *TraefikOidc) processAuthorizedRequest(rw http.ResponseWriter, req *http rw.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin") } + // Strip OIDC session cookies before forwarding to the backend to prevent + // HTTP 431 "Request Header Fields Too Large" errors (GitHub issue #122). + if t.stripAuthCookies { + prefix := t.sessionManager.GetCookiePrefix() + filtered := make([]*http.Cookie, 0, len(req.Cookies())) + for _, c := range req.Cookies() { + if !strings.HasPrefix(c.Name, prefix) { + filtered = append(filtered, c) + } + } + req.Header.Del("Cookie") + for _, c := range filtered { + req.AddCookie(c) + } + } + t.logger.Debugf("Request authorized for user %s, forwarding to next handler", email) t.next.ServeHTTP(rw, req) diff --git a/session.go b/session.go index d977ced..e806c50 100644 --- a/session.go +++ b/session.go @@ -500,6 +500,11 @@ func (sm *SessionManager) combinedChunkCookieName(chunkIndex int) string { return fmt.Sprintf("%s_%d", sm.combinedCookieName(), chunkIndex) } +// GetCookiePrefix returns the cookie prefix used for all OIDC session cookies. +func (sm *SessionManager) GetCookiePrefix() string { + return sm.cookiePrefix +} + // Shutdown gracefully shuts down the SessionManager and all its background tasks func (sm *SessionManager) Shutdown() error { var shutdownErr error diff --git a/settings.go b/settings.go index 4f0fc83..af20edb 100644 --- a/settings.go +++ b/settings.go @@ -65,6 +65,7 @@ type Config struct { ForceHTTPS bool `json:"forceHTTPS"` AllowPrivateIPAddresses bool `json:"allowPrivateIPAddresses,omitempty"` MinimalHeaders bool `json:"minimalHeaders,omitempty"` + StripAuthCookies bool `json:"stripAuthCookies,omitempty"` EnableBackchannelLogout bool `json:"enableBackchannelLogout,omitempty"` EnableFrontchannelLogout bool `json:"enableFrontchannelLogout,omitempty"` BackchannelLogoutURL string `json:"backchannelLogoutURL,omitempty"` diff --git a/types.go b/types.go index 759470a..bd6f980 100644 --- a/types.go +++ b/types.go @@ -130,6 +130,7 @@ type TraefikOidc struct { firstRequestMutex sync.Mutex sessionInvalidationCache CacheInterface minimalHeaders bool + stripAuthCookies bool enableBackchannelLogout bool enableFrontchannelLogout bool firstRequestReceived bool