Files
certctl/internal/auth/session/www_authenticate_test.go
shankar0123 0f340beb14 fix(auth/ux): cause-aware OIDC + session error surfacing (HIGH-7 + HIGH-8 closure)
Server (HIGH-7): the OIDC callback failure path now 302-redirects to
/login?error=oidc_failed&reason=<category> 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=<cause> 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=<cause> 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
2026-05-10 21:12:11 +00:00

136 lines
5.0 KiB
Go

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="<cause>"
//
// where <cause> ∈ {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=<cause> 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 }