diff --git a/src/app.test.ts b/src/app.test.ts index 67fed32..eb4f8ff 100644 --- a/src/app.test.ts +++ b/src/app.test.ts @@ -397,33 +397,30 @@ const stubKeto = (over: Partial): 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, cookie?: string) => { + await new Promise((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((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((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) => { diff --git a/src/jwt-middleware.test.ts b/src/jwt-middleware.test.ts index 4340f1d..aa0d4a0 100644 --- a/src/jwt-middleware.test.ts +++ b/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 = {}, 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); }); diff --git a/todo.md b/todo.md index 87ffc63..e1aca57 100644 --- a/todo.md +++ b/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.