From 8da75b4ca7243d547c92b296c16a964e6316faa0 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 07:36:08 +0200 Subject: [PATCH 01/10] Gate a route or nav node on a session, not only a permission --- AGENTS.md | 6 ++++ README.md | 23 +++++++++---- e2e-tests/visual.spec.ts | 22 ++++++++++--- examples/plugins/scheduling/README.md | 12 ++++--- examples/plugins/scheduling/i18n/en-US.ts | 3 ++ examples/plugins/scheduling/i18n/sv-SE.ts | 3 ++ examples/plugins/scheduling/plugin.ts | 4 ++- examples/plugins/scheduling/shifts.test.ts | 25 +++++++++++--- examples/plugins/scheduling/shifts.ts | 38 +++++++++++++++++++++- examples/plugins/scheduling/views/mine.ejs | 20 ++++++++++++ src/auth/gate.test.ts | 28 ++++++++++++++++ src/auth/gate.ts | 24 ++++++++++++++ src/http/app.ts | 5 +-- src/plugin-host/discovery.test.ts | 20 +++++++++--- src/plugin-host/discovery.ts | 19 ++++++----- src/plugin-host/plugin.ts | 3 ++ src/plugin-host/router.test.ts | 13 +------- src/plugin-host/router.ts | 7 ---- src/ui/chrome.ts | 3 +- src/ui/nav.test.ts | 28 +++++++++++----- src/ui/nav.ts | 19 ++++++----- 21 files changed, 250 insertions(+), 75 deletions(-) create mode 100644 examples/plugins/scheduling/views/mine.ejs create mode 100644 src/auth/gate.test.ts create mode 100644 src/auth/gate.ts diff --git a/AGENTS.md b/AGENTS.md index 0b170e2..adb1a36 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -182,6 +182,12 @@ Revisit only if the stated reason stops holding. example it keeps the route table and the in-handler guard deriving from one function, so 29 routes × 2 gate sites cannot drift. Generalised, it would make authorization a function of the transport verb — a route table must answer "what does this need?" on its own. +- **A gate is one of three, and `session` is a first-class one.** A route or nav node names exactly + one of `public`, `session`, `permission` (discovery refuses two), and `src/auth/gate.ts` is the one + home of the rule the router and the menu both read. `session` exists because a plugin whose data is + the visitor's own — their upstream account, their own tokens — has no distinction a permission could + name; the alternative, granting every newly registered user a permission, couples the identity + lifecycle to a Keto write that nothing retries when it fails. - **A `:read`-only holder must never be shown a write affordance.** The list/detail models carry `canWrite` and the views drop create/save/delete/add/remove; the permission picker still renders, disabled, because *seeing* who holds what is the point of `:read`. A **write-intent GET** (a create diff --git a/README.md b/README.md index 32d8a85..b5caf6e 100644 --- a/README.md +++ b/README.md @@ -389,13 +389,14 @@ A plugin may be routes-only, nav-only, or hooks-only — every collection field ### Routes & handlers -A route is `{ method, path, permission?, public?, handler }`. `path` is **relative to the plugin's +A route is `{ method, path, permission?, public?, session?, handler }`. `path` is **relative to the plugin's mount path `/`** (so `path: "/:id"` in the `things` plugin serves `/things/:id`); the host matches `method` + the resolved full path, extracts `:name` segments into `ctx.params.name`, runs the `permission` gate ([a coarse JWT-claim check](#nav--permission-gates)), then calls the handler with the [request context](#requestcontext). A failed gate redirects an **anonymous** visitor to `/login` with the page as `return_to`; a **signed-in** user lacking the permission gets the **403** page. -`public: true` means no gate at all (see [Public pages](#public-pages--menu-items)). +`public: true` means no gate at all, `session: true` any signed-in user (see +[Public pages](#public-pages--menu-items)). `method` is one of `GET HEAD POST PUT PATCH DELETE`. A `GET` route also answers `HEAD`. @@ -569,7 +570,8 @@ system plugins you author or vendor. An ordinary domain plugin ignores it. A plugin's `nav` fragment is merged into the global menu by `composeNav` (`src/ui/nav.ts`), which applies the central override and then **filters per user** by the permissions in the session JWT: a -node shows iff it is `public`, declares no `permission`, or the user holds that name. A node's `icon` +node shows iff it is `public`, is `session` and someone is signed in, declares no `permission`, or the +user holds that name. A node's `icon` is a **Lucide icon** by sprite id (e.g. `i-cal` → lucide `calendar`); the available ids are `ICON_NAMES` in `src/ui/icons.ts`, and adding one means registering its lucide name there. @@ -582,7 +584,15 @@ drops a header whose children all filtered out. That only works while the header A route or nav node marked **`public: true`** is reachable by anyone and shows in everyone's menu. That is the same as omitting `permission`, but stated outright so public is a deliberate choice -rather than a forgotten gate. The two are **mutually exclusive** — declaring both is refused at boot. +rather than a forgotten gate. + +**`session: true`** takes any signed-in user, with no grant to hold — the gate for a plugin whose +data is the visitor's own (their upstream account, their own tokens), where a permission would name +a distinction that does not exist. An anonymous visitor is bounced to `/login` with the page as +`return_to`, exactly as a permission gate does. + +A declaration names **exactly one** of the three; two of them contradict, and discovery refuses the +plugin at boot. A public page still renders in the native shell; for an anonymous visitor `ctx.user` is `null`, the shell shows a **Sign in** link in place of the profile block, the gated **Dashboard** link is hidden, @@ -635,7 +645,7 @@ The host detects collisions across all discovered plugins with `findConflicts` a Mount-path uniqueness needs no rule of its own — it follows from the id check. Discovery also rejects **per-manifest shape errors**: a non-array `nav`/`routes`/`permissions`, a non-function `home`/`dashboard`, a permission name that isn't [`:`](#naming-a-permission), or a -route/nav node setting both `public` and `permission`. +route/nav node naming more than one gate. ### Hooks @@ -913,7 +923,8 @@ The menu is **driven entirely by config** and assembled from two sources: Every nav item may carry a `permission`; the rendered tree is **filtered per user** from the session JWT (no per-request authz call), so the menu only shows what that person can reach. An item may -instead be **`public: true`** to show it to everyone — mutually exclusive with `permission`. +instead be **`public: true`** (everyone) or **`session: true`** (anyone signed in) — one gate per +item, never two. Branding (name, logo, default theme) renders in the app shell. **One menu, one shell, everywhere.** A single menu (`src/ui/chrome.ts` `buildPluginChrome`) renders diff --git a/e2e-tests/visual.spec.ts b/e2e-tests/visual.spec.ts index 02dd07e..8d9d1f2 100644 --- a/e2e-tests/visual.spec.ts +++ b/e2e-tests/visual.spec.ts @@ -143,11 +143,11 @@ test("unknown routes serve the 404 page (a real user-facing flow, covered end-to await expect(page.getByRole("link", { name: "Back home" })).toBeVisible(); }); -// The reference plugin (plugins/scheduling) ships discovered in the image. Its public Overview is -// reachable by anyone and its menu header shows for everyone; the shifts list stays permission-gated, -// so an anonymous visitor is bounced to sign in. The authenticated list/form flow is the full -// E2E (full-flow.spec). Side-effect-free. -test("the reference plugin: public Overview is open to all, the gated Shifts redirects to /login", async ({ page, request }) => { +// The reference plugin (plugins/scheduling) ships discovered in the image, and shows all three +// gates: the public Overview is reachable by anyone, My shifts takes any session, and the shifts +// list needs a permission. The authenticated list/form flow is the full E2E (full-flow.spec). +// Side-effect-free. +test("the reference plugin: public Overview is open to all, My shifts takes any session, the gated Shifts redirects to /login", async ({ page, request }) => { // `request` is the isolated API context — it doesn't carry the beforeEach session cookie, so these // probes are genuinely anonymous. // The public overview is reachable with no session (200), not bounced to sign in. @@ -166,10 +166,22 @@ test("the reference plugin: public Overview is open to all, the gated Shifts red expect(res.status()).toBe(303); expect(res.headers()["location"]).toBe("/login?return_to=%2Fscheduling%2Fshifts"); + // A `session: true` route bounces an anonymous visitor the same way — no permission involved. + const mine = await request.get("/scheduling/mine", { maxRedirects: 0 }); + expect(mine.status()).toBe(303); + expect(mine.headers()["location"]).toBe("/login?return_to=%2Fscheduling%2Fmine"); + // The signed-in member (no scheduling permission) sees the public Scheduling → Overview leaf in the nav, // but the gated Shifts leaf is filtered out. await page.goto("/dashboard"); await expect(page.locator('.sidebar a[href="/dashboard"]')).toHaveCount(1); // the one unified menu renders await expect(page.locator('.sidebar a[href="/scheduling"]')).toHaveCount(1); // public Overview shown await expect(page.locator('.sidebar a[href="/scheduling/shifts"]')).toHaveCount(0); // gated leaf filtered out + await expect(page.locator('.sidebar a[href="/scheduling/mine"]')).toHaveCount(1); // session gate: a session is enough + + // And the page itself renders for that same member, holding no permission at all. This stack runs + // no shifts upstream, so the list degrades to its empty state — which still names whose page it is. + await page.goto("/scheduling/mine"); + await expect(page.getByRole("heading", { name: "My shifts" })).toBeVisible(); + await expect(page.getByText("No shifts are assigned to demo@plainpages.local.")).toBeVisible(); }); diff --git a/examples/plugins/scheduling/README.md b/examples/plugins/scheduling/README.md index 50c623d..5698100 100644 --- a/examples/plugins/scheduling/README.md +++ b/examples/plugins/scheduling/README.md @@ -15,8 +15,9 @@ What it demonstrates: `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`, reusing the core `field` partial. -- **Permission-gated nav** — the "Shifts" nav leaf and routes are gated on `scheduling:read` / - `scheduling:write`; the whole "Scheduling" section is invisible to anyone without the grant. +- **All three route gates** — the Overview is `public` (anyone), "My shifts" is `session` (any + signed-in visitor, showing only rows assigned to them), and "Shifts" is gated on `scheduling:read` / + `scheduling:write`; a leaf whose gate a visitor fails is invisible in the menu. - **Its own translations** — every string comes from `i18n/en-US.ts` (`sv-SE.ts` beside it), including the nav labels, which are catalog keys in the manifest. `shifts.count` shows a plural message, and the views carry the visitor's language onto their links with `localeHref()`. @@ -50,6 +51,7 @@ cosmetically) — normalise to your backend's format there if it matters. ## Granting access -A user sees Scheduling once they hold the `scheduling:read` permission in Keto (and `scheduling:write` -to create). The one-command bootstrap grants both to the demo admin, so the seeded -`admin@plainpages.local` can use it immediately. +A user sees the shift list once they hold the `scheduling:read` permission in Keto (and +`scheduling:write` to create). The one-command bootstrap grants both to the demo admin, so the seeded +`admin@plainpages.local` can use it immediately. "My shifts" needs no grant at all — signing in is +its whole gate. diff --git a/examples/plugins/scheduling/i18n/en-US.ts b/examples/plugins/scheduling/i18n/en-US.ts index 1b59bd7..c7253c8 100644 --- a/examples/plugins/scheduling/i18n/en-US.ts +++ b/examples/plugins/scheduling/i18n/en-US.ts @@ -13,6 +13,9 @@ const messages = { "scheduling.filter.searchLabel": "Search shifts", "scheduling.filter.searchPlaceholder": "Search title or assignee…", "scheduling.form.submit": "Create shift", + "scheduling.mine.empty": "No shifts are assigned to {{email}}.", + "scheduling.mine.title": "My shifts", + "scheduling.nav.mine": "My shifts", "scheduling.nav.overview": "Overview", "scheduling.nav.section": "Scheduling", "scheduling.nav.shifts": "Shifts", diff --git a/examples/plugins/scheduling/i18n/sv-SE.ts b/examples/plugins/scheduling/i18n/sv-SE.ts index 24edc86..ccd281a 100644 --- a/examples/plugins/scheduling/i18n/sv-SE.ts +++ b/examples/plugins/scheduling/i18n/sv-SE.ts @@ -9,6 +9,9 @@ const messages: SchedulingMessages = { "scheduling.filter.searchLabel": "Sök pass", "scheduling.filter.searchPlaceholder": "Sök på namn eller person…", "scheduling.form.submit": "Skapa pass", + "scheduling.mine.empty": "Inga pass är tilldelade {{email}}.", + "scheduling.mine.title": "Mina pass", + "scheduling.nav.mine": "Mina pass", "scheduling.nav.overview": "Översikt", "scheduling.nav.section": "Schemaläggning", "scheduling.nav.shifts": "Pass", diff --git a/examples/plugins/scheduling/plugin.ts b/examples/plugins/scheduling/plugin.ts index eb2b30f..9c6bcad 100644 --- a/examples/plugins/scheduling/plugin.ts +++ b/examples/plugins/scheduling/plugin.ts @@ -3,7 +3,7 @@ // folder, rename it, point it at your own backend. Full contract: README.md → Building plugins. import { definePlugin } from "@plainpages/plugin-api"; -import { createShift, createUpstream, listShifts, newShiftForm, overview, READ, SCHEDULING_PATH, SHIFTS_PATH, WRITE } from "./shifts.ts"; +import { createShift, createUpstream, listShifts, MINE_PATH, myShifts, newShiftForm, overview, READ, SCHEDULING_PATH, SHIFTS_PATH, WRITE } from "./shifts.ts"; // The upstream this plugin reads/writes — a stand-in for your real backend (the plugin is // stateless). Its URL is a declared setting, so it is resolved and validated before onBoot hands it @@ -25,6 +25,7 @@ export default definePlugin({ nav: [{ children: [ { href: SCHEDULING_PATH, id: "scheduling:overview", label: "scheduling.nav.overview", public: true }, + { href: MINE_PATH, id: "scheduling:mine", label: "scheduling.nav.mine", session: true }, { href: SHIFTS_PATH, id: "scheduling:shifts", label: "scheduling.nav.shifts", permission: READ }, ], icon: "i-cal", @@ -42,6 +43,7 @@ export default definePlugin({ // (anyone may reach /scheduling, signed in or not); the rest need a permission. routes: [ { handler: overview(), method: "GET", path: "/", public: true }, + { handler: myShifts(upstream), method: "GET", path: "/mine", session: true }, { handler: listShifts(upstream), method: "GET", path: "/shifts", permission: READ }, { handler: newShiftForm(), method: "GET", path: "/shifts/new", permission: WRITE }, { handler: createShift(upstream), method: "POST", path: "/shifts", permission: WRITE }, diff --git a/examples/plugins/scheduling/shifts.test.ts b/examples/plugins/scheduling/shifts.test.ts index 9e43918..76a4b7e 100644 --- a/examples/plugins/scheduling/shifts.test.ts +++ b/examples/plugins/scheduling/shifts.test.ts @@ -4,21 +4,21 @@ import { Readable } from "node:stream"; import test from "node:test"; // Import only from the @plainpages/plugin-api barrel — the same contract boundary shifts.ts uses (the host may // refactor any deeper src/* freely behind it); the test models the dev/test story the contract preaches. -import { englishTranslator, GuardError, Log, type PageChrome, type RequestContext, type RouteResult } from "@plainpages/plugin-api"; +import { englishTranslator, GuardError, Log, type PageChrome, type RequestContext, type RouteResult, type User } from "@plainpages/plugin-api"; import enUS from "./i18n/en-US.ts"; import { - buildFormModel, createShift, createUpstream, listShifts, newShiftForm, overview, readInput, + buildFormModel, createShift, createUpstream, listShifts, myShifts, newShiftForm, overview, readInput, SHIFTS_PATH, type Shift, type ShiftInput, type ShiftsUpstream, UpstreamError, validate, } from "./shifts.ts"; const t = englishTranslator(enUS); // this plugin's catalog then the host's, as the host would chain them const CHROME: PageChrome = { brand: { name: "Test" }, csrfToken: "tok", nav: [], signInHref: "/login", user: { email: "", initials: "T", name: "Tester" } }; -function fakeCtx(opts: { body?: string; permissions?: string[]; url?: string; verifyCsrf?: (s: string | null | undefined) => boolean } = {}): RequestContext { +function fakeCtx(opts: { body?: string; permissions?: string[]; url?: string; user?: User; verifyCsrf?: (s: string | null | undefined) => boolean } = {}): RequestContext { const url = new URL(opts.url ?? "http://localhost/scheduling/shifts"); const req = Readable.from(opts.body != null ? [Buffer.from(opts.body)] : []) as unknown as IncomingMessage; return { - chrome: CHROME, declaredPermissions: [], declaredSettings: [], user: null, locale: "en-US", localeHref: (href) => href, locales: ["en-US"], log: new Log("none"), params: {}, + chrome: CHROME, declaredPermissions: [], declaredSettings: [], user: opts.user ?? null, locale: "en-US", localeHref: (href) => href, locales: ["en-US"], log: new Log("none"), params: {}, query: url.searchParams, req, res: {} as ServerResponse, permissions: opts.permissions ?? [], t, url, verifyCsrf: opts.verifyCsrf ?? (() => true), }; @@ -171,3 +171,20 @@ test("buildFormModel marks title/assignee required and attaches field errors", ( assert.equal(title.error, "needed"); assert.equal(fields.find((f) => f.name === "start")!.required, undefined); }); + +// ---- the session-gated page: the visitor's own rows ---- + +test("my shifts renders only the signed-in visitor's own rows, and names them in the empty state", async () => { + const user: User = { email: "Blair.Mora@example.test", id: "01a06091-baa3-71f4-a068-4879972979ff", permissions: [] }; + const mine: Shift = { assignee: "blair.mora@example.test", end: "22:00", id: "3", start: "17:00", title: "Evening on-call" }; + const listed = [...SHIFTS, mine]; // SHIFTS are assigned to other people + + const r = asView(await myShifts(fakeUpstream({ list: async () => listed }))(fakeCtx({ url: "http://localhost/scheduling/mine", user }))); + assert.equal(r.view, "mine"); + const table = r.data["table"] as { emptyText: string; rows: { name: string }[] }; + assert.deepEqual(table.rows.map((row) => row.name), ["Evening on-call"]); // matched case-insensitively + assert.match(table.emptyText, /Blair\.Mora@example\.test/); // an empty page still says whose it is + + // The route carries `session: true`, but the handler asserts the session itself rather than trusting it. + await assert.rejects(async () => { await myShifts(fakeUpstream())(fakeCtx()); }, GuardError); +}); diff --git a/examples/plugins/scheduling/shifts.ts b/examples/plugins/scheduling/shifts.ts index 2ba44f9..bb59196 100644 --- a/examples/plugins/scheduling/shifts.ts +++ b/examples/plugins/scheduling/shifts.ts @@ -6,7 +6,7 @@ // pure functions against a mock upstream with no network (README.md → Local dev & test story). // One import from the host's @plainpages/plugin-api barrel — the stable author surface (see README.md → Building plugins). -import { can, CSRF_FIELD, englishTranslator, GuardError, type PageChrome, parseListQuery, readFormBody, type RouteHandler, type Translate, tracedFetch } from "@plainpages/plugin-api"; +import { can, CSRF_FIELD, englishTranslator, GuardError, type PageChrome, parseListQuery, readFormBody, requireSession, type RouteHandler, type Translate, tracedFetch } from "@plainpages/plugin-api"; import enUS from "./i18n/en-US.ts"; // The plugin's own English (its catalog, then the host's), for a view model built outside a request: @@ -16,6 +16,7 @@ const EN: Translate = englishTranslator(enUS); export const SCHEDULING_PATH = "/scheduling"; // the plugin's public overview page export const SHIFTS_PATH = "/scheduling/shifts"; +export const MINE_PATH = "/scheduling/mine"; // the visitor's own shifts — a session is the whole gate export const READ = "scheduling:read"; // the permission gating the list + nav export const WRITE = "scheduling:write"; // the permission gating create @@ -190,6 +191,41 @@ export function newShiftForm(): RouteHandler { // gate lets an anonymous visitor through and the menu option shows for everyone. The real data // (the shifts list) stays behind `scheduling:read`; a reader gets a link straight to it, anyone // else a prompt to sign in. ctx.user may be null here, so read the permission via can() (zero I/O). +// The `session: true` archetype: the rows are the visitor's own, so there is no distinction a +// permission could name — anyone signed in sees theirs and only theirs. +export function myShifts(upstream: ShiftsUpstream): RouteHandler { + return async (ctx) => { + const user = requireSession(ctx); + let shifts: Shift[] = []; + let error: string | undefined; + try { + shifts = await upstream.list(); + } catch (err) { + ctx.log.warn("scheduling upstream unreachable", { error: String(err) }); + error = ctx.t("scheduling.upstream.list"); + } + const mine = shifts.filter((s) => s.assignee.toLowerCase() === user.email.toLowerCase()); + return { data: buildMineModel({ chrome: ctx.chrome, email: user.email, ...(error ? { error } : {}), shifts: mine, t: ctx.t }), view: "mine" }; + }; +} + +export function buildMineModel(opts: { chrome: PageChrome; email: string; error?: string; shifts: Shift[]; t?: Translate }) { + const t = opts.t ?? EN; + return { + breadcrumbs: [{ label: t("scheduling.mine.title") }], + chrome: opts.chrome, + count: t("scheduling.shifts.count", { count: opts.shifts.length }), + ...(opts.error ? { error: opts.error } : {}), + table: { + caption: t("scheduling.mine.title"), + columns: [{ label: t("scheduling.table.shift") }, { label: t("scheduling.table.start") }, { label: t("scheduling.table.end") }], + emptyText: t("scheduling.mine.empty", { email: opts.email }), + rows: opts.shifts.map((s) => ({ cells: [{ rowHeader: { text: s.title } }, s.start, s.end], name: s.title })), + }, + title: t("scheduling.mine.title"), + }; +} + export function overview(): RouteHandler { return (ctx) => ({ data: { diff --git a/examples/plugins/scheduling/views/mine.ejs b/examples/plugins/scheduling/views/mine.ejs new file mode 100644 index 0000000..57d94d3 --- /dev/null +++ b/examples/plugins/scheduling/views/mine.ejs @@ -0,0 +1,20 @@ +<%# + Scheduling · the visitor's own shifts (reference plugin). Reached behind `session: true`, so + ctx.user is always set by the time this renders. + Data: chrome, title, breadcrumbs, count, table, error? +%><% + const navHtml = include("partials/nav-tree", { nodes: chrome.nav }); + const tableHtml = include("partials/data-table", table); + const alertHtml = locals.error ? include("partials/alert", { text: locals.error, tone: "neg" }) : ""; +-%> +<%- include("partials/shell", { + body: '
' + alertHtml + '

' + count + '

' + tableHtml + '
', + brand: chrome.brand, + breadcrumbs, + csrfToken: chrome.csrfToken, + nav: navHtml, + styles: ["/public/scheduling/scheduling.css"], + theme: chrome.theme, + title, + user: chrome.user, +}) %> diff --git a/src/auth/gate.test.ts b/src/auth/gate.test.ts new file mode 100644 index 0000000..fe6e50c --- /dev/null +++ b/src/auth/gate.test.ts @@ -0,0 +1,28 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import type { User } from "../http/context.ts"; +import { allows, gatesSet } from "./gate.ts"; + +const holder: User = { email: "holder@example.test", id: "01a06091-ba9f-765f-abf4-b5144c314bc7", permissions: ["x:read"] }; +const stranger: User = { email: "stranger@example.test", id: "01a06091-baa3-7b4d-810a-c9ee7e559d98", permissions: [] }; + +test("allows: ungated and public are open to anyone; session needs a user; permission needs the token", () => { + assert.equal(allows({}, null), true); + assert.equal(allows({ public: true }, null), true); + + assert.equal(allows({ session: true }, null), false); + assert.equal(allows({ session: true }, stranger), true); // signed in is the whole gate — no grant + + assert.equal(allows({ permission: "x:read" }, null), false); + assert.equal(allows({ permission: "x:read" }, stranger), false); + assert.equal(allows({ permission: "x:read" }, holder), true); +}); + +test("gatesSet names the gates a declaration sets, so discovery can refuse more than one", () => { + assert.deepEqual(gatesSet({}), []); + assert.deepEqual(gatesSet({ session: true }), ["session"]); + assert.deepEqual(gatesSet({ permission: "x:read", public: true }), ["public", "permission"]); + assert.deepEqual(gatesSet({ permission: "x:read", public: true, session: true }), ["public", "session", "permission"]); + // `false` is not a gate — only a set one counts, so { session: false } is an ungated route. + assert.deepEqual(gatesSet({ public: false, session: false }), []); +}); diff --git a/src/auth/gate.ts b/src/auth/gate.ts new file mode 100644 index 0000000..6e47fcc --- /dev/null +++ b/src/auth/gate.ts @@ -0,0 +1,24 @@ +// The coarse gate a route or nav node declares. One home for the rule, so the router and the menu +// can never disagree about what a visitor may reach. +import type { User } from "../http/context.ts"; + +// Widest first: whoever passes an earlier gate passes it without holding anything. +const GATES = ["public", "session", "permission"] as const; + +export interface Gate { + permission?: string | undefined; // the Keto Permission the caller must hold + public?: boolean | undefined; // anyone, signed in or not + session?: boolean | undefined; // any signed-in user, no grant needed +} + +export function allows(gate: Gate, user: User | null): boolean { + if (gate.public === true) return true; + if (gate.session === true) return user !== null; + return gate.permission == null || (user?.permissions.includes(gate.permission) ?? false); +} + +// Which gates a declaration sets — discovery refuses more than one, since they contradict. +export function gatesSet(gate: Gate | null | undefined): string[] { + if (gate == null) return []; + return GATES.filter((name) => (name === "permission" ? gate.permission != null : gate[name] === true)); +} diff --git a/src/http/app.ts b/src/http/app.ts index 39d57f1..9d21a78 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -28,7 +28,8 @@ import { DEFAULT_MENU, type MenuConfig } from "../ui/menu-config.ts"; import { declaredPermissions, type Plugin, type RouteHandler, type RouteResult } from "../plugin-host/plugin.ts"; import type { PluginSettings } from "../plugin-host/settings.ts"; import type { SystemCapabilities } from "../plugin-host/system.ts"; -import { allowedMethods, isAuthorized, matchRoute } from "../plugin-host/router.ts"; +import { allows } from "../auth/gate.ts"; +import { allowedMethods, matchRoute } from "../plugin-host/router.ts"; import { buildAuthRoutes } from "../auth/routes.ts"; import { securityHeaders } from "./security-headers.ts"; import { localPath } from "./safe-url.ts"; @@ -281,7 +282,7 @@ export function createApp(options: AppOptions = {}): Server { const match = matchRoute(plugins, method, pathname); if (match) { const routeCtx = contextFor(match.plugin.id, match.params); - if (!isAuthorized(match.route, routeCtx.permissions)) { + if (!allows(match.route, routeCtx.user)) { // Anonymous → sign in, remembering the page as return_to; a signed-in user who simply // lacks the permission gets the 403 page. if (!routeCtx.user) { res.writeHead(303, { location: loginRedirect(routeCtx) }).end(); return; } diff --git a/src/plugin-host/discovery.test.ts b/src/plugin-host/discovery.test.ts index 14ef216..b47ccbc 100644 --- a/src/plugin-host/discovery.test.ts +++ b/src/plugin-host/discovery.test.ts @@ -62,6 +62,9 @@ const badCases: Array<{ name: string; files: Record; match: RegE { name: "duplicate nav id across plugins", files: { "a/plugin.ts": full("a").replace("a:root", "dup"), "b/plugin.ts": full("b").replace("b:root", "dup") }, match: /nav id "dup"/ }, { name: "a route marked public AND permission is contradictory", files: { "contra/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", public: true, permission: "x:read", handler: () => ({ html: "x" }) }] };` }, match: /contra.*public.*permission/s }, { name: "a nav node marked public AND permission is contradictory", files: { "contranav/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "n", label: "N", public: true, permission: "x:read" }] };` }, match: /contranav.*public.*permission/s }, + { name: "a route marked session AND permission is contradictory", files: { "contrasess/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", session: true, permission: "x:read", handler: () => ({ html: "x" }) }] };` }, match: /contrasess.*session.*permission/s }, + { name: "a route marked public AND session is contradictory", files: { "contrapub/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", public: true, session: true, handler: () => ({ html: "x" }) }] };` }, match: /contrapub.*public.*session/s }, + { name: "a nav node marked session AND permission is contradictory", files: { "contrasessnav/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "n", label: "N", session: true, permission: "x:read" }] };` }, match: /contrasessnav.*session.*permission/s }, // A permission name is : wherever the manifest mentions one. Enforced here, not // only in the admin GUI, so it holds for a plugin installed without that GUI. { name: "a route gating on a bare word", files: { "bare/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", permission: "admin", handler: () => ({ html: "x" }) }] };` }, match: /bare.*admin.*:/s }, @@ -96,12 +99,19 @@ test("a discovery failure tells the operator their plugins/ copy may just be out }); }); -test("a route + nav node may be marked public and load fine", async (t) => { - const dir = scaffold(t, { "pub/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ href: "/pub", id: "n", label: "N", public: true }], routes: [{ method: "GET", path: "/", public: true, handler: () => ({ html: "x" }) }] };` }); +test("a route + nav node may be marked public, or session, and load fine", async (t) => { + const dir = scaffold(t, { + "pub/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ href: "/pub", id: "n", label: "N", public: true }], routes: [{ method: "GET", path: "/", public: true, handler: () => ({ html: "x" }) }] };`, + "sess/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ href: "/sess", id: "s", label: "S", session: true }], routes: [{ method: "GET", path: "/", session: true, handler: () => ({ html: "x" }) }] };`, + }); const plugins = await discoverPlugins({ dir }); - assert.equal(plugins.length, 1); - assert.equal(plugins[0]?.routes?.[0]?.public, true); - assert.equal(plugins[0]?.nav?.[0]?.public, true); + assert.equal(plugins.length, 2); + const pub = plugins.find((p) => p.id === "pub"); + const sess = plugins.find((p) => p.id === "sess"); + assert.equal(pub?.routes?.[0]?.public, true); + assert.equal(pub?.nav?.[0]?.public, true); + assert.equal(sess?.routes?.[0]?.session, true); + assert.equal(sess?.nav?.[0]?.session, true); }); test("`admin` is not reserved — the admin screens ship as a drop-in plugin mounted at /admin", async (t) => { diff --git a/src/plugin-host/discovery.ts b/src/plugin-host/discovery.ts index 660d603..8b8433c 100644 --- a/src/plugin-host/discovery.ts +++ b/src/plugin-host/discovery.ts @@ -7,6 +7,7 @@ import { existsSync, readdirSync, readFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; +import { gatesSet } from "../auth/gate.ts"; import { checkApiVersion, findConflicts, isValidPermissionName, isValidPluginId, RESERVED_PLUGIN_IDS, type Plugin, type PluginManifest } from "./plugin.ts"; import { settingsDeclError } from "./settings.ts"; import { isValidStoragePluginId, MAX_STORAGE_PLUGIN_ID_LENGTH } from "./storage.ts"; @@ -146,12 +147,13 @@ function shapeError(manifest: PluginManifest): string | null { const settings = settingsDeclError(manifest.settings); if (settings) return settings; } - // `public` and `permission` are contradictory on the same route/nav node — "open to all" vs - // "needs this permission". Refuse rather than silently pick one, so the author's intent is unambiguous. + // Two gates on one route or nav node contradict each other — "open to all" vs "needs a session" + // vs "needs this permission". Refuse rather than silently pick one, so intent stays unambiguous. for (const route of Array.isArray(manifest.routes) ? manifest.routes : []) { - if (route?.public === true && route.permission != null) return `route "${route.method} ${route.path}" sets both public and permission — they are mutually exclusive`; + const gates = gatesSet(route); + if (gates.length > 1) return `route "${route?.method} ${route?.path}" sets ${gates.join(" and ")}; a route names exactly one gate — public, session or permission`; } - const navContradiction = findPublicNavContradiction(manifest.nav); + const navContradiction = findNavGateContradiction(manifest.nav); if (navContradiction) return navContradiction; // Every permission name the manifest mentions — gated on or declared — must be `:`. // A bare word names a role, and roles are groups here (README → Naming a permission). @@ -170,11 +172,12 @@ function shapeError(manifest: PluginManifest): string | null { return null; } -// Recurse the nav fragment: a node that is both `public` and `permission`-gated is contradictory. -function findPublicNavContradiction(nodes: PluginManifest["nav"]): string | null { +// Recurse the nav fragment: a node naming more than one gate is contradictory, same as a route. +function findNavGateContradiction(nodes: PluginManifest["nav"]): string | null { for (const node of Array.isArray(nodes) ? nodes : []) { - if (node?.public === true && node.permission != null) return `nav node "${node.label ?? node.id ?? "?"}" sets both public and permission — they are mutually exclusive`; - const inChild = findPublicNavContradiction(node?.children); + const gates = gatesSet(node); + if (gates.length > 1) return `nav node "${node?.label ?? node?.id ?? "?"}" sets ${gates.join(" and ")}; a node names exactly one gate — public, session or permission`; + const inChild = findNavGateContradiction(node?.children); if (inChild) return inChild; } return null; diff --git a/src/plugin-host/plugin.ts b/src/plugin-host/plugin.ts index 313d667..df97a45 100644 --- a/src/plugin-host/plugin.ts +++ b/src/plugin-host/plugin.ts @@ -32,6 +32,9 @@ export interface Route { // Same as omitting `permission`, but stated outright so public is a deliberate choice rather than // a forgotten gate. Mutually exclusive with `permission` (discovery refuses both). public?: boolean; + // Any signed-in user, no grant to hold — for a plugin whose data is the visitor's own. Anonymous + // is bounced to /login, never 403. Mutually exclusive with the other two (discovery refuses both). + session?: boolean; } // A Keto Permission this plugin gates on — declared for docs/seeding. Names are a shared global diff --git a/src/plugin-host/router.test.ts b/src/plugin-host/router.test.ts index 27e3447..a4246a6 100644 --- a/src/plugin-host/router.test.ts +++ b/src/plugin-host/router.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import type { Plugin, Route } from "./plugin.ts"; -import { allowedMethods, isAuthorized, matchRoute } from "./router.ts"; +import { allowedMethods, matchRoute } from "./router.ts"; const noop: Route["handler"] = () => ({ html: "x" }); @@ -54,14 +54,3 @@ test("allowedMethods lists methods at a path (GET implies HEAD); empty when the assert.deepEqual(allowedMethods(plugins, "/x/a"), ["GET", "HEAD", "POST"]); assert.deepEqual(allowedMethods(plugins, "/x/missing"), []); }); - -test("isAuthorized: open routes pass; gated routes require the permission token; public is explicitly open", () => { - const open: Route = { handler: noop, method: "GET", path: "/" }; - const gated: Route = { handler: noop, method: "GET", path: "/", permission: "x:read" }; - const pub: Route = { handler: noop, method: "GET", path: "/", public: true }; // blessed public alias - assert.equal(isAuthorized(open, []), true); - assert.equal(isAuthorized(gated, []), false); - assert.equal(isAuthorized(gated, ["x:read"]), true); - assert.equal(isAuthorized(gated, ["other"]), false); - assert.equal(isAuthorized(pub, []), true); // open to anonymous, like omitting permission — but stated outright -}); diff --git a/src/plugin-host/router.ts b/src/plugin-host/router.ts index 072e735..8ac5c70 100644 --- a/src/plugin-host/router.ts +++ b/src/plugin-host/router.ts @@ -73,10 +73,3 @@ export function allowedMethods(plugins: Plugin[], pathname: string): string[] { } return [...methods].sort(); } - -// Coarse permission gate: a route marked `public` (or one with no `permission`) is open; otherwise -// the user's permissions (from the session JWT) must include the token. The same rule composeNav uses -// for the menu. `public` and `permission` are mutually exclusive (discovery refuses both). -export function isAuthorized(route: Route, permissions: string[]): boolean { - return route.public === true || route.permission == null || permissions.includes(route.permission); -} diff --git a/src/ui/chrome.ts b/src/ui/chrome.ts index 24e1b04..ce0bf7b 100644 --- a/src/ui/chrome.ts +++ b/src/ui/chrome.ts @@ -44,8 +44,7 @@ export function buildPluginChrome(opts: ChromeOptions): PageChrome { if (p.nav?.length) fragments.push(translateNav(p.nav, opts.translatorFor?.(p.id) ?? t)); } - const permissions = opts.user?.permissions ?? []; - const nav = composeNav(fragments, opts.menu.override, permissions, t); + const nav = composeNav(fragments, opts.menu.override, opts.user ?? null, t); if (opts.currentPath) { // Mark by the *best* (longest) href that is the path or a parent of it, so a sub-path like // /admin/users/new marks the Users base leaf (/admin/users) and the dashboard marks Dashboard. diff --git a/src/ui/nav.test.ts b/src/ui/nav.test.ts index 90a2a07..12e79fc 100644 --- a/src/ui/nav.test.ts +++ b/src/ui/nav.test.ts @@ -1,7 +1,12 @@ import assert from "node:assert/strict"; import { test } from "node:test"; +import type { User } from "../http/context.ts"; import { composeNav, type NavNode } from "./nav.ts"; +function viewer(...permissions: string[]): User { + return { email: "viewer@example.test", id: "01a06091-ba9f-765f-abf4-b5144c314bc7", permissions }; +} + // Two plugin fragments; ids let the override target nodes, `permission` gates per permission. const fragments: NavNode[][] = [ [{ @@ -15,7 +20,7 @@ const fragments: NavNode[][] = [ ]; test("composeNav merges fragments, filters by permission, and emits clean render nodes", () => { - const tree = composeNav(fragments, {}, ["scheduling:read"]); + const tree = composeNav(fragments, {}, viewer("scheduling:read")); // Reports gone (no reports:read), Manage gone (no scheduling:admin), header kept with Shifts. // Output carries no `id`/`permission` and omits absent fields — ready for nav-tree.ejs. @@ -30,7 +35,7 @@ test("composeNav drops gated subtrees, empty headers, and (with no permissions) { id: "admin", label: "Admin", permission: "users:read", children: [{ href: "/u", id: "u", label: "Users" }] }, { id: "free", label: "Free", children: [{ href: "/d", id: "d", label: "Docs" }] }, ]]; - assert.deepEqual(composeNav(gatedHeader, {}, []), [ + assert.deepEqual(composeNav(gatedHeader, {}, viewer()), [ { label: "Free", children: [{ href: "/d", label: "Docs" }] }, ]); @@ -39,26 +44,31 @@ test("composeNav drops gated subtrees, empty headers, and (with no permissions) { id: "sec", label: "Section", children: [{ href: "/x", id: "x", label: "X", permission: "x:read" }] }, { href: "/hub", id: "hub", label: "Hub", children: [{ href: "/y", id: "y", label: "Y", permission: "y:read" }] }, ]]; - assert.deepEqual(composeNav(emptyHeader, {}, []), [{ href: "/hub", label: "Hub" }]); + assert.deepEqual(composeNav(emptyHeader, {}, viewer()), [{ href: "/hub", label: "Hub" }]); // No fragments / no permissions → empty tree, never throws. assert.deepEqual(composeNav(), []); }); -test("composeNav keeps a node marked public for everyone — the blessed public alias", () => { - // A header with one public child + one gated child: with no permissions, the public child keeps the - // header alive (the gated child is filtered out) — so a plugin can show a public menu option to all. +test("composeNav shows a public node to everyone and a session node to any signed-in user", () => { + // A header with a public child, a session child and a gated child: the public child keeps the + // header alive for an anonymous visitor — so a plugin can show a menu option to all. const frag: NavNode[][] = [[{ icon: "i-cal", id: "sched", label: "Scheduling", children: [ { href: "/scheduling", id: "overview", label: "Overview", public: true }, + { href: "/scheduling/mine", id: "mine", label: "Mine", session: true }, { href: "/scheduling/shifts", id: "shifts", label: "Shifts", permission: "scheduling:read" }, ], }]]; - // `public` is filter-only (like id/permission) — never rendered into the output node. - assert.deepEqual(composeNav(frag, {}, []), [ + // `public`/`session` are filter-only (like id/permission) — never rendered into the output node. + assert.deepEqual(composeNav(frag, {}, null), [ { icon: "i-cal", label: "Scheduling", children: [{ href: "/scheduling", label: "Overview" }] }, ]); + // Signed in with no permission at all: the session node appears, the permission-gated one does not. + assert.deepEqual(composeNav(frag, {}, viewer()), [ + { icon: "i-cal", label: "Scheduling", children: [{ href: "/scheduling", label: "Overview" }, { href: "/scheduling/mine", label: "Mine" }] }, + ]); }); test("composeNav applies the override: rename, group, order, hide (then filters)", () => { @@ -74,7 +84,7 @@ test("composeNav applies the override: rename, group, order, hide (then filters) groups: [{ icon: "i-box", id: "grp", label: "Group", open: true, children: ["b", "c"] }], // wrap b+c order: ["grp", "a"], // grp before the lone a hide: ["c"], // remove c from inside the group - }, ["secrets:read"]); + }, viewer("secrets:read")); // grp emitted (b only, c hidden), reordered before a; Secret kept now that permission "secrets:read" is present. assert.deepEqual(tree, [ diff --git a/src/ui/nav.ts b/src/ui/nav.ts index 9cf72f8..689a450 100644 --- a/src/ui/nav.ts +++ b/src/ui/nav.ts @@ -1,8 +1,10 @@ // composeNav: merge each plugin's nav fragment into one tree, apply the central override, then -// permission-filter per user. Pure and I/O-free — menu gating reads the JWT `permissions` claim, -// never Keto. A node is visible iff it is `public`, declares no `permission`, or the user holds that -// name; a gated header hides its whole subtree, and a pure header left with no children is dropped. +// filter per user. Pure and I/O-free — menu gating reads the JWT `permissions` claim, never Keto. +// A node is visible iff `allows` passes its gate; a gated header hides its whole subtree, and a pure +// header left with no children is dropped. +import { allows } from "../auth/gate.ts"; +import type { User } from "../http/context.ts"; import { ENGLISH } from "../i18n/english.ts"; import type { Translate } from "../i18n/translate.ts"; @@ -17,6 +19,7 @@ export interface NavNode { open?: boolean; permission?: string; // required permission token; consumed by the filter, never rendered public?: boolean; // show to everyone, signed in or not — the blessed alias for "no permission", stated outright; consumed by the filter, never rendered. Mutually exclusive with permission (discovery refuses both). + session?: boolean; // show to any signed-in user, no grant to hold; consumed by the filter, never rendered. Mutually exclusive with the other two (discovery refuses both). } // Central override (config/menu.ts). Targets nodes by `id`; applied rename → group → @@ -39,7 +42,7 @@ export interface NavGroupSpec { export function composeNav( fragments: NavNode[][] = [], override: NavOverride = {}, - permissions: string[] = [], + user: User | null = null, t: Translate = ENGLISH, ): NavNode[] { let nodes: NavNode[] = fragments.flat(); @@ -47,7 +50,7 @@ export function composeNav( if (override.groups?.length) nodes = applyGroups(nodes, override.groups); if (override.order?.length) nodes = applyOrder(nodes, override.order); if (override.hide?.length) nodes = hideTree(nodes, new Set(override.hide)); - return filterByRoles(nodes, new Set(permissions)).map((node) => toRenderNode(node, t)); + return filterByGate(nodes, user).map((node) => toRenderNode(node, t)); } function renameTree(nodes: NavNode[], rename: Record): NavNode[] { @@ -104,12 +107,12 @@ function hideTree(nodes: NavNode[], hide: Set): NavNode[] { return out; } -function filterByRoles(nodes: NavNode[], permissions: Set): NavNode[] { +function filterByGate(nodes: NavNode[], user: User | null): NavNode[] { const out: NavNode[] = []; for (const n of nodes) { - if (n.public !== true && n.permission != null && !permissions.has(n.permission)) continue; // gated → drop node + subtree (public always shows) + if (!allows(n, user)) continue; // gated → drop node + subtree if (!n.children) { out.push(n); continue; } - const children = filterByRoles(n.children, permissions); + const children = filterByGate(n.children, user); if (children.length === 0 && n.href == null) continue; // empty pure header → drop out.push({ ...n, children }); } -- 2.52.0 From c7e6d6675053bef6f0bfc155380506f56d1e6b5e Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 08:15:08 +0200 Subject: [PATCH 02/10] Scope My shifts at the upstream, and give Route and NavNode one gate declaration --- e2e-tests/full-flow.spec.ts | 7 ++++++ examples/plugins/scheduling/README.md | 6 +++++- examples/plugins/scheduling/shifts.test.ts | 12 +++++++---- examples/plugins/scheduling/shifts.ts | 25 ++++++++++++---------- examples/shifts-upstream/server.ts | 7 +++++- src/auth/gate.ts | 6 ++++-- src/plugin-host/plugin-api.ts | 1 + src/plugin-host/plugin.ts | 11 +++------- src/ui/nav.ts | 8 +++---- 9 files changed, 51 insertions(+), 32 deletions(-) diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index a884569..6dc8723 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -193,6 +193,13 @@ test.describe.serial("authenticated admin journey", () => { await page.goto("/scheduling/shifts"); await expect(page.locator("h1")).toHaveText("Shifts"); await expect(page.locator("table")).toContainText("Morning — Front desk"); // seeded by the mock upstream + + // The session-gated page asks the upstream for this visitor's rows: the one seeded against the + // signed-in admin is there, and another person's shift is not. + await page.goto("/scheduling/mine"); + await expect(page.locator("h1")).toHaveText("My shifts"); + await expect(page.locator("table")).toContainText("Night — Escalations"); + await expect(page.locator("table")).not.toContainText("Morning — Front desk"); }); test("plugin settings: the screen names the variable that sets each declared key", async () => { diff --git a/examples/plugins/scheduling/README.md b/examples/plugins/scheduling/README.md index 5698100..f0e142b 100644 --- a/examples/plugins/scheduling/README.md +++ b/examples/plugins/scheduling/README.md @@ -26,6 +26,10 @@ What it demonstrates: The plugin holds **no state** — data lives upstream (README → *Stateless*). Handlers are thin and `fetch` is injectable, so they unit-test as pure functions (`shifts.test.ts`). +The shifts list and "My shifts" repeat a little view-model and markup rather than sharing a +parameterised one: an example is read far more often than it is changed, and each page is meant to be +followed top to bottom on its own. + ## Upstream Set `PLUGIN_SETTING_SCHEDULING_UPSTREAM` to your backend's base URL. The dev compose points it at a tiny in-memory @@ -39,7 +43,7 @@ Your backend must expose two routes; the plugin treats any non-2xx as a recovera | Route | Request | Success | Response body | | --- | --- | --- | --- | -| `GET /shifts` | `Accept: application/json` | `200` | JSON array of `{ id, title, assignee, start, end }` (all strings; missing fields coerce to `""`) | +| `GET /shifts` | `Accept: application/json`, optional `?assignee=` | `200` | JSON array of `{ id, title, assignee, start, end }` (all strings; missing fields coerce to `""`). With `assignee`, only that person's rows — "My shifts" asks for them rather than filtering everyone's here, because ownership is the backend's rule to enforce | | `POST /shifts` | JSON body `{ title, assignee, start, end }` | `2xx` | ignored (the plugin POST-redirect-GETs back to the list) | Domain rules (overlap, capacity, time ordering) live in your backend — reject with a 4xx and the diff --git a/examples/plugins/scheduling/shifts.test.ts b/examples/plugins/scheduling/shifts.test.ts index 76a4b7e..e848bee 100644 --- a/examples/plugins/scheduling/shifts.test.ts +++ b/examples/plugins/scheduling/shifts.test.ts @@ -174,15 +174,19 @@ test("buildFormModel marks title/assignee required and attaches field errors", ( // ---- the session-gated page: the visitor's own rows ---- -test("my shifts renders only the signed-in visitor's own rows, and names them in the empty state", async () => { +test("my shifts asks the upstream for the visitor's own rows, and names them in the empty state", async () => { const user: User = { email: "Blair.Mora@example.test", id: "01a06091-baa3-71f4-a068-4879972979ff", permissions: [] }; const mine: Shift = { assignee: "blair.mora@example.test", end: "22:00", id: "3", start: "17:00", title: "Evening on-call" }; - const listed = [...SHIFTS, mine]; // SHIFTS are assigned to other people + let asked: { assignee?: string } | undefined; - const r = asView(await myShifts(fakeUpstream({ list: async () => listed }))(fakeCtx({ url: "http://localhost/scheduling/mine", user }))); + const upstream = fakeUpstream({ list: async (opts) => { asked = opts; return [mine]; } }); + const r = asView(await myShifts(upstream)(fakeCtx({ url: "http://localhost/scheduling/mine", user }))); assert.equal(r.view, "mine"); + // The ownership rule is the upstream's: the page asks for one person's rows rather than filtering + // everyone's here, so a real backend never hands this handler another visitor's shifts. + assert.deepEqual(asked, { assignee: "Blair.Mora@example.test" }); const table = r.data["table"] as { emptyText: string; rows: { name: string }[] }; - assert.deepEqual(table.rows.map((row) => row.name), ["Evening on-call"]); // matched case-insensitively + assert.deepEqual(table.rows.map((row) => row.name), ["Evening on-call"]); assert.match(table.emptyText, /Blair\.Mora@example\.test/); // an empty page still says whose it is // The route carries `session: true`, but the handler asserts the session itself rather than trusting it. diff --git a/examples/plugins/scheduling/shifts.ts b/examples/plugins/scheduling/shifts.ts index bb59196..469d8b9 100644 --- a/examples/plugins/scheduling/shifts.ts +++ b/examples/plugins/scheduling/shifts.ts @@ -47,7 +47,9 @@ export class UpstreamError extends Error { export interface ShiftsUpstream { create(input: ShiftInput): Promise; - list(): Promise; + // `assignee` scopes the read at the source, which is where an ownership rule belongs (README → + // Three tiers of "may I?"); without it the caller would hold everyone's rows to render one page. + list(opts?: { assignee?: string }): Promise; } // REST client over the upstream service (a stand-in for the customer's real backend). `fetch` @@ -66,8 +68,9 @@ export function createUpstream(baseUrl: () => string, fetchImpl: typeof fetch = }); if (!res.ok) throw new UpstreamError(`create shift failed (${res.status})`, res.status); }, - async list() { - const res = await fetchImpl(`${base()}/shifts`, { headers: { accept: "application/json" } }); + async list(opts = {}) { + const query = opts.assignee == null ? "" : `?${new URLSearchParams({ assignee: opts.assignee })}`; + const res = await fetchImpl(`${base()}/shifts${query}`, { headers: { accept: "application/json" } }); if (!res.ok) throw new UpstreamError(`list shifts failed (${res.status})`, res.status); const data: unknown = await res.json(); return Array.isArray(data) ? data.map(toShift) : []; @@ -187,25 +190,21 @@ export function newShiftForm(): RouteHandler { return (ctx) => ({ data: buildFormModel({ chrome: ctx.chrome, t: ctx.t }), view: "shift-new" }); } -// Public overview: a page anyone may reach — its route + nav node are marked `public`, so the -// gate lets an anonymous visitor through and the menu option shows for everyone. The real data -// (the shifts list) stays behind `scheduling:read`; a reader gets a link straight to it, anyone -// else a prompt to sign in. ctx.user may be null here, so read the permission via can() (zero I/O). // The `session: true` archetype: the rows are the visitor's own, so there is no distinction a -// permission could name — anyone signed in sees theirs and only theirs. +// permission could name — anyone signed in sees theirs and only theirs. The scoping is the +// upstream's, never a filter here: it owns the data and answers for one person's rows. export function myShifts(upstream: ShiftsUpstream): RouteHandler { return async (ctx) => { const user = requireSession(ctx); let shifts: Shift[] = []; let error: string | undefined; try { - shifts = await upstream.list(); + shifts = await upstream.list({ assignee: user.email }); } catch (err) { ctx.log.warn("scheduling upstream unreachable", { error: String(err) }); error = ctx.t("scheduling.upstream.list"); } - const mine = shifts.filter((s) => s.assignee.toLowerCase() === user.email.toLowerCase()); - return { data: buildMineModel({ chrome: ctx.chrome, email: user.email, ...(error ? { error } : {}), shifts: mine, t: ctx.t }), view: "mine" }; + return { data: buildMineModel({ chrome: ctx.chrome, email: user.email, ...(error ? { error } : {}), shifts, t: ctx.t }), view: "mine" }; }; } @@ -226,6 +225,10 @@ export function buildMineModel(opts: { chrome: PageChrome; email: string; error? }; } +// Public overview: a page anyone may reach — its route + nav node are marked `public`, so the +// gate lets an anonymous visitor through and the menu option shows for everyone. The real data +// (the shifts list) stays behind `scheduling:read`; a reader gets a link straight to it, anyone +// else a prompt to sign in. ctx.user may be null here, so read the permission via can() (zero I/O). export function overview(): RouteHandler { return (ctx) => ({ data: { diff --git a/examples/shifts-upstream/server.ts b/examples/shifts-upstream/server.ts index c769d4f..8e6be97 100644 --- a/examples/shifts-upstream/server.ts +++ b/examples/shifts-upstream/server.ts @@ -15,6 +15,7 @@ const shifts = [ { id: randomUUID(), title: "Morning — Front desk", assignee: "Avery Kline", start: "2026-06-22 08:00", end: "2026-06-22 12:00" }, { id: randomUUID(), title: "Afternoon — Support", assignee: "Blair Mora", start: "2026-06-22 12:00", end: "2026-06-22 17:00" }, { id: randomUUID(), title: "Evening — On-call", assignee: "Casey Nguyen", start: "2026-06-22 17:00", end: "2026-06-22 22:00" }, + { id: randomUUID(), title: "Night — Escalations", assignee: "admin@plainpages.local", start: "2026-06-22 22:00", end: "2026-06-23 06:00" }, ]; const json = (res, status, body) => { @@ -33,7 +34,11 @@ const readBody = (req) => createServer(async (req, res) => { const url = new URL(req.url ?? "/", "http://localhost"); - if (url.pathname === "/shifts" && req.method === "GET") return json(res, 200, shifts); + if (url.pathname === "/shifts" && req.method === "GET") { + const assignee = url.searchParams.get("assignee"); + if (assignee === null) return json(res, 200, shifts); + return json(res, 200, shifts.filter((s) => s.assignee.toLowerCase() === assignee.toLowerCase())); + } if (url.pathname === "/shifts" && req.method === "POST") { const b = await readBody(req); const shift = { id: randomUUID(), assignee: String(b.assignee ?? ""), end: String(b.end ?? ""), start: String(b.start ?? ""), title: String(b.title ?? "") }; diff --git a/src/auth/gate.ts b/src/auth/gate.ts index 6e47fcc..270c516 100644 --- a/src/auth/gate.ts +++ b/src/auth/gate.ts @@ -5,10 +5,12 @@ import type { User } from "../http/context.ts"; // Widest first: whoever passes an earlier gate passes it without holding anything. const GATES = ["public", "session", "permission"] as const; +// A route or nav node names exactly one of these; discovery refuses two. Omitting all three is the +// same as `public`, which is why stating it outright makes an open gate a choice, not an oversight. export interface Gate { - permission?: string | undefined; // the Keto Permission the caller must hold + permission?: string | undefined; // the Keto Permission the caller must hold, `:` public?: boolean | undefined; // anyone, signed in or not - session?: boolean | undefined; // any signed-in user, no grant needed + session?: boolean | undefined; // any signed-in user, no grant to hold; anonymous is sent to /login } export function allows(gate: Gate, user: User | null): boolean { diff --git a/src/plugin-host/plugin-api.ts b/src/plugin-host/plugin-api.ts index 03d1ec7..70b6228 100644 --- a/src/plugin-host/plugin-api.ts +++ b/src/plugin-host/plugin-api.ts @@ -39,6 +39,7 @@ export { CSRF_FIELD } from "../auth/csrf.ts"; // reference consumer. The Ory client types + their error classes are re-exported so a system // plugin can type against them and `instanceof`-match their errors. See README → System capabilities. export type { SystemCapabilities } from "./system.ts"; +export type { Gate } from "../auth/gate.ts"; export type { Identity, KratosAdmin, RecoveryCode } from "../auth/kratos-admin.ts"; export type { ExpandTree, KetoClient, RelationQuery, RelationTuple, SubjectSet } from "../auth/keto-client.ts"; export type { HydraAdmin, OAuth2Client } from "../auth/hydra-admin.ts"; diff --git a/src/plugin-host/plugin.ts b/src/plugin-host/plugin.ts index df97a45..84d749a 100644 --- a/src/plugin-host/plugin.ts +++ b/src/plugin-host/plugin.ts @@ -4,6 +4,7 @@ // A plugin's identity is its folder under plugins/: folder name = `id` (isValidPluginId), mount = // `/`. Neither is in the manifest — the host derives them, so they can't drift or be claimed twice. +import type { Gate } from "../auth/gate.ts"; import type { RequestContext } from "../http/context.ts"; import type { NavNode } from "../ui/nav.ts"; import { envName, type SettingDecl, type SettingsOf } from "./settings.ts"; @@ -24,17 +25,11 @@ export type RouteResult = export type RouteHandler = (ctx: RequestContext) => Promise | RouteResult | void; -export interface Route { +// `Gate` carries `permission`/`public`/`session`, checked before the handler runs. +export interface Route extends Gate { handler: RouteHandler; method: HttpMethod; path: string; // relative to the plugin's mount path `/`; ":name" segments → ctx.params.name - permission?: string; // coarse gate — the Keto Permission the caller must hold; checked before the handler runs - // Same as omitting `permission`, but stated outright so public is a deliberate choice rather than - // a forgotten gate. Mutually exclusive with `permission` (discovery refuses both). - public?: boolean; - // Any signed-in user, no grant to hold — for a plugin whose data is the visitor's own. Anonymous - // is bounced to /login, never 403. Mutually exclusive with the other two (discovery refuses both). - session?: boolean; } // A Keto Permission this plugin gates on — declared for docs/seeding. Names are a shared global diff --git a/src/ui/nav.ts b/src/ui/nav.ts index 689a450..f098a91 100644 --- a/src/ui/nav.ts +++ b/src/ui/nav.ts @@ -3,12 +3,13 @@ // A node is visible iff `allows` passes its gate; a gated header hides its whole subtree, and a pure // header left with no children is dropped. -import { allows } from "../auth/gate.ts"; +import { allows, type Gate } from "../auth/gate.ts"; import type { User } from "../http/context.ts"; import { ENGLISH } from "../i18n/english.ts"; import type { Translate } from "../i18n/translate.ts"; -export interface NavNode { +// `Gate` carries `permission`/`public`/`session` — consumed by the filter, never rendered. +export interface NavNode extends Gate { id?: string; // stable key for override targeting; stripped from the rendered tree children?: NavNode[]; count?: number; @@ -17,9 +18,6 @@ export interface NavNode { icon?: string; label: string; open?: boolean; - permission?: string; // required permission token; consumed by the filter, never rendered - public?: boolean; // show to everyone, signed in or not — the blessed alias for "no permission", stated outright; consumed by the filter, never rendered. Mutually exclusive with permission (discovery refuses both). - session?: boolean; // show to any signed-in user, no grant to hold; consumed by the filter, never rendered. Mutually exclusive with the other two (discovery refuses both). } // Central override (config/menu.ts). Targets nodes by `id`; applied rename → group → -- 2.52.0 From a17ed96b54629440e28a6648cc7ab431923daa92 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 08:17:10 +0200 Subject: [PATCH 03/10] Say gate where the code now gates, in the two contracts that still said permission --- examples/shifts-upstream/server.ts | 2 +- src/ui/nav.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/shifts-upstream/server.ts b/examples/shifts-upstream/server.ts index 8e6be97..a06e528 100644 --- a/examples/shifts-upstream/server.ts +++ b/examples/shifts-upstream/server.ts @@ -3,7 +3,7 @@ // of the app: stdlib only, in-memory (state resets on restart), no auth. Point PLUGIN_SETTING_SCHEDULING_UPSTREAM // at your real service in production. // -// GET /shifts → 200 [ { id, title, assignee, start, end }, … ] +// GET /shifts → 200 [ { id, title, assignee, start, end }, … ] (?assignee= → only theirs) // POST /shifts → 201 { id, … } (body: { title, assignee, start, end }) import { randomUUID } from "node:crypto"; diff --git a/src/ui/nav.ts b/src/ui/nav.ts index f098a91..4526fc4 100644 --- a/src/ui/nav.ts +++ b/src/ui/nav.ts @@ -21,7 +21,7 @@ export interface NavNode extends Gate { } // Central override (config/menu.ts). Targets nodes by `id`; applied rename → group → -// order → hide, then the per-user permission filter runs last. +// order → hide, then the per-user gate filter runs last. export interface NavOverride { groups?: NavGroupSpec[]; // wrap top-level nodes (by id) under a new header hide?: string[]; // remove nodes by id, at any depth (incl. a group's id) @@ -117,7 +117,7 @@ function filterByGate(nodes: NavNode[], user: User | null): NavNode[] { return out; } -// Strip the helper-only fields (id/permission) and drop absent ones, so the tree is exactly +// Strip the helper-only fields (id and the gate) and drop absent ones, so the tree is exactly // what nav-tree.ejs reads. Labels (a manifest's, or the central override's rename) pass through // `t` on the way out: a label that names a catalog key is translated, any other renders as written. function toRenderNode(n: NavNode, t: Translate): NavNode { -- 2.52.0 From bf146c07e7352111f4f89a305617a53aef139844 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 08:36:16 +0200 Subject: [PATCH 04/10] Bump the contract for the new gate, and close the ways it could read as open --- AGENTS.md | 3 +++ README.md | 15 +++++++-------- e2e-tests/visual.spec.ts | 6 ++++-- examples/plugins/admin/plugin.ts | 2 +- examples/plugins/scheduling/README.md | 4 ---- examples/plugins/scheduling/plugin.ts | 4 +--- examples/plugins/scheduling/shifts.ts | 3 ++- examples/plugins/scheduling/views/mine.ejs | 3 +-- release-tooling/contract-version.test.ts | 2 +- release-tooling/dockerhub-overview.md.tmpl | 2 +- src/auth/gate.ts | 14 +++++--------- src/auth/login.ts | 6 ++++-- src/http/app.test.ts | 7 +++++++ src/plugin-host/discovery.test.ts | 2 ++ src/plugin-host/discovery.ts | 18 ++++++++++++++---- src/plugin-host/plugin-api.ts | 1 - src/plugin-host/plugin.ts | 2 +- src/ui/chrome.ts | 5 ++--- views/index.ejs | 2 +- 19 files changed, 57 insertions(+), 44 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index adb1a36..27e75f0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -188,6 +188,9 @@ Revisit only if the stated reason stops holding. the visitor's own — their upstream account, their own tokens — has no distinction a permission could name; the alternative, granting every newly registered user a permission, couples the identity lifecycle to a Keto write that nothing retries when it fails. +- **The reference plugin's two shift pages duplicate a view model and markup on purpose.** An example + is read far more often than it is changed, and each page reads top to bottom on its own. **Valid + while `examples/plugins/scheduling` stays a teaching artifact rather than a maintained product.** - **A `:read`-only holder must never be shown a write affordance.** The list/detail models carry `canWrite` and the views drop create/save/delete/add/remove; the permission picker still renders, disabled, because *seeing* who holds what is the point of `:read`. A **write-intent GET** (a create diff --git a/README.md b/README.md index b5caf6e..eace79f 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ folder under `plugins/` goes live after a restart. Create `plugins/hello/plugin. import { definePlugin } from "@plainpages/plugin-api"; export default definePlugin({ - apiVersion: "0.3.0", + apiVersion: "0.4.0", nav: [{ href: "/hello", id: "hello", label: "Hello", public: true }], routes: [ { method: "GET", path: "/", public: true, handler: () => ({ html: "

Hello from my plugin

" }) }, @@ -349,7 +349,7 @@ import { definePlugin } from "@plainpages/plugin-api"; import { listThings, createThings } from "./handlers.ts"; export default definePlugin({ - apiVersion: "0.3.0", // semver string of the host contract this plugin was built against (see Versioning) + apiVersion: "0.4.0", // semver string of the host contract this plugin was built against (see Versioning) // Nav fragment, merged into the global menu and permission-filtered per user. // `icon` is a Lucide icon by its sprite id (src/ui/icons.ts). @@ -471,7 +471,7 @@ import { definePlugin } from "@plainpages/plugin-api"; import { landing, board } from "./pages.ts"; export default definePlugin({ - apiVersion: "0.3.0", + apiVersion: "0.4.0", home: landing, // owns "/" — the public front page dashboard: board, // owns "/dashboard" — the post-login app home }); @@ -570,9 +570,8 @@ system plugins you author or vendor. An ordinary domain plugin ignores it. A plugin's `nav` fragment is merged into the global menu by `composeNav` (`src/ui/nav.ts`), which applies the central override and then **filters per user** by the permissions in the session JWT: a -node shows iff it is `public`, is `session` and someone is signed in, declares no `permission`, or the -user holds that name. A node's `icon` -is a **Lucide icon** by sprite id (e.g. `i-cal` → lucide `calendar`); the available ids are +node shows iff it is `public`, is `session` and someone is signed in, declares no `permission`, or +the user holds that name. A node's `icon` is a **Lucide icon** by sprite id (e.g. `i-cal` → lucide `calendar`); the available ids are `ICON_NAMES` in `src/ui/icons.ts`, and adding one means registering its lucide name there. **Gating a section header.** A `permission` on the header takes the whole subtree with it. When the @@ -757,7 +756,7 @@ camel humps both becoming underscores — so `upstream` on the `scheduling` plug ```ts export default definePlugin({ - apiVersion: "0.3.0", + apiVersion: "0.4.0", settings: [ { key: "upstream", type: "url", required: true, description: "Base URL of the backend" }, { key: "pageSize", type: "number", default: 25 }, @@ -811,7 +810,7 @@ import { definePlugin } from "@plainpages/plugin-api"; let sql: ReturnType; export default definePlugin({ - apiVersion: "0.3.0", + apiVersion: "0.4.0", storage: true, hooks: { onBoot: async (boot) => { diff --git a/e2e-tests/visual.spec.ts b/e2e-tests/visual.spec.ts index 8d9d1f2..57f435a 100644 --- a/e2e-tests/visual.spec.ts +++ b/e2e-tests/visual.spec.ts @@ -180,8 +180,10 @@ test("the reference plugin: public Overview is open to all, My shifts takes any await expect(page.locator('.sidebar a[href="/scheduling/mine"]')).toHaveCount(1); // session gate: a session is enough // And the page itself renders for that same member, holding no permission at all. This stack runs - // no shifts upstream, so the list degrades to its empty state — which still names whose page it is. + // no shifts upstream, so it also pins the degraded page: the reason, never a 500 and never a claim + // about what is assigned. The working page is asserted against a real upstream in full-flow.spec. await page.goto("/scheduling/mine"); await expect(page.getByRole("heading", { name: "My shifts" })).toBeVisible(); - await expect(page.getByText("No shifts are assigned to demo@plainpages.local.")).toBeVisible(); + await expect(page.getByText("Couldn't reach the scheduling service")).toBeVisible(); + await expect(page.getByText("No shifts are assigned to")).toHaveCount(0); }); diff --git a/examples/plugins/admin/plugin.ts b/examples/plugins/admin/plugin.ts index d5ce0c6..6a407c0 100644 --- a/examples/plugins/admin/plugin.ts +++ b/examples/plugins/admin/plugin.ts @@ -28,7 +28,7 @@ const clients = on("oauth2-clients"); const pluginSettings = on("plugin-settings"); export default definePlugin({ - apiVersion: "0.3.0", // the host contract this was built against — a literal, never HOST_API_VERSION + apiVersion: "0.4.0", // the host contract this was built against — a literal, never HOST_API_VERSION nav: [ADMIN_NAV], diff --git a/examples/plugins/scheduling/README.md b/examples/plugins/scheduling/README.md index f0e142b..9a07bc9 100644 --- a/examples/plugins/scheduling/README.md +++ b/examples/plugins/scheduling/README.md @@ -26,10 +26,6 @@ What it demonstrates: The plugin holds **no state** — data lives upstream (README → *Stateless*). Handlers are thin and `fetch` is injectable, so they unit-test as pure functions (`shifts.test.ts`). -The shifts list and "My shifts" repeat a little view-model and markup rather than sharing a -parameterised one: an example is read far more often than it is changed, and each page is meant to be -followed top to bottom on its own. - ## Upstream Set `PLUGIN_SETTING_SCHEDULING_UPSTREAM` to your backend's base URL. The dev compose points it at a tiny in-memory diff --git a/examples/plugins/scheduling/plugin.ts b/examples/plugins/scheduling/plugin.ts index 9c6bcad..28090c2 100644 --- a/examples/plugins/scheduling/plugin.ts +++ b/examples/plugins/scheduling/plugin.ts @@ -12,7 +12,7 @@ let upstreamUrl = ""; const upstream = createUpstream(() => upstreamUrl); export default definePlugin({ - apiVersion: "0.3.0", // the host contract this was built against — a literal, never HOST_API_VERSION + apiVersion: "0.4.0", // the host contract this was built against — a literal, never HOST_API_VERSION // onBoot runs after discovery, before the server listens — where a plugin receives its resolved // settings. A malformed URL already failed the boot by then; the host validated the declared type. @@ -39,8 +39,6 @@ export default definePlugin({ { description: "Create and edit shifts", name: WRITE }, ], - // Mounted under /scheduling; `permission` gates before the handler runs. The overview is `public` - // (anyone may reach /scheduling, signed in or not); the rest need a permission. routes: [ { handler: overview(), method: "GET", path: "/", public: true }, { handler: myShifts(upstream), method: "GET", path: "/mine", session: true }, diff --git a/examples/plugins/scheduling/shifts.ts b/examples/plugins/scheduling/shifts.ts index 469d8b9..0eadfd9 100644 --- a/examples/plugins/scheduling/shifts.ts +++ b/examples/plugins/scheduling/shifts.ts @@ -218,7 +218,8 @@ export function buildMineModel(opts: { chrome: PageChrome; email: string; error? table: { caption: t("scheduling.mine.title"), columns: [{ label: t("scheduling.table.shift") }, { label: t("scheduling.table.start") }, { label: t("scheduling.table.end") }], - emptyText: t("scheduling.mine.empty", { email: opts.email }), + // Only when the upstream answered: a failed read knows nothing about what is assigned. + ...(opts.error === undefined ? { emptyText: t("scheduling.mine.empty", { email: opts.email }) } : {}), rows: opts.shifts.map((s) => ({ cells: [{ rowHeader: { text: s.title } }, s.start, s.end], name: s.title })), }, title: t("scheduling.mine.title"), diff --git a/examples/plugins/scheduling/views/mine.ejs b/examples/plugins/scheduling/views/mine.ejs index 57d94d3..f4bb88c 100644 --- a/examples/plugins/scheduling/views/mine.ejs +++ b/examples/plugins/scheduling/views/mine.ejs @@ -1,6 +1,5 @@ <%# - Scheduling · the visitor's own shifts (reference plugin). Reached behind `session: true`, so - ctx.user is always set by the time this renders. + Scheduling · the visitor's own shifts (reference plugin). Data: chrome, title, breadcrumbs, count, table, error? %><% const navHtml = include("partials/nav-tree", { nodes: chrome.nav }); diff --git a/release-tooling/contract-version.test.ts b/release-tooling/contract-version.test.ts index 65fec87..df5b356 100644 --- a/release-tooling/contract-version.test.ts +++ b/release-tooling/contract-version.test.ts @@ -12,7 +12,7 @@ test("readHostApiVersion pulls the constant out of the real source, and returns test("bumping HOST_API_VERSION is a deliberate act, so pin the shipped value", () => { // Not a substitute for the release gate — this test cannot see a tag. It is the tripwire that // makes an accidental edit fail here rather than at release time. - assert.equal(readHostApiVersion(readFileSync("src/plugin-host/plugin.ts", "utf8")), "0.3.0"); + assert.equal(readHostApiVersion(readFileSync("src/plugin-host/plugin.ts", "utf8")), "0.4.0"); }); test("every author-facing apiVersion sample matches the shipped contract", () => { diff --git a/release-tooling/dockerhub-overview.md.tmpl b/release-tooling/dockerhub-overview.md.tmpl index 988dac5..bd4a988 100644 --- a/release-tooling/dockerhub-overview.md.tmpl +++ b/release-tooling/dockerhub-overview.md.tmpl @@ -182,7 +182,7 @@ into the app. Create `plugins/hello/plugin.ts`: import { definePlugin } from "@plainpages/plugin-api"; export default definePlugin({ - apiVersion: "0.3.0", + apiVersion: "0.4.0", nav: [{ href: "/hello", id: "hello", label: "Hello", public: true }], routes: [ { method: "GET", path: "/", public: true, handler: () => ({ html: "

Hello from my plugin

" }) }, diff --git a/src/auth/gate.ts b/src/auth/gate.ts index 270c516..7c7857d 100644 --- a/src/auth/gate.ts +++ b/src/auth/gate.ts @@ -1,16 +1,13 @@ -// The coarse gate a route or nav node declares. One home for the rule, so the router and the menu -// can never disagree about what a visitor may reach. +// One home for the gate rule, so the router and the menu can never disagree about what a visitor +// may reach. README → Public pages & menu items. import type { User } from "../http/context.ts"; -// Widest first: whoever passes an earlier gate passes it without holding anything. const GATES = ["public", "session", "permission"] as const; -// A route or nav node names exactly one of these; discovery refuses two. Omitting all three is the -// same as `public`, which is why stating it outright makes an open gate a choice, not an oversight. export interface Gate { - permission?: string | undefined; // the Keto Permission the caller must hold, `:` - public?: boolean | undefined; // anyone, signed in or not - session?: boolean | undefined; // any signed-in user, no grant to hold; anonymous is sent to /login + permission?: string; // the Keto Permission the caller must hold, `:` + public?: boolean; // anyone, signed in or not + session?: boolean; // any signed-in user, no grant to hold; anonymous is sent to /login } export function allows(gate: Gate, user: User | null): boolean { @@ -19,7 +16,6 @@ export function allows(gate: Gate, user: User | null): boolean { return gate.permission == null || (user?.permissions.includes(gate.permission) ?? false); } -// Which gates a declaration sets — discovery refuses more than one, since they contradict. export function gatesSet(gate: Gate | null | undefined): string[] { if (gate == null) return []; return GATES.filter((name) => (name === "permission" ? gate.permission != null : gate[name] === true)); diff --git a/src/auth/login.ts b/src/auth/login.ts index 7cc55c2..d13d332 100644 --- a/src/auth/login.ts +++ b/src/auth/login.ts @@ -86,8 +86,10 @@ export interface Reminted { // anonymous instead of re-hitting Ory on every one. export async function remintSession(deps: LoginDeps, cookie: string | undefined, options: { secure?: boolean } = {}): Promise { const completed = await completeLogin(deps, cookie); - if (!completed) return { setCookie: clearSessionCookie(options), user: null }; - return { setCookie: sessionCookie(completed.jwt, options), user: { email: completed.email ?? "", id: completed.userId, permissions: completed.permissions } }; + // No email is no session, exactly as `claimsToUser` reads a token carrying none: a User with an + // empty email reads as anonymous in the shell, and is a blank key to whatever scopes on it. + if (!completed?.email) return { setCookie: clearSessionCookie(options), user: null }; + return { setCookie: sessionCookie(completed.jwt, options), user: { email: completed.email, id: completed.userId, permissions: completed.permissions } }; } // Build the Set-Cookie for our session JWT. HttpOnly + SameSite=Lax by default; `secure` is diff --git a/src/http/app.test.ts b/src/http/app.test.ts index b4041f6..5901282 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -609,6 +609,7 @@ test("guards map to responses: requireSession → /login, a failed can/check → { 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 + { handler: () => ({ html: "mine" }), method: "GET", path: "/mine", session: true }, // declarative session gate ], }; const app = createApp({ jwks: staticJwks([ecJwk]), plugins: [guarded] }); @@ -642,6 +643,12 @@ test("guards map to responses: requireSession → /login, a failed can/check → 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); + + // declarative `session` gate: anonymous → sign in, and any signed-in user through, grant or none. + const sAnon = await fetch(url + "/guarded/mine", { redirect: "manual" }); + assert.equal(sAnon.status, 303); + assert.equal(sAnon.headers.get("location"), "/login?return_to=%2Fguarded%2Fmine"); + assert.equal((await fetch(url + "/guarded/mine", auth([]))).status, 200); }); test("plugin hooks: onRequest can short-circuit a request and onResponse observes the handler result", async (t) => { diff --git a/src/plugin-host/discovery.test.ts b/src/plugin-host/discovery.test.ts index b47ccbc..0b7ef5d 100644 --- a/src/plugin-host/discovery.test.ts +++ b/src/plugin-host/discovery.test.ts @@ -64,6 +64,8 @@ const badCases: Array<{ name: string; files: Record; match: RegE { name: "a nav node marked public AND permission is contradictory", files: { "contranav/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "n", label: "N", public: true, permission: "x:read" }] };` }, match: /contranav.*public.*permission/s }, { name: "a route marked session AND permission is contradictory", files: { "contrasess/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", session: true, permission: "x:read", handler: () => ({ html: "x" }) }] };` }, match: /contrasess.*session.*permission/s }, { name: "a route marked public AND session is contradictory", files: { "contrapub/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", public: true, session: true, handler: () => ({ html: "x" }) }] };` }, match: /contrapub.*public.*session/s }, + { name: "a route whose session flag is a truthy non-boolean is refused, not read as ungated", files: { "truthy/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", session: "yes", handler: () => ({ html: "x" }) }] };` }, match: /truthy.*session.*true/s }, + { name: "a nav node whose public flag is a truthy non-boolean is refused too", files: { "truthynav/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "n", label: "N", public: 1 }] };` }, match: /truthynav.*public.*true/s }, { name: "a nav node marked session AND permission is contradictory", files: { "contrasessnav/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "n", label: "N", session: true, permission: "x:read" }] };` }, match: /contrasessnav.*session.*permission/s }, // A permission name is : wherever the manifest mentions one. Enforced here, not // only in the admin GUI, so it holds for a plugin installed without that GUI. diff --git a/src/plugin-host/discovery.ts b/src/plugin-host/discovery.ts index 8b8433c..db83441 100644 --- a/src/plugin-host/discovery.ts +++ b/src/plugin-host/discovery.ts @@ -7,7 +7,7 @@ import { existsSync, readdirSync, readFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; -import { gatesSet } from "../auth/gate.ts"; +import { type Gate, gatesSet } from "../auth/gate.ts"; import { checkApiVersion, findConflicts, isValidPermissionName, isValidPluginId, RESERVED_PLUGIN_IDS, type Plugin, type PluginManifest } from "./plugin.ts"; import { settingsDeclError } from "./settings.ts"; import { isValidStoragePluginId, MAX_STORAGE_PLUGIN_ID_LENGTH } from "./storage.ts"; @@ -147,9 +147,9 @@ function shapeError(manifest: PluginManifest): string | null { const settings = settingsDeclError(manifest.settings); if (settings) return settings; } - // Two gates on one route or nav node contradict each other — "open to all" vs "needs a session" - // vs "needs this permission". Refuse rather than silently pick one, so intent stays unambiguous. for (const route of Array.isArray(manifest.routes) ? manifest.routes : []) { + const flag = gateFlagError(`route "${route?.method} ${route?.path}"`, route); + if (flag) return flag; const gates = gatesSet(route); if (gates.length > 1) return `route "${route?.method} ${route?.path}" sets ${gates.join(" and ")}; a route names exactly one gate — public, session or permission`; } @@ -172,9 +172,19 @@ function shapeError(manifest: PluginManifest): string | null { return null; } -// Recurse the nav fragment: a node naming more than one gate is contradictory, same as a route. +// A truthy non-boolean sets no gate at all, so `session: "yes"` would read as an open page. +function gateFlagError(what: string, gate: Gate | null | undefined): string | null { + for (const flag of ["public", "session"] as const) { + const value = gate?.[flag]; + if (value !== undefined && typeof value !== "boolean") return `${what} sets ${flag} to ${JSON.stringify(value)}; a gate is declared with \`true\``; + } + return null; +} + function findNavGateContradiction(nodes: PluginManifest["nav"]): string | null { for (const node of Array.isArray(nodes) ? nodes : []) { + const flag = gateFlagError(`nav node "${node?.label ?? node?.id ?? "?"}"`, node); + if (flag) return flag; const gates = gatesSet(node); if (gates.length > 1) return `nav node "${node?.label ?? node?.id ?? "?"}" sets ${gates.join(" and ")}; a node names exactly one gate — public, session or permission`; const inChild = findNavGateContradiction(node?.children); diff --git a/src/plugin-host/plugin-api.ts b/src/plugin-host/plugin-api.ts index 70b6228..03d1ec7 100644 --- a/src/plugin-host/plugin-api.ts +++ b/src/plugin-host/plugin-api.ts @@ -39,7 +39,6 @@ export { CSRF_FIELD } from "../auth/csrf.ts"; // reference consumer. The Ory client types + their error classes are re-exported so a system // plugin can type against them and `instanceof`-match their errors. See README → System capabilities. export type { SystemCapabilities } from "./system.ts"; -export type { Gate } from "../auth/gate.ts"; export type { Identity, KratosAdmin, RecoveryCode } from "../auth/kratos-admin.ts"; export type { ExpandTree, KetoClient, RelationQuery, RelationTuple, SubjectSet } from "../auth/keto-client.ts"; export type { HydraAdmin, OAuth2Client } from "../auth/hydra-admin.ts"; diff --git a/src/plugin-host/plugin.ts b/src/plugin-host/plugin.ts index 84d749a..23547b8 100644 --- a/src/plugin-host/plugin.ts +++ b/src/plugin-host/plugin.ts @@ -11,7 +11,7 @@ import { envName, type SettingDecl, type SettingsOf } from "./settings.ts"; import type { StorageCredentials } from "./storage.ts"; // The Plainpages release this contract ships in — see README → Contract versioning. -export const HOST_API_VERSION = "0.3.0"; +export const HOST_API_VERSION = "0.4.0"; export type HttpMethod = "DELETE" | "GET" | "HEAD" | "PATCH" | "POST" | "PUT"; diff --git a/src/ui/chrome.ts b/src/ui/chrome.ts index ce0bf7b..984cd87 100644 --- a/src/ui/chrome.ts +++ b/src/ui/chrome.ts @@ -10,7 +10,7 @@ import { composeNav, type NavNode } from "./nav.ts"; import type { Plugin } from "../plugin-host/plugin.ts"; import { branding, shellUser, type ShellUser } from "./shell-context.ts"; -const DASHBOARD_NAV: NavNode = { href: "/dashboard", icon: "i-grid", id: "dashboard", label: "nav.dashboard" }; +const DASHBOARD_NAV: NavNode = { href: "/dashboard", icon: "i-grid", id: "dashboard", label: "nav.dashboard", session: true }; export interface PageChrome { brand: { logo?: string; name: string; sub?: string }; @@ -35,8 +35,7 @@ export interface ChromeOptions { export function buildPluginChrome(opts: ChromeOptions): PageChrome { const t = opts.t ?? ENGLISH; const carryLocale = opts.localeHref ?? ((href: string) => href); - // Dashboard is gated, so an anonymous click would only dead-end at /login. - const fragments: NavNode[][] = opts.user ? [[DASHBOARD_NAV]] : []; + const fragments: NavNode[][] = [[DASHBOARD_NAV]]; // A plugin's nav labels are keys in *its* catalog, so translate each fragment with that plugin's // translator before merging. composeNav then runs the core one over the result; already-translated // text passes through it. diff --git a/views/index.ejs b/views/index.ejs index 988cd16..29a9dee 100644 --- a/views/index.ejs +++ b/views/index.ejs @@ -14,7 +14,7 @@

${t("dashboard.starter.intro")}

${t("dashboard.starter.replace")}

export default definePlugin({
-  apiVersion: "0.3.0",
+  apiVersion: "0.4.0",
   // view names plugins/<id>/views/<view>.ejs, rendered in this same shell
   dashboard: (ctx) => ({ view: "dashboard", data: { /* … */ } }),
 });
-- 2.52.0 From 5d1e8f230900d295926484c600425c612694a629 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 08:50:44 +0200 Subject: [PATCH 05/10] Say the gate rule once, and key the example's shifts by the identifier it scopes on --- README.md | 8 ++++---- examples/plugins/scheduling/README.md | 2 +- examples/plugins/scheduling/shifts.test.ts | 2 -- examples/plugins/scheduling/shifts.ts | 3 --- examples/shifts-upstream/server.ts | 6 +++--- 5 files changed, 8 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index eace79f..8621d3c 100644 --- a/README.md +++ b/README.md @@ -231,6 +231,7 @@ Against the reference plugins' actual routes: | Request | Gate | alice | bob | carol | anonymous | | --- | --- | --- | --- | --- | --- | | `GET /scheduling` | `public: true` | ✅ | ✅ | ✅ | ✅ | +| `GET /scheduling/mine` | `session: true` | ✅ | ✅ | ✅ | → `/login` | | `GET /scheduling/shifts` | `scheduling:read` | ✅ | ✅ | 403 | → `/login` | | `GET /scheduling/shifts/new` | `scheduling:write` | ✅ | 403 | 403 | → `/login` | | `POST /scheduling/shifts` | `scheduling:write` | ✅ | 403 | 403 | → `/login` | @@ -585,10 +586,9 @@ A route or nav node marked **`public: true`** is reachable by anyone and shows i That is the same as omitting `permission`, but stated outright so public is a deliberate choice rather than a forgotten gate. -**`session: true`** takes any signed-in user, with no grant to hold — the gate for a plugin whose -data is the visitor's own (their upstream account, their own tokens), where a permission would name -a distinction that does not exist. An anonymous visitor is bounced to `/login` with the page as -`return_to`, exactly as a permission gate does. +**`session: true`** takes any signed-in user, with no grant to hold — for a plugin whose data is the +visitor's own. An anonymous visitor is bounced to `/login` with the page as `return_to`, exactly as a +permission gate does. A declaration names **exactly one** of the three; two of them contradict, and discovery refuses the plugin at boot. diff --git a/examples/plugins/scheduling/README.md b/examples/plugins/scheduling/README.md index 9a07bc9..c78c70d 100644 --- a/examples/plugins/scheduling/README.md +++ b/examples/plugins/scheduling/README.md @@ -39,7 +39,7 @@ Your backend must expose two routes; the plugin treats any non-2xx as a recovera | Route | Request | Success | Response body | | --- | --- | --- | --- | -| `GET /shifts` | `Accept: application/json`, optional `?assignee=` | `200` | JSON array of `{ id, title, assignee, start, end }` (all strings; missing fields coerce to `""`). With `assignee`, only that person's rows — "My shifts" asks for them rather than filtering everyone's here, because ownership is the backend's rule to enforce | +| `GET /shifts` | `Accept: application/json`, optional `?assignee=` | `200` | JSON array of `{ id, title, assignee, start, end }` (all strings; missing fields coerce to `""`). With `assignee`, only that person's rows | | `POST /shifts` | JSON body `{ title, assignee, start, end }` | `2xx` | ignored (the plugin POST-redirect-GETs back to the list) | Domain rules (overlap, capacity, time ordering) live in your backend — reject with a 4xx and the diff --git a/examples/plugins/scheduling/shifts.test.ts b/examples/plugins/scheduling/shifts.test.ts index e848bee..403652e 100644 --- a/examples/plugins/scheduling/shifts.test.ts +++ b/examples/plugins/scheduling/shifts.test.ts @@ -182,8 +182,6 @@ test("my shifts asks the upstream for the visitor's own rows, and names them in const upstream = fakeUpstream({ list: async (opts) => { asked = opts; return [mine]; } }); const r = asView(await myShifts(upstream)(fakeCtx({ url: "http://localhost/scheduling/mine", user }))); assert.equal(r.view, "mine"); - // The ownership rule is the upstream's: the page asks for one person's rows rather than filtering - // everyone's here, so a real backend never hands this handler another visitor's shifts. assert.deepEqual(asked, { assignee: "Blair.Mora@example.test" }); const table = r.data["table"] as { emptyText: string; rows: { name: string }[] }; assert.deepEqual(table.rows.map((row) => row.name), ["Evening on-call"]); diff --git a/examples/plugins/scheduling/shifts.ts b/examples/plugins/scheduling/shifts.ts index 0eadfd9..fe4a66e 100644 --- a/examples/plugins/scheduling/shifts.ts +++ b/examples/plugins/scheduling/shifts.ts @@ -190,9 +190,6 @@ export function newShiftForm(): RouteHandler { return (ctx) => ({ data: buildFormModel({ chrome: ctx.chrome, t: ctx.t }), view: "shift-new" }); } -// The `session: true` archetype: the rows are the visitor's own, so there is no distinction a -// permission could name — anyone signed in sees theirs and only theirs. The scoping is the -// upstream's, never a filter here: it owns the data and answers for one person's rows. export function myShifts(upstream: ShiftsUpstream): RouteHandler { return async (ctx) => { const user = requireSession(ctx); diff --git a/examples/shifts-upstream/server.ts b/examples/shifts-upstream/server.ts index a06e528..c534504 100644 --- a/examples/shifts-upstream/server.ts +++ b/examples/shifts-upstream/server.ts @@ -12,9 +12,9 @@ import { createServer } from "node:http"; const PORT = Number(process.env.PORT ?? 4000); const shifts = [ - { id: randomUUID(), title: "Morning — Front desk", assignee: "Avery Kline", start: "2026-06-22 08:00", end: "2026-06-22 12:00" }, - { id: randomUUID(), title: "Afternoon — Support", assignee: "Blair Mora", start: "2026-06-22 12:00", end: "2026-06-22 17:00" }, - { id: randomUUID(), title: "Evening — On-call", assignee: "Casey Nguyen", start: "2026-06-22 17:00", end: "2026-06-22 22:00" }, + { id: randomUUID(), title: "Morning — Front desk", assignee: "avery.kline@plainpages.local", start: "2026-06-22 08:00", end: "2026-06-22 12:00" }, + { id: randomUUID(), title: "Afternoon — Support", assignee: "blair.mora@plainpages.local", start: "2026-06-22 12:00", end: "2026-06-22 17:00" }, + { id: randomUUID(), title: "Evening — On-call", assignee: "casey.nguyen@plainpages.local", start: "2026-06-22 17:00", end: "2026-06-22 22:00" }, { id: randomUUID(), title: "Night — Escalations", assignee: "admin@plainpages.local", start: "2026-06-22 22:00", end: "2026-06-23 06:00" }, ]; -- 2.52.0 From dfb043c3bdec673036064e8b4f3a7f76822b8203 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 12:32:51 +0200 Subject: [PATCH 06/10] Read one gate everywhere, and check a declaration's gate in one pass --- src/auth/login.ts | 5 +++- src/http/app.ts | 20 ++++++++------ src/http/builtin-routes.ts | 5 +++- src/plugin-host/discovery.ts | 49 ++++++++++++----------------------- src/plugin-host/plugin-api.ts | 2 ++ 5 files changed, 38 insertions(+), 43 deletions(-) diff --git a/src/auth/login.ts b/src/auth/login.ts index d13d332..9f15624 100644 --- a/src/auth/login.ts +++ b/src/auth/login.ts @@ -88,7 +88,10 @@ export async function remintSession(deps: LoginDeps, cookie: string | undefined, const completed = await completeLogin(deps, cookie); // No email is no session, exactly as `claimsToUser` reads a token carrying none: a User with an // empty email reads as anonymous in the shell, and is a blank key to whatever scopes on it. - if (!completed?.email) return { setCookie: clearSessionCookie(options), user: null }; + if (!completed?.email) { + if (completed) currentLog()?.warn("session dropped: identity has no email", { sub: completed.userId }); + return { setCookie: clearSessionCookie(options), user: null }; + } return { setCookie: sessionCookie(completed.jwt, options), user: { email: completed.email, id: completed.userId, permissions: completed.permissions } }; } diff --git a/src/http/app.ts b/src/http/app.ts index 9d21a78..0903df8 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -28,7 +28,7 @@ import { DEFAULT_MENU, type MenuConfig } from "../ui/menu-config.ts"; import { declaredPermissions, type Plugin, type RouteHandler, type RouteResult } from "../plugin-host/plugin.ts"; import type { PluginSettings } from "../plugin-host/settings.ts"; import type { SystemCapabilities } from "../plugin-host/system.ts"; -import { allows } from "../auth/gate.ts"; +import { allows, type Gate } from "../auth/gate.ts"; import { allowedMethods, matchRoute } from "../plugin-host/router.ts"; import { buildAuthRoutes } from "../auth/routes.ts"; import { securityHeaders } from "./security-headers.ts"; @@ -157,7 +157,6 @@ export function createApp(options: AppOptions = {}): Server { // "/dashboard", gated to a signed-in user. A plugin may own it via `dashboard`; else the built-in // starter page. const serveDashboard = async (ctx: RequestContext, csrf: RequestCsrf, contextFor: PluginContextFactory): Promise => { - if (!ctx.user) return { redirect: loginRedirect(ctx), status: 303 }; // The page carries the Sign-out form, so Set-Cookie a fresh CSRF token here when absent. csrf.setCookie(); if (dashboardPlugin) { @@ -175,7 +174,7 @@ export function createApp(options: AppOptions = {}): Server { const builtinRoutes: BuiltinRoute[] = [ ...buildAuthRoutes({ hydra, keto, kratos, kratosAdmin, menu, secureCookies }), { handler: serveHome, method: "GET", path: "/" }, - { handler: serveDashboard, method: "GET", path: "/dashboard" }, + { handler: serveDashboard, method: "GET", path: "/dashboard", session: true }, ]; // The request handler. Run inside runWithLog (below) so the per-request logger is ambient: every @@ -279,15 +278,19 @@ export function createApp(options: AppOptions = {}): Server { } } + // Anonymous → sign in, remembering the page as return_to; a signed-in user who simply lacks + // the permission gets the 403 page. + const refuse = async (gate: Gate, gateCtx: RequestContext): Promise => { + if (!gateCtx.user) { res.writeHead(303, { location: carryLocale(loginRedirect(gateCtx)) }).end(); return; } + reqLog.warn("forbidden: missing permission", { path: pathname, required: gate.permission ?? "", sub: gateCtx.user.id }); + sendHtml(res, 403, await renderPage("403", {})); + }; + const match = matchRoute(plugins, method, pathname); if (match) { const routeCtx = contextFor(match.plugin.id, match.params); if (!allows(match.route, routeCtx.user)) { - // Anonymous → sign in, remembering the page as return_to; a signed-in user who simply - // lacks the permission gets the 403 page. - if (!routeCtx.user) { res.writeHead(303, { location: loginRedirect(routeCtx) }).end(); return; } - reqLog.warn("forbidden: missing permission", { path: pathname, required: match.route.permission ?? "", sub: routeCtx.user.id }); - sendHtml(res, 403, await renderPage("403", {})); + await refuse(match.route, routeCtx); return; } csrfMint.setCookie(); @@ -301,6 +304,7 @@ export function createApp(options: AppOptions = {}): Server { const builtin = matchBuiltinRoute(builtinRoutes, method, pathname); if (builtin) { + if (!allows(builtin, ctx.user)) { await refuse(builtin, ctx); return; } await sendResult(res, await builtin.handler(ctx, csrfMint, contextFor), viewsFor(ctx), carryLocale); return; } diff --git a/src/http/builtin-routes.ts b/src/http/builtin-routes.ts index b1877a9..055ec8e 100644 --- a/src/http/builtin-routes.ts +++ b/src/http/builtin-routes.ts @@ -3,6 +3,7 @@ // mint (host-only — a plugin reads the token via ctx.chrome instead). app.ts matches this table // after plugin routes — exact path, a GET route also answering HEAD like the plugin router — and // pipes the result through sendResult against the core views. +import type { Gate } from "../auth/gate.ts"; import type { RequestContext } from "./context.ts"; import type { RouteResult } from "../plugin-host/plugin.ts"; @@ -19,7 +20,9 @@ export interface RequestCsrf { // own context — otherwise the plugin's keys render as bare keys on the pages it owns. export type PluginContextFactory = (pluginId: string) => RequestContext; -export interface BuiltinRoute { +// `Gate` carries `permission`/`public`/`session`, checked before the handler runs — the same rule +// the plugin router and the menu read. +export interface BuiltinRoute extends Gate { // Returns a RouteResult, or null when the handler wrote to ctx.res itself // (the landing slots dispatch a plugin's own result against that plugin's views). handler: (ctx: RequestContext, csrf: RequestCsrf, contextFor: PluginContextFactory) => Promise | RouteResult | null; diff --git a/src/plugin-host/discovery.ts b/src/plugin-host/discovery.ts index db83441..7c137d8 100644 --- a/src/plugin-host/discovery.ts +++ b/src/plugin-host/discovery.ts @@ -148,57 +148,40 @@ function shapeError(manifest: PluginManifest): string | null { if (settings) return settings; } for (const route of Array.isArray(manifest.routes) ? manifest.routes : []) { - const flag = gateFlagError(`route "${route?.method} ${route?.path}"`, route); - if (flag) return flag; - const gates = gatesSet(route); - if (gates.length > 1) return `route "${route?.method} ${route?.path}" sets ${gates.join(" and ")}; a route names exactly one gate — public, session or permission`; - } - const navContradiction = findNavGateContradiction(manifest.nav); - if (navContradiction) return navContradiction; - // Every permission name the manifest mentions — gated on or declared — must be `:`. - // A bare word names a role, and roles are groups here (README → Naming a permission). - for (const route of Array.isArray(manifest.routes) ? manifest.routes : []) { - if (route?.permission != null && !isValidPermissionName(route.permission)) { - return `route "${route.method} ${route.path}" gates on "${route.permission}"; a permission name is :, e.g. "things:read"`; - } + const gate = gateError(`route "${route?.method} ${route?.path}"`, route); + if (gate) return gate; } + const navGate = findNavGateError(manifest.nav); + if (navGate) return navGate; for (const decl of Array.isArray(manifest.permissions) ? manifest.permissions : []) { if (decl?.name == null || !isValidPermissionName(decl.name)) { return `declared permission "${decl?.name}" is not :, e.g. "things:read"`; } } - const navPermission = findInvalidNavPermission(manifest.nav); - if (navPermission) return navPermission; return null; } -// A truthy non-boolean sets no gate at all, so `session: "yes"` would read as an open page. -function gateFlagError(what: string, gate: Gate | null | undefined): string | null { +// Every rule a declaration's gate must satisfy. A truthy non-boolean sets no gate at all, so +// `session: "yes"` would read as an open page; a permission name is `:` because a +// bare word names a role, and roles are groups here (README → Naming a permission). +function gateError(what: string, gate: Gate | null | undefined): string | null { for (const flag of ["public", "session"] as const) { const value = gate?.[flag]; if (value !== undefined && typeof value !== "boolean") return `${what} sets ${flag} to ${JSON.stringify(value)}; a gate is declared with \`true\``; } - return null; -} - -function findNavGateContradiction(nodes: PluginManifest["nav"]): string | null { - for (const node of Array.isArray(nodes) ? nodes : []) { - const flag = gateFlagError(`nav node "${node?.label ?? node?.id ?? "?"}"`, node); - if (flag) return flag; - const gates = gatesSet(node); - if (gates.length > 1) return `nav node "${node?.label ?? node?.id ?? "?"}" sets ${gates.join(" and ")}; a node names exactly one gate — public, session or permission`; - const inChild = findNavGateContradiction(node?.children); - if (inChild) return inChild; + const gates = gatesSet(gate); + if (gates.length > 1) return `${what} sets ${gates.join(" and ")}; name one gate — public, session or permission`; + if (gate?.permission != null && !isValidPermissionName(gate.permission)) { + return `${what} gates on "${gate.permission}"; a permission name is :, e.g. "things:read"`; } return null; } -function findInvalidNavPermission(nodes: PluginManifest["nav"]): string | null { +function findNavGateError(nodes: PluginManifest["nav"]): string | null { for (const node of Array.isArray(nodes) ? nodes : []) { - if (node?.permission != null && !isValidPermissionName(node.permission)) { - return `nav node "${node.label ?? node.id ?? "?"}" gates on "${node.permission}"; a permission name is :, e.g. "things:read"`; - } - const inChild = findInvalidNavPermission(node?.children); + const err = gateError(`nav node "${node?.label ?? node?.id ?? "?"}"`, node); + if (err) return err; + const inChild = findNavGateError(node?.children); if (inChild) return inChild; } return null; diff --git a/src/plugin-host/plugin-api.ts b/src/plugin-host/plugin-api.ts index 03d1ec7..e3adaca 100644 --- a/src/plugin-host/plugin-api.ts +++ b/src/plugin-host/plugin-api.ts @@ -14,6 +14,8 @@ export type { RequestContext, User } from "../http/context.ts"; export type { PageChrome } from "../ui/chrome.ts"; export type { NavNode } from "../ui/nav.ts"; export { can, check, GuardError, requireSession } from "../auth/guards.ts"; +// The three coarse gates a route or nav node may declare — `Route` and `NavNode` both extend it. +export type { Gate } from "../auth/gate.ts"; // Translation: `ctx.t` and the view-level `t(...)` do the work at runtime — these are for // authoring a plugin's own catalogs (plugins//i18n/.ts) and for building a translator // in a unit test. `PluralMessage` types a message that varies with a count. -- 2.52.0 From 6a0d11d9d31998fdfd1cba5a4430d86bbf25212b Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 14:00:04 +0200 Subject: [PATCH 07/10] Name exactly one gate on every declaration, and own a shift by identity id --- AGENTS.md | 12 ++++-- README.md | 45 +++++++++++----------- e2e-tests/full-flow.spec.ts | 8 ++-- examples/plugins/admin/admin-shared.ts | 1 + examples/plugins/scheduling/README.md | 10 +++-- examples/plugins/scheduling/plugin.ts | 1 + examples/plugins/scheduling/shifts.test.ts | 19 ++++----- examples/plugins/scheduling/shifts.ts | 15 +++++--- examples/shifts-upstream/server.ts | 21 +++++----- src/auth/gate.test.ts | 2 +- src/auth/routes.ts | 16 ++++---- src/http/app.ts | 2 +- src/plugin-host/discovery.test.ts | 10 +++-- src/plugin-host/discovery.ts | 12 +++--- 14 files changed, 98 insertions(+), 76 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 27e75f0..c2d4e5a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -182,12 +182,16 @@ Revisit only if the stated reason stops holding. example it keeps the route table and the in-handler guard deriving from one function, so 29 routes × 2 gate sites cannot drift. Generalised, it would make authorization a function of the transport verb — a route table must answer "what does this need?" on its own. -- **A gate is one of three, and `session` is a first-class one.** A route or nav node names exactly - one of `public`, `session`, `permission` (discovery refuses two), and `src/auth/gate.ts` is the one - home of the rule the router and the menu both read. `session` exists because a plugin whose data is +- **A gate is one of three, named exactly once, and `session` is a first-class one.** A route or nav + node names exactly one of `public`, `session`, `permission` — discovery refuses none, two, and a + flag spelled anything but `true`, so a forgotten gate fails the boot rather than publishing a page. + `src/auth/gate.ts` is the one home of the rule the plugin router, the host's own route table and + the menu all read. `session` exists because a plugin whose data is the visitor's own — their upstream account, their own tokens — has no distinction a permission could name; the alternative, granting every newly registered user a permission, couples the identity - lifecycle to a Keto write that nothing retries when it fails. + lifecycle to a Keto write that nothing retries when it fails. A page scoped to "mine" joins on + `ctx.user.id`, never the email — an address is user-changeable and can be reassigned to someone + who would then inherit the previous holder's rows. - **The reference plugin's two shift pages duplicate a view model and markup on purpose.** An example is read far more often than it is changed, and each page reads top to bottom on its own. **Valid while `examples/plugins/scheduling` stays a teaching artifact rather than a maintained product.** diff --git a/README.md b/README.md index 8621d3c..e4d42d8 100644 --- a/README.md +++ b/README.md @@ -352,7 +352,7 @@ import { listThings, createThings } from "./handlers.ts"; export default definePlugin({ apiVersion: "0.4.0", // semver string of the host contract this plugin was built against (see Versioning) - // Nav fragment, merged into the global menu and permission-filtered per user. + // Nav fragment, merged into the global menu and gate-filtered per user. // `icon` is a Lucide icon by its sprite id (src/ui/icons.ts). nav: [{ href: "/things", icon: "i-cal", id: "things:list", label: "Things", permission: "things:read" }], @@ -362,7 +362,7 @@ export default definePlugin({ { description: "Create and edit things", name: "things:write" }, ], - // Route handlers, mounted under the plugin's path (/things). `permission` gates first. + // Route handlers, mounted under the plugin's path (/things). The gate runs first. routes: [ { method: "GET", path: "/", permission: "things:read", handler: listThings }, { method: "POST", path: "/", permission: "things:write", handler: createThings }, @@ -379,7 +379,7 @@ folder-derived `id` to produce the loaded `Plugin`. | `apiVersion` | yes | Semver string of the host contract the plugin was built against. See [Versioning](#contract-versioning). | | `home` | no | A `RouteHandler` that owns the **public** landing `/`. At most one plugin may declare it. See [The landing pages](#the-landing-pages-home--dashboard). | | `dashboard` | no | A `RouteHandler` that owns the **gated** app home `/dashboard`. At most one plugin may declare it. See [The landing pages](#the-landing-pages-home--dashboard). | -| `nav` | no | `NavNode[]` fragment (same shape `composeNav` consumes). `icon` is a Lucide sprite id (`src/ui/icons.ts`); node `id`s must be globally unique. A `label` that names a catalog key is [translated](#languages-i18n); anything else renders as written. | +| `nav` | no | `NavNode[]` fragment (same shape `composeNav` consumes). Every node names [exactly one gate](#public-pages--menu-items). `icon` is a Lucide sprite id (`src/ui/icons.ts`); node `id`s must be globally unique. A `label` that names a catalog key is [translated](#languages-i18n); anything else renders as written. | | `permissions` | no | Permissions this plugin gates on. See [Nav & permission gates](#nav--permission-gates). | | `routes` | no | See [Routes & handlers](#routes--handlers). | | `hooks` | no | See [Hooks](#hooks). | @@ -390,14 +390,13 @@ A plugin may be routes-only, nav-only, or hooks-only — every collection field ### Routes & handlers -A route is `{ method, path, permission?, public?, session?, handler }`. `path` is **relative to the plugin's +A route is `{ method, path, handler }` plus [exactly one gate](#public-pages--menu-items) — +`permission`, `public: true` or `session: true`. `path` is **relative to the plugin's mount path `/`** (so `path: "/:id"` in the `things` plugin serves `/things/:id`); the host matches `method` + the resolved full path, extracts `:name` segments into `ctx.params.name`, runs the -`permission` gate ([a coarse JWT-claim check](#nav--permission-gates)), then calls the handler with +gate ([a coarse JWT-claim check](#nav--permission-gates)), then calls the handler with the [request context](#requestcontext). A failed gate redirects an **anonymous** visitor to `/login` with the page as `return_to`; a **signed-in** user lacking the permission gets the **403** page. -`public: true` means no gate at all, `session: true` any signed-in user (see -[Public pages](#public-pages--menu-items)). `method` is one of `GET HEAD POST PUT PATCH DELETE`. A `GET` route also answers `HEAD`. @@ -571,27 +570,28 @@ system plugins you author or vendor. An ordinary domain plugin ignores it. A plugin's `nav` fragment is merged into the global menu by `composeNav` (`src/ui/nav.ts`), which applies the central override and then **filters per user** by the permissions in the session JWT: a -node shows iff it is `public`, is `session` and someone is signed in, declares no `permission`, or -the user holds that name. A node's `icon` is a **Lucide icon** by sprite id (e.g. `i-cal` → lucide `calendar`); the available ids are -`ICON_NAMES` in `src/ui/icons.ts`, and adding one means registering its lucide name there. +node shows iff it is `public`, is `session` and someone is signed in, or names a `permission` the +user holds. A node's `icon` is a **Lucide icon** by sprite id (e.g. `i-cal` → lucide `calendar`); the +available ids are `ICON_NAMES` in `src/ui/icons.ts`, and adding one means registering its lucide name +there. **Gating a section header.** A `permission` on the header takes the whole subtree with it. When the -children need *different* permissions, leave the header ungated and gate each child — `composeNav` -drops a header whose children all filtered out. That only works while the header carries **no -`href`**: give it one and it survives as an ungated leaf, visible to everyone. +children need *different* permissions, mark the header `public: true` — it then gates nothing, each +child decides, and `composeNav` drops a header whose children all filtered out. That only works while +the header carries **no `href`**: give it one and it survives as a leaf, visible to everyone. #### Public pages & menu items -A route or nav node marked **`public: true`** is reachable by anyone and shows in everyone's menu. -That is the same as omitting `permission`, but stated outright so public is a deliberate choice -rather than a forgotten gate. +A route or nav node marked **`public: true`** is reachable by anyone and shows in everyone's menu — +open stated outright, so it is a deliberate choice rather than a forgotten gate. **`session: true`** takes any signed-in user, with no grant to hold — for a plugin whose data is the visitor's own. An anonymous visitor is bounced to `/login` with the page as `return_to`, exactly as a permission gate does. -A declaration names **exactly one** of the three; two of them contradict, and discovery refuses the -plugin at boot. +Every route and nav node names **exactly one** of the three, spelled `true` (or a permission name). +Naming none, naming two, or spelling one `false` is refused at boot — so a forgotten gate fails the +plugin instead of publishing a page. A public page still renders in the native shell; for an anonymous visitor `ctx.user` is `null`, the shell shows a **Sign in** link in place of the profile block, the gated **Dashboard** link is hidden, @@ -644,7 +644,7 @@ The host detects collisions across all discovered plugins with `findConflicts` a Mount-path uniqueness needs no rule of its own — it follows from the id check. Discovery also rejects **per-manifest shape errors**: a non-array `nav`/`routes`/`permissions`, a non-function `home`/`dashboard`, a permission name that isn't [`:`](#naming-a-permission), or a -route/nav node naming more than one gate. +route/nav node that does not name [exactly one gate](#public-pages--menu-items). ### Hooks @@ -920,10 +920,9 @@ The menu is **driven entirely by config** and assembled from two sources: export default defineMenu({ branding: { name: "Acme Ops" }, override: { hide: ["teams"] } }); ``` -Every nav item may carry a `permission`; the rendered tree is **filtered per user** from the session -JWT (no per-request authz call), so the menu only shows what that person can reach. An item may -instead be **`public: true`** (everyone) or **`session: true`** (anyone signed in) — one gate per -item, never two. +Every nav item names one gate — a `permission`, **`public: true`** (everyone) or **`session: true`** +(anyone signed in); the rendered tree is **filtered per user** from the session JWT (no per-request +authz call), so the menu only shows what that person can reach. Branding (name, logo, default theme) renders in the app shell. **One menu, one shell, everywhere.** A single menu (`src/ui/chrome.ts` `buildPluginChrome`) renders diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index 6dc8723..a66831e 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -194,11 +194,13 @@ test.describe.serial("authenticated admin journey", () => { await expect(page.locator("h1")).toHaveText("Shifts"); await expect(page.locator("table")).toContainText("Morning — Front desk"); // seeded by the mock upstream - // The session-gated page asks the upstream for this visitor's rows: the one seeded against the - // signed-in admin is there, and another person's shift is not. + // The session-gated page scopes the upstream read by the visitor's identity id. The demo + // upstream's rows belong to three made-up people, so the admin's own page is empty — which is + // the assertion that matters: nobody else's shifts come back. (A matching row rendering is + // covered where it is exact, in the plugin's own unit test.) await page.goto("/scheduling/mine"); await expect(page.locator("h1")).toHaveText("My shifts"); - await expect(page.locator("table")).toContainText("Night — Escalations"); + await expect(page.getByText("No shifts are assigned to admin@plainpages.local")).toBeVisible(); await expect(page.locator("table")).not.toContainText("Morning — Front desk"); }); diff --git a/examples/plugins/admin/admin-shared.ts b/examples/plugins/admin/admin-shared.ts index 97b28fd..3c41068 100644 --- a/examples/plugins/admin/admin-shared.ts +++ b/examples/plugins/admin/admin-shared.ts @@ -49,6 +49,7 @@ export const ADMIN_NAV: NavNode = { icon: "i-shield", id: "admin", label: "admin.nav.section", // a key in this plugin's catalog; the host translates nav labels + public: true, // the header gates nothing; every child needs a permission, and an empty header is dropped }; // The screen gate: a signed-in user holding this request's `:`. Each route already diff --git a/examples/plugins/scheduling/README.md b/examples/plugins/scheduling/README.md index c78c70d..5986d8d 100644 --- a/examples/plugins/scheduling/README.md +++ b/examples/plugins/scheduling/README.md @@ -18,6 +18,9 @@ What it demonstrates: - **All three route gates** — the Overview is `public` (anyone), "My shifts" is `session` (any signed-in visitor, showing only rows assigned to them), and "Shifts" is gated on `scheduling:read` / `scheduling:write`; a leaf whose gate a visitor fails is invisible in the menu. +- **Ownership joined on the identity id** — "My shifts" asks the upstream for `assigneeId=ctx.user.id`, + the opaque subject id, and renders the row's separate `assignee` display name. An email address is + user-changeable and can be reassigned to someone else, who would then inherit those rows. - **Its own translations** — every string comes from `i18n/en-US.ts` (`sv-SE.ts` beside it), including the nav labels, which are catalog keys in the manifest. `shifts.count` shows a plural message, and the views carry the visitor's language onto their links with `localeHref()`. @@ -39,8 +42,8 @@ Your backend must expose two routes; the plugin treats any non-2xx as a recovera | Route | Request | Success | Response body | | --- | --- | --- | --- | -| `GET /shifts` | `Accept: application/json`, optional `?assignee=` | `200` | JSON array of `{ id, title, assignee, start, end }` (all strings; missing fields coerce to `""`). With `assignee`, only that person's rows | -| `POST /shifts` | JSON body `{ title, assignee, start, end }` | `2xx` | ignored (the plugin POST-redirect-GETs back to the list) | +| `GET /shifts` | `Accept: application/json`, optional `?assigneeId=` | `200` | JSON array of `{ id, title, assignee, assigneeId, start, end }` (all strings; missing fields coerce to `""`). With `assigneeId`, only that person's rows | +| `POST /shifts` | JSON body `{ title, assignee, assigneeId?, start, end }` | `2xx` | ignored (the plugin POST-redirect-GETs back to the list) | 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. @@ -54,4 +57,5 @@ cosmetically) — normalise to your backend's format there if it matters. A user sees the shift list once they hold the `scheduling:read` permission in Keto (and `scheduling:write` to create). The one-command bootstrap grants both to the demo admin, so the seeded `admin@plainpages.local` can use it immediately. "My shifts" needs no grant at all — signing in is -its whole gate. +its whole gate; it lists the rows this demo upstream holds against the signed-in visitor's id, and +the demo's seeded rows belong to three made-up people, so a freshly seeded admin sees it empty. diff --git a/examples/plugins/scheduling/plugin.ts b/examples/plugins/scheduling/plugin.ts index 28090c2..215a24d 100644 --- a/examples/plugins/scheduling/plugin.ts +++ b/examples/plugins/scheduling/plugin.ts @@ -31,6 +31,7 @@ export default definePlugin({ icon: "i-cal", id: "scheduling", label: "scheduling.nav.section", + public: true, // the header gates nothing; each child names its own gate, and an empty header is dropped }], // Roles this plugin introduces (docs + Keto seeding). Namespaced `:`. diff --git a/examples/plugins/scheduling/shifts.test.ts b/examples/plugins/scheduling/shifts.test.ts index 403652e..88fe3f0 100644 --- a/examples/plugins/scheduling/shifts.test.ts +++ b/examples/plugins/scheduling/shifts.test.ts @@ -25,8 +25,8 @@ function fakeCtx(opts: { body?: string; permissions?: string[]; url?: string; us } const SHIFTS: Shift[] = [ - { assignee: "Avery Kline", end: "12:00", id: "1", start: "08:00", title: "Morning desk" }, - { assignee: "Blair Mora", end: "17:00", id: "2", start: "12:00", title: "Afternoon support" }, + { assignee: "Avery Kline", assigneeId: "019bdc1a-3f27-7c41-9a6e-2b1d4f8e05a3", end: "12:00", id: "1", start: "08:00", title: "Morning desk" }, + { assignee: "Blair Mora", assigneeId: "019bdc1a-4a83-7de2-8f05-6c93a71be4d8", end: "17:00", id: "2", start: "12:00", title: "Afternoon support" }, ]; const fakeUpstream = (over: Partial = {}): ShiftsUpstream => ({ create: async () => {}, list: async () => SHIFTS, ...over }); @@ -63,11 +63,11 @@ test("createUpstream.list fetches /shifts, asks for JSON, and maps the rows", as const http = (async (url, init) => { seen = String(url); assert.equal((init?.headers as Record).accept, "application/json"); - return new Response(JSON.stringify([{ assignee: "A", end: "2", id: "x", start: "1", title: "T", extra: "ignored" }]), { status: 200 }); + return new Response(JSON.stringify([{ assignee: "A", assigneeId: "019bdc1a-5b6e-7a90-b3c7-84f01d2ea9b6", end: "2", id: "x", start: "1", title: "T", extra: "ignored" }]), { status: 200 }); }) as typeof fetch; const shifts = await createUpstream(() => "http://up:4000/", http).list(); // trailing slash trimmed assert.equal(seen, "http://up:4000/shifts"); - assert.deepEqual(shifts, [{ assignee: "A", end: "2", id: "x", start: "1", title: "T" }]); + assert.deepEqual(shifts, [{ assignee: "A", assigneeId: "019bdc1a-5b6e-7a90-b3c7-84f01d2ea9b6", end: "2", id: "x", start: "1", title: "T" }]); }); test("createUpstream throws UpstreamError carrying the status on a non-2xx", async () => { @@ -174,19 +174,20 @@ test("buildFormModel marks title/assignee required and attaches field errors", ( // ---- the session-gated page: the visitor's own rows ---- -test("my shifts asks the upstream for the visitor's own rows, and names them in the empty state", async () => { +test("my shifts scopes the upstream read by the visitor's id, and names them in the empty state", async () => { const user: User = { email: "Blair.Mora@example.test", id: "01a06091-baa3-71f4-a068-4879972979ff", permissions: [] }; - const mine: Shift = { assignee: "blair.mora@example.test", end: "22:00", id: "3", start: "17:00", title: "Evening on-call" }; - let asked: { assignee?: string } | undefined; + const mine: Shift = { assignee: "Blair Mora", assigneeId: user.id, end: "22:00", id: "3", start: "17:00", title: "Evening on-call" }; + let asked: { assigneeId?: string } | undefined; const upstream = fakeUpstream({ list: async (opts) => { asked = opts; return [mine]; } }); const r = asView(await myShifts(upstream)(fakeCtx({ url: "http://localhost/scheduling/mine", user }))); assert.equal(r.view, "mine"); - assert.deepEqual(asked, { assignee: "Blair.Mora@example.test" }); + assert.deepEqual(asked, { assigneeId: "01a06091-baa3-71f4-a068-4879972979ff" }); // the id, never the address const table = r.data["table"] as { emptyText: string; rows: { name: string }[] }; assert.deepEqual(table.rows.map((row) => row.name), ["Evening on-call"]); assert.match(table.emptyText, /Blair\.Mora@example\.test/); // an empty page still says whose it is - // The route carries `session: true`, but the handler asserts the session itself rather than trusting it. + // `requireSession` narrows `ctx.user` from `User | null` to `User` — the one part of the route's + // `session: true` guarantee the contract cannot state in the handler's type. await assert.rejects(async () => { await myShifts(fakeUpstream())(fakeCtx()); }, GuardError); }); diff --git a/examples/plugins/scheduling/shifts.ts b/examples/plugins/scheduling/shifts.ts index fe4a66e..e278d75 100644 --- a/examples/plugins/scheduling/shifts.ts +++ b/examples/plugins/scheduling/shifts.ts @@ -22,7 +22,8 @@ export const WRITE = "scheduling:write"; // the permission gating create export interface Shift { id: string; - assignee: string; + assignee: string; // display name, rendered in the table + assigneeId: string; // who the shift belongs to — an opaque id, the same one `ctx.user.id` carries end: string; start: string; title: string; @@ -47,9 +48,9 @@ export class UpstreamError extends Error { export interface ShiftsUpstream { create(input: ShiftInput): Promise; - // `assignee` scopes the read at the source, which is where an ownership rule belongs (README → + // `assigneeId` scopes the read at the source, which is where an ownership rule belongs (README → // Three tiers of "may I?"); without it the caller would hold everyone's rows to render one page. - list(opts?: { assignee?: string }): Promise; + list(opts?: { assigneeId?: string }): Promise; } // REST client over the upstream service (a stand-in for the customer's real backend). `fetch` @@ -69,7 +70,7 @@ export function createUpstream(baseUrl: () => string, fetchImpl: typeof fetch = if (!res.ok) throw new UpstreamError(`create shift failed (${res.status})`, res.status); }, async list(opts = {}) { - const query = opts.assignee == null ? "" : `?${new URLSearchParams({ assignee: opts.assignee })}`; + const query = opts.assigneeId == null ? "" : `?${new URLSearchParams({ assigneeId: opts.assigneeId })}`; const res = await fetchImpl(`${base()}/shifts${query}`, { headers: { accept: "application/json" } }); if (!res.ok) throw new UpstreamError(`list shifts failed (${res.status})`, res.status); const data: unknown = await res.json(); @@ -82,7 +83,7 @@ const str = (v: unknown): string => (typeof v === "string" ? v : v == null ? "" function toShift(raw: unknown): Shift { const r = (raw ?? {}) as Record; - return { assignee: str(r["assignee"]), end: str(r["end"]), id: str(r["id"]), start: str(r["start"]), title: str(r["title"]) }; + return { assignee: str(r["assignee"]), assigneeId: str(r["assigneeId"]), end: str(r["end"]), id: str(r["id"]), start: str(r["start"]), title: str(r["title"]) }; } // ---- view models (pure; the EJS views read these) ----------------------------------- @@ -196,7 +197,9 @@ export function myShifts(upstream: ShiftsUpstream): RouteHandler { let shifts: Shift[] = []; let error: string | undefined; try { - shifts = await upstream.list({ assignee: user.email }); + // Join on the id, never the email: an address is user-changeable and can be reassigned to + // someone else, which would hand them the previous holder's rows. + shifts = await upstream.list({ assigneeId: user.id }); } catch (err) { ctx.log.warn("scheduling upstream unreachable", { error: String(err) }); error = ctx.t("scheduling.upstream.list"); diff --git a/examples/shifts-upstream/server.ts b/examples/shifts-upstream/server.ts index c534504..8e85c21 100644 --- a/examples/shifts-upstream/server.ts +++ b/examples/shifts-upstream/server.ts @@ -3,19 +3,20 @@ // of the app: stdlib only, in-memory (state resets on restart), no auth. Point PLUGIN_SETTING_SCHEDULING_UPSTREAM // at your real service in production. // -// GET /shifts → 200 [ { id, title, assignee, start, end }, … ] (?assignee= → only theirs) -// POST /shifts → 201 { id, … } (body: { title, assignee, start, end }) +// GET /shifts → 200 [ { id, title, assigneeId, assignee, start, end }, … ] (?assigneeId= → only theirs) +// POST /shifts → 201 { id, … } (body: { title, assignee, assigneeId?, start, end }) import { randomUUID } from "node:crypto"; import { createServer } from "node:http"; const PORT = Number(process.env.PORT ?? 4000); +// `assigneeId` is the identity the rows are owned by — an opaque, stable subject id, which is what +// `ctx.user.id` carries. These are this demo's own people; a real backend joins on your IdP's ids. const shifts = [ - { id: randomUUID(), title: "Morning — Front desk", assignee: "avery.kline@plainpages.local", start: "2026-06-22 08:00", end: "2026-06-22 12:00" }, - { id: randomUUID(), title: "Afternoon — Support", assignee: "blair.mora@plainpages.local", start: "2026-06-22 12:00", end: "2026-06-22 17:00" }, - { id: randomUUID(), title: "Evening — On-call", assignee: "casey.nguyen@plainpages.local", start: "2026-06-22 17:00", end: "2026-06-22 22:00" }, - { id: randomUUID(), title: "Night — Escalations", assignee: "admin@plainpages.local", start: "2026-06-22 22:00", end: "2026-06-23 06:00" }, + { id: randomUUID(), title: "Morning — Front desk", assigneeId: "019bdc1a-3f27-7c41-9a6e-2b1d4f8e05a3", assignee: "Avery Kline", start: "2026-06-22 08:00", end: "2026-06-22 12:00" }, + { id: randomUUID(), title: "Afternoon — Support", assigneeId: "019bdc1a-4a83-7de2-8f05-6c93a71be4d8", assignee: "Blair Mora", start: "2026-06-22 12:00", end: "2026-06-22 17:00" }, + { id: randomUUID(), title: "Evening — On-call", assigneeId: "019bdc1a-5b6e-7a90-b3c7-84f01d2ea9b6", assignee: "Casey Nguyen", start: "2026-06-22 17:00", end: "2026-06-22 22:00" }, ]; const json = (res, status, body) => { @@ -35,13 +36,13 @@ const readBody = (req) => createServer(async (req, res) => { const url = new URL(req.url ?? "/", "http://localhost"); if (url.pathname === "/shifts" && req.method === "GET") { - const assignee = url.searchParams.get("assignee"); - if (assignee === null) return json(res, 200, shifts); - return json(res, 200, shifts.filter((s) => s.assignee.toLowerCase() === assignee.toLowerCase())); + const assigneeId = url.searchParams.get("assigneeId"); + if (assigneeId === null) return json(res, 200, shifts); + return json(res, 200, shifts.filter((s) => s.assigneeId === assigneeId)); } if (url.pathname === "/shifts" && req.method === "POST") { const b = await readBody(req); - const shift = { id: randomUUID(), assignee: String(b.assignee ?? ""), end: String(b.end ?? ""), start: String(b.start ?? ""), title: String(b.title ?? "") }; + const shift = { id: randomUUID(), assignee: String(b.assignee ?? ""), assigneeId: String(b.assigneeId ?? ""), end: String(b.end ?? ""), start: String(b.start ?? ""), title: String(b.title ?? "") }; shifts.push(shift); return json(res, 201, shift); } diff --git a/src/auth/gate.test.ts b/src/auth/gate.test.ts index fe6e50c..0c133cb 100644 --- a/src/auth/gate.test.ts +++ b/src/auth/gate.test.ts @@ -23,6 +23,6 @@ test("gatesSet names the gates a declaration sets, so discovery can refuse more assert.deepEqual(gatesSet({ session: true }), ["session"]); assert.deepEqual(gatesSet({ permission: "x:read", public: true }), ["public", "permission"]); assert.deepEqual(gatesSet({ permission: "x:read", public: true, session: true }), ["public", "session", "permission"]); - // `false` is not a gate — only a set one counts, so { session: false } is an ungated route. + // Only `true` sets a gate, so a manifest spelling one `false` names none — which discovery refuses. assert.deepEqual(gatesSet({ public: false, session: false }), []); }); diff --git a/src/auth/routes.ts b/src/auth/routes.ts index da64706..df810e0 100644 --- a/src/auth/routes.ts +++ b/src/auth/routes.ts @@ -240,20 +240,20 @@ export function buildAuthRoutes({ hydra, keto, kratos, kratosAdmin, menu, secure const routes: BuiltinRoute[] = []; if (kratos) { for (const [path, flowType] of Object.entries(AUTH_FLOWS)) { - routes.push({ handler: flowPage(kratos, flowType, secureCookies), method: "GET", path }); + routes.push({ handler: flowPage(kratos, flowType, secureCookies), method: "GET", path, public: true }); } - routes.push({ handler: logout(kratos, secureCookies), method: "POST", path: "/logout" }); + routes.push({ handler: logout(kratos, secureCookies), method: "POST", path: "/logout", public: true }); } if (hydra && kratos) { const provider = { hydra, kratos }; - routes.push({ handler: oauthLogin(provider, secureCookies), method: "GET", path: "/oauth2/login" }); - routes.push({ handler: consentScreen(provider, menu.branding.name), method: "GET", path: "/oauth2/consent" }); - routes.push({ handler: consentDecision(provider), method: "POST", path: "/oauth2/consent" }); + routes.push({ handler: oauthLogin(provider, secureCookies), method: "GET", path: "/oauth2/login", public: true }); + routes.push({ handler: consentScreen(provider, menu.branding.name), method: "GET", path: "/oauth2/consent", public: true }); + routes.push({ handler: consentDecision(provider), method: "POST", path: "/oauth2/consent", public: true }); } - if (hydra) routes.push({ handler: oauthLogout(hydra), method: "GET", path: "/oauth2/logout" }); + if (hydra) routes.push({ handler: oauthLogout(hydra), method: "GET", path: "/oauth2/logout", public: true }); if (kratos && kratosAdmin && keto) { - routes.push({ handler: completeAuth({ keto, kratosAdmin, kratosPublic: kratos }, secureCookies), method: "GET", path: "/auth/complete" }); + routes.push({ handler: completeAuth({ keto, kratosAdmin, kratosPublic: kratos }, secureCookies), method: "GET", path: "/auth/complete", public: true }); } - routes.push({ handler: errorSink, method: "GET", path: "/error" }); + routes.push({ handler: errorSink, method: "GET", path: "/error", public: true }); return routes; } diff --git a/src/http/app.ts b/src/http/app.ts index 0903df8..6a65e9c 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -173,7 +173,7 @@ export function createApp(options: AppOptions = {}): Server { // routes.ts, capability-gated on the wired clients) plus the two landing slots above. const builtinRoutes: BuiltinRoute[] = [ ...buildAuthRoutes({ hydra, keto, kratos, kratosAdmin, menu, secureCookies }), - { handler: serveHome, method: "GET", path: "/" }, + { handler: serveHome, method: "GET", path: "/", public: true }, { handler: serveDashboard, method: "GET", path: "/dashboard", session: true }, ]; diff --git a/src/plugin-host/discovery.test.ts b/src/plugin-host/discovery.test.ts index 0b7ef5d..76d2665 100644 --- a/src/plugin-host/discovery.test.ts +++ b/src/plugin-host/discovery.test.ts @@ -20,8 +20,8 @@ function scaffold(t: TestContext, files: Record): string { } const full = (id: string): string => - `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "${id}:root", label: "${id}" }], ` + - `routes: [{ method: "GET", path: "/", handler: () => ({ html: "${id}" }) }] };`; + `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "${id}:root", label: "${id}", public: true }], ` + + `routes: [{ method: "GET", path: "/", public: true, handler: () => ({ html: "${id}" }) }] };`; test("a missing plugins/ dir means zero plugins, not an error (clean clone)", async () => { assert.deepEqual(await discoverPlugins({ dir: join(tmpdir(), "pp-does-not-exist-xyz") }), []); @@ -67,6 +67,10 @@ const badCases: Array<{ name: string; files: Record; match: RegE { name: "a route whose session flag is a truthy non-boolean is refused, not read as ungated", files: { "truthy/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", session: "yes", handler: () => ({ html: "x" }) }] };` }, match: /truthy.*session.*true/s }, { name: "a nav node whose public flag is a truthy non-boolean is refused too", files: { "truthynav/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "n", label: "N", public: 1 }] };` }, match: /truthynav.*public.*true/s }, { name: "a nav node marked session AND permission is contradictory", files: { "contrasessnav/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "n", label: "N", session: true, permission: "x:read" }] };` }, match: /contrasessnav.*session.*permission/s }, + // A gate is named, never forgotten: a route or node without one would be an open page nobody chose. + { name: "a route naming no gate at all is refused, not served to everyone", files: { "nogate/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", handler: () => ({ html: "x" }) }] };` }, match: /nogate.*names no gate/s }, + { name: "a nav node naming no gate at all is refused too — a section header says `public` outright", files: { "nogatenav/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "n", label: "N" }] };` }, match: /nogatenav.*names no gate/s }, + { name: "a gate set to false is refused — it reads as a gate but sets none", files: { "falsegate/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", public: false, handler: () => ({ html: "x" }) }] };` }, match: /falsegate.*public.*true/s }, // A permission name is : wherever the manifest mentions one. Enforced here, not // only in the admin GUI, so it holds for a plugin installed without that GUI. { name: "a route gating on a bare word", files: { "bare/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", permission: "admin", handler: () => ({ html: "x" }) }] };` }, match: /bare.*admin.*:/s }, @@ -139,7 +143,7 @@ test("a plugin may carry its own package.json, node_modules and dependencies", a "shop/node_modules/price-tag/package.json": `{ "name": "price-tag", "version": "1.0.0", "type": "module", "exports": "./index.js" }`, "shop/node_modules/price-tag/index.js": `export default (n) => \`\${n} kr\`;`, "shop/plugin.ts": `import { definePlugin } from "@plainpages/plugin-api";\nimport price from "price-tag";\n` + - `export default definePlugin({ apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", handler: () => ({ html: price(20) }) }] });`, + `export default definePlugin({ apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", public: true, handler: () => ({ html: price(20) }) }] });`, }); const plugins = await discoverPlugins({ dir }); diff --git a/src/plugin-host/discovery.ts b/src/plugin-host/discovery.ts index 7c137d8..9b73049 100644 --- a/src/plugin-host/discovery.ts +++ b/src/plugin-host/discovery.ts @@ -161,16 +161,18 @@ function shapeError(manifest: PluginManifest): string | null { return null; } -// Every rule a declaration's gate must satisfy. A truthy non-boolean sets no gate at all, so -// `session: "yes"` would read as an open page; a permission name is `:` because a -// bare word names a role, and roles are groups here (README → Naming a permission). +// Every rule a declaration's gate must satisfy. Exactly one gate, always: a missing one would be an +// open page nobody chose, and anything but `true` (a `false`, a `"yes"`) sets no gate while looking +// like it does. A permission name is `:` because a bare word names a role, and +// roles are groups here (README → Naming a permission). function gateError(what: string, gate: Gate | null | undefined): string | null { for (const flag of ["public", "session"] as const) { const value = gate?.[flag]; - if (value !== undefined && typeof value !== "boolean") return `${what} sets ${flag} to ${JSON.stringify(value)}; a gate is declared with \`true\``; + if (value !== undefined && value !== true) return `${what} sets ${flag} to ${JSON.stringify(value)}; a gate is declared with \`true\``; } const gates = gatesSet(gate); - if (gates.length > 1) return `${what} sets ${gates.join(" and ")}; name one gate — public, session or permission`; + if (gates.length === 0) return `${what} names no gate; name exactly one — public, session or permission`; + if (gates.length > 1) return `${what} sets ${gates.join(" and ")}; name exactly one — public, session or permission`; if (gate?.permission != null && !isValidPermissionName(gate.permission)) { return `${what} gates on "${gate.permission}"; a permission name is :, e.g. "things:read"`; } -- 2.52.0 From 390ac5f112b276d09aca710e26208707b89d1927 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 14:03:06 +0200 Subject: [PATCH 08/10] Say where the one-gate rule applies, and record why allows stays open --- AGENTS.md | 4 +++- README.md | 7 ++++--- examples/plugins/admin/admin-shared.ts | 7 +++---- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c2d4e5a..960e72f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -186,7 +186,9 @@ Revisit only if the stated reason stops holding. node names exactly one of `public`, `session`, `permission` — discovery refuses none, two, and a flag spelled anything but `true`, so a forgotten gate fails the boot rather than publishing a page. `src/auth/gate.ts` is the one home of the rule the plugin router, the host's own route table and - the menu all read. `session` exists because a plugin whose data is + the menu all read. Exactly-one-gate is a discovery-time rule on manifests, not a runtime invariant: + `allows({}, user)` stays open **by design**, because the central override's `groups` builds header + nodes that carry no gate. Making `allows` fail closed would hide every operator-grouped section. `session` exists because a plugin whose data is the visitor's own — their upstream account, their own tokens — has no distinction a permission could name; the alternative, granting every newly registered user a permission, couples the identity lifecycle to a Keto write that nothing retries when it fails. A page scoped to "mine" joins on diff --git a/README.md b/README.md index e4d42d8..9a30aba 100644 --- a/README.md +++ b/README.md @@ -920,9 +920,10 @@ The menu is **driven entirely by config** and assembled from two sources: export default defineMenu({ branding: { name: "Acme Ops" }, override: { hide: ["teams"] } }); ``` -Every nav item names one gate — a `permission`, **`public: true`** (everyone) or **`session: true`** -(anyone signed in); the rendered tree is **filtered per user** from the session JWT (no per-request -authz call), so the menu only shows what that person can reach. +Every nav node a **plugin** declares names one gate — a `permission`, **`public: true`** (everyone) +or **`session: true`** (anyone signed in); a header this override groups takes none, and shows +whenever a child does. The rendered tree is **filtered per user** from the session JWT (no +per-request authz call), so the menu only shows what that person can reach. Branding (name, logo, default theme) renders in the app shell. **One menu, one shell, everywhere.** A single menu (`src/ui/chrome.ts` `buildPluginChrome`) renders diff --git a/examples/plugins/admin/admin-shared.ts b/examples/plugins/admin/admin-shared.ts index 3c41068..8e8b424 100644 --- a/examples/plugins/admin/admin-shared.ts +++ b/examples/plugins/admin/admin-shared.ts @@ -35,10 +35,9 @@ export function actionForMethod(method: string): AdminAction { return verb === "GET" || verb === "HEAD" ? "read" : "write"; } -// The plugin's nav fragment: an ungated "Admin" header + its three screens, each gated on its own -// read permission. The header carries no `permission` because a user may hold one screen's and not -// another's; composeNav drops a header left with no visible children, so a user holding none of the -// three never sees the section. The host current-marks the active item — no `current`/`open` here. +// The plugin's nav fragment: the "Admin" header + its four screens, each gated on its own read +// permission. composeNav drops a header left with no visible children, so a user holding none of +// them never sees the section. The host current-marks the active item — no `current`/`open` here. export const ADMIN_NAV: NavNode = { children: [ { href: ADMIN_USERS_BASE, icon: "i-users", id: "users", label: "admin.nav.users", permission: permissionName("users", "read") }, -- 2.52.0 From 18dc4f3136f8909572d429d8e172a8c2376f22f6 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 18:24:56 +0200 Subject: [PATCH 09/10] Refuse an emailless identity where the session is minted, and rows the upstream should not have sent --- AGENTS.md | 7 ++++--- e2e-tests/full-flow.spec.ts | 5 +---- e2e-tests/visual.spec.ts | 5 ++--- examples/plugins/scheduling/README.md | 2 +- examples/plugins/scheduling/shifts.test.ts | 15 +++++++++++++++ examples/plugins/scheduling/shifts.ts | 8 ++++---- examples/plugins/scheduling/views/mine.ejs | 2 +- examples/shifts-upstream/server.ts | 4 ++-- src/auth/login.test.ts | 16 ++++++++++++++-- src/auth/login.ts | 17 +++++++++-------- src/auth/routes.test.ts | 7 ++++++- src/http/builtin-routes.ts | 2 -- src/plugin-host/plugin.ts | 1 - src/ui/nav.ts | 1 - 14 files changed, 59 insertions(+), 33 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 960e72f..ca83fd3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -186,9 +186,10 @@ Revisit only if the stated reason stops holding. node names exactly one of `public`, `session`, `permission` — discovery refuses none, two, and a flag spelled anything but `true`, so a forgotten gate fails the boot rather than publishing a page. `src/auth/gate.ts` is the one home of the rule the plugin router, the host's own route table and - the menu all read. Exactly-one-gate is a discovery-time rule on manifests, not a runtime invariant: - `allows({}, user)` stays open **by design**, because the central override's `groups` builds header - nodes that carry no gate. Making `allows` fail closed would hide every operator-grouped section. `session` exists because a plugin whose data is + the menu all read. Exactly-one-gate is a discovery-time rule on manifests, not a runtime + invariant: `allows({}, user)` stays open **by design**, because the central override's `groups` + builds header nodes that carry no gate. Making `allows` fail closed would hide every + operator-grouped section. `session` exists because a plugin whose data is the visitor's own — their upstream account, their own tokens — has no distinction a permission could name; the alternative, granting every newly registered user a permission, couples the identity lifecycle to a Keto write that nothing retries when it fails. A page scoped to "mine" joins on diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index a66831e..b043e7d 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -194,10 +194,7 @@ test.describe.serial("authenticated admin journey", () => { await expect(page.locator("h1")).toHaveText("Shifts"); await expect(page.locator("table")).toContainText("Morning — Front desk"); // seeded by the mock upstream - // The session-gated page scopes the upstream read by the visitor's identity id. The demo - // upstream's rows belong to three made-up people, so the admin's own page is empty — which is - // the assertion that matters: nobody else's shifts come back. (A matching row rendering is - // covered where it is exact, in the plugin's own unit test.) + // The admin owns none of the demo's rows, so an empty page is the no-leak assertion. await page.goto("/scheduling/mine"); await expect(page.locator("h1")).toHaveText("My shifts"); await expect(page.getByText("No shifts are assigned to admin@plainpages.local")).toBeVisible(); diff --git a/e2e-tests/visual.spec.ts b/e2e-tests/visual.spec.ts index 57f435a..01c2720 100644 --- a/e2e-tests/visual.spec.ts +++ b/e2e-tests/visual.spec.ts @@ -179,9 +179,8 @@ test("the reference plugin: public Overview is open to all, My shifts takes any await expect(page.locator('.sidebar a[href="/scheduling/shifts"]')).toHaveCount(0); // gated leaf filtered out await expect(page.locator('.sidebar a[href="/scheduling/mine"]')).toHaveCount(1); // session gate: a session is enough - // And the page itself renders for that same member, holding no permission at all. This stack runs - // no shifts upstream, so it also pins the degraded page: the reason, never a 500 and never a claim - // about what is assigned. The working page is asserted against a real upstream in full-flow.spec. + // No shifts upstream on this stack, so this also pins the degraded page: the reason, never a 500 + // and never a claim about what is assigned. await page.goto("/scheduling/mine"); await expect(page.getByRole("heading", { name: "My shifts" })).toBeVisible(); await expect(page.getByText("Couldn't reach the scheduling service")).toBeVisible(); diff --git a/examples/plugins/scheduling/README.md b/examples/plugins/scheduling/README.md index 5986d8d..b43d96d 100644 --- a/examples/plugins/scheduling/README.md +++ b/examples/plugins/scheduling/README.md @@ -43,7 +43,7 @@ Your backend must expose two routes; the plugin treats any non-2xx as a recovera | Route | Request | Success | Response body | | --- | --- | --- | --- | | `GET /shifts` | `Accept: application/json`, optional `?assigneeId=` | `200` | JSON array of `{ id, title, assignee, assigneeId, start, end }` (all strings; missing fields coerce to `""`). With `assigneeId`, only that person's rows | -| `POST /shifts` | JSON body `{ title, assignee, assigneeId?, start, end }` | `2xx` | ignored (the plugin POST-redirect-GETs back to the list) | +| `POST /shifts` | JSON body `{ title, assignee, start, end }` | `2xx` | ignored (the plugin POST-redirect-GETs back to the list) | 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. diff --git a/examples/plugins/scheduling/shifts.test.ts b/examples/plugins/scheduling/shifts.test.ts index 88fe3f0..62f547f 100644 --- a/examples/plugins/scheduling/shifts.test.ts +++ b/examples/plugins/scheduling/shifts.test.ts @@ -191,3 +191,18 @@ test("my shifts scopes the upstream read by the visitor's id, and names them in // `session: true` guarantee the contract cannot state in the handler's type. await assert.rejects(async () => { await myShifts(fakeUpstream())(fakeCtx()); }, GuardError); }); + +test("my shifts degrades to the reason alone when the upstream is down, claiming nothing about what is assigned", async () => { + const user: User = { email: "Blair.Mora@example.test", id: "01a06091-baa3-71f4-a068-4879972979ff", permissions: [] }; + const down = fakeUpstream({ list: async () => { throw new UpstreamError("down", 503); } }); + const r = asView(await myShifts(down)(fakeCtx({ url: "http://localhost/scheduling/mine", user }))); + assert.match(String(r.data["error"]), /scheduling service/i); + assert.deepEqual((r.data["table"] as { rows: unknown[] }).rows, []); // mine.ejs drops the count + table while `error` is set +}); + +test("my shifts drops a row the upstream returned that is not the visitor's", async () => { + const user: User = { email: "Blair.Mora@example.test", id: "01a06091-baa3-71f4-a068-4879972979ff", permissions: [] }; + const theirs: Shift = { assignee: "Avery Kline", assigneeId: "019bdc1a-3f27-7c41-9a6e-2b1d4f8e05a3", end: "12:00", id: "9", start: "08:00", title: "Not mine" }; + const r = asView(await myShifts(fakeUpstream({ list: async () => [theirs] }))(fakeCtx({ url: "http://localhost/scheduling/mine", user }))); + assert.deepEqual((r.data["table"] as { rows: unknown[] }).rows, []); // a backend ignoring the scope must not leak through this page +}); diff --git a/examples/plugins/scheduling/shifts.ts b/examples/plugins/scheduling/shifts.ts index e278d75..e8984dc 100644 --- a/examples/plugins/scheduling/shifts.ts +++ b/examples/plugins/scheduling/shifts.ts @@ -198,8 +198,9 @@ export function myShifts(upstream: ShiftsUpstream): RouteHandler { let error: string | undefined; try { // Join on the id, never the email: an address is user-changeable and can be reassigned to - // someone else, which would hand them the previous holder's rows. - shifts = await upstream.list({ assigneeId: user.id }); + // someone else, which would hand them the previous holder's rows. The re-filter is + // defence-in-depth: a backend that ignores an unknown query param would answer with everyone. + shifts = (await upstream.list({ assigneeId: user.id })).filter((s) => s.assigneeId === user.id); } catch (err) { ctx.log.warn("scheduling upstream unreachable", { error: String(err) }); error = ctx.t("scheduling.upstream.list"); @@ -218,8 +219,7 @@ export function buildMineModel(opts: { chrome: PageChrome; email: string; error? table: { caption: t("scheduling.mine.title"), columns: [{ label: t("scheduling.table.shift") }, { label: t("scheduling.table.start") }, { label: t("scheduling.table.end") }], - // Only when the upstream answered: a failed read knows nothing about what is assigned. - ...(opts.error === undefined ? { emptyText: t("scheduling.mine.empty", { email: opts.email }) } : {}), + emptyText: t("scheduling.mine.empty", { email: opts.email }), rows: opts.shifts.map((s) => ({ cells: [{ rowHeader: { text: s.title } }, s.start, s.end], name: s.title })), }, title: t("scheduling.mine.title"), diff --git a/examples/plugins/scheduling/views/mine.ejs b/examples/plugins/scheduling/views/mine.ejs index f4bb88c..d92dc20 100644 --- a/examples/plugins/scheduling/views/mine.ejs +++ b/examples/plugins/scheduling/views/mine.ejs @@ -7,7 +7,7 @@ const alertHtml = locals.error ? include("partials/alert", { text: locals.error, tone: "neg" }) : ""; -%> <%- include("partials/shell", { - body: '
' + alertHtml + '

' + count + '

' + tableHtml + '
', + body: '
' + alertHtml + (locals.error ? '' : '

' + count + '

' + tableHtml) + '
', brand: chrome.brand, breadcrumbs, csrfToken: chrome.csrfToken, diff --git a/examples/shifts-upstream/server.ts b/examples/shifts-upstream/server.ts index 8e85c21..959f3e6 100644 --- a/examples/shifts-upstream/server.ts +++ b/examples/shifts-upstream/server.ts @@ -4,7 +4,7 @@ // at your real service in production. // // GET /shifts → 200 [ { id, title, assigneeId, assignee, start, end }, … ] (?assigneeId= → only theirs) -// POST /shifts → 201 { id, … } (body: { title, assignee, assigneeId?, start, end }) +// POST /shifts → 201 { id, … } (body: { title, assignee, start, end }) import { randomUUID } from "node:crypto"; import { createServer } from "node:http"; @@ -42,7 +42,7 @@ createServer(async (req, res) => { } if (url.pathname === "/shifts" && req.method === "POST") { const b = await readBody(req); - const shift = { id: randomUUID(), assignee: String(b.assignee ?? ""), assigneeId: String(b.assigneeId ?? ""), end: String(b.end ?? ""), start: String(b.start ?? ""), title: String(b.title ?? "") }; + const shift = { id: randomUUID(), assignee: String(b.assignee ?? ""), assigneeId: "", end: String(b.end ?? ""), start: String(b.start ?? ""), title: String(b.title ?? "") }; shifts.push(shift); return json(res, 201, shift); } diff --git a/src/auth/login.test.ts b/src/auth/login.test.ts index abbe3d7..5327bad 100644 --- a/src/auth/login.test.ts +++ b/src/auth/login.test.ts @@ -92,12 +92,24 @@ test("completeLogin returns null and touches nothing when there is no active ses assert.equal(touched, false); }); -test("completeLogin maps a missing email trait to null and throws if the tokenizer yields no JWT", async () => { - const identity: Identity = { id: ID, traits: {} }; +test("completeLogin throws if the tokenizer yields no JWT", async () => { + const identity: Identity = { id: ID, traits: { email: "admin@plainpages.local" } }; const kratosPublic = publicStub({ whoami: async () => ({ active: true, identity }) as Session }); // never returns a tokenized JWT await assert.rejects(completeLogin({ keto: ketoStub(), kratosAdmin: adminStub(), kratosPublic }, "c"), /tokenizer returned no JWT/); }); +// An identity with no email is no session, decided here so /auth/complete and remintSession cannot +// disagree: `claimsToUser` reads a token carrying none as anonymous, so minting one would hand the +// browser a cookie every later request refuses. +test("completeLogin refuses an identity carrying no email, before it mints anything", async () => { + const identity: Identity = { id: ID, traits: {} }; + let touched = false; + const kratosAdmin = adminStub({ updateMetadataPublic: async () => { touched = true; return { id: ID }; } }); + const kratosPublic = publicStub({ whoami: async () => ({ active: true, identity, tokenized: "h.p.s" }) as Session }); + assert.equal(await completeLogin({ keto: ketoStub(), kratosAdmin, kratosPublic }, "c"), null); + assert.equal(touched, false); // no Keto read, no metadata write, no JWT +}); + test("remintSession: a live Kratos session → fresh cookie + refreshed user; a dead session → a clearing cookie + null", async () => { const identity: Identity = { id: ID, traits: { email: "admin@plainpages.local" } }; const kratosPublic = publicStub({ whoami: async (o) => (o?.tokenizeAs ? { active: true, identity, tokenized: "h.p.s" } : { active: true, identity }) as Session }); diff --git a/src/auth/login.ts b/src/auth/login.ts index 9f15624..621c24d 100644 --- a/src/auth/login.ts +++ b/src/auth/login.ts @@ -31,7 +31,7 @@ export interface LoginDeps { } export interface CompletedLogin { - email: string | null; + email: string; userId: string; jwt: string; permissions: string[]; @@ -61,7 +61,13 @@ export async function completeLogin(deps: LoginDeps, cookie: string | undefined) if (!session?.identity) return null; const userId = session.identity.id; const emailTrait = session.identity.traits?.["email"]; - const email = typeof emailTrait === "string" ? emailTrait : null; + const email = typeof emailTrait === "string" ? emailTrait : ""; + // No email is no session: `claimsToUser` reads a token carrying none as anonymous, so minting one + // would hand the browser a cookie every later request refuses. + if (!email) { + currentLog()?.warn("session dropped: identity has no email", { sub: userId }); + return null; + } const permissions = await readPermissions(deps.keto, userId); await deps.kratosAdmin.updateMetadataPublic(userId, { permissions }); @@ -86,12 +92,7 @@ export interface Reminted { // anonymous instead of re-hitting Ory on every one. export async function remintSession(deps: LoginDeps, cookie: string | undefined, options: { secure?: boolean } = {}): Promise { const completed = await completeLogin(deps, cookie); - // No email is no session, exactly as `claimsToUser` reads a token carrying none: a User with an - // empty email reads as anonymous in the shell, and is a blank key to whatever scopes on it. - if (!completed?.email) { - if (completed) currentLog()?.warn("session dropped: identity has no email", { sub: completed.userId }); - return { setCookie: clearSessionCookie(options), user: null }; - } + if (!completed) return { setCookie: clearSessionCookie(options), user: null }; return { setCookie: sessionCookie(completed.jwt, options), user: { email: completed.email, id: completed.userId, permissions: completed.permissions } }; } diff --git a/src/auth/routes.test.ts b/src/auth/routes.test.ts index fa9ed2d..89bf278 100644 --- a/src/auth/routes.test.ts +++ b/src/auth/routes.test.ts @@ -4,6 +4,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { AUTH_FLOWS } from "./flow-view.ts"; +import { gatesSet } from "./gate.ts"; import type { HydraAdmin } from "./hydra-admin.ts"; import type { KetoClient } from "./keto-client.ts"; import type { KratosAdmin } from "./kratos-admin.ts"; @@ -39,8 +40,12 @@ test("hydra alone ⇒ only RP-initiated logout of the OAuth2 group (login/consen }); test("everything wired ⇒ the full group: OAuth2 challenges, consent GET+POST, /auth/complete", () => { - const got = keys(buildAuthRoutes(deps({ hydra, keto, kratos, kratosAdmin }))); + const routes = buildAuthRoutes(deps({ hydra, keto, kratos, kratosAdmin })); + const got = keys(routes); for (const key of ["GET /auth/complete", "GET /login", "GET /oauth2/consent", "GET /oauth2/login", "GET /oauth2/logout", "POST /logout", "POST /oauth2/consent"]) { assert.ok(got.includes(key), key); } + // Discovery enforces exactly one gate per plugin declaration; nothing checks the host's own table + // at boot, so a route added here without a gate would be silently public. + for (const route of routes) assert.deepEqual(gatesSet(route), ["public"], `${route.method} ${route.path}`); }); diff --git a/src/http/builtin-routes.ts b/src/http/builtin-routes.ts index 055ec8e..1c24997 100644 --- a/src/http/builtin-routes.ts +++ b/src/http/builtin-routes.ts @@ -20,8 +20,6 @@ export interface RequestCsrf { // own context — otherwise the plugin's keys render as bare keys on the pages it owns. export type PluginContextFactory = (pluginId: string) => RequestContext; -// `Gate` carries `permission`/`public`/`session`, checked before the handler runs — the same rule -// the plugin router and the menu read. export interface BuiltinRoute extends Gate { // Returns a RouteResult, or null when the handler wrote to ctx.res itself // (the landing slots dispatch a plugin's own result against that plugin's views). diff --git a/src/plugin-host/plugin.ts b/src/plugin-host/plugin.ts index 23547b8..4c4716f 100644 --- a/src/plugin-host/plugin.ts +++ b/src/plugin-host/plugin.ts @@ -25,7 +25,6 @@ export type RouteResult = export type RouteHandler = (ctx: RequestContext) => Promise | RouteResult | void; -// `Gate` carries `permission`/`public`/`session`, checked before the handler runs. export interface Route extends Gate { handler: RouteHandler; method: HttpMethod; diff --git a/src/ui/nav.ts b/src/ui/nav.ts index 4526fc4..6ee6885 100644 --- a/src/ui/nav.ts +++ b/src/ui/nav.ts @@ -8,7 +8,6 @@ import type { User } from "../http/context.ts"; import { ENGLISH } from "../i18n/english.ts"; import type { Translate } from "../i18n/translate.ts"; -// `Gate` carries `permission`/`public`/`session` — consumed by the filter, never rendered. export interface NavNode extends Gate { id?: string; // stable key for override targeting; stripped from the rendered tree children?: NavNode[]; -- 2.52.0 From 9912dd64f1b12faa573276175acec6085f426758 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Thu, 3 Sep 2026 17:22:35 +0200 Subject: [PATCH 10/10] Offer a signed-in visitor the page they can reach, and say where the assignee join belongs --- examples/plugins/scheduling/README.md | 5 +++++ examples/plugins/scheduling/i18n/en-US.ts | 1 + examples/plugins/scheduling/i18n/sv-SE.ts | 1 + examples/plugins/scheduling/shifts.test.ts | 9 ++++++++- examples/plugins/scheduling/shifts.ts | 2 ++ examples/plugins/scheduling/views/overview.ejs | 8 ++++++-- todo.md | 1 + 7 files changed, 24 insertions(+), 3 deletions(-) diff --git a/examples/plugins/scheduling/README.md b/examples/plugins/scheduling/README.md index b43d96d..70f64e3 100644 --- a/examples/plugins/scheduling/README.md +++ b/examples/plugins/scheduling/README.md @@ -45,6 +45,11 @@ Your backend must expose two routes; the plugin treats any non-2xx as a recovera | `GET /shifts` | `Accept: application/json`, optional `?assigneeId=` | `200` | JSON array of `{ id, title, assignee, assigneeId, start, end }` (all strings; missing fields coerce to `""`). With `assigneeId`, only that person's rows | | `POST /shifts` | JSON body `{ title, assignee, start, end }` | `2xx` | ignored (the plugin POST-redirect-GETs back to the list) | +`POST /shifts` carries the assignee as a **display name only**, so a shift created through this +plugin's form belongs to nobody and surfaces on no one's "My shifts" — don't go hunting for it +there. Resolving a name to an identity id needs a directory this demo has none of; a real backend +does that join at create time and stores the `assigneeId` alongside the name. + 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. diff --git a/examples/plugins/scheduling/i18n/en-US.ts b/examples/plugins/scheduling/i18n/en-US.ts index c7253c8..cd4fa9a 100644 --- a/examples/plugins/scheduling/i18n/en-US.ts +++ b/examples/plugins/scheduling/i18n/en-US.ts @@ -22,6 +22,7 @@ const messages = { "scheduling.new.title": "New shift", "scheduling.overview.lead": "Scheduling coordinates shifts across your team. Anyone can read this overview; the shift list itself is available to people with the scheduling:read permission.", + "scheduling.overview.mine": "See my shifts", "scheduling.overview.signIn": "Sign in to view shifts", "scheduling.overview.title": "Scheduling", "scheduling.overview.view": "View shifts", diff --git a/examples/plugins/scheduling/i18n/sv-SE.ts b/examples/plugins/scheduling/i18n/sv-SE.ts index ccd281a..20cb8c5 100644 --- a/examples/plugins/scheduling/i18n/sv-SE.ts +++ b/examples/plugins/scheduling/i18n/sv-SE.ts @@ -18,6 +18,7 @@ const messages: SchedulingMessages = { "scheduling.new.title": "Nytt pass", "scheduling.overview.lead": "Schemaläggningen samordnar teamets pass. Alla kan läsa den här översikten; själva passlistan kräver behörigheten scheduling:read.", + "scheduling.overview.mine": "Visa mina pass", "scheduling.overview.signIn": "Logga in för att se passen", "scheduling.overview.title": "Schemaläggning", "scheduling.overview.view": "Visa pass", diff --git a/examples/plugins/scheduling/shifts.test.ts b/examples/plugins/scheduling/shifts.test.ts index 62f547f..41742bb 100644 --- a/examples/plugins/scheduling/shifts.test.ts +++ b/examples/plugins/scheduling/shifts.test.ts @@ -115,14 +115,21 @@ test("listShifts degrades to a recoverable error page when the upstream is down // ---- public overview handler (a page anyone can reach, gated data stays behind the permission) ---- -test("overview renders a public page for anyone; it links straight to Shifts only for a reader", async () => { +test("overview renders a public page for anyone, and its CTA names the best gate the visitor passes", async () => { const anon = asView(await overview()(fakeCtx())); // user null, no permissions assert.equal(anon.view, "overview"); assert.equal(anon.data["chrome"], CHROME); assert.equal(anon.data["canRead"], false); // anonymous → prompt to sign in, no shifts link + assert.equal(anon.data["signedIn"], false); const reader = asView(await overview()(fakeCtx({ permissions: ["scheduling:read"] }))); assert.equal(reader.data["canRead"], true); // a reader gets a link straight to the shifts list + + // Signed in but ungranted: the page must not invite them to sign in again. + const member = asView(await overview()(fakeCtx({ user: { email: "m@example.test", id: "01a06091-baa3-7a1f-9c62-0e3ab6d2f5c1", permissions: [] } }))); + assert.equal(member.data["canRead"], false); + assert.equal(member.data["signedIn"], true); + assert.equal(member.data["mineHref"], "/scheduling/mine"); }); // ---- create handler ---- diff --git a/examples/plugins/scheduling/shifts.ts b/examples/plugins/scheduling/shifts.ts index e8984dc..47bc6ab 100644 --- a/examples/plugins/scheduling/shifts.ts +++ b/examples/plugins/scheduling/shifts.ts @@ -236,7 +236,9 @@ export function overview(): RouteHandler { breadcrumbs: [{ label: ctx.t("scheduling.nav.overview") }], canRead: can(ctx, READ), chrome: ctx.chrome, + mineHref: ctx.localeHref(MINE_PATH), shiftsHref: ctx.localeHref(SHIFTS_PATH), // a plugin carries the visitor's locale onto its own links + signedIn: ctx.user !== null, signInHref: ctx.localeHref(`/login?return_to=${encodeURIComponent(ctx.localeHref(SHIFTS_PATH))}`), title: ctx.t("scheduling.overview.title"), }, diff --git a/examples/plugins/scheduling/views/overview.ejs b/examples/plugins/scheduling/views/overview.ejs index 01ac6cf..10a7d20 100644 --- a/examples/plugins/scheduling/views/overview.ejs +++ b/examples/plugins/scheduling/views/overview.ejs @@ -3,12 +3,16 @@ nav node are marked `public`, so an anonymous visitor is let through and the menu option shows for everyone. The actual shifts data stays behind `scheduling:read`: a reader gets a link straight to it, anyone else a prompt to sign in. Rendered in the native shell via ctx.chrome. - Data: chrome, title, breadcrumbs, canRead, shiftsHref, signInHref + Data: chrome, title, breadcrumbs, canRead, mineHref, shiftsHref, signedIn, signInHref %><% const navHtml = include("partials/nav-tree", { nodes: chrome.nav }); + // One CTA per gate the visitor passes: the list needs the permission, "My shifts" only a session, + // and sign-in is offered to nobody who already has one. const cta = canRead ? '' + t("scheduling.overview.view") + '' - : '' + t("scheduling.overview.signIn") + ''; + : signedIn + ? '' + t("scheduling.overview.mine") + '' + : '' + t("scheduling.overview.signIn") + ''; -%> <%- include("partials/shell", { actions: "", diff --git a/todo.md b/todo.md index b4250c1..a69772a 100644 --- a/todo.md +++ b/todo.md @@ -20,6 +20,7 @@ - [ ] Decide what `ICON_NAMES` (`src/ui/icons.ts`) actually is. `i-chart`, `i-copy`, `i-download` and `i-sliders` have no caller anywhere — so either they go, or the comment should say the palette is curated and may carry an id ahead of its first use. Not cosmetic: the sprite is inlined into every page, and the rule decides whether a future removal is routine cleanup or a plugin-facing regression. - [ ] 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 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 and README → Security model; not accepted ⇒ bind the nonce to `sub`. - [ ] Verify the documented Docker commands on macOS and fix whatever misbehaves — **macOS is a supported dev host**, but nothing here has been run on one. Two suspects, both from the `--user "$(id -u):$(id -g)"` idiom: a macOS `id -g` is `20`, which is `dialout` inside the noble image rather than a user group, and Docker Desktop remaps bind-mount ownership in its own VM layer. The same question covers rootless Docker, where README already says to *drop* the flag. +- [ ] Map Kratos' 401 on a self-service flow init, so an anonymous `GET /settings` with no `?flow` renders instead of 500ing. `flowPage` (`src/auth/routes.ts`) maps 403/404/410 → restart the flow, 400 `session_already_available` → `/auth/complete`, and ≥500 → the themed 503, then rethrows everything else — and Kratos answers the settings-flow init with 401 when there is no session. `/settings` is correctly `public` (the recovery flow lands there with a live Kratos session but no app JWT), so the gate is not the fix: a 401 should redirect to `/login` with the page as `return_to`. No E2E covers an anonymous hit on a flow page that needs a session. ### Architectural review findings (2026-07-02) -- 2.52.0