449 lines
19 KiB
Markdown
449 lines
19 KiB
Markdown
# 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=<path>`. 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: `<Route path="/login">`, `<Route element={<RequireAuth/>}>…</Route>`, `<Route path="*">`. 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 `<Routes>` 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` → `<AppShellSkeleton/>`; `!user` → `<Navigate to="/login" replace/>`. Test: `require-auth.test.tsx`.
|
||
- `user-menu.tsx`: `useLogout()`, `onSignOut` navigates to `/login` on success; `<MenuItem onClick={onSignOut}>{t("auth.signOut")}</MenuItem>`; 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 (
|
||
<>
|
||
<NavigationBridge />
|
||
<Outlet />
|
||
</>
|
||
);
|
||
}
|
||
```
|
||
Wrap the existing top-level fragment children in a pathless `<Route element={<RootLayout />}>` — i.e. change `createRoutesFromElements(<> … </>)` so the outer element is `<Route element={<RootLayout />}> … </Route>` containing the existing `/login`, `RequireAuth`, and `*` routes unchanged:
|
||
```tsx
|
||
const router = createBrowserRouter(
|
||
createRoutesFromElements(
|
||
<Route element={<RootLayout />}>
|
||
<Route path="/login" element={<LoginPage />} />
|
||
<Route element={<RequireAuth />}>
|
||
{/* …AppShell subtree exactly as before… */}
|
||
</Route>
|
||
<Route path="*" element={<Navigate to="/objects" replace />} />
|
||
</Route>,
|
||
),
|
||
);
|
||
```
|
||
|
||
- [ ] **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 `<form>`, above the email field (after the `<h1>`):
|
||
```tsx
|
||
{sessionExpired && (
|
||
<p className="text-sm text-muted-foreground">{t("auth.sessionExpired")}</p>
|
||
)}
|
||
```
|
||
Change the submit button disabled condition:
|
||
```tsx
|
||
<Button type="submit" className="w-full" disabled={login.isPending || !email.trim() || !password}>
|
||
```
|
||
|
||
- [ ] **Step 3: Update `web/src/auth/login-page.test.tsx`.** Extend `tree()` with an `:id` destination and add tests. Replace the file's `tree()` and append the new tests:
|
||
```tsx
|
||
function tree() {
|
||
return (
|
||
<Routes>
|
||
<Route path="/login" element={<LoginPage />} />
|
||
<Route path="/objects" element={<div>objects landing</div>} />
|
||
<Route path="/objects/:id" element={<div>object detail</div>} />
|
||
</Routes>
|
||
);
|
||
}
|
||
```
|
||
Add these tests (keep the two existing ones):
|
||
```tsx
|
||
test("shows the session-expired notice when reason=expired", async () => {
|
||
renderApp(tree(), { route: "/login?reason=expired" });
|
||
expect(await screen.findByText(/session expired/i)).toBeInTheDocument();
|
||
});
|
||
|
||
test("returns to the from path on success", async () => {
|
||
renderApp(tree(), { route: "/login?from=%2Fobjects%2F123" });
|
||
await userEvent.type(screen.getByLabelText(/email/i), "editor@example.com");
|
||
await userEvent.type(screen.getByLabelText(/password/i), "pw-editor-123");
|
||
await userEvent.click(screen.getByRole("button", { name: /sign in/i }));
|
||
expect(await screen.findByText("object detail")).toBeInTheDocument();
|
||
});
|
||
|
||
test("rejects an off-site from and falls back to /objects", async () => {
|
||
renderApp(tree(), { route: "/login?from=%2F%2Fevil.com" });
|
||
await userEvent.type(screen.getByLabelText(/email/i), "editor@example.com");
|
||
await userEvent.type(screen.getByLabelText(/password/i), "pw-editor-123");
|
||
await userEvent.click(screen.getByRole("button", { name: /sign in/i }));
|
||
expect(await screen.findByText("objects landing")).toBeInTheDocument();
|
||
});
|
||
|
||
test("disables submit until both fields are filled", async () => {
|
||
renderApp(tree(), { route: "/login" });
|
||
const button = screen.getByRole("button", { name: /sign in/i });
|
||
expect(button).toBeDisabled();
|
||
await userEvent.type(screen.getByLabelText(/email/i), "a@b.se");
|
||
expect(button).toBeDisabled();
|
||
await userEvent.type(screen.getByLabelText(/password/i), "pw");
|
||
expect(button).toBeEnabled();
|
||
});
|
||
```
|
||
(The existing "successful login navigates to /objects" test has no `from`, so `safeFrom(null)` → `/objects` keeps it green. The default MSW `/api/admin/login` handler returns 204 for `editor@example.com` / `pw-editor-123`.)
|
||
|
||
- [ ] **Step 4: Run the login + i18n tests (vitest ONCE):**
|
||
```bash
|
||
cd web && pnpm vitest run src/auth/login-page.test.tsx src/i18n
|
||
```
|
||
Expected: all green (login-page tests + the i18n parity test covering the 2 new keys).
|
||
|
||
- [ ] **Step 5: Commit**
|
||
```bash
|
||
git add web/src/auth/login-page.tsx web/src/auth/login-page.test.tsx web/src/i18n/en.json web/src/i18n/sv.json
|
||
git commit -m "feat(web): login reason banner + return-to + empty-field guard (#48)"
|
||
```
|
||
|
||
---
|
||
|
||
# Task 3: RequireAuth return-to + logout pending + full gate
|
||
|
||
**Files:**
|
||
- Modify: `web/src/auth/require-auth.tsx`
|
||
- Modify: `web/src/auth/require-auth.test.tsx`
|
||
- Modify: `web/src/shell/user-menu.tsx`
|
||
- Modify: `web/src/shell/user-menu.test.tsx`
|
||
|
||
- [ ] **Step 1: Update `web/src/auth/require-auth.tsx`** to capture the attempted path:
|
||
```tsx
|
||
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 <AppShellSkeleton />;
|
||
|
||
if (!user) {
|
||
const from = encodeURIComponent(location.pathname + location.search);
|
||
return <Navigate to={`/login?from=${from}`} replace />;
|
||
}
|
||
|
||
return <Outlet />;
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Update `web/src/auth/require-auth.test.tsx`** so the login stub echoes the search string, and assert `from` is carried. Replace the `tree()` and the redirect test:
|
||
```tsx
|
||
import { screen, waitFor } from "@testing-library/react";
|
||
import { http, HttpResponse } from "msw";
|
||
import { expect, test } from "vitest";
|
||
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 <div>login page {location.search}</div>;
|
||
}
|
||
|
||
function tree() {
|
||
return (
|
||
<Routes>
|
||
<Route path="/login" element={<LoginStub />} />
|
||
<Route element={<RequireAuth />}>
|
||
<Route path="/objects" element={<div>secret objects</div>} />
|
||
</Route>
|
||
</Routes>
|
||
);
|
||
}
|
||
|
||
test("renders children when authenticated", async () => {
|
||
renderApp(tree(), { route: "/objects" });
|
||
expect(await screen.findByText("secret objects")).toBeInTheDocument();
|
||
});
|
||
|
||
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(/from=%2Fobjects/)).toBeInTheDocument());
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 3: Update `web/src/shell/user-menu.tsx`** to show a logout-pending state. Change only the `<MenuItem>`:
|
||
```tsx
|
||
<MenuItem closeOnClick={false} disabled={logout.isPending} onClick={onSignOut}>
|
||
{logout.isPending ? t("auth.signingOut") : t("auth.signOut")}
|
||
</MenuItem>
|
||
```
|
||
(Everything else in the file is unchanged. `onSignOut` already navigates to `/login` on success, which unmounts the menu since `useMe` becomes null.)
|
||
|
||
- [ ] **Step 4: Add a pending-state test to `web/src/shell/user-menu.test.tsx`.** Add `delay` to the msw import and append a test:
|
||
```tsx
|
||
import { delay, http, HttpResponse } from "msw";
|
||
```
|
||
```tsx
|
||
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(<UserMenu />);
|
||
|
||
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();
|
||
});
|
||
```
|
||
(The existing "signs out" test still passes: `closeOnClick={false}` keeps the item, the POST still fires, `loggedOut` flips true.)
|
||
|
||
- [ ] **Step 5: FULL FRONTEND GATE (run tests EXACTLY ONCE):**
|
||
```bash
|
||
cd web && pnpm typecheck && pnpm lint && pnpm test && pnpm build && pnpm check:size && pnpm check:colors
|
||
```
|
||
All green. Report test totals, largest chunk (gz), and the `check:colors` line.
|
||
|
||
- [ ] **Step 6: Codename + status:**
|
||
```bash
|
||
cd /Users/olsson/Laboratory/biggus-dickus
|
||
git grep -in 'biggus\|dickus' -- web/src; echo "codename-exit=$?"
|
||
git status --short
|
||
```
|
||
Expected: no codename matches (`codename-exit=1`).
|
||
|
||
- [ ] **Step 7: Manual smoke (recommended).** With the stack + server + `pnpm dev` up: open an object edit, let the session expire (or delete the session cookie) and trigger a save → you land on `/login` **without a full reload**, see "Your session expired…", and after re-login return to the same record. Deep-link to a route while logged out → login → returns there. Empty login form → Sign in disabled. Click Sign out → brief "Signing out…".
|
||
|
||
- [ ] **Step 8: Commit**
|
||
```bash
|
||
git add web/src/auth/require-auth.tsx web/src/auth/require-auth.test.tsx web/src/shell/user-menu.tsx web/src/shell/user-menu.test.tsx
|
||
git commit -m "feat(web): return-to-destination on auth redirect; logout pending state (#48)"
|
||
```
|
||
|
||
---
|
||
|
||
## Self-Review (completed)
|
||
|
||
**Spec coverage:** navigate bridge + soft redirect with reason/from (T1 — AC1); login reason banner +
|
||
validated return-to + empty-field guard (T2 — AC2, AC4 first half); `RequireAuth` return-to (T3 S1–S2 —
|
||
AC3); logout pending state (T3 S3–S4 — AC4 second half); i18n 2 keys + parity (T2 S1); full gate +
|
||
codename (T3 S5–S6 — AC5). All 5 acceptance criteria mapped. ✓
|
||
|
||
**Placeholder scan:** every code step shows complete code; tests have concrete assertions and exact
|
||
encoded URLs (`%2Fobjects%2Fabc%3Fx%3D1`, `%2F%2Fevil.com`); `vi.stubGlobal("location", …)` is the
|
||
established stub. No TODO/TBD. ✓
|
||
|
||
**Type consistency:** `setNavigate(fn: NavigateFn | null)` defined in T1 and called with React Router's
|
||
`navigate` (compatible signature `(to, {replace})`) in `NavigationBridge`, and with `null` in cleanup;
|
||
`redirectToLogin()` signature unchanged so `client.ts` needs no edit; `safeFrom(raw: string | null)`
|
||
consumed only in `login-page.tsx`. i18n keys `auth.sessionExpired` / `auth.signingOut` added in T2 and
|
||
`auth.signingOut` consumed in T3. ✓
|
||
|
||
## Notes
|
||
- No new dependency. `ui/menu.tsx` is **not** modified (Base UI `MenuItem` already exposes `closeOnClick`
|
||
+ `disabled`). en/sv parity preserved (2 new keys, guarded by the #60 parity test).
|
||
- `client.ts` is intentionally untouched — only `redirectToLogin`'s behaviour changes.
|
||
- The in-place re-auth modal (full field-level preservation) is a deferred follow-up per the spec.
|