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); + }); +});