mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 11:21:29 +00:00
Refresh-after-login wiped the in-memory apiKey and the next API call returned a bare 401 (no WWW-Authenticate header). The pre-Hotfix-19 401 handler in AuthProvider only redirected when cause was a non-'invalid_token' OIDC session-expiry category; bare 401s fell through to an in-place AuthGate state flip that unmounted BrowserRouter under an in-flight <Link>, triggering a react-router-dom invariant that surfaced via ErrorBoundary as "Something went wrong." Fix: always hard-navigate to /login on 401 regardless of cause. Preserve cause-aware UX by forwarding cause to /login?session_expired= only when present; emit plain /login redirect for bare 401s. Closes #13.
This commit is contained in:
@@ -46,6 +46,29 @@
|
|||||||
manually. Production deploys: this guard is irrelevant
|
manually. Production deploys: this guard is irrelevant
|
||||||
(`CERTCTL_DEMO_MODE_ACK` should not be set in production).
|
(`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 `<Link>`, 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=<cause>` 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
|
### Security
|
||||||
|
|
||||||
- **Alg-downgrade defense relaxed for Keycloak-shape IdPs (v2.1.0 pre-tag fix).**
|
- **Alg-downgrade defense relaxed for Keycloak-shape IdPs (v2.1.0 pre-tag fix).**
|
||||||
|
|||||||
@@ -132,3 +132,130 @@ describe('AuthProvider — LOW-1 demo-mode banner', () => {
|
|||||||
await waitFor(() => screen.getByTestId('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
|
||||||
|
// <Link>, 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(<AuthProvider><div data-testid="child">child</div></AuthProvider>);
|
||||||
|
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(<AuthProvider><div data-testid="child">child</div></AuthProvider>);
|
||||||
|
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(<AuthProvider><div data-testid="child">child</div></AuthProvider>);
|
||||||
|
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(<AuthProvider><div data-testid="child">child</div></AuthProvider>);
|
||||||
|
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();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -90,10 +90,26 @@ export default function AuthProvider({ children }: { children: ReactNode }) {
|
|||||||
// (not React Router's navigate) because this listener fires
|
// (not React Router's navigate) because this listener fires
|
||||||
// outside any route component's render and we want a hard
|
// outside any route component's render and we want a hard
|
||||||
// navigation that clears any stale state.
|
// navigation that clears any stale state.
|
||||||
if (cause && cause !== 'invalid_token' &&
|
//
|
||||||
window.location.pathname !== '/login') {
|
// Hotfix #19 (GitHub #13): always hard-navigate to /login on a
|
||||||
const params = new URLSearchParams({ session_expired: cause });
|
// 401, regardless of cause. Pre-Hotfix-19 the conditional only
|
||||||
window.location.href = '/login?' + params.toString();
|
// 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 <Link>, 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);
|
window.addEventListener('certctl:auth-required', handler);
|
||||||
|
|||||||
Reference in New Issue
Block a user