AGENTS.md

@@ -213,10 +213,11 @@ Revisit only if the stated reason stops holding.
   form, a delete-confirm page) is the exception to `actionForMethod` and gates on `:write`. Two
   grant-specific guards go with it: you cannot revoke your own **direct** grants (self-lockout would
   need a `curl` against Keto to undo), and a permission held *through a group* renders
-  ticked-but-disabled, because unticked stated the opposite of the truth. **Known gap:** the group
-  paths are unguarded — unticking a permission on a group you belong to, leaving it, or deleting it
-  can still strip your own access. The robust "last effective holder" check needs a reverse Keto
-  query and is deferred.
+  ticked-but-disabled, because unticked stated the opposite of the truth. The group paths are
+  guarded too: unticking a permission on a group you are in, removing yourself or a group you are in,
+  or deleting the group is refused when you would lose a permission you don't hold directly. One also
+  held through another group counts as lost — telling them apart needs a reverse Keto query, and a
+  false refusal costs only asking another administrator.
 - **`users:write` and `groups:write` are equivalent to full administrative access**: `groups:write`
   adds you to any group, including one holding every permission; `users:write` mints a recovery code
   for any account. The containment the split buys is real on the **read** half only (`users:read` is

CHANGELOG.md

@@ -6,6 +6,18 @@ at boot. Entries start at 0.3.0.
 
 ## Unreleased
 
+### The admin example stops an administrator locking themselves out through a group
+
+On the Groups screen, unticking a permission on a group you are in, removing yourself or a group
+you are in, or deleting the group is refused when it would take away a permission you don't hold
+directly. The page says so, and another administrator can make the change. A permission you also
+hold through another group counts as lost, so such a change is refused as well.
+
+### The admin example titles a read-only user page "View user"
+
+A holder of `users:read` without `users:write` sees the user page titled "View user", with a "View"
+breadcrumb, instead of "Edit user".
+
 ### Browsers cache stylesheets instead of downloading them on every page
 
 Every `/public/` URL the host renders carries `?v=<hash of the file>` and is cached for a year as

examples/plugins/admin/admin-groups.ts

@@ -7,7 +7,7 @@
 // each returning a RouteResult.
 
 import { can, type KetoClient, type KratosAdmin, paginate, parseListQuery, type RelationQuery, type RelationTuple, type RequestContext, type RouteHandler, type RouteResult, type SubjectSet, type Translate, type User } from "@plainpages/plugin-api";
-import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, grantTuple, groupSubject, heldPermissions, isSavedNotice, isStale, type PermissionPicker, PERMISSIONS_FIELD, SAVED_NOTICE } from "./admin-grants.ts";
+import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, grantTuple, groupSubject, heldPermissions, isSavedNotice, isStale, type PermissionPicker, PERMISSIONS_FIELD, SAVED_NOTICE, userSubject } from "./admin-grants.ts";
 import { ADMIN_EN, type AdminAction, ADMIN_GROUPS_BASE, buildConfirmModel, guardedForm, notFound, partialSave, permissionName, requirePermission, unavailable } from "./admin-shared.ts";
 import type { FieldConfig } from "./admin-users.ts";
 
@@ -293,6 +293,35 @@ async function groupExists(keto: KetoClient, name: string): Promise<boolean> {
   return page.tuples.length > 0;
 }
 
+// Self-lockout guard, the group-side twin of the Users screen's self-revoke one: a change that would
+// take from the acting user a permission they hold through this group, and not directly, is refused.
+// One they also hold through another group counts as lost: telling the two apart needs a reverse
+// Keto query, and a false refusal only costs asking another administrator.
+async function locksSelfOut(keto: KetoClient, actor: string, group: string, lost: readonly string[]): Promise<boolean> {
+  if (lost.length === 0) return false;
+  if (!(await keto.check({ namespace: GROUP_NS, object: group, relation: MEMBERS, subject_id: `user:${actor}` }))) return false;
+  const direct = new Set(await heldPermissions(keto, userSubject(actor)));
+  return lost.some((name) => !direct.has(name));
+}
+
+// Leaving the group or deleting it loses everything the group confers, including through its own parents.
+async function leavingLocksSelfOut({ ctx, keto, user }: GroupsDeps, group: string): Promise<boolean> {
+  return locksSelfOut(keto, user.id, group, await effectivePermissions(keto, groupSubject(group), ctx.declaredPermissions));
+}
+
+// True when removing this member value takes the actor out of the group: it is them, or a nested group they are in.
+async function removesSelf(keto: KetoClient, actor: string, value: string): Promise<boolean> {
+  const subject = parseSubject(value);
+  if (!subject) return false;
+  if ("subject_id" in subject) return subject.subject_id === `user:${actor}`;
+  return keto.check({ namespace: GROUP_NS, object: subject.subject_set.object, relation: MEMBERS, subject_id: `user:${actor}` });
+}
+
+async function refuseSelfLockout(deps: GroupsDeps, name: string, refused: string): Promise<RouteResult> {
+  deps.ctx.log.warn("admin: refused a group change that would lock the actor out", { actor: deps.user.id, group: name, refused });
+  return { ...(await groupDetailPage(deps, name, { pageError: deps.ctx.t("admin.groups.error.selfLockout") })), status: 400 };
+}
+
 // Shared per-request deps for the Groups screen, resolved by `withGroups`: the gate (`groups:read` on
 // a GET, `groups:write` on a POST) + the Keto and Kratos capabilities (else a themed 503). Each route
 // below is a thin handler over these.
@@ -347,8 +376,9 @@ export const groupsCreate = withGroups(async (deps) => {
 // GET /admin/groups/new — the create form.
 export const groupsNewForm = withGroups((deps) => groupFormResult(deps, {}), "write");
 
-// The detail + membership page. `notice` and `error` ride the permission picker, which is where a save reports.
-async function groupDetailPage({ ctx, keto, kratosAdmin }: GroupsDeps, name: string, say: { error?: string; notice?: string } = {}): Promise<RouteResult> {
+// The detail + membership page. `notice` and `error` ride the permission picker, which is where a save
+// reports; `pageError` tops the page, for a refused membership change or delete.
+async function groupDetailPage({ ctx, keto, kratosAdmin }: GroupsDeps, name: string, say: { error?: string; notice?: string; pageError?: string } = {}): Promise<RouteResult> {
   const { emailById, options } = await memberCandidates(keto, kratosAdmin);
   const members = (await pagedTuples(keto, { namespace: GROUP_NS, object: name, relation: MEMBERS })).map((t) => memberView(t, emailById));
   const subject = groupSubject(name);
@@ -366,7 +396,7 @@ async function groupDetailPage({ ctx, keto, kratosAdmin }: GroupsDeps, name: str
     ...(say.error ? { error: say.error } : {}),
     ...(say.notice ? { notice: say.notice } : {}),
   };
-  return { data: { chrome: ctx.chrome, model: buildGroupDetailModel({ canWrite: !permissions.readOnly, candidates: options, csrfToken: ctx.chrome.csrfToken, group: { name }, members, permissions, t: ctx.t }) }, view: "group-detail" };
+  return { data: { chrome: ctx.chrome, model: buildGroupDetailModel({ canWrite: !permissions.readOnly, candidates: options, csrfToken: ctx.chrome.csrfToken, group: { name }, members, permissions, t: ctx.t, ...(say.pageError ? { error: say.pageError } : {}) }) }, view: "group-detail" };
 }
 
 // GET /admin/groups/:name — the detail + membership page.
@@ -383,6 +413,7 @@ export const groupsPermissions = withGroupName(async (deps, name) => {
   const held = await heldPermissions(keto, subject);
   const diff = grantDiff(ctx.declaredPermissions, held, form.getAll(PERMISSIONS_FIELD));
   if (isStale(form, ctx.declaredPermissions, held, diff)) return { ...(await groupDetailPage(deps, name, { error: ctx.t("admin.grants.stale") })), status: 409 };
+  if (await locksSelfOut(keto, user.id, name, diff.revoke)) return refuseSelfLockout(deps, name, diff.revoke.join(","));
   const { applied, error } = await applyGrants(keto, subject, diff);
   if (applied.grant.length > 0 || applied.revoke.length > 0) {
     ctx.log.info("admin: group permissions changed", { actor: user.id, granted: applied.grant.join(","), group: name, revoked: applied.revoke.join(",") });
@@ -403,19 +434,22 @@ export const groupsAddMember = withGroupName(async ({ ctx, keto }, name) => {
 });
 
 // GET /admin/groups/:name/delete — the deliberate confirm step.
-export const groupsDeleteConfirm = withGroupName((deps, name) => {
+export const groupsDeleteConfirm = withGroupName(async (deps, name) => {
+  if (await leavingLocksSelfOut(deps, name)) return refuseSelfLockout(deps, name, "delete");
   const base = detailHref(name);
   const tt = deps.ctx.t;
-  return Promise.resolve({ data: { chrome: deps.ctx.chrome, model: buildConfirmModel({
+  return { data: { chrome: deps.ctx.chrome, model: buildConfirmModel({
     breadcrumbs: [{ href: ADMIN_GROUPS_BASE, label: tt("admin.groups.title") }, { href: base, label: name }, { label: tt("common.delete") }],
     cancelHref: base, confirmAction: `${base}/delete`, confirmLabel: tt("admin.groups.delete"),
     message: tt("admin.groups.deleteMessage", { name }), title: tt("admin.groups.delete"),
-  }) }, view: "confirm" });
+  }) }, view: "confirm" };
 }, "write");
 
 // POST /admin/groups/:name/delete — remove every member tuple (the group ceases to exist).
-export const groupsDelete = withGroupName(async ({ ctx, keto, user }, name) => {
+export const groupsDelete = withGroupName(async (deps, name) => {
+  const { ctx, keto, user } = deps;
   await guardedForm(ctx); // CSRF-verify the POST
+  if (await leavingLocksSelfOut(deps, name)) return refuseSelfLockout(deps, name, "delete");
   // Drop what the group *holds* before what it *contains*: a Keto set exists only through its
   // tuples, so leaving the grants behind would resurrect every permission the moment someone
   // re-created a group with the same name.
@@ -428,9 +462,13 @@ export const groupsDelete = withGroupName(async ({ ctx, keto, user }, name) => {
 });
 
 // POST /admin/groups/:name/members/delete — remove one member.
-export const groupsRemoveMember = withGroupName(async ({ ctx, keto }, name) => {
+export const groupsRemoveMember = withGroupName(async (deps, name) => {
+  const { ctx, keto, user } = deps;
   const form = (await guardedForm(ctx))!;
-  const tuple = memberTuple(name, (form.get("member") ?? "").trim());
-  if (tuple) await keto.deleteTuple(tuple);
+  const value = (form.get("member") ?? "").trim();
+  const tuple = memberTuple(name, value);
+  if (!tuple) return { redirect: detailHref(name) };
+  if (await removesSelf(keto, user.id, value) && await leavingLocksSelfOut(deps, name)) return refuseSelfLockout(deps, name, value);
+  await keto.deleteTuple(tuple);
   return { redirect: detailHref(name) };
 });

examples/plugins/admin/admin-users.test.ts

@@ -100,6 +100,12 @@ test("buildUserFormModel: edit mode prefills, locks email, and exposes state/del
   assert.equal(buildUserFormModel({ identity: identity(4, { state: "inactive" }) }).edit!.nextLabel, "Reactivate");
 });
 
+test("buildUserFormModel: a users:read holder sees the page titled View user, with a View breadcrumb", () => {
+  const m = buildUserFormModel({ canWrite: false, identity: identity(3) });
+  assert.equal(m.title, "View user");
+  assert.deepEqual(m.breadcrumbs.at(-1), { label: "View" });
+});
+
 test("createIdentityPayload: schema/state/traits, name only when given, password only when set", () => {
   assert.deepEqual(createIdentityPayload({ email: "a@b.c", first: "Ada", last: "Lovelace", password: "" }), {
     schema_id: "default",

examples/plugins/admin/admin-users.ts

@@ -243,8 +243,9 @@ export function buildUserFormModel(opts: {
   if (!editing) fields.push({ autocomplete: "new-password", hint: t("admin.users.field.passwordHint"), icon: "i-lock", id: "password", label: t("admin.users.field.password"), name: "password", optional: true, type: "password" });
 
   const canWrite = opts.canWrite !== false;
+  const mode = !editing ? "new" : canWrite ? "edit" : "view";
   return {
-    breadcrumbs: [{ href: ADMIN_USERS_BASE, label: t("admin.users.title") }, { label: editing ? t("common.edit") : t("common.new") }],
+    breadcrumbs: [{ href: ADMIN_USERS_BASE, label: t("admin.users.title") }, { label: t({ edit: "common.edit", new: "common.new", view: "admin.users.view" }[mode]) }],
     canWrite, // the view drops every write affordance when false; the host already 403s the POSTs
     edit: editing ? {
       deleteAction: `${idPath}/delete`,
@@ -258,7 +259,7 @@ export function buildUserFormModel(opts: {
     form: { action: idPath, cancelHref: ADMIN_USERS_BASE, csrfToken: opts.csrfToken ?? "", fields, submitLabel: editing ? t("admin.users.save") : t("admin.users.create") },
     permissions: editing ? opts.permissions : undefined,
     recovery: opts.recovery,
-    title: editing ? t("admin.users.edit") : t("admin.users.new"),
+    title: t({ edit: "admin.users.edit", new: "admin.users.new", view: "admin.users.viewTitle" }[mode]),
   };
 }
 

examples/plugins/admin/i18n/en-US.ts

@@ -68,6 +68,7 @@ const messages = {
   "admin.groups.create": "Create group",
   "admin.groups.delete": "Delete group",
   "admin.groups.deleteMessage": "Delete group {{name}}? This can't be undone.",
+  "admin.groups.error.selfLockout": "This would take away permissions you hold through this group — ask another administrator, so you can't lock yourself out.",
   "admin.groups.field.name": "Group name",
   "admin.groups.field.nameHint": "Lowercase letters, digits, dashes and underscores.",
   "admin.groups.filter": "Filter groups",
@@ -153,6 +154,7 @@ const messages = {
   "admin.users.status.label": "Status",
   "admin.users.title": "Users",
   "admin.users.view": "View",
+  "admin.users.viewTitle": "View user",
 };
 
 export type AdminMessages = typeof messages;

examples/plugins/admin/i18n/sv-SE.ts

@@ -68,6 +68,7 @@ const messages: AdminMessages = {
   "admin.groups.create": "Skapa grupp",
   "admin.groups.delete": "Radera grupp",
   "admin.groups.deleteMessage": "Ta bort gruppen {{name}}? Det går inte att ångra.",
+  "admin.groups.error.selfLockout": "Det här skulle ta ifrån dig behörigheter du har genom den här gruppen — be en annan administratör, så att du inte låser ute dig själv.",
   "admin.groups.field.name": "Gruppnamn",
   "admin.groups.field.nameHint": "Små bokstäver, siffror, bindestreck och understreck.",
   "admin.groups.filter": "Filtrera grupper",
@@ -153,6 +154,7 @@ const messages: AdminMessages = {
   "admin.users.status.label": "Status",
   "admin.users.title": "Användare",
   "admin.users.view": "Visa",
+  "admin.users.viewTitle": "Visa användare",
 };
 
 export default messages;

src/http/app-admin.test.ts

@@ -26,7 +26,7 @@ const adminPlugin: Plugin = { ...adminManifest, id: "admin" };
 // Shared harness for the admin-screen HTTP tests: an app on a random port with an admin JWT +
 // CSRF cookie. get(path, permissions)/post(path, body) carry them; `token` is the matching CSRF field.
 const ADMIN_CSRF = "admin-secret";
-async function adminHarness(t: TestContext, opts: AppOptions = {}) {
+async function adminHarness(t: TestContext, opts: AppOptions = {}, sub = "admin1") {
   // Mount the plugin's catalogs the way server.ts does, so its screens render words, not keys.
   const i18n = createI18n(await loadI18n({ pluginIds: [adminPlugin.id], pluginsDir: examplesPluginsDir }));
   const app = createApp({ csrfSecret: ADMIN_CSRF, i18n, jwks: staticJwks([ecJwk]), pluginsDir: examplesPluginsDir, plugins: [adminPlugin], ...opts });
@@ -35,7 +35,7 @@ async function adminHarness(t: TestContext, opts: AppOptions = {}) {
   const url = `http://localhost:${(app.address() as AddressInfo).port}`;
   const token = issueCsrfToken(ADMIN_CSRF);
   const nowSec = Math.floor(Date.now() / 1000);
-  const cookie = (permissions: string[]) => `${SESSION_COOKIE}=${mintJwt({ email: "admin@x", exp: nowSec + 600, permissions, sub: "admin1" })}; ${CSRF_COOKIE}=${token}`;
+  const cookie = (permissions: string[]) => `${SESSION_COOKIE}=${mintJwt({ email: "admin@x", exp: nowSec + 600, permissions, sub })}; ${CSRF_COOKIE}=${token}`;
   const get = (path: string, permissions: string[] = ADMIN_ALL) => fetch(url + path, { headers: { cookie: cookie(permissions) }, redirect: "manual" });
   const post = (path: string, body: string) =>
     fetch(url + path, { body, headers: { "content-type": "application/x-www-form-urlencoded", cookie: cookie(ADMIN_ALL) }, method: "POST", redirect: "manual" });
@@ -430,6 +430,65 @@ test("admin permission grants: you can't revoke your own permissions, but you ca
   assert.ok(tuples.some((tp) => tp.object === "groups:read" && tp.subject_id === "user:admin1"));
 });
 
+// Keto's check over the tuple array, expanding nested groups the way Keto does.
+function resolvingCheck(tuples: RelationTuple[]) {
+  const reaches = (q: RelationTuple, ns: string, object: string, relation: string, seen: Set<string>): boolean => {
+    const key = `${ns}:${object}#${relation}`;
+    if (seen.has(key)) return false;
+    seen.add(key);
+    return tuples.some((tp) => tp.namespace === ns && tp.object === object && tp.relation === relation && (
+      (q.subject_id !== undefined && tp.subject_id === q.subject_id) ||
+      (q.subject_set !== undefined && tp.subject_set?.namespace === q.subject_set.namespace && tp.subject_set.object === q.subject_set.object) ||
+      (tp.subject_set !== undefined && reaches(q, tp.subject_set.namespace, tp.subject_set.object, tp.subject_set.relation, seen))));
+  };
+  return async (q: RelationTuple) => reaches(q, q.namespace, q.object, q.relation, new Set());
+}
+
+// The group paths can strip your access as surely as the user path: a group change reaches your
+// token at its next re-mint. Each is refused when you are in the group and would lose a permission
+// you don't hold directly. A member value must be a UUID, so this test signs in as `you`.
+test("admin groups: you can't take away permissions you hold only through a group you are in", async (t) => {
+  const ada = "01902d5e-7b6c-7e3a-9f21-3c8d1e0a4b01";
+  const you = "01902d5e-7b6c-7e3a-9f21-3c8d1e0a4b02";
+  const identities: Identity[] = [{ id: you, traits: { email: "you@example.com" } }, { id: ada, traits: { email: "ada@example.com" } }];
+  const member = (group: string, subject: string): RelationTuple => ({ namespace: "Group", object: group, relation: "members", subject_id: subject });
+  const nested = (group: string, child: string): RelationTuple => ({ namespace: "Group", object: group, relation: "members", subject_set: { namespace: "Group", object: child, relation: "members" } });
+  const grant = (permission: string, group: string): RelationTuple => ({ namespace: "Permission", object: permission, relation: "granted", subject_set: { namespace: "Group", object: group, relation: "members" } });
+  const tuples: RelationTuple[] = [
+    member("eng", `user:${you}`), member("eng", `user:${ada}`), grant("users:write", "eng"), grant("groups:write", "eng"),
+    { namespace: "Permission", object: "groups:write", relation: "granted", subject_id: `user:${you}` }, // also held directly
+    nested("leads", "eng"), grant("oauth2-clients:write", "leads"), // reached through eng
+    member("social", `user:${you}`), // confers nothing
+    member("design", `user:${ada}`), grant("users:write", "design"), // you are not in it
+  ];
+  const keto = fakeKeto(tuples, { check: resolvingCheck(tuples) });
+  const kratosAdmin = stubAdmin({ getIdentity: async (id) => identities.find((i) => i.id === id) ?? null, listIdentities: async () => ({ identities, nextPageToken: null }) });
+  const { get, post, token } = await adminHarness(t, { keto, kratosAdmin }, you);
+  const baseline = baselineOf(await (await get("/admin/groups/eng")).text());
+  const refusedWithMessage = async (res: Response) => {
+    assert.equal(res.status, 400);
+    assert.match(await res.text(), /lock yourself out/);
+  };
+
+  // Unticking users:write on eng would take it from you: refused, nothing revoked.
+  const before = JSON.stringify(tuples);
+  await refusedWithMessage(await post("/admin/groups/eng/permissions", `_csrf=${token}&baseline=${baseline}&permission=groups%3Awrite`));
+  // Leaving eng, removing eng from leads, or deleting eng (the confirm step too) are refused the same way.
+  await refusedWithMessage(await post("/admin/groups/eng/members/delete", `_csrf=${token}&member=user:${you}`));
+  await refusedWithMessage(await post("/admin/groups/leads/members/delete", `_csrf=${token}&member=group:eng`));
+  await refusedWithMessage(await get("/admin/groups/eng/delete"));
+  await refusedWithMessage(await post("/admin/groups/eng/delete", `_csrf=${token}`));
+  assert.equal(JSON.stringify(tuples), before, "nothing changed");
+
+  // You hold groups:write directly too, so unticking it on eng costs you nothing.
+  assert.equal((await post("/admin/groups/eng/permissions", `_csrf=${token}&baseline=${baseline}&permission=users%3Awrite`)).status, 303);
+  // Removing someone else, leaving a group that confers nothing, and deleting a group you are not in all go through.
+  assert.equal((await post("/admin/groups/eng/members/delete", `_csrf=${token}&member=user:${ada}`)).status, 303);
+  assert.equal((await post("/admin/groups/social/members/delete", `_csrf=${token}&member=user:${you}`)).status, 303);
+  assert.equal((await post("/admin/groups/design/delete", `_csrf=${token}`)).status, 303);
+  assert.ok(!tuples.some((tp) => tp.object === "design" || tp.subject_set?.object === "design"));
+});
+
 // The read/write split is only honest if the UI models it: a users:read holder must not be shown
 // buttons that 403 on submit. The gate already refuses them (asserted above); this is the affordance.
 test("admin screens render no write affordance for a read-only holder", async (t) => {

todo.md

@@ -12,8 +12,6 @@
 
 | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score |
 |---|---|---|---|---|---|---|---|---|---|---|---|
-| 3 | 0.4.21 |  |  | **Guard the group paths against self-lockout, or accept the gap explicitly.** | 3 | 2 | 2 | 5 | 6 | 2 by 6 | 1.4 |
-| 40 | 0.4.21 |  |  | **Show a `users:read` holder "View user" as the user page's heading and breadcrumb, not "Edit user".** | 1 | 2 | 1 | 2 | 2 | 2 by 1 | 1.4 |
 | 1 | 0.4.21 |  | owner | **Publish every release to Docker Hub, and give the Hub page its short description.** | 3 | 3 | 2 | 6 | 5 | 1 by 3 | 1.3 |
 | 32 | 0.4.21 |  |  | **Fail the gate on a stylesheet feature the browser floor lacks, unless it is guarded.** | 4 | 2 | 2 | 6 | 7 | code health by 2 | 1.1 |
 | 13 | 0.4.21 |  |  | **Accept a verification code pasted with surrounding whitespace.** | 4 | 2 | 3 | 7 | 7 | 2 by 8 | 1.0 |
@@ -30,20 +28,6 @@
 
 ## Details
 
-### 3. Guard the group paths against self-lockout, or accept the gap explicitly.
-
-The self-revoke guard covers only your own direct grants on the Users screen. Unticking a permission
-on a group you belong to, leaving that group, or deleting it can still strip your own access with no
-warning. AGENTS.md records this as a known gap. The robust fix is a "last effective holder" check,
-which needs a reverse Keto query.
-
-### 40. Show a `users:read` holder "View user" as the user page's heading and breadcrumb, not "Edit user".
-
-The users list now says View for a read-only holder, but the page it opens still titles itself
-`admin.users.edit` ("Edit user") with an "Edit" breadcrumb. `buildUserFormModel` in
-`examples/plugins/admin/admin-users.ts` already receives `canWrite`; the two admin catalog keys it
-needs are new.
-
 ### 1. Publish every release to Docker Hub, and give the Hub page its short description.
 
 Hub's newest tag is 0.4.0, while git has v0.4.1 onwards, and the overview still says 0.4.0. No