From bf146c07e7352111f4f89a305617a53aef139844 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 08:36:16 +0200 Subject: [PATCH] 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: { /* … */ } }),
 });