-
Notifications
You must be signed in to change notification settings - Fork 3
feat(auth): restart login on auth_error callback bounces, loop-guarded #239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
cyberantonz
merged 3 commits into
constructorfabric:main
from
cyberantonz:feat/auth-error-loop-guard
Jul 30, 2026
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<History["replaceState"]>; | ||
|
|
||
| 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); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| // A failed OIDC callback bounces the browser back to the SPA with | ||
| // `?auth_error=<reason>` (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; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 ( | ||
| <main className="flex min-h-svh w-full items-center justify-center p-6"> | ||
| <Card className="w-full max-w-sm"> | ||
| <CardContent className="flex flex-col items-center gap-4 text-center"> | ||
| <h1 className="text-sm font-medium">{t("auth.loginFailedTitle")}</h1> | ||
| <p className="text-sm text-muted-foreground" role="alert"> | ||
| {code === "access_denied" | ||
| ? t("auth.loginFailedAccessDenied") | ||
| : t("auth.loginFailedRetryable")} | ||
| </p> | ||
| <Button | ||
| onClick={() => { | ||
| clearAuthErrorAttempts(); | ||
| signIn(); | ||
| }} | ||
| > | ||
| {t("auth.loginFailedTryAgain")} | ||
| </Button> | ||
| </CardContent> | ||
| </Card> | ||
| </main> | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.