diff --git a/README.md b/README.md index 4a5e30f..c3f143e 100644 --- a/README.md +++ b/README.md @@ -304,9 +304,11 @@ WebSocket upgrade. The middleware handles this automatically: - They are **not** unauthenticated — a valid encrypted session cookie is required, otherwise the request is rejected. The session must already exist (i.e. the user logged in via a normal HTTP page first). -- `X-Forwarded-User` is forwarded from the session. +- `X-Forwarded-User` and the configured `headers` are forwarded from the + session. Header templates render from the claims in the session cookie. - Validation is cookie-only (no JWK fetch), so streaming keeps working during - brief IdP outages. + brief IdP outages. Back-channel and front-channel logout still apply: a + session that the IdP logged out is rejected. No configuration needed — this is implicit behavior. diff --git a/cmd/yaegicheck/main.go b/cmd/yaegicheck/main.go index cf4aace..377a1d3 100644 --- a/cmd/yaegicheck/main.go +++ b/cmd/yaegicheck/main.go @@ -23,6 +23,7 @@ package main import ( "context" + "encoding/base64" "errors" "fmt" "io" @@ -137,6 +138,7 @@ func main() { runCheck("fix06-sse-flush-reaches-next", checkSSEFlushReachesNext) runCheck("middleware-erraborthandler-aborts-under-yaegi", checkErrAbortHandlerAbortsUnderYaegi) runCheck("fix17-setifabsent-claims-once", checkSetIfAbsentUnderYaegi) + runCheck("issue162-websocket-forwards-header-templates", checkIssue162WebSocketHeaderTemplates) fmt.Println("OK: all yaegi regression checks passed") } @@ -723,6 +725,87 @@ func checkSSEFlushReachesNext() (string, error) { return "", nil } +// checkIssue162WebSocketHeaderTemplates pins #162 under the interpreter: an +// authenticated WebSocket upgrade takes the streaming bypass, which must +// still render the configured headers (static and claim-derived) for next. +func checkIssue162WebSocketHeaderTemplates() (string, error) { + discovery := servers.NewOIDCServer(nil) + defer discovery.Close() + + cfg := oidc.CreateConfig() + cfg.ProviderURL = discovery.URL + cfg.ClientID = "yaegi-check-162-client" + cfg.ClientSecret = "yaegi-check-162-secret" + cfg.CallbackURL = "/oauth2/callback" + cfg.SessionEncryptionKey = "0123456789abcdef0123456789abcdef" + cfg.Headers = []oidc.TemplatedHeader{ + {Name: "X-Proxy-Secret", Value: "s3cret"}, + {Name: "X-Forwarded-Preferred-Username", Value: "{{.Claims.preferred_username}}"}, + } + + var seen http.Header + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seen = r.Header.Clone() + }) + h, err := oidc.New(context.Background(), next, cfg, "yaegi-check-162") + if err != nil { + return "", fmt.Errorf("New: %w", err) + } + if closer, ok := h.(interface{ Close() error }); ok { + defer closer.Close() + } + + sm, err := oidc.NewSessionManager(cfg.SessionEncryptionKey, cfg.ForceHTTPS, cfg.CookieDomain, cfg.CookiePrefix, time.Duration(cfg.SessionMaxAge)*time.Second, oidc.NewLogger("error")) + if err != nil { + return "", fmt.Errorf("NewSessionManager: %w", err) + } + defer sm.Shutdown() + + setupReq := httptest.NewRequest("GET", "/ws", nil) + session, err := sm.GetSession(setupReq) + if err != nil { + return "", fmt.Errorf("GetSession: %w", err) + } + if err := session.SetAuthenticated(true); err != nil { + return "", fmt.Errorf("SetAuthenticated: %w", err) + } + session.SetUserIdentifier("alice@example.com") + // Unsigned JWT: the bypass reads cookie-sealed claims without signature + // verification. Built at runtime because a JWT literal trips the + // pre-commit secret scanner. + b64 := base64.RawURLEncoding.EncodeToString + sig := make([]byte, 32) + for i := range sig { + sig[i] = byte(i + 1) + } + session.SetIDToken(b64([]byte(`{"alg":"RS256","typ":"JWT"}`)) + "." + b64([]byte(`{"preferred_username":"alice"}`)) + "." + b64(sig)) + rec := httptest.NewRecorder() + if err := session.Save(setupReq, rec); err != nil { + return "", fmt.Errorf("session.Save: %w", err) + } + session.ReturnToPool() + + req := httptest.NewRequest("GET", "/ws", nil) + for _, c := range rec.Result().Cookies() { + req.AddCookie(c) + } + req.Header.Set("Upgrade", "websocket") + req.Header.Set("Connection", "Upgrade") + rw := httptest.NewRecorder() + h.ServeHTTP(rw, req) + + if seen == nil { + return "", fmt.Errorf("next was not reached (status %d)", rw.Code) + } + if got := seen.Get("X-Proxy-Secret"); got != "s3cret" { + return "", fmt.Errorf("X-Proxy-Secret = %q, want %q", got, "s3cret") + } + if got := seen.Get("X-Forwarded-Preferred-Username"); got != "alice" { + return "", fmt.Errorf("X-Forwarded-Preferred-Username = %q, want %q", got, "alice") + } + return "", nil +} + // checkErrAbortHandlerAbortsUnderYaegi guards the re-review finding on // ServeHTTP's ErrAbortHandler branch (middleware.go): under yaegi v0.16.1, // `r == http.ErrAbortHandler` only ever matches a panic raised by COMPILED diff --git a/issue162_regression_test.go b/issue162_regression_test.go new file mode 100644 index 0000000..3117669 --- /dev/null +++ b/issue162_regression_test.go @@ -0,0 +1,240 @@ +package traefikoidc + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "text/template" + "time" +) + +// Issue #162: the SSE/WebSocket bypass forwarded only X-Forwarded-User, not +// the operator-configured `headers`. Backends that authenticate the proxy +// with a shared-secret header (Frigate proxy auth) answered 401 on /ws while +// normal requests, which go through forwardAuthorized, kept working. + +func issue162Templates(t *testing.T, headers map[string]string) map[string]*template.Template { + t.Helper() + funcMap := headerTemplateFuncMap(claimsWhitelist(nil)) + out := make(map[string]*template.Template, len(headers)) + for name, value := range headers { + tmpl, err := template.New(name).Funcs(funcMap).Option("missingkey=zero").Parse(value) + if err != nil { + t.Fatalf("parse template %s: %v", name, err) + } + out[name] = tmpl + } + return out +} + +// issue162Request builds a request carrying a saved authenticated session. +// idClaims and accessClaims are optional; nil leaves that token unset. +func issue162Request(t *testing.T, sm *SessionManager, idClaims, accessClaims map[string]interface{}) *http.Request { + t.Helper() + setupReq := httptest.NewRequest(http.MethodGet, "https://app.example.com/ws", nil) + session, err := sm.GetSession(setupReq) + if err != nil { + t.Fatalf("GetSession: %v", err) + } + if err := session.SetAuthenticated(true); err != nil { + t.Fatalf("SetAuthenticated: %v", err) + } + session.SetUserIdentifier("alice@example.com") + if idClaims != nil { + session.SetIDToken(tokenWithGroups(t, idClaims)) + } + if accessClaims != nil { + session.SetAccessToken(tokenWithGroups(t, accessClaims)) + } + rr := httptest.NewRecorder() + if err := session.Save(setupReq, rr); err != nil { + t.Fatalf("Save: %v", err) + } + session.returnToPoolSafely() + + req := httptest.NewRequest(http.MethodGet, "https://app.example.com/ws", nil) + for _, c := range rr.Result().Cookies() { + req.AddCookie(c) + } + return req +} + +func issue162Middleware(t *testing.T, sm *SessionManager, templates map[string]*template.Template, seen *http.Header) *TraefikOidc { + t.Helper() + initComplete := make(chan struct{}) + close(initComplete) + return &TraefikOidc{ + logger: GetSingletonNoOpLogger(), + name: "test", + sessionManager: sm, + initComplete: initComplete, + extractClaimsFunc: extractClaims, + groupClaimName: "groups", + headerTemplates: templates, + next: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + *seen = r.Header.Clone() + w.WriteHeader(http.StatusOK) + }), + } +} + +func issue162SessionManager(t *testing.T) *SessionManager { + t.Helper() + sm, err := NewSessionManager(strings.Repeat("k", 32), false, "", "", time.Hour, NewLogger("error")) + if err != nil { + t.Fatalf("NewSessionManager: %v", err) + } + t.Cleanup(func() { + if err := sm.Shutdown(); err != nil { + t.Errorf("Shutdown: %v", err) + } + }) + return sm +} + +func TestIssue162_StreamingBypassForwardsHeaderTemplates(t *testing.T) { + templates := map[string]string{ + "X-Proxy-Secret": "s3cret", + "X-Forwarded-Preferred-Username": "{{.Claims.preferred_username}}", + "X-Forwarded-Groups": "{{range $i, $e := .Claims.groups}}{{if $i}},{{end}}{{$e}}{{end}}", + } + idClaims := map[string]interface{}{ + "preferred_username": "alice", + "groups": []string{"admins", "viewers"}, + } + + cases := []struct { + name string + prepare func(*http.Request) + }{ + {"websocket", func(r *http.Request) { + r.Header.Set("Upgrade", "websocket") + r.Header.Set("Connection", "Upgrade") + }}, + {"sse", func(r *http.Request) { + r.Header.Set("Accept", "text/event-stream") + }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + sm := issue162SessionManager(t) + var seen http.Header + m := issue162Middleware(t, sm, issue162Templates(t, templates), &seen) + + req := issue162Request(t, sm, idClaims, nil) + tc.prepare(req) + rw := httptest.NewRecorder() + m.ServeHTTP(rw, req) + + if seen == nil { + t.Fatalf("next not invoked (status=%d body=%q)", rw.Code, rw.Body.String()) + } + want := map[string]string{ + "X-Forwarded-User": "alice@example.com", + "X-Proxy-Secret": "s3cret", + "X-Forwarded-Preferred-Username": "alice", + "X-Forwarded-Groups": "admins,viewers", + } + for h, v := range want { + if got := seen.Get(h); got != v { + t.Errorf("%s = %q, want %q", h, got, v) + } + } + }) + } +} + +// Opaque-ID-token providers: normal requests render templates from +// access-token claims when the session has no ID token, so the bypass must too. +func TestIssue162_StreamingBypassTemplatesFallBackToAccessTokenClaims(t *testing.T) { + sm := issue162SessionManager(t) + var seen http.Header + m := issue162Middleware(t, sm, issue162Templates(t, map[string]string{ + "X-Forwarded-Preferred-Username": "{{.Claims.preferred_username}}", + }), &seen) + + req := issue162Request(t, sm, nil, map[string]interface{}{"preferred_username": "bob"}) + req.Header.Set("Accept", "text/event-stream") + m.ServeHTTP(httptest.NewRecorder(), req) + + if seen == nil { + t.Fatal("next not invoked") + } + if got := seen.Get("X-Forwarded-Preferred-Username"); got != "bob" { + t.Fatalf("X-Forwarded-Preferred-Username = %q, want %q", got, "bob") + } +} + +// An unauthenticated upgrade must still be rejected before any template +// (which can carry a shared secret) is rendered onto the request. +func TestIssue162_StreamingBypassRejectsUnauthenticatedWithoutTemplates(t *testing.T) { + sm := issue162SessionManager(t) + var seen http.Header + m := issue162Middleware(t, sm, issue162Templates(t, map[string]string{"X-Proxy-Secret": "s3cret"}), &seen) + + req := httptest.NewRequest(http.MethodGet, "https://app.example.com/ws", nil) + req.Header.Set("Upgrade", "websocket") + req.Header.Set("Connection", "Upgrade") + rw := httptest.NewRecorder() + m.ServeHTTP(rw, req) + + if seen != nil { + t.Fatal("next invoked for unauthenticated upgrade") + } + if rw.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401", rw.Code) + } + if req.Header.Get("X-Proxy-Secret") != "" { + t.Fatal("shared-secret template rendered onto a rejected request") + } +} + +// Now that the bypass forwards proxy credentials (#162), it must honor +// IdP-initiated logout like the normal path does; otherwise a logged-out +// cookie keeps opening WebSocket/SSE with the shared secret attached. +func TestIssue162_StreamingBypassRejectsIdPLoggedOutSession(t *testing.T) { + idClaims := map[string]interface{}{ + "sid": "sess-162", + "sub": "alice", + "iat": float64(time.Now().Add(-time.Minute).Unix()), + } + cases := []struct { + name string + invalidate bool + wantStatus int + }{ + {"active session forwards", false, http.StatusOK}, + {"logged-out session rejected", true, http.StatusUnauthorized}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + sm := issue162SessionManager(t) + cache := NewCache() + defer cache.Close() + + var seen http.Header + m := issue162Middleware(t, sm, issue162Templates(t, map[string]string{"X-Proxy-Secret": "s3cret"}), &seen) + m.enableBackchannelLogout = true + m.sessionInvalidationCache = cache + if tc.invalidate { + if err := m.invalidateSession("sess-162", ""); err != nil { + t.Fatalf("invalidateSession: %v", err) + } + } + + req := issue162Request(t, sm, idClaims, nil) + req.Header.Set("Upgrade", "websocket") + req.Header.Set("Connection", "Upgrade") + rw := httptest.NewRecorder() + m.ServeHTTP(rw, req) + + if rw.Code != tc.wantStatus { + t.Fatalf("status = %d, want %d", rw.Code, tc.wantStatus) + } + if forwarded := seen != nil; forwarded != (tc.wantStatus == http.StatusOK) { + t.Fatalf("next invoked = %v, want %v", forwarded, tc.wantStatus == http.StatusOK) + } + }) + } +} diff --git a/jwt_test_helpers_test.go b/jwt_test_helpers_test.go index 6aa5baf..7afd246 100644 --- a/jwt_test_helpers_test.go +++ b/jwt_test_helpers_test.go @@ -80,3 +80,21 @@ func makeTestJWT(t *testing.T, claims map[string]interface{}) string { return base64.RawURLEncoding.EncodeToString(hdr) + "." + base64.RawURLEncoding.EncodeToString(pl) + ".c2ln" } + +// tokenWithGroups builds a JWT whose signature segment is long enough to pass +// the session chunk manager's format validation (>=10 chars) so the access +// token round-trips through Set/GetAccessToken. +func tokenWithGroups(t *testing.T, claims map[string]interface{}) string { + t.Helper() + hdr, _ := json.Marshal(map[string]string{"alg": "RS256", "typ": "JWT"}) + pl, _ := json.Marshal(claims) + // Varied (non-repeating) signature bytes so the session chunk manager's + // repeated-character heuristic does not reject it. + sigBytes := make([]byte, 32) + for i := range sigBytes { + sigBytes[i] = byte(i + 1) + } + sig := base64.RawURLEncoding.EncodeToString(sigBytes) + return base64.RawURLEncoding.EncodeToString(hdr) + "." + + base64.RawURLEncoding.EncodeToString(pl) + "." + sig +} diff --git a/middleware.go b/middleware.go index ec37d2b..817e0a1 100644 --- a/middleware.go +++ b/middleware.go @@ -296,6 +296,19 @@ func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) ( return false, http.StatusUnauthorized } + // Mirror the normal path's IdP-initiated logout check: this path forwards + // configured headers (often a shared proxy secret), so a logged-out cookie + // must not keep opening streams until the absolute session timeout. + if t.enableBackchannelLogout || t.enableFrontchannelLogout { + if idToken := session.GetIDToken(); idToken != "" { + sid, sub, _ := t.extractSessionInfo(idToken) + if t.isSessionInvalidated(sid, sub, t.sessionCreatedAtForInvalidation(idToken, session)) { + t.logger.Infof("User %s session invalidated via IdP-initiated logout (bypass)", userIdentifier) + return false, http.StatusUnauthorized + } + } + } + // Enforce the user allowlist, mirroring the normal path's authorization // check (isAllowedUser). Without this, an authenticated user not in // allowedUsers (or whose domain is not in allowedUserDomains) could @@ -314,19 +327,18 @@ func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) ( // fallback, opaque-ID-token providers (groups only in the access token) // were granted on normal requests but 403'd on the streaming bypass // (R118). + var claims map[string]interface{} + var claimsErr error + if len(t.allowedRolesAndGroups) > 0 || len(t.headerTemplates) > 0 { + claims, claimsErr = t.bypassSessionClaims(session) + } + if len(t.allowedRolesAndGroups) > 0 { - var groupClaims map[string]interface{} - var claimsErr error - if idToken := session.GetIDToken(); idToken != "" { - groupClaims, claimsErr = session.GetIDTokenClaims(t.extractClaimsFunc) - } else if accessToken := session.GetAccessToken(); accessToken != "" { - groupClaims, claimsErr = t.extractClaimsFunc(accessToken) - } - if claimsErr != nil || groupClaims == nil { + if claimsErr != nil || claims == nil { t.logger.Debugf("%s bypass: cannot read claims for role check (err=%v): %s", reason, claimsErr, userIdentifier) return false, http.StatusForbidden } - groups, roles, extErr := t.extractGroupsAndRolesFromClaims(groupClaims) + groups, roles, extErr := t.extractGroupsAndRolesFromClaims(claims) if extErr != nil { t.logger.Debugf("%s bypass: role extraction failed for %s: %v", reason, userIdentifier, extErr) return false, http.StatusForbidden @@ -351,8 +363,8 @@ func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) ( // only sets X-Forwarded-User / X-Auth-Request-User (never groups/roles), // forged group/role headers would otherwise survive to the backend, // which commonly trusts them for downstream authorization. Must precede - // the sanitize gate so even the drop-on-unsafe early return has already - // cleared forged values (R100). + // the sanitize gate so forged values are cleared even when the + // identifier is dropped as unsafe (R100). stripIdentityHeaders(req) // Sanitize the claim-derived identifier before it is injected as a @@ -361,20 +373,54 @@ func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) ( // or delimiter characters that would otherwise inject or confuse // downstream header parsing. On failure, drop the header but still // honor the bypass (the identity headers are decoration here). - safeIdentifier, ok := sanitizeHeaderClaimValue(userIdentifier, t.headerClaimMaxLen()) - if !ok { + if safeIdentifier, ok := sanitizeHeaderClaimValue(userIdentifier, t.headerClaimMaxLen()); ok { + req.Header.Set("X-Forwarded-User", safeIdentifier) + if !t.minimalHeaders { + req.Header.Set("X-Auth-Request-User", safeIdentifier) + } + t.logger.Debugf("%s bypass: forwarded user %s from session", reason, safeIdentifier) + } else { t.logger.Debugf("%s bypass: dropping unsafe user-identifier header: %s", reason, headerClaimValueReason(userIdentifier, t.headerClaimMaxLen())) - return true, 0 } - req.Header.Set("X-Forwarded-User", safeIdentifier) - if !t.minimalHeaders { - req.Header.Set("X-Auth-Request-User", safeIdentifier) + // Backends that authenticate the proxy via configured headers (shared + // secret, user/groups) need them on the upgrade too (#162). Claims are + // cookie-sealed but not re-verified here; see the doc comment above. + if len(t.headerTemplates) > 0 { + if claimsErr != nil { + t.logger.Debugf("%s bypass: rendering header templates without claims: %v", reason, claimsErr) + } + t.applyHeaderTemplates(req, &principal{ + Source: sourceSession, + Identifier: userIdentifier, + AccessToken: session.GetAccessToken(), + IDToken: session.GetIDToken(), + RefreshToken: session.GetRefreshToken(), + Claims: claims, + }) } - t.logger.Debugf("%s bypass: forwarded user %s from session", reason, safeIdentifier) return true, 0 } +// bypassSessionClaims returns the session's ID-token claims, falling back to +// access-token claims when there is no ID token, matching the source the +// normal authorization path uses (R118). +func (t *TraefikOidc) bypassSessionClaims(session *SessionData) (map[string]interface{}, error) { + if t.extractClaimsFunc == nil { + return nil, nil + } + // Explicit assignments, not tail-call returns: yaegi can zero + // multi-value tail-call results (see issue151_regression_test.go). + var claims map[string]interface{} + var err error + if session.GetIDToken() != "" { + claims, err = session.GetIDTokenClaims(t.extractClaimsFunc) + } else if accessToken := session.GetAccessToken(); accessToken != "" { + claims, err = t.extractClaimsFunc(accessToken) + } + return claims, err +} + // ServeHTTP implements the main middleware logic for processing HTTP requests. // It handles the complete OIDC authentication flow including: // - Excluded URL bypass @@ -1248,6 +1294,64 @@ func joinBoundedClaimHeader(values []string) string { return b.String() } +// applyHeaderTemplates renders the configured header templates for p onto req. +func (t *TraefikOidc) applyHeaderTemplates(req *http.Request, p *principal) { + if len(t.headerTemplates) == 0 { + return + } + // p.Claims may be nil (e.g. session without an ID token). Templates + // referencing .Claims.* will simply produce empty values — matches + // the prior behavior. Bearer-source principals always carry access- + // token claims (post-verifyToken). + templateData := map[string]interface{}{ + "AccessToken": p.AccessToken, + "IDToken": p.IDToken, + "IdToken": p.IDToken, // documented spelling (README/CONFIGURATION); alias so {{.IdToken}} renders (issue #149 review) + "RefreshToken": p.RefreshToken, + "Claims": p.Claims, + } + + for headerName, tmpl := range t.headerTemplates { + var buf bytes.Buffer + if err := tmpl.Execute(&buf, templateData); err != nil { + t.logger.Errorf("Failed to execute template for header %s: %v", headerName, err) + continue + } + headerValue := buf.String() + // Go's text/template renders a missing key in an interface-valued + // map as the literal "" even with missingkey=zero + // (zero only applies to typed keys). Replace that sentinel so an + // optional claim the provider did not emit (e.g. email) does not + // leak "" into the downstream header. + if strings.Contains(headerValue, noValueSentinel) { + headerValue = stripNoValueSentinels(headerValue) + } + // Skip an empty render: Setting "" would clobber an identity + // header (X-Forwarded-User / X-Auth-Request-*) the caller already + // injected with the authenticated value, silently + // dropping the user identity at the backend. + if headerValue == "" { + t.logger.Debugf("Skipping templated header %s: rendered value is empty", headerName) + continue + } + // Sanitize the rendered output: template inputs are claim-derived + // and attacker-influenceable, so reject control chars (header + // injection), bidi-override runes, the , ; = delimiters, and an + // over-long value. Fail closed by dropping the header rather than + // forwarding a tainted value. Do not log the value (it commonly + // carries the access token); log only name + reason. + if reason := headerValueReason(headerValue, headerTemplateMaxLen); reason != "" { + t.logger.Debugf("Dropping templated header %s: value failed sanitization (%s)", headerName, reason) + continue + } + req.Header.Set(headerName, headerValue) + // Do not log the value: templated headers commonly carry the access + // token (e.g. "Authorization: Bearer {{.AccessToken}}"), and logging + // it — even at debug — leaks credentials into logs. + t.logger.Debugf("Set templated header %s (%d bytes)", headerName, len(headerValue)) + } +} + func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Request, p *principal) { // This middleware fully owns the identity headers it injects upstream. // Unconditionally remove any inbound values with the same names first: @@ -1383,59 +1487,7 @@ func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Reques req.Header.Del("Authorization") } - if len(t.headerTemplates) > 0 { - // p.Claims may be nil (e.g. session without an ID token). Templates - // referencing .Claims.* will simply produce empty values — matches - // the prior behavior. Bearer-source principals always carry access- - // token claims (post-verifyToken). - templateData := map[string]interface{}{ - "AccessToken": p.AccessToken, - "IDToken": p.IDToken, - "IdToken": p.IDToken, // documented spelling (README/CONFIGURATION); alias so {{.IdToken}} renders (issue #149 review) - "RefreshToken": p.RefreshToken, - "Claims": p.Claims, - } - - for headerName, tmpl := range t.headerTemplates { - var buf bytes.Buffer - if err := tmpl.Execute(&buf, templateData); err != nil { - t.logger.Errorf("Failed to execute template for header %s: %v", headerName, err) - continue - } - headerValue := buf.String() - // Go's text/template renders a missing key in an interface-valued - // map as the literal "" even with missingkey=zero - // (zero only applies to typed keys). Replace that sentinel so an - // optional claim the provider did not emit (e.g. email) does not - // leak "" into the downstream header. - if strings.Contains(headerValue, noValueSentinel) { - headerValue = stripNoValueSentinels(headerValue) - } - // Skip an empty render: Setting "" would clobber an identity - // header (X-Forwarded-User / X-Auth-Request-*) already injected - // earlier in this function with the authenticated value, silently - // dropping the user identity at the backend. - if headerValue == "" { - t.logger.Debugf("Skipping templated header %s: rendered value is empty", headerName) - continue - } - // Sanitize the rendered output: template inputs are claim-derived - // and attacker-influenceable, so reject control chars (header - // injection), bidi-override runes, the , ; = delimiters, and an - // over-long value. Fail closed by dropping the header rather than - // forwarding a tainted value. Do not log the value (it commonly - // carries the access token); log only name + reason. - if reason := headerValueReason(headerValue, headerTemplateMaxLen); reason != "" { - t.logger.Debugf("Dropping templated header %s: value failed sanitization (%s)", headerName, reason) - continue - } - req.Header.Set(headerName, headerValue) - // Do not log the value: templated headers commonly carry the access - // token (e.g. "Authorization: Bearer {{.AccessToken}}"), and logging - // it — even at debug — leaks credentials into logs. - t.logger.Debugf("Set templated header %s (%d bytes)", headerName, len(headerValue)) - } - } + t.applyHeaderTemplates(req, p) // Strip OIDC session cookies before forwarding to the backend to prevent // HTTP 431 "Request Header Fields Too Large" errors (GitHub issue #122). diff --git a/review_r118_test.go b/review_r118_test.go index 20ae445..f416570 100644 --- a/review_r118_test.go +++ b/review_r118_test.go @@ -2,8 +2,6 @@ package traefikoidc import ( "context" - "encoding/base64" - "encoding/json" "errors" "net/http" "net/http/httptest" @@ -64,24 +62,6 @@ func TestIsRetryableError_HTTPErrorStatus(t *testing.T) { // --- R118 fix C: bypass role gate must fall back to access-token claims --- -// tokenWithGroups builds a JWT whose signature segment is long enough to pass -// the session chunk manager's format validation (>=10 chars) so the access -// token round-trips through Set/GetAccessToken. -func tokenWithGroups(t *testing.T, claims map[string]interface{}) string { - t.Helper() - hdr, _ := json.Marshal(map[string]string{"alg": "RS256", "typ": "JWT"}) - pl, _ := json.Marshal(claims) - // Varied (non-repeating) signature bytes so the session chunk manager's - // repeated-character heuristic does not reject it. - sigBytes := make([]byte, 32) - for i := range sigBytes { - sigBytes[i] = byte(i + 1) - } - sig := base64.RawURLEncoding.EncodeToString(sigBytes) - return base64.RawURLEncoding.EncodeToString(hdr) + "." + - base64.RawURLEncoding.EncodeToString(pl) + "." + sig -} - // TestApplyBypassUserHeaders_AccessTokenGroupFallback regresses R118: the // SSE/WebSocket bypass role gate read only the session's ID token, while the // main authorization path falls back to access-token claims when no ID token