diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index bab89a0..45efc09 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -74,6 +74,25 @@ test.describe.serial("authenticated admin journey", () => { await expect(page.locator("html")).toHaveAttribute("lang", "sv-SE"); }); + // A POST that re-renders a page: the write must keep the language, and the picker must not offer + // a link to a URL that only answers POST. + test("a write keeps the visitor's language, and offers no language link on the POST-rendered page", async () => { + await page.goto("/admin/users?locale=sv-SE"); + await page.getByRole("link", { name: "Ny användare" }).click(); + await page.fill('input[name="email"]', `lang-${suffix}@plainpages.local`); + await page.getByRole("button", { name: "Skapa användare" }).click(); + await expect(page).toHaveURL(/locale=sv-SE/); // the POST → redirect → GET keeps it + await expect(page.locator("html")).toHaveAttribute("lang", "sv-SE"); + + // Open the new user's edit page the way the CRUD test does — the row's Edit link carries the id. + const row = page.locator("tr", { hasText: `lang-${suffix}@plainpages.local` }); + const editHref = await row.locator('a[href^="/admin/users/"]').first().getAttribute("href"); + await page.goto(`${editHref}`); + await page.getByRole("button", { name: "Skapa återställningskod" }).click(); // POST-only route + await expect(page.getByText("Återställningskod skapad")).toBeVisible(); + await expect(page.locator('summary[aria-label="Språk"]')).toHaveCount(0); // no dead-end link offered + }); + test("menu filters by permission: an admin sees the gated Admin section + the plugin", async () => { // The signed-in admin holds admin + scheduling:read/write, so both gated sections are present // in the menu (collapsed by default → assert they're in the DOM, not necessarily visible). diff --git a/examples/plugins/admin/views/partials/confirm-body.ejs b/examples/plugins/admin/views/partials/confirm-body.ejs index 84a1e51..8b5cbca 100644 --- a/examples/plugins/admin/views/partials/confirm-body.ejs +++ b/examples/plugins/admin/views/partials/confirm-body.ejs @@ -11,7 +11,7 @@

<%= locals.message %>

<%= t("common.cancel") %> -
+
diff --git a/src/http/app.ts b/src/http/app.ts index 592d84e..5a7d741 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -14,7 +14,7 @@ import { GuardError, loginRedirect } from "../auth/guards.ts"; import { ENGLISH_I18N } from "../i18n/english.ts"; import type { I18n } from "../i18n/runtime.ts"; import { localeHref } from "../i18n/locale.ts"; -import { ENGLISH_LOCALS, i18nLocals } from "../i18n/view-locals.ts"; +import { ENGLISH_LOCALS, i18nLocals, type I18nRequest } from "../i18n/view-locals.ts"; import { runRequestHooks, runResponseHooks } from "../plugin-host/hooks.ts"; import type { HydraAdmin } from "../auth/hydra-admin.ts"; import type { JwksProvider } from "../auth/jwks.ts"; @@ -120,8 +120,9 @@ export function createApp(options: AppOptions = {}): Server { // it. A plugin's context carries that plugin's translator, so its own catalog wins in its own views. // They are merged LAST: these names are reserved (README → Building plugins), and a handler that // happens to use one loses that key rather than breaking the shell that renders around it. - const viewsFor = (ctx: RequestContext): ViewRenderer => (view, data) => render(view, { ...data, ...i18nLocals(ctx) }); - const pluginViewsFor = (ctx: RequestContext, id: string): ViewRenderer => (view, data) => renderView(id, view, { ...data, ...i18nLocals(ctx) }); + const localsOf = (ctx: RequestContext): I18nRequest => ({ ...ctx, method: ctx.req.method ?? "GET" }); + const viewsFor = (ctx: RequestContext): ViewRenderer => (view, data) => render(view, { ...data, ...i18nLocals(localsOf(ctx)) }); + const pluginViewsFor = (ctx: RequestContext, id: string): ViewRenderer => (view, data) => renderView(id, view, { ...data, ...i18nLocals(localsOf(ctx)) }); const sendHtml = (res: ServerResponse, status: number, html: string): void => { res.writeHead(status, { "content-type": "text/html; charset=utf-8" }); @@ -138,7 +139,7 @@ export function createApp(options: AppOptions = {}): Server { // The plugin owns this page, so it runs on its own context — its catalog first, then core. const pluginCtx = contextFor(homePlugin.id); const result = (await homePlugin.home(pluginCtx)) ?? null; - if (anyResponseHooks) await runResponseHooks(plugins, pluginCtx, result); + if (anyResponseHooks) await runResponseHooks(plugins, contextFor, result); await sendResult(ctx.res, result, pluginViewsFor(pluginCtx, homePlugin.id), pluginCtx.localeHref); return null; } @@ -156,7 +157,7 @@ export function createApp(options: AppOptions = {}): Server { if (dashboardPlugin) { const pluginCtx = contextFor(dashboardPlugin.id); // as serveHome: the owner's own translator const result = (await dashboardPlugin.dashboard(pluginCtx)) ?? null; - if (anyResponseHooks) await runResponseHooks(plugins, pluginCtx, result); + if (anyResponseHooks) await runResponseHooks(plugins, contextFor, result); await sendResult(ctx.res, result, pluginViewsFor(pluginCtx, dashboardPlugin.id), pluginCtx.localeHref); return null; } @@ -186,9 +187,6 @@ export function createApp(options: AppOptions = {}): Server { // Set before any branch so every response — static/redirect/error included — inherits them // (writeHead merges these with its own headers; a plugin's RouteResult.headers can override). for (const [name, value] of secHeaderEntries) res.setHeader(name, value); - // The same URL renders in different languages depending on Accept-Language, so a cache in - // front of us must key on it — otherwise the first visitor's language is served to everyone. - res.setHeader("vary", "accept-language"); if (pathname.startsWith("/public/") && (method === "GET" || method === "HEAD")) { // /public//… serves a plugin's public/; everything else the core public/. @@ -198,6 +196,12 @@ export function createApp(options: AppOptions = {}): Server { return; } + // Rendered pages content-negotiate on Accept-Language, so a cache in front of us must key on + // it — otherwise the first visitor's language is served to everyone. Set after the static + // branch above: an asset is the same bytes in every language, and a Vary there would fragment + // its cache entry per raw header string. + res.setHeader("vary", "accept-language"); + // Canonical host (APP_URL): a visitor who reached us on a different host (localhost vs // 127.0.0.1, a secondary domain) is sent to the configured origin, path + query preserved, so // the browser, the themed forms, and the cross-origin Kratos POST all share one cookie host — @@ -303,7 +307,7 @@ export function createApp(options: AppOptions = {}): Server { } csrfMint.setCookie(); const result = (await match.route.handler(routeCtx)) ?? null; - if (anyResponseHooks) await runResponseHooks(plugins, routeCtx, result); // observers; a throw → 500 + if (anyResponseHooks) await runResponseHooks(plugins, contextFor, result); // observers; a throw → 500 await sendResult(res, result, pluginViewsFor(routeCtx, match.plugin.id), carryLocale); return; } diff --git a/src/i18n/load.ts b/src/i18n/load.ts index 7faee39..76be109 100644 --- a/src/i18n/load.ts +++ b/src/i18n/load.ts @@ -24,7 +24,7 @@ export const MOUNTED_LOCALES_DIR = join(rootDir, "locales"); // A catalog file is named for the full locale it holds — sv-SE.ts, never sv.ts — with the script // subtag when the language needs one (sr-Latn-RS). Anything else in the folder is a mistake worth // stopping for. -const LOCALE_FILE = /^([a-z]{2,3}(?:-[A-Z][a-z]{3})?-[A-Z]{2})\.ts$/; +const LOCALE_FILE = /^([a-z]{2,3}(?:-[A-Z][a-z]{3})?-(?:[A-Z]{2}|[0-9]{3}))\.ts$/; export interface LoadI18nOptions { localesDir?: string; @@ -90,7 +90,7 @@ async function readSet(dir: string, label: string, errors: string[]): Promise-.ts (e.g. sv-SE.ts)`); + errors.push(`${label}: "${entry.name}" is not a locale catalog — name it -.ts (sv-SE.ts, es-419.ts, sr-Latn-RS.ts)`); continue; } let mod: { default?: unknown }; diff --git a/src/i18n/view-locals.test.ts b/src/i18n/view-locals.test.ts index c29345b..0df872c 100644 --- a/src/i18n/view-locals.test.ts +++ b/src/i18n/view-locals.test.ts @@ -8,6 +8,7 @@ const request = (overrides: Partial = {}): I18nRequest => ({ locale: "sv-SE", localeHref: (href) => href, locales: ["en-US", "sv-SE"], + method: "GET", t: ENGLISH, url: new URL("http://localhost/admin/users?q=ada"), ...overrides, @@ -30,3 +31,9 @@ test("dir follows the locale's script", () => { assert.equal(i18nLocals(request()).dir, "ltr"); assert.equal(i18nLocals(request({ locale: "ar-EG" })).dir, "rtl"); }); + +test("a page rendered from a POST offers no language links — that URL may have no GET at all", () => { + // Following one would dead-end on a 405 (a POST-only route), or silently discard a re-rendered + // form's input. The picker renders nothing below two choices, so an empty list hides it. + assert.deepEqual(i18nLocals(request({ method: "POST" })).localeSwitch, []); +}); diff --git a/src/i18n/view-locals.ts b/src/i18n/view-locals.ts index a76415c..f16f1ef 100644 --- a/src/i18n/view-locals.ts +++ b/src/i18n/view-locals.ts @@ -32,6 +32,7 @@ export interface I18nRequest { locale: string; localeHref: (href: string) => string; locales: string[]; + method: string; // a page rendered in response to a POST has no linkable URL — see localeSwitch t: Translate; url: URL; } @@ -50,6 +51,10 @@ export const ENGLISH_LOCALS: I18nLocals = { export function i18nLocals(ctx: I18nRequest): I18nLocals { const here = `${ctx.url.pathname}${ctx.url.search}`; + // The picker links to this same page in another language. After a POST that page's URL often has + // no GET at all (the admin's recovery-code screen, say), so linking there would dead-end on a 405 + // — and on a re-rendered form it would silently discard what the user typed. Offer nothing. + const linkable = ctx.method === "GET" || ctx.method === "HEAD"; // ctx.localeHref is a no-op unless the URL asked for a locale, so it is also the honest answer to // "did it?" — asking the function that decides keeps the two from drifting apart. const carried = ctx.localeHref("/") === "/" ? null : ctx.locale; @@ -58,7 +63,7 @@ export function i18nLocals(ctx: I18nRequest): I18nLocals { locale: ctx.locale, localeHref: (href) => ctx.localeHref(href), localeParam: carried, - localeSwitch: ctx.locales.map((tag) => ({ current: tag === ctx.locale, href: localeHref(here, tag), label: localeLabel(tag), tag })), + localeSwitch: linkable ? ctx.locales.map((tag) => ({ current: tag === ctx.locale, href: localeHref(here, tag), label: localeLabel(tag), tag })) : [], locales: ctx.locales, t: ctx.t, }; diff --git a/src/plugin-host/hooks.test.ts b/src/plugin-host/hooks.test.ts index 1acb126..bc91a9a 100644 --- a/src/plugin-host/hooks.test.ts +++ b/src/plugin-host/hooks.test.ts @@ -44,11 +44,12 @@ test("runRequestHooks short-circuits on the first RouteResult (with its plugin); test("runResponseHooks runs every onResponse as an observer with the result; a throw fails", async () => { const seen: unknown[] = []; + const contextFor = () => ctx; // each observer gets a context scoped to its own plugin await runResponseHooks([ plugin("a", { onResponse: (_c, r) => void seen.push(r) }), plugin("b", {}), // no onResponse → skipped - ], ctx, { html: "ok" }); + ], contextFor, { html: "ok" }); assert.deepEqual(seen, [{ html: "ok" }]); - await assert.rejects(runResponseHooks([plugin("x", { onResponse: () => { throw new Error("boom"); } })], ctx, null), /boom/); + await assert.rejects(runResponseHooks([plugin("x", { onResponse: () => { throw new Error("boom"); } })], contextFor, null), /boom/); }); diff --git a/src/plugin-host/hooks.ts b/src/plugin-host/hooks.ts index ceae572..04fd1c6 100644 --- a/src/plugin-host/hooks.ts +++ b/src/plugin-host/hooks.ts @@ -29,7 +29,14 @@ export async function runRequestHooks( } // After a route handler produces its result. Observers only — the return value is ignored, so a -// hook cannot change the response; a throw fails the request. -export async function runResponseHooks(plugins: Plugin[], ctx: RequestContext, result: RouteResult | null): Promise { - for (const plugin of plugins) await plugin.hooks?.onResponse?.(ctx, result); +// hook cannot change the response; a throw fails the request. Each observer gets a context scoped to +// its own plugin, like onRequest, so `ctx.t` is never another plugin's translator. +export async function runResponseHooks( + plugins: Plugin[], + contextFor: (pluginId: string) => RequestContext, + result: RouteResult | null, +): Promise { + for (const plugin of plugins) { + if (plugin.hooks?.onResponse) await plugin.hooks.onResponse(contextFor(plugin.id), result); + } } diff --git a/todo.md b/todo.md index 746197e..6f4130c 100644 --- a/todo.md +++ b/todo.md @@ -18,6 +18,7 @@ - [x] The human developer understands the security model in the auth in this project. (Two README sections. [Users, groups & permissions](README.md#users-groups--permissions) carries the weight: the entity model, a worked graph, a per-route can/cannot walkthrough, and the trap that a per-row grant never widens a coarse gate — placed before Building plugins because a manifest's `permission:` gate is unreadable without it. [Security model](README.md#security-model) is deliberately short, only the facts a deployment gets wrong without them: the private network as the *only* guard on the Ory APIs, signed-not-encrypted claims, the 30-day Kratos session behind the ~10m JWT, and non-instant offboarding. The first attempt answered the *threat* model instead — a 12-row attack/defense table — which was the wrong question and mostly restated code readable at its source; cut. Also corrected the hardening checklist: `REQUIRE_SECURE_SECRETS` guards only `CSRF_SECRET`, so the committed Kratos/Hydra/Postgres/demo-admin secrets are now listed in "What you must supply". The mandatory-`exp` guard gained a test in `src/auth/jwt-middleware.test.ts`.) - [x] Add i18n support. (Catalogs are TS modules per locale — `src/i18n/locales/.ts` for the host, `plugins//i18n/.ts` for a plugin, looked up plugin-first then core; en-US + sv-SE ship. A request is served by `?locale=sv-SE` → `Accept-Language` → `en-US`, exact on a full tag but a lone language takes the first regional catalog; no cookie — when the URL asked, the host carries `?locale` onto the links it renders and `ctx.localeHref()` does it for a plugin's. `ctx.t(key, vars)` plus `t`/`locale`/`locales`/`localeHref`/`dir` merged into every view (any include depth); `{{var}}` interpolation, plurals via `Intl.PluralRules`, an unknown key renders as itself — which is what makes a nav label either a key or plain text. Every catalog is checked against its set's en-US at boot (keys, kind, plural categories) and a mismatch stops startup. Kratos' own flow text is mapped by its numeric id (only ids verified against the live stack; its generic trait-label id is deliberately unmapped, field labels key on the input name instead). Zero-JS language picker in the shell + the auth/consent pages, `` from the locale. Core, both example plugins and their views translated; unit tests + `e2e-tests/language.spec.ts` in the visual gate; documented in README → Languages, decisions in AGENTS.md.) - [x] Settle the identity-vs-user vocabulary. (Plainpages says **user** everywhere — Keto namespace `User`, subjects `user:`, `ctx.user`. Ory calls the record an "identity", but its own docs say it uses that term interchangeably with "users"/"accounts", so this is house style rather than a renamed concept, and "user" is the word readers know (Nielsen heuristic #2). README → Auth carries one note recording the mapping; the only place Ory's spelling survives is the `Identity` DTO in `src/auth/kratos-admin.ts`, which mirrors the Kratos wire shape. Recorded in AGENTS.md.) +- [ ] Decide the caching contract for rendered pages. Responses now carry `Vary: Accept-Language` (they content-negotiate), but nothing sets `Cache-Control` — so a shared cache in front of the app has no instruction, and a signed-in page is not marked `private`. Pre-existing, surfaced by the i18n review 2026-08-03: either set the headers deliberately (public pages cacheable, gated pages `private, no-store`) or record in AGENTS.md that the reverse proxy owns this. - [ ] Decide whether the single generic Keto `Resource` namespace should become per-domain namespaces (`Shift`, `Document`, …), as Ory's own examples model it. One global `Resource` bucket is the project's own "no catch-all names" rule (`utils`, `helpers`, `misc`) applied to namespaces. Raised 2026-08-03; a design question, not a naming one. - [ ] Decide (once) whether the CSRF token staying unbound to `sub`/session is accepted. `src/auth/csrf.ts` signs `.` with no session binding, so any validly-signed token passes for any user — an attacker who can write cookies on the origin (a sibling subdomain, or a plaintext hop with `SECURE_COOKIES=false`) can fix a token they know. Standard for unbound signed double-submit and plausibly fine behind `SameSite=Lax` + HSTS. Accepted ⇒ record it in AGENTS.md → "Deliberate architectural deviations" and in README → Security model under "Not guaranteed"; not accepted ⇒ bind the nonce to `sub` (small change). Raised by review 2026-08-02; left undecided because it is a maintainer call, and an undocumented exception reads as a bug to the next reviewer. diff --git a/views/partials/locale-switch.ejs b/views/partials/locale-switch.ejs index 058f487..358e5e1 100644 --- a/views/partials/locale-switch.ejs +++ b/views/partials/locale-switch.ejs @@ -9,6 +9,6 @@ <%- include("menu", { up: locals.up !== false, trigger: { class: "btn icon-btn", icon: "i-globe", label: t("locale.label") }, - items: [{ head: t("locale.label") }, ...choices.map((c) => ({ current: c.current, href: c.href, hreflang: c.tag, label: c.label }))], + items: [{ head: t("locale.label") }, ...choices.map((c) => ({ current: c.current, href: c.href, hreflang: c.tag, label: c.label, ownLocale: true }))], }) %> <% } -%> diff --git a/views/partials/menu.ejs b/views/partials/menu.ejs index 200ec67..5cc72cf 100644 --- a/views/partials/menu.ejs +++ b/views/partials/menu.ejs @@ -8,7 +8,8 @@ kebab? boolean bare kebab trigger (adds .kebab) width? number|string popover min-width (number ⇒ px) items: Item[] popover content, top→bottom - Item ∈ { head } · { sep } · { label, icon?, href? ⇒ , hreflang?, current?, danger? } (default: menu-item button) + Item ∈ { head } · { sep } · { label, icon?, href? ⇒ , hreflang?, ownLocale?, current?, danger? } (default: menu-item button) + ownLocale: the href already states its language (the picker) — don't carry the current one onto it · { group: { legend?, name, control?(="checkbox"|"radio"), options:{value,label,checked?}[] } } %><% const trigger = locals.trigger || {}; // not `t` — that name is the translator in every view @@ -28,7 +29,7 @@ <% } else if (it.group) { const g = it.group; -%>
<% if (g.legend) { %><%= g.legend %><% } %><% g.options.forEach((o) => { %><% }) %>
<% } else if (it.href) { -%> - " href="<%= it.hreflang ? it.href : localeHref(it.href) %>"<% if (it.hreflang) { %> hreflang="<%= it.hreflang %>" lang="<%= it.hreflang %>"<% } %><% if (it.current) { %> aria-current="true"<% } %>><% if (it.icon) { %><% } %><%= it.label %> + " href="<%= it.ownLocale ? it.href : localeHref(it.href) %>"<% if (it.hreflang) { %> hreflang="<%= it.hreflang %>" lang="<%= it.hreflang %>"<% } %><% if (it.current) { %> aria-current="true"<% } %>><% if (it.icon) { %><% } %><%= it.label %> <% } else { -%> <% } -%>