diff --git a/src/auth/auth-error.test.ts b/src/auth/auth-error.test.ts new file mode 100644 index 0000000..b7c374f --- /dev/null +++ b/src/auth/auth-error.test.ts @@ -0,0 +1,133 @@ +import { + afterEach, + beforeEach, + describe, + expect, + it, + vi, + type MockInstance, +} from "vitest"; + +import { clearAuthErrorAttempts, consumeAuthErrorParam } from "./auth-error"; + +describe("consumeAuthErrorParam", () => { + let replaceState: MockInstance; + + function stubLocation(href: string) { + const url = new URL(href); + Object.defineProperty(window, "location", { + configurable: true, + value: { ...window.location, href: url.href }, + }); + } + + beforeEach(() => { + replaceState = vi + .spyOn(window.history, "replaceState") + .mockImplementation(() => undefined); + sessionStorage.clear(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("is a no-op without the parameter", () => { + stubLocation("https://insight.test/?tab=stats"); + + expect(consumeAuthErrorParam()).toBeNull(); + expect(replaceState).not.toHaveBeenCalled(); + }); + + it("consumes the parameter, keeps the rest of the URL, and allows one auto-retry", () => { + stubLocation("https://insight.test/?auth_error=state_expired&tab=stats"); + + expect(consumeAuthErrorParam()).toEqual({ + code: "state_expired", + autoRetry: true, + }); + expect(replaceState).toHaveBeenCalledWith(null, "", "/?tab=stats"); + }); + + it("halts after the auto-retry budget is spent", () => { + stubLocation("https://insight.test/?auth_error=state_expired"); + expect(consumeAuthErrorParam()?.autoRetry).toBe(true); + + // The retried login failed again: same bounce, same tab. + stubLocation("https://insight.test/?auth_error=state_expired"); + expect(consumeAuthErrorParam()?.autoRetry).toBe(false); + }); + + it("strips an empty auth_error without counting it", () => { + stubLocation("https://insight.test/?auth_error=&tab=stats"); + + expect(consumeAuthErrorParam()).toBeNull(); + expect(replaceState).toHaveBeenCalledWith(null, "", "/?tab=stats"); + // The budget is untouched: the next real failure still auto-retries. + stubLocation("https://insight.test/?auth_error=state_expired"); + expect(consumeAuthErrorParam()?.autoRetry).toBe(true); + }); + + it("preserves the hash when stripping the parameter", () => { + stubLocation("https://insight.test/board?auth_error=idp_error#section-2"); + + expect(consumeAuthErrorParam()?.code).toBe("idp_error"); + expect(replaceState).toHaveBeenCalledWith(null, "", "/board#section-2"); + }); + + it("never auto-retries access_denied", () => { + stubLocation("https://insight.test/?auth_error=access_denied"); + + expect(consumeAuthErrorParam()).toEqual({ + code: "access_denied", + autoRetry: false, + }); + }); + + it("clearAuthErrorAttempts restores the auto-retry budget", () => { + stubLocation("https://insight.test/?auth_error=state_expired"); + expect(consumeAuthErrorParam()?.autoRetry).toBe(true); + stubLocation("https://insight.test/?auth_error=state_expired"); + expect(consumeAuthErrorParam()?.autoRetry).toBe(false); + + clearAuthErrorAttempts(); + + stubLocation("https://insight.test/?auth_error=state_expired"); + expect(consumeAuthErrorParam()?.autoRetry).toBe(true); + }); + + it("does not auto-retry when the attempt cannot be persisted", () => { + stubLocation("https://insight.test/?auth_error=state_expired"); + // Reads work, writes fail (e.g. quota): the next bounce would read zero + // again, so an unpersisted attempt must not spend a retry. + vi.stubGlobal("sessionStorage", { + getItem: () => null, + setItem: () => { + throw new Error("quota exceeded"); + }, + removeItem: () => undefined, + }); + + expect(consumeAuthErrorParam()).toEqual({ + code: "state_expired", + autoRetry: false, + }); + }); + + it("fails closed to the error screen when storage is unavailable", () => { + stubLocation("https://insight.test/?auth_error=state_expired"); + // Replace the global outright — spying on methods of the environment's + // Storage object does not reliably intercept the module's binding. + const disabled = () => { + throw new Error("storage disabled"); + }; + vi.stubGlobal("sessionStorage", { + getItem: disabled, + setItem: disabled, + removeItem: disabled, + }); + + expect(consumeAuthErrorParam()?.autoRetry).toBe(false); + }); +}); diff --git a/src/auth/auth-error.ts b/src/auth/auth-error.ts new file mode 100644 index 0000000..e13af71 --- /dev/null +++ b/src/auth/auth-error.ts @@ -0,0 +1,75 @@ +// A failed OIDC callback bounces the browser back to the SPA with +// `?auth_error=` (insight#2032) — there is no page loaded at +// `/auth/callback`, so the authenticator redirects instead of answering +// problem+json the user cannot act on. The retryable reasons (an expired +// login state after a slow IdP round-trip, a replayed callback, an IdP +// hiccup) are fixed by simply logging in again, so boot restarts the flow +// once; a sessionStorage attempt counter halts a persistent failure on the +// error screen instead of looping browser -> IdP forever. `access_denied` +// (unknown person / no tenant) never auto-retries — a silent SSO hop would +// just reproduce it. + +const ATTEMPTS_KEY = "insight.auth-error-attempts"; +const MAX_AUTO_RETRIES = 1; + +export type AuthError = { + /** The authenticator's fixed reason code, e.g. `state_expired`. */ + code: string; + /** Restart the login automatically, or halt on the error screen. */ + autoRetry: boolean; +}; + +/** + * If the current URL carries `auth_error`, strip it (so reloads, copied + * links, and the next login's `return_to` don't carry it), count the attempt, + * and return the code plus the auto-retry verdict. `null` when absent. + */ +export function consumeAuthErrorParam(): AuthError | null { + const url = new URL(window.location.href); + if (!url.searchParams.has("auth_error")) return null; + const code = url.searchParams.get("auth_error") ?? ""; + url.searchParams.delete("auth_error"); + window.history.replaceState(null, "", url.pathname + url.search + url.hash); + // An empty value (hand-crafted URL — the authenticator always sends a + // reason) is stripped but neither counted nor acted on. + if (!code) return null; + const attempts = readAttempts() + 1; + // An unpersisted attempt must not auto-retry: the next bounce would read + // zero again and the guard would never trip. + const counted = writeAttempts(attempts); + return { + code, + autoRetry: + counted && code !== "access_denied" && attempts <= MAX_AUTO_RETRIES, + }; +} + +/** Reset the counter: on a confirmed session, or on a user-driven retry. */ +export function clearAuthErrorAttempts(): void { + try { + sessionStorage.removeItem(ATTEMPTS_KEY); + } catch { + // Storage unavailable — nothing to clear. + } +} + +function readAttempts(): number { + try { + const parsed = Number(sessionStorage.getItem(ATTEMPTS_KEY) ?? "0"); + return Number.isFinite(parsed) && parsed >= 0 ? parsed : 0; + } catch { + // Storage unavailable: attempts cannot be counted across the login + // round-trips, so fail closed to the error screen rather than risk an + // uncounted redirect loop. + return MAX_AUTO_RETRIES; + } +} + +function writeAttempts(attempts: number): boolean { + try { + sessionStorage.setItem(ATTEMPTS_KEY, String(attempts)); + return true; + } catch { + return false; + } +} diff --git a/src/auth/index.ts b/src/auth/index.ts index 3bbbc81..d62e2b2 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -1,4 +1,9 @@ export { authStore } from "./auth-store"; +export { + clearAuthErrorAttempts, + consumeAuthErrorParam, + type AuthError, +} from "./auth-error"; export { consumeOverrideParam } from "./override"; export { loadSession } from "./session"; export { startSessionRefresh } from "./refresh"; diff --git a/src/auth/use-auth.test.ts b/src/auth/use-auth.test.ts index 2f82649..4db3b3b 100644 --- a/src/auth/use-auth.test.ts +++ b/src/auth/use-auth.test.ts @@ -47,6 +47,24 @@ describe("signIn", () => { expect(assign).toHaveBeenCalledWith("/auth/login?return_to=%2F"); }); + it("defaults return_to to the current path, query, and hash", async () => { + Object.defineProperty(window, "location", { + configurable: true, + value: { + ...window.location, + pathname: "/board", + search: "?tab=1", + hash: "#row-9", + assign, + }, + }); + const signIn = await freshSignIn(); + signIn(); + expect(assign).toHaveBeenCalledWith( + `/auth/login?return_to=${encodeURIComponent("/board?tab=1#row-9")}` + ); + }); + it("does not stack redirects while one is in flight", async () => { const signIn = await freshSignIn(); signIn("/a"); diff --git a/src/auth/use-auth.ts b/src/auth/use-auth.ts index 36a4737..2280869 100644 --- a/src/auth/use-auth.ts +++ b/src/auth/use-auth.ts @@ -5,6 +5,15 @@ import type { AuthSnapshot } from "./types"; let redirecting = false; +// A bfcache restore (browser Back from the IdP) revives this module with +// `redirecting` still true, which would turn every later signIn — e.g. the +// login-error screen's "Try again" button — into a silent no-op. +if (typeof window !== "undefined") { + window.addEventListener("pageshow", (event) => { + if (event.persisted) redirecting = false; + }); +} + /** * Sanitize a return-to into a site-relative path (mirrors the backend guard). * `/auth/*` paths collapse to `/` — a return-to pointing back into the login @@ -24,7 +33,8 @@ export function signIn(returnTo?: string): void { if (redirecting) return; redirecting = true; const dest = safeReturnTo( - returnTo ?? window.location.pathname + window.location.search + returnTo ?? + window.location.pathname + window.location.search + window.location.hash ); window.location.assign(`/auth/login?return_to=${encodeURIComponent(dest)}`); } diff --git a/src/components/login-error.tsx b/src/components/login-error.tsx new file mode 100644 index 0000000..cc40f59 --- /dev/null +++ b/src/components/login-error.tsx @@ -0,0 +1,41 @@ +import { useTranslation } from "react-i18next"; + +import { clearAuthErrorAttempts, signIn } from "@/auth"; +import { Button } from "@/components/ui/button"; +import { Card, CardContent } from "@/components/ui/card"; + +type LoginErrorProps = { + /** The authenticator's `auth_error` reason code (insight#2032). */ + code: string; +}; + +/** + * Full-page stop after a failed login that auto-retry did not fix (or must + * not attempt — `access_denied`). Rendered by the boot sequence instead of + * the router, so it depends on nothing but the auth module and i18n. + */ +export function LoginError({ code }: LoginErrorProps): React.ReactElement { + const { t } = useTranslation(); + return ( +
+ + +

{t("auth.loginFailedTitle")}

+

+ {code === "access_denied" + ? t("auth.loginFailedAccessDenied") + : t("auth.loginFailedRetryable")} +

+ + + +
+ ); +} diff --git a/src/locales/en/translation.json b/src/locales/en/translation.json index 890f16f..c4c6b6f 100644 --- a/src/locales/en/translation.json +++ b/src/locales/en/translation.json @@ -191,6 +191,10 @@ "footer": "Insight · what's new · 13 July 2026" }, "auth": { + "loginFailedAccessDenied": "Your account is not authorized to access this application. Contact your administrator if you believe this is a mistake.", + "loginFailedRetryable": "Your sign-in could not be completed. This can happen when signing in takes too long — please try again.", + "loginFailedTitle": "Sign-in failed", + "loginFailedTryAgain": "Try again", "redirecting": "Redirecting to sign in…" }, "error_boundary": { diff --git a/src/main.tsx b/src/main.tsx index 8443605..9f35b15 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -6,8 +6,16 @@ import { I18nextProvider } from "react-i18next"; import "./index.css"; import { CatalogProvider } from "@/api/catalog-provider"; -import { consumeOverrideParam, loadSession, startSessionRefresh } from "@/auth"; +import { + clearAuthErrorAttempts, + consumeAuthErrorParam, + consumeOverrideParam, + loadSession, + signIn, + startSessionRefresh, +} from "@/auth"; import { AppErrorBoundary } from "@/components/app-error-boundary"; +import { LoginError } from "@/components/login-error"; import { ThemeProvider } from "@/components/theme-provider"; import i18n from "@/i18n"; import { queryClient } from "@/query-client"; @@ -25,30 +33,68 @@ async function enableMocking(): Promise { if (!consumeOverrideParam()) bootstrap(); function bootstrap(): void { + // A failed OIDC callback lands here as `?auth_error=` (#2032); + // consumed before the session probe so the router never sees it. + const authError = consumeAuthErrorParam(); void enableMocking() // Probe the session once (mocks, if enabled, intercept /auth/me) before the // router mounts, so the root beforeLoad reads a resolved auth store. .then(() => loadSession()) .then((status) => { - // The session is non-sliding — without the refresh driver it dies - // session_ttl (~10 min) after login regardless of activity (#1854). - if (status === "authenticated") startSessionRefresh(); - }) - .then(() => { - createRoot(document.getElementById("root")!).render( - - - - - - - - - - - - - - ); + if (status === "authenticated") { + // Covers the replayed-callback bounce too: the first callback already + // set the cookie, so an `auth_error` here is stale. + clearAuthErrorAttempts(); + // The session is non-sliding — without the refresh driver it dies + // session_ttl (~10 min) after login regardless of activity (#1854). + startSessionRefresh(); + renderApp(); + return; + } + if (authError?.autoRetry) { + // A fresh login fixes the retryable reasons (expired state after a + // slow IdP round-trip, IdP hiccup); the attempt counter halts a + // persistent failure on the error screen instead of looping. No-arg + // signIn: return to the current URL, already stripped of auth_error. + signIn(); + return; + } + if (authError) { + renderLoginError(authError.code); + return; + } + // Unauthenticated without an auth_error: the root beforeLoad bounces + // into the login flow. + renderApp(); }); } + +function renderApp(): void { + createRoot(document.getElementById("root")!).render( + + + + + + + + + + + + + + ); +} + +function renderLoginError(code: string): void { + createRoot(document.getElementById("root")!).render( + + + + + + + + ); +}