diff --git a/frontend/components/__tests__/auth-provider.test.tsx b/frontend/components/__tests__/auth-provider.test.tsx index 9c780b3e..176b0b90 100644 --- a/frontend/components/__tests__/auth-provider.test.tsx +++ b/frontend/components/__tests__/auth-provider.test.tsx @@ -11,11 +11,13 @@ */ import React from "react"; +import { readdirSync } from "node:fs"; +import { join } from "node:path"; import { act, render, waitFor } from "@testing-library/react"; import { http, HttpResponse } from "msw"; import { describe, it, expect, beforeEach, vi } from "vitest"; import { server } from "../../test/msw/server"; -import { AuthProvider, useAuth } from "../auth-provider"; +import { AuthProvider, PUBLIC_ROUTES, isPublicRoute, useAuth } from "../auth-provider"; // ── Next.js navigation mocks ────────────────────────────────────────────────── const mockReplace = vi.fn(); @@ -262,3 +264,86 @@ describe("AuthProvider — silent refresh on page load", () => { expect(localStorage.getItem(TOKEN_KEY)).toBe(JSON.stringify(sameAcctToken)); }); }); + +/** + * The public-route allowlist, derived rather than enumerated. + * + * `/api-docs` shipped outside `app/(dashboard)/`, fetching without a token, and was still unreachable — + * absent from `PUBLIC_ROUTES`, the provider redirected it to `/login`. Nothing caught it: the page's own + * tests render the component directly, and an HTTP probe returns 200 because the redirect is client-side. + * + * So this walks `app/` for real `page` files instead of restating the list. Directory names are NOT URL + * segments (review): a route group `(public)` contributes nothing to the URL, so scanning top-level + * directory names would demand a nonexistent `/(public)` entry while never checking the `/help` a reader + * would actually visit. Routes are resolved the way the App Router resolves them, and then checked + * against the provider's own `isPublicRoute` rather than a reimplementation of its matching. + */ +describe("PUBLIC_ROUTES", () => { + /** A route group — `(dashboard)` — wraps pages without contributing a URL segment. */ + const isRouteGroup = (name: string) => name.startsWith("(") && name.endsWith(")"); + /** `[id]` / `[...slug]`. Matching is prefix-based, so the static ancestor is what must be listed. */ + const isDynamic = (name: string) => name.startsWith("["); + /** The one authenticated group. A route inside it needs a session by design. */ + const AUTHENTICATED_GROUP = "(dashboard)"; + + /** Every URL `app/` actually serves, paired with whether it sits inside the authenticated group. */ + function appRoutes(): Array<{ url: string; authenticated: boolean }> { + const found: Array<{ url: string; authenticated: boolean }> = []; + + const walk = (dir: string, segments: string[], authenticated: boolean) => { + const entries = readdirSync(dir, { withFileTypes: true }); + if (entries.some((e) => e.isFile() && /^page\.(tsx|ts|jsx|js)$/.test(e.name))) { + found.push({ url: `/${segments.join("/")}`.replace(/\/+$/, "") || "/", authenticated }); + } + for (const entry of entries) { + if (!entry.isDirectory()) continue; + if (isRouteGroup(entry.name)) { + // Contributes no URL segment — but `(dashboard)` marks everything beneath it as authenticated. + walk(join(dir, entry.name), segments, authenticated || entry.name === AUTHENTICATED_GROUP); + continue; + } + // A dynamic segment is covered by its static ancestor, since matching is prefix-based. + if (isDynamic(entry.name)) continue; + walk(join(dir, entry.name), [...segments, entry.name], authenticated); + } + }; + + walk(join(process.cwd(), "app"), [], false); + return found; + } + + it("covers every route app/ serves outside the authenticated group", () => { + const routes = appRoutes(); + // The walk is only meaningful if it resolved real URLs — including one inside a route group, which + // is the case that a directory-name scan gets wrong. + expect(routes.map((r) => r.url)).toEqual(expect.arrayContaining(["/", "/api-docs", "/sandbox", "/login"])); + expect(routes.filter((r) => r.authenticated).length).toBeGreaterThan(0); + expect(routes.some((r) => r.url.includes("("))).toBe(false); + + for (const { url, authenticated } of routes) { + if (authenticated || url === "/login") continue; + expect(isPublicRoute(url), `app serves ${url} outside ${AUTHENTICATED_GROUP} but it is not public`) + .toBe(true); + } + }); + + it("keeps the authenticated group gated", () => { + // The converse: the allowlist must not accidentally open the dashboard. `/` covering everything + // by prefix would satisfy the test above while unlocking the whole app. + for (const { url, authenticated } of appRoutes()) { + if (!authenticated) continue; + expect(isPublicRoute(url), `${url} is inside ${AUTHENTICATED_GROUP} but PUBLIC_ROUTES exempts it`) + .toBe(false); + } + }); + + it("does not redirect any route it lists", async () => { + for (const route of PUBLIC_ROUTES) { + mockReplace.mockClear(); + mockPathname.mockReturnValue(route); + renderProvider(); + await new Promise((r) => setTimeout(r, 20)); + expect(mockReplace, `${route} is listed as public but redirected`).not.toHaveBeenCalled(); + } + }); +}); diff --git a/frontend/components/auth-provider.tsx b/frontend/components/auth-provider.tsx index b355037e..2fd6e133 100644 --- a/frontend/components/auth-provider.tsx +++ b/frontend/components/auth-provider.tsx @@ -6,7 +6,30 @@ import { usePathname, useRouter } from "next/navigation"; const TOKEN_KEY = "ww_token"; const USER_KEY = "ww_user"; const API_BASE = (process.env.NEXT_PUBLIC_API_BASE_URL ?? "").trim().replace(/\/+$/, ""); -const PUBLIC_ROUTES = ["/", "/sandbox"]; +/** + * Routes that render without a session, and therefore must not trigger a silent refresh or a redirect + * to `/login`. + * + * This list is the authority for the whole app, and it is easy to forget: a page can be correct in every + * other respect — outside `app/(dashboard)/`, fetching without a bearer token — and still be unreachable + * because it is absent here. `/api-docs` shipped that way (ADR-068), and an HTTP probe could not see it, + * since the redirect is client-side and the server still returns 200. `auth-provider.test.tsx` walks + * `app/` for the URLs it really serves, so a new public route cannot be added without landing here — and + * asserts the converse too, that nothing here exempts the authenticated group. + */ +export const PUBLIC_ROUTES = ["/", "/sandbox", "/api-docs"]; + +/** + * Whether `pathname` renders without a session. An entry covers itself and everything nested under it, + * so a public section needs one entry rather than one per page. + * + * Exported because `auth-provider.test.tsx` checks the real routes of `app/` against this predicate. A + * test that reimplemented the matching would agree with itself rather than with the provider. + */ +export function isPublicRoute(pathname: string | null | undefined): boolean { + if (!pathname) return false; + return PUBLIC_ROUTES.some((route) => pathname === route || pathname.startsWith(`${route}/`)); +} type AuthUser = { email: string; @@ -126,9 +149,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { }, [token]); useEffect(() => { - if (pathname && PUBLIC_ROUTES.some((route) => pathname === route || pathname.startsWith(`${route}/`))) { - return; - } + if (isPublicRoute(pathname)) return; if (pathname?.startsWith("/login")) return; if (token) return;