examples/plugins/admin/admin-groups.ts

@@ -293,33 +293,30 @@ 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
+const isMember = (keto: KetoClient, actor: string, group: string): Promise<boolean> =>
-// take from the acting user a permission they hold through this group, and not directly, is refused.
+  keto.check({ namespace: GROUP_NS, object: group, relation: MEMBERS, subject_id: `user:${actor}` });
-// 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.
+// The self-lockout guard, the group-side twin of the Users screen's self-revoke one: of the permissions
-async function locksSelfOut(keto: KetoClient, actor: string, group: string, lost: readonly string[]): Promise<boolean> {
+// a change takes from a group the actor is in, those they don't hold directly. One they also hold
-  if (lost.length === 0) return false;
+// through another group is included, since telling those apart needs a reverse Keto query.
-  if (!(await keto.check({ namespace: GROUP_NS, object: group, relation: MEMBERS, subject_id: `user:${actor}` }))) return false;
+async function lostToSelf(keto: KetoClient, actor: string, taken: readonly string[]): Promise<string[]> {
+  if (taken.length === 0) return [];
   const direct = new Set(await heldPermissions(keto, userSubject(actor)));
-  return lost.some((name) => !direct.has(name));
+  return taken.filter((name) => !direct.has(name));
 }
 
-// Leaving the group or deleting it loses everything the group confers, including through its own parents.
+// Leaving a group takes everything it confers, including through its own parents.
-async function leavingLocksSelfOut({ ctx, keto, user }: GroupsDeps, group: string): Promise<boolean> {
+async function lostOnLeaving({ ctx, keto, user }: GroupsDeps, group: string): Promise<string[]> {
-  return locksSelfOut(keto, user.id, group, await effectivePermissions(keto, groupSubject(group), ctx.declaredPermissions));
+  return lostToSelf(keto, user.id, 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 lostOnDelete(deps: GroupsDeps, group: string): Promise<string[]> {
-async function removesSelf(keto: KetoClient, actor: string, value: string): Promise<boolean> {
+  return (await isMember(deps.keto, deps.user.id, group)) ? lostOnLeaving(deps, group) : [];
-  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> {
+async function refuseSelfLockout(deps: GroupsDeps, name: string, action: string, lost: 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 });
+  deps.ctx.log.warn("admin: refused a group change that would lock the actor out", { action, actor: deps.user.id, group: name, lost: lost.join(",") });
-  return { ...(await groupDetailPage(deps, name, { pageError: deps.ctx.t("admin.groups.error.selfLockout") })), status: 400 };
+  return { ...(await groupDetailPage(deps, name, { pageError: deps.ctx.t("admin.groups.error.selfLockout", { permissions: lost.join(", ") }) })), status: 400 };
 }
 
 // Shared per-request deps for the Groups screen, resolved by `withGroups`: the gate (`groups:read` on
@@ -376,8 +373,8 @@ 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
+// The detail + membership page. `notice` and `error` ride the permission picker; `pageError` tops the
-// reports; `pageError` tops the page, for a refused membership change or delete.
+// page, for any change refused because it would lock the actor out.
 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));
@@ -413,7 +410,8 @@ 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 lost = diff.revoke.length > 0 && await isMember(keto, user.id, name) ? await lostToSelf(keto, user.id, diff.revoke) : [];
+  if (lost.length > 0) return refuseSelfLockout(deps, name, "save", lost);
   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(",") });
@@ -435,7 +433,8 @@ export const groupsAddMember = withGroupName(async ({ ctx, keto }, name) => {
 
 // GET /admin/groups/:name/delete — the deliberate confirm step.
 export const groupsDeleteConfirm = withGroupName(async (deps, name) => {
-  if (await leavingLocksSelfOut(deps, name)) return refuseSelfLockout(deps, name, "delete");
+  const lost = await lostOnDelete(deps, name);
+  if (lost.length > 0) return refuseSelfLockout(deps, name, "delete", lost);
   const base = detailHref(name);
   const tt = deps.ctx.t;
   return { data: { chrome: deps.ctx.chrome, model: buildConfirmModel({
@@ -449,7 +448,8 @@ export const groupsDeleteConfirm = withGroupName(async (deps, 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");
+  const lost = await lostOnDelete(deps, name);
+  if (lost.length > 0) return refuseSelfLockout(deps, name, "delete", lost);
   // 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.
@@ -465,10 +465,12 @@ export const groupsDelete = withGroupName(async (deps, name) => {
 export const groupsRemoveMember = withGroupName(async (deps, name) => {
   const { ctx, keto, user } = deps;
   const form = (await guardedForm(ctx))!;
-  const value = (form.get("member") ?? "").trim();
+  const tuple = memberTuple(name, (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);
+  // Removing the actor, or a nested group they are in, can take them out of this group.
+  const removesSelf = tuple.subject_set ? await isMember(keto, user.id, tuple.subject_set.object) : tuple.subject_id === `user:${user.id}`;
+  const lost = removesSelf ? await lostOnLeaving(deps, name) : [];
+  if (lost.length > 0) return refuseSelfLockout(deps, name, "remove", lost);
   await keto.deleteTuple(tuple);
   return { redirect: detailHref(name) };
 });

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

@@ -100,10 +100,17 @@ 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", () => {
+test("buildUserFormModel: the title and breadcrumb say New, Edit, or View for a users:read holder", () => {
-  const m = buildUserFormModel({ canWrite: false, identity: identity(3) });
+  const cases: [Parameters<typeof buildUserFormModel>[0], string, string][] = [
-  assert.equal(m.title, "View user");
+    [{}, "New user", "New"],
-  assert.deepEqual(m.breadcrumbs.at(-1), { label: "View" });
+    [{ identity: identity(3) }, "Edit user", "Edit"],
+    [{ canWrite: false, identity: identity(3) }, "View user", "View"],
+  ];
+  for (const [opts, title, crumb] of cases) {
+    const m = buildUserFormModel(opts);
+    assert.equal(m.title, title);
+    assert.deepEqual(m.breadcrumbs, [{ href: "/admin/users", label: "Users" }, { label: crumb }]);
+  }
 });
 
 test("createIdentityPayload: schema/state/traits, name only when given, password only when set", () => {

src/http/app-admin.test.ts

@@ -430,7 +430,7 @@ 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.
+// A check over the tuple array that expands nested groups.
 function resolvingCheck(tuples: RelationTuple[]) {
   const reaches = (q: RelationTuple, ns: string, object: string, relation: string, seen: Set<string>): boolean => {
     const key = `${ns}:${object}#${relation}`;
@@ -444,9 +444,7 @@ function resolvingCheck(tuples: RelationTuple[]) {
   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
+// A member value must be a UUID, so this test signs in as `you`.
-// 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";
@@ -465,19 +463,23 @@ test("admin groups: you can't take away permissions you hold only through a grou
   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) => {
+  // A refusal renders that group's own page and names what you would lose.
+  const refusedWithMessage = async (res: Response, group: string, lost: string) => {
     assert.equal(res.status, 400);
-    assert.match(await res.text(), /lock yourself out/);
+    const html = await res.text();
+    assert.match(html, new RegExp(`lock you out of ${lost}, which`));
+    assert.match(html, new RegExp(`action="/admin/groups/${group}/members"`));
   };
 
   // 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`));
+  await refusedWithMessage(await post("/admin/groups/eng/permissions", `_csrf=${token}&baseline=${baseline}&permission=groups%3Awrite`), "eng", "users:write");
-  // Leaving eng, removing eng from leads, or deleting eng (the confirm step too) are refused the same way.
+  // Leaving eng, removing eng from leads, or deleting eng (the confirm step too) are refused the same
-  await refusedWithMessage(await post("/admin/groups/eng/members/delete", `_csrf=${token}&member=user:${you}`));
+  // way; leaving eng also loses what it gets from leads.
-  await refusedWithMessage(await post("/admin/groups/leads/members/delete", `_csrf=${token}&member=group:eng`));
+  await refusedWithMessage(await post("/admin/groups/eng/members/delete", `_csrf=${token}&member=user:${you}`), "eng", "oauth2-clients:write, users:write");
-  await refusedWithMessage(await get("/admin/groups/eng/delete"));
+  await refusedWithMessage(await post("/admin/groups/leads/members/delete", `_csrf=${token}&member=group:eng`), "leads", "oauth2-clients:write");
-  await refusedWithMessage(await post("/admin/groups/eng/delete", `_csrf=${token}`));
+  await refusedWithMessage(await get("/admin/groups/eng/delete"), "eng", "oauth2-clients:write, users:write");
+  await refusedWithMessage(await post("/admin/groups/eng/delete", `_csrf=${token}`), "eng", "oauth2-clients:write, users:write");
   assert.equal(JSON.stringify(tuples), before, "nothing changed");
 
   // You hold groups:write directly too, so unticking it on eng costs you nothing.