diff --git a/docs/superpowers/plans/2026-06-08-session-expiry-ux.md b/docs/superpowers/plans/2026-06-08-session-expiry-ux.md new file mode 100644 index 0000000..0a3565b --- /dev/null +++ b/docs/superpowers/plans/2026-06-08-session-expiry-ux.md @@ -0,0 +1,448 @@ +# Session-Expiry Soft Redirect + Auth Feedback — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the full-page-reload 401 redirect with a router soft-redirect that preserves SPA state, carries a "session expired" reason + return-to path, and add the missing login/logout feedback. + +**Architecture:** A non-React navigate-bridge module (`auth-redirect.ts`) holds a settable `navigate` ref; a one-line `NavigationBridge` component (mounted at the router root) registers React Router's navigate into it. The openapi-fetch 401 middleware calls `redirectToLogin()`, which soft-navigates to `/login?reason=expired&from=`. The login page reads `reason`/`from` (validated against open redirects), `RequireAuth` captures the attempted path, and the user menu shows a logout-pending state. + +**Tech Stack:** React 19 + TS + pnpm, React Router 7 (data router), TanStack Query v5, react-i18next, Base UI menu, Vitest 4 (jsdom project) + RTL + MSW. Test runner: `pnpm test` (single pass). + +**Conventions:** pnpm; **no `any`/`eslint-disable`/`@ts-ignore`**; no codename; en/sv parity; app source double-quote+semicolon; token classes only; `ui/` files use no-semicolon style (do not touch `ui/menu.tsx`). Run a single test pass. + +**Spec:** `docs/superpowers/specs/2026-06-08-session-expiry-ux-design.md` + +**Key facts:** +- `web/src/api/auth-redirect.ts` currently: `redirectToLogin()` → `window.location.assign("/login")` (guarded by `pathname !== "/login"`). Called from `web/src/api/client.ts` middleware on `response.status === 401`. Do **not** change `client.ts`. +- `web/src/app.tsx` builds a data router via `createBrowserRouter(createRoutesFromElements(<>…))`. Top-level children: ``, `}>…`, ``. Imports `Navigate`, `Route`, `Outlet` is **not** yet imported (add it). +- `web/src/test/render.tsx` `renderApp(ui, {route})` mounts `ui` at `path:"*"` in a `createMemoryRouter` — tests pass their own `` tree. jsdom project url is `http://localhost`. +- `vi.stubGlobal("location", {...})` is the established way to stub `window.location` here (see `theme-switch.test.tsx` stubbing `matchMedia`); restore with `vi.unstubAllGlobals()`. +- `login-page.tsx`: `useLogin()` mutation; success currently `navigate("/objects", {replace:true})`; submit `disabled={login.isPending}`; existing error alert uses `role="alert"`. Existing tests: `login-page.test.tsx` (`tree()` with `/login` + `/objects` routes). +- `require-auth.tsx`: `useMe()`; `isLoading` → ``; `!user` → ``. Test: `require-auth.test.tsx`. +- `user-menu.tsx`: `useLogout()`, `onSignOut` navigates to `/login` on success; `{t("auth.signOut")}`; returns `null` when `!me`. Base UI `MenuItem` supports `closeOnClick` + `disabled`. Test: `user-menu.test.tsx`. +- i18n `auth` block keys: `email/password/signIn/signOut/invalid/networkError` in both `en.json` and `sv.json`. + +--- + +# Task 1: Navigate bridge + soft redirect + +**Files:** +- Modify: `web/src/api/auth-redirect.ts` +- Create: `web/src/api/auth-redirect.test.ts` +- Create: `web/src/shell/navigation-bridge.tsx` +- Modify: `web/src/app.tsx` + +- [ ] **Step 1: Rewrite `web/src/api/auth-redirect.ts`:** +```ts +type NavigateFn = (to: string, opts?: { replace?: boolean }) => void; + +let navigateFn: NavigateFn | null = null; + +/** Register (or clear) the router's navigate fn. Called by NavigationBridge. */ +export function setNavigate(fn: NavigateFn | null): void { + navigateFn = fn; +} + +/** Soft-redirect to login on a 401, preserving SPA state and the return path. + * Falls back to a hard navigation when no router navigate is registered yet + * (e.g. a 401 during the very first load). No-op when already on /login. */ +export function redirectToLogin(): void { + const { pathname, search } = window.location; + if (pathname === "/login") return; + const from = encodeURIComponent(pathname + search); + const target = `/login?reason=expired&from=${from}`; + if (navigateFn) { + navigateFn(target, { replace: true }); + } else { + window.location.assign(target); + } +} +``` + +- [ ] **Step 2: Write the failing test `web/src/api/auth-redirect.test.ts`:** +```ts +import { afterEach, expect, test, vi } from "vitest"; + +import { redirectToLogin, setNavigate } from "./auth-redirect"; + +function stubLocation(pathname: string, search = "") { + const assign = vi.fn(); + vi.stubGlobal("location", { pathname, search, assign }); + return assign; +} + +afterEach(() => { + setNavigate(null); + vi.unstubAllGlobals(); +}); + +test("uses the registered navigate to soft-redirect with reason + from", () => { + const assign = stubLocation("/objects/abc", "?x=1"); + const navigate = vi.fn(); + setNavigate(navigate); + + redirectToLogin(); + + expect(navigate).toHaveBeenCalledWith( + "/login?reason=expired&from=%2Fobjects%2Fabc%3Fx%3D1", + { replace: true }, + ); + expect(assign).not.toHaveBeenCalled(); +}); + +test("falls back to a hard navigation when no navigate is registered", () => { + const assign = stubLocation("/objects/abc"); + + redirectToLogin(); + + expect(assign).toHaveBeenCalledWith("/login?reason=expired&from=%2Fobjects%2Fabc"); +}); + +test("does nothing when already on /login", () => { + stubLocation("/login"); + const navigate = vi.fn(); + setNavigate(navigate); + + redirectToLogin(); + + expect(navigate).not.toHaveBeenCalled(); +}); +``` + +- [ ] **Step 3: Run the test — expect PASS** (the implementation in Step 1 already satisfies it): +```bash +cd web && pnpm vitest run src/api/auth-redirect.test.ts +``` +Expected: 3 passing. (If you wrote the test before the impl, it would fail on the missing `setNavigate` export — either order is fine; end state is green.) + +- [ ] **Step 4: Create `web/src/shell/navigation-bridge.tsx`:** +```tsx +import { useEffect } from "react"; +import { useNavigate } from "react-router-dom"; + +import { setNavigate } from "../api/auth-redirect"; + +/** Bridges React Router's navigate to the non-React 401 handler. Renders nothing. */ +export function NavigationBridge() { + const navigate = useNavigate(); + + useEffect(() => { + setNavigate(navigate); + return () => setNavigate(null); + }, [navigate]); + + return null; +} +``` + +- [ ] **Step 5: Wrap the routes in `web/src/app.tsx`** so the bridge is always mounted. Add `Outlet` to the `react-router-dom` import and import the bridge: +```tsx +import { createBrowserRouter, createRoutesFromElements, Navigate, Outlet, Route, RouterProvider } from "react-router-dom"; +``` +```tsx +import { NavigationBridge } from "./shell/navigation-bridge"; +``` +Add this component above `const router = …`: +```tsx +function RootLayout() { + return ( + <> + + + + ); +} +``` +Wrap the existing top-level fragment children in a pathless `}>` — i.e. change `createRoutesFromElements(<> … )` so the outer element is `}> … ` containing the existing `/login`, `RequireAuth`, and `*` routes unchanged: +```tsx +const router = createBrowserRouter( + createRoutesFromElements( + }> + } /> + }> + {/* …AppShell subtree exactly as before… */} + + } /> + , + ), +); +``` + +- [ ] **Step 6: Verify build + lint + existing app test (vitest ONCE for the touched files):** +```bash +cd web && pnpm vitest run src/api/auth-redirect.test.ts src/app.test.tsx && pnpm typecheck && pnpm lint +``` +Expected: PASS. `app.test.tsx` must stay green (the pathless wrapper is transparent — same rendered routes). + +- [ ] **Step 7: Commit** +```bash +git add web/src/api/auth-redirect.ts web/src/api/auth-redirect.test.ts web/src/shell/navigation-bridge.tsx web/src/app.tsx +git commit -m "feat(web): soft-redirect to login on 401 via a navigate bridge (#48)" +``` + +--- + +# Task 2: Login page — reason banner, return-to, empty-field guard + +**Files:** +- Modify: `web/src/auth/login-page.tsx` +- Modify: `web/src/auth/login-page.test.tsx` +- Modify: `web/src/i18n/en.json`, `web/src/i18n/sv.json` + +- [ ] **Step 1: Add i18n keys** (both locales, parity). In `web/src/i18n/en.json` `auth` block add: +```json + "sessionExpired": "Your session expired — please sign in again.", + "signingOut": "Signing out…" +``` +In `web/src/i18n/sv.json` `auth` block add: +```json + "sessionExpired": "Din session har gått ut — logga in igen.", + "signingOut": "Loggar ut…" +``` +(Place them after the existing `networkError` entry; mind trailing commas. `signingOut` is consumed in Task 3 — add both now so the parity test stays green.) + +- [ ] **Step 2: Update `web/src/auth/login-page.tsx`.** Add `useSearchParams` to the import and a `safeFrom` helper; show the reason banner; route to `safeFrom` on success; guard the submit. Full changes: + +Import line: +```tsx +import { useNavigate, useSearchParams } from "react-router-dom"; +``` +Add a module-level helper (above `export function LoginPage`): +```tsx +/** Accept only a single-leading-slash local path; reject protocol-relative + * ("//host") and absolute URLs to avoid an open redirect. */ +function safeFrom(raw: string | null): string { + if (!raw) return "/objects"; + return /^\/(?!\/)/.test(raw) ? raw : "/objects"; +} +``` +Inside the component, after `const navigate = useNavigate();`: +```tsx + const [params] = useSearchParams(); + const sessionExpired = params.get("reason") === "expired"; +``` +Change the success navigation: +```tsx + { onSuccess: () => navigate(safeFrom(params.get("from")), { replace: true }) }, +``` +Add the banner just inside the `
`, above the email field (after the `

`): +```tsx + {sessionExpired && ( +

{t("auth.sessionExpired")}

+ )} +``` +Change the submit button disabled condition: +```tsx + diff --git a/web/src/auth/require-auth.test.tsx b/web/src/auth/require-auth.test.tsx index 643e9cc..d0faa1a 100644 --- a/web/src/auth/require-auth.test.tsx +++ b/web/src/auth/require-auth.test.tsx @@ -1,16 +1,21 @@ import { screen, waitFor } from "@testing-library/react"; import { http, HttpResponse } from "msw"; import { expect, test } from "vitest"; -import { Route, Routes } from "react-router-dom"; +import { Route, Routes, useLocation } from "react-router-dom"; import { server } from "../test/server"; import { renderApp } from "../test/render"; import { RequireAuth } from "./require-auth"; +function LoginStub() { + const location = useLocation(); + return
login page {location.search}
; +} + function tree() { return ( - login page} /> + } /> }> secret objects} /> @@ -23,8 +28,8 @@ test("renders children when authenticated", async () => { expect(await screen.findByText("secret objects")).toBeInTheDocument(); }); -test("redirects to /login when unauthenticated", async () => { +test("redirects unauthenticated users to /login carrying the attempted path", async () => { server.use(http.get("/api/admin/me", () => new HttpResponse(null, { status: 401 }))); renderApp(tree(), { route: "/objects" }); - await waitFor(() => expect(screen.getByText("login page")).toBeInTheDocument()); + await waitFor(() => expect(screen.getByText(/from=%2Fobjects/)).toBeInTheDocument()); }); diff --git a/web/src/auth/require-auth.tsx b/web/src/auth/require-auth.tsx index 23876f3..7adb927 100644 --- a/web/src/auth/require-auth.tsx +++ b/web/src/auth/require-auth.tsx @@ -1,14 +1,18 @@ -import { Navigate, Outlet } from "react-router-dom"; +import { Navigate, Outlet, useLocation } from "react-router-dom"; import { useMe } from "../api/queries"; import { AppShellSkeleton } from "@/components/ui/skeletons"; export function RequireAuth() { const { data: user, isLoading } = useMe(); + const location = useLocation(); if (isLoading) return ; - if (!user) return ; + if (!user) { + const from = encodeURIComponent(location.pathname + location.search); + return ; + } return ; } diff --git a/web/src/i18n/en.json b/web/src/i18n/en.json index a8a70bf..02a74a1 100644 --- a/web/src/i18n/en.json +++ b/web/src/i18n/en.json @@ -1,7 +1,7 @@ { "common": { "yes": "Yes", "no": "No", "close": "Close", "loading": "Loading", "filter": "Filter…", "noMatches": "No matches", "language": "Language", "skipToContent": "Skip to content" }, "nav": { "objects": "Objects", "vocabularies": "Vocabularies", "authorities": "Authorities", "fields": "Fields", "search": "Search", "collapseSidebar": "Collapse sidebar", "expandSidebar": "Expand sidebar" }, - "auth": { "email": "Email", "password": "Password", "signIn": "Sign in", "signOut": "Sign out", "invalid": "Invalid email or password", "networkError": "Could not reach the server" }, + "auth": { "email": "Email", "password": "Password", "signIn": "Sign in", "signOut": "Sign out", "invalid": "Invalid email or password", "networkError": "Could not reach the server", "sessionExpired": "Your session expired — please sign in again.", "signingOut": "Signing out…" }, "objects": { "title": "Objects", "empty": "No objects yet", "loadError": "Could not load objects", "notFound": "Object not found", "prev": "Previous", "next": "Next", "of": "of", "new": "New object", "filter": "Filter objects…", "pageSize": "Per page", "columns": { "number": "Object №", "name": "Name", "visibility": "Visibility", "location": "Location", "count": "#", "updated": "Updated" }, "unknownRef": "(unknown)" }, "fieldsLabels": { "objectNumber": "Object number", "objectName": "Name", "count": "Number of objects", "briefDescription": "Brief description", "currentLocation": "Current location", "currentOwner": "Current owner", "recorder": "Recorder", "recordingDate": "Recording date", "visibility": "Visibility" }, "visibility": { "draft": "Draft", "internal": "Internal", "public": "Public" }, diff --git a/web/src/i18n/sv.json b/web/src/i18n/sv.json index 59d9c0c..f0ffe52 100644 --- a/web/src/i18n/sv.json +++ b/web/src/i18n/sv.json @@ -1,7 +1,7 @@ { "common": { "yes": "Ja", "no": "Nej", "close": "Stäng", "loading": "Laddar", "filter": "Filtrera…", "noMatches": "Inga träffar", "language": "Språk", "skipToContent": "Hoppa till innehåll" }, "nav": { "objects": "Föremål", "vocabularies": "Vokabulär", "authorities": "Auktoriteter", "fields": "Fält", "search": "Sök", "collapseSidebar": "Fäll ihop sidofältet", "expandSidebar": "Fäll ut sidofältet" }, - "auth": { "email": "E-post", "password": "Lösenord", "signIn": "Logga in", "signOut": "Logga ut", "invalid": "Fel e-post eller lösenord", "networkError": "Kunde inte nå servern" }, + "auth": { "email": "E-post", "password": "Lösenord", "signIn": "Logga in", "signOut": "Logga ut", "invalid": "Fel e-post eller lösenord", "networkError": "Kunde inte nå servern", "sessionExpired": "Din session har gått ut — logga in igen.", "signingOut": "Loggar ut…" }, "objects": { "title": "Föremål", "empty": "Inga föremål ännu", "loadError": "Kunde inte ladda föremål", "notFound": "Föremålet hittades inte", "prev": "Föregående", "next": "Nästa", "of": "av", "new": "Nytt föremål", "filter": "Filtrera föremål…", "pageSize": "Per sida", "columns": { "number": "Föremålsnr", "name": "Namn", "visibility": "Synlighet", "location": "Plats", "count": "Antal", "updated": "Uppdaterad" }, "unknownRef": "(okänd)" }, "fieldsLabels": { "objectNumber": "Föremålsnummer", "objectName": "Namn", "count": "Antal föremål", "briefDescription": "Kort beskrivning", "currentLocation": "Nuvarande plats", "currentOwner": "Nuvarande ägare", "recorder": "Registrerad av", "recordingDate": "Registreringsdatum", "visibility": "Synlighet" }, "visibility": { "draft": "Utkast", "internal": "Intern", "public": "Publik" }, diff --git a/web/src/shell/navigation-bridge.tsx b/web/src/shell/navigation-bridge.tsx new file mode 100644 index 0000000..f920826 --- /dev/null +++ b/web/src/shell/navigation-bridge.tsx @@ -0,0 +1,16 @@ +import { useEffect } from "react"; +import { useNavigate } from "react-router-dom"; + +import { setNavigate } from "../api/auth-redirect"; + +/** Bridges React Router's navigate to the non-React 401 handler. Renders nothing. */ +export function NavigationBridge() { + const navigate = useNavigate(); + + useEffect(() => { + setNavigate(navigate); + return () => setNavigate(null); + }, [navigate]); + + return null; +} diff --git a/web/src/shell/user-menu.test.tsx b/web/src/shell/user-menu.test.tsx index 70085b9..784c478 100644 --- a/web/src/shell/user-menu.test.tsx +++ b/web/src/shell/user-menu.test.tsx @@ -1,7 +1,7 @@ import { expect, test } from "vitest"; import { screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { http, HttpResponse } from "msw"; +import { delay, http, HttpResponse } from "msw"; import { server } from "../test/server"; import { renderApp } from "../test/render"; import { UserMenu } from "./user-menu"; @@ -33,3 +33,22 @@ test("opens the menu showing email + role and signs out", async () => { await userEvent.click(signOut); await waitFor(() => expect(loggedOut).toBe(true)); }); + +test("shows a pending state on Sign out while logging out", async () => { + server.use( + http.post("/api/admin/logout", async () => { + await delay(50); + return new HttpResponse(null, { status: 204 }); + }), + ); + + renderApp(); + + const trigger = await screen.findByRole("button", { name: /editor@example.com/ }); + await userEvent.click(trigger); + + const menu = within(document.body); + await userEvent.click(await menu.findByText("Sign out")); + + expect(await menu.findByText(/signing out/i)).toBeInTheDocument(); +}); diff --git a/web/src/shell/user-menu.tsx b/web/src/shell/user-menu.tsx index a8779be..643feb1 100644 --- a/web/src/shell/user-menu.tsx +++ b/web/src/shell/user-menu.tsx @@ -35,7 +35,9 @@ export function UserMenu() {
{me.role}
- {t("auth.signOut")} + + {logout.isPending ? t("auth.signingOut") : t("auth.signOut")} + );