From cfbf6011d46e734da18081a1e51a042212ba1d83 Mon Sep 17 00:00:00 2001 From: Babissimo Date: Mon, 14 Sep 2026 15:34:10 +0100 Subject: [PATCH] Stop the dashboard reloading itself when it cannot authenticate The API client answered every 401 by assigning window.location.href = "/login". On the login page that is a same-URL assignment, which reloads; the reload remounts AuthProvider, which calls /api/auth/me, which 401s, which assigns it again. Where nothing can mint a session the loop does not terminate, and the login card is only visible between reloads. Nothing has hit this yet because no deployed environment has ever reported the enforced auth mode: AUTH_BYPASS is only true when no OAuth client is configured, and while it was set /api/auth/me answered 200 to everyone. Closing the anonymous-admin bypass is what makes dash.retina.fm reach this path, and that vhost has no Cloudflare Access application in front of it by design. So: navigate only when the caller is somewhere else in the app. The comparison trims trailing slashes, because the router matches /login/ to the same route and a raw string compare would let that spelling through for one more reload. A 401 also stops being a bare Error, because AuthProvider could not tell it from the network and timeout failures its four-attempt backoff exists for, and spending those on a settled answer held the login card behind a loading state for around nine seconds. Every guard is mutation-checked one at a time: removing the path check fails the already-on-login test, removing the slash trim fails its /login/ case, and removing the break fails the settles-without-retrying test. Co-Authored-By: Claude Opus 5 --- dashboard/src/api/client.ts | 29 ++++++- dashboard/src/context/AuthContext.tsx | 8 +- dashboard/src/test/authUnauthorized.test.tsx | 79 ++++++++++++++++++++ 3 files changed, 111 insertions(+), 5 deletions(-) create mode 100644 dashboard/src/test/authUnauthorized.test.tsx diff --git a/dashboard/src/api/client.ts b/dashboard/src/api/client.ts index 96b5719d..3dcecd21 100644 --- a/dashboard/src/api/client.ts +++ b/dashboard/src/api/client.ts @@ -1,5 +1,25 @@ const BASE = ""; +/** A 401 from the API: an answer, not a failure to obtain one. Distinct from a + * network or timeout error so callers can tell "not signed in" from "no reply + * yet" and decline to retry the first. */ +export class UnauthorizedError extends Error { + constructor() { + super("Unauthorized"); + this.name = "UnauthorizedError"; + } +} + +/** Must match the route in App.tsx. */ +const LOGIN_PATH = "/login"; + +/** Trailing slashes trimmed: the router matches `/login/` to the same route, so + * comparing the raw pathname would send a caller who arrived that way through + * one more reload before the guard below started holding. */ +function onLoginPage() { + return window.location.pathname.replace(/\/+$/, "") === LOGIN_PATH; +} + async function request(path: string, opts: any = {}) { const controller = new AbortController(); const timeoutMs = path === "/api/auth/me" ? 30000 : 10000; @@ -13,8 +33,13 @@ async function request(path: string, opts: any = {}) { }); clearTimeout(timer); if (res.status === 401) { - window.location.href = "/login"; - throw new Error("Unauthorized"); + // Not when already on the login page. Assigning the same URL reloads it, + // the reload re-runs this request, and its 401 assigns it again; where + // nothing can mint a session that does not terminate. + if (!onLoginPage()) { + window.location.href = LOGIN_PATH; + } + throw new UnauthorizedError(); } if (!res.ok) throw new Error(`${res.status} ${res.statusText}`); return res.json(); diff --git a/dashboard/src/context/AuthContext.tsx b/dashboard/src/context/AuthContext.tsx index 7d556b45..c3065953 100644 --- a/dashboard/src/context/AuthContext.tsx +++ b/dashboard/src/context/AuthContext.tsx @@ -1,5 +1,5 @@ import { createContext, useContext, useState, useEffect } from "react"; -import { api } from "../api/client"; +import { api, UnauthorizedError } from "../api/client"; const AuthContext = createContext(null); @@ -17,8 +17,10 @@ export function AuthProvider({ children }) { if (!cancelled) { setUser(u); setLoading(false); } return; } catch (e) { - // client.js already redirects to /login on 401 — here we only - // land when there's a network/timeout error (server busy). + // A 401 is settled, so stop: the retries exist for a busy server, and + // repeating an unauthenticated call only holds the login card behind + // a loading state for the length of the backoff. + if (e instanceof UnauthorizedError) break; if (attempt < 3) { await new Promise((r) => setTimeout(r, 1500 * (attempt + 1))); } diff --git a/dashboard/src/test/authUnauthorized.test.tsx b/dashboard/src/test/authUnauthorized.test.tsx new file mode 100644 index 00000000..390ec641 --- /dev/null +++ b/dashboard/src/test/authUnauthorized.test.tsx @@ -0,0 +1,79 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, screen, waitFor } from "@testing-library/react"; +import { api, UnauthorizedError } from "../api/client"; +import { AuthProvider, useAuth } from "../context/AuthContext"; + +/** + * A 401 is an answer, not a failure to get one. + * + * The client used to navigate to /login on every 401. On /login that is a + * same-URL assignment, which reloads; the reload re-runs the auth call, which + * 401s again. Where nothing can mint a session the page never settles, and the + * login card only flashes between reloads. + */ + +const realLocation = window.location; + +function stubLocation(pathname: string) { + const loc = { pathname, href: `https://dash.retina.fm${pathname}` }; + Object.defineProperty(window, "location", { value: loc, writable: true, configurable: true }); + return loc; +} + +describe("the API client on a 401", () => { + beforeEach(() => { + vi.stubGlobal("fetch", vi.fn(async () => new Response(null, { status: 401 }))); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + Object.defineProperty(window, "location", { + value: realLocation, + writable: true, + configurable: true, + }); + }); + + it("sends a caller elsewhere in the app to the login page", async () => { + const loc = stubLocation("/nodes"); + await expect(api.me()).rejects.toBeInstanceOf(UnauthorizedError); + expect(loc.href).toBe("/login"); + }); + + it.each(["/login", "/login/"])( + "does not navigate when the caller is already on the login page (%s)", + async (pathname) => { + // Both spellings, because the router matches them to one route and only + // the raw string comparison would tell them apart. + const loc = stubLocation(pathname); + const untouched = loc.href; + await expect(api.me()).rejects.toBeInstanceOf(UnauthorizedError); + expect(loc.href).toBe(untouched); + } + ); +}); + +function Probe() { + const { user, loading } = useAuth(); + if (loading) return deciding; + return {user ? "signed in" : "signed out"}; +} + +describe("AuthProvider on a 401", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("settles as signed out rather than retrying", async () => { + // The retries exist for a busy server. Spending them on a 401 leaves the + // login card behind a loading state for ~9s, which reads as a hung page. + vi.spyOn(api, "me").mockRejectedValue(new UnauthorizedError()); + render( + + + + ); + await waitFor(() => expect(screen.getByText("signed out")).toBeInTheDocument()); + expect(api.me).toHaveBeenCalledTimes(1); + }); +});