diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c9311f..65b550e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,29 @@ manually. Production deploys: this guard is irrelevant (`CERTCTL_DEMO_MODE_ACK` should not be set in production). +### Fixed + +- **GitHub #13 / Hotfix #19 — GUI "Something went wrong" after browser + refresh on a real (non-demo) install.** Refresh-after-login wipes the + in-memory `apiKey` (deliberate — the GUI never persists it to + localStorage as a security posture). The next API call returns a + bare 401 with no `WWW-Authenticate` header. Pre-Hotfix-19 the + AuthProvider 401 handler only hard-navigated to `/login` when `cause` + was a recognised OIDC session-expiry category (`idle_timeout` / + `absolute_timeout` / `back_channel_revoked`); bare 401s + (`cause === ''`) and `invalid_token` causes fell through to an + in-place `AuthGate` state flip that unmounted `BrowserRouter` under + an in-flight ``, triggering a `react-router-dom` invariant + that surfaced via `ErrorBoundary` as the "Something went wrong" + screen. **Fix:** every 401 now hard-navigates to `/login` regardless + of cause; the cause-aware UX is preserved by forwarding + `?session_expired=` only when cause is non-empty (bare 401s + redirect to plain `/login`). Three-line change in + `web/src/components/AuthProvider.tsx`; 4 regression tests added to + `AuthProvider.test.tsx` (empty cause from `/targets`, `invalid_token` + cause, `idle_timeout` cause, already-on-`/login` no-op guard). + Closes #13. + ### Security - **Alg-downgrade defense relaxed for Keycloak-shape IdPs (v2.1.0 pre-tag fix).** diff --git a/web/src/components/AuthProvider.test.tsx b/web/src/components/AuthProvider.test.tsx index c5ef50c..037c787 100644 --- a/web/src/components/AuthProvider.test.tsx +++ b/web/src/components/AuthProvider.test.tsx @@ -132,3 +132,130 @@ describe('AuthProvider — LOW-1 demo-mode banner', () => { await waitFor(() => screen.getByTestId('demo-mode-banner')); }); }); + +// ============================================================================= +// Hotfix #19 (GitHub #13) — AuthProvider 401 unconditional-redirect. +// +// The pre-Hotfix-19 401 handler only redirected to /login when `cause` +// was a recognised OIDC session-expiry category. A bare 401 (no +// WWW-Authenticate header → cause === '') fell through to an in-place +// AuthGate state flip that unmounted BrowserRouter under an in-flight +// , triggering a react-router-dom invariant that surfaced via +// ErrorBoundary as "Something went wrong" (GitHub #13). +// +// These tests pin: every 401 (regardless of cause) hard-navigates to +// /login when the caller is not already on /login. Cause-aware +// session_expired= query param is preserved when cause is non-empty. +// ============================================================================= + +describe('AuthProvider — Hotfix #19 401 always-redirects', () => { + let originalLocation: Location; + let hrefAssignments: string[]; + + beforeEach(() => { + // /auth/info is unrelated to the 401 path but must not hang the + // mount. Resolve it as the demo case (the cheapest non-pending + // shape) — the redirect handler doesn't care about authType. + vi.mocked(client.getAuthInfo).mockResolvedValue({ + auth_type: 'none', + required: false, + }); + + // jsdom forbids writing to window.location.href directly without + // a settable property descriptor. Replace window.location with a + // mock that captures assignments while letting tests pre-set + // pathname. Restored in afterEach. + originalLocation = window.location; + hrefAssignments = []; + }); + + function installLocationMock(pathname: string): void { + Object.defineProperty(window, 'location', { + configurable: true, + writable: true, + value: { + pathname, + get href() { return ''; }, + set href(v: string) { hrefAssignments.push(v); }, + }, + }); + } + + function restoreLocation(): void { + Object.defineProperty(window, 'location', { + configurable: true, + writable: true, + value: originalLocation, + }); + } + + it('redirects to /login with no query param when cause is empty (bare 401)', async () => { + installLocationMock('/targets'); + try { + render(
child
); + await waitFor(() => screen.getByTestId('child')); + + window.dispatchEvent( + new CustomEvent('certctl:auth-required', { detail: { cause: '' } }), + ); + + expect(hrefAssignments).toEqual(['/login']); + } finally { + restoreLocation(); + } + }); + + it('redirects to /login?session_expired=invalid_token when cause is invalid_token (new behavior)', async () => { + // Pre-Hotfix-19 this cause fell through the conditional with no + // redirect. Post-Hotfix-19 every 401 redirects; cause is preserved + // in the query param for any LoginPage banner that wants it. + installLocationMock('/targets'); + try { + render(
child
); + await waitFor(() => screen.getByTestId('child')); + + window.dispatchEvent( + new CustomEvent('certctl:auth-required', { detail: { cause: 'invalid_token' } }), + ); + + expect(hrefAssignments).toEqual(['/login?session_expired=invalid_token']); + } finally { + restoreLocation(); + } + }); + + it('redirects to /login?session_expired=idle_timeout when cause is idle_timeout (existing OIDC UX preserved)', async () => { + installLocationMock('/targets'); + try { + render(
child
); + await waitFor(() => screen.getByTestId('child')); + + window.dispatchEvent( + new CustomEvent('certctl:auth-required', { detail: { cause: 'idle_timeout' } }), + ); + + expect(hrefAssignments).toEqual(['/login?session_expired=idle_timeout']); + } finally { + restoreLocation(); + } + }); + + it('does not redirect when caller is already on /login (no-op guard preserved)', async () => { + installLocationMock('/login'); + try { + render(
child
); + await waitFor(() => screen.getByTestId('child')); + + window.dispatchEvent( + new CustomEvent('certctl:auth-required', { detail: { cause: '' } }), + ); + window.dispatchEvent( + new CustomEvent('certctl:auth-required', { detail: { cause: 'idle_timeout' } }), + ); + + expect(hrefAssignments).toEqual([]); + } finally { + restoreLocation(); + } + }); +}); diff --git a/web/src/components/AuthProvider.tsx b/web/src/components/AuthProvider.tsx index b67fefa..66b612a 100644 --- a/web/src/components/AuthProvider.tsx +++ b/web/src/components/AuthProvider.tsx @@ -90,10 +90,26 @@ export default function AuthProvider({ children }: { children: ReactNode }) { // (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(); + // + // Hotfix #19 (GitHub #13): always hard-navigate to /login on a + // 401, regardless of cause. Pre-Hotfix-19 the conditional only + // redirected when cause was a non-'invalid_token' OIDC + // session-expiry category (idle_timeout / absolute_timeout / + // back_channel_revoked). Bare 401s (refresh-after-login wipes + // the in-memory apiKey → no Authorization header → server + // returns 401 with no WWW-Authenticate header → cause === '') + // fell through to an in-place AuthGate state flip that + // unmounted BrowserRouter under an in-flight , triggering + // a react-router-dom invariant that surfaced via ErrorBoundary + // as "Something went wrong." The unconditional hard-navigation + // forecloses the in-place tear-down path; cause-aware UX is + // preserved by forwarding ?session_expired= only when cause is + // non-empty. + if (window.location.pathname !== '/login') { + const url = cause + ? '/login?' + new URLSearchParams({ session_expired: cause }).toString() + : '/login'; + window.location.href = url; } }; window.addEventListener('certctl:auth-required', handler);