Gate a route or nav node on a session, not only a permission #103

Merged
lilleman merged 12 commits from session-gate into main 2026-09-03 17:47:05 +02:00
7 changed files with 24 additions and 3 deletions
Showing only changes of commit 9912dd64f1 - Show all commits
+5
View File
@@ -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=<id>` | `200` | JSON array of `{ id, title, assignee, assigneeId, start, end }` (all strings; missing fields coerce to `""`). With `assigneeId`, only that person's rows | | `GET /shifts` | `Accept: application/json`, optional `?assigneeId=<id>` | `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` | 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 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. form re-renders. The plugin only validates that `title` and `assignee` are non-empty.
@@ -22,6 +22,7 @@ const messages = {
"scheduling.new.title": "New shift", "scheduling.new.title": "New shift",
"scheduling.overview.lead": "scheduling.overview.lead":
"Scheduling coordinates shifts across your team. Anyone can read this overview; the shift list itself is available to people with the <code>scheduling:read</code> permission.", "Scheduling coordinates shifts across your team. Anyone can read this overview; the shift list itself is available to people with the <code>scheduling:read</code> permission.",
"scheduling.overview.mine": "See my shifts",
"scheduling.overview.signIn": "Sign in to view shifts", "scheduling.overview.signIn": "Sign in to view shifts",
"scheduling.overview.title": "Scheduling", "scheduling.overview.title": "Scheduling",
"scheduling.overview.view": "View shifts", "scheduling.overview.view": "View shifts",
@@ -18,6 +18,7 @@ const messages: SchedulingMessages = {
"scheduling.new.title": "Nytt pass", "scheduling.new.title": "Nytt pass",
"scheduling.overview.lead": "scheduling.overview.lead":
"Schemaläggningen samordnar teamets pass. Alla kan läsa den här översikten; själva passlistan kräver behörigheten <code>scheduling:read</code>.", "Schemaläggningen samordnar teamets pass. Alla kan läsa den här översikten; själva passlistan kräver behörigheten <code>scheduling:read</code>.",
"scheduling.overview.mine": "Visa mina pass",
"scheduling.overview.signIn": "Logga in för att se passen", "scheduling.overview.signIn": "Logga in för att se passen",
"scheduling.overview.title": "Schemaläggning", "scheduling.overview.title": "Schemaläggning",
"scheduling.overview.view": "Visa pass", "scheduling.overview.view": "Visa pass",
+8 -1
View File
@@ -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) ---- // ---- 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 const anon = asView(await overview()(fakeCtx())); // user null, no permissions
assert.equal(anon.view, "overview"); assert.equal(anon.view, "overview");
assert.equal(anon.data["chrome"], CHROME); assert.equal(anon.data["chrome"], CHROME);
assert.equal(anon.data["canRead"], false); // anonymous → prompt to sign in, no shifts link 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"] }))); 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 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 ---- // ---- create handler ----
+2
View File
@@ -236,7 +236,9 @@ export function overview(): RouteHandler {
breadcrumbs: [{ label: ctx.t("scheduling.nav.overview") }], breadcrumbs: [{ label: ctx.t("scheduling.nav.overview") }],
canRead: can(ctx, READ), canRead: can(ctx, READ),
chrome: ctx.chrome, chrome: ctx.chrome,
mineHref: ctx.localeHref(MINE_PATH),
shiftsHref: ctx.localeHref(SHIFTS_PATH), // a plugin carries the visitor's locale onto its own links 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))}`), signInHref: ctx.localeHref(`/login?return_to=${encodeURIComponent(ctx.localeHref(SHIFTS_PATH))}`),
title: ctx.t("scheduling.overview.title"), title: ctx.t("scheduling.overview.title"),
}, },
@@ -3,12 +3,16 @@
nav node are marked `public`, so an anonymous visitor is let through and the menu option shows for 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 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. 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 }); 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 const cta = canRead
? '<a class="btn btn-primary" href="' + shiftsHref + '">' + t("scheduling.overview.view") + '</a>' ? '<a class="btn btn-primary" href="' + shiftsHref + '">' + t("scheduling.overview.view") + '</a>'
: '<a class="btn btn-primary" href="' + signInHref + '">' + t("scheduling.overview.signIn") + '</a>'; : signedIn
? '<a class="btn btn-primary" href="' + mineHref + '">' + t("scheduling.overview.mine") + '</a>'
: '<a class="btn btn-primary" href="' + signInHref + '">' + t("scheduling.overview.signIn") + '</a>';
-%> -%>
<%- include("partials/shell", { <%- include("partials/shell", {
actions: "", actions: "",
+1
View File
@@ -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 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 `<nonce>.<HMAC(secret, nonce)>` 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`. - [ ] Decide (once) whether the CSRF token staying unbound to `sub`/session is accepted. `src/auth/csrf.ts` signs `<nonce>.<HMAC(secret, nonce)>` 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. - [ ] 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) ### Architectural review findings (2026-07-02)