docs(specs): session-expiry soft redirect + auth feedback (#48)

2026-06-08 14:52:16 +02:00
parent 6e72f24f0a
commit a0aab6571f
1 changed files with 194 additions and 0 deletions
@@ -0,0 +1,194 @@
+# Session-Expiry Soft Redirect + Auth Feedback — Design
+
+**Date:** 2026-06-08
+**Status:** Approved (brainstorming) — ready for implementation planning.
+**Issue:** #48 (Frontend UX: session-expiry handling loses in-progress work; auth feedback gaps).
+
+## Context
+
+The session-expiry path is the most damaging daily failure mode for a long-session data-entry tool.
+Today a 401 from any API call runs `redirectToLogin()` (`web/src/api/auth-redirect.ts`) →
+`window.location.assign("/login")`, a **full page reload**: it tears down the React app and query
+cache, gives the login page no reason ("session expired"), and after re-login drops the user on
+`/objects` rather than where they were. Login also hardcodes `navigate("/objects")` and ignores the
+attempted destination, and the Sign out control has no pending state.
+
+**Already fixed since the audit (verified):** `require-auth.tsx` now renders `<AppShellSkeleton />`
+during the `useMe` fetch (not the blank `<div role="status">` the audit cited), so the "blank screen on
+load" problem is gone and is **out of scope**. Logout **error** feedback also already exists: `useLogout`
+carries no `meta.suppressErrorToast`, so the global `MutationCache.onError` (`api/query-client.ts`)
+already shows an error toast on logout failure — the only remaining logout gap is a pending/disabled
+state.
+
+**The crux:** the 401 handler lives in an openapi-fetch `Middleware` (`api/client.ts`), which runs
+**outside** React and therefore cannot call `useNavigate`. The app uses a React Router 7 **data router**
+(`createBrowserRouter`, `app.tsx`). The test harness `renderApp` (`src/test/render.tsx`) mounts the
+component under test at `path:"*"` in a **fresh `createMemoryRouter`** — it does not use the app's real
+`router` singleton.
+
+### Decisions (from brainstorming)
+1. **Soft redirect + return** (chosen over an in-place re-auth modal): router-navigate (no full reload)
+   to `/login?reason=expired&from=<path>`; login shows the reason and returns the user to `from`. The
+   React app + query cache survive; an unsaved edit form's typed values are still lost, but the user
+   lands back on the same record. (Full field-level preservation via an in-place modal is a deferred
+   follow-up.)
+2. **Navigate-bridge module** (chosen over exporting the `router` singleton or a custom DOM event): a
+   module holds a settable `navigate` ref that a one-line component registers. This is testable with the
+   memory-router harness (tests call `setNavigate(mock)` directly); importing the singleton `router`
+   would be untestable because `renderApp` never mounts it.
+
+## Components
+
+### `api/auth-redirect.ts` (rewrite)
+```ts
+type NavigateFn = (to: string, opts?: { replace?: boolean }) => void;
+
+let navigateFn: NavigateFn | null = null;
+
+/** Register/unregister 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);
+  }
+}
+```
+`api/client.ts` is unchanged — the middleware still calls `redirectToLogin()`; only its behaviour changes.
+
+### `shell/navigation-bridge.tsx` (new)
+```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;
+}
+```
+
+### `app.tsx` (wrap routes in a pathless layout)
+Add a pathless layout route around **all** existing routes whose element mounts the bridge plus an
+`<Outlet/>`, so the bridge is always present (covers `/login` and the authed area):
+```tsx
+function RootLayout() {
+  return (
+    <>
+      <NavigationBridge />
+      <Outlet />
+    </>
+  );
+}
+```
+```tsx
+createRoutesFromElements(
+  <Route element={<RootLayout />}>
+    <Route path="/login" element={<LoginPage />} />
+    {/* …existing RequireAuth / AppShell subtree unchanged… */}
+    <Route path="*" element={<Navigate to="/objects" replace />} />
+  </Route>,
+)
+```
+
+### `auth/login-page.tsx` (reason banner + return-to + empty-field guard)
+- `const [params] = useSearchParams();`
+- Reason banner when `params.get("reason") === "expired"`: render `t("auth.sessionExpired")` (a
+  non-error info note, distinct from the existing `role="alert"` invalid-credentials message).
+- On success, navigate to `safeFrom(params.get("from"))` instead of the hardcoded `/objects`:
+```ts
+function safeFrom(raw: string | null): string {
+  if (!raw) return "/objects";
+  // single leading slash only — reject protocol-relative ("//host") / absolute URLs (open redirect)
+  return /^\/(?!\/)/.test(raw) ? raw : "/objects";
+}
+```
+  (`raw` from `useSearchParams` is already percent-decoded.)
+- Disable submit on empty fields: `disabled={login.isPending || !email.trim() || !password}`.
+
+### `auth/require-auth.tsx` (capture the attempted location)
+```tsx
+const location = useLocation();
+// …
+if (!user) {
+  const from = encodeURIComponent(location.pathname + location.search);
+  return <Navigate to={`/login?from=${from}`} replace />;
+}
+```
+No `reason` here — an unauthenticated deep-link is not an expiry; login simply returns the user to `from`.
+
+### `shell/user-menu.tsx` (logout pending state)
+Disable the Sign out item while the mutation is in flight:
+`<MenuItem disabled={logout.isPending} onClick={onSignOut}>`. Error already surfaces via the global toast.
+
+### i18n (en + sv parity — 1 new key)
+- `auth.sessionExpired` = "Your session expired — please sign in again." /
+  "Din session har gått ut — logga in igen."
+
+## Data flow
+API 401 → `client.ts` middleware → `redirectToLogin()` → registered `navigate("/login?reason=expired&
+from=<path>", {replace})` (no reload) → `LoginPage` reads `reason`/`from`, shows the banner → on success
+`navigate(safeFrom(from), {replace})`. Deep-link-while-unauthed → `RequireAuth` → `/login?from=<path>` →
+same return-to. The `NavigationBridge` keeps `setNavigate` current for the lifetime of the app.
+
+## Error handling / edges
+- **Already on `/login`:** `redirectToLogin()` is a no-op (no redirect loop).
+- **401 before the bridge mounts** (first paint): `navigateFn` is null → hard `window.location.assign`
+  to `/login?reason=expired&from=…` (graceful fallback; reason/return still preserved).
+- **Open redirect:** `safeFrom` accepts only a single-leading-slash local path; `//evil.com`,
+  `https://…`, and empty → `/objects`.
+- **Login success with no `from`:** defaults to `/objects` (unchanged behaviour).
+- **StrictMode double-invoke:** the bridge effect is idempotent (sets the same fn; cleanup nulls it) —
+  safe.
+- **Reason banner is informational**, not `role="alert"` (it is not an error and should not preempt the
+  credential-error alert region).
+
+## Testing
+- **`api/auth-redirect.test.ts`** (unit, jsdom): with `setNavigate(mock)` and a stubbed
+  `window.location` (pathname `/objects/abc`), `redirectToLogin()` calls `mock` with
+  `"/login?reason=expired&from=%2Fobjects%2Fabc"` and `{replace:true}`; with `setNavigate(null)` it calls
+  `window.location.assign` with the same target; when `pathname === "/login"` it does nothing.
+- **`auth/login-page` test:** rendering at `/login?reason=expired` shows the `auth.sessionExpired` text;
+  a successful login at `/login?from=%2Fobjects%2F123` navigates to `/objects/123`; a bad `from`
+  (`//evil.com`) falls back to `/objects`; the submit button is disabled until both fields are non-empty.
+- **`auth/require-auth` test:** when `useMe` resolves to no user, it renders a redirect to
+  `/login?from=…` carrying the attempted path.
+- **`shell/user-menu` test:** the Sign out item is disabled while `logout.isPending`.
+- **Gate:** `typecheck`/`lint`/`test`/`build`/`check:size`/`check:colors` green; en/sv parity (the #60
+  parity test guards the new key); no codename; no new dependency.
+
+## Acceptance criteria
+1. A 401 from an API call performs a **router** navigation (no full page reload) to
+   `/login?reason=expired&from=<attempted path>`; the React app/query cache are not torn down.
+2. The login page shows a "session expired" message when `reason=expired`, and on success returns the
+   user to a **validated** `from` path (defaulting to `/objects`); protocol-relative/absolute `from`
+   values are rejected.
+3. `RequireAuth` redirects unauthenticated users to `/login?from=<attempted path>` so deep links return
+   after login.
+4. Login submit is disabled while either field is empty; the Sign out item shows a disabled/pending state
+   while logging out.
+5. `typecheck`/`lint`/`test`/`build`/`check:colors` green; `check:size` reported; en/sv parity (1 new
+   key); no codename; no new dependency.
+
+## Out of scope → follow-ups
+- **In-place re-auth modal** that keeps the edit form mounted for full field-level preservation of
+  in-progress work (deferred by decision; soft redirect ships first).
+- Idle/expiry **pre-warning** (e.g. "your session will expire soon") and token/silent refresh.
+- Broader auth-event audit surfacing on the client.