diff --git a/CHANGELOG.md b/CHANGELOG.md index a3b9cf6..512625e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). never reads a preflight body (`settings.go`, `middleware.go`, FIX-02). See the option table in [CONFIGURATION.md](docs/CONFIGURATION.md). +- **`streamingRefresh` (default `false`).** SSE and WebSocket requests do + not check the token expiry, so a session stays usable for streaming until + the session cookie expires. Set the option to `true` to refresh an expired + token on these requests. If the IdP rejects the refresh with + `invalid_grant`, the request gets `401`. If the IdP cannot be reached, the + request is forwarded as before. The request waits for the full refresh, so + a rotated refresh token is never lost (`middleware.go`). + ### Changed - **Opaque bearer tokens when all three bearer flags are set.** A deployment diff --git a/README.md b/README.md index c3f143e..6fc8065 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,7 @@ Full reference in [docs/CONFIGURATION.md](docs/CONFIGURATION.md). | `disableReplayDetection` | `false` | Disable JTI cache. Use Redis instead for multi-replica. | | `allowPrivateIPAddresses` | `false` | Permit private-IP `providerURL` (internal Keycloak, etc.). | | `allowUnauthenticatedPreflight` | `false` | Bypass auth for a genuine CORS preflight (`OPTIONS` + `Origin` + `Access-Control-Request-Method`); the response body is still discarded. Default requires auth for every `OPTIONS` request, preflight or not. | +| `streamingRefresh` | `false` | Refresh an expired session on SSE/WebSocket requests. Reject the request when the IdP revokes the grant. | | `minimalHeaders` | `false` | Reduce forwarded headers (mitigates HTTP 431). | | `stripAuthCookies` | `false` | Strip OIDC cookies from backend hop (mitigates HTTP 431). | | `caCertPath` / `caCertPEM` | none | Trust an internal CA for the provider's TLS. | @@ -309,6 +310,11 @@ WebSocket upgrade. The middleware handles this automatically: - Validation is cookie-only (no JWK fetch), so streaming keeps working during brief IdP outages. Back-channel and front-channel logout still apply: a session that the IdP logged out is rejected. +- By default, the token expiry is not checked, so a session stays usable + for streaming until the session cookie expires. Set `streamingRefresh: true` + to refresh an expired token on the request. If the IdP rejects the refresh + (`invalid_grant`), the request gets `401`. If the IdP cannot be reached, + the request is forwarded as before. No configuration needed — this is implicit behavior. diff --git a/cmd/yaegicheck/main.go b/cmd/yaegicheck/main.go index 377a1d3..ec72252 100644 --- a/cmd/yaegicheck/main.go +++ b/cmd/yaegicheck/main.go @@ -139,6 +139,7 @@ func main() { runCheck("middleware-erraborthandler-aborts-under-yaegi", checkErrAbortHandlerAbortsUnderYaegi) runCheck("fix17-setifabsent-claims-once", checkSetIfAbsentUnderYaegi) runCheck("issue162-websocket-forwards-header-templates", checkIssue162WebSocketHeaderTemplates) + runCheck("streaming-refresh-revoked-grant-rejected", checkStreamingRefreshRevokedGrant) fmt.Println("OK: all yaegi regression checks passed") } @@ -806,6 +807,87 @@ func checkIssue162WebSocketHeaderTemplates() (string, error) { return "", nil } +// checkStreamingRefreshRevokedGrant drives streamingRefresh under the +// interpreter through the real RefreshCoordinator and token exchanger: an +// expired session whose refresh the IdP answers with invalid_grant must get +// 401 on a WebSocket upgrade instead of reaching next. +func checkStreamingRefreshRevokedGrant() (string, error) { + idpCfg := servers.DefaultConfig() + idpCfg.RefreshError = &servers.OIDCError{Error: "invalid_grant"} + idp := servers.NewOIDCServer(idpCfg) + defer idp.Close() + + cfg := oidc.CreateConfig() + cfg.ProviderURL = idp.URL + cfg.ClientID = "yaegi-check-refresh-client" + cfg.ClientSecret = "yaegi-check-refresh-secret" + cfg.CallbackURL = "/oauth2/callback" + cfg.SessionEncryptionKey = "0123456789abcdef0123456789abcdef" + cfg.StreamingRefresh = true + + reached := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = true + }) + h, err := oidc.New(context.Background(), next, cfg, "yaegi-check-refresh") + if err != nil { + return "", fmt.Errorf("New: %w", err) + } + if closer, ok := h.(interface{ Close() error }); ok { + defer closer.Close() + } + // The streaming bypass does not wait for provider metadata, so a normal + // request (which does) runs first to make the token endpoint known. + h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/", nil)) + + 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") + // 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) + } + payload := fmt.Sprintf(`{"email":"alice@example.com","exp":%d}`, time.Now().Add(-time.Minute).Unix()) + session.SetAccessToken(b64([]byte(`{"alg":"RS256","typ":"JWT"}`)) + "." + b64([]byte(payload)) + "." + b64(sig)) + session.SetRefreshToken("yaegi-check-refresh-token") + 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 reached { + return "", fmt.Errorf("next reached for a revoked grant (status %d)", rw.Code) + } + if rw.Code != http.StatusUnauthorized { + return "", fmt.Errorf("status = %d, want %d", rw.Code, http.StatusUnauthorized) + } + 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/config_marshalling.go b/config_marshalling.go index 9346d14..6c7b1bb 100644 --- a/config_marshalling.go +++ b/config_marshalling.go @@ -76,6 +76,7 @@ func (c Config) configToMap() map[string]interface{} { result["enablePKCE"] = c.EnablePKCE result["allowPrivateIPAddresses"] = c.AllowPrivateIPAddresses result["allowUnauthenticatedPreflight"] = c.AllowUnauthenticatedPreflight + result["streamingRefresh"] = c.StreamingRefresh result["minimalHeaders"] = c.MinimalHeaders result["stripAuthCookies"] = c.StripAuthCookies result["cookiePath"] = c.CookiePath diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 5a50e8b..25cbe98 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -313,6 +313,7 @@ strictAudienceValidation: true | `disableReplayDetection` | bool | `false` | Disable JTI-based replay attack detection | | `allowPrivateIPAddresses` | bool | `false` | Allow private IPs in provider URLs | | `allowUnauthenticatedPreflight` | bool | `false` | Bypass auth for a genuine CORS preflight (`OPTIONS` with `Origin` + `Access-Control-Request-Method`); response body is still discarded. Default requires auth for every `OPTIONS` request. | +| `streamingRefresh` | bool | `false` | Refresh an expired session on SSE/WebSocket requests. Reject the request with `401` when the IdP revokes the grant (`invalid_grant`). Forward it as before when the IdP cannot be reached. | ### Discovered Endpoint Validation diff --git a/main.go b/main.go index e01de7b..332326e 100644 --- a/main.go +++ b/main.go @@ -336,6 +336,7 @@ func NewWithContext(ctx context.Context, config *Config, next http.Handler, name dcrConfig: config.DynamicClientRegistration, allowPrivateIPAddresses: config.AllowPrivateIPAddresses, allowUnauthenticatedPreflight: config.AllowUnauthenticatedPreflight, + streamingRefresh: config.StreamingRefresh, allowLoopbackHosts: isLoopbackProviderURL(config.ProviderURL), minimalHeaders: config.MinimalHeaders, stripAuthCookies: config.StripAuthCookies, diff --git a/middleware.go b/middleware.go index 817e0a1..346cba5 100644 --- a/middleware.go +++ b/middleware.go @@ -273,7 +273,7 @@ func stripIdentityHeaders(req *http.Request) { } } -func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) (bool, int) { +func (t *TraefikOidc) applyBypassUserHeaders(rw http.ResponseWriter, req *http.Request, reason string) (bool, int) { if t.sessionManager == nil { return false, http.StatusUnauthorized } @@ -296,6 +296,17 @@ func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) ( return false, http.StatusUnauthorized } + // Opt-in: refresh an expired session so an IdP-revoked grant stops opening + // streams. Only a definitive rejection (refreshToken clears the session on + // invalid_grant) denies; IdP outages keep forwarding as before. The full + // coordinator wait is kept so a rotated refresh token is never dropped. + if t.streamingRefresh && bypassTokenExpired(session) { + if !t.refreshToken(rw, req, session) && !session.GetAuthenticated() { + t.logger.Infof("User %s refresh rejected by IdP (bypass)", userIdentifier) + 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. @@ -402,6 +413,25 @@ func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) ( return true, 0 } +// bypassTokenExpired reports whether the session's access token, or its ID +// token when the access token carries no readable exp (opaque), has expired. +// exp is read unverified: the cookie is sealed and a verified refresh follows. +func bypassTokenExpired(session *SessionData) bool { + for _, token := range []string{session.GetAccessToken(), session.GetIDToken()} { + if token == "" { + continue + } + jwt, err := parseJWT(token) + if err != nil { + continue + } + if exp, ok := jwt.Claims["exp"].(float64); ok { + return !time.Now().Before(time.Unix(int64(exp), 0)) + } + } + return false +} + // 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). @@ -640,7 +670,7 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) { // Otherwise an unauthenticated client could hit the backend // just by setting Accept: text/event-stream or sending a // WebSocket upgrade. - if ok, status := t.applyBypassUserHeaders(req, reason); !ok { + if ok, status := t.applyBypassUserHeaders(rw, req, reason); !ok { msg := "Authentication required" if status == http.StatusForbidden { msg = "Access denied" diff --git a/review_r100_test.go b/review_r100_test.go index 42e71a4..3cb3f4e 100644 --- a/review_r100_test.go +++ b/review_r100_test.go @@ -53,7 +53,7 @@ func TestApplyBypassUserHeaders_ClearsForgedIdentityHeaders(t *testing.T) { logger: NewLogger("debug"), } - if ok, _ := oidc.applyBypassUserHeaders(req, "test"); !ok { + if ok, _ := oidc.applyBypassUserHeaders(httptest.NewRecorder(), req, "test"); !ok { t.Fatalf("applyBypassUserHeaders returned false; bypass should be honored") } @@ -110,7 +110,7 @@ func TestApplyBypassUserHeaders_UnsafeIdentifierClearsForgedValue(t *testing.T) logger: NewLogger("debug"), } - if ok, _ := oidc.applyBypassUserHeaders(req, "test"); !ok { + if ok, _ := oidc.applyBypassUserHeaders(httptest.NewRecorder(), req, "test"); !ok { t.Fatalf("applyBypassUserHeaders returned false; bypass should be honored") } if v := req.Header.Get("X-Forwarded-User"); v != "" { diff --git a/review_r118_test.go b/review_r118_test.go index f416570..ae1c4fe 100644 --- a/review_r118_test.go +++ b/review_r118_test.go @@ -100,7 +100,7 @@ func TestApplyBypassUserHeaders_AccessTokenGroupFallback(t *testing.T) { allowedRolesAndGroups: map[string]struct{}{"team-a": {}}, } - ok, status := oidc.applyBypassUserHeaders(req, "test") + ok, status := oidc.applyBypassUserHeaders(httptest.NewRecorder(), req, "test") if !ok { t.Fatalf("bypass role gate denied access-token group; got status %d, want granted", status) } diff --git a/review_r48_regression_test.go b/review_r48_regression_test.go index a90f695..516b2b0 100644 --- a/review_r48_regression_test.go +++ b/review_r48_regression_test.go @@ -42,7 +42,7 @@ func TestApplyBypassUserHeaders_SanitizesIdentifier(t *testing.T) { logger: NewLogger("debug"), } - if ok, _ := oidc.applyBypassUserHeaders(req, "test"); !ok { + if ok, _ := oidc.applyBypassUserHeaders(httptest.NewRecorder(), req, "test"); !ok { t.Fatalf("applyBypassUserHeaders returned false; bypass should be honored") } if v := req.Header.Get("X-Forwarded-User"); v != "" { @@ -83,7 +83,7 @@ func TestApplyBypassUserHeaders_KeepsCleanIdentifier(t *testing.T) { logger: NewLogger("debug"), } - if ok, _ := oidc.applyBypassUserHeaders(req, "test"); !ok { + if ok, _ := oidc.applyBypassUserHeaders(httptest.NewRecorder(), req, "test"); !ok { t.Fatalf("applyBypassUserHeaders returned false; bypass should be honored") } if v := req.Header.Get("X-Forwarded-User"); v != "user@company.com" { diff --git a/settings.go b/settings.go index 76605b2..4df11a6 100644 --- a/settings.go +++ b/settings.go @@ -115,8 +115,12 @@ type Config struct { // or not, like any other method. Enable only if your backend itself // answers CORS preflights and must see them unauthenticated. AllowUnauthenticatedPreflight bool `json:"allowUnauthenticatedPreflight,omitempty"` - MinimalHeaders bool `json:"minimalHeaders,omitempty"` - StripAuthCookies bool `json:"stripAuthCookies,omitempty"` + // StreamingRefresh refreshes an expired session on the SSE/WebSocket + // bypass and rejects the request when the IdP revokes the grant. + // Default false: the bypass trusts the session cookie until it expires. + StreamingRefresh bool `json:"streamingRefresh,omitempty"` + MinimalHeaders bool `json:"minimalHeaders,omitempty"` + StripAuthCookies bool `json:"stripAuthCookies,omitempty"` // CookiePath restricts session cookies to a specific path prefix instead of "/". // When traefikoidc protects some but not all paths on a domain, set this to the // middleware's path prefix (e.g. "/app-protegido") so the browser does not send diff --git a/streaming_refresh_test.go b/streaming_refresh_test.go new file mode 100644 index 0000000..58864a4 --- /dev/null +++ b/streaming_refresh_test.go @@ -0,0 +1,189 @@ +package traefikoidc + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + "text/template" + "time" +) + +// streamingRefreshSession stores a session whose tokens expire at exp and +// returns a WebSocket upgrade request carrying its cookies. An empty +// refreshToken leaves the refresh token unset; opaqueAccess stores a +// non-JWT access token so only the ID token carries an exp. +func streamingRefreshSession(t *testing.T, sm *SessionManager, exp time.Time, refreshToken string, opaqueAccess bool) *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") + claims := map[string]interface{}{ + "email": "alice@example.com", + "preferred_username": "alice-old", + "exp": float64(exp.Unix()), + } + session.SetIDToken(tokenWithGroups(t, claims)) + if opaqueAccess { + session.SetAccessToken("opaque-access-token-without-dots") + } else { + session.SetAccessToken(tokenWithGroups(t, claims)) + } + if refreshToken != "" { + session.SetRefreshToken(refreshToken) + } + 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) + } + req.Header.Set("Upgrade", "websocket") + req.Header.Set("Connection", "Upgrade") + return req +} + +func TestStreamingBypassRefresh(t *testing.T) { + past := time.Now().Add(-time.Minute) + future := time.Now().Add(time.Hour) + errInvalidGrant := errors.New(`token endpoint returned 400: {"error":"invalid_grant"}`) + errTransient := errors.New("dial tcp 10.0.0.1:443: connect: connection refused") + + cases := []struct { + name string + enabled bool + exp time.Time + refreshToken string + opaqueAccess bool + refreshErr error + wantStatus int + wantRefresh int + wantUsername string + }{ + {name: "flag off keeps expired session forwarding", enabled: false, exp: past, refreshToken: "rt-old", wantStatus: http.StatusOK, wantRefresh: 0, wantUsername: "alice-old"}, + {name: "unexpired token skips refresh", enabled: true, exp: future, refreshToken: "rt-old", wantStatus: http.StatusOK, wantRefresh: 0, wantUsername: "alice-old"}, + {name: "expired token refreshes and forwards fresh claims", enabled: true, exp: past, refreshToken: "rt-old", wantStatus: http.StatusOK, wantRefresh: 1, wantUsername: "alice-new"}, + {name: "opaque access token uses ID token expiry", enabled: true, exp: past, refreshToken: "rt-old", opaqueAccess: true, wantStatus: http.StatusOK, wantRefresh: 1, wantUsername: "alice-new"}, + {name: "revoked grant is rejected", enabled: true, exp: past, refreshToken: "rt-old", refreshErr: errInvalidGrant, wantStatus: http.StatusUnauthorized, wantRefresh: 1}, + {name: "transient IdP failure still forwards", enabled: true, exp: past, refreshToken: "rt-old", refreshErr: errTransient, wantStatus: http.StatusOK, wantRefresh: 1, wantUsername: "alice-old"}, + {name: "no refresh token still forwards", enabled: true, exp: past, wantStatus: http.StatusOK, wantRefresh: 0, wantUsername: "alice-old"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + 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) + } + }) + + newClaims := map[string]interface{}{ + "email": "alice@example.com", + "preferred_username": "alice-new", + "exp": float64(future.Unix()), + } + exchanger := &EnhancedMockTokenExchanger{ + RefreshErr: tc.refreshErr, + RefreshResponse: &TokenResponse{ + AccessToken: tokenWithGroups(t, newClaims), + IDToken: tokenWithGroups(t, newClaims), + RefreshToken: "rt-new", + }, + } + tmpl := template.Must(template.New("X-Forwarded-Preferred-Username"). + Funcs(headerTemplateFuncMap(claimsWhitelist(nil))). + Option("missingkey=zero"). + Parse("{{.Claims.preferred_username}}")) + + initComplete := make(chan struct{}) + close(initComplete) + var seen http.Header + m := &TraefikOidc{ + logger: GetSingletonNoOpLogger(), + name: "test", + sessionManager: sm, + initComplete: initComplete, + extractClaimsFunc: extractClaims, + userIdentifierClaim: "email", + tokenExchanger: exchanger, + tokenVerifier: &EnhancedMockTokenVerifier{}, + streamingRefresh: tc.enabled, + headerTemplates: map[string]*template.Template{"X-Forwarded-Preferred-Username": tmpl}, + next: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seen = r.Header.Clone() + w.WriteHeader(http.StatusOK) + }), + } + + req := streamingRefreshSession(t, sm, tc.exp, tc.refreshToken, tc.opaqueAccess) + rw := httptest.NewRecorder() + m.ServeHTTP(rw, req) + + if rw.Code != tc.wantStatus { + t.Fatalf("status = %d, want %d", rw.Code, tc.wantStatus) + } + if got := len(exchanger.RefreshCalls); got != tc.wantRefresh { + t.Fatalf("refresh calls = %d, want %d", got, tc.wantRefresh) + } + if tc.wantStatus != http.StatusOK { + if seen != nil { + t.Fatal("next invoked for a rejected request") + } + return + } + if seen == nil { + t.Fatal("next not invoked") + } + if got := seen.Get("X-Forwarded-Preferred-Username"); got != tc.wantUsername { + t.Fatalf("X-Forwarded-Preferred-Username = %q, want %q", got, tc.wantUsername) + } + if tc.wantRefresh == 1 && tc.refreshErr == nil && len(rw.Result().Cookies()) == 0 { + t.Fatal("refreshed session was not written back as a cookie") + } + }) + } +} + +func TestStreamingRefreshConfigWiring(t *testing.T) { + if CreateConfig().StreamingRefresh { + t.Fatal("streamingRefresh must default to false") + } + for _, enabled := range []bool{false, true} { + config := CreateConfig() + config.ProviderURL = "https://provider.example.com" + config.ClientID = "test-client-id" + config.ClientSecret = "test-secret" + config.SessionEncryptionKey = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + config.CallbackURL = "/callback" + config.StreamingRefresh = enabled + + if got, ok := config.configToMap()["streamingRefresh"].(bool); !ok || got != enabled { + t.Fatalf("configToMap streamingRefresh = %v (present=%v), want %v", got, ok, enabled) + } + oidc, err := NewWithContext(context.Background(), config, http.NotFoundHandler(), "test") + if err != nil { + t.Fatalf("NewWithContext: %v", err) + } + if oidc.streamingRefresh != enabled { + t.Errorf("streamingRefresh = %v, want %v", oidc.streamingRefresh, enabled) + } + if err := oidc.Close(); err != nil { + t.Errorf("Close: %v", err) + } + } +} diff --git a/types.go b/types.go index 4c7408d..ae33b5d 100644 --- a/types.go +++ b/types.go @@ -235,6 +235,8 @@ type TraefikOidc struct { // (default false): gates whether shouldBypassAuth's genuine-preflight // check (middleware.go) bypasses OIDC auth at all. allowUnauthenticatedPreflight bool + // streamingRefresh mirrors Config.StreamingRefresh (default false). + streamingRefresh bool // allowLoopbackHosts permits loopback/localhost hosts in outbound URL // validation (validateHost). Derived at construction time from a loopback // providerURL (local development), never operator-set directly. Mirrors