fix(auth): sign out and redirect on 401 from the CM API

[DavidCockerill]

Draft — companion to the central-manager fabric-admin session work (fabric-admin-auth-hardening, adds a FABRIC_ADMIN_SESSION_MAX_AGE_HOURS cap). Keeping this in draft until that lands. Useful on its own, though — it fixes the client reaction to any lost session.

Problem

The CM apiClient (apiClient.ts) has no response interceptor, and the dashboard guard (dashboardRoute.ts) only redirects when the cached authStore user is already null. So when a cloud session is lost mid-use — normal cookie expiry, server-side revocation, or a fabric admin hitting the new max-age cap — the SPA keeps rendering off the stale cached user while every data call fails with Not allowed errors. The user isn't bounced to sign-in until they manually refresh (which re-runs getCurrentUser, gets 401, and empties the store).

Verified live on dev: after a fabric-admin session was invalidated server-side, the UI kept navigating (cached authStore) and only redirected on a hard refresh.

Fix

Add a response interceptor on apiClient: on 401, clear the cached cloud auth (authStore.setUserForEntity(OverallAppSignIn, null)). That triggers the existing AppRouted router.invalidate() effect → guards re-run → redirect to /sign-in. No imperative router access needed.

Only 401 triggers it. A dead session yields 401 from GET /User/current (its allowRead returns false when there's no user). 403 is left alone — it's a legitimate "authenticated but not permitted" response and must not log the user out.

Where to look

• src/lib/unauthorizedResponseHandler.ts — the 401 detection, factored app-import-free so it unit-tests in node env (importing authStore pulls in localStorage at module load).
• src/lib/installApiUnauthorizedRedirect.ts — wires it to apiClient with the authStore-clearing callback.
• src/main.tsx — installed at startup alongside the other install* guards.

Testing

• vitest run src/lib/unauthorizedResponseHandler.test.ts — 3 passing (401 clears + rejects; 403 doesn't; network error doesn't).
• Not yet covered: the full interceptor→guard→redirect path in a running app. Worth a manual check once merged with the CM cap (sign in, let the session expire, confirm auto-redirect without a manual refresh).

Open question for reviewers

403s from a dead session (admin endpoints return Not allowed/403, not 401) won't trigger this. Keying on 401 only is the safe choice (avoids logging out on genuine permission denials), but it means a data call that only ever 403s won't itself force logout — the redirect comes from the next 401-returning call (e.g. getCurrentUser on focus/refetch). Flagging in case we want 403-on-/User/current handling too.

Generated with the assistance of an LLM (Claude Opus 4.8).

[gemini-code-assist]

Code Review

This pull request introduces an Axios response interceptor to automatically clear the cached user authentication state and redirect to the sign-in page when a 401 Unauthorized response is received. The feedback suggests making the error handler more robust by guarding against undefined or non-Axios errors, and adding corresponding unit tests to verify this behavior.

[gemini-code-assist]

Axios response interceptor rejection handlers can receive non-Axios errors, generic Error objects, or even undefined depending on how upstream interceptors or network adapters behave. Accessing error.response directly without guarding against a null/undefined error can lead to unhandled runtime TypeError exceptions. Typing the parameter as any and using optional chaining on error ensures robust error handling.

Suggested change

	return (error: AxiosError): Promise<never> => {
	if (error.response?.status === 401) {
	clearAuth();
	}
	// Re-reject so individual callers still see (and can handle) the error.
	return Promise.reject(error);
	};
	return (error: any): Promise<never> => {
	if (error?.response?.status === 401) {
	clearAuth();
	}
	// Re-reject so individual callers still see (and can handle) the error.
	return Promise.reject(error);
	};

[DavidCockerill]

Applied in ba6a7f8 — good defensive catch. One deviation from the suggestion: kept the AxiosError typing with error?.response?.status rather than widening the parameter to any, which would drop type safety for no extra robustness. (Today this handler is the only response interceptor on apiClient, and axios itself always rejects with an AxiosError, so the guard is future-proofing for interceptors added later.)

— Claude Opus 4.8 (on behalf of @DavidCockerill)

[gemini-code-assist]

Add a test case to verify that the handler does not crash when receiving an undefined error or a generic error without a response object.

	it("does not clear auth on a network error with no response", async () => {
		const clearAuth = vi.fn();
		const error = errorWithStatus(undefined);
		await expect(makeUnauthorizedResponseHandler(clearAuth)(error)).rejects.toBe(error);
		expect(clearAuth).not.toHaveBeenCalled();
	});

	it("does not crash when error is undefined", async () => {
		const clearAuth = vi.fn();
		await expect(makeUnauthorizedResponseHandler(clearAuth)(undefined as any)).rejects.toBe(undefined);
		expect(clearAuth).not.toHaveBeenCalled();
	});

[DavidCockerill]

Partially applied in ba6a7f8. Note the first suggested test ("does not clear auth on a network error with no response") already exists in this file — see the third it block. Added the second one (undefined rejection reason passes through without throwing), which became meaningful once the guard landed.

— Claude Opus 4.8 (on behalf of @DavidCockerill)

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	50.99%	5436 / 10660
🔵	Statements	51.56%	5820 / 11287
🔵	Functions	43.26%	1334 / 3083
🔵	Branches	44.48%	3671 / 8252

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/lib/unauthorizedResponseHandler.ts	100%	100%	100%	100%

Generated in workflow #1556 for commit 70fb4dc by the Vitest Coverage Report Action

[dawsontoth]

sounds good to me, lemme know when you're out of draft :D

[DavidCockerill]

@dawsontoth once my CM work relating to this is done i'll move it out of draft. Just in case i need to modify anything

[kriszyp]

Nice

[kriszyp]

Question, not a blocker: apiClient also carries /Login/, /ResetPassword/, /VerifyEmail/, and /ForgotPassword/ — none of which represent a lost CM session. For a signed-out visitor clearAuth() is a no-op today (already null), but if the CM API ever models an invalid/expired reset or verify-email token as 401 (rather than 400/410), a user who's signed in in one tab and opens a stale verify/reset link in another tab would get signed out of their live session as a side effect. Could you confirm those endpoints don't 401 for "bad token" cases? If they can, might be worth scoping this interceptor away from the unauthenticated auth-flow requests.

[DavidCockerill]

Great catch — and it's not hypothetical: CM's verifyToken (sharedHelpers.js) throws ERROR_CODE.UNAUTHORIZED for a bad/expired token, and both ResetPassword and VerifyEmail route through it — plus /Login 401s on bad credentials. So a stale verify/reset link in a second tab would have signed out a live session today. Fixed in 70fb4dc: the handler now takes an exemption predicate and the installer scopes it away from the unauthenticated auth flows (/Login/, /ForgotPassword/, /ResetPassword/, /VerifyEmail/, /ResendVerificationEmail/), with tests for exempt/non-exempt/missing-config cases.

— Claude Opus 4.8 (on behalf of @DavidCockerill)

[kriszyp]

Nit: the other install*.ts helpers in this directory (installBrowserTranslationDomGuard, installStaleDeployReload) explicitly guard/document being safe to call more than once (HMR, accidental re-invocation). This one registers the interceptor unconditionally, so a second call would stack a duplicate (harmless today since main.tsx calls it once and clearAuth() is idempotent, but worth matching the sibling convention for consistency).

[DavidCockerill]

Applied in 70fb4dc — module-level installed flag, matching the sibling install* convention; re-invocations are no-ops.

— Claude Opus 4.8 (on behalf of @DavidCockerill)

[dawsontoth]

I like Kris' suggestions, mind folding those in to this PR?

[DavidCockerill]

@dawsontoth folded in (70fb4dc) — Kris's first point turned out to be a live bug, not just a question: CM 401s stale reset/verify-email tokens, so a stale link in another tab would have signed out an active session. The interceptor is now scoped away from the unauthenticated auth flows, and install is idempotent. Still holding draft until the related CM session work (central-manager#459) lands.

— Claude Opus 4.8 (on behalf of @DavidCockerill)

[kriszyp]

Looks good, maybe a race condition to address.

[kriszyp]

This clears only the cloud-user slot, so it is not equivalent to the existing logout path. After A's CM session expires, the in-memory entity connections/Fabric tokens, persisted basic/Fabric flags, and React Query cache remain; if B signs in without a reload, instanceLayoutRoute can trust A's still-populated entity connection before checking B's cloud permissions, and queries can reuse A's cached data. Could we centralize a local-only full-sign-out cleanup (all auth connections/tokens/flags plus user-scoped query/session data, without retrying the failed CM logout) and use it here? An A → 401 → B test should assert those stores are empty before B's routes load.

[kriszyp]

Could we guard this with the auth generation that was current when the request was sent? A normal expiry sequence can otherwise clear a fresh login: requests A/B leave with the expired cookie, A's 401 redirects to sign-in, /Login/ succeeds and stores the new user, then slower B returns its old 401 and this unconditional handler clears the new session. Stamping the request config in a request interceptor (generation or user identity) and only clearing when it still matches would close the race; a deferred-response test resolving B after simulated login would cover it.