Address whole-project architecture + product reviews (todo §5): make readRoles transitive so group→role grants reach the JWT (matches the Roles 'Effective access' view + OPL model; per-login only), per the user's call; add a zero-JS server-rendered confirm step for delete user/group/role (views/admin/confirm.ejs + shared buildConfirmModel; the Delete control is now a GET link, the delete stays a CSRF-guarded POST); self-lockout guards — no self-delete/deactivate (Users), no self-revoke of the direct admin grant + no delete of the admin role (Roles), each → 400 + inline error (direct-grant paths incl. the seeded admin; group-only-admin lockout = robust last-effective-admin check deferred §9); extract the gate+CSRF preamble copied across the 3 admin handlers into admin-nav.ts requireAdmin/guardedForm; shellUser keeps the email (name = local part, full email beneath). Reviewers: architecture no Critical/High, product 2 Critical + 1 High (all fixed). Deferred (scoped): host route-table→§6, list/template dedup→§5 cleanup, success-flash/empty-states/dangling-refs→§5 polish/§8, safeUrl→§7, 413/https/§N-drift→§9. Tests-first (extended the 3 admin HTTP tests + login/shell-context units); typecheck + 244 units + 8 visual + auth-refresh E2E green; stability-reviewer APPROVE

2026-06-18 19:18:50 +02:00
parent 6920751cb8
commit c78e95889c
16 changed files with 213 additions and 99 deletions
--- a/src/admin-groups.ts
+++ b/src/admin-groups.ts
@@ -7,12 +7,9 @@
 // building-block view models; `handleAdminGroups` is the imperative shell app.ts dispatches to — it
 // gates (admin only), CSRF-guards every mutation, and maps each action to a RouteResult.

-import { ADMIN_GROUPS_BASE, ADMIN_PERMISSION, adminNav } from "./admin-nav.ts";
+import { ADMIN_GROUPS_BASE, adminNav, buildConfirmModel, guardedForm, requireAdmin } from "./admin-nav.ts";
 import type { FieldConfig } from "./admin-users.ts";
-import { readFormBody } from "./body.ts";
 import type { RequestContext, User } from "./context.ts";
-import { CSRF_FIELD, verifyCsrfRequest } from "./csrf.ts";
-import { GuardError } from "./guards.ts";
 import type { KetoClient, RelationQuery, RelationTuple, SubjectSet } from "./keto-client.ts";
 import type { KratosAdmin } from "./kratos-admin.ts";
 import { parseListQuery } from "./list-query.ts";
@@ -334,21 +331,11 @@ export async function handleAdminGroups(ctx: RequestContext, csrfToken: string,
  const path = ctx.url.pathname;
  if (path !== ADMIN_GROUPS_BASE && !path.startsWith(`${ADMIN_GROUPS_BASE}/`)) return null;

-  if (!ctx.user) throw new GuardError(401, "authentication required", "/login");
-  if (!ctx.roles.includes(ADMIN_PERMISSION)) throw new GuardError(403, "admin role required");
-
+  const user = requireAdmin(ctx); // signed-in admin only (else GuardError → /login or 403)
  const { keto, kratosAdmin, menu, render } = deps;
-  const user = ctx.user;
  const method = (ctx.req.method ?? "GET").toUpperCase();
  const seg = path.slice(ADMIN_GROUPS_BASE.length).split("/").filter(Boolean);
-
-  let form: URLSearchParams | undefined;
-  if (method === "POST") {
-    form = await readFormBody(ctx.req);
-    if (!verifyCsrfRequest({ cookieHeader: ctx.req.headers.cookie, secret: deps.csrfSecret, submitted: form.get(CSRF_FIELD) })) {
-      throw new GuardError(403, "invalid CSRF token");
-    }
-  }
+  const form = await guardedForm(ctx, deps.csrfSecret); // parsed + CSRF-verified on POST, else undefined

  const renderList = async (): Promise<RouteResult> => {
    const groups = groupsFromTuples(await pagedTuples(keto, { namespace: GROUP_NS, relation: MEMBERS }));
@@ -397,6 +384,13 @@ export async function handleAdminGroups(ctx: RequestContext, csrfToken: string,
    if (tuple && tuple.subject_set?.object !== name) await keto.writeTuple(tuple);
    return { redirect: base };
  }
+  if (seg.length === 2 && seg[1] === "delete" && method === "GET") {
+    return { html: await render("admin/confirm", { model: buildConfirmModel({
+      breadcrumbs: [{ href: ADMIN_GROUPS_BASE, label: "Groups" }, { href: base, label: name }, { label: "Delete" }],
+      cancelHref: base, confirmAction: `${base}/delete`, confirmLabel: "Delete group", csrfToken,
+      current: "groups", menu, message: `Delete group ${name}? This removes the group and all its memberships.`, title: "Delete group", user,
+    }) }) };
+  }
  if (seg.length === 2 && seg[1] === "delete" && method === "POST") {
    await keto.deleteTuple({ namespace: GROUP_NS, object: name, relation: MEMBERS }); // removes every member tuple
    return { redirect: ADMIN_GROUPS_BASE };
--- a/src/admin-nav.ts
+++ b/src/admin-nav.ts
@@ -4,8 +4,13 @@
 // for a non-admin), and `adminNav()` is the in-screen sidebar each admin screen renders (a link home
 // + the same section, with the active item marked `current`).

+import { readFormBody } from "./body.ts";
+import type { RequestContext, User } from "./context.ts";
+import { CSRF_FIELD, verifyCsrfRequest } from "./csrf.ts";
+import { GuardError } from "./guards.ts";
 import { type MenuConfig } from "./menu-config.ts";
 import { composeNav, type NavNode } from "./nav.ts";
+import { buildShellContext } from "./shell-context.ts";

 export const ADMIN_PERMISSION = "admin"; // role token gating the admin section
 export const ADMIN_USERS_BASE = "/admin/users";
@@ -40,3 +45,45 @@ export function adminNav(roles: string[], menu: MenuConfig, current: AdminScreen
    adminSection(current),
  ]], menu.override, roles);
 }
+
+// The shared gate for every admin screen: a signed-in admin only. Throws GuardError that app.ts maps
+// (anonymous → /login, non-admin → 403). Returns the (non-null) user for the handler to thread on.
+export function requireAdmin(ctx: RequestContext): User {
+  if (!ctx.user) throw new GuardError(401, "authentication required", "/login");
+  if (!ctx.roles.includes(ADMIN_PERMISSION)) throw new GuardError(403, "admin role required");
+  return ctx.user;
+}
+
+// Read + CSRF-verify a mutation's form body once. Every admin write is a first-party POST form, so a
+// POST without a valid double-submit token is refused (GuardError → 403); non-POST ⇒ undefined.
+export async function guardedForm(ctx: RequestContext, csrfSecret: string): Promise<URLSearchParams | undefined> {
+  if ((ctx.req.method ?? "GET").toUpperCase() !== "POST") return undefined;
+  const form = await readFormBody(ctx.req);
+  if (!verifyCsrfRequest({ cookieHeader: ctx.req.headers.cookie, secret: csrfSecret, submitted: form.get(CSRF_FIELD) })) {
+    throw new GuardError(403, "invalid CSRF token");
+  }
+  return form;
+}
+
+// Build the model for the shared destructive-action confirm page (views/admin/confirm.ejs): a single
+// danger action behind a deliberate second step, plus a cancel link. Reused by all three screens.
+export function buildConfirmModel(opts: {
+  breadcrumbs: { href?: string; label: string }[];
+  cancelHref: string;
+  confirmAction: string;
+  confirmLabel: string;
+  csrfToken: string;
+  current: AdminScreen;
+  menu: MenuConfig;
+  message: string;
+  title: string;
+  user: User | null;
+}) {
+  return {
+    cancelHref: opts.cancelHref,
+    confirm: { action: opts.confirmAction, label: opts.confirmLabel },
+    message: opts.message,
+    nav: adminNav(opts.user?.roles ?? [], opts.menu, opts.current),
+    shell: buildShellContext({ breadcrumbs: opts.breadcrumbs, csrfToken: opts.csrfToken, menu: opts.menu, title: opts.title, user: opts.user }),
+  };
+}
--- a/src/admin-roles.ts
+++ b/src/admin-roles.ts
@@ -4,12 +4,12 @@
 // the user|group membership model of the Groups screen, so the pure helpers (parseSubject, member
 // pickers, tuple paging) are reused from admin-groups. The one role-specific piece is the **effective
 // access** view: `keto.expand(Role:<name>#members)` returns the membership tree, which we flatten to
-// the distinct set of users who hold the role directly or transitively via a group. (The coarse JWT
-// projection reads only direct grants per the README's one-read-per-login design; this view is where
-// group→role inheritance is surfaced.) Writes go only to Keto; Kratos is read only to label members.
+// the distinct set of users who hold the role directly or transitively via a group. Login resolves
+// the same transitive membership into the JWT `roles` (login.ts readRoles), so this view matches what
+// a user's token actually grants. Writes go only to Keto; Kratos is read only to label members.
 // `handleAdminRoles` is the imperative shell app.ts dispatches to — gated admin-only, CSRF-guarded.

-import { ADMIN_PERMISSION, ADMIN_ROLES_BASE, adminNav } from "./admin-nav.ts";
+import { ADMIN_PERMISSION, ADMIN_ROLES_BASE, adminNav, buildConfirmModel, guardedForm, requireAdmin } from "./admin-nav.ts";
 import {
  type GroupView,
  groupsFromTuples,
@@ -23,10 +23,7 @@ import {
  safeDecode,
 } from "./admin-groups.ts";
 import type { FieldConfig } from "./admin-users.ts";
-import { readFormBody } from "./body.ts";
 import type { RequestContext, User } from "./context.ts";
-import { CSRF_FIELD, verifyCsrfRequest } from "./csrf.ts";
-import { GuardError } from "./guards.ts";
 import type { ExpandTree, KetoClient, RelationTuple } from "./keto-client.ts";
 import type { KratosAdmin } from "./kratos-admin.ts";
 import { parseListQuery } from "./list-query.ts";
@@ -298,21 +295,11 @@ export async function handleAdminRoles(ctx: RequestContext, csrfToken: string, d
  const path = ctx.url.pathname;
  if (path !== ADMIN_ROLES_BASE && !path.startsWith(`${ADMIN_ROLES_BASE}/`)) return null;

-  if (!ctx.user) throw new GuardError(401, "authentication required", "/login");
-  if (!ctx.roles.includes(ADMIN_PERMISSION)) throw new GuardError(403, "admin role required");
-
+  const user = requireAdmin(ctx); // signed-in admin only (else GuardError → /login or 403)
  const { keto, kratosAdmin, menu, render } = deps;
-  const user = ctx.user;
  const method = (ctx.req.method ?? "GET").toUpperCase();
  const seg = path.slice(ADMIN_ROLES_BASE.length).split("/").filter(Boolean);
-
-  let form: URLSearchParams | undefined;
-  if (method === "POST") {
-    form = await readFormBody(ctx.req);
-    if (!verifyCsrfRequest({ cookieHeader: ctx.req.headers.cookie, secret: deps.csrfSecret, submitted: form.get(CSRF_FIELD) })) {
-      throw new GuardError(403, "invalid CSRF token");
-    }
-  }
+  const form = await guardedForm(ctx, deps.csrfSecret); // parsed + CSRF-verified on POST, else undefined

  const renderList = async (): Promise<RouteResult> => {
    const roles = rolesFromTuples(await pagedTuples(keto, { namespace: ROLE_NS, relation: MEMBERS }));
@@ -322,12 +309,13 @@ export async function handleAdminRoles(ctx: RequestContext, csrfToken: string, d
    const { options } = await memberCandidates(keto, kratosAdmin);
    return { html: await render("admin/role-form", { model: buildRoleFormModel({ csrfToken, memberOptions: options, menu, user, ...extra }) }) };
  };
-  const renderDetail = async (name: string): Promise<RouteResult> => {
+  const renderDetail = async (name: string, error?: string): Promise<RouteResult> => {
    const { emailById, options } = await memberCandidates(keto, kratosAdmin);
    const tuples = await pagedTuples(keto, { namespace: ROLE_NS, object: name, relation: MEMBERS });
    const members = tuples.map((t) => memberView(t, emailById));
    const effective = await effectiveUsers(keto, name, tuples.length > 0, emailById);
-    return { html: await render("admin/role-detail", { model: buildRoleDetailModel({ candidates: options, csrfToken, effective, members, menu, role: { name }, user }) }) };
+    const html = await render("admin/role-detail", { model: buildRoleDetailModel({ candidates: options, csrfToken, effective, members, menu, role: { name }, user, ...(error ? { error } : {}) }) });
+    return error ? { html, status: 400 } : { html };
  };

  // /admin/roles — list (GET) · create (POST)
@@ -362,12 +350,26 @@ export async function handleAdminRoles(ctx: RequestContext, csrfToken: string, d
    if (tuple) await keto.writeTuple(tuple); // the picker only offers real users/groups
    return { redirect: base };
  }
+  if (seg.length === 2 && seg[1] === "delete" && method === "GET") {
+    // Self-protection: deleting the admin role removes everyone's admin — refuse it outright.
+    if (name === ADMIN_PERMISSION) return renderDetail(name, "The admin role can't be deleted — it would remove all admin access.");
+    return { html: await render("admin/confirm", { model: buildConfirmModel({
+      breadcrumbs: [{ href: ADMIN_ROLES_BASE, label: "Roles" }, { href: base, label: name }, { label: "Delete" }],
+      cancelHref: base, confirmAction: `${base}/delete`, confirmLabel: "Delete role", csrfToken,
+      current: "roles", menu, message: `Delete role ${name}? This revokes it from everyone it's assigned to.`, title: "Delete role", user,
+    }) }) };
+  }
  if (seg.length === 2 && seg[1] === "delete" && method === "POST") {
+    if (name === ADMIN_PERMISSION) return renderDetail(name, "The admin role can't be deleted — it would remove all admin access.");
    await keto.deleteTuple({ namespace: ROLE_NS, object: name, relation: MEMBERS }); // removes every member tuple
    return { redirect: ADMIN_ROLES_BASE };
  }
  if (seg.length === 3 && seg[1] === "members" && seg[2] === "delete" && method === "POST") {
-    const tuple = roleMemberTuple(name, (form!.get("member") ?? "").trim());
+    const member = (form!.get("member") ?? "").trim();
+    // Self-protection: don't let an admin revoke their own *direct* admin grant (would lock them out).
+    // Admin held only via a group isn't covered here — the robust "last effective admin" check is §9.
+    if (name === ADMIN_PERMISSION && member === `user:${user.id}`) return renderDetail(name, "You can't revoke your own admin access.");
+    const tuple = roleMemberTuple(name, member);
    if (tuple) await keto.deleteTuple(tuple);
    return { redirect: base };
  }
--- a/src/admin-users.ts
+++ b/src/admin-users.ts
@@ -5,11 +5,8 @@
 // imperative shell app.ts dispatches to — it gates (admin only), CSRF-guards every mutation, and
 // maps each action to a RouteResult (render a page, or redirect after a write — PRG).

-import { ADMIN_PERMISSION, ADMIN_USERS_BASE, adminNav } from "./admin-nav.ts";
-import { readFormBody } from "./body.ts";
+import { ADMIN_USERS_BASE, adminNav, buildConfirmModel, guardedForm, requireAdmin } from "./admin-nav.ts";
 import type { RequestContext, User } from "./context.ts";
-import { CSRF_FIELD, verifyCsrfRequest } from "./csrf.ts";
-import { GuardError } from "./guards.ts";
 import type { Identity, KratosAdmin, RecoveryCode } from "./kratos-admin.ts";
 import { KratosError } from "./kratos-public.ts";
 import { parseListQuery } from "./list-query.ts";
@@ -306,23 +303,11 @@ export async function handleAdminUsers(ctx: RequestContext, csrfToken: string, d
  const path = ctx.url.pathname;
  if (path !== ADMIN_USERS_BASE && !path.startsWith(`${ADMIN_USERS_BASE}/`)) return null;

-  if (!ctx.user) throw new GuardError(401, "authentication required", "/login");
-  if (!ctx.roles.includes(ADMIN_PERMISSION)) throw new GuardError(403, "admin role required");
-
+  const user = requireAdmin(ctx); // signed-in admin only (else GuardError → /login or 403)
  const { kratosAdmin, menu, render } = deps;
-  const user = ctx.user;
  const method = (ctx.req.method ?? "GET").toUpperCase();
  const seg = path.slice(ADMIN_USERS_BASE.length).split("/").filter(Boolean);
-
-  // Every mutation is a first-party form → CSRF-guard it (the host doesn't gate plugin routes,
-  // but it owns these). Reads the body once; the action handlers reuse the parsed form.
-  let form: URLSearchParams | undefined;
-  if (method === "POST") {
-    form = await readFormBody(ctx.req);
-    if (!verifyCsrfRequest({ cookieHeader: ctx.req.headers.cookie, secret: deps.csrfSecret, submitted: form.get(CSRF_FIELD) })) {
-      throw new GuardError(403, "invalid CSRF token");
-    }
-  }
+  const form = await guardedForm(ctx, deps.csrfSecret); // parsed + CSRF-verified on POST, else undefined

  const renderList = async (): Promise<RouteResult> => {
    const { identities } = await kratosAdmin.listIdentities({ pageSize: LIST_FETCH_SIZE });
@@ -370,18 +355,32 @@ export async function handleAdminUsers(ctx: RequestContext, csrfToken: string, d
    return null;
  }

-  if (seg.length === 2 && method === "POST") {
-    if (seg[1] === "state") {
-      await kratosAdmin.updateIdentity(targetId, setStatePayload(identity, identity.state === "inactive" ? "active" : "inactive"));
-      return { redirect: back };
+  if (seg.length === 2) {
+    const isSelf = targetId === user.id; // self-protection: an admin must not lock themselves out
+    if (seg[1] === "delete" && method === "GET") {
+      if (isSelf) return { ...(await renderForm({ error: "You can't delete your own account.", identity })), status: 400 };
+      const view = toUserView(identity);
+      return { html: await render("admin/confirm", { model: buildConfirmModel({
+        breadcrumbs: [{ href: ADMIN_USERS_BASE, label: "Users" }, { href: back, label: view.name }, { label: "Delete" }],
+        cancelHref: back, confirmAction: `${back}/delete`, confirmLabel: "Delete user", csrfToken,
+        current: "users", menu, message: `Delete ${view.email}? This permanently removes the account and can't be undone.`, title: "Delete user", user,
+      }) }) };
    }
-    if (seg[1] === "delete") {
-      await kratosAdmin.deleteIdentity(targetId);
-      return { redirect: ADMIN_USERS_BASE };
-    }
-    if (seg[1] === "recovery") {
-      const recovery = await kratosAdmin.createRecoveryCode(targetId);
-      return renderForm({ identity, recovery });
+    if (method === "POST") {
+      if (seg[1] === "state") {
+        if (isSelf) return { ...(await renderForm({ error: "You can't deactivate your own account.", identity })), status: 400 };
+        await kratosAdmin.updateIdentity(targetId, setStatePayload(identity, identity.state === "inactive" ? "active" : "inactive"));
+        return { redirect: back };
+      }
+      if (seg[1] === "delete") {
+        if (isSelf) return { ...(await renderForm({ error: "You can't delete your own account.", identity })), status: 400 };
+        await kratosAdmin.deleteIdentity(targetId);
+        return { redirect: ADMIN_USERS_BASE };
+      }
+      if (seg[1] === "recovery") {
+        const recovery = await kratosAdmin.createRecoveryCode(targetId);
+        return renderForm({ identity, recovery });
+      }
    }
  }
  return null;
--- a/src/app.test.ts
+++ b/src/app.test.ts
@@ -228,7 +228,7 @@ test("session re-mint: an expired JWT backed by a live Kratos session is silentl
  const nowSec = Math.floor(Date.now() / 1000);
  const freshJwt = mintJwt({ email: "a@b.c", exp: nowSec + 600, roles: ["demo:read"], sub: "u1" });
  const live = withWhoami(async (o) => (o?.tokenizeAs ? { active: true, identity, tokenized: freshJwt } : { active: true, identity }) as Session);
-  const keto = stubKeto({ listRelations: async () => ({ nextPageToken: null, tuples: [{ namespace: "Role", object: "demo:read", relation: "members", subject_id: "user:u1" }] }) });
+  const keto = stubKeto({ check: async () => true, listRelations: async () => ({ nextPageToken: null, tuples: [{ namespace: "Role", object: "demo:read", relation: "members", subject_id: "user:u1" }] }) });
  const expired = `${SESSION_COOKIE}=${mintJwt({ email: "a@b.c", exp: nowSec - 600, roles: ["demo:read"], sub: "u1" })}; plainpages_session=s`;

  // Live Kratos session: the lapsed token is re-minted — the gated route runs AND a fresh cookie rides the response.
@@ -408,7 +408,7 @@ test("login completion (/auth/complete): a live session mints the JWT cookie; no
  let projected: unknown;
  const kratos = withWhoami(async (o) => (o?.tokenizeAs ? { active: true, identity, tokenized: "h.p.s" } : { active: true, identity }) as Session);
  const kratosAdmin = stubAdmin({ updateMetadataPublic: async (_id, meta) => { projected = meta; return identity; } });
-  const keto = stubKeto({ listRelations: async () => ({ nextPageToken: null, tuples: [{ namespace: "Role", object: "admin", relation: "members", subject_id: `user:${identity.id}` }] }) });
+  const keto = stubKeto({ check: async () => true, listRelations: async () => ({ nextPageToken: null, tuples: [{ namespace: "Role", object: "admin", relation: "members", subject_id: `user:${identity.id}` }] }) });
  const complete = async (app: ReturnType<typeof createApp>, cookie?: string) => {
    await new Promise<void>((r) => app.listen(0, r));
    t.after(() => app.close());
@@ -464,7 +464,7 @@ test("logout (CSRF-guarded POST): valid token revokes the Kratos session + clear
 test("admin Users screen: gate, list/filter, create, edit, deactivate, delete, recovery (CSRF-guarded)", async (t) => {
  const mk = (email: string, over: Partial<Identity> = {}): Identity =>
    ({ id: randomUUID(), schema_id: "default", state: "active", traits: { email, name: { first: "Ada", last: "Lovelace" } }, ...over });
-  const store: Identity[] = [mk("ada@example.com"), mk("babbage@example.com", { state: "inactive" })];
+  const store: Identity[] = [mk("ada@example.com"), mk("babbage@example.com", { state: "inactive" }), mk("you@example.com", { id: "admin1" })];
  let lastCreate: { traits?: unknown } | undefined;
  const kratosAdmin = stubAdmin({
    createIdentity: async (payload) => { lastCreate = payload as { traits?: unknown }; const created = mk("grace@example.com"); store.push(created); return created; },
@@ -528,11 +528,20 @@ test("admin Users screen: gate, list/filter, create, edit, deactivate, delete, r
  assert.equal(rec.status, 200);
  assert.match(await rec.text(), /self-service\/recovery\?code=123456/);

-  // Delete: removes the identity, back to the list.
+  // Delete needs a deliberate confirm step (zero-JS): GET renders the interstitial, POST performs it.
+  const confirm = await (await get(`/admin/users/${target.id}/delete`)).text();
+  assert.match(confirm, /Cancel/);
+  assert.match(confirm, new RegExp(`action="/admin/users/${target.id}/delete"`));
  const del = await post(`/admin/users/${target.id}/delete`, `_csrf=${token}`);
  assert.equal(del.status, 303);
  assert.ok(!store.some((x) => x.id === target.id));

+  // Self-protection: an admin can't delete or deactivate their own account (JWT sub = admin1).
+  assert.equal((await post(`/admin/users/admin1/delete`, `_csrf=${token}`)).status, 400);
+  assert.ok(store.some((x) => x.id === "admin1"));
+  assert.equal((await post(`/admin/users/admin1/state`, `_csrf=${token}`)).status, 400);
+  assert.equal(store.find((x) => x.id === "admin1")!.state, "active");
+
  // Unknown id → 404.
  assert.equal((await get(`/admin/users/${randomUUID()}`)).status, 404);
 });
@@ -610,7 +619,8 @@ test("admin Groups screen: gate, list, create, detail/membership, delete (CSRF-g
  await post("/admin/groups/eng/members/delete", `_csrf=${token}&member=user:${grace}`);
  assert.ok(!tuples.some((tp) => tp.object === "eng" && tp.subject_id === `user:${grace}`));

-  // Delete the group: removes every member tuple, back to the list.
+  // Delete the group: a confirm step (GET) then the POST removes every member tuple, back to the list.
+  assert.match(await (await get("/admin/groups/eng/delete")).text(), /Cancel/);
  const del = await post("/admin/groups/eng/delete", `_csrf=${token}`);
  assert.equal(del.status, 303);
  assert.equal(del.headers.get("location"), "/admin/groups");
@@ -706,12 +716,20 @@ test("admin Roles screen: gate, list, create, assign user/group, effective acces
  await post("/admin/roles/editor/members/delete", `_csrf=${token}&member=group:eng`);
  assert.ok(!tuples.some((tp) => tp.namespace === "Role" && tp.object === "editor" && tp.subject_set?.object === "eng"));

-  // Delete the role: removes every member tuple, back to the list.
+  // Delete the role: a confirm step (GET) then the POST removes every member tuple, back to the list.
+  assert.match(await (await get("/admin/roles/editor/delete")).text(), /Cancel/);
  const del = await post("/admin/roles/editor/delete", `_csrf=${token}`);
  assert.equal(del.status, 303);
  assert.equal(del.headers.get("location"), "/admin/roles");
  assert.ok(!tuples.some((tp) => tp.namespace === "Role" && tp.object === "editor"));

+  // Self-protection: the admin role can't be deleted, nor can you revoke your own admin (sub admin1).
+  tuples.push({ namespace: "Role", object: "admin", relation: "members", subject_id: "user:admin1" });
+  assert.equal((await post("/admin/roles/admin/delete", `_csrf=${token}`)).status, 400);
+  assert.ok(tuples.some((tp) => tp.object === "admin"));
+  assert.equal((await post("/admin/roles/admin/members/delete", `_csrf=${token}&member=user:admin1`)).status, 400);
+  assert.ok(tuples.some((tp) => tp.object === "admin" && tp.subject_id === "user:admin1"));
+
  // An invalid role name in the path → 404; malformed %-encoding doesn't 500.
  assert.equal((await get("/admin/roles/Bad%20Name")).status, 404);
  assert.equal((await get("/admin/roles/%ZZ")).status, 404);
--- a/src/login.test.ts
+++ b/src/login.test.ts
@@ -40,18 +40,29 @@ const publicStub = (over: Partial<KratosPublic> = {}): KratosPublic => ({
  ...over,
 });

-test("readRoles reads direct Role memberships from Keto — paged, de-duped, sorted", async () => {
-  const calls: unknown[] = [];
+test("readRoles returns roles held directly OR transitively (enumerate defined roles → Keto-check each)", async () => {
+  const listQ: unknown[] = [];
+  const checked: string[] = [];
+  const role = (object: string, subject: Partial<RelationTuple>): RelationTuple => ({ namespace: "Role", object, relation: "members", ...subject });
  const keto = ketoStub({
+    // Enumerate every Role tuple (paged, no subject filter) to find the distinct role names —
+    // subjects vary (a direct user, a group) and a name repeats across pages → de-duped.
    listRelations: async (q) => {
-      calls.push(q);
-      if (!q?.pageToken) return { nextPageToken: "p2", tuples: [roleTuple("editor"), roleTuple("admin")] };
-      return { nextPageToken: null, tuples: [roleTuple("admin")] }; // duplicate across pages
+      listQ.push(q);
+      if (q?.pageToken === "p2") return { nextPageToken: null, tuples: [role("editor", { subject_id: "user:other" })] };
+      return { nextPageToken: "p2", tuples: [
+        role("editor", { subject_set: { namespace: "Group", object: "eng", relation: "members" } }),
+        role("admin", { subject_id: `user:${ID}` }),
+        role("viewer", { subject_id: "user:stranger" }),
+      ] };
    },
+    // Keto resolves transitively: the user holds editor (via a group) + admin (direct), not viewer.
+    check: async (t) => { checked.push(t.object); return t.object === "admin" || t.object === "editor"; },
  });
  assert.deepEqual(await readRoles(keto, ID), ["admin", "editor"]);
-  assert.deepEqual(calls[0], { namespace: "Role", relation: "members", subject_id: `user:${ID}` });
-  assert.equal((calls[1] as { pageToken?: string }).pageToken, "p2"); // second page follows the cursor
+  assert.deepEqual(listQ[0], { namespace: "Role", relation: "members" }); // enumerate, not subject-filtered
+  assert.equal((listQ[1] as { pageToken?: string }).pageToken, "p2"); // second page follows the cursor
+  assert.deepEqual(checked.sort(), ["admin", "editor", "viewer"]); // every distinct role checked for the user
 });

 test("completeLogin: read roles → project onto metadata_public → tokenize → JWT (in that order)", async () => {
@@ -65,7 +76,7 @@ test("completeLogin: read roles → project onto metadata_public → tokenize
    },
  });
  const kratosAdmin = adminStub({ updateMetadataPublic: async (_id, meta) => { events.push("project"); projected = meta; return identity; } });
-  const keto = ketoStub({ listRelations: async () => ({ nextPageToken: null, tuples: [roleTuple("admin")] }) });
+  const keto = ketoStub({ check: async () => true, listRelations: async () => ({ nextPageToken: null, tuples: [roleTuple("admin")] }) });

  const out = await completeLogin({ keto, kratosAdmin, kratosPublic }, "plainpages_session=s");
  assert.deepEqual(out, { email: "admin@plainpages.local", identityId: ID, jwt: "h.p.s", roles: ["admin"] });
@@ -90,7 +101,7 @@ test("completeLogin maps a missing email trait to null and throws if the tokeniz
 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 });
-  const keto = ketoStub({ listRelations: async () => ({ nextPageToken: null, tuples: [roleTuple("admin")] }) });
+  const keto = ketoStub({ check: async () => true, listRelations: async () => ({ nextPageToken: null, tuples: [roleTuple("admin")] }) });

  // TTL lapsed but the Kratos session lives → re-read roles from Keto, re-tokenize, fresh cookie.
  const live = await remintSession({ keto, kratosAdmin: adminStub(), kratosPublic }, "plainpages_session=s");
--- a/src/login.ts
+++ b/src/login.ts
@@ -36,19 +36,23 @@ export interface CompletedLogin {
  roles: string[];
 }

-// The coarse roles Keto grants a subject directly: `Role:<name>#members@user:<id>`. Returns
-// the de-duped, sorted role names (the tuple `object`). One logical read, paged defensively.
-// Group→role inheritance lands with the Groups screen (§5); MVP grants are direct.
+// The coarse roles a user holds — directly (`Role:<name>#members@user:<id>`) or transitively via a
+// group that is a member of the role. Enumerates the defined roles (the distinct objects in the Role
+// namespace) and asks Keto to resolve each membership, so a role granted to a group reaches the JWT —
+// matching the OPL model and the admin "Effective access" view. At login/refresh only, never per
+// request; role count is small, so the per-role checks are cheap and run in parallel.
 export async function readRoles(keto: KetoClient, identityId: string): Promise<string[]> {
  const subject_id = `user:${identityId}`;
-  const roles = new Set<string>();
+  const names = new Set<string>();
  let pageToken: string | undefined;
  do {
-    const page = await keto.listRelations({ namespace: "Role", relation: "members", subject_id, ...(pageToken ? { pageToken } : {}) });
-    for (const t of page.tuples) roles.add(t.object);
+    const page = await keto.listRelations({ namespace: "Role", relation: "members", ...(pageToken ? { pageToken } : {}) });
+    for (const t of page.tuples) names.add(t.object);
    pageToken = page.nextPageToken ?? undefined;
  } while (pageToken);
-  return [...roles].sort();
+  const roles = [...names];
+  const held = await Promise.all(roles.map((object) => keto.check({ namespace: "Role", object, relation: "members", subject_id })));
+  return roles.filter((_, i) => held[i]).sort();
 }

 export async function completeLogin(deps: LoginDeps, cookie: string | undefined): Promise<CompletedLogin | null> {
--- a/src/shell-context.test.ts
+++ b/src/shell-context.test.ts
@@ -4,8 +4,8 @@ import { buildShellContext, shellUser } from "./shell-context.ts";

 test("shellUser derives the profile from the real user; anonymous → Guest", () => {
  assert.deepEqual(shellUser(null), { email: "", initials: "G", name: "Guest" });
-  // Real user: name = email, initials = first two letters of the local part, upper-cased.
-  assert.deepEqual(shellUser({ email: "ada@example.com", id: "u1", roles: [] }), { email: "", initials: "AD", name: "ada@example.com" });
+  // Real user: name = email local part, email kept, initials = first two letters of the local part.
+  assert.deepEqual(shellUser({ email: "ada@example.com", id: "u1", roles: [] }), { email: "ada@example.com", initials: "AD", name: "ada" });
 });

 test("buildShellContext maps branding + breadcrumbs, omitting unset optional fields", () => {
--- a/src/shell-context.ts
+++ b/src/shell-context.ts
@@ -2,7 +2,8 @@
 // (the home dashboard, the built-in admin screens) hands to shell.ejs. Pure. Extracted so the
 // shell user is the *real* signed-in identity (§4) — no hardcoded demo profile — and branding is
 // read from one place. The User carries no display name (the JWT holds only id/email/roles), so
-// the profile shows the email with initials derived from its local part; anonymous ⇒ "Guest".
+// the profile shows the email's local part as the name with the full email beneath, initials from
+// the local part; anonymous ⇒ "Guest".

 import type { User } from "./context.ts";
 import { type MenuConfig } from "./menu-config.ts";
@@ -25,7 +26,7 @@ export interface ShellModel {
 export function shellUser(user: User | null | undefined): ShellUser {
  if (!user) return { email: "", initials: "G", name: "Guest" };
  const local = user.email.split("@")[0] || user.email;
-  return { email: "", initials: (local.slice(0, 2) || "U").toUpperCase(), name: user.email };
+  return { email: user.email, initials: (local.slice(0, 2) || "U").toUpperCase(), name: local };
 }

 export function buildShellContext(opts: {