diff --git a/CHANGELOG.md b/CHANGELOG.md index 6034fee..4fc38df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ at boot. Entries start at 0.3.0. ## Unreleased +### A verification or recovery code pasted with whitespace around it is accepted + +The code field on the verification and recovery pages takes a code with spaces or tabs around it. +Pasting a code from an email often adds them, and before, the browser refused the code. Those two +forms now post to Plainpages, which trims the code and passes the form on to Kratos at +`KRATOS_PUBLIC_URL`. A visitor who picked a language now keeps it through both flows. + ### A filter shows its selected and focused choice in browsers without `:has()` On Chromium below 105, Firefox below 121 and Safari below 15.4, a filter's segments and chips now show diff --git a/README.md b/README.md index 8e6b706..cfdb1ed 100644 --- a/README.md +++ b/README.md @@ -1522,7 +1522,7 @@ no Node/browsers on the host. Five suites, each on its own stack: | `visual.spec.ts` + `language.spec.ts` | `e2e-tests/compose.visual.yml` | Ory-free: the rendered design system (app shell, theme switch, mobile off-canvas, icon sprite, CSRF-guarded sign-out, landing, 404), plugin permission-gating, and [language switching](#languages-i18n) | | `auth-refresh.spec.ts` | `e2e-tests/compose.auth.yml` | The real Ory stack with the session→JWT TTL cut to 8s: a lapsed JWT is silently re-minted from the live Kratos session, and once revoked the stale cookie is cleared | | `oauth-login.spec.ts` | `e2e-tests/compose.oauth.yml` | Another app logging in through us — register a client, start an authorization flow, `/oauth2/login` accept, `/oauth2/consent` Allow → authorization code | -| `full-flow.spec.ts` | `e2e-tests/compose.full.yml` | The browser UI: password + mocked-SSO login, menu filtering by permission, admin users/groups/permissions CRUD, OAuth2-clients register → one-time secret → delete, a gated plugin page, logout | +| `full-flow.spec.ts` | `e2e-tests/compose.full.yml` | The browser UI: password + mocked-SSO login, emailed-code verification + recovery, menu filtering by permission, admin users/groups/permissions CRUD, OAuth2-clients register → one-time secret → delete, a gated plugin page, logout | | `devstack-login.spec.ts` | `compose.override.yml` + `e2e-tests/compose.devstack.yml` | The plain `docker compose up` topology on the **host network**: sign-in works both from the banner's `localhost:3000` and from `127.0.0.1:3000` via the [canonical-host redirect](#canonical-host-one-public-url) | ```bash diff --git a/docs/reading-cost.md b/docs/reading-cost.md index 15a4500..69e6c51 100644 --- a/docs/reading-cost.md +++ b/docs/reading-cost.md @@ -243,3 +243,17 @@ Read most (opens and searches): - `package.json` 2 - `public/css/styles.css` 2 - `ci.sh` 1 + +## Reading cost, chunk de26287532f7, finished 2026-10-31T11:29:32Z + +68410 new input tokens before the first commit. + +Read most (opens and searches): + +- `todo.md` 5 +- `public/css/styles.css` 3 +- `src/ui/stylesheet-floor.test.ts` 3 +- `CHANGELOG.md` 2 +- `README.md` 2 +- `AGENTS.md` 1 +- `src/ui/theme-switch.test.ts` 1 diff --git a/e2e-tests/compose.full.yml b/e2e-tests/compose.full.yml index 605eea0..de71fb0 100644 --- a/e2e-tests/compose.full.yml +++ b/e2e-tests/compose.full.yml @@ -1,5 +1,5 @@ # Full browser E2E — the real Playwright UI flow against the live stack: password + mocked-SSO -# login, menu filtering by permission, users/groups/OAuth2-clients CRUD + permission granting, a plugin page, logout. A +# login, emailed-code verification + recovery, menu filtering by permission, users/groups/OAuth2-clients CRUD + permission granting, a plugin page, logout. A # tiny same-origin gateway (proxy, e2e-tests/proxy.ts) fronts web + Kratos on one host so the browser's cookies # round-trip (ory/kratos/e2e-proxy.yml points Kratos at it); a mock OIDC provider backs the SSO test. # docker compose -f compose.yml -f e2e-tests/compose.full.yml run --user "$(id -u):$(id -g)" --build --rm e2e @@ -79,6 +79,17 @@ services: timeout: 4s retries: 15 + # Catches the courier's mail (kratos.yml → smtp://mailpit:1025), so a test can read an emailed code. + mailpit: + image: axllent/mailpit:v1.33.0 + mem_limit: 128m + memswap_limit: 128m + healthcheck: + test: ["CMD", "/mailpit", "readyz"] + interval: 2s + timeout: 4s + retries: 15 + # Same-origin gateway: Kratos-owned paths → kratos, everything else → web (e2e-tests/proxy.ts). proxy: image: node:24.22.2-alpine3.24 @@ -103,6 +114,8 @@ services: dockerfile: e2e-tests/Dockerfile command: ["npx", "playwright", "test", "full-flow.spec.ts"] depends_on: + mailpit: + condition: service_healthy mock-oidc: condition: service_healthy proxy: @@ -110,5 +123,6 @@ services: environment: BASE_URL: http://proxy KRATOS_ADMIN_URL: http://kratos:4434 + MAILPIT_URL: http://mailpit:8025 volumes: - ./e2e-tests/artifacts:/e2e-tests/artifacts diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index 0f822f7..3ae774e 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -14,6 +14,8 @@ const ADMIN_EMAIL = "admin@plainpages.local"; // seeded by bootstrap, holds the const ADMIN_PASSWORD = "admin"; const SSO_EMAIL = "sso-user@plainpages.local"; // minted by the mock OIDC provider on first SSO login const suffix = randomUUID().slice(0, 8); // unique per run so re-runs don't collide on names +const KRATOS_ADMIN = process.env.KRATOS_ADMIN_URL ?? "http://kratos:4434"; +const MAILPIT = process.env.MAILPIT_URL ?? "http://mailpit:8025"; // Drive the themed password login form → Kratos → /auth/complete → dashboard, signed in. async function loginPassword(page: Page): Promise { @@ -274,3 +276,54 @@ test("mocked SSO login: the provider button signs a user in via OIDC", async ({ // A fresh SSO identity holds no permissions, so the gated Admin section stays hidden. await expect(page.locator('.sidebar a[href="/admin/users"]')).toHaveCount(0); }); + +// The newest one-time code Kratos' courier mailed to `to`, read from the mail catcher (compose.full.yml). +async function mailedCode(to: string): Promise { + let code = ""; + await expect.poll(async () => { + const search = (await (await fetch(`${MAILPIT}/api/v1/search?query=${encodeURIComponent(`to:"${to}"`)}`)).json()) as { messages: { ID: string }[] }; + const id = search.messages[0]?.ID; + if (!id) return ""; + const message = (await (await fetch(`${MAILPIT}/api/v1/message/${id}`)).json()) as { Text: string }; + code = /\b(\d{6})\b/.exec(message.Text)?.[1] ?? ""; + return code; + }, { timeout: 30_000 }).not.toBe(""); + return code; +} + +async function createIdentity(email: string): Promise { + const created = await fetch(`${KRATOS_ADMIN}/admin/identities`, { + body: JSON.stringify({ schema_id: "default", traits: { email } }), + headers: { "content-type": "application/json" }, + method: "POST", + }); + expect(created.status).toBe(201); +} + +// Fills the email step, then pastes the mailed code with whitespace around it, as a paste often brings. +async function enterMailedCode(page: Page, email: string): Promise { + await page.fill('input[name="email"]', email); + await page.getByRole("button", { name: "Continue" }).click(); + await expect(page.locator('input[name="code"]')).toBeVisible(); + await page.fill('input[name="code"]', ` ${await mailedCode(email)}\t `); + await page.getByRole("button", { name: "Continue" }).click(); +} + +test("verification: a code pasted with whitespace around it is accepted", async ({ page }) => { + test.setTimeout(90_000); + const email = `verify-${suffix}@plainpages.local`; + await createIdentity(email); + await page.goto("/verification"); + await enterMailedCode(page, email); + await expect(page.getByText("You successfully verified your email address.")).toBeVisible(); +}); + +test("recovery: a pasted code signs in to the settings page, in the chosen language", async ({ page }) => { + test.setTimeout(90_000); + const email = `recover-${suffix}@plainpages.local`; + await createIdentity(email); + await page.goto("/recovery?locale=en-US"); + await enterMailedCode(page, email); + await expect(page).toHaveURL(/\/settings\?.*locale=en-US/); + await expect(page.locator("h1")).toHaveText("Account settings"); +}); diff --git a/src/auth/flow-view.test.ts b/src/auth/flow-view.test.ts index 7e9aef9..6cf8527 100644 --- a/src/auth/flow-view.test.ts +++ b/src/auth/flow-view.test.ts @@ -100,9 +100,7 @@ test("collects oidc nodes as SSO providers (text logo = initial), keeping csrf a assert.deepEqual(view.buttons, [{ label: "Sign in", name: "method", value: "password" }]); }); -test("the code field guards a pasted space: one-time-code autofill + numeric inputmode + digits-only pattern", () => { - // Verification/recovery enter a numeric OTP. Kratos doesn't trim, so a stray pasted space makes it - // reject the code as "invalid"; a digits-only pattern blocks that in the browser before submit. +test("the code field takes a pasted code: one-time-code autofill + numeric inputmode + digits, whitespace around them allowed", () => { const view = buildFlowView( flow([ node({ name: "csrf_token", type: "hidden", value: "tok" }), @@ -113,18 +111,26 @@ test("the code field guards a pasted space: one-time-code autofill + numeric inp ); assert.deepEqual(view.fields.find((f) => f.name === "code"), { autocomplete: "one-time-code", // Kratos sends none for the OTP node — enable OS/email autofill - hint: "Digits only — no spaces.", // the pattern refusal alone reads as a bare "match the requested format" + hint: "Digits only.", // the pattern refusal alone reads as a bare "match the requested format" icon: "i-shield", id: "field-code", inputmode: "numeric", label: "Verification code", name: "code", - pattern: "[0-9]*", + pattern: "\\s*[0-9]*\\s*", required: true, type: "text", }); }); +test("the code flows post to the host, which trims the code; every other flow posts to Kratos", () => { + for (const type of ["recovery", "verification"] as const) assert.equal(buildFlowView(flow([]), type).action, `/${type}?flow=f1`); + for (const type of ["login", "registration", "settings"] as const) { + assert.equal(buildFlowView(flow([]), type).action, "http://127.0.0.1:4433/self-service/login?flow=f1"); + } + assert.equal(buildFlowView({ ...flow([]), id: "a&b" }, "recovery").action, "/recovery?flow=a%26b"); +}); + test("chrome varies per flow type: registration alt, recovery back link", () => { const reg = buildFlowView(flow([]), "registration"); assert.equal(reg.title, "Create account"); diff --git a/src/auth/flow-view.ts b/src/auth/flow-view.ts index 8a8d677..da0ad25 100644 --- a/src/auth/flow-view.ts +++ b/src/auth/flow-view.ts @@ -1,8 +1,8 @@ // Kratos flow → themed view model. Pure: turns a fetched self-service Flow // (src/auth/kratos-public.ts) into the data views/auth.ejs renders — hidden inputs (incl. the // CSRF token), themed fields, submit buttons, tone-mapped messages, and one SSO button per -// configured `oidc` provider. The form posts straight back to `flow.ui.action`, so Kratos -// owns its CSRF; we only render and map errors. No providers configured ⇒ no SSO buttons. +// configured `oidc` provider. The form posts to `flow.ui.action`, except a CODE_FLOWS form, which +// posts to the host first; Kratos owns the CSRF of both. No providers configured ⇒ no SSO buttons. import { ENGLISH } from "../i18n/english.ts"; import type { Translate } from "../i18n/translate.ts"; @@ -17,7 +17,7 @@ export interface FlowField { inputmode?: string; // virtual-keyboard hint (e.g. "numeric" for the OTP code) label: string; name: string; - pattern?: string; // client-side validity regex; blocks a pasted space before it reaches Kratos + pattern?: string; // client-side validity regex required?: boolean; type: string; value?: string; @@ -69,6 +69,9 @@ export const AUTH_FLOWS: Record = { "/verification": "verification", }; +// The flows that take an emailed one-time code. Their form posts to the host, which trims the code. +export const CODE_FLOWS: ReadonlySet = new Set(["recovery", "verification"]); + // Where each flow's card links; its words come from the catalog under `auth..*`. const LINKS: Record = { login: { alt: "/registration" }, @@ -123,9 +126,9 @@ const ssoLogo = (value: string): string => (value.charAt(0) || "?").toUpperCase( function toField(node: UiNode, name: string, type: string, t: Translate): FlowField { const value = str(node.attributes["value"]); - // The recovery/verification one-time code: numeric, and Kratos doesn't trim it, so a stray pasted - // space makes it reject the code as "invalid". A digits-only pattern + numeric keypad block that in - // the browser; one-time-code enables OS/email autofill (Kratos sends no autocomplete for the node). + // The recovery/verification one-time code is numeric: a numeric keypad and a digits pattern. The + // pattern allows whitespace around the digits, as a paste brings; routes.ts → codeFlowSubmit trims it. + // one-time-code enables OS/email autofill (Kratos sends no autocomplete for the node). const isCode = name === "code"; const autocomplete = str(node.attributes["autocomplete"]) ?? (isCode ? "one-time-code" : undefined); const icon = iconFor(name, type); @@ -140,12 +143,16 @@ function toField(node: UiNode, name: string, type: string, t: Translate): FlowFi ...(autocomplete ? { autocomplete } : {}), ...(errorMsg ? { error: { text: kratosText(t, errorMsg.text, idKey(errorMsg.id)) } } : {}), ...(icon ? { icon } : {}), - ...(isCode ? { hint: t("auth.field.code.hint"), inputmode: "numeric", pattern: "[0-9]*" } : {}), + ...(isCode ? { hint: t("auth.field.code.hint"), inputmode: "numeric", pattern: "\\s*[0-9]*\\s*" } : {}), ...(node.attributes["required"] === true ? { required: true } : {}), ...(value ? { value } : {}), }; } +function formAction(flow: Flow, type: FlowType): string { + return CODE_FLOWS.has(type) ? `/${type}?flow=${encodeURIComponent(flow.id)}` : flow.ui.action; +} + export function buildFlowView(flow: Flow, type: FlowType, t: Translate = ENGLISH): FlowView { const hidden: { name: string; value: string }[] = []; const fields: FlowField[] = []; @@ -174,7 +181,7 @@ export function buildFlowView(flow: Flow, type: FlowType, t: Translate = ENGLISH } return { - action: flow.ui.action, + action: formAction(flow, type), buttons, fields, hidden, diff --git a/src/auth/kratos-public.test.ts b/src/auth/kratos-public.test.ts index 0aacd1d..2c24726 100644 --- a/src/auth/kratos-public.test.ts +++ b/src/auth/kratos-public.test.ts @@ -1,6 +1,6 @@ // Kratos public-API client: typed fetch wrappers over Ory Kratos' public endpoints. // Guards the request contracts (URLs, JSON-accept, cookie relay) and the result mapping -// (200/401/4xx, validation-flow vs success, tokenized JWT). Live wiring is verified by the +// (200/401/4xx, a submit's redirect, tokenized JWT). Live wiring is verified by the // flow pages; these catch contract drift with a mock fetch. import { test } from "node:test"; import assert from "node:assert/strict"; @@ -17,12 +17,13 @@ function res(status: number, body?: unknown, setCookie: string[] = []): Response // Records each call so a test can assert URL/method/headers/body. function recorder(handler: (url: string, init: RequestInit | undefined) => Response) { - const calls: { body: string | undefined; headers: Headers; method: string; url: string }[] = []; + const calls: { body: string | undefined; headers: Headers; method: string; redirect: string | undefined; url: string }[] = []; const fetchImpl = (async (input: unknown, init?: RequestInit) => { calls.push({ body: init?.body as string | undefined, headers: new Headers(init?.headers), method: init?.method ?? "GET", + redirect: init?.redirect, url: String(input), }); return handler(String(input), init); @@ -57,35 +58,28 @@ test("getFlow throws a KratosError carrying the status when the flow is gone (41 ); }); -test("submitFlow POSTs urlencoded to the action and reports success + relays Set-Cookie", async () => { - const { calls, fetchImpl } = recorder(() => res(200, { session: { active: true } }, ["plainpages_session=s; Path=/"])); +test("submitFlow POSTs the form to the flow on Kratos' own base URL, as a browser would, and relays the redirect + Set-Cookie", async () => { + const { calls, fetchImpl } = recorder(() => new Response(null, { + headers: new Headers([["location", "http://app/settings?flow=s1"], ["set-cookie", "plainpages_session=s; Path=/"]]), + status: 303, + })); const out = await createKratosPublic({ baseUrl: BASE, fetchImpl }) - .submitFlow(`${BASE}/self-service/login?flow=f`, { body: "identifier=a&password=b", cookie: "csrf_token=abc" }); - assert.equal(out.ok, true); - assert.equal(out.status, 200); - assert.deepEqual(out.setCookie, ["plainpages_session=s; Path=/"]); + .submitFlow("recovery", "f 1", { body: "code=123456&method=code", cookie: "csrf_token=abc" }); + assert.deepEqual(out, { location: "http://app/settings?flow=s1", setCookie: ["plainpages_session=s; Path=/"], status: 303 }); + assert.equal(calls[0]!.url, `${BASE}/self-service/recovery?flow=f+1`); assert.equal(calls[0]!.method, "POST"); + assert.equal(calls[0]!.redirect, "manual"); + assert.equal(calls[0]!.headers.get("accept"), "text/html"); + assert.equal(calls[0]!.headers.get("referer"), `${BASE}/`); assert.equal(calls[0]!.headers.get("content-type"), "application/x-www-form-urlencoded"); - assert.equal(calls[0]!.body, "identifier=a&password=b"); + assert.equal(calls[0]!.headers.get("cookie"), "csrf_token=abc"); + assert.equal(calls[0]!.body, "code=123456&method=code"); }); -test("submitFlow returns the re-rendered flow (no throw) on a 400 validation error", async () => { - const flow = { id: "f", ui: { action: "x", messages: [{ id: 4000006, text: "invalid credentials", type: "error" }], method: "POST", nodes: [] } }; - const { fetchImpl } = recorder(() => res(400, flow)); - const out = await createKratosPublic({ baseUrl: BASE, fetchImpl }).submitFlow(`${BASE}/x`, { body: "x=1" }); - assert.equal(out.ok, false); - assert.equal(out.status, 400); - assert.deepEqual(out.body, flow); -}); - -test("submitFlow surfaces the redirect target — Location header or a 422 redirect_browser_to body", async () => { - const k = (handler: () => Response) => createKratosPublic({ baseUrl: BASE, fetchImpl: recorder(handler).fetchImpl }); - const viaHeader = await k(() => new Response(null, { headers: new Headers({ location: "http://app/" }), status: 303 })) - .submitFlow(`${BASE}/x`, { body: "x=1" }); - assert.equal(viaHeader.location, "http://app/"); - const viaBody = await k(() => res(422, { redirect_browser_to: "http://app/login?flow=next" })) - .submitFlow(`${BASE}/x`, { body: "x=1" }); - assert.equal(viaBody.location, "http://app/login?flow=next"); +test("submitFlow reports a 4xx without a redirect as a null location, and throws on a Kratos 5xx", async () => { + const k = (status: number) => createKratosPublic({ baseUrl: BASE, fetchImpl: recorder(() => res(status, { error: "x" })).fetchImpl }); + assert.deepEqual(await k(400).submitFlow("verification", "f", { body: "x=1" }), { location: null, setCookie: [], status: 400 }); + await assert.rejects(k(500).submitFlow("verification", "f", { body: "x=1" }), (e: unknown) => e instanceof KratosError && e.status === 500); }); test("whoami returns the session on 200 (cookie forwarded) and null on 401", async () => { diff --git a/src/auth/kratos-public.ts b/src/auth/kratos-public.ts index d462094..4bd50e6 100644 --- a/src/auth/kratos-public.ts +++ b/src/auth/kratos-public.ts @@ -21,7 +21,7 @@ export interface UiNode { } export interface FlowUi { - action: string; // absolute Kratos URL the browser POSTs the form to (Kratos owns its CSRF) + action: string; // absolute Kratos URL the form submits to messages?: UiText[]; method: string; nodes: UiNode[]; @@ -51,9 +51,7 @@ export interface LogoutFlow { } export interface FlowSubmission { - body: unknown; // parsed JSON: the re-rendered flow on 400, the success payload on 200 - location: string | null; // redirect target (Location header, or a 422 redirect_browser_to) - ok: boolean; // status === 200 + location: string | null; // where Kratos sends the browser next setCookie: string[]; status: number; } @@ -74,23 +72,10 @@ export interface KratosPublic { createLogoutFlow(opts?: { cookie?: string }): Promise; // null ⇒ no active session (401) getFlow(type: FlowType, id: string, opts?: { cookie?: string }): Promise; initBrowserFlow(type: FlowType, opts?: { cookie?: string; returnTo?: string }): Promise; - submitFlow(action: string, opts: { body: string; contentType?: string; cookie?: string }): Promise; + submitFlow(type: FlowType, flowId: string, opts: { body: string; cookie?: string }): Promise; whoami(opts?: { cookie?: string; tokenizeAs?: string }): Promise; } -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} - -function parseBody(text: string): unknown { - if (!text) return null; - try { - return JSON.parse(text); - } catch { - return text; - } -} - export function createKratosPublic(config: { baseUrl: string; fetchImpl?: typeof fetch }): KratosPublic { const base = config.baseUrl.replace(/\/+$/, ""); const http = config.fetchImpl ?? fetch; @@ -129,16 +114,16 @@ export function createKratosPublic(config: { baseUrl: string; fetchImpl?: typeof return (await res.json()) as Flow; }, - async submitFlow(action, opts) { - const h = headers(opts.cookie); - h["content-type"] = opts.contentType ?? "application/x-www-form-urlencoded"; - // Manual redirect so we can read a 303 Location instead of following it server-side. - const res = await http(action, { body: opts.body, headers: h, method: "POST", redirect: "manual" }); - const body = parseBody(await res.text()); - const location = - res.headers.get("location") ?? - (isRecord(body) && typeof body["redirect_browser_to"] === "string" ? body["redirect_browser_to"] : null); - return { body, location, ok: res.status === 200, setCookie: res.headers.getSetCookie(), status: res.status }; + async submitFlow(type, flowId, opts) { + const url = new URL(`${base}/self-service/${type}`); + url.searchParams.set("flow", flowId); + // Asked for HTML, Kratos answers with the redirect a browser posting to it would get. Its CSRF + // check demands a same-origin Referer on an https request, and this server-side POST has none. + const h = { ...headers(opts.cookie), accept: "text/html", "content-type": "application/x-www-form-urlencoded", referer: `${base}/` }; + const res = await http(url, { body: opts.body, headers: h, method: "POST", redirect: "manual" }); + if (res.status >= 500) throw new KratosError(`Kratos submit ${type} flow failed (${res.status})`, res.status, await res.text()); + await res.body?.cancel(); + return { location: res.headers.get("location"), setCookie: res.headers.getSetCookie(), status: res.status }; }, async whoami(opts = {}) { diff --git a/src/auth/routes.test.ts b/src/auth/routes.test.ts index a4ac5c2..7d4f959 100644 --- a/src/auth/routes.test.ts +++ b/src/auth/routes.test.ts @@ -26,12 +26,14 @@ test("nothing wired ⇒ only the flow-error sink exists", () => { assert.deepEqual(keys(buildAuthRoutes(deps({}))), ["GET /error"]); }); -test("kratos wired ⇒ the themed flow pages + POST /logout; no OAuth2, no /auth/complete", () => { +test("kratos wired ⇒ the themed flow pages, the two code-flow submits + POST /logout; no OAuth2, no /auth/complete", () => { const got = keys(buildAuthRoutes(deps({ kratos }))); assert.deepEqual(got, keys([ ...Object.keys(AUTH_FLOWS).map((path) => ({ method: "GET", path })), { method: "GET", path: "/error" }, { method: "POST", path: "/logout" }, + { method: "POST", path: "/recovery" }, + { method: "POST", path: "/verification" }, ])); }); diff --git a/src/auth/routes.ts b/src/auth/routes.ts index ef764f4..5938edc 100644 --- a/src/auth/routes.ts +++ b/src/auth/routes.ts @@ -1,5 +1,5 @@ // The built-in auth/OAuth2 endpoints as named handlers on the host's internal route table -// (src/http/builtin-routes.ts): the themed Kratos self-service pages, the Hydra OAuth2 +// (src/http/builtin-routes.ts): the themed Kratos self-service pages and their code submits, the Hydra OAuth2 // login/consent/logout challenges, login completion, the first-party logout, and Kratos' // flow-error sink. buildAuthRoutes registers only what the wired clients support — a missing // client means the path simply doesn't exist (404). `view` results render the core views. @@ -8,11 +8,11 @@ import type { BuiltinRoute, RequestCsrf } from "../http/builtin-routes.ts"; import type { RequestContext } from "../http/context.ts"; import { CSRF_FIELD } from "./csrf.ts"; import { chosenLocale } from "../i18n/locale.ts"; -import { AUTH_FLOWS, buildFlowView } from "./flow-view.ts"; +import { AUTH_FLOWS, buildFlowView, CODE_FLOWS } from "./flow-view.ts"; import { HydraError, type HydraAdmin } from "./hydra-admin.ts"; import type { KetoClient } from "./keto-client.ts"; import type { KratosAdmin } from "./kratos-admin.ts"; -import { type Flow, type FlowType, KratosError, type KratosPublic } from "./kratos-public.ts"; +import { type Flow, type FlowSubmission, type FlowType, KratosError, type KratosPublic } from "./kratos-public.ts"; import { clearSessionCookie, completeLogin, sessionCookie } from "./login.ts"; import type { MenuConfig } from "../ui/menu-config.ts"; import { acceptConsent, rejectConsent, resolveConsentChallenge } from "./oauth-consent.ts"; @@ -114,6 +114,33 @@ function flowPage(kratos: KratosPublic, flowType: FlowType, secureCookies: boole }; } +// A code flow's POST (flow-view.ts → CODE_FLOWS). Trims the pasted one-time code, since Kratos +// refuses one with whitespace around it, then passes the form on to Kratos and sends the browser +// where Kratos redirects. Kratos' csrf_token field guards the POST, so the host's own CSRF token +// isn't checked here. +function codeFlowSubmit(kratos: KratosPublic, flowType: FlowType, secureCookies: boolean): BuiltinRoute["handler"] { + return async (ctx: RequestContext): Promise => { + const pathname = ctx.url.pathname; + const flowId = ctx.url.searchParams.get("flow"); + if (!flowId) return { redirect: pathname }; + const form = await readFormBody(ctx.req); + const code = form.get("code"); + if (code !== null) form.set("code", code.trim()); + const cookie = ctx.req.headers.cookie; + let submitted: FlowSubmission; + try { + submitted = await kratos.submitFlow(flowType, flowId, { body: form.toString(), ...(cookie ? { cookie } : {}) }); + } catch (err) { + return flowFailure(err, ctx, flowType); + } + if (submitted.setCookie.length) ctx.res.appendHeader("set-cookie", submitted.setCookie); + // Kratos names our own pages by absolute URL; made host-relative, the redirect keeps a chosen locale. + const own = selfOrigin(ctx, secureCookies); + const location = submitted.location?.startsWith(`${own}/`) ? submitted.location.slice(own.length) : submitted.location; + return { redirect: location ?? `${pathname}?flow=${encodeURIComponent(flowId)}` }; + }; +} + // OAuth2 login challenge: Hydra hands the browser here when another app logs in *through* us. // Resolve it via the Kratos session and accept; an unauthenticated user bounces to our themed // login and returns here once signed in. Provider-only. @@ -250,6 +277,7 @@ export function buildAuthRoutes({ hydra, keto, kratos, kratosAdmin, menu, secure if (kratos) { for (const [path, flowType] of Object.entries(AUTH_FLOWS)) { routes.push({ handler: flowPage(kratos, flowType, secureCookies), method: "GET", path, public: true }); + if (CODE_FLOWS.has(flowType)) routes.push({ handler: codeFlowSubmit(kratos, flowType, secureCookies), method: "POST", path, public: true }); } routes.push({ handler: logout(kratos, secureCookies), method: "POST", path: "/logout", public: true }); } diff --git a/src/http/app-auth.test.ts b/src/http/app-auth.test.ts index 67aad67..1d68273 100644 --- a/src/http/app-auth.test.ts +++ b/src/http/app-auth.test.ts @@ -6,7 +6,7 @@ import { CSRF_COOKIE, issueCsrfToken } from "../auth/csrf.ts"; import { HydraError, type HydraAdmin } from "../auth/hydra-admin.ts"; import { staticJwks } from "../auth/jwks.ts"; import type { Identity } from "../auth/kratos-admin.ts"; -import { KratosError, type Flow, type FlowType, type KratosPublic, type Session, type UiNode } from "../auth/kratos-public.ts"; +import { KratosError, type Flow, type FlowSubmission, type FlowType, type KratosPublic, type Session, type UiNode } from "../auth/kratos-public.ts"; import { SESSION_COOKIE } from "../auth/login.ts"; import { createI18n } from "../i18n/runtime.ts"; import { loadI18n } from "../i18n/load.ts"; @@ -389,3 +389,51 @@ test("OAuth2 challenge endpoints degrade identically: stale Hydra 4xx → 400, o } }); +test("code-flow POST: the host trims the code, relays the POST to Kratos and Kratos' redirect + cookies back", async (t) => { + const calls: { body: string; cookie: string | undefined; flowId: string; type: FlowType }[] = []; + let answer: () => Promise = async () => + ({ location: "http://localhost:3000/settings?flow=s1", setCookie: ["plainpages_session=s; Path=/; HttpOnly"], status: 303 }); + const kratos: KratosPublic = { + ...mockKratos(async () => { throw new Error("unused"); }), + submitFlow: async (type, flowId, opts) => { calls.push({ body: opts.body, cookie: opts.cookie, flowId, type }); return answer(); }, + }; + const app = createApp({ jwks: staticJwks([ecJwk]), kratos }); + await new Promise((r) => app.listen(0, r)); + t.after(() => app.close()); + const url = `http://localhost:${(app.address() as AddressInfo).port}`; + const post = (path: string, body: string) => fetch(url + path, { + body, headers: { "content-type": "application/x-www-form-urlencoded", cookie: "csrf_token_x=c" }, method: "POST", redirect: "manual", + }); + + const res = await post("/recovery?flow=f1", "csrf_token=tok&code=%C2%A0123456%09%0A&method=code"); + assert.equal(res.status, 303); + assert.equal(res.headers.get("location"), "http://localhost:3000/settings?flow=s1"); + assert.match(res.headers.get("set-cookie") ?? "", /plainpages_session=s/); + assert.deepEqual(calls[0], { body: "csrf_token=tok&code=123456&method=code", cookie: "csrf_token_x=c", flowId: "f1", type: "recovery" }); + + // The first step has no code field; its body passes through unchanged. + await post("/verification?flow=f2", "csrf_token=tok&email=a%40b.c&method=code"); + assert.deepEqual(calls[1], { body: "csrf_token=tok&email=a%40b.c&method=code", cookie: "csrf_token_x=c", flowId: "f2", type: "verification" }); + + // No ?flow= → start over rather than post a flow-less submit. + const noFlow = await post("/recovery", "code=1"); + assert.equal(noFlow.headers.get("location"), "/recovery"); + assert.equal(calls.length, 2); + + // Kratos' absolute redirect to this host becomes host-relative, so it keeps the chosen locale. + answer = async () => ({ location: `${url}/settings?flow=s1`, setCookie: [], status: 303 }); + assert.equal((await post("/recovery?flow=f1&locale=en-US", "code=1")).headers.get("location"), "/settings?flow=s1&locale=en-US"); + + // Kratos answered without a redirect → back to the flow, whose page shows its state. + answer = async () => ({ location: null, setCookie: [], status: 400 }); + assert.equal((await post("/recovery?flow=f1", "code=1")).headers.get("location"), "/recovery?flow=f1"); + + // Kratos down → the honest 503 the flow pages render, not a 500. + answer = async () => { throw new KratosError("bad gateway", 502, ""); }; + assert.equal((await post("/recovery?flow=f1", "code=1")).status, 503); + answer = async () => { throw new TypeError("fetch failed"); }; + assert.equal((await post("/recovery?flow=f1", "code=1")).status, 503); + + // Sign-in posts straight to Kratos, so the host has no POST /login. + assert.notEqual((await post("/login?flow=f1", "x=1")).status, 303); +}); diff --git a/src/i18n/locales/en-US.ts b/src/i18n/locales/en-US.ts index 471eb78..81e12e6 100644 --- a/src/i18n/locales/en-US.ts +++ b/src/i18n/locales/en-US.ts @@ -9,7 +9,7 @@ const messages = { "auth.continue": "Continue", // Kratos labels its own form fields; these translate the ones the built-in identity schema uses, // keyed on the input name. A deployment's extra traits keep Kratos' label until a plugin covers them. - "auth.field.code.hint": "Digits only — no spaces.", + "auth.field.code.hint": "Digits only.", "auth.field.email": "Email", "auth.field.identifier": "Email", "auth.field.password": "Password", diff --git a/src/i18n/locales/sv-SE.ts b/src/i18n/locales/sv-SE.ts index 85487eb..d36d850 100644 --- a/src/i18n/locales/sv-SE.ts +++ b/src/i18n/locales/sv-SE.ts @@ -2,7 +2,7 @@ import type { CoreMessages } from "./en-US.ts"; const messages: CoreMessages = { "auth.continue": "Fortsätt", - "auth.field.code.hint": "Endast siffror — inga mellanslag.", + "auth.field.code.hint": "Endast siffror.", "auth.field.email": "E-postadress", "auth.field.identifier": "E-postadress", "auth.field.password": "Lösenord", diff --git a/todo.md b/todo.md index 8022d7f..9c1dcb5 100644 --- a/todo.md +++ b/todo.md @@ -6,13 +6,13 @@ `Bar = 1` -`Next ID = 45` +`Next ID = 46` ## Items | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| -| 13 | 0.4.21 | | | **Accept a verification code pasted with surrounding whitespace.** | 4 | 2 | 3 | 7 | 7 | 2 by 8 | 1.0 | +| 45 | 0.4.21 | | | **Link the admin's recovery-code notice to the flow the code was minted on, so the user can enter it.** | 1 | 2 | 2 | 3 | 2 | 2 by 5 | 7.0 | | 19 | 0.5.0 | | question | **Run the documented Docker commands on macOS and fix whatever misbehaves.** | 2 | 2 | 1 | 3 | 3 | 1 by 3 | 3.3 | | 16 | 0.5.0 | | question | **Decide whether the single Keto `Resource` namespace splits into per-domain namespaces.** | 3 | 2 | 3 | 6 | 6 | 7 by 2 | 0.5 | | 6 | 0.5.0 | | | **Warn at discovery when a route or nav node gates on a permission no plugin declares.** | 2 | 1 | 1 | 2 | 2 | 7 by 3, 9 by 3 | 8.0 | @@ -26,17 +26,12 @@ ## Details -### 13. Accept a verification code pasted with surrounding whitespace. +### 45. Link the admin's recovery-code notice to the flow the code was minted on, so the user can enter it. -A pasted code fails the browser's `pattern` check. No zero-JS fix exists: `pattern` -cannot transform, so loosening it only lets the untrimmed value through to Kratos. **Check first:** -`kratos-admin.ts` returns a `recovery_link` alongside the code; if the stock courier mail can carry -one, a template change fixes this with no host code. Otherwise proxy the flow POST through the host: -`submitFlow` (`src/auth/kratos-public.ts`) already relays cookies and normalises a 422 -`redirect_browser_to`, and has no callers outside tests. CSRF improves, since the POST becomes -same-origin and `form-action 'self'` can join the CSP. Risks: one `flow.ui.action` serves all five -flows, so this rewrites the sign-in path; and Go's nosurf checks Referer only over https, which no -test covers. Code entry has no E2E coverage today. +The notice (`examples/plugins/admin/views/partials/user-form-body.ejs`) links a bare `/recovery`, +which starts a fresh flow at its email step. Kratos binds an admin-minted code to the flow it was minted on, +whose URL is the `recovery_link` that `createRecoveryCode` returns as `RecoveryCode.link`. Render that +link instead. ### 19. Run the documented Docker commands on macOS and fix whatever misbehaves. diff --git a/views/auth.ejs b/views/auth.ejs index 32d2fec..9bfae0b 100644 --- a/views/auth.ejs +++ b/views/auth.ejs @@ -1,9 +1,9 @@ <%# Themed Kratos self-service page inside the unified app shell: sign-in / register / reset / verify / settings. Renders a FlowView (src/auth/flow-view.ts) into the shell content, reusing the - auth-card + field partials. The form posts straight to flow.ui.action — Kratos owns its CSRF. The - shell's menu is permission-filtered (anonymous ⇒ public items + Sign in); the topbar carries no heading, - so the card's own

is the page's single heading. Data: chrome (PageChrome), flow (FlowView). + auth-card + field partials. The shell's menu is permission-filtered (anonymous ⇒ public items + + Sign in); the topbar carries no heading, so the card's own

is the page's single + heading. Data: chrome (PageChrome), flow (FlowView). %><% const nav = include("partials/nav-tree", { nodes: chrome.nav }); const card = include("partials/auth-card", {