diff --git a/README.md b/README.md index 27727ef..8e08571 100644 --- a/README.md +++ b/README.md @@ -148,13 +148,15 @@ auto-merged by `docker compose up`) turns them back off for live editing. | `PORT` | `3000` | web listen port | | `CACHE_TEMPLATES` | `false` | cache compiled EJS templates (`true` in prod) | | `SECURE_COOKIES` | `false` | mark our session/CSRF cookies `Secure` (`true` in prod https; off in dev http) | -| `REQUIRE_SECURE_SECRETS` | `false` | when `true`, the two secrets must be supplied and differ from the dev throwaways | +| `REQUIRE_SECURE_SECRETS` | `false` | when `true`, `CSRF_SECRET` must be supplied and differ from the dev throwaway | | `KRATOS_PUBLIC_URL` / `KRATOS_ADMIN_URL` | `http://kratos:4433` / `:4434` | identity (self-service / admin) | | `KETO_READ_URL` / `KETO_WRITE_URL` | `http://keto:4466` / `:4467` | permission check / write | +| `HYDRA_ADMIN_URL` | `http://hydra:4445` | OAuth2 provider admin API (§6 login/consent handshake) | | `JWKS_URL` | `file://…/tokenizer/jwks.json` | the Kratos tokenizer signing key; verifies the session JWT (§4) | | `JWT_ISSUER` / `JWT_AUDIENCE` | _unset_ | optional: when set, the session JWT's `iss` / `aud` must match (the dev tokenizer sets neither) | | `JWT_CLOCK_SKEW_SEC` | `60` | exp/nbf leeway (s) for Kratos↔web clock drift (the auth E2E sets `0`) | -| `COOKIE_SECRET` / `CSRF_SECRET` | dev throwaways | enforced by `REQUIRE_SECURE_SECRETS` | +| `ORY_TIMEOUT_SEC` | `5` | per-call timeout for outbound Kratos/Keto/Hydra (and http JWKS) fetches, so a hung Ory can't park a request | +| `CSRF_SECRET` | dev throwaway | signs our double-submit CSRF token; enforced by `REQUIRE_SECURE_SECRETS` | ### What you must supply (the only manual prep) @@ -163,11 +165,11 @@ stack with dev-throwaway secrets, an auto-generated signing key, and a seeded ad (see [Development](#development)). Exactly **two** things can't be auto-generated, and **both are production-only** — neither blocks a clean clone: -1. **Production secrets** — replace the committed dev throwaways: `COOKIE_SECRET` and - `CSRF_SECRET` (env), plus the **JWT signing key** (mount a real `jwks.json` or set +1. **Production secrets** — replace the committed dev throwaway `CSRF_SECRET` (env), plus the + **JWT signing key** (mount a real `jwks.json` or set `…_JWKS_URL` — see [JWT signing key & rotation](#jwt-signing-key--rotation)). Set - `REQUIRE_SECURE_SECRETS=true` and the app refuses to boot until the two secrets are - supplied and differ from the throwaways. + `REQUIRE_SECURE_SECRETS=true` and the app refuses to boot until `CSRF_SECRET` is + supplied and differs from the throwaway. 2. **SSO provider client id/secret** — **optional**; password login works without them. Supplying a provider's creds via env activates it; no creds ⇒ no SSO button (see [Social sign-in (SSO)](#social-sign-in-sso)). diff --git a/compose.yml b/compose.yml index fb777d1..0422019 100644 --- a/compose.yml +++ b/compose.yml @@ -6,7 +6,7 @@ services: ports: - "3000:3000" # Explicit behaviour toggles (the app is environment-agnostic — see AGENTS.md). - # Supply COOKIE_SECRET / CSRF_SECRET via env; REQUIRE_SECURE_SECRETS refuses dev throwaways. + # Supply CSRF_SECRET via env; REQUIRE_SECURE_SECRETS refuses the dev throwaway. environment: CACHE_TEMPLATES: "true" REQUIRE_SECURE_SECRETS: "true" diff --git a/docs/plugin-contract.md b/docs/plugin-contract.md index 9f38eab..f0fe32b 100644 --- a/docs/plugin-contract.md +++ b/docs/plugin-contract.md @@ -111,7 +111,10 @@ A route is `{ method, path, permission?, handler }`. `path` is **relative to the path `/`** (so `/shifts` in the `scheduling` plugin serves `/scheduling/shifts`); the host matches `method` + the resolved full path, extracts `:name` segments into `ctx.params.name`, runs the `permission` gate (a coarse JWT-claim check — see the README), and only then calls the -handler with the [request context](#requestcontext). +handler with the [request context](#requestcontext). When the gate fails, an **anonymous** visitor +is redirected to `/login` to sign in (same as the built-in admin screens; after login they land on +the dashboard, not back on the requested page); a **signed-in** user who simply lacks the role gets +the **403** page. `method` is one of `GET HEAD POST PUT PATCH DELETE`. A `GET` route also answers `HEAD`. diff --git a/e2e/visual.spec.ts b/e2e/visual.spec.ts index e246126..a72d6d7 100644 --- a/e2e/visual.spec.ts +++ b/e2e/visual.spec.ts @@ -123,11 +123,13 @@ test("unknown routes serve the 404 page (a real user-facing flow, covered end-to }); // The reference plugin (plugins/scheduling) ships discovered in the image. Its nav + routes are -// permission-gated, so an anonymous visitor never sees or reaches them (the authenticated list/form -// flow needs cross-host login infra — deferred to the §8 full E2E, todo line 121). Side-effect-free. -test("the reference plugin is permission-gated: anonymous → 403, hidden from the dashboard nav", async ({ page }) => { - const res = await page.goto("/scheduling/shifts"); - expect(res?.status()).toBe(403); +// permission-gated, so an anonymous visitor is bounced to sign in (and never sees it in the nav). +// The authenticated list/form flow is the §8 full E2E (full-flow.spec). Side-effect-free. +test("the reference plugin is permission-gated: anonymous → redirect to /login, hidden from the dashboard nav", async ({ page }) => { + // Don't follow the redirect — this Ory-free suite has no /login handler; assert the gate's 303 itself. + const res = await page.request.get("/scheduling/shifts", { maxRedirects: 0 }); + expect(res.status()).toBe(303); + expect(res.headers()["location"]).toBe("/login"); await page.goto("/"); await expect(page.locator(".sidebar")).toContainText("People"); // dashboard nav renders diff --git a/plugins/scheduling/README.md b/plugins/scheduling/README.md index 06273da..3d9f76a 100644 --- a/plugins/scheduling/README.md +++ b/plugins/scheduling/README.md @@ -7,7 +7,9 @@ What it demonstrates: - **A list page that fetches upstream data** — `GET /scheduling/shifts` calls the upstream REST service and renders the rows with the core building blocks (`shifts.ejs` → app shell, filter-bar, - data-table). Search round-trips the URL; zero-JS. + data-table). Search round-trips the URL; zero-JS. (It fetches **all** rows for brevity — for a + large list, parse `page`/`pageSize` from `parseListQuery`, forward them upstream as a `?limit`/ + `?offset`, and render `pagination.ejs` with `paginate()`, exactly as the built-in admin screens do.) - **A form that forwards a write upstream** — `GET /scheduling/shifts/new` renders the form, `POST /scheduling/shifts` CSRF-verifies it (`ctx.verifyCsrf`) and forwards the create upstream, then POST-redirect-GET. The form body lives in the plugin's own `views/partials/shift-form.ejs`, @@ -37,6 +39,10 @@ Your backend must expose two routes; the plugin treats any non-2xx as a recovera Domain rules (overlap, capacity, time ordering) live in your backend — reject with a 4xx and the form re-renders. The plugin only validates that `title` and `assignee` are non-empty. +`start`/`end` come from the form's `datetime-local` inputs as `YYYY-MM-DDTHH:mm` and are stored and +shown verbatim (the dev mock seeds a space-separated style, so created vs seeded rows differ only +cosmetically) — normalise to your backend's format there if it matters. + ## Granting access A user sees Scheduling once they hold the `scheduling:read` role in Keto (and `scheduling:write` diff --git a/plugins/scheduling/shifts.ts b/plugins/scheduling/shifts.ts index 28753b0..9355210 100644 --- a/plugins/scheduling/shifts.ts +++ b/plugins/scheduling/shifts.ts @@ -87,7 +87,7 @@ function toShift(raw: unknown): Shift { export function buildListModel(opts: { canWrite: boolean; chrome: PageChrome; error?: string; q: string; shifts: Shift[] }) { return { - breadcrumbs: [{ href: SHIFTS_PATH, label: "Scheduling" }, { label: "Shifts" }], + breadcrumbs: [{ label: "Shifts" }], // SHIFTS_PATH is the list itself; the form links back to it as "Shifts" canWrite: opts.canWrite, chrome: opts.chrome, ...(opts.error ? { error: opts.error } : {}), diff --git a/public/css/styles.css b/public/css/styles.css index 7e72732..6b1003d 100644 --- a/public/css/styles.css +++ b/public/css/styles.css @@ -684,4 +684,4 @@ th[aria-sort="descending"] .sort-ico { transform: rotate(180deg); } .detail-list dd { margin: 0; word-break: break-word; } .btn-danger { color: var(--neg); border-color: var(--neg-bd); } .btn-danger:hover { background: var(--neg-bg); } -.recovery-link { word-break: break-all; } +.recovery-code code { font-size: 1.15rem; font-weight: 600; letter-spacing: 0.04em; } diff --git a/src/admin-users.ts b/src/admin-users.ts index 77a8d07..619f465 100644 --- a/src/admin-users.ts +++ b/src/admin-users.ts @@ -4,6 +4,7 @@ // models; `handleAdminUsers` is the imperative shell app.ts dispatches to — gated admin-only, // CSRF-guarded, each action mapped to a RouteResult (render, or redirect after a write — PRG). +import { safeDecode } from "./admin-groups.ts"; import { ADMIN_USERS_BASE, adminNav, buildConfirmModel, guardedForm, requireAdmin } from "./admin-nav.ts"; import type { RequestContext, User } from "./context.ts"; import type { Identity, KratosAdmin, RecoveryCode } from "./kratos-admin.ts"; @@ -253,7 +254,7 @@ export function buildUserFormModel(opts: { { id: "first", label: "First name", name: "first", optional: true, value: np.first }, { id: "last", label: "Last name", name: "last", optional: true, value: np.last }, ]; - if (!editing) fields.push({ autocomplete: "new-password", hint: "Optional — leave blank to have the user set one via a recovery link.", icon: "i-lock", id: "password", label: "Password", name: "password", optional: true, type: "password" }); + if (!editing) fields.push({ autocomplete: "new-password", hint: "Optional — leave blank to have the user set one via a recovery code.", icon: "i-lock", id: "password", label: "Password", name: "password", optional: true, type: "password" }); return { edit: editing ? { @@ -335,7 +336,8 @@ export async function handleAdminUsers(ctx: RequestContext, csrfToken: string, d if (seg.length === 1 && seg[0] === "new" && method === "GET") return renderForm({}); // /admin/users/:id … - const targetId = decodeURIComponent(seg[0]!); + const targetId = safeDecode(seg[0]!); // malformed %-encoding → 404, not a 500 (matches groups/roles/clients) + if (targetId === null) return { html: await render("404", { title: "Not found" }), status: 404 }; const identity = await kratosAdmin.getIdentity(targetId); if (!identity) return { html: await render("404", { title: "Not found" }), status: 404 }; const back = `${ADMIN_USERS_BASE}/${encodeURIComponent(targetId)}`; diff --git a/src/app.test.ts b/src/app.test.ts index facfe20..723af82 100644 --- a/src/app.test.ts +++ b/src/app.test.ts @@ -179,12 +179,10 @@ test("mounts plugin routes: params, html/json/redirect/view results, and the per assert.match(await css.text(), /\.demo/); assert.equal((await fetch(url + "/public/demo/..%2f..%2fplugin.ts")).status, 403); // traversal still blocked - // gated route with no session → the rendered 403 page (covers the gate + 403.ejs over HTTP) - const denied = await fetch(url + "/demo/secret"); - assert.equal(denied.status, 403); - const deniedBody = await denied.text(); - assert.match(deniedBody, /403/); - assert.match(deniedBody, /styles\.css/); + // gated route, anonymous → redirect to sign in (like the built-in screens), not a dead-end 403 + const denied = await fetch(url + "/demo/secret", { redirect: "manual" }); + assert.equal(denied.status, 303); + assert.equal(denied.headers.get("location"), "/login"); // known path + wrong method → 405 with Allow; unknown path → 404 const wrong = await fetch(url + "/demo/data", { method: "DELETE" }); @@ -255,22 +253,24 @@ function mintJwt(payload: Record): string { return `${input}.${b64url(sign("SHA256", Buffer.from(input), { dsaEncoding: "ieee-p1363", key: ec.privateKey }))}`; } -test("a verified session JWT authorizes a role-gated route; no cookie / expired token → 403", async (t) => { +test("a verified session JWT authorizes a role-gated route; no cookie / expired token → sign in", async (t) => { const app = createApp({ jwks: staticJwks([ecJwk]), plugins: [demoPlugin] }); await new Promise((r) => app.listen(0, r)); t.after(() => app.close()); const url = `http://localhost:${(app.address() as AddressInfo).port}`; const nowSec = Math.floor(Date.now() / 1000); - const secret = (cookie?: string) => fetch(url + "/demo/secret", cookie ? { headers: { cookie } } : {}); + const secret = (cookie?: string) => fetch(url + "/demo/secret", { redirect: "manual", ...(cookie ? { headers: { cookie } } : {}) }); // Token carrying the gating role → the handler runs (200). const ok = await secret(`${SESSION_COOKIE}=${mintJwt({ email: "a@b.c", exp: nowSec + 600, roles: ["demo:read"], sub: "u1" })}`); assert.equal(ok.status, 200); assert.equal(await ok.text(), "secret"); - // No cookie and an expired token both render anonymous → the gate denies (403). - assert.equal((await secret()).status, 403); - assert.equal((await secret(`${SESSION_COOKIE}=${mintJwt({ email: "a@b.c", exp: nowSec - 600, roles: ["demo:read"], sub: "u1" })}`)).status, 403); + // No cookie and an expired token both render anonymous → the gate bounces to sign in (303 → /login). + const noCookie = await secret(); + assert.equal(noCookie.status, 303); + assert.equal(noCookie.headers.get("location"), "/login"); + assert.equal((await secret(`${SESSION_COOKIE}=${mintJwt({ email: "a@b.c", exp: nowSec - 600, roles: ["demo:read"], sub: "u1" })}`)).status, 303); // The home menu wires in the permission-gated Admin section: an admin's roles surface the links. const home = (cookie?: string) => fetch(url + "/", cookie ? { headers: { cookie } } : {}); @@ -295,21 +295,23 @@ test("session re-mint: an expired JWT backed by a live Kratos session is silentl assert.equal(await ok.text(), "secret"); assert.match(ok.headers.get("set-cookie") ?? "", /^plainpages_jwt=/); - // Kratos session gone: no re-mint, the stale cookie is cleared, the gate denies. + // Kratos session gone: no re-mint, the stale cookie is cleared, the now-anonymous request bounces to sign in. const dead = createApp({ jwks: staticJwks([ecJwk]), keto, kratos: withWhoami(async () => null), kratosAdmin: stubAdmin({}), plugins: [demoPlugin] }); await new Promise((r) => dead.listen(0, r)); t.after(() => dead.close()); - const denied = await fetch(`http://localhost:${(dead.address() as AddressInfo).port}/demo/secret`, { headers: { cookie: expired } }); - assert.equal(denied.status, 403); + const denied = await fetch(`http://localhost:${(dead.address() as AddressInfo).port}/demo/secret`, { headers: { cookie: expired }, redirect: "manual" }); + assert.equal(denied.status, 303); + assert.equal(denied.headers.get("location"), "/login"); assert.match(denied.headers.get("set-cookie") ?? "", /^plainpages_jwt=;.*Max-Age=0/); - // Ory unreachable (not a dead session): whoami throws → degrade to anonymous (403, not 500), + // Ory unreachable (not a dead session): whoami throws → degrade to anonymous (bounce to /login, not 500), // and leave the cookie untouched so the token can re-mint once Ory recovers. const down = createApp({ jwks: staticJwks([ecJwk]), keto, kratos: withWhoami(async () => { throw new KratosError("kratos down", 503, ""); }), kratosAdmin: stubAdmin({}), plugins: [demoPlugin] }); await new Promise((r) => down.listen(0, r)); t.after(() => down.close()); - const outage = await fetch(`http://localhost:${(down.address() as AddressInfo).port}/demo/secret`, { headers: { cookie: expired } }); - assert.equal(outage.status, 403); + const outage = await fetch(`http://localhost:${(down.address() as AddressInfo).port}/demo/secret`, { headers: { cookie: expired }, redirect: "manual" }); + assert.equal(outage.status, 303); + assert.equal(outage.headers.get("location"), "/login"); assert.equal(outage.headers.get("set-cookie"), null); }); @@ -322,6 +324,7 @@ test("guards map to responses: requireSession → /login, a failed can/check → { handler: (ctx) => ({ html: `hi ${requireSession(ctx).email}` }), method: "GET", path: "/me" }, { handler: (ctx) => { if (!can(ctx, "admin")) throw new GuardError(403, "no"); return { html: "ok" }; }, method: "GET", path: "/admin-only" }, { handler: async (ctx) => { if (!(await check(keto, ctx, { namespace: "Resource", object: ctx.params.id ?? "", relation: "view" }))) throw new GuardError(403, "no"); return { html: "seen" }; }, method: "GET", path: "/doc/:id" }, + { handler: () => ({ html: "gated" }), method: "GET", path: "/gated", permission: "secret:read" }, // declarative route gate ], }; const app = createApp({ jwks: staticJwks([ecJwk]), plugins: [guarded] }); @@ -346,6 +349,15 @@ test("guards map to responses: requireSession → /login, a failed can/check → // check (live Keto): the keto verdict gates the handler. assert.equal((await fetch(url + "/guarded/doc/open", auth([]))).status, 200); assert.equal((await fetch(url + "/guarded/doc/shut", auth([]))).status, 403); + + // declarative route `permission` gate: anonymous → sign in, signed-in-without-role → the 403 page, with → 200. + const gAnon = await fetch(url + "/guarded/gated", { redirect: "manual" }); + assert.equal(gAnon.status, 303); + assert.equal(gAnon.headers.get("location"), "/login"); + const gDenied = await fetch(url + "/guarded/gated", auth([])); + assert.equal(gDenied.status, 403); + assert.match(await gDenied.text(), /403/); // the rendered 403.ejs over HTTP + assert.equal((await fetch(url + "/guarded/gated", auth(["secret:read"]))).status, 200); }); test("plugin hooks: onRequest can short-circuit a request and onResponse observes the handler result", async (t) => { @@ -361,10 +373,12 @@ test("plugin hooks: onRequest can short-circuit a request and onResponse observe }; const url = await startApp(t, [hooked]); - // onRequest short-circuits before routing — handler never runs. + // onRequest short-circuits before routing — handler never runs; a fresh CSRF cookie still rides the + // response so a form the hook renders has its matching double-submit cookie. const blocked = await fetch(url + "/hooked/blocked"); assert.equal(blocked.status, 403); assert.match(await blocked.text(), /blocked by hook/); + assert.match(blocked.headers.get("set-cookie") ?? "", /plainpages_csrf=/); // A normal route runs the handler; onResponse observed its result. assert.match(await (await fetch(url + "/hooked/ok")).text(), /handler ran/); @@ -416,6 +430,24 @@ test("themed flow init: no ?flow= initialises one, relays Kratos' CSRF cookie, a assert.equal(stale.headers.get("location"), "/login"); }); +test("themed auth: an already-signed-in user is sent home from /login and /registration, not /settings", async (t) => { + const app = createApp({ jwks: staticJwks([ecJwk]), kratos: mockKratos(async (_t, id) => loginFlow(id)) }); + await new Promise((r) => app.listen(0, r)); + t.after(() => app.close()); + const url = `http://localhost:${(app.address() as AddressInfo).port}`; + const signedIn = { headers: { cookie: `${SESSION_COOKIE}=${mintJwt({ email: "a@b.c", exp: Math.floor(Date.now() / 1000) + 600, roles: [], sub: "u1" })}` }, redirect: "manual" as const }; + + for (const path of ["/login", "/registration"]) { + const res = await fetch(url + path, signedIn); + assert.equal(res.status, 303, `${path} while signed in → 303`); + assert.equal(res.headers.get("location"), "/"); + } + // /settings stays reachable when signed in (inits its flow, not bounced home). + assert.equal((await fetch(url + "/settings", signedIn)).headers.get("location"), "/settings?flow=new1"); + // Anonymous still gets the login flow (no short-circuit). + assert.equal((await fetch(url + "/login", { redirect: "manual" })).headers.get("location"), "/login?flow=new1"); +}); + test("renders a fetched flow as the themed auth page: fields post straight to Kratos, errors surface", async (t) => { const app = createApp({ kratos: mockKratos(async (_t, id) => loginFlow(id)) }); await new Promise((r) => app.listen(0, r)); @@ -751,10 +783,13 @@ test("admin Users screen: gate, list/filter, create, edit, deactivate, delete, r await post(`/admin/users/${target.id}/state`, `_csrf=${token}`); assert.equal(target.state, "inactive"); - // Recovery: renders the edit page (200) carrying the generated link. + // Recovery: renders the edit page (200) showing the generated code (code-based; no admin-host link). const rec = await post(`/admin/users/${target.id}/recovery`, `_csrf=${token}`); assert.equal(rec.status, 200); - assert.match(await rec.text(), /self-service\/recovery\?code=123456/); + const recHtml = await rec.text(); + assert.match(recHtml, /Recovery code generated/); + assert.match(recHtml, /123456<\/code>/); + assert.doesNotMatch(recHtml, /self-service\/recovery\?code=/); // the unreachable admin-API link is gone // Delete needs a deliberate confirm step (zero-JS): GET renders the interstitial, POST performs it. const confirm = await (await get(`/admin/users/${target.id}/delete`)).text(); @@ -770,8 +805,9 @@ test("admin Users screen: gate, list/filter, create, edit, deactivate, delete, r assert.equal((await post(`/admin/users/admin1/state`, `_csrf=${token}`)).status, 400); assert.equal(store.find((x) => x.id === "admin1")!.state, "active"); - // Unknown id → 404. + // Unknown id → 404; malformed %-encoding → 404 (not a 500), matching groups/roles/clients. assert.equal((await get(`/admin/users/${randomUUID()}`)).status, 404); + assert.equal((await get("/admin/users/%ZZ")).status, 404); }); // Built-in Groups admin screen (§5): gate + list/create/membership/delete over HTTP against a diff --git a/src/app.ts b/src/app.ts index 16b266c..a7e1a48 100644 --- a/src/app.ts +++ b/src/app.ts @@ -152,6 +152,9 @@ export function createApp(options: AppOptions = {}): Server { if (anyRequestHooks) { const short = await runRequestHooks(plugins, ctx); if (short) { + // Set the fresh CSRF cookie like every other page-emitting path, so a form the hook + // renders (its token is in ctx.chrome.csrfToken) has the matching double-submit cookie. + if (csrf.fresh) res.appendHeader("set-cookie", csrfCookie(csrf.token, { secure: secureCookies })); await sendResult(res, short.result, (view, data) => renderView(short.plugin.id, view, data)); return; } @@ -164,6 +167,9 @@ export function createApp(options: AppOptions = {}): Server { if (match) { const routeCtx = buildContext(req, res, { chrome: chrome(), params: match.params, user, verifyCsrf }); if (!isAuthorized(match.route, routeCtx.roles)) { + // Anonymous → sign in (like the built-in screens' requireSession); a signed-in user who + // simply lacks the role gets the 403 page. + if (!routeCtx.user) { res.writeHead(303, { location: "/login" }).end(); return; } sendHtml(res, 403, await render("403", { title: "Forbidden" })); return; } @@ -213,6 +219,12 @@ export function createApp(options: AppOptions = {}): Server { // Themed Kratos self-service pages (login/registration/recovery/verification/settings). const flowType = AUTH_FLOWS[pathname]; if (kratos && flowType && (method === "GET" || method === "HEAD")) { + // Already signed in? Re-authenticating / re-registering is pointless — send them home. + // (/settings, /recovery, /verification stay reachable — a signed-in user can use those.) + if (ctx.user && (pathname === "/login" || pathname === "/registration")) { + res.writeHead(303, { location: "/" }).end(); + return; + } const cookie = req.headers.cookie; const flowId = ctx.url.searchParams.get("flow"); if (!flowId) { diff --git a/src/config.test.ts b/src/config.test.ts index 501c472..eb9da10 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -5,7 +5,6 @@ import { loadConfig } from "./config.ts"; // Explicit secure-secret enforcement (no environment sniffing): secrets are the only // thing a hardened deploy must supply. const secureEnv = { - COOKIE_SECRET: "real-cookie-secret", CSRF_SECRET: "real-csrf-secret", REQUIRE_SECURE_SECRETS: "true", }; @@ -20,7 +19,6 @@ test("loads dev defaults when the environment is empty", () => { assert.equal(c.ketoReadUrl, "http://keto:4466"); assert.equal(c.ketoWriteUrl, "http://keto:4467"); assert.equal(c.hydraAdminUrl, "http://hydra:4445"); - assert.match(c.cookieSecret, /dev-insecure/); assert.match(c.csrfSecret, /dev-insecure/); assert.equal(c.jwtClockSkewSec, 60); // default exp/nbf leeway for Kratos↔web clock drift }); @@ -51,10 +49,10 @@ test("parses explicit boolean toggles and rejects non-boolean values", () => { }); test("reads overrides from the environment", () => { - const c = loadConfig({ COOKIE_SECRET: "x", KRATOS_PUBLIC_URL: "https://id.example.com", PORT: "8080" }); + const c = loadConfig({ CSRF_SECRET: "x", KRATOS_PUBLIC_URL: "https://id.example.com", PORT: "8080" }); assert.equal(c.port, 8080); assert.equal(c.kratosPublicUrl, "https://id.example.com"); - assert.equal(c.cookieSecret, "x"); + assert.equal(c.csrfSecret, "x"); }); test("rejects an invalid PORT", () => { @@ -67,22 +65,26 @@ test("JWT_CLOCK_SKEW_SEC: parses a non-negative integer, rejects junk (E2E short for (const v of ["-1", "1.5", "abc"]) assert.throws(() => loadConfig({ JWT_CLOCK_SKEW_SEC: v }), /JWT_CLOCK_SKEW_SEC/); }); +test("ORY_TIMEOUT_SEC: defaults to 5 and must be a positive integer (0 would abort every Ory call)", () => { + assert.equal(loadConfig({}).oryTimeoutSec, 5); + assert.equal(loadConfig({ ORY_TIMEOUT_SEC: "10" }).oryTimeoutSec, 10); + for (const v of ["0", "-1", "1.5", "abc"]) assert.throws(() => loadConfig({ ORY_TIMEOUT_SEC: v }), /ORY_TIMEOUT_SEC/); +}); + test("rejects a malformed Ory URL", () => { assert.throws(() => loadConfig({ KETO_READ_URL: "not a url" }), /KETO_READ_URL/); }); test("REQUIRE_SECURE_SECRETS rejects a missing or dev-throwaway secret", () => { - assert.throws(() => loadConfig({ REQUIRE_SECURE_SECRETS: "true" }), /COOKIE_SECRET/); - assert.throws(() => loadConfig({ COOKIE_SECRET: "real", REQUIRE_SECURE_SECRETS: "true" }), /CSRF_SECRET/); + assert.throws(() => loadConfig({ REQUIRE_SECURE_SECRETS: "true" }), /CSRF_SECRET/); assert.throws( - () => loadConfig({ COOKIE_SECRET: "dev-insecure-cookie-secret", CSRF_SECRET: "real", REQUIRE_SECURE_SECRETS: "true" }), - /COOKIE_SECRET/, + () => loadConfig({ CSRF_SECRET: "dev-insecure-csrf-secret", REQUIRE_SECURE_SECRETS: "true" }), + /CSRF_SECRET/, ); }); test("REQUIRE_SECURE_SECRETS succeeds with real secrets and still defaults the Ory URLs", () => { const c = loadConfig(secureEnv); - assert.equal(c.cookieSecret, "real-cookie-secret"); assert.equal(c.csrfSecret, "real-csrf-secret"); assert.equal(c.kratosPublicUrl, "http://kratos:4433"); // only secrets are enforced; URLs still default }); diff --git a/src/config.ts b/src/config.ts index bd75519..1e532ae 100644 --- a/src/config.ts +++ b/src/config.ts @@ -10,7 +10,6 @@ export interface Config { cacheTemplates: boolean; - cookieSecret: string; csrfSecret: string; hydraAdminUrl: string; jwksUrl: string; @@ -21,6 +20,7 @@ export interface Config { ketoWriteUrl: string; kratosAdminUrl: string; kratosPublicUrl: string; + oryTimeoutSec: number; // per-call timeout for outbound Kratos/Keto/Hydra fetches (bounds a hung Ory) port: number; secureCookies: boolean; } @@ -82,11 +82,18 @@ function readNonNegInt(env: Env, key: string, devDefault: number): number { return n; } +function readPosInt(env: Env, key: string, devDefault: number): number { + const raw = env[key]; + if (raw === undefined) return devDefault; + const n = Number(raw); + if (!Number.isInteger(n) || n < 1) throw new Error(`config: ${key} must be a positive integer, got "${raw}"`); + return n; +} + export function loadConfig(env: Env = process.env): Config { const requireSecure = readBool(env, "REQUIRE_SECURE_SECRETS", false); return { cacheTemplates: readBool(env, "CACHE_TEMPLATES", false), - cookieSecret: readSecret(env, "COOKIE_SECRET", "dev-insecure-cookie-secret", requireSecure), csrfSecret: readSecret(env, "CSRF_SECRET", "dev-insecure-csrf-secret", requireSecure), // Hydra admin API — the OAuth2 login/consent challenge handshake (§6); not on the first-party path. hydraAdminUrl: readUrl(env, "HYDRA_ADMIN_URL", "http://hydra:4445"), @@ -103,6 +110,7 @@ export function loadConfig(env: Env = process.env): Config { ketoWriteUrl: readUrl(env, "KETO_WRITE_URL", "http://keto:4467"), kratosAdminUrl: readUrl(env, "KRATOS_ADMIN_URL", "http://kratos:4434"), kratosPublicUrl: readUrl(env, "KRATOS_PUBLIC_URL", "http://kratos:4433"), + oryTimeoutSec: readPosInt(env, "ORY_TIMEOUT_SEC", 5), port: readPort(env), // Set Secure on our session/CSRF cookies. Off by default (dev runs http); prod (https) sets it. secureCookies: readBool(env, "SECURE_COOKIES", false), diff --git a/src/fetch-timeout.test.ts b/src/fetch-timeout.test.ts new file mode 100644 index 0000000..4bdc7b6 --- /dev/null +++ b/src/fetch-timeout.test.ts @@ -0,0 +1,24 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { withTimeout } from "./fetch-timeout.ts"; + +test("withTimeout injects an abort signal that fires after the deadline", async () => { + let seenSignal: AbortSignal | undefined; + const slow: typeof fetch = ((_input, init) => { + seenSignal = (init as RequestInit | undefined)?.signal ?? undefined; + return new Promise((_resolve, reject) => { + seenSignal?.addEventListener("abort", () => reject(seenSignal!.reason)); + }); + }) as typeof fetch; + + await assert.rejects(withTimeout(slow, 20)("http://x/"), (e: unknown) => (e as Error).name === "TimeoutError"); + assert.ok(seenSignal instanceof AbortSignal); // the wrapped call received a real signal +}); + +test("withTimeout keeps a caller-supplied signal instead of overriding it", async () => { + let seen: AbortSignal | undefined; + const fake: typeof fetch = ((_input, init) => { seen = (init as RequestInit | undefined)?.signal ?? undefined; return Promise.resolve(new Response("ok")); }) as typeof fetch; + const mine = new AbortController().signal; + await withTimeout(fake, 50)("http://x/", { signal: mine }); + assert.equal(seen, mine); +}); diff --git a/src/fetch-timeout.ts b/src/fetch-timeout.ts new file mode 100644 index 0000000..5ced9b5 --- /dev/null +++ b/src/fetch-timeout.ts @@ -0,0 +1,9 @@ +// Bound every outbound Ory call (todo §8 review): a reachable-but-silent host — a hung container, a +// black-holed socket, an LB holding the connection — would otherwise park a request handler forever +// (and exhaust the pool under load). Wrap the injected `fetch` so each call aborts after `ms` unless +// the caller already passed its own signal. server.ts wires this into the Kratos/Keto/Hydra clients. + +export function withTimeout(fetchImpl: typeof fetch, ms: number): typeof fetch { + // A caller-supplied signal wins (so an explicit abort still works); otherwise inject the timeout. + return (input, init) => fetchImpl(input, { ...init, signal: init?.signal ?? AbortSignal.timeout(ms) }); +} diff --git a/src/jwks.test.ts b/src/jwks.test.ts index 7256968..a9f30d4 100644 --- a/src/jwks.test.ts +++ b/src/jwks.test.ts @@ -26,6 +26,12 @@ test("loadJwks reads a file:// set and a base64:// inline set, rejects http", () assert.equal(loadJwks(`base64://${Buffer.from(inline).toString("base64")}`)[0]?.kid, "inline"); assert.throws(() => loadJwks("http://keto:4466/keys"), /unsupported/); + + // Malformed sets fail loud at load, not as an opaque crypto error at verify time. + const b64 = (o: unknown) => `base64://${Buffer.from(JSON.stringify(o)).toString("base64")}`; + assert.throws(() => loadJwks(b64({})), /missing `keys`/); + assert.throws(() => loadJwks(b64({ keys: ["nope"] })), /string `kty`/); // a non-object key + assert.throws(() => loadJwks(b64({ keys: [{ kid: "x" }] })), /string `kty`/); // key missing kty }); test("cachingJwks caches within TTL, reloads after expiry", async () => { diff --git a/src/jwks.ts b/src/jwks.ts index 5960d88..f3a7131 100644 --- a/src/jwks.ts +++ b/src/jwks.ts @@ -24,6 +24,13 @@ export interface JwksCacheOptions { function parseJwks(text: string): JsonWebKey[] { const parsed = JSON.parse(text) as { keys?: unknown }; if (!Array.isArray(parsed.keys)) throw new Error("JWKS: missing `keys` array"); + // Validate element shape here so a malformed key fails loud at load, not as an opaque crypto + // error on the first authenticated request (the verifier keys off `kty`/`kid`). + for (const k of parsed.keys) { + if (typeof k !== "object" || k === null || typeof (k as JsonWebKey).kty !== "string") { + throw new Error("JWKS: each key must be an object with a string `kty`"); + } + } return parsed.keys as JsonWebKey[]; } diff --git a/src/server.ts b/src/server.ts index 49be9c6..2052c7d 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,6 +1,7 @@ import { createApp } from "./app.ts"; import { loadConfig } from "./config.ts"; import { discoverPlugins } from "./discovery.ts"; +import { withTimeout } from "./fetch-timeout.ts"; import { runBootHooks } from "./hooks.ts"; import { createHydraAdmin } from "./hydra-admin.ts"; import { createJwksProvider } from "./jwks.ts"; @@ -11,15 +12,17 @@ import { loadMenuConfig } from "./menu-config.ts"; const config = loadConfig(); // validates the env (incl. enforced secrets) — fails loud at boot const menu = await loadMenuConfig(); // config/menu.ts override + branding — fails loud if malformed +// Every outbound Ory call is bounded so a hung/silent Ory can't park a request handler forever. +const oryFetch = withTimeout(fetch, config.oryTimeoutSec * 1000); // Ory clients for the themed self-service routes + login completion (§4). -const kratos = createKratosPublic({ baseUrl: config.kratosPublicUrl }); -const kratosAdmin = createKratosAdmin({ baseUrl: config.kratosAdminUrl }); -const keto = createKetoClient({ readUrl: config.ketoReadUrl, writeUrl: config.ketoWriteUrl }); +const kratos = createKratosPublic({ baseUrl: config.kratosPublicUrl, fetchImpl: oryFetch }); +const kratosAdmin = createKratosAdmin({ baseUrl: config.kratosAdminUrl, fetchImpl: oryFetch }); +const keto = createKetoClient({ fetchImpl: oryFetch, readUrl: config.ketoReadUrl, writeUrl: config.ketoWriteUrl }); // Hydra admin client for the OAuth2 login/consent challenge handshake (§6). -const hydra = createHydraAdmin({ baseUrl: config.hydraAdminUrl }); +const hydra = createHydraAdmin({ baseUrl: config.hydraAdminUrl, fetchImpl: oryFetch }); // Session-JWT verify key: primed at boot from the configured JWKS (file mount, base64 inline, // or fetched http), then served from cache with TTL refresh + rotation-on-miss (§4). -const jwks = await createJwksProvider(config.jwksUrl); +const jwks = await createJwksProvider(config.jwksUrl, { fetchImpl: oryFetch }); // bound an http JWKS fetch too const plugins = await discoverPlugins(); // scans plugins/, validates — fails loud on a bad plugin console.log(`Discovered ${plugins.length} plugin(s)${plugins.length ? `: ${plugins.map((p) => p.id).join(", ")}` : ""}`); diff --git a/todo.md b/todo.md index f124bd2..ee95bfc 100644 --- a/todo.md +++ b/todo.md @@ -120,7 +120,7 @@ everything via Docker. - [x] node --test units across helpers / router / nav / auth (tests-first throughout). → Audited unit coverage across the four areas; built tests-first through §0–§7, it was already near-complete — **helpers** (`list-query`/`paginate`/`body`/`icons`/`config`/`context`/`flow-view`/`gen-jwks`/`hooks`/`shell-context`, `static` via `app.test.ts`), **router** (`router`/`view-resolver`/`plugin`/`discovery`), **nav** (`nav`/`nav-tree`/`chrome`/`menu-config`/dashboard merge), **auth** (`jwt`/`jwt-middleware`/`jwks`/`guards`/`login`/`csrf`/`cookie`/`kratos-*`/`keto-client`/`oauth-*`/`hydra-admin`) all carry direct `node --test` units. **One genuine gap closed:** `admin-nav.ts` — its pure nav helpers (`adminSection`/`adminNav`) and security-critical auth gates (`requireAdmin`/`guardedForm`, the shared gate+CSRF preamble for every admin write) were exercised only *indirectly* via the admin HTTP integration tests. Added `src/admin-nav.test.ts` (tests-first style, against the existing contract): `adminSection` (gated "Admin" header over the 4 screens, `current` marks+opens), `adminNav` (prepends Dashboard, role-filters the section — admin sees it, non-admin/anon get only Dashboard; asserts via `href` since composeNav strips `id` but keeps `current`), `requireAdmin` (anon→401→/login, non-admin→403, admin→user), `guardedForm` (valid double-submit→parsed body, missing/forged token→403, non-POST→undefined), `buildConfirmModel`. Only `server.ts` (entry-point composition root, exercised by every E2E boot) has no dedicated unit. 300 → 305 units; typecheck + tests green. Tests-only, no production code (no stability reviewer, per the §6/§7 test-cleanup precedent). - [x] **Playwright full E2E**: login (password + mocked SSO), menu filtering by role, users/groups/permissions CRUD, a plugin page, logout. → New browser-UI suite `e2e/full-flow.spec.ts` (`compose.e2e-full.yml`) — the real Playwright UI the earlier full-stack suites deferred here ("browser-UI login is owned by §8"). The themed login form posts straight to Kratos' action and cookies are host-scoped, so a tiny **stdlib reverse proxy** (`e2e/proxy.mjs`) fronts web + Kratos on **one origin** (the browser's only host), exactly like a prod reverse proxy; `ory/kratos/e2e-proxy.yml` points Kratos' base_url + every self-service URL at it, and Kratos runs `--dev` so cookies aren't marked Secure over http (Kratos marks them Secure for a non-loopback host like the gateway). Coverage (6 tests, all green): **password login** (themed form → Kratos → `/auth/complete` → dashboard); **mocked SSO** (a stdlib **mock OIDC provider** `e2e/mock-oidc.mjs` — RS256-signed id_token, nonce-bound single-use codes — wired via `SELFSERVICE_METHODS_OIDC_*` env + the committed claims jsonnet; click the provider button → auto-approve → identity created → signed in); **menu filtering by role** (the admin sees the gated Admin section + the plugin nav; anon/SSO-user don't); **users/groups/roles CRUD** (create → list → delete a user via the confirm step; create a group + role, each with a first member since a Keto set needs ≥1); the **permission-gated plugin page** (`/scheduling/shifts` renders the mock upstream's shifts in the native shell); **logout** (sign-out ends the session → back to /login, admin nav gone). **Found + fixed a real bug the E2E surfaced:** the SSO submit button sits in the same `
` as the `required` email/password fields, so clicking it tripped HTML5 validation ("Please fill out this field") and never submitted — added `formnovalidate` to the SSO buttons (`views/partials/auth-card.ejs`), tests-first (`auth-card.test.ts` + `app.test.ts`); password login still validates (separate button). Stability-reviewer run as a local PR: **APPROVE, no Critical/High** — every dev/insecure knob (`--dev`, `SECURE_COOKIES=false`, the OIDC provider + mock + proxy) is confined to the e2e overlay, base/prod compose unaffected; addressed its top follow-up (documented the file's parallel-safety invariant). typecheck + **305 units** green; full-flow **6/6 green on a clean stack**, then `down -v` torn down. README E2E section + suite count updated. - [x] E2E harness: bring up the full compose stack, seed Keto roles + a test identity, **tear down after**. → Delivered by the §8 full-flow harness (`compose.e2e-full.yml`): one `run` brings up the whole stack (Postgres + Kratos + Keto + the one-command **bootstrap** + web + the same-origin gateway + the reference plugin's mock upstream + the mock OIDC provider), the bootstrap **seeds the demo admin identity in Kratos + its Keto roles** (`admin` + the plugin's declared `scheduling:read`/`write` tokens), the browser suite runs against that seeded identity, and `docker compose … down -v` **tears everything down** (run live, 6/6 green, torn down). The §4 auth-refresh + §6 oauth-login suites use the same full-stack-up / seed / tear-down pattern; this completes it for the browser-UI flows. -- [x] Run the architecture and the product reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on the whole project (weighted to the §8 Testing & CI surfaces). **Fixed now (tests-first where code):** (1) **CRITICAL (arch)** — §8 is "Testing & **CI**" but the CI automation was missing: the gate was five hand-run commands with manual teardown. Added **`scripts/ci.sh`** — the whole gate in one reproducible command: a Playwright pin-lockstep check (M4) → typecheck → units (with a count-floor guard, L1) → the four E2E suites, each on its **own named fresh stack** with a guaranteed `down -v` and a non-zero exit on first failure (documented in README). **The gate immediately earned its keep:** it surfaced a latent bug — the auth-refresh suite's `web` inherited the §6 `web→hydra` dep, but the e2e overlays don't run Hydra with `--dev`, so Hydra refused its http issuer and never went healthy → the suite couldn't boot. Fixed by dropping Hydra from the auth suite's `web` deps (`!override`, mirroring the full suite — it never needed Hydra). (2) **Product 🔴 (doc correctness)** — the README **Status** note still claimed auth + Hydra handlers were "the roadmap"/unbuilt (materially false after §4/§6/§8); rewrote it to reflect built reality and dropped the now-false `_(planned)_` markers from the **Auth** + **MVP** headings + fixed their anchor links. (3) **Product 🟡** — the recovery flow existed but nothing linked to it from `/login` (an end-user lockout gap): added a login-only **"Forgot password?"** link (`flow-view.ts` `recoverHref` → `flow-body.ejs`, `.auth-aside` CSS), tests-first (flow-view unit + a full-flow E2E assertion). (4) **Product 🟡 (recurring §5/§6/§7)** — empty list tables rendered blank: added an empty-state row to `data-table.ejs` (overridable `emptyText`, colspan over all columns, only when the table has columns) + `.table-empty` CSS — fixes all five list surfaces at once; tests-first (data-table unit). (5) **M3 (docs)** — README Layout `e2e/` line + `e2e/package.json` description updated for the §8 full-flow suite + harness files. Stability-reviewer run as a local PR: **APPROVE (with nits), no Critical/High**; addressed both robustness nits (per-suite compose **project names** so a flaky teardown can't cross-contaminate; `|| true` on the pin/count greps so `set -e`+pipefail doesn't abort before the diagnostic) — and caught a bug they introduced (a `.` in the derived project name is invalid → sanitized with `tr`). **Corrected a reviewer error:** arch claimed the bootstrap service uses `restart: unless-stopped` (infinite crash-loop); it actually uses `restart: "on-failure:5"` (bounded retries for transient Ory-not-ready blips) — defensible, left as-is. typecheck + **306 units** green; **`scripts/ci.sh` green end-to-end** (visual 9 · auth 1 · oauth 2 · full 6, every stack torn down). **Deferred (reviewer-scoped):** the host **internal route-table** (fold the admin/oauth if-ladder in `app.ts` into one `{method,prefix,handler}` table, derive `RESERVED_PLUGIN_IDS`/`allowedMethods` — arch H1, widened by §6/§7) → **§9** (top structural item, raised urgency); **visual-parity (computed-style) checks for the admin + consent screens** (visual.spec only covers the dashboard — arch M2) → §9/polish; a **JWT key-rotation E2E** (L2) → ships with the §9 rotation-runbook item; `shifts.test.ts` deep `src/*` imports → the plugin-api barrel (L3) → the §8 test-cleanup item below; success-flash after writes (product 🟢) → deferred (stateless, known); a **CI-service workflow** (Actions YAML calling `scripts/ci.sh`) → needs the operator's CI platform + Docker-capable runners (the portable gate script is in place). +- [x] Run the architecture and the product reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on the whole project (weighted to the §8 Testing & CI surfaces). **Fixed now (tests-first where code):** (1) **CRITICAL (arch)** — §8 is "Testing & **CI**" but the CI automation was missing: the gate was five hand-run commands with manual teardown. Added **`scripts/ci.sh`** — the whole gate in one reproducible command: a Playwright pin-lockstep check (M4) → typecheck → units (with a count-floor guard, L1) → the four E2E suites, each on its **own named fresh stack** with a guaranteed `down -v` and a non-zero exit on first failure (documented in README). **The gate immediately earned its keep:** it surfaced a latent bug — the auth-refresh suite's `web` inherited the §6 `web→hydra` dep, but the e2e overlays don't run Hydra with `--dev`, so Hydra refused its http issuer and never went healthy → the suite couldn't boot. Fixed by dropping Hydra from the auth suite's `web` deps (`!override`, mirroring the full suite — it never needed Hydra). (2) **Product 🔴 (doc correctness)** — the README **Status** note still claimed auth + Hydra handlers were "the roadmap"/unbuilt (materially false after §4/§6/§8); rewrote it to reflect built reality and dropped the now-false `_(planned)_` markers from the **Auth** + **MVP** headings + fixed their anchor links. (3) **Product 🟡** — the recovery flow existed but nothing linked to it from `/login` (an end-user lockout gap): added a login-only **"Forgot password?"** link (`flow-view.ts` `recoverHref` → `flow-body.ejs`, `.auth-aside` CSS), tests-first (flow-view unit + a full-flow E2E assertion). (4) **Product 🟡 (recurring §5/§6/§7)** — empty list tables rendered blank: added an empty-state row to `data-table.ejs` (overridable `emptyText`, colspan over all columns, only when the table has columns) + `.table-empty` CSS — fixes all five list surfaces at once; tests-first (data-table unit). (5) **M3 (docs)** — README Layout `e2e/` line + `e2e/package.json` description updated for the §8 full-flow suite + harness files. Stability-reviewer run as a local PR: **APPROVE (with nits), no Critical/High**; addressed both robustness nits (per-suite compose **project names** so a flaky teardown can't cross-contaminate; `|| true` on the pin/count greps so `set -e`+pipefail doesn't abort before the diagnostic) — and caught a bug they introduced (a `.` in the derived project name is invalid → sanitized with `tr`). **Corrected a reviewer error:** arch claimed the bootstrap service uses `restart: unless-stopped` (infinite crash-loop); it actually uses `restart: "on-failure:5"` (bounded retries for transient Ory-not-ready blips) — defensible, left as-is. typecheck + **306 units** green; **`scripts/ci.sh` green end-to-end** (visual 9 · auth 1 · oauth 2 · full 6, every stack torn down). **Deferred (reviewer-scoped):** the host **internal route-table** (fold the admin/oauth if-ladder in `app.ts` into one `{method,prefix,handler}` table, derive `RESERVED_PLUGIN_IDS`/`allowedMethods` — arch H1, widened by §6/§7) → **§9** (top structural item, raised urgency); **visual-parity (computed-style) checks for the admin + consent screens** (visual.spec only covers the dashboard — arch M2) → §9/polish; a **JWT key-rotation E2E** (L2) → ships with the §9 rotation-runbook item; `shifts.test.ts` deep `src/*` imports → the plugin-api barrel (L3) → the §8 test-cleanup item below; success-flash after writes (product 🟢) → deferred (stateless, known); a **CI-service workflow** (Actions YAML calling `scripts/ci.sh`) → needs the operator's CI platform + Docker-capable runners (the portable gate script is in place). **Then re-ran both reviewers to convergence (5 rounds, until both returned zero new actionable findings).** Rounds 1–4 fixed, tests-first: bounded every outbound Ory `fetch` with a timeout (`src/fetch-timeout.ts` + `ORY_TIMEOUT_SEC`, default 5; also the http JWKS fetch) so a hung Ory can't park a handler; **anonymous on a gated plugin route now 303→`/login`** (was a dead-end 403; signed-in-without-role still 403); an already-signed-in user is sent home from `/login`/`/registration`; the **`onRequest` short-circuit now sets the fresh CSRF cookie**; `admin-users` malformed `:id` → 404 (was 500); **`parseJwks` validates key shape** (fails loud at load); **removed the dead `COOKIE_SECRET`** (loaded + enforced + documented but never read); documented `HYDRA_ADMIN_URL`; the admin **recovery** UI now shows the **code** (links to the public `/recovery`) instead of the browser-unreachable admin-API link; reference-plugin breadcrumb-label + pagination/datetime notes; corrected the contract doc to not over-promise a post-login "retry". **Declined:** unconditional base-ctx chrome (would build the menu per request, regressing the lazy hot path). **Deferred → §9:** `return_to`-preservation so a deep-link login lands on the requested page (non-trivial: must route through `/auth/complete` to mint the JWT without colliding with the OAuth2-challenge `return_to`). Stability-reviewer on the cumulative diff: **APPROVE, no Critical/High** (addressed its Low nits). typecheck + **310 units** + the full `scripts/ci.sh` gate green throughout. - [ ] 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. - [ ] 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. diff --git a/views/partials/user-form-body.ejs b/views/partials/user-form-body.ejs index 6706d87..e5cc60a 100644 --- a/views/partials/user-form-body.ejs +++ b/views/partials/user-form-body.ejs @@ -2,7 +2,7 @@ Admin user create/edit form body (todo §5), captured into the shell content slot. Config: form { action, csrfToken, submitLabel, cancelHref, fields: field.ejs config[] } edit? { nextLabel, stateAction, recoveryAction, deleteAction } (edit mode only) - recovery? { code?, link? } shown after a recovery link is generated + recovery? { code? } shown after a recovery code is generated (recovery is code-based) error? string shown when a write was rejected %><% const form = locals.form; @@ -14,7 +14,7 @@ <%- include("alert", { text: locals.error, tone: "neg" }) %> <% } -%> <% if (recovery) { -%> -
Recovery link generated<% if (recovery.link) { %><%= recovery.link %><% } %><% if (recovery.code) { %>Code: <%= recovery.code %><% } %>
+
Recovery code generatedGive it to the user — they enter it on the password-reset screen to set a new password (generate a fresh one if it has expired).<% if (recovery.code) { %><%= recovery.code %><% } %>
<% } -%> @@ -28,7 +28,7 @@
<% if (edit) { -%>
-
+
Delete user