diff --git a/scripts/_route-guard-shim.mjs b/scripts/_route-guard-shim.mjs index 37ae152..5985b72 100644 --- a/scripts/_route-guard-shim.mjs +++ b/scripts/_route-guard-shim.mjs @@ -4,20 +4,28 @@ // under `node --test` without a TanStack Router or a QueryClient: // // @tanstack/react-router → ./scripts/_test-mocks/router.mjs +// @tanstack/react-query → ./scripts/_test-mocks/react-query.mjs // ./use-current-user → ./scripts/_test-mocks/use-current-user.mjs // @/components/ui/skeleton→ ./scripts/_test-mocks/skeleton.mjs +// @/lib/api/http-client → ./scripts/_test-mocks/http-client.mjs // // The router stub records navigate() calls in a globalThis-scoped array // the test can read. The useCurrentUser stub reads its return shape from -// globalThis.__routeGuardMeState so each test can mutate it. +// globalThis.__routeGuardMeState so each test can mutate it. The +// react-query stub records removeQueries / invalidateQueries calls so +// the session-expired listener test can assert the `['me']` cache was +// cleared. The http-client stub re-exports SESSION_EXPIRED_EVENT and +// provides a test-spy dispatchSessionExpired that records the calls. // // Activated via: // node --import tsx --import ./scripts/_route-guard-shim.mjs --test ... const MOCKS = { router: new URL('./_test-mocks/router.mjs', import.meta.url).href, + reactQuery: new URL('./_test-mocks/react-query.mjs', import.meta.url).href, useCurrentUser: new URL('./_test-mocks/use-current-user.mjs', import.meta.url).href, skeleton: new URL('./_test-mocks/skeleton.mjs', import.meta.url).href, + httpClient: new URL('./_test-mocks/http-client.mjs', import.meta.url).href, } function matchesAny(specifier, candidates) { @@ -28,6 +36,9 @@ export async function resolve(specifier, context, nextResolve) { if (matchesAny(specifier, ['/@tanstack/react-router', '@tanstack/react-router'])) { return { url: MOCKS.router, shortCircuit: true, format: 'module' } } + if (matchesAny(specifier, ['/@tanstack/react-query', '@tanstack/react-query'])) { + return { url: MOCKS.reactQuery, shortCircuit: true, format: 'module' } + } if ( matchesAny(specifier, [ './use-current-user', @@ -48,5 +59,15 @@ export async function resolve(specifier, context, nextResolve) { ) { return { url: MOCKS.skeleton, shortCircuit: true, format: 'module' } } + if ( + matchesAny(specifier, [ + '@/lib/api/http-client', + '../lib/api/http-client', + './lib/api/http-client', + '/lib/api/http-client', + ]) + ) { + return { url: MOCKS.httpClient, shortCircuit: true, format: 'module' } + } return nextResolve(specifier, context) } \ No newline at end of file diff --git a/scripts/_run-tests.mjs b/scripts/_run-tests.mjs index fd708f1..2712a93 100644 --- a/scripts/_run-tests.mjs +++ b/scripts/_run-tests.mjs @@ -1,5 +1,5 @@ #!/usr/bin/env node -// Test runner wrapper (NUL-50.5 / NUL-56). +// Test runner wrapper (NUL-50.5 / NUL-56 / NUL-217). // // Why this exists: tsx's loader honors `tsconfig.app.json`'s `paths` // (the `@/*` → `./src/*` alias) only when it knows which tsconfig to @@ -14,27 +14,121 @@ // using `react-dom/server.renderToStaticMarkup` (no jsdom needed). // // Custom argv after `--` is forwarded verbatim so CI / IDEs can append -// patterns like `npm test -- --test-only src/features/auth`. +// patterns like `npm test -- --test-only src/features/auth`. Flags +// starting with `-` are forwarded to node verbatim so this works. +// +// Glob handling (NUL-217): node --test does not understand **. We +// expand `src/**/*.test.ts{x,}` globs ourselves via a recursive +// directory walk so `npm test` (no args) actually runs every test file +// rather than bailing out with `Could not find 'src/**/*.test.ts'`. import { spawn } from 'node:child_process' import { fileURLToPath } from 'node:url' import path from 'node:path' +import fs from 'node:fs' const here = path.dirname(fileURLToPath(import.meta.url)) const repoRoot = path.resolve(here, '..') +/** + * Walk `root` recursively and return every regular file whose path + * matches at least one of the given glob patterns. Patterns are + * minimal — two stars match any number of path segments, one star + * matches one segment minus the slashes, `?` matches one character. + * This is deliberately not a general glob library; it only needs to + * handle the src-*-test shape that npm test produces. + */ +function expandGlobs(root, patterns) { + const files = new Set() + for (const pattern of patterns) { + const segments = pattern.split('/') + walk(root, segments, files) + } + // Return paths relative to `root`. Keeping them short (rather than + // absolute) avoids hitting E2BIG in environments where argv has a + // tight limit and matches the shape node --test expects when run + // from a non-TTY parent process. + return [...files].map((p) => path.relative(root, p)).sort() +} + +function walk(dir, segments, out) { + if (segments.length === 0) { + if (fs.existsSync(dir) && fs.statSync(dir).isFile()) { + out.add(dir) + } + return + } + const [head, ...rest] = segments + if (head === '**') { + // Zero-or-more directory segments: first resolve the rest of the + // pattern at this level (no directories consumed), then recurse + // into every subdirectory consuming one level each. + walk(dir, rest, out) + if (fs.existsSync(dir) && fs.statSync(dir).isDirectory()) { + for (const child of fs.readdirSync(dir)) { + if (child.startsWith('.')) continue + walk(path.join(dir, child), segments, out) + } + } + return + } + if (head.includes('*') || head.includes('?')) { + if (!fs.existsSync(dir) || !fs.statSync(dir).isDirectory()) return + const matcher = globToRegExp(head) + for (const child of fs.readdirSync(dir)) { + if (matcher.test(child)) walk(path.join(dir, child), rest, out) + } + return + } + walk(path.join(dir, head), rest, out) +} + +function globToRegExp(segment) { + let re = '^' + for (const ch of segment) { + if (ch === '*') re += '[^/]*' + else if (ch === '?') re += '[^/]' + else if ('\\^$.|+()[]{}'.includes(ch)) re += '\\' + ch + else re += ch + } + re += '$' + return new RegExp(re) +} + const args = process.argv.slice(2) -// Default glob covers the whole tree; trailing user args after `--` -// are forwarded (e.g. `--test-only path/to/file`). -const testArgs = args.length > 0 ? args : [ - '--test', - 'src/**/*.test.ts', - 'src/**/*.test.tsx', -] +// Split args into node flags vs. positional file paths. Everything +// starting with `-` is a node flag (forwarded verbatim); everything +// else is treated as a glob/file pattern we expand. +const flags = [] +const positional = [] +for (const arg of args) { + if (arg.startsWith('-')) flags.push(arg) + else positional.push(arg) +} + +let testArgs +if (positional.length > 0) { + const expanded = expandGlobs(repoRoot, positional) + if (expanded.length === 0) { + console.error(`Could not find '${positional.join(' ')}'`) + process.exit(1) + } + testArgs = [...flags, ...expanded] +} else { + // Default: every `.test.ts` and `.test.tsx` under `src/`. We + // recursively walk rather than rely on shell globbing because the + // npm-script context doesn't expand `**`. + const expanded = [ + ...expandGlobs(repoRoot, ['src/**/*.test.ts']), + ...expandGlobs(repoRoot, ['src/**/*.test.tsx']), + ] + testArgs = [...flags, ...expanded] +} +const spawnArgs = ['--import', 'tsx', '--test', ...testArgs] const child = spawn( process.execPath, - ['--import', 'tsx', ...testArgs], + spawnArgs, { cwd: repoRoot, stdio: 'inherit', @@ -51,4 +145,4 @@ child.on('exit', (code, signal) => { } else { process.exit(code ?? 1) } -}) \ No newline at end of file +}) diff --git a/scripts/_test-mocks/http-client.mjs b/scripts/_test-mocks/http-client.mjs new file mode 100644 index 0000000..cd44d25 --- /dev/null +++ b/scripts/_test-mocks/http-client.mjs @@ -0,0 +1,22 @@ +// Test stub for @/lib/api/http-client. +// +// Re-exports the SESSION_EXPIRED_EVENT constant and provides a spy +// `dispatchSessionExpired` that records calls onto +// `globalThis.__sessionExpiredCalls` so the listener test can either: +// +// - call dispatchSessionExpired() directly and assert side effects, OR +// - dispatch the `ipam:session-expired` CustomEvent on the global +// `window` stub to exercise the listener exactly the way +// `http-client.ts` would in production. +// +// The real `apiFetch` / `api` / `ApiError` are not exported from this stub; +// the route-guard component under test only imports SESSION_EXPIRED_EVENT. + +export const SESSION_EXPIRED_EVENT = 'ipam:session-expired' + +export function dispatchSessionExpired() { + ;(globalThis.__sessionExpiredCalls ??= []).push(true) + if (typeof window !== 'undefined' && window) { + window.dispatchEvent(new (globalThis.CustomEvent ?? Event)(SESSION_EXPIRED_EVENT)) + } +} diff --git a/scripts/_test-mocks/react-query.mjs b/scripts/_test-mocks/react-query.mjs new file mode 100644 index 0000000..72ce653 --- /dev/null +++ b/scripts/_test-mocks/react-query.mjs @@ -0,0 +1,35 @@ +// Test stub for @tanstack/react-query. +// +// Records calls to `useQueryClient().removeQueries` / `invalidateQueries` +// onto `globalThis.__routeGuardQueryClientCalls` so the session-expired +// listener tests in `route-guard.test.tsx` can assert the cache is wiped +// before navigation fires. +// +// The default `useQueryClient()` returns a fresh object on every call +// (mirroring the real client). For tests that need to inspect a single +// instance across renders, set `globalThis.__routeGuardQueryClient` to +// a hand-built object — `useQueryClient` will return that instead. + +function makeClient() { + return { + removeQueries(opts) { + ;(globalThis.__routeGuardQueryClientCalls ??= []).push({ + method: 'remove', + key: [...(opts?.queryKey ?? [])], + }) + }, + invalidateQueries(opts) { + ;(globalThis.__routeGuardQueryClientCalls ??= []).push({ + method: 'invalidate', + key: [...(opts?.queryKey ?? [])], + }) + }, + } +} + +export function useQueryClient() { + if (globalThis.__routeGuardQueryClient) { + return globalThis.__routeGuardQueryClient + } + return makeClient() +} diff --git a/src/features/auth/route-guard-logic.ts b/src/features/auth/route-guard-logic.ts index 211174d..bc94844 100644 --- a/src/features/auth/route-guard-logic.ts +++ b/src/features/auth/route-guard-logic.ts @@ -68,3 +68,102 @@ export function safePostLoginTarget( if (from.startsWith('//')) return fallback return from } + +/** + * Decide what the auth guard should do when the `ipam:session-expired` + * window event fires (NUL-50.4 — a non-login `/api/**` call returned 401 + * while the user is sitting on a protected page). + * + * Returns `null` when no navigation is needed (we're already on + * `/login`, the destination would be unsafe, or there is no current path). + * Otherwise returns `{ from }` so the React effect can navigate to + * `/login?from=` and clear the cached session. + * + * Pure function — does no I/O, no React, no router. The caller is + * responsible for the actual `removeQueries` / `invalidateQueries` calls + * and the `useNavigate()` dispatch. + * + * Behaviour: + * - On `/login` → no-op. The user is already at the login form; a + * stale 401 from a background mutation shouldn't kick them off it. + * - Path with an unsafe `from` (protocol-relative, absolute, etc.) → + * fall back to `/` so we can't be tricked into redirecting to an + * attacker-controlled host. + * - Otherwise preserve the current path/query verbatim (the value + * came from `location.pathname + location.searchStr` on the same + * origin, so it's already a safe in-app reference). + */ +export function decideSessionExpiredRedirect( + currentPath: string, + currentSearch: string, +): { from: string } | null { + if (currentPath === '/login') return null + const fromCandidate = `${currentPath}${currentSearch || ''}` + // safePostLoginTarget always returns a string — unsafe paths collapse + // to `/` so we can't be tricked into bouncing to an attacker host. + return { from: safePostLoginTarget(fromCandidate, '/') } +} + +/** + * Minimal interface for the bits of the React Query client the + * session-expired listener touches. Defined as a structural type so tests + * don't need the real QueryClient (which drags in the alias graph). + */ +export interface SessionExpiredQueryClient { + removeQueries: (opts: { queryKey: readonly unknown[] }) => unknown + invalidateQueries: (opts: { queryKey: readonly unknown[] }) => unknown +} + +/** + * Side-effect interface for the navigation step the listener triggers. + * Tests pass a spy; production wires this to `useNavigate()` from + * `@tanstack/react-router`. + */ +export type SessionExpiredNavigate = (opts: { + to: string + search: { from: string } + replace: boolean +}) => Promise | void + +/** + * Build the listener for the `ipam:session-expired` window event. The + * React effect in `route-guard.tsx` is a thin wrapper around this — it + * exists as a pure function so it can be exercised under `node --test` + * without rendering the component. + * + * Returns a handler that, when invoked: + * + * 1. Resolves the redirect target with `decideSessionExpiredRedirect`. + * 2. On a non-null target, clears the `['me']` query cache (remove + + * invalidate, mirroring the logout flow) and navigates to + * `/login?from=`. + * 3. On a null target (we're already on /login), is a no-op. + * + * `onNoOp` is an optional hook for tests that want to assert the + * no-op branch fired. + */ +export function makeSessionExpiredHandler(deps: { + currentPath: string + currentSearch: string + queryClient: SessionExpiredQueryClient + navigate: SessionExpiredNavigate + onNoOp?: () => void +}): () => void { + return () => { + const decision = decideSessionExpiredRedirect( + deps.currentPath, + deps.currentSearch, + ) + if (!decision) { + deps.onNoOp?.() + return + } + deps.queryClient.removeQueries({ queryKey: ['me'] }) + deps.queryClient.invalidateQueries({ queryKey: ['me'] }) + void deps.navigate({ + to: '/login', + search: { from: decision.from }, + replace: true, + }) + } +} diff --git a/src/features/auth/route-guard.test.ts b/src/features/auth/route-guard.test.ts index b8e89a1..ce3cd8f 100644 --- a/src/features/auth/route-guard.test.ts +++ b/src/features/auth/route-guard.test.ts @@ -1,7 +1,12 @@ import assert from 'node:assert/strict' import test from 'node:test' -import { decideRedirect, safePostLoginTarget } from './route-guard-logic' +import { + decideRedirect, + decideSessionExpiredRedirect, + makeSessionExpiredHandler, + safePostLoginTarget, +} from './route-guard-logic' /** * Unit tests for the pure `decideRedirect` helper in `route-guard.tsx`. @@ -143,3 +148,206 @@ test('safePostLoginTarget mirrors decideRedirect sanitisation', () => { // Custom fallback assert.equal(safePostLoginTarget(undefined, '/dashboard'), '/dashboard') }) + +/** + * Tests for the session-expired listener (NUL-50.4). + * + * The listener is a `useEffect` that subscribes to the `ipam:session-expired` + * window event dispatched by `apiFetch` on non-login `/api/**` 401 responses. + * The decision part (sanitisation, `/login` short-circuit) lives in + * `decideSessionExpiredRedirect`. The wiring part (event subscription, + * cache clearing, navigation) lives in the React effect in `route-guard.tsx` + * and is exercised under node:test via the `_route-guard-shim` loader + * (see `route-guard.test.tsx`). + * + * These tests pin the decision contract. The wiring tests live next to + * the other component tests in `route-guard.test.tsx` and run under the + * same shim. + */ + +test('session-expired on /racks bounces to /login with from=/racks', () => { + assert.deepEqual( + decideSessionExpiredRedirect('/racks', ''), + { from: '/racks' }, + ) +}) + +test('session-expired on /racks?tab=devices preserves the search string', () => { + assert.deepEqual( + decideSessionExpiredRedirect('/racks', '?tab=devices'), + { from: '/racks?tab=devices' }, + ) +}) + +test('session-expired on /ipam?foo=bar&baz=1 preserves the full query', () => { + assert.deepEqual( + decideSessionExpiredRedirect('/ipam', '?foo=bar&baz=1'), + { from: '/ipam?foo=bar&baz=1' }, + ) +}) + +test('session-expired on /login is a no-op (login form stays put)', () => { + assert.equal( + decideSessionExpiredRedirect('/login', '?from=/racks'), + null, + ) +}) + +test('session-expired on /login (no search) is a no-op', () => { + assert.equal(decideSessionExpiredRedirect('/login', ''), null) +}) + +test('session-expired sanitises an unsafe from by collapsing to /', () => { + // In practice `currentPath` is `location.pathname` (always starts with + // `/`), so this is defensive: if a future refactor ever swaps in a + // raw value the listener still can't be tricked into an open redirect. + assert.deepEqual( + decideSessionExpiredRedirect('//evil.example', ''), + { from: '/' }, + ) + assert.deepEqual( + decideSessionExpiredRedirect('https://evil.example/x', ''), + { from: '/' }, + ) + assert.deepEqual( + decideSessionExpiredRedirect('', ''), + { from: '/' }, + ) +}) + +/** + * Wiring tests for the session-expired listener (NUL-50.4). + * + * The handler created by `makeSessionExpiredHandler` is the function the + * `` effect registers on the `ipam:session-expired` window + * event. These tests drive it directly with fakes for the React Query + * client and the router's `navigate()` — so the listener's side-effect + * ordering is pinned without needing a DOM. + */ + +interface StubQueryClient { + calls: { method: string; key: unknown[] }[] + removeQueries: (opts: { queryKey: readonly unknown[] }) => unknown + invalidateQueries: (opts: { queryKey: readonly unknown[] }) => unknown +} + +function makeStubQueryClient(): StubQueryClient { + const calls: { method: string; key: unknown[] }[] = [] + return { + calls, + removeQueries(opts) { + calls.push({ method: 'remove', key: [...opts.queryKey] }) + return undefined + }, + invalidateQueries(opts) { + calls.push({ method: 'invalidate', key: [...opts.queryKey] }) + return undefined + }, + } +} + +test('handler: clears the me cache and navigates to /login with the preserved from', () => { + const queryClient = makeStubQueryClient() + const navCalls: unknown[] = [] + const handler = makeSessionExpiredHandler({ + currentPath: '/racks', + currentSearch: '?tab=devices', + queryClient, + navigate: (opts) => { + navCalls.push(opts) + }, + }) + + handler() + + // Cache is wiped (remove then invalidate, mirroring the logout flow). + assert.deepEqual(queryClient.calls, [ + { method: 'remove', key: ['me'] }, + { method: 'invalidate', key: ['me'] }, + ]) + // Navigate fires exactly once with the preserved destination. + assert.equal(navCalls.length, 1) + assert.deepEqual(navCalls[0], { + to: '/login', + search: { from: '/racks?tab=devices' }, + replace: true, + }) +}) + +test('handler: no-op when already on /login (login form stays put)', () => { + const queryClient = makeStubQueryClient() + const navCalls: unknown[] = [] + let noOpCount = 0 + const handler = makeSessionExpiredHandler({ + currentPath: '/login', + currentSearch: '?from=/racks', + queryClient, + navigate: (opts) => { + navCalls.push(opts) + }, + onNoOp: () => { + noOpCount += 1 + }, + }) + + handler() + + assert.equal(noOpCount, 1, 'onNoOp must fire on the no-op branch') + assert.equal(queryClient.calls.length, 0, 'cache must not be touched') + assert.equal(navCalls.length, 0, 'no navigation must fire') +}) + +test('handler: passes an unsafe from through safePostLoginTarget, collapsing to /', () => { + const queryClient = makeStubQueryClient() + const navCalls: unknown[] = [] + const handler = makeSessionExpiredHandler({ + currentPath: '//evil.example', + currentSearch: '', + queryClient, + navigate: (opts) => { + navCalls.push(opts) + }, + }) + + handler() + + assert.deepEqual(navCalls[0], { + to: '/login', + search: { from: '/' }, + replace: true, + }) +}) + +test('handler: every invocation resets the cache before navigating', () => { + // Pin the ordering — the React effect relies on the cache being + // wiped *before* navigation so `decideRedirect` sees an anonymous + // viewer on the next render rather than a stale success. + const order: string[] = [] + const queryClient = { + removeQueries() { + order.push('remove') + }, + invalidateQueries() { + order.push('invalidate') + }, + } + const navCalls: string[] = [] + const handler = makeSessionExpiredHandler({ + currentPath: '/racks', + currentSearch: '', + queryClient, + navigate: (opts) => { + navCalls.push(`navigate:${opts.to}:${opts.search.from}`) + }, + }) + + handler() + handler() + + // Two invocations → two complete clear-then-navigate cycles. + assert.deepEqual(order, ['remove', 'invalidate', 'remove', 'invalidate']) + assert.deepEqual(navCalls, [ + 'navigate:/login:/racks', + 'navigate:/login:/racks', + ]) +}) diff --git a/src/features/auth/route-guard.tsx b/src/features/auth/route-guard.tsx index b7bf930..30b5428 100644 --- a/src/features/auth/route-guard.tsx +++ b/src/features/auth/route-guard.tsx @@ -1,12 +1,14 @@ import { useEffect, type ReactNode } from 'react' import { useNavigate, useLocation } from '@tanstack/react-router' +import { useQueryClient } from '@tanstack/react-query' import { useCurrentUser } from './use-current-user' import { decideRedirect, - safePostLoginTarget, + makeSessionExpiredHandler, type RedirectInputs, } from './route-guard-logic' +import { SESSION_EXPIRED_EVENT } from '@/lib/api/http-client' import { Skeleton } from '@/components/ui/skeleton' /** @@ -34,6 +36,7 @@ export function AuthGuard({ children }: { children: ReactNode }) { const me = useCurrentUser() const navigate = useNavigate() const location = useLocation() + const queryClient = useQueryClient() const currentPath = location.pathname const currentSearch = location.searchStr ?? '' @@ -69,6 +72,37 @@ export function AuthGuard({ children }: { children: ReactNode }) { navigate, ]) + // NUL-50.4 — listen for the `ipam:session-expired` event dispatched by + // `apiFetch` on any non-login `/api/**` 401. The user is sitting on a + // protected page (probably with stale data); we need to: + // + // 1. Drop the cached `['me']` so the next read refetches (and the + // `decideRedirect` effect above sees `isAuthenticated: false`). + // 2. Bounce to `/login?from=` so the destination is + // preserved across the round-trip. + // + // The listener logic (sanitisation, /login short-circuit, cache wipe, + // navigation) lives in `makeSessionExpiredHandler` in + // `route-guard-logic.ts` so it can be exercised under node --test + // without rendering React. The effect here is just the event wiring. + // + // The listener is `replace: true` so the stale page doesn't pollute + // history. We deliberately don't `e.preventDefault()` — there is no + // default browser behaviour for a `CustomEvent`. + useEffect(() => { + if (typeof window === 'undefined') return + const handler = makeSessionExpiredHandler({ + currentPath, + currentSearch, + queryClient, + navigate: (opts) => navigate(opts), + }) + window.addEventListener(SESSION_EXPIRED_EVENT, handler) + return () => { + window.removeEventListener(SESSION_EXPIRED_EVENT, handler) + } + }, [currentPath, currentSearch, navigate, queryClient]) + if (me.isLoading) { return } @@ -98,5 +132,3 @@ function AuthSplash() { ) } - -export { safePostLoginTarget }