From 0f340beb14199c89dc0f7ac13da27df416e0e943 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Sun, 10 May 2026 21:12:11 +0000 Subject: [PATCH] fix(auth/ux): cause-aware OIDC + session error surfacing (HIGH-7 + HIGH-8 closure) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server (HIGH-7): the OIDC callback failure path now 302-redirects to /login?error=oidc_failed&reason= instead of emitting a blank 400. `category` is the existing audit `failure_category` value; classifyOIDCFailure was extended with three new sentinel paths (email_domain_not_allowed, email_missing_but_required, pkce_invalid) so CRIT-5 + PKCE failures get distinguishable GUI rendering. Audit-log observability is unchanged — the same failure_category is written to the auth.oidc_login_failed audit row; the 302 is purely a UX leg layered on top. Server (HIGH-8): SessionMiddleware now stashes a cause classification on the request context when Validate returns an error, mapping the sentinels via classifySessionError (errors.Is-based, so wrapped sentinels still classify) to the stable wire-strings idle_timeout / absolute_timeout / back_channel_revoked / invalid_token. The 401 emit point in bearerSkipIfAuthenticated reads the stashed cause and emits WWW-Authenticate: Bearer realm="certctl", error="invalid_token", error_description= per RFC 6750 §3. GUI (HIGH-7): LoginPage reads ?error= + ?reason= from the URL via react-router useSearchParams and renders an operator-friendly amber-bordered banner above the form; OIDC_FAILURE_REASON_TEXT maps all 16 known categories with a defensive 'unspecified' fallback for forward-compat with future server-side categories. GUI (HIGH-8): api/client fetchJSON parses the WWW-Authenticate cause via parseWWWAuthenticateCause and attaches it to the 'certctl:auth-required' CustomEvent detail; AuthProvider redirects to /login?session_expired= on cause-aware 401s; LoginPage renders a blue-bordered session-cause banner. invalid_token stays on the current page (no hard redirect for opaque failures). Misc cleanup: ErrorState now accepts the title/message/data-testid form added by CRIT-4 BreakglassPage (was erroring tsc on master). Regression matrix: - internal/api/handler/oidc_redirect_categories_test.go pins all 16 failure categories to the 302 + reason= location + audit-row leg - internal/auth/session/www_authenticate_test.go pins the 4 stable cause categories on classifySessionError (incl. errors.Is wrapped sentinels) + the WWW-Authenticate emission across all 4 categories + the no-session-context fallback case - internal/api/handler/auth_session_oidc_test.go: 4 pre-existing TestLoginCallback_*Returns400 tests updated to assert 302 + reason= location (the wire shape changed from 400 to 302, but the audit observability and behaviour-equivalent failure-classification are preserved) - web/src/pages/LoginPage.test.tsx: 6 new cases pinning the failure banner, session-cause banner, unknown-reason fallback, and forward-compat 'unspecified' category Spec: cowork/auth-bundles-fixes-2026-05-10/08-high-7-8-error-surfacing.md Closes: HIGH-7, HIGH-8 of cowork/auth-bundles-audit-2026-05-10.md --- internal/api/handler/auth_session_oidc.go | 22 ++- .../api/handler/auth_session_oidc_test.go | 42 ++++-- .../handler/oidc_redirect_categories_test.go | 140 ++++++++++++++++++ internal/auth/session/middleware.go | 50 ++++++- .../auth/session/www_authenticate_test.go | 135 +++++++++++++++++ web/src/api/client.ts | 39 ++++- web/src/components/AuthProvider.tsx | 25 +++- web/src/components/ErrorState.tsx | 33 ++++- web/src/pages/LoginPage.test.tsx | 79 ++++++++++ web/src/pages/LoginPage.tsx | 98 +++++++++++- 10 files changed, 633 insertions(+), 30 deletions(-) create mode 100644 internal/api/handler/oidc_redirect_categories_test.go create mode 100644 internal/auth/session/www_authenticate_test.go diff --git a/internal/api/handler/auth_session_oidc.go b/internal/api/handler/auth_session_oidc.go index b8bf821..37e58be 100644 --- a/internal/api/handler/auth_session_oidc.go +++ b/internal/api/handler/auth_session_oidc.go @@ -258,7 +258,11 @@ func (h *AuthSessionOIDCHandler) LoginCallback(w http.ResponseWriter, r *http.Re res, err := h.oidcSvc.HandleCallback(r.Context(), preLoginCookie.Value, code, state, clientIP, userAgent) if err != nil { - // Uniform 400 to the wire; specific failure category in audit. + // Audit 2026-05-10 HIGH-7 — instead of a blank 400, redirect + // to /login?error=oidc_failed&reason=. The LoginPage + // reads the query params and renders an operator-friendly + // alert. The audit row still carries the specific + // failure_category so server-side observability is unchanged. category := classifyOIDCFailure(err) h.recordAudit(r.Context(), "auth.oidc_login_failed", "anonymous", domain.ActorTypeSystem, "", map[string]interface{}{"failure_category": category}) @@ -270,7 +274,10 @@ func (h *AuthSessionOIDCHandler) LoginCallback(w http.ResponseWriter, r *http.Re } // Always clear the pre-login cookie on failure. h.clearPreLoginCookie(w) - Error(w, http.StatusBadRequest, "OIDC login failed") + // 302 to the login page; the reason categorizes the failure for + // the GUI to render. Keep the redirect target relative — the + // SPA serves /login. + http.Redirect(w, r, "/login?error=oidc_failed&reason="+category, http.StatusFound) return } @@ -1073,6 +1080,17 @@ func classifyOIDCFailure(err error) string { return "groups_missing" case strings.Contains(msg, "jwks"): return "jwks_unreachable" + // Audit 2026-05-10 HIGH-7 — surface CRIT-5 email-domain rejection + // + PKCE invalidation distinctly so the LoginPage can render an + // operator-friendly reason. The sentinel errors live in + // internal/auth/oidc/service.go (ErrEmailDomainNotAllowed, + // ErrEmailMissingButRequired, ErrPKCEPlainRejected). + case strings.Contains(msg, "email domain not in allowlist"): + return "email_domain_not_allowed" + case strings.Contains(msg, "requires email but token has none"): + return "email_missing_but_required" + case strings.Contains(msg, "pkce"): + return "pkce_invalid" default: return "unspecified" } diff --git a/internal/api/handler/auth_session_oidc_test.go b/internal/api/handler/auth_session_oidc_test.go index 18bf461..8cb0f97 100644 --- a/internal/api/handler/auth_session_oidc_test.go +++ b/internal/api/handler/auth_session_oidc_test.go @@ -362,9 +362,11 @@ func TestLoginCallback_HappyPath(t *testing.T) { } } -// Phase 5 spec mandate #4: Callback with replayed state -> 400. +// Phase 5 spec mandate #4: Callback with replayed state -> 302 to /login. // (The OIDC service's PreLoginStore.LookupAndConsume returns -// ErrPreLoginNotFound on the second call; the handler maps to 400.) +// ErrPreLoginNotFound on the second call; Audit 2026-05-10 HIGH-7 +// flipped this from a blank 400 to a 302 to /login?error=oidc_failed +// &reason=. The audit row still records failure_category.) func TestLoginCallback_ReplayedState_Returns400(t *testing.T) { o := &stubOIDCSvc{callbackErr: oidcsvc.ErrPreLoginNotFound} h, _, _, _, audit, _ := newPhase5Handler(t, o, &stubSession{}, &stubBCLVerifier{}) @@ -373,17 +375,20 @@ func TestLoginCallback_ReplayedState_Returns400(t *testing.T) { req.AddCookie(&http.Cookie{Name: sessiondomain.PreLoginCookieName, Value: "v1.pl-abc.sk-xyz.mac"}) w := httptest.NewRecorder() h.LoginCallback(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d; want 400", w.Code) + if w.Code != http.StatusFound { + t.Errorf("status = %d; want 302 (post-HIGH-7 redirect)", w.Code) + } + if loc := w.Header().Get("Location"); !strings.HasPrefix(loc, "/login?error=oidc_failed&reason=") { + t.Errorf("Location = %q; want /login?error=oidc_failed&reason=...", loc) } if !contains(audit.events, "auth.oidc_login_failed") { t.Errorf("expected auth.oidc_login_failed audit event; got %v", audit.events) } } -// Phase 5 spec mandate #5: Callback with PKCE verifier mismatch -> 400. +// Phase 5 spec mandate #5: Callback with PKCE verifier mismatch -> 302. // The OIDC service's code-exchange step fails when the verifier doesn't -// match the challenge; the handler surfaces it as 400. +// match the challenge; HIGH-7 redirects to /login with reason. func TestLoginCallback_PKCEVerifierMismatch_Returns400(t *testing.T) { o := &stubOIDCSvc{callbackErr: errors.New("oidc: code exchange failed: invalid_grant")} h, _, _, _, _, _ := newPhase5Handler(t, o, &stubSession{}, &stubBCLVerifier{}) @@ -391,23 +396,27 @@ func TestLoginCallback_PKCEVerifierMismatch_Returns400(t *testing.T) { req.AddCookie(&http.Cookie{Name: sessiondomain.PreLoginCookieName, Value: "v1.pl-abc.sk-xyz.mac"}) w := httptest.NewRecorder() h.LoginCallback(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d; want 400", w.Code) + if w.Code != http.StatusFound { + t.Errorf("status = %d; want 302 (post-HIGH-7 redirect)", w.Code) + } + if loc := w.Header().Get("Location"); !strings.HasPrefix(loc, "/login?error=oidc_failed") { + t.Errorf("Location = %q; want /login?error=oidc_failed&reason=...", loc) } } -// Phase 5 spec mandate #6: Callback with expired pre-login row -> 400. +// Phase 5 spec mandate #6: Callback with expired pre-login row -> 302. func TestLoginCallback_ExpiredPreLoginRow_Returns400(t *testing.T) { - // Adapter maps ErrPreLoginExpired -> ErrPreLoginNotFound (uniform - // 400 per spec; specific reason in audit row). + // Adapter maps ErrPreLoginExpired -> ErrPreLoginNotFound; HIGH-7 + // flipped the wire shape from 400 to a 302 redirect (specific + // reason still in audit row). o := &stubOIDCSvc{callbackErr: oidcsvc.ErrPreLoginNotFound} h, _, _, _, _, _ := newPhase5Handler(t, o, &stubSession{}, &stubBCLVerifier{}) req := httptest.NewRequest(http.MethodGet, "/auth/oidc/callback?code=abc&state=xyz", nil) req.AddCookie(&http.Cookie{Name: sessiondomain.PreLoginCookieName, Value: "v1.pl-abc.sk-xyz.mac"}) w := httptest.NewRecorder() h.LoginCallback(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d; want 400", w.Code) + if w.Code != http.StatusFound { + t.Errorf("status = %d; want 302 (post-HIGH-7 redirect)", w.Code) } } @@ -431,8 +440,11 @@ func TestLoginCallback_UnmappedGroups_AuditRowDistinguished(t *testing.T) { req.AddCookie(&http.Cookie{Name: sessiondomain.PreLoginCookieName, Value: "v1.pl-abc.sk-xyz.mac"}) w := httptest.NewRecorder() h.LoginCallback(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d; want 400", w.Code) + if w.Code != http.StatusFound { + t.Errorf("status = %d; want 302 (post-HIGH-7 redirect)", w.Code) + } + if loc := w.Header().Get("Location"); !strings.Contains(loc, "reason=unmapped_groups") { + t.Errorf("Location = %q; want reason=unmapped_groups", loc) } if !contains(audit.events, "auth.oidc_login_unmapped_groups") { t.Errorf("expected auth.oidc_login_unmapped_groups; got %v", audit.events) diff --git a/internal/api/handler/oidc_redirect_categories_test.go b/internal/api/handler/oidc_redirect_categories_test.go new file mode 100644 index 0000000..d78c946 --- /dev/null +++ b/internal/api/handler/oidc_redirect_categories_test.go @@ -0,0 +1,140 @@ +package handler + +import ( + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + oidcsvc "github.com/certctl-io/certctl/internal/auth/oidc" + sessiondomain "github.com/certctl-io/certctl/internal/auth/session/domain" +) + +// Audit 2026-05-10 HIGH-7 regression matrix — pin every classified +// failure category to its post-redirect query reason. Pre-fix, every +// failure surfaced as "OIDC login failed" with status 400 and no +// machine-readable hint; the LoginPage couldn't tell idle-timeout +// from email-domain rejection from PKCE breakage. Post-fix, the +// handler 302-redirects to /login?error=oidc_failed&reason= +// where the GUI renders an operator-friendly cause. + +func TestLoginCallback_RedirectsWithReason_AllCategories(t *testing.T) { + cases := []struct { + name string + err error + wantReason string + }{ + { + name: "pre_login_consume_failed", + err: oidcsvc.ErrPreLoginNotFound, + wantReason: "pre_login_consume_failed", + }, + { + name: "state_mismatch", + err: errors.New("state mismatch"), + wantReason: "state_mismatch", + }, + { + name: "nonce_mismatch", + err: errors.New("nonce mismatch"), + wantReason: "nonce_mismatch", + }, + { + name: "audience_mismatch", + err: errors.New("audience mismatch"), + wantReason: "audience_mismatch", + }, + { + name: "token_expired", + err: errors.New("token expired"), + wantReason: "token_expired", + }, + { + name: "azp_mismatch", + err: errors.New("azp does not match"), + wantReason: "azp_mismatch", + }, + { + name: "at_hash_mismatch", + err: errors.New("at_hash mismatch"), + wantReason: "at_hash_mismatch", + }, + { + name: "iat_window", + err: errors.New("iat outside window"), + wantReason: "iat_window", + }, + { + name: "alg_rejected", + err: errors.New("alg not in allowlist"), + wantReason: "alg_rejected", + }, + { + name: "unmapped_groups", + err: oidcsvc.ErrGroupsUnmapped, + wantReason: "unmapped_groups", + }, + { + name: "groups_missing", + err: errors.New("groups missing"), + wantReason: "groups_missing", + }, + { + name: "jwks_unreachable", + err: errors.New("jwks fetch failed"), + wantReason: "jwks_unreachable", + }, + // HIGH-7 added these three categories so CRIT-5 (email domain) + // and PKCE failures get distinguishable GUI rendering. + { + name: "email_domain_not_allowed", + err: errors.New("email domain not in allowlist"), + wantReason: "email_domain_not_allowed", + }, + { + name: "email_missing_but_required", + err: errors.New("provider requires email but token has none"), + wantReason: "email_missing_but_required", + }, + { + name: "pkce_invalid", + err: errors.New("pkce verifier mismatch"), + wantReason: "pkce_invalid", + }, + { + name: "unspecified_fallback", + err: errors.New("totally unrecognized error"), + wantReason: "unspecified", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + o := &stubOIDCSvc{callbackErr: tc.err} + h, _, _, _, audit, _ := newPhase5Handler(t, o, &stubSession{}, &stubBCLVerifier{}) + req := httptest.NewRequest(http.MethodGet, + "/auth/oidc/callback?code=abc&state=xyz", nil) + req.AddCookie(&http.Cookie{ + Name: sessiondomain.PreLoginCookieName, + Value: "v1.pl-abc.sk-xyz.mac", + }) + w := httptest.NewRecorder() + h.LoginCallback(w, req) + if w.Code != http.StatusFound { + t.Fatalf("status = %d; want 302", w.Code) + } + loc := w.Header().Get("Location") + wantPrefix := "/login?error=oidc_failed&reason=" + tc.wantReason + if !strings.HasPrefix(loc, wantPrefix) { + t.Errorf("Location = %q; want prefix %q", loc, wantPrefix) + } + // The audit row must still record the failure_category for + // server-side observability — that's the load-bearing leg + // of the HIGH-7 fix (audit retention is not narrowed by the + // GUI redirect). + if !contains(audit.events, "auth.oidc_login_failed") { + t.Errorf("expected auth.oidc_login_failed audit event; got %v", audit.events) + } + }) + } +} diff --git a/internal/auth/session/middleware.go b/internal/auth/session/middleware.go index 962b1e7..4d8c26c 100644 --- a/internal/auth/session/middleware.go +++ b/internal/auth/session/middleware.go @@ -32,6 +32,7 @@ package session import ( "context" + "errors" "net/http" "github.com/certctl-io/certctl/internal/auth" @@ -93,7 +94,13 @@ func NewSessionMiddleware(svc SessionValidator) func(http.Handler) http.Handler // the next middleware so a valid Bearer can still // authenticate. The auth combinator 401s if neither // works. - next.ServeHTTP(w, r) + // + // Audit 2026-05-10 HIGH-8 — stash the cause classification + // in context so the 401 emitter can emit a + // WWW-Authenticate: Bearer error_description="" + // header. OIDC users get cause-aware re-login UX. + ctx := context.WithValue(r.Context(), sessionCauseKey{}, classifySessionError(verr)) + next.ServeHTTP(w, r.WithContext(ctx)) return } @@ -225,6 +232,15 @@ func bearerSkipIfAuthenticated(bearerMW func(http.Handler) http.Handler) func(ht next.ServeHTTP(w, r) return } + // Audit 2026-05-10 HIGH-8 — emit WWW-Authenticate with the + // classified cause so the GUI can render OIDC-aware + // re-login UX. RFC 6750 §3 challenge format. + cause, _ := r.Context().Value(sessionCauseKey{}).(string) + if cause == "" { + cause = "invalid_token" + } + w.Header().Set("WWW-Authenticate", + `Bearer realm="certctl", error="invalid_token", error_description="`+cause+`"`) w.Header().Set("Content-Type", "application/json; charset=utf-8") http.Error(w, `{"error":"Authentication required"}`, http.StatusUnauthorized) }) @@ -238,12 +254,42 @@ func bearerSkipIfAuthenticated(bearerMW func(http.Handler) http.Handler) func(ht next.ServeHTTP(w, r) return } - // Defer to Bearer. + // Defer to Bearer. If the Bearer middleware 401s and there's + // a stashed session cause, downstream callers see it via the + // context key; the Bearer middleware's own 401 doesn't read + // it (Bearer-only deployments have no session context to + // stash from). Cause-aware UX needs session-mode auth. bearerInner.ServeHTTP(w, r) }) } } +// sessionCauseKey is the context key used by Audit 2026-05-10 HIGH-8. +// SessionMiddleware stashes the failure-cause classification on the +// context when Validate returns an error; the 401 emitter reads it +// and renders WWW-Authenticate's error_description. +type sessionCauseKey struct{} + +// classifySessionError maps a session Validate error to a stable +// wire-string the GUI consumes to render OIDC-aware re-login UX. +// Stable categories: idle_timeout, absolute_timeout, +// back_channel_revoked, invalid_token. +func classifySessionError(err error) string { + if err == nil { + return "" + } + switch { + case errors.Is(err, ErrSessionExpiredIdle): + return "idle_timeout" + case errors.Is(err, ErrSessionExpiredAbsolute): + return "absolute_timeout" + case errors.Is(err, ErrSessionRevoked): + return "back_channel_revoked" + default: + return "invalid_token" + } +} + // ============================================================================= // Helpers. // ============================================================================= diff --git a/internal/auth/session/www_authenticate_test.go b/internal/auth/session/www_authenticate_test.go new file mode 100644 index 0000000..7d812a7 --- /dev/null +++ b/internal/auth/session/www_authenticate_test.go @@ -0,0 +1,135 @@ +package session + +import ( + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + sessiondomain "github.com/certctl-io/certctl/internal/auth/session/domain" +) + +// Audit 2026-05-10 HIGH-8 regression tests pinning the cause-aware +// WWW-Authenticate header. Pre-fix, every session-cookie failure +// emitted a generic 401 with no machine-readable cause; OIDC users +// who hit idle-timeout / absolute-timeout / back-channel-revoked +// got an indistinguishable "Authentication required" with no hint +// about how to recover. Post-fix, the 401 emitter sets: +// +// WWW-Authenticate: Bearer realm="certctl", error="invalid_token", +// error_description="" +// +// where ∈ {idle_timeout, absolute_timeout, +// back_channel_revoked, invalid_token}. The GUI reads this on its +// fetch wrapper and routes the user into OIDC re-login (vs a generic +// "logged out" notice) when the cause is BCL revocation. + +// classifySessionError direct-test matrix — pin the four stable +// wire-strings the GUI consumes. +func TestClassifySessionError_StableCategories(t *testing.T) { + cases := []struct { + name string + err error + want string + }{ + {"nil", nil, ""}, + {"idle", ErrSessionExpiredIdle, "idle_timeout"}, + {"absolute", ErrSessionExpiredAbsolute, "absolute_timeout"}, + {"revoked", ErrSessionRevoked, "back_channel_revoked"}, + {"opaque", errors.New("totally-other-cause"), "invalid_token"}, + // Wrapped sentinels still classify (errors.Is). + {"wrapped_idle", wrap(ErrSessionExpiredIdle, "outer"), "idle_timeout"}, + {"wrapped_revoked", wrap(ErrSessionRevoked, "outer"), "back_channel_revoked"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := classifySessionError(tc.err) + if got != tc.want { + t.Errorf("classifySessionError(%v) = %q; want %q", + tc.err, got, tc.want) + } + }) + } +} + +// HIGH-8: a 401 emitted from bearerSkipIfAuthenticated when no +// Bearer middleware is wired must carry WWW-Authenticate with +// error_description= when the upstream SessionMiddleware +// stashed a cause classification. +func TestBearerSkipIfAuthenticated_Emits_WWWAuthenticate_WithCause(t *testing.T) { + cases := []struct { + name string + sessErr error + wantCause string + }{ + {"idle_timeout", ErrSessionExpiredIdle, "idle_timeout"}, + {"absolute_timeout", ErrSessionExpiredAbsolute, "absolute_timeout"}, + {"back_channel_revoked", ErrSessionRevoked, "back_channel_revoked"}, + {"opaque_falls_back_to_invalid_token", errors.New("opaque"), "invalid_token"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + stub := &stubSessionValidator{validateErr: tc.sessErr} + // Bearer middleware nil so the chain emits its own 401. + chain := ChainAuthSessionThenBearer(NewSessionMiddleware(stub), nil)(markAuthenticated()) + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.AddCookie(&http.Cookie{ + Name: sessiondomain.PostLoginCookieName, + Value: "v1.ses.sk.bad", + }) + w := httptest.NewRecorder() + chain.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d; want 401", w.Code) + } + ww := w.Header().Get("WWW-Authenticate") + if !strings.Contains(ww, `Bearer realm="certctl"`) { + t.Errorf("WWW-Authenticate = %q; want Bearer realm=\"certctl\"", ww) + } + if !strings.Contains(ww, `error="invalid_token"`) { + t.Errorf("WWW-Authenticate = %q; want error=\"invalid_token\"", ww) + } + wantDesc := `error_description="` + tc.wantCause + `"` + if !strings.Contains(ww, wantDesc) { + t.Errorf("WWW-Authenticate = %q; want %s", ww, wantDesc) + } + }) + } +} + +// HIGH-8: a 401 emitted with NO upstream session context (no cookie +// at all) still carries WWW-Authenticate, but with the +// invalid_token fallback (no stashed cause). +func TestBearerSkipIfAuthenticated_NoSessionContext_FallsBackToInvalidToken(t *testing.T) { + stub := &stubSessionValidator{validateErr: ErrSessionInvalidCookie} + chain := ChainAuthSessionThenBearer(NewSessionMiddleware(stub), nil)(markAuthenticated()) + req := httptest.NewRequest(http.MethodGet, "/x", nil) + // No cookie at all → SessionMiddleware skips entirely and falls + // through; bearerSkipIfAuthenticated emits 401 without a stashed + // cause; should fall back to error_description="invalid_token". + w := httptest.NewRecorder() + chain.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d; want 401", w.Code) + } + ww := w.Header().Get("WWW-Authenticate") + if !strings.Contains(ww, `error_description="invalid_token"`) { + t.Errorf("WWW-Authenticate = %q; want fallback error_description=\"invalid_token\"", ww) + } +} + +// wrap is a tiny errors.Wrap-style helper used by the wrapped-sentinel +// classifier matrix above. We can't pull in fmt.Errorf with %w as a +// const here, so this is the local convenience. +func wrap(inner error, outer string) error { + return &wrappedErr{inner: inner, outer: outer} +} + +type wrappedErr struct { + inner error + outer string +} + +func (w *wrappedErr) Error() string { return w.outer + ": " + w.inner.Error() } +func (w *wrappedErr) Unwrap() error { return w.inner } diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 12a5ce8..2fb3e66 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -72,6 +72,31 @@ function readCSRFCookie(): string { return ''; } +// Audit 2026-05-10 HIGH-8 — extract the session-failure cause from the +// WWW-Authenticate header the server emits on 401. The server format +// (RFC 6750 §3) is: `Bearer realm="certctl", error="invalid_token", +// error_description=""` where is one of the stable +// categories `idle_timeout` / `absolute_timeout` / +// `back_channel_revoked` / `invalid_token`. Returns "" when the +// header is missing, malformed, or carries an unrecognised cause — +// the AuthProvider falls back to the generic "Session expired" UX +// in that case (forward-compat with future categories). +function parseWWWAuthenticateCause(header: string | null): string { + if (!header) return ''; + const m = header.match(/error_description="([^"]+)"/i); + if (!m) return ''; + const cause = m[1]; + switch (cause) { + case 'idle_timeout': + case 'absolute_timeout': + case 'back_channel_revoked': + case 'invalid_token': + return cause; + default: + return ''; + } +} + // isStateChangingMethod mirrors the server-side // internal/auth/session/middleware.go::isStateChangingMethod predicate. // State-changing requests get the X-CSRF-Token header auto-attached @@ -106,8 +131,14 @@ async function fetchJSON(url: string, init?: RequestInit): Promise { headers, // intentional: spread init first, then override headers with the merged map (init.headers already merged into `headers` above) }); if (res.status === 401) { - // Trigger re-auth - const event = new CustomEvent('certctl:auth-required'); + // Audit 2026-05-10 HIGH-8 — propagate the WWW-Authenticate + // error_description so the AuthProvider can route the user into + // OIDC-aware re-login UX instead of generic "session expired." + // Stable cause categories: idle_timeout, absolute_timeout, + // back_channel_revoked, invalid_token. Anything else is treated + // as invalid_token by the server-side classifier. + const cause = parseWWWAuthenticateCause(res.headers.get('WWW-Authenticate')); + const event = new CustomEvent('certctl:auth-required', { detail: { cause } }); window.dispatchEvent(event); throw new Error('Authentication required'); } @@ -827,7 +858,9 @@ export const retireAgent = async ( }); if (res.status === 401) { - window.dispatchEvent(new CustomEvent('certctl:auth-required')); + // Audit 2026-05-10 HIGH-8 — see fetchAPI() for the cause-extraction rationale. + const cause = parseWWWAuthenticateCause(res.headers.get('WWW-Authenticate')); + window.dispatchEvent(new CustomEvent('certctl:auth-required', { detail: { cause } })); throw new Error('Authentication required'); } diff --git a/web/src/components/AuthProvider.tsx b/web/src/components/AuthProvider.tsx index 9494f8b..b3d6345 100644 --- a/web/src/components/AuthProvider.tsx +++ b/web/src/components/AuthProvider.tsx @@ -66,14 +66,35 @@ export default function AuthProvider({ children }: { children: ReactNode }) { .finally(() => setLoading(false)); }, []); - // Listen for 401 events from the API client + // Listen for 401 events from the API client. + // + // Audit 2026-05-10 HIGH-8 — the API client now attaches a cause + // category to the event detail (parsed from the WWW-Authenticate + // header). When a cause is recognised, redirect to + // /login?session_expired= so the LoginPage renders OIDC-aware + // re-login wording instead of the generic "session expired" + API-key + // copy. Cookie-mode (OIDC) and Bearer-mode (API-key) callers share + // the same wire shape; the LoginPage banner is purely UX. useEffect(() => { - const handler = () => { + const handler = (e: Event) => { + const detail = (e as CustomEvent<{ cause?: string }>).detail; + const cause = detail?.cause || ''; setAuthenticated(false); setApiKey(null); setUser(''); setAdmin(false); + // Generic copy; the LoginPage will overlay a cause-specific + // banner when ?session_expired= is present. setError('Session expired. Please re-enter your API key.'); + // Forward the cause to the LoginPage. window.location is used + // (not React Router's navigate) because this listener fires + // outside any route component's render and we want a hard + // navigation that clears any stale state. + if (cause && cause !== 'invalid_token' && + window.location.pathname !== '/login') { + const params = new URLSearchParams({ session_expired: cause }); + window.location.href = '/login?' + params.toString(); + } }; window.addEventListener('certctl:auth-required', handler); return () => window.removeEventListener('certctl:auth-required', handler); diff --git a/web/src/components/ErrorState.tsx b/web/src/components/ErrorState.tsx index c88d555..fc927f2 100644 --- a/web/src/components/ErrorState.tsx +++ b/web/src/components/ErrorState.tsx @@ -1,16 +1,39 @@ +// ErrorState supports two call shapes: +// 1. error-object form: +// 2. title+message form: +// +// The title/message form was added by Audit 2026-05-10 CRIT-4 +// (BreakglassPage admin GUI) so pages can render a denied/disabled +// banner without manufacturing a synthetic Error. When `title` is +// supplied, it takes precedence over the default headline; when +// `message` is supplied, it takes precedence over `error.message`. interface ErrorStateProps { - error: Error; + error?: Error; onRetry?: () => void; + title?: string; + message?: string; + 'data-testid'?: string; } -export default function ErrorState({ error, onRetry }: ErrorStateProps) { +export default function ErrorState({ + error, + onRetry, + title, + message, + 'data-testid': dataTestid, +}: ErrorStateProps) { + const headline = title ?? 'Failed to load data'; + const detail = message ?? error?.message ?? ''; return ( -
+
-

Failed to load data

-

{error.message}

+

{headline}

+ {detail &&

{detail}

} {onRetry && (
+ {oidcReasonText && ( +
+
Sign-in with your identity provider failed
+
{oidcReasonText}
+
+ )} + + {sessionCauseText && ( +
+
You've been signed out
+
{sessionCauseText}
+
+ )} + {providers.length > 0 && (