Refuse an emailless identity where the session is minted, and rows the upstream should not have sent

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:
+  the menu all read. Exactly-one-gate is a discovery-time rule on manifests, not a runtime
-  `allows({}, user)` stays open **by design**, because the central override's `groups` builds header
+  invariant: `allows({}, user)` stays open **by design**, because the central override's `groups`
-  nodes that carry no gate. Making `allows` fail closed would hide every operator-grouped section. `session` exists because a plugin whose data is
+  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

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
+    // The admin owns none of the demo's rows, so an empty page is the no-leak assertion.
-    // 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.getByText("No shifts are assigned to admin@plainpages.local")).toBeVisible();

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 on this stack, so this also pins the degraded page: the reason, never a 500
-  // no shifts upstream, so it also pins the degraded page: the reason, never a 500 and never a claim
+  // and never a claim about what is assigned.
-  // 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("Couldn't reach the scheduling service")).toBeVisible();

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=<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, 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.

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
+});

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.
+      // someone else, which would hand them the previous holder's rows. The re-filter is
-      shifts = await upstream.list({ assigneeId: user.id });
+      // 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.
+      emptyText: t("scheduling.mine.empty", { email: opts.email }),
-      ...(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"),

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: '<div class="scheduling-page">' + alertHtml + '<p class="shift-count">' + count + '</p>' + tableHtml + '</div>',
+  body: '<div class="scheduling-page">' + alertHtml + (locals.error ? '' : '<p class="shift-count">' + count + '</p>' + tableHtml) + '</div>',
   brand: chrome.brand,
   breadcrumbs,
   csrfToken: chrome.csrfToken,

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=<id> → 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);
   }

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 () => {
+test("completeLogin throws if the tokenizer yields no JWT", async () => {
-  const identity: Identity = { id: ID, traits: {} };
+  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 });

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<Reminted> {
   const completed = await completeLogin(deps, cookie);
-  // No email is no session, exactly as `claimsToUser` reads a token carrying none: a User with an
+  if (!completed) return { setCookie: clearSessionCookie(options), user: null };
-  // 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 };
-  }
   return { setCookie: sessionCookie(completed.jwt, options), user: { email: completed.email, id: completed.userId, permissions: completed.permissions } };
 }
 

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}`);
 });

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).

src/plugin-host/plugin.ts

@@ -25,7 +25,6 @@ export type RouteResult =
 
 export type RouteHandler = (ctx: RequestContext) => Promise<RouteResult | void> | RouteResult | void;
 
-// `Gate` carries `permission`/`public`/`session`, checked before the handler runs.
 export interface Route extends Gate {
   handler: RouteHandler;
   method: HttpMethod;

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[];