Combine/unify §4 auth tests (todo §4); fold authenticate's matrix into resolveSession (it's the .user wrapper) and the two /auth/complete HTTP tests into one — 219→217, zero coverage lost

src/app.test.ts

@@ -397,33 +397,30 @@ const stubKeto = (over: Partial<KetoClient>): KetoClient => ({
 });
 const withWhoami = (whoami: KratosPublic["whoami"]): KratosPublic => ({ ...mockKratos(async () => { throw new Error("unused"); }), whoami });
 
-test("login completion: mints the session JWT (roles from Keto → projection → tokenize) and sets the cookie", async (t) => {
+test("login completion (/auth/complete): a live session mints the JWT cookie; no session → /login, no cookie", async (t) => {
   const identity: Identity = { id: "01902d5e-7b6c-7e3a-9f21-3c8d1e0a4b55", traits: { email: "admin@plainpages.local" } };
   let projected: unknown;
   const kratos = withWhoami(async (o) => (o?.tokenizeAs ? { active: true, identity, tokenized: "h.p.s" } : { active: true, identity }) as Session);
   const kratosAdmin = stubAdmin({ updateMetadataPublic: async (_id, meta) => { projected = meta; return identity; } });
   const keto = stubKeto({ listRelations: async () => ({ nextPageToken: null, tuples: [{ namespace: "Role", object: "admin", relation: "members", subject_id: `user:${identity.id}` }] }) });
+  const complete = async (app: ReturnType<typeof createApp>, cookie?: string) => {
+    await new Promise<void>((r) => app.listen(0, r));
+    t.after(() => app.close());
+    return fetch(`http://localhost:${(app.address() as AddressInfo).port}/auth/complete`, { headers: cookie ? { cookie } : {}, redirect: "manual" });
+  };
 
-  const app = createApp({ keto, kratos, kratosAdmin });
-  await new Promise<void>((r) => app.listen(0, r));
-  t.after(() => app.close());
-  const res = await fetch(`http://localhost:${(app.address() as AddressInfo).port}/auth/complete`, { headers: { cookie: "plainpages_session=s" }, redirect: "manual" });
-
-  assert.equal(res.status, 303);
-  assert.equal(res.headers.get("location"), "/");
-  assert.match(res.headers.get("set-cookie") ?? "", /^plainpages_jwt=h\.p\.s;.*HttpOnly/);
+  // Live Kratos session: roles from Keto → projection → tokenize → JWT cookie, land on /.
+  const ok = await complete(createApp({ keto, kratos, kratosAdmin }), "plainpages_session=s");
+  assert.equal(ok.status, 303);
+  assert.equal(ok.headers.get("location"), "/");
+  assert.match(ok.headers.get("set-cookie") ?? "", /^plainpages_jwt=h\.p\.s;.*HttpOnly/);
   assert.deepEqual(projected, { roles: ["admin"] }); // Keto roles projected onto the identity for the tokenizer
-});
 
-test("login completion with no Kratos session redirects to /login and sets no cookie", async (t) => {
-  const app = createApp({ keto: stubKeto({}), kratos: withWhoami(async () => null), kratosAdmin: stubAdmin({}) });
-  await new Promise<void>((r) => app.listen(0, r));
-  t.after(() => app.close());
-  const res = await fetch(`http://localhost:${(app.address() as AddressInfo).port}/auth/complete`, { redirect: "manual" });
-
-  assert.equal(res.status, 303);
-  assert.equal(res.headers.get("location"), "/login");
-  assert.equal(res.headers.get("set-cookie"), null);
+  // No Kratos session: nothing minted, bounce to /login with no cookie.
+  const none = await complete(createApp({ keto: stubKeto({}), kratos: withWhoami(async () => null), kratosAdmin: stubAdmin({}) }));
+  assert.equal(none.status, 303);
+  assert.equal(none.headers.get("location"), "/login");
+  assert.equal(none.headers.get("set-cookie"), null);
 });
 
 test("logout (CSRF-guarded POST): valid token revokes the Kratos session + clears our JWT; bad token → 403", async (t) => {

src/jwt-middleware.test.ts

@@ -64,25 +64,22 @@ test("claimsToUser requires sub + email, defaults roles to [], keeps only string
   assert.deepEqual(claimsToUser({ email: "a@b.c", roles: ["a", 1, "b"], sub: "u" }).roles, ["a", "b"]);
 });
 
-test("authenticate: a valid cookie → User; no cookie / invalid / expired → null (fail-closed)", async () => {
-  const cookie = `${SESSION_COOKIE}=${mint(k1.privateKey, "k1", valid)}`;
-  assert.deepEqual(await authenticate(cookie, jwks, { now: NOW }), { email: "a@b.c", id: "u1", roles: ["admin"] });
-  assert.equal(await authenticate(undefined, jwks, { now: NOW }), null);
-  assert.equal(await authenticate("other=1", jwks, { now: NOW }), null);
-  assert.equal(await authenticate(`${SESSION_COOKIE}=not.a.jwt`, jwks, { now: NOW }), null);
-  assert.equal(await authenticate(`${SESSION_COOKIE}=${mint(k1.privateKey, "k1", { ...valid, exp: NOW - 999 })}`, jwks, { now: NOW }), null);
-});
+test("resolveSession classifies the cookie; authenticate is its fail-closed user projection", async () => {
+  const cookie = (extra: Record<string, unknown> = {}, kid = "k1") => `${SESSION_COOKIE}=${mint(k1.privateKey, kid, { ...valid, ...extra })}`;
+  const user = { email: "a@b.c", id: "u1", roles: ["admin"] };
 
-test("resolveSession flags a lapsed token for re-mint, but not no-cookie / tampered tokens", async () => {
-  const ok = await resolveSession(`${SESSION_COOKIE}=${mint(k1.privateKey, "k1", valid)}`, jwks, { now: NOW });
-  assert.deepEqual(ok, { expired: false, user: { email: "a@b.c", id: "u1", roles: ["admin"] } });
-
-  // Present but past exp → the §4 re-mint trigger.
-  const lapsed = await resolveSession(`${SESSION_COOKIE}=${mint(k1.privateKey, "k1", { ...valid, exp: NOW - 999 })}`, jwks, { now: NOW });
-  assert.deepEqual(lapsed, { expired: true, user: null });
-
-  // No cookie / garbage / bad-signature are NOT re-mint candidates (no Ory round-trip).
+  // A valid token → the user, not expired.
+  assert.deepEqual(await resolveSession(cookie(), jwks, { now: NOW }), { expired: false, user });
+  // Present but past exp → the §4 re-mint trigger (expired flagged, no user).
+  assert.deepEqual(await resolveSession(cookie({ exp: NOW - 999 }), jwks, { now: NOW }), { expired: true, user: null });
+  // No cookie / non-ours / garbage / bad-signature are NOT re-mint candidates (no Ory round-trip).
   assert.deepEqual(await resolveSession(undefined, jwks, { now: NOW }), { expired: false, user: null });
+  assert.deepEqual(await resolveSession("other=1", jwks, { now: NOW }), { expired: false, user: null });
   assert.deepEqual(await resolveSession(`${SESSION_COOKIE}=not.a.jwt`, jwks, { now: NOW }), { expired: false, user: null });
-  assert.deepEqual(await resolveSession(`${SESSION_COOKIE}=${mint(k1.privateKey, "nope", valid)}`, jwks, { now: NOW }), { expired: false, user: null });
+  assert.deepEqual(await resolveSession(cookie({}, "nope"), jwks, { now: NOW }), { expired: false, user: null });
+
+  // authenticate() is the convenience wrapper — resolveSession(...).user, dropping the flag.
+  assert.deepEqual(await authenticate(cookie(), jwks, { now: NOW }), user);
+  assert.equal(await authenticate(cookie({ exp: NOW - 999 }), jwks, { now: NOW }), null); // expired ⇒ null
+  assert.equal(await authenticate(undefined, jwks, { now: NOW }), null);
 });

todo.md

@@ -90,7 +90,7 @@ everything via Docker.
 - [x] Make sure we have E2E tests for token timeouts and refresh (maybe by shorten the token lifetime to very low or something). → New full-stack Playwright suite `e2e/auth-refresh.spec.ts` (run via `compose.e2e-auth.yml`): boots the **real** Ory stack (Postgres + Kratos + Keto + bootstrap + web), logs in the seeded admin, completes login on web → session JWT, then proves the §4 "stay signed in" hot path end-to-end — once the token lapses the next request is silently **re-minted** from the live Kratos session (fresh JWT, later `exp`, roles re-read from Keto = `["admin"]`); revoking the Kratos session (admin API) then makes the next lapsed request **clear** the stale cookie (→ anonymous). To make timeout/refresh observable in seconds not ~10m: `ory/kratos/e2e.yml` (merged via a second `-c`) shortens the tokenizer `ttl` to **8s** and points `serve.public.base_url` at `kratos:4433` (so the runner drives self-service over the compose network), and a new explicit **`JWT_CLOCK_SKEW_SEC`** config (default 60, the E2E sets `0`) makes web treat the JWT as expired the instant its ttl lapses instead of +60s. The flow is driven over HTTP (fetch + manual cookie relay) because Kratos/web sit on different hosts here — it exercises web's own server-side relay; the browser-UI login stays §8. Scoped the existing visual suite to `visual.spec.ts` (stays Ory-free/fast) so the two suites don't cross-run. Tests-first for the config knob (`config.test.ts`). Verified live: auth suite green (re-mint + clear), visual suite still 8/8 green; typecheck + 218 units green; both stacks torn down.
 - [x] Run the architecture _and_ the stability reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on the whole project (weighted to the §4 auth hot path). Verdict: no Critical/High; both confirmed the auth core (alg-allowlist JWS verify, fail-closed `resolveSession`, key-by-`kid` cache, timing-safe CSRF, traversal guards) is sound, and that a tampered/garbage cookie **can't** drive the Ory re-mint round-trip (only a validly-signed, time-expired token sets `expired`). **Fixed now (tests-first):** (1) MEDIUM (stability) — the re-mint hot path turned an Ory *outage* into a 500 on **every** lapsed request (a dead Kratos session returns `null` and clears cleanly, but a 5xx/refused/timeout *throws* and escaped to the 500 handler). Wrapped the `remintSession` call in `app.ts` in try/catch → degrade to **anonymous** (route renders signed-out / guard bounces to `/login`), and leave the cookie untouched so it re-mints once Ory recovers; `app.test.ts` re-mint test now also asserts outage→403-not-500 + no cleared cookie. (2) MEDIUM (architecture) — a plugin folder named after a host route (`login`/`logout`/`auth`/`public`/`recovery`/`registration`/`settings`/`verification`) would **silently shadow** it (plugin routes resolve first), the one collision `findConflicts` didn't catch. Added `RESERVED_PLUGIN_IDS` (`plugin.ts`) checked in `discovery.ts` → fails boot loud, like every other conflict; documented in `docs/plugin-contract.md` Identity; `discovery.test.ts` covers it. **Deferred (reviewer-scoped, not §4):** extract `buildShellContext` out of `dashboard.ts` + thread the real `ctx.user` into the shell (kills the hardcoded "Sam Rivers" demo profile) **and** give the host its own internal route table via `matchRoute`/`isAuthorized` → **§5** (the 2nd/3rd built-in screen is the forcing function; the hardcoded user is the one user-visible §4 gap, so §5 opens with it); `/auth/complete` login-CSRF hardening + the `POST /logout` oversized-body→500 papercut → **§9** (security headers/CSRF/cookies); retarget the stale `safeUrl()` §4 reference in the contract doc → the next §4 comment-cleanup item (line 92), helper itself deferred to §5/§7 when untrusted URL data first flows. No action: forwarding the full cookie header to Kratos on re-mint (works, mild over-coupling), the deliberately-opt-in `iss`/`aud` claim checks, the `serializeCookie` length bound. typecheck + 219 units green.
 - [x] Go over all comments in the code and the README and try to make it shorter and more information dense. Remove not strictly needed stuff. → Pass over the §4 auth accretion (the §3 cleanup at line 74 stands). The §4 comments were authored dense, so the wins are targeted: tightened the verbose client module-headers — `kratos-public.ts` (dropped the "themed flow pages build on this" forward-ref, kept the loose-`ui.nodes`-types rationale), `kratos-admin.ts` (folded the admin-port note up, trimmed the `KratosError` restatement), `keto-client.ts` (dropped the caller-listing tail). Retargeted the stale `safeUrl()` ref in `docs/plugin-contract.md` (the §4 reviewer flag at line 91): the helper was deferred to §5/§7, not §4. Left intact: app.ts's per-branch *why* comments (right altitude for scanning the request flow), config.ts's dense field notes, and the §4 README **Auth, sessions & permissions** sections (the canonical design rationale, authored concise in §4). `_(planned)_` markers stay for §9 (line 133 owns dropping them). typecheck + 219 units green.
-- [ ] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us.
+- [x] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us. → Pass over the §4 auth tests. The clients (`kratos-public`/`kratos-admin`/`keto-client`) and the focused units (`jwks`/`flow-view`/`guards`/`csrf`/`body`/`login`) already follow the per-module "matrix + edge" pattern, no fat to cut. Removed the two genuine §4-era overlaps: (1) `jwt-middleware.test.ts` re-ran `resolveSession`'s whole classification matrix again under `authenticate` — but `authenticate` is just `resolveSession(...).user`, so merged into one test where `resolveSession` owns the matrix and `authenticate` is asserted as its fail-closed user-projection (kept `authenticate` itself — a documented convenience export, just not double-tested). (2) `app.test.ts` had two `/auth/complete` HTTP tests (live-session vs no-session) for one route → merged into one (happy path + edge), mirroring the project's style. 219 → 217 tests, zero coverage lost; typecheck + tests green.
 
 ## 5. Built-in admin screens (writes go only to Keto/Kratos)
 - [ ] Users: list (Kratos identities) with filter/sort/pagination; create/edit/deactivate/delete; trigger recovery.