From 4d2e7b59fde073751ca0d213f6f67f836d6e684a Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 8 Nov 2026 22:04:52 +0100 Subject: [PATCH 1/7] List grants no plugin declares with a Remove, and save a permission change as one Keto transaction --- CHANGELOG.md | 17 +++ docs/decisions.md | 2 +- docs/reading-cost.md | 11 ++ e2e-tests/compose.full.yml | 1 + e2e-tests/full-flow.spec.ts | 17 +++ .../plugins/auth-admin/admin-grants.test.ts | 61 +++++++--- examples/plugins/auth-admin/admin-grants.ts | 52 ++++++-- examples/plugins/auth-admin/admin-groups.ts | 23 ++-- examples/plugins/auth-admin/admin-shared.ts | 10 -- examples/plugins/auth-admin/admin-users.ts | 25 ++-- examples/plugins/auth-admin/i18n/en-US.ts | 4 +- examples/plugins/auth-admin/i18n/sv-SE.ts | 4 +- examples/plugins/auth-admin/plugin.ts | 6 +- .../views/partials/permission-picker.ejs | 25 +++- src/auth/keto-client.test.ts | 17 +++ src/auth/keto-client.ts | 51 +++++--- src/auth/login.test.ts | 1 + src/http/app-admin.test.ts | 115 ++++++++---------- src/test-tooling/app-harness.ts | 21 ++-- todo.md | 15 --- 20 files changed, 318 insertions(+), 160 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ecfd68..9001d8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,23 @@ The host's contract version is now 0.5.0, so discovery refuses a plugin whose `a `0.4.x`. Change the literal in each plugin's manifest when you upgrade, and add `assetHref` to any `RequestContext` your tests build by hand: `assetHref: (href) => href`. +### `KetoClient.transactTuples` applies inserts and deletes as one transaction + +`transactTuples({ delete, insert })` sends both lists to Keto in one `PATCH /admin/relation-tuples`, +so all of them apply or none does. A `KetoClient` you build by hand, such as a test stub, must add the +method. The admin example saves a user's or group's permissions through it: a Keto failure now +changes nothing and answers with the error page, instead of a "stopped part-way" report. Its +catalogs lose `admin.grants.partial`. + +### The admin example lists a user's or group's grants that no installed plugin declares, with a Remove + +A direct grant of a name no installed plugin declares, such as one left by an uninstalled plugin, +used to stay invisible and unchangeable, and reinstalling the plugin silently reactivated it. The +user and group pages now list these apart from the checkboxes, each with a Remove button (hidden from +a `:read`-only holder). The example's catalogs gain `admin.grants.remove`, `admin.grants.undeclaredHint` +and `admin.grants.undeclaredLegend`, and its routes gain `POST /auth-admin/users/:id/permissions/remove` +and `POST /auth-admin/groups/:name/permissions/remove`. + ### An operator can hide or rename a plugin's profile-menu entry A `profileMenu` entry takes an optional `id`, and `config/menu.ts`'s `override.hide` and diff --git a/docs/decisions.md b/docs/decisions.md index 96a4a1c..5bdb1ff 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -198,7 +198,7 @@ The host collects every installed plugin's declarations into one catalog (`decla Permissions admin screen: nothing in a GUI invents a name, and holding one is a property of a user or group, edited as a checkbox list there. A Keto tuple naming something no installed plugin declares gates nothing, is not offered, and is never revoked by an unrelated save: the picker only -speaks for what it showed. +speaks for what it showed. The subject's page lists such a direct grant apart, with a Remove of its own. ## A permission's `` is global, not plugin-scoped diff --git a/docs/reading-cost.md b/docs/reading-cost.md index e63b9bf..89bfff6 100644 --- a/docs/reading-cost.md +++ b/docs/reading-cost.md @@ -474,3 +474,14 @@ Read most (opens and searches): - `src/plugin-host/conflicts.ts` 3 - `AGENTS.md` 2 - `compose.yml` 2 + +## Reading cost, chunk 0d9d4ffb47aa, finished 2026-11-08T20:58:38Z + +48347 new input tokens before the first commit. + +Read most (opens and searches): + +- `src/http/app.ts` 4 +- `src/http/app.test.ts` 2 +- `todo.md` 2 +- `src/plugin-host/plugin.ts` 1 diff --git a/e2e-tests/compose.full.yml b/e2e-tests/compose.full.yml index 731333e..65db792 100644 --- a/e2e-tests/compose.full.yml +++ b/e2e-tests/compose.full.yml @@ -122,6 +122,7 @@ services: condition: service_healthy environment: BASE_URL: http://proxy + KETO_WRITE_URL: http://keto:4467 KRATOS_ADMIN_URL: http://kratos:4434 MAILPIT_URL: http://mailpit:8025 volumes: diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index f8ef089..250a618 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -15,6 +15,7 @@ const ADMIN_PASSWORD = "admin"; const SSO_EMAIL = "sso-user@plainpages.local"; // minted by the mock OIDC provider on first SSO login const suffix = randomUUID().slice(0, 8); // unique per run so re-runs don't collide on names const KRATOS_ADMIN = process.env.KRATOS_ADMIN_URL ?? "http://kratos:4434"; +const KETO_WRITE = process.env.KETO_WRITE_URL ?? "http://keto:4467"; const MAILPIT = process.env.MAILPIT_URL ?? "http://mailpit:8025"; // Drive the themed password login form → Kratos → /auth/complete → dashboard, signed in. @@ -193,6 +194,22 @@ test.describe.serial("authenticated admin journey", () => { await expect(staleTab.locator('input[name="permission"][value="scheduling:read"]')).toBeChecked(); await expect(staleTab.locator('input[name="permission"][value="scheduling:write"]')).not.toBeChecked(); await staleTab.context().close(); + + // The picker offers only declared names, so a grant no plugin declares can't be unticked: it is + // listed apart, and its own Remove deletes it. Keto is written directly, as only a removed plugin leaves one. + const stray = "retired:write"; + const put = await fetch(`${KETO_WRITE}/admin/relation-tuples`, { + body: JSON.stringify({ namespace: "Permission", object: stray, relation: "granted", subject_set: { namespace: "Group", object: group, relation: "members" } }), + headers: { "content-type": "application/json" }, + method: "PUT", + }); + expect(put.ok).toBe(true); + await page.goto(`/auth-admin/groups/${group}`); + await expect(page.getByRole("heading", { name: "Permissions no installed plugin declares" })).toBeVisible(); + await page.getByRole("button", { name: `Remove ${stray}` }).click(); + await expect(page.getByText("Permissions saved.")).toBeVisible(); + await expect(page.getByText(stray)).toHaveCount(0); + await expect(page.locator('input[name="permission"][value="scheduling:read"]')).toBeChecked(); // the declared grant stays }); test("OAuth2 clients CRUD: register a client (writes go to Hydra), see the one-time secret once, then delete it via the confirm step", async () => { diff --git a/examples/plugins/auth-admin/admin-grants.test.ts b/examples/plugins/auth-admin/admin-grants.test.ts index 67bab54..498ca6e 100644 --- a/examples/plugins/auth-admin/admin-grants.test.ts +++ b/examples/plugins/auth-admin/admin-grants.test.ts @@ -4,7 +4,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import type { PermissionDecl } from "@plainpages/plugin-api"; import type { KetoClient } from "@plainpages/plugin-api"; -import { applyGrants, baselineToken, buildPermissionPicker, grantDiff, grantTuple, groupSubject, isStale, userSubject } from "./admin-grants.ts"; +import { applyGrants, baselineToken, buildPermissionPicker, grantDiff, grantTuple, groupSubject, isStale, removeUndeclaredGrant, userSubject } from "./admin-grants.ts"; const declared: PermissionDecl[] = [ { description: "View users", name: "users:read" }, @@ -106,24 +106,55 @@ test("isStale refuses a missing or old token, but only for a save that would cha assert.equal(isStale(old, declared, held, nothing), false); // a repeated save has nothing to refuse }); -test("applyGrants applies writes then deletes, and on a Keto failure stops and reports what was applied", async () => { - const log: string[] = []; - const keto = (failOn?: string): KetoClient => ({ +test("applyGrants sends the whole change as one Keto transaction, and a failure applies none of it", async () => { + const calls: { delete: string[]; insert: string[] }[] = []; + const keto = (fail = false): KetoClient => ({ check: async () => false, - deleteTuple: async (tp) => { if (tp.object === failOn) throw new Error("boom"); log.push(`delete ${tp.object}`); }, + deleteTuple: async () => { throw new Error("unused"); }, expand: async () => ({ type: "leaf" }), listRelations: async () => ({ nextPageToken: null, tuples: [] }), - writeTuple: async (tp) => { if (tp.object === failOn) throw new Error("boom"); log.push(`write ${tp.object}`); }, + transactTuples: async (c) => { + if (fail) throw new Error("boom"); + calls.push({ delete: c.delete.map((tp) => tp.object), insert: c.insert.map((tp) => tp.object) }); + }, + writeTuple: async () => { throw new Error("unused"); }, }); const diff = { grant: ["a:read", "b:read"], revoke: ["c:read"] }; - const ok = await applyGrants(keto(), userSubject("u1"), diff); - assert.deepEqual(ok, { applied: diff }); - assert.deepEqual(log, ["write a:read", "write b:read", "delete c:read"]); - - log.length = 0; - const failed = await applyGrants(keto("b:read"), userSubject("u1"), diff); - assert.deepEqual(failed.applied, { grant: ["a:read"], revoke: [] }); - assert.ok(failed.error instanceof Error); - assert.deepEqual(log, ["write a:read"]); // nothing after the failure was attempted + await applyGrants(keto(), userSubject("u1"), diff); + assert.deepEqual(calls, [{ delete: ["c:read"], insert: ["a:read", "b:read"] }]); + await assert.rejects(applyGrants(keto(true), userSubject("u1"), diff), /boom/); +}); + +// grantDiff never touches a held name nobody declares, so the picker lists them apart and offers a +// Remove for each. A reader is told what is held but offered no button. +test("buildPermissionPicker lists direct grants no plugin declares, with a remove action only for a writer", () => { + const direct = ["zed:read", "users:read", "old:write"]; + const picker = buildPermissionPicker({ action: "/auth-admin/users/u1/permissions", declared, direct }); + assert.deepEqual(picker.undeclared?.names, ["old:write", "zed:read"]); + assert.equal(picker.undeclared?.action, "/auth-admin/users/u1/permissions/remove"); + assert.ok(picker.undeclared?.remove); + assert.equal(buildPermissionPicker({ action: "/x", declared, direct, readOnly: true }).undeclared?.remove, undefined); + assert.equal(buildPermissionPicker({ action: "/x", declared, direct: ["users:read"] }).undeclared, undefined); + assert.equal(picker.baseline, buildPermissionPicker({ action: "/x", declared, direct: ["users:read"] }).baseline, "an undeclared grant doesn't make a form stale"); +}); + +test("removeUndeclaredGrant deletes one held, undeclared grant and refuses anything else", async () => { + const subject = userSubject("u1"); + const tuples = [grantTuple("old:write", subject), grantTuple("users:read", subject), grantTuple("old:write", userSubject("u2"))]; + const deleted: string[] = []; + const keto: KetoClient = { + check: async () => false, + deleteTuple: async (tp) => { deleted.push(`${tp.object} ${tp.subject_id}`); }, + expand: async () => ({ type: "leaf" }), + listRelations: async (q = {}) => ({ nextPageToken: null, tuples: tuples.filter((tp) => tp.subject_id === q.subject_id) }), + transactTuples: async () => { throw new Error("unused"); }, + writeTuple: async () => { throw new Error("unused"); }, + }; + assert.equal(await removeUndeclaredGrant(keto, subject, declared, "old:write"), "old:write"); + assert.deepEqual(deleted, ["old:write user:u1"]); + assert.equal(await removeUndeclaredGrant(keto, subject, declared, "users:read"), undefined, "a declared name is the picker's to change"); + assert.equal(await removeUndeclaredGrant(keto, subject, declared, "never:held"), undefined); + assert.equal(await removeUndeclaredGrant(keto, subject, declared, null), undefined); + assert.deepEqual(deleted, ["old:write user:u1"]); }); diff --git a/examples/plugins/auth-admin/admin-grants.ts b/examples/plugins/auth-admin/admin-grants.ts index a6b7513..101d281 100644 --- a/examples/plugins/auth-admin/admin-grants.ts +++ b/examples/plugins/auth-admin/admin-grants.ts @@ -12,6 +12,7 @@ import type { KetoClient, PermissionDecl, RelationTuple, SubjectSet, Translate } const PERMISSION_NS = "Permission"; const GRANTED = "granted"; export const PERMISSIONS_FIELD = "permission"; // the checkbox name the two forms post +export const REMOVE_FIELD = "remove"; // the hidden field naming the undeclared grant a Remove form drops export const BASELINE_FIELD = "baseline"; // the hidden field carrying baselineToken export const SAVED_NOTICE = "permissions-saved"; // the `info-msg` a successful save redirects with @@ -73,6 +74,18 @@ export interface PermissionPicker { pending: string | undefined; readOnly: boolean; // the viewer holds :read but not :write — show the state, offer no save submit: string; + undeclared: UndeclaredGrants | undefined; // set when the subject holds a name no installed plugin declares +} + +// Direct grants of names nobody declares (their plugin was uninstalled, or the name was written to +// Keto by hand). grantDiff never touches them, so they get their own list and a Remove per name. +export interface UndeclaredGrants { + action: string; // where each Remove posts + field: string; + hint: string; + legend: string; + names: string[]; + remove: string | undefined; // the button label; unset for a reader, who is shown the names only } // The checkbox list: every declared permission, ticked where this subject holds it. A fixed list @@ -89,6 +102,7 @@ export function buildPermissionPicker(opts: { }): PermissionPicker { const t = opts.t ?? ((k: string) => k); const directSet = new Set(opts.direct); + const undeclared = undeclaredHeld(opts.declared, opts.direct); const effectiveSet = new Set(opts.effective ?? opts.direct); const choices = opts.declared.map((decl) => ({ checked: directSet.has(decl.name) || effectiveSet.has(decl.name), @@ -110,9 +124,22 @@ export function buildPermissionPicker(opts: { pending: opts.transitive === true ? t("admin.grants.pending") : undefined, readOnly: opts.readOnly === true, submit: t("admin.grants.save"), + undeclared: undeclared.length === 0 ? undefined : { + action: `${opts.action}/remove`, + field: REMOVE_FIELD, + hint: t("admin.grants.undeclaredHint"), + legend: t("admin.grants.undeclaredLegend"), + names: undeclared, + remove: opts.readOnly === true ? undefined : t("admin.grants.remove"), + }, }; } +const undeclaredHeld = (declared: readonly PermissionDecl[], held: readonly string[]): string[] => { + const offered = new Set(declared.map((d) => d.name)); + return held.filter((name) => !offered.has(name)).sort(); +}; + // What a submitted set changes. Pure so the diff is testable without Keto: only declared names are // considered, so a crafted POST cannot grant something no plugin gates on, and a held-but-undeclared // name (left over from an uninstalled plugin) is never silently revoked by an unrelated save. @@ -126,6 +153,15 @@ export function grantDiff(declared: readonly PermissionDecl[], held: string[], w }; } +// Deletes one held grant of a name no plugin declares; returns the name, or undefined when there was +// nothing to do (a declared name, which the picker owns, or one already gone, as after a double click). +export async function removeUndeclaredGrant(keto: KetoClient, subject: GrantSubject, declared: readonly PermissionDecl[], name: string | null): Promise { + if (name === null || declared.some((d) => d.name === name)) return undefined; + if (!(await heldPermissions(keto, subject)).includes(name)) return undefined; + await keto.deleteTuple(grantTuple(name, subject)); + return name; +} + // A token for the direct grants a form was rendered from. Like grantDiff it covers only the declared // names the form offered. It is a digest so an empty set is still a token; it is a guard against a // stale page, not a secret, since anyone who can read the page can compute it. @@ -142,16 +178,6 @@ export const isStale = (form: { get(name: string): string | null }, declared: re export interface GrantChange { grant: string[]; revoke: string[] } -// KetoClient writes one tuple per call, so the writes and deletes run one by one and stop at the first -// failure. What was applied by then is returned beside the error, so the caller can still revoke live -// tokens and tell the admin the save is only partly done. -export async function applyGrants(keto: KetoClient, subject: GrantSubject, diff: GrantChange): Promise<{ applied: GrantChange; error?: unknown }> { - const applied: GrantChange = { grant: [], revoke: [] }; - try { - for (const name of diff.grant) { await keto.writeTuple(grantTuple(name, subject)); applied.grant.push(name); } - for (const name of diff.revoke) { await keto.deleteTuple(grantTuple(name, subject)); applied.revoke.push(name); } - } catch (error) { - return { applied, error }; - } - return { applied }; -} +// One Keto transaction, so a failure leaves the subject's grants exactly as they were. +export const applyGrants = (keto: KetoClient, subject: GrantSubject, diff: GrantChange): Promise => + keto.transactTuples({ delete: diff.revoke.map((name) => grantTuple(name, subject)), insert: diff.grant.map((name) => grantTuple(name, subject)) }); diff --git a/examples/plugins/auth-admin/admin-groups.ts b/examples/plugins/auth-admin/admin-groups.ts index b4e7267..4bfcb56 100644 --- a/examples/plugins/auth-admin/admin-groups.ts +++ b/examples/plugins/auth-admin/admin-groups.ts @@ -7,8 +7,8 @@ // each returning a RouteResult. import { buildPaginationModel, 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, 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 { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, grantTuple, groupSubject, heldPermissions, isSavedNotice, isStale, type PermissionPicker, PERMISSIONS_FIELD, REMOVE_FIELD, removeUndeclaredGrant, SAVED_NOTICE, userSubject } from "./admin-grants.ts"; +import { ADMIN_EN, type AdminAction, ADMIN_GROUPS_BASE, buildConfirmModel, guardedForm, notFound, permissionName, requirePermission, unavailable } from "./admin-shared.ts"; import type { FieldConfig } from "./admin-users.ts"; const GROUP_NS = "Group"; @@ -395,17 +395,22 @@ export const groupsPermissions = withGroupName(async (deps, name) => { if (isStale(form, ctx.declaredPermissions, held, diff)) return { ...(await groupDetailPage(deps, name, { error: ctx.t("admin.grants.stale") })), status: 409 }; 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(",") }); - } - if (error) { - ctx.log.error("admin: group permissions only partly saved", { actor: user.id, error: String(error), group: name }); - return partialSave(ctx, async () => ({ ...(await groupDetailPage(deps, name, { error: ctx.t("admin.grants.partial") })), status: 502 })); + await applyGrants(keto, subject, diff); + if (diff.grant.length > 0 || diff.revoke.length > 0) { + ctx.log.info("admin: group permissions changed", { actor: user.id, granted: diff.grant.join(","), group: name, revoked: diff.revoke.join(",") }); } return { redirect: `${detailHref(name)}?info-msg=${SAVED_NOTICE}` }; }); +// POST /auth-admin/groups/:name/permissions/remove — drop one grant no installed plugin declares. +// No self-lockout guard: this screen's own gates are declared, so they are never what is removed here. +export const groupsPermissionsRemove = withGroupName(async ({ ctx, keto, user }, name) => { + const form = (await guardedForm(ctx))!; + const removed = await removeUndeclaredGrant(keto, groupSubject(name), ctx.declaredPermissions, form.get(REMOVE_FIELD)); + if (removed) ctx.log.info("admin: undeclared group permission removed", { actor: user.id, group: name, removed }); + return { redirect: `${detailHref(name)}?info-msg=${SAVED_NOTICE}` }; +}); + // POST /auth-admin/groups/:name/members — add a member (skip an invalid member or a self-nest). export const groupsAddMember = withGroupName(async ({ ctx, keto }, name) => { const form = (await guardedForm(ctx))!; diff --git a/examples/plugins/auth-admin/admin-shared.ts b/examples/plugins/auth-admin/admin-shared.ts index 3e322cb..e62b2c5 100644 --- a/examples/plugins/auth-admin/admin-shared.ts +++ b/examples/plugins/auth-admin/admin-shared.ts @@ -85,16 +85,6 @@ export function unavailable(ctx: RequestContext, what: string): RouteResult { return { data: { chrome: ctx.chrome, message: ctx.t("admin.unavailable.message", { what }), title: ctx.t("admin.unavailable.title") }, status: 503, view: "notice" }; } -// The page a partial permission save reports on reads Keto again. If that read fails too, the report -// stands alone on the notice page, so the admin still hears that part of the save landed. -export async function partialSave(ctx: RequestContext, render: () => Promise): Promise { - try { - return await render(); - } catch { - return { data: { chrome: ctx.chrome, message: ctx.t("admin.grants.partial"), title: ctx.t("admin.unavailable.title") }, status: 502, view: "notice" }; - } -} - // Model for the shared destructive-confirm page (views/confirm.ejs). The view reads the shell fields // (brand/csrf/theme/user/nav) from ctx.chrome; this carries only the page body + title/breadcrumbs. export function buildConfirmModel(opts: { diff --git a/examples/plugins/auth-admin/admin-users.ts b/examples/plugins/auth-admin/admin-users.ts index 5c0fe60..cbd58bd 100644 --- a/examples/plugins/auth-admin/admin-users.ts +++ b/examples/plugins/auth-admin/admin-users.ts @@ -4,8 +4,8 @@ // a shared `withUser` gate. import { buildPaginationModel, can, type Identity, type KetoClient, type KratosAdmin, KratosError, paginate, parseListQuery, type RecoveryCode, type RequestContext, type RouteHandler, type RouteResult, type Translate, type User } from "@plainpages/plugin-api"; -import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, heldPermissions, isSavedNotice, isStale, type PermissionPicker, PERMISSIONS_FIELD, SAVED_NOTICE, userSubject } from "./admin-grants.ts"; -import { ADMIN_EN, type AdminAction, ADMIN_USERS_BASE, buildConfirmModel, guardedForm, notFound, partialSave, permissionName, requirePermission, unavailable } from "./admin-shared.ts"; +import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, heldPermissions, isSavedNotice, isStale, type PermissionPicker, PERMISSIONS_FIELD, REMOVE_FIELD, removeUndeclaredGrant, SAVED_NOTICE, userSubject } from "./admin-grants.ts"; +import { ADMIN_EN, type AdminAction, ADMIN_USERS_BASE, buildConfirmModel, guardedForm, notFound, permissionName, requirePermission, unavailable } from "./admin-shared.ts"; const SCHEMA_ID = "default"; // matches kratos.yml identity.default_schema_id const DEFAULT_PAGE_SIZE = 25; @@ -364,14 +364,23 @@ export const usersPermissions = withTarget(async (deps, identity, id) => { ctx.log.warn("admin: refused a self-revoke of permissions", { actor: user.id, refused: diff.revoke.join(",") }); return refuse(ctx.t("admin.grants.selfRevoke"), 400); } - const { applied, error } = await applyGrants(keto, subject, diff); - if (applied.grant.length > 0 || applied.revoke.length > 0) { + await applyGrants(keto, subject, diff); + if (diff.grant.length > 0 || diff.revoke.length > 0) { revoke?.(id); - ctx.log.info("admin: user permissions changed", { actor: user.id, granted: applied.grant.join(","), revoked: applied.revoke.join(","), target: id }); + ctx.log.info("admin: user permissions changed", { actor: user.id, granted: diff.grant.join(","), revoked: diff.revoke.join(","), target: id }); } - if (error) { - ctx.log.error("admin: user permissions only partly saved", { actor: user.id, error: String(error), target: id }); - return partialSave(ctx, () => refuse(ctx.t("admin.grants.partial"), 502)); + return { redirect: `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}?info-msg=${SAVED_NOTICE}` }; +}); + +// POST /auth-admin/users/:id/permissions/remove — drop one direct grant no installed plugin declares. +// No self-lockout guard: this screen's own gates are declared, so they are never what is removed here. +export const usersPermissionsRemove = withTarget(async ({ ctx, keto, revoke, user }, _identity, id) => { + const form = (await guardedForm(ctx))!; + if (!keto) return unavailable(ctx, ctx.t("admin.capability.keto")); + const removed = await removeUndeclaredGrant(keto, userSubject(id), ctx.declaredPermissions, form.get(REMOVE_FIELD)); + if (removed) { + revoke?.(id); + ctx.log.info("admin: undeclared user permission removed", { actor: user.id, removed, target: id }); } return { redirect: `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}?info-msg=${SAVED_NOTICE}` }; }); diff --git a/examples/plugins/auth-admin/i18n/en-US.ts b/examples/plugins/auth-admin/i18n/en-US.ts index d7669eb..5b3a78a 100644 --- a/examples/plugins/auth-admin/i18n/en-US.ts +++ b/examples/plugins/auth-admin/i18n/en-US.ts @@ -53,12 +53,14 @@ const messages = { "admin.grants.inherited": "Greyed-out permissions come from a group. Change them on that group.", "admin.grants.legend": "Permissions", "admin.grants.none": "No installed plugin declares a permission, so there is nothing to grant.", - "admin.grants.partial": "Saving stopped part-way because the permission service failed, so only some changes were applied. Check the permissions and make any remaining changes again.", "admin.grants.pending": "Members get this at their next sign-in (up to 10 minutes).", + "admin.grants.remove": "Remove", "admin.grants.save": "Save permissions", "admin.grants.saved": "Permissions saved.", "admin.grants.selfRevoke": "You can't revoke your own permissions — ask another administrator, so you can't lock yourself out.", "admin.grants.stale": "These permissions changed while you had the page open, so nothing was saved. The list now shows the current state. Review it and save again.", + "admin.grants.undeclaredHint": "Held here, but no installed plugin declares them, so the list above can't change them. Installing a plugin that gates on one would activate it again. Remove any you don't recognise.", + "admin.grants.undeclaredLegend": "Permissions no installed plugin declares", "admin.groups.actions": "Group actions", "admin.groups.addMember": "Add a member", diff --git a/examples/plugins/auth-admin/i18n/sv-SE.ts b/examples/plugins/auth-admin/i18n/sv-SE.ts index 6df8e10..27a7a04 100644 --- a/examples/plugins/auth-admin/i18n/sv-SE.ts +++ b/examples/plugins/auth-admin/i18n/sv-SE.ts @@ -53,12 +53,14 @@ const messages: AdminMessages = { "admin.grants.inherited": "Gråmarkerade behörigheter kommer från en grupp. Ändra dem på gruppen.", "admin.grants.legend": "Behörigheter", "admin.grants.none": "Ingen installerad plugin deklarerar någon behörighet, så det finns inget att tilldela.", - "admin.grants.partial": "Sparandet avbröts halvvägs eftersom behörighetstjänsten gav fel, så bara en del av ändringarna genomfördes. Kontrollera behörigheterna och gör de återstående ändringarna igen.", "admin.grants.pending": "Medlemmar får detta vid nästa inloggning (upp till 10 minuter).", + "admin.grants.remove": "Ta bort", "admin.grants.save": "Spara behörigheter", "admin.grants.saved": "Behörigheterna är sparade.", "admin.grants.selfRevoke": "Du kan inte återkalla dina egna behörigheter — be en annan administratör, så att du inte låser ute dig själv.", "admin.grants.stale": "Behörigheterna ändrades medan du hade sidan öppen, så ingenting sparades. Listan visar nu aktuellt läge. Granska den och spara igen.", + "admin.grants.undeclaredHint": "Innehas här, men ingen installerad plugin deklarerar dem, så listan ovan kan inte ändra dem. Installerar du en plugin som spärrar på någon av dem blir den aktiv igen. Ta bort dem du inte känner igen.", + "admin.grants.undeclaredLegend": "Behörigheter som ingen installerad plugin deklarerar", "admin.groups.actions": "Gruppåtgärder", "admin.groups.addMember": "Lägg till en medlem", diff --git a/examples/plugins/auth-admin/plugin.ts b/examples/plugins/auth-admin/plugin.ts index f5fc5b5..3e66a68 100644 --- a/examples/plugins/auth-admin/plugin.ts +++ b/examples/plugins/auth-admin/plugin.ts @@ -6,9 +6,9 @@ import { definePlugin, type HttpMethod, type Route, type RouteHandler } from "@plainpages/plugin-api"; import { clientsCreate, clientsDeleteConfirm, clientsDelete, clientsDetail, clientsList, clientsNewForm } from "./admin-clients.ts"; -import { groupsAddMember, groupsCreate, groupsDelete, groupsDeleteConfirm, groupsDetail, groupsList, groupsNewForm, groupsPermissions, groupsRemoveMember } from "./admin-groups.ts"; +import { groupsAddMember, groupsCreate, groupsDelete, groupsDeleteConfirm, groupsDetail, groupsList, groupsNewForm, groupsPermissions, groupsPermissionsRemove, groupsRemoveMember } from "./admin-groups.ts"; import { pluginSettingsList } from "./admin-plugin-settings.ts"; -import { usersCreate, usersDeleteConfirm, usersDelete, usersEditForm, usersList, usersNewForm, usersPermissions, usersRecovery, usersState, usersUpdate } from "./admin-users.ts"; +import { usersCreate, usersDeleteConfirm, usersDelete, usersEditForm, usersList, usersNewForm, usersPermissions, usersPermissionsRemove, usersRecovery, usersState, usersUpdate } from "./admin-users.ts"; import { ADMIN_NAV, actionForMethod, type AdminAction, type AdminResource, permissionName } from "./admin-shared.ts"; // One route factory per screen: a GET gates on `:read` and a POST on `:write`, @@ -54,6 +54,7 @@ export default definePlugin({ users("POST", "/users/:id/delete", usersDelete), users("POST", "/users/:id/recovery", usersRecovery), users("POST", "/users/:id/permissions", usersPermissions), + users("POST", "/users/:id/permissions/remove", usersPermissionsRemove), // Groups groups("GET", "/groups", groupsList), groups("POST", "/groups", groupsCreate), @@ -64,6 +65,7 @@ export default definePlugin({ groups("POST", "/groups/:name/delete", groupsDelete), groups("POST", "/groups/:name/members/delete", groupsRemoveMember), groups("POST", "/groups/:name/permissions", groupsPermissions), + groups("POST", "/groups/:name/permissions/remove", groupsPermissionsRemove), // OAuth2 clients clients("GET", "/clients", clientsList), clients("POST", "/clients", clientsCreate), diff --git a/examples/plugins/auth-admin/views/partials/permission-picker.ejs b/examples/plugins/auth-admin/views/partials/permission-picker.ejs index 8cb25df..a66fc91 100644 --- a/examples/plugins/auth-admin/views/partials/permission-picker.ejs +++ b/examples/plugins/auth-admin/views/partials/permission-picker.ejs @@ -7,7 +7,10 @@ there) and every row when `readOnly` (the viewer holds :read but not :write). Neither can be diffed into an accidental revoke, because grantDiff compares against the direct grants only. - Locals: csrfToken, permissions ({ action, baseline, choices, empty, error, field, hint, inheritedNote, legend, notice, pending, readOnly, submit }). + Locals: csrfToken, permissions ({ action, baseline, choices, empty, error, field, hint, inheritedNote, legend, notice, pending, readOnly, submit, undeclared }). + + `undeclared` lists direct grants of names no plugin declares, outside the form above because grantDiff + never revokes one. Each has its own Remove form, and a reader gets the names without the button. %>

<%= permissions.legend %>

@@ -50,5 +53,25 @@ <% if (permissions.pending) { -%>

<%= permissions.pending %>

<% } -%> +<% } -%> +<% if (permissions.undeclared) { const u = permissions.undeclared; -%> +

<%= u.legend %>

+

<%= u.hint %>

+
    +<% u.names.forEach((name) => { -%> +
  • +<% if (u.remove) { -%> +
    + + + <%= name %> + +
    +<% } else { -%> + <%= name %> +<% } -%> +
    • +<% }) -%> +
<% } -%>
diff --git a/src/auth/keto-client.test.ts b/src/auth/keto-client.test.ts index b4f719f..5acfbaf 100644 --- a/src/auth/keto-client.test.ts +++ b/src/auth/keto-client.test.ts @@ -104,3 +104,20 @@ test("deleteTuple DELETEs the write API by query params (204 resolves; non-204 t (e: unknown) => e instanceof KetoError && e.status === 404, ); }); + +test("transactTuples PATCHes every insert and delete in one request (204 resolves; non-204 throws; empty sends nothing)", async () => { + const add = { namespace: "Permission", object: "a:read", relation: "granted", subject_id: USER }; + const drop = { namespace: "Permission", object: "b:read", relation: "granted", subject_id: USER }; + const { calls, fetchImpl } = recorder(() => res(204)); + await keto(fetchImpl).transactTuples({ delete: [drop], insert: [add] }); + assert.equal(calls.length, 1); + assert.equal(calls[0]!.method, "PATCH"); + assert.equal(calls[0]!.url, `${WRITE}/admin/relation-tuples`); + assert.deepEqual(JSON.parse(calls[0]!.body!), [{ action: "insert", relation_tuple: add }, { action: "delete", relation_tuple: drop }]); + await keto(fetchImpl).transactTuples({ delete: [], insert: [] }); + assert.equal(calls.length, 1, "an empty change is not a request"); + await assert.rejects( + keto((async () => res(400, { error: "bad" })) as typeof fetch).transactTuples({ delete: [], insert: [add] }), + (e: unknown) => e instanceof KetoError && e.status === 400, + ); +}); diff --git a/src/auth/keto-client.ts b/src/auth/keto-client.ts index 98852e0..28420d8 100644 --- a/src/auth/keto-client.ts +++ b/src/auth/keto-client.ts @@ -1,6 +1,6 @@ // Keto client: typed `fetch` wrappers over Ory Keto's relation-tuple APIs — // `check` a permission, `listRelations`/`expand` to inspect them (read API), `writeTuple`/ -// `deleteTuple` to grant/revoke them (write API). Built-in `fetch` only, no SDK dep (AGENTS.md); +// `deleteTuple`/`transactTuples` to grant/revoke them (write API). Built-in `fetch` only, no SDK dep (AGENTS.md); // `fetchImpl`-injectable like the kratos clients. Read/write split onto the two ports config.ts // targets (ketoReadUrl 4466 / ketoWriteUrl 4467). @@ -56,6 +56,8 @@ export interface KetoClient { deleteTuple(tuple: RelationTuple): Promise; expand(set: SubjectSet, opts?: { maxDepth?: number }): Promise; listRelations(query?: RelationQuery): Promise; + // Inserts and deletes in one Keto transaction: all of them apply or none does. + transactTuples(changes: { delete: RelationTuple[]; insert: RelationTuple[] }): Promise; writeTuple(tuple: RelationTuple): Promise; } @@ -75,16 +77,14 @@ function tupleParams(t: Partial): URLSearchParams { return p; } -export function createKetoClient(config: { fetchImpl?: typeof fetch; readUrl: string; writeUrl: string }): KetoClient { - const read = config.readUrl.replace(/\/+$/, ""); - const write = config.writeUrl.replace(/\/+$/, ""); - const http = config.fetchImpl ?? fetch; - const tuples = `${write}/admin/relation-tuples`; +async function fail(action: string, res: Response): Promise { + throw new KetoError(`Keto ${action} failed (${res.status})`, res.status, await res.text()); +} - async function fail(action: string, res: Response): Promise { - throw new KetoError(`Keto ${action} failed (${res.status})`, res.status, await res.text()); - } +type ReadApi = Pick; +type WriteApi = Pick; +function readApi(http: typeof fetch, read: string): ReadApi { return { async check(tuple, opts = {}) { const params = tupleParams(tuple); @@ -95,11 +95,6 @@ export function createKetoClient(config: { fetchImpl?: typeof fetch; readUrl: st return ((await res.json()) as { allowed?: boolean }).allowed === true; }, - async deleteTuple(tuple) { - const res = await http(`${tuples}?${tupleParams(tuple)}`, { method: "DELETE" }); - if (res.status !== 204) await fail("delete tuple", res); - }, - async expand(set, opts = {}) { const params = tupleParams(set); if (opts.maxDepth !== undefined) params.set("max-depth", String(opts.maxDepth)); @@ -117,11 +112,37 @@ export function createKetoClient(config: { fetchImpl?: typeof fetch; readUrl: st const body = (await res.json()) as { next_page_token?: string; relation_tuples?: RelationTuple[] }; return { nextPageToken: body.next_page_token || null, tuples: body.relation_tuples ?? [] }; }, + }; +} + +function writeApi(http: typeof fetch, write: string): WriteApi { + const tuples = `${write}/admin/relation-tuples`; + const json = { "content-type": "application/json" }; + return { + async deleteTuple(tuple) { + const res = await http(`${tuples}?${tupleParams(tuple)}`, { method: "DELETE" }); + if (res.status !== 204) await fail("delete tuple", res); + }, + + async transactTuples(changes) { + const patches = [ + ...changes.insert.map((relation_tuple) => ({ action: "insert", relation_tuple })), + ...changes.delete.map((relation_tuple) => ({ action: "delete", relation_tuple })), + ]; + if (patches.length === 0) return; + const res = await http(tuples, { body: JSON.stringify(patches), headers: json, method: "PATCH" }); + if (res.status !== 204) await fail("transact tuples", res); + }, // PUT is idempotent — re-asserting an existing tuple is a no-op grant. async writeTuple(tuple) { - const res = await http(tuples, { body: JSON.stringify(tuple), headers: { "content-type": "application/json" }, method: "PUT" }); + const res = await http(tuples, { body: JSON.stringify(tuple), headers: json, method: "PUT" }); if (!res.ok) await fail("write tuple", res); }, }; } + +export function createKetoClient(config: { fetchImpl?: typeof fetch; readUrl: string; writeUrl: string }): KetoClient { + const http = config.fetchImpl ?? fetch; + return { ...readApi(http, config.readUrl.replace(/\/+$/, "")), ...writeApi(http, config.writeUrl.replace(/\/+$/, "")) }; +} diff --git a/src/auth/login.test.ts b/src/auth/login.test.ts index 5327bad..afedd71 100644 --- a/src/auth/login.test.ts +++ b/src/auth/login.test.ts @@ -16,6 +16,7 @@ const ketoStub = (over: Partial = {}): KetoClient => ({ deleteTuple: async () => {}, expand: async () => ({ type: "leaf" }), listRelations: async () => ({ nextPageToken: null, tuples: [] }), + transactTuples: async () => {}, writeTuple: async () => {}, ...over, }); diff --git a/src/http/app-admin.test.ts b/src/http/app-admin.test.ts index 63fc627..94d1638 100644 --- a/src/http/app-admin.test.ts +++ b/src/http/app-admin.test.ts @@ -16,7 +16,7 @@ import type { Plugin } from "../plugin-host/plugin.ts"; import adminManifest from "../../examples/plugins/auth-admin/plugin.ts"; import { createI18n } from "../i18n/runtime.ts"; import { loadI18n } from "../i18n/load.ts"; -import { ecJwk, fakeKeto, matchesTuple, mintJwt, stubAdmin, stubHydra } from "../test-tooling/app-harness.ts"; +import { ecJwk, fakeKeto, mintJwt, stubAdmin, stubHydra } from "../test-tooling/app-harness.ts"; // The HTTP-level admin tests mount the example plugin via createApp — stub Ory clients on // ctx.system, views from examples/plugins — exactly as an operator would after copying it in. @@ -314,31 +314,28 @@ test("admin permission grants: a save from a stale page is refused, and the page assert.ok(tuples.some((tp) => tp.object === "groups:read" && tp.subject_set?.object === "eng"), "the group keeps its grant"); }); -// A Keto error midway leaves some changes applied; the save says so and still revokes the user's live tokens. -test("admin permission grants: a Keto failure midway reports a partial save and keeps what was applied", async (t) => { +// The grant changes go to Keto as one transaction, so a Keto failure applies none of them and +// there is nothing to revoke: the host's error page answers, and the stored grants are unchanged. +test("admin permission grants: a Keto failure applies none of the changes and revokes no live token", async (t) => { const ada = randomUUID(); const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }]; - const tuples: RelationTuple[] = []; - let writes = 0; - const keto = fakeKeto(tuples, { - writeTuple: async (tp) => { - if (++writes === 2) throw new Error("keto unreachable"); - tuples.push(tp); - }, - }); + const tuples: RelationTuple[] = [{ namespace: "Permission", object: "users:read", relation: "granted", subject_id: `user:${ada}` }]; + const keto = fakeKeto(tuples, { transactTuples: async () => { throw new Error("keto unreachable"); } }); const kratosAdmin = stubAdmin({ getIdentity: async (id) => identities.find((i) => i.id === id) ?? null, listIdentities: async () => ({ identities, nextPageToken: null }) }); const denylist = createDenylist(); const { get, post, token } = await adminHarness(t, { denylist, keto, kratosAdmin }); const baseline = baselineOf(await (await get(`/auth-admin/users/${ada}`)).text()); - const res = await post(`/auth-admin/users/${ada}/permissions`, `_csrf=${token}&baseline=${baseline}&permission=groups%3Aread&permission=users%3Aread&permission=users%3Awrite`); - assert.equal(res.status, 502); - const page = await res.text(); - assert.match(page, /stopped part-way/); - assert.match(page, /value="groups:read"[^>]*checked/); // the first write landed, and the page shows what is stored - assert.doesNotMatch(page, /value="users:write"[^>]*checked/); - assert.deepEqual(tuples.map((tp) => tp.object), ["groups:read"]); - assert.equal(denylist.isRevoked(ada, 0), true); // the half that was applied still revokes live tokens + const res = await post(`/auth-admin/users/${ada}/permissions`, `_csrf=${token}&baseline=${baseline}&permission=groups%3Aread&permission=users%3Awrite`); + assert.equal(res.status, 500); + assert.deepEqual(tuples.map((tp) => tp.object), ["users:read"]); + assert.equal(denylist.isRevoked(ada, 0), false); + + tuples.push({ namespace: "Group", object: "eng", relation: "members", subject_id: `user:${ada}` }); + const groupBaseline = baselineOf(await (await get("/auth-admin/groups/eng")).text()); + const groupRes = await post("/auth-admin/groups/eng/permissions", `_csrf=${token}&baseline=${groupBaseline}&permission=groups%3Aread`); + assert.equal(groupRes.status, 500); + assert.equal(tuples.filter((tp) => tp.subject_set?.object === "eng").length, 0); }); // A double-clicked Save posts twice from one page; the second post finds the first one's result. @@ -360,54 +357,50 @@ test("admin permission grants: a save that changes nothing is not refused for a assert.equal(denylist.isRevoked(ada, 0), false); }); -// Keto answered the writes but not the re-read: the report must still reach the admin. -test("admin permission grants: a partial save is still reported when Keto cannot be read again", async (t) => { +// grantDiff never revokes a name nobody declares, so the page lists it apart and Remove is the only way out. +test("admin permission grants: a grant no plugin declares is listed with a Remove, which deletes it and revokes live tokens", async (t) => { const ada = randomUUID(); const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }]; - const tuples: RelationTuple[] = []; - let writes = 0; - let down = false; - let failureTakesKetoDown = true; - const keto = fakeKeto(tuples, { - listRelations: async (q = {}) => { - if (down) throw new Error("keto unreachable"); - return { nextPageToken: null, tuples: tuples.filter((tp) => matchesTuple(tp, q)) }; - }, - writeTuple: async (tp) => { - if (++writes === 2) { down = failureTakesKetoDown; throw new Error("keto unreachable"); } - tuples.push(tp); - }, - }); + const held = (object: string, subject: Partial): RelationTuple => ({ namespace: "Permission", object, relation: "granted", ...subject }); + const eng = { subject_set: { namespace: "Group", object: "eng", relation: "members" } }; + const tuples: RelationTuple[] = [ + held("users:read", { subject_id: `user:${ada}` }), held("old:write", { subject_id: `user:${ada}` }), + held("old:write", eng), held("old:write", { subject_id: "user:someone-else" }), + { namespace: "Group", object: "eng", relation: "members", subject_id: `user:${ada}` }, + ]; 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 }); - const baseline = baselineOf(await (await get(`/auth-admin/users/${ada}`)).text()); + const denylist = createDenylist(); + const { get, post, token } = await adminHarness(t, { denylist, keto: fakeKeto(tuples), kratosAdmin }); - const res = await post(`/auth-admin/users/${ada}/permissions`, `_csrf=${token}&baseline=${baseline}&permission=groups%3Aread&permission=users%3Aread`); - assert.equal(res.status, 502); - assert.match(await res.text(), /stopped part-way/); - assert.deepEqual(tuples.map((tp) => tp.object), ["groups:read"]); + const page = await (await get(`/auth-admin/users/${ada}`)).text(); + assert.match(page, /Permissions no installed plugin declares/); + assert.match(page, new RegExp(`action="/auth-admin/users/${ada}/permissions/remove#permissions-h"`)); + assert.match(page, /name="remove" value="old:write"/); + assert.doesNotMatch(page, /name="remove" value="users:read"/, "a declared name has no Remove"); + const readerPage = await (await get(`/auth-admin/users/${ada}`, ["users:read"])).text(); + assert.match(readerPage, /old:write/); + assert.doesNotMatch(readerPage, /permissions\/remove/); - // The same on a group, whose page needs Keto for its members as well. - down = false; - writes = 0; - tuples.length = 0; - tuples.push({ namespace: "Group", object: "eng", relation: "members", subject_id: `user:${ada}` }); - const groupBaseline = baselineOf(await (await get("/auth-admin/groups/eng")).text()); - const groupRes = await post("/auth-admin/groups/eng/permissions", `_csrf=${token}&baseline=${groupBaseline}&permission=groups%3Aread&permission=users%3Aread`); - assert.equal(groupRes.status, 502); - assert.match(await groupRes.text(), /stopped part-way/); - assert.ok(tuples.some((tp) => tp.object === "groups:read" && tp.subject_set?.object === "eng"), "the first write landed"); + // A declared name is the picker's to change, so a crafted Remove for one does nothing. + const declared = await post(`/auth-admin/users/${ada}/permissions/remove`, `_csrf=${token}&remove=users%3Aread`); + assert.equal(declared.status, 303); + assert.equal(tuples.length, 5); + assert.equal(denylist.isRevoked(ada, 0), false); - // With Keto still answering, the group page itself reports it and shows what is stored. - failureTakesKetoDown = false; - down = false; - writes = 0; - tuples.length = 1; - const again = await post("/auth-admin/groups/eng/permissions", `_csrf=${token}&baseline=${baselineOf(await (await get("/auth-admin/groups/eng")).text())}&permission=groups%3Aread&permission=users%3Aread`); - assert.equal(again.status, 502); - const page = await again.text(); - assert.match(page, /stopped part-way/); - assert.match(page, /value="groups:read"[^>]*checked/); + const removed = await post(`/auth-admin/users/${ada}/permissions/remove`, `_csrf=${token}&remove=old%3Awrite`); + assert.equal(removed.status, 303); + assert.equal(tuples.length, 4); + assert.ok(!tuples.some((tp) => tp.object === "old:write" && tp.subject_id === `user:${ada}`)); + assert.ok(tuples.some((tp) => tp.object === "old:write" && tp.subject_set?.object === "eng"), "the group's grant is untouched"); + assert.equal(denylist.isRevoked(ada, 0), true); + assert.equal((await post(`/auth-admin/users/${ada}/permissions/remove`, `_csrf=${token}&remove=old%3Awrite`)).status, 303, "a second click is harmless"); + assert.equal((await post(`/auth-admin/users/${ada}/permissions/remove`, "remove=old%3Awrite")).status, 403, "CSRF-guarded"); + + const groupPage = await (await get("/auth-admin/groups/eng")).text(); + assert.match(groupPage, /action="\/auth-admin\/groups\/eng\/permissions\/remove#permissions-h"/); + assert.equal((await post("/auth-admin/groups/eng/permissions/remove", `_csrf=${token}&remove=old%3Awrite`)).status, 303); + assert.ok(!tuples.some((tp) => tp.object === "old:write" && tp.subject_set?.object === "eng")); + assert.ok(tuples.some((tp) => tp.object === "old:write" && tp.subject_id === "user:someone-else"), "another subject's grant is untouched"); }); // Revoking your own grants can remove the last users:write on the deployment, and the instant-revoke diff --git a/src/test-tooling/app-harness.ts b/src/test-tooling/app-harness.ts index 4adaa79..23f7ca9 100644 --- a/src/test-tooling/app-harness.ts +++ b/src/test-tooling/app-harness.ts @@ -57,14 +57,19 @@ export const matchesTuple = (t: RelationTuple, f: Partial): boole (f.subject_id === undefined || t.subject_id === f.subject_id) && (f.subject_set === undefined || sameSet(t.subject_set, f.subject_set)); // A stateful in-memory KetoClient over a tuple array (writes mutate it); used by login + the admin screens. -export const fakeKeto = (tuples: RelationTuple[] = [], over: Partial = {}): KetoClient => ({ - check: async () => false, - deleteTuple: async (f) => { for (let i = tuples.length - 1; i >= 0; i--) if (matchesTuple(tuples[i]!, f)) tuples.splice(i, 1); }, - expand: async () => ({ type: "leaf" }), - listRelations: async (q = {}) => ({ nextPageToken: null, tuples: tuples.filter((t) => matchesTuple(t, q)) }), - writeTuple: async (tp) => { if (!tuples.some((t) => matchesTuple(t, tp) && sameSet(t.subject_set, tp.subject_set))) tuples.push(tp); }, - ...over, -}); +export const fakeKeto = (tuples: RelationTuple[] = [], over: Partial = {}): KetoClient => { + const remove = (f: Partial): void => { for (let i = tuples.length - 1; i >= 0; i--) if (matchesTuple(tuples[i]!, f)) tuples.splice(i, 1); }; + const add = (tp: RelationTuple): void => { if (!tuples.some((t) => matchesTuple(t, tp) && sameSet(t.subject_set, tp.subject_set))) tuples.push(tp); }; + return { + check: async () => false, + deleteTuple: async (f) => remove(f), + expand: async () => ({ type: "leaf" }), + listRelations: async (q = {}) => ({ nextPageToken: null, tuples: tuples.filter((t) => matchesTuple(t, q)) }), + transactTuples: async (c) => { c.insert.forEach(add); c.delete.forEach(remove); }, + writeTuple: async (tp) => add(tp), + ...over, + }; +}; export const withWhoami = (whoami: KratosPublic["whoami"]): KratosPublic => ({ ...mockKratos(async () => { throw new Error("unused"); }), whoami }); // OAuth2 login challenge: another app logs in *through* us; Hydra hands the browser here. diff --git a/todo.md b/todo.md index 755276f..9acc202 100644 --- a/todo.md +++ b/todo.md @@ -12,8 +12,6 @@ | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| -| 5 | 0.5.0 | | | **List grants that no installed plugin declares, with a remove action.** | 2 | 2 | 2 | 4 | 4 | 2 by 3 | 2.1 | -| 41 | 0.5.0 | | | **Apply a permission save as one Keto call, so it cannot stop part-way.** | 2 | 2 | 2 | 4 | 3 | 2 by 2 | 2.1 | | 19 | 0.6.0 | | question | **Run the documented Docker commands on macOS and fix whatever misbehaves.** | 2 | 2 | 1 | 3 | 3 | 1 by 3 | 3.3 | | 16 | 0.6.0 | | question | **Decide whether the single Keto `Resource` namespace splits into per-domain namespaces.** | 3 | 2 | 3 | 6 | 6 | 7 by 2 | 0.5 | | 14 | 0.6.0 | | | **Drop a double-clicked duplicate submit, with no client-side JavaScript.** | 4 | 2 | 3 | 7 | 7 | 2 by 7, 1 by 4 | 1.4 | @@ -25,19 +23,6 @@ ## Details -### 5. List grants that no installed plugin declares, with a remove action. - -`grantDiff` never revokes an undeclared name, and nothing shows one either, so it can't be audited or -cleaned, and reinstalling its plugin silently reactivates access nobody remembers granting. - -### 41. Apply a permission save as one Keto call, so it cannot stop part-way. - -`applyGrants` writes and deletes one tuple per request and reports a partial save when one fails. -**Check first:** Keto documents `PATCH /admin/relation-tuples`, which should apply a list of inserts -and deletes in one transaction; confirm that against the Keto version `ory/` pins. If so, add the -call to `KetoClient`, which is contract surface (`@plainpages/plugin-api`) and so needs 0.5.0, and the -partial-failure path in the admin example can go. - ### 19. Run the documented Docker commands on macOS and fix whatever misbehaves. Needs a Mac. Two suspects, both from `--user "$(id -u):$(id -g)"`: a macOS `id -g` is `20`, which is -- 2.52.0 From 2de7e4d79ce6749665a1d37ca5caf0cb379ed197 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 8 Nov 2026 22:15:54 +0100 Subject: [PATCH 2/7] =?UTF-8?q?Answer=20product-owner=20round=201:=200=20?= =?UTF-8?q?=F0=9F=94=B4,=201=20=F0=9F=9F=A1=20(0=20misread);=20a=20Remove?= =?UTF-8?q?=20confirms=20with=20its=20own=20notice,=20the=20hint=20no=20lo?= =?UTF-8?q?nger=20says=20to=20remove,=20the=20example=20README=20names=20t?= =?UTF-8?q?he=20list?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- e2e-tests/full-flow.spec.ts | 5 ++--- examples/plugins/auth-admin/README.md | 1 + examples/plugins/auth-admin/admin-grants.ts | 15 ++++++++++----- examples/plugins/auth-admin/admin-groups.ts | 7 +++---- examples/plugins/auth-admin/admin-users.ts | 7 +++---- examples/plugins/auth-admin/i18n/en-US.ts | 3 ++- examples/plugins/auth-admin/i18n/sv-SE.ts | 3 ++- 7 files changed, 23 insertions(+), 18 deletions(-) diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index 250a618..2d04861 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -195,8 +195,7 @@ test.describe.serial("authenticated admin journey", () => { await expect(staleTab.locator('input[name="permission"][value="scheduling:write"]')).not.toBeChecked(); await staleTab.context().close(); - // The picker offers only declared names, so a grant no plugin declares can't be unticked: it is - // listed apart, and its own Remove deletes it. Keto is written directly, as only a removed plugin leaves one. + // No screen can grant a name no plugin declares, so write one to Keto directly. const stray = "retired:write"; const put = await fetch(`${KETO_WRITE}/admin/relation-tuples`, { body: JSON.stringify({ namespace: "Permission", object: stray, relation: "granted", subject_set: { namespace: "Group", object: group, relation: "members" } }), @@ -207,7 +206,7 @@ test.describe.serial("authenticated admin journey", () => { await page.goto(`/auth-admin/groups/${group}`); await expect(page.getByRole("heading", { name: "Permissions no installed plugin declares" })).toBeVisible(); await page.getByRole("button", { name: `Remove ${stray}` }).click(); - await expect(page.getByText("Permissions saved.")).toBeVisible(); + await expect(page.getByText("Permission removed.")).toBeVisible(); await expect(page.getByText(stray)).toHaveCount(0); await expect(page.locator('input[name="permission"][value="scheduling:read"]')).toBeChecked(); // the declared grant stays }); diff --git a/examples/plugins/auth-admin/README.md b/examples/plugins/auth-admin/README.md index 1114424..16980d5 100644 --- a/examples/plugins/auth-admin/README.md +++ b/examples/plugins/auth-admin/README.md @@ -42,6 +42,7 @@ Admin section entirely. There is **no Permissions screen**. Permission names are declared in plugin code, not created in a GUI, so the host's catalog (`ctx.declaredPermissions`) is the fixed list — and holding one is a property of a user or a group, edited as a checkbox list on those two screens (`admin-grants.ts`). +A grant of a name no installed plugin declares is listed apart on the same page, with a Remove. ## Layout diff --git a/examples/plugins/auth-admin/admin-grants.ts b/examples/plugins/auth-admin/admin-grants.ts index 101d281..c90c79c 100644 --- a/examples/plugins/auth-admin/admin-grants.ts +++ b/examples/plugins/auth-admin/admin-grants.ts @@ -15,9 +15,14 @@ export const PERMISSIONS_FIELD = "permission"; // the checkbox name the two form export const REMOVE_FIELD = "remove"; // the hidden field naming the undeclared grant a Remove form drops export const BASELINE_FIELD = "baseline"; // the hidden field carrying baselineToken export const SAVED_NOTICE = "permissions-saved"; // the `info-msg` a successful save redirects with +export const REMOVED_NOTICE = "permission-removed"; // the `info-msg` a Remove redirects with; its own words, since the checkboxes were not saved -// Only that exact value counts, so a crafted link can't put words on the page. -export const isSavedNotice = (url: URL): boolean => url.searchParams.get("info-msg") === SAVED_NOTICE; +// Only these exact values count, so a crafted link can't put words on the page. +export function grantsNotice(url: URL, t: Translate): string | undefined { + const msg = url.searchParams.get("info-msg"); + if (msg === SAVED_NOTICE) return t("admin.grants.saved"); + return msg === REMOVED_NOTICE ? t("admin.grants.removed") : undefined; +} export type GrantSubject = { subject_id: string } | { subject_set: SubjectSet }; @@ -153,8 +158,9 @@ export function grantDiff(declared: readonly PermissionDecl[], held: string[], w }; } -// Deletes one held grant of a name no plugin declares; returns the name, or undefined when there was -// nothing to do (a declared name, which the picker owns, or one already gone, as after a double click). +// Deletes one held grant of a name no plugin declares and returns the name. Deletes nothing for a +// declared name, which the picker owns (so a Remove can't take away this plugin's own gates and needs +// no self-lockout guard), or for one not held, as after a double click. export async function removeUndeclaredGrant(keto: KetoClient, subject: GrantSubject, declared: readonly PermissionDecl[], name: string | null): Promise { if (name === null || declared.some((d) => d.name === name)) return undefined; if (!(await heldPermissions(keto, subject)).includes(name)) return undefined; @@ -178,6 +184,5 @@ export const isStale = (form: { get(name: string): string | null }, declared: re export interface GrantChange { grant: string[]; revoke: string[] } -// One Keto transaction, so a failure leaves the subject's grants exactly as they were. export const applyGrants = (keto: KetoClient, subject: GrantSubject, diff: GrantChange): Promise => keto.transactTuples({ delete: diff.revoke.map((name) => grantTuple(name, subject)), insert: diff.grant.map((name) => grantTuple(name, subject)) }); diff --git a/examples/plugins/auth-admin/admin-groups.ts b/examples/plugins/auth-admin/admin-groups.ts index 4bfcb56..d767aa8 100644 --- a/examples/plugins/auth-admin/admin-groups.ts +++ b/examples/plugins/auth-admin/admin-groups.ts @@ -7,7 +7,7 @@ // each returning a RouteResult. import { buildPaginationModel, 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, REMOVE_FIELD, removeUndeclaredGrant, SAVED_NOTICE, userSubject } from "./admin-grants.ts"; +import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, grantTuple, groupSubject, heldPermissions, grantsNotice, isStale, type PermissionPicker, PERMISSIONS_FIELD, REMOVE_FIELD, removeUndeclaredGrant, REMOVED_NOTICE, SAVED_NOTICE, userSubject } from "./admin-grants.ts"; import { ADMIN_EN, type AdminAction, ADMIN_GROUPS_BASE, buildConfirmModel, guardedForm, notFound, permissionName, requirePermission, unavailable } from "./admin-shared.ts"; import type { FieldConfig } from "./admin-users.ts"; @@ -381,7 +381,7 @@ async function groupDetailPage({ ctx, keto, kratosAdmin }: GroupsDeps, name: str // GET /auth-admin/groups/:name — the detail + membership page. export const groupsDetail = withGroupName((deps, name) => - groupDetailPage(deps, name, isSavedNotice(deps.ctx.url) ? { notice: deps.ctx.t("admin.grants.saved") } : {})); + groupDetailPage(deps, name, { notice: grantsNotice(deps.ctx.url, deps.ctx.t) ?? "" })); // POST /auth-admin/groups/:name/permissions — the submitted checkboxes are the desired set. Members hold // a group's permissions transitively, so the change reaches them at their next login or re-mint — @@ -403,12 +403,11 @@ export const groupsPermissions = withGroupName(async (deps, name) => { }); // POST /auth-admin/groups/:name/permissions/remove — drop one grant no installed plugin declares. -// No self-lockout guard: this screen's own gates are declared, so they are never what is removed here. export const groupsPermissionsRemove = withGroupName(async ({ ctx, keto, user }, name) => { const form = (await guardedForm(ctx))!; const removed = await removeUndeclaredGrant(keto, groupSubject(name), ctx.declaredPermissions, form.get(REMOVE_FIELD)); if (removed) ctx.log.info("admin: undeclared group permission removed", { actor: user.id, group: name, removed }); - return { redirect: `${detailHref(name)}?info-msg=${SAVED_NOTICE}` }; + return { redirect: `${detailHref(name)}?info-msg=${REMOVED_NOTICE}` }; }); // POST /auth-admin/groups/:name/members — add a member (skip an invalid member or a self-nest). diff --git a/examples/plugins/auth-admin/admin-users.ts b/examples/plugins/auth-admin/admin-users.ts index cbd58bd..4a4ac9f 100644 --- a/examples/plugins/auth-admin/admin-users.ts +++ b/examples/plugins/auth-admin/admin-users.ts @@ -4,7 +4,7 @@ // a shared `withUser` gate. import { buildPaginationModel, can, type Identity, type KetoClient, type KratosAdmin, KratosError, paginate, parseListQuery, type RecoveryCode, type RequestContext, type RouteHandler, type RouteResult, type Translate, type User } from "@plainpages/plugin-api"; -import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, heldPermissions, isSavedNotice, isStale, type PermissionPicker, PERMISSIONS_FIELD, REMOVE_FIELD, removeUndeclaredGrant, SAVED_NOTICE, userSubject } from "./admin-grants.ts"; +import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, heldPermissions, grantsNotice, isStale, type PermissionPicker, PERMISSIONS_FIELD, REMOVE_FIELD, removeUndeclaredGrant, REMOVED_NOTICE, SAVED_NOTICE, userSubject } from "./admin-grants.ts"; import { ADMIN_EN, type AdminAction, ADMIN_USERS_BASE, buildConfirmModel, guardedForm, notFound, permissionName, requirePermission, unavailable } from "./admin-shared.ts"; const SCHEMA_ID = "default"; // matches kratos.yml identity.default_schema_id @@ -312,7 +312,7 @@ export const usersNewForm = withUser(({ ctx }) => Promise.resolve(formResult(ctx // GET /auth-admin/users/:id — the edit form, prefilled. export const usersEditForm = withTarget(async (deps, identity, id) => { - const permissions = await userPermissionPicker(deps, id, undefined, isSavedNotice(deps.ctx.url) ? deps.ctx.t("admin.grants.saved") : undefined); + const permissions = await userPermissionPicker(deps, id, undefined, grantsNotice(deps.ctx.url, deps.ctx.t)); return formResult(deps.ctx, { canWrite: canWriteUsers(deps.ctx), identity, ...(permissions ? { permissions } : {}) }); }); @@ -373,7 +373,6 @@ export const usersPermissions = withTarget(async (deps, identity, id) => { }); // POST /auth-admin/users/:id/permissions/remove — drop one direct grant no installed plugin declares. -// No self-lockout guard: this screen's own gates are declared, so they are never what is removed here. export const usersPermissionsRemove = withTarget(async ({ ctx, keto, revoke, user }, _identity, id) => { const form = (await guardedForm(ctx))!; if (!keto) return unavailable(ctx, ctx.t("admin.capability.keto")); @@ -382,7 +381,7 @@ export const usersPermissionsRemove = withTarget(async ({ ctx, keto, revoke, use revoke?.(id); ctx.log.info("admin: undeclared user permission removed", { actor: user.id, removed, target: id }); } - return { redirect: `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}?info-msg=${SAVED_NOTICE}` }; + return { redirect: `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}?info-msg=${REMOVED_NOTICE}` }; }); // POST /auth-admin/users/:id — save edits; a Kratos 4xx re-renders the form (400). diff --git a/examples/plugins/auth-admin/i18n/en-US.ts b/examples/plugins/auth-admin/i18n/en-US.ts index 5b3a78a..f9a5b9c 100644 --- a/examples/plugins/auth-admin/i18n/en-US.ts +++ b/examples/plugins/auth-admin/i18n/en-US.ts @@ -55,11 +55,12 @@ const messages = { "admin.grants.none": "No installed plugin declares a permission, so there is nothing to grant.", "admin.grants.pending": "Members get this at their next sign-in (up to 10 minutes).", "admin.grants.remove": "Remove", + "admin.grants.removed": "Permission removed.", "admin.grants.save": "Save permissions", "admin.grants.saved": "Permissions saved.", "admin.grants.selfRevoke": "You can't revoke your own permissions — ask another administrator, so you can't lock yourself out.", "admin.grants.stale": "These permissions changed while you had the page open, so nothing was saved. The list now shows the current state. Review it and save again.", - "admin.grants.undeclaredHint": "Held here, but no installed plugin declares them, so the list above can't change them. Installing a plugin that gates on one would activate it again. Remove any you don't recognise.", + "admin.grants.undeclaredHint": "These permissions are held here, but no installed plugin declares them, so the list above can't change them. A plugin may still check one without declaring it.", "admin.grants.undeclaredLegend": "Permissions no installed plugin declares", "admin.groups.actions": "Group actions", diff --git a/examples/plugins/auth-admin/i18n/sv-SE.ts b/examples/plugins/auth-admin/i18n/sv-SE.ts index 27a7a04..fd4fba0 100644 --- a/examples/plugins/auth-admin/i18n/sv-SE.ts +++ b/examples/plugins/auth-admin/i18n/sv-SE.ts @@ -55,11 +55,12 @@ const messages: AdminMessages = { "admin.grants.none": "Ingen installerad plugin deklarerar någon behörighet, så det finns inget att tilldela.", "admin.grants.pending": "Medlemmar får detta vid nästa inloggning (upp till 10 minuter).", "admin.grants.remove": "Ta bort", + "admin.grants.removed": "Behörigheten är borttagen.", "admin.grants.save": "Spara behörigheter", "admin.grants.saved": "Behörigheterna är sparade.", "admin.grants.selfRevoke": "Du kan inte återkalla dina egna behörigheter — be en annan administratör, så att du inte låser ute dig själv.", "admin.grants.stale": "Behörigheterna ändrades medan du hade sidan öppen, så ingenting sparades. Listan visar nu aktuellt läge. Granska den och spara igen.", - "admin.grants.undeclaredHint": "Innehas här, men ingen installerad plugin deklarerar dem, så listan ovan kan inte ändra dem. Installerar du en plugin som spärrar på någon av dem blir den aktiv igen. Ta bort dem du inte känner igen.", + "admin.grants.undeclaredHint": "Behörigheterna innehas här, men ingen installerad plugin deklarerar dem, så listan ovan kan inte ändra dem. En plugin kan ändå kontrollera någon av dem utan att deklarera den.", "admin.grants.undeclaredLegend": "Behörigheter som ingen installerad plugin deklarerar", "admin.groups.actions": "Gruppåtgärder", -- 2.52.0 From 63c90ea3d617bbb8429c59f78add1f85ce370b8b Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 8 Nov 2026 22:15:54 +0100 Subject: [PATCH 3/7] =?UTF-8?q?Answer=20stability=20round=201:=200=20?= =?UTF-8?q?=F0=9F=94=B4,=205=20=F0=9F=9F=A1=20(1=20misread);=20pin=20the?= =?UTF-8?q?=20revoke,=20redirect,=20503=20and=20URL-slash=20paths=20with?= =?UTF-8?q?=20tests,=20and=20stop=20calling=20an=20undeclared=20grant=20do?= =?UTF-8?q?rmant?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/decisions.md | 5 +++-- src/auth/keto-client.test.ts | 9 ++++++++ src/http/app-admin.test.ts | 42 +++++++++++++++++++++++++++++++----- src/plugin-host/plugin.ts | 2 +- 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/docs/decisions.md b/docs/decisions.md index 5bdb1ff..6737914 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -197,8 +197,9 @@ The host collects every installed plugin's declarations into one catalog (`decla `ctx.declaredPermissions`), and that catalog is the list the admin screens offer. Hence there is no Permissions admin screen: nothing in a GUI invents a name, and holding one is a property of a user or group, edited as a checkbox list there. A Keto tuple naming something no installed plugin -declares gates nothing, is not offered, and is never revoked by an unrelated save: the picker only -speaks for what it showed. The subject's page lists such a direct grant apart, with a Remove of its own. +declares is not offered as a checkbox, and is never revoked by an unrelated save: the picker only +speaks for what it showed. A plugin may still gate on such a name, so it can be live. The user's or +group's page lists such a direct grant separately, each with its own Remove button. ## A permission's `` is global, not plugin-scoped diff --git a/src/auth/keto-client.test.ts b/src/auth/keto-client.test.ts index 5acfbaf..1474d2f 100644 --- a/src/auth/keto-client.test.ts +++ b/src/auth/keto-client.test.ts @@ -121,3 +121,12 @@ test("transactTuples PATCHes every insert and delete in one request (204 resolve (e: unknown) => e instanceof KetoError && e.status === 400, ); }); + +test("trailing slashes on the read and write URLs are stripped before a path is appended", async () => { + const { calls, fetchImpl } = recorder((url) => (url.startsWith("http://r") ? res(200, {}) : res(204))); + const client = createKetoClient({ fetchImpl, readUrl: "http://r//", writeUrl: "http://w/" }); + await client.listRelations(); + await client.transactTuples({ delete: [], insert: [{ namespace: "Permission", object: "a:read", relation: "granted", subject_id: USER }] }); + assert.match(calls[0]!.url, /^http:\/\/r\/relation-tuples\?/); + assert.equal(calls[1]!.url, "http://w/admin/relation-tuples"); +}); diff --git a/src/http/app-admin.test.ts b/src/http/app-admin.test.ts index 94d1638..02a7d4c 100644 --- a/src/http/app-admin.test.ts +++ b/src/http/app-admin.test.ts @@ -275,6 +275,38 @@ test("admin permission grants: the picker offers the declared catalog, and a sav assert.ok(tuples.some((tp) => tp.namespace === "Permission" && tp.object === "groups:read" && tp.subject_set?.object === "eng")); }); +// Either half of a change reaches the user's live sessions, not just a save that does both. +test("admin permission grants: a grant-only and a revoke-only user save each revoke live tokens", async (t) => { + const [grantee, revokee] = [randomUUID(), randomUUID()]; + const identities: Identity[] = [{ id: grantee, traits: { email: "g@example.com" } }, { id: revokee, traits: { email: "r@example.com" } }]; + const tuples: RelationTuple[] = [{ namespace: "Permission", object: "users:read", relation: "granted", subject_id: `user:${revokee}` }]; + const kratosAdmin = stubAdmin({ getIdentity: async (id) => identities.find((i) => i.id === id) ?? null, listIdentities: async () => ({ identities, nextPageToken: null }) }); + const denylist = createDenylist(); + const { get, post, token } = await adminHarness(t, { denylist, keto: fakeKeto(tuples), kratosAdmin }); + const save = async (id: string, permissions: string) => + post(`/auth-admin/users/${id}/permissions`, `_csrf=${token}&baseline=${baselineOf(await (await get(`/auth-admin/users/${id}`)).text())}${permissions}`); + + await save(grantee, "&permission=users%3Aread"); + assert.equal(denylist.isRevoked(grantee, 0), true); + await save(revokee, ""); + assert.equal(denylist.isRevoked(revokee, 0), true); + assert.equal(tuples.filter((tp) => tp.subject_id === `user:${revokee}`).length, 0); +}); + +// Remove needs Keto, but not the revocation denylist. +test("admin permission grants: Remove answers 503 without Keto, and still deletes where no denylist is wired", async (t) => { + const ada = randomUUID(); + const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }]; + const kratosAdmin = stubAdmin({ getIdentity: async (id) => identities.find((i) => i.id === id) ?? null, listIdentities: async () => ({ identities, nextPageToken: null }) }); + const noKeto = await adminHarness(t, { kratosAdmin }); + assert.equal((await noKeto.post(`/auth-admin/users/${ada}/permissions/remove`, `_csrf=${noKeto.token}&remove=old%3Awrite`)).status, 503); + + const tuples: RelationTuple[] = [{ namespace: "Permission", object: "old:write", relation: "granted", subject_id: `user:${ada}` }]; + const noDenylist = await adminHarness(t, { keto: fakeKeto(tuples), kratosAdmin }); + assert.equal((await noDenylist.post(`/auth-admin/users/${ada}/permissions/remove`, `_csrf=${noDenylist.token}&remove=old%3Awrite`)).status, 303); + assert.equal(tuples.length, 0); +}); + // Two admins open the same page; the second save would silently undo the first. test("admin permission grants: a save from a stale page is refused, and the page re-renders the current state", async (t) => { const ada = randomUUID(); @@ -314,8 +346,6 @@ test("admin permission grants: a save from a stale page is refused, and the page assert.ok(tuples.some((tp) => tp.object === "groups:read" && tp.subject_set?.object === "eng"), "the group keeps its grant"); }); -// The grant changes go to Keto as one transaction, so a Keto failure applies none of them and -// there is nothing to revoke: the host's error page answers, and the stored grants are unchanged. test("admin permission grants: a Keto failure applies none of the changes and revokes no live token", async (t) => { const ada = randomUUID(); const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }]; @@ -357,7 +387,6 @@ test("admin permission grants: a save that changes nothing is not refused for a assert.equal(denylist.isRevoked(ada, 0), false); }); -// grantDiff never revokes a name nobody declares, so the page lists it apart and Remove is the only way out. test("admin permission grants: a grant no plugin declares is listed with a Remove, which deletes it and revokes live tokens", async (t) => { const ada = randomUUID(); const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }]; @@ -381,7 +410,6 @@ test("admin permission grants: a grant no plugin declares is listed with a Remov assert.match(readerPage, /old:write/); assert.doesNotMatch(readerPage, /permissions\/remove/); - // A declared name is the picker's to change, so a crafted Remove for one does nothing. const declared = await post(`/auth-admin/users/${ada}/permissions/remove`, `_csrf=${token}&remove=users%3Aread`); assert.equal(declared.status, 303); assert.equal(tuples.length, 5); @@ -389,6 +417,8 @@ test("admin permission grants: a grant no plugin declares is listed with a Remov const removed = await post(`/auth-admin/users/${ada}/permissions/remove`, `_csrf=${token}&remove=old%3Awrite`); assert.equal(removed.status, 303); + assert.equal(removed.headers.get("location"), `/auth-admin/users/${ada}?info-msg=permission-removed`); + assert.match(await (await get(removed.headers.get("location")!)).text(), /Permission removed\./); assert.equal(tuples.length, 4); assert.ok(!tuples.some((tp) => tp.object === "old:write" && tp.subject_id === `user:${ada}`)); assert.ok(tuples.some((tp) => tp.object === "old:write" && tp.subject_set?.object === "eng"), "the group's grant is untouched"); @@ -398,7 +428,9 @@ test("admin permission grants: a grant no plugin declares is listed with a Remov const groupPage = await (await get("/auth-admin/groups/eng")).text(); assert.match(groupPage, /action="\/auth-admin\/groups\/eng\/permissions\/remove#permissions-h"/); - assert.equal((await post("/auth-admin/groups/eng/permissions/remove", `_csrf=${token}&remove=old%3Awrite`)).status, 303); + const groupRemoved = await post("/auth-admin/groups/eng/permissions/remove", `_csrf=${token}&remove=old%3Awrite`); + assert.equal(groupRemoved.status, 303); + assert.equal(groupRemoved.headers.get("location"), "/auth-admin/groups/eng?info-msg=permission-removed"); assert.ok(!tuples.some((tp) => tp.object === "old:write" && tp.subject_set?.object === "eng")); assert.ok(tuples.some((tp) => tp.object === "old:write" && tp.subject_id === "user:someone-else"), "another subject's grant is untouched"); }); diff --git a/src/plugin-host/plugin.ts b/src/plugin-host/plugin.ts index fa66b3c..15b2ece 100644 --- a/src/plugin-host/plugin.ts +++ b/src/plugin-host/plugin.ts @@ -50,7 +50,7 @@ export function isValidPermissionName(name: string): boolean { // Every permission the installed plugins declare, deduped by name and sorted — the fixed list the // admin screens offer when granting. Permissions are authored in code, never invented in the GUI, so -// this *is* the catalog; a name in Keto that no plugin declares gates nothing and is not offered. +// this *is* the catalog; a name in Keto that no plugin declares is not offered. // First declaration of a name wins its description (shared names are legitimate, findConflicts warns). export function declaredPermissions(plugins: Plugin[]): PermissionDecl[] { const byName = new Map(); -- 2.52.0 From 2b19ed27d29ed8f2a63924fe2b74a3ddc9651c1f Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 8 Nov 2026 22:15:54 +0100 Subject: [PATCH 4/7] =?UTF-8?q?Answer=20prose=20round=201:=201=20?= =?UTF-8?q?=F0=9F=94=B4,=204=20=F0=9F=9F=A1=20(0=20misread);=20correct=20t?= =?UTF-8?q?he=20older=20changelog=20entries,=20keep=20each=20fact=20in=20o?= =?UTF-8?q?ne=20home,=20rewrap=20the=20Keto=20header?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 18 ++++++++---------- .../plugins/auth-admin/admin-grants.test.ts | 2 -- .../views/partials/permission-picker.ejs | 2 -- src/auth/keto-client.ts | 4 ++-- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9001d8e..fbe1b75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,18 +16,17 @@ The host's contract version is now 0.5.0, so discovery refuses a plugin whose `a `transactTuples({ delete, insert })` sends both lists to Keto in one `PATCH /admin/relation-tuples`, so all of them apply or none does. A `KetoClient` you build by hand, such as a test stub, must add the -method. The admin example saves a user's or group's permissions through it: a Keto failure now -changes nothing and answers with the error page, instead of a "stopped part-way" report. Its -catalogs lose `admin.grants.partial`. +method. The admin example saves a user's or group's permissions through it, so a Keto failure leaves +every grant as it was and answers with the error page. ### The admin example lists a user's or group's grants that no installed plugin declares, with a Remove A direct grant of a name no installed plugin declares, such as one left by an uninstalled plugin, used to stay invisible and unchangeable, and reinstalling the plugin silently reactivated it. The user and group pages now list these apart from the checkboxes, each with a Remove button (hidden from -a `:read`-only holder). The example's catalogs gain `admin.grants.remove`, `admin.grants.undeclaredHint` -and `admin.grants.undeclaredLegend`, and its routes gain `POST /auth-admin/users/:id/permissions/remove` -and `POST /auth-admin/groups/:name/permissions/remove`. +a `:read`-only holder). The example's catalogs gain `admin.grants.remove`, `admin.grants.removed`, +`admin.grants.undeclaredHint` and `admin.grants.undeclaredLegend`, and its routes gain +`POST /auth-admin/users/:id/permissions/remove` and `POST /auth-admin/groups/:name/permissions/remove`. ### An operator can hide or rename a plugin's profile-menu entry @@ -174,14 +173,13 @@ page's `baseline` field. ### The admin example confirms a permission save and reports a partial one -A save now lands on the page with "Permissions saved." If the permission service fails midway, the -page says the save stopped part-way instead of showing the generic error page. On a user, the -changes that landed still reach the user's live sessions at once when the revocation denylist is on. +A save now lands on the page with "Permissions saved." On a user, a changed grant reaches the user's +live sessions at once when the revocation denylist is on. ### A read-only holder sees View, not Edit, on the admin example's users list A holder of `users:read` without `users:write` gets a "View" row action with an eye icon. The admin -example's catalogs gain `admin.grants.partial`, `admin.grants.saved`, `admin.grants.stale` and +example's catalogs gain `admin.grants.saved`, `admin.grants.stale` and `admin.users.view`, so a translated catalog mounted under `locales/plugins/auth-admin/` must add them. ### Every page carries a smaller icon sprite diff --git a/examples/plugins/auth-admin/admin-grants.test.ts b/examples/plugins/auth-admin/admin-grants.test.ts index 498ca6e..3b213d9 100644 --- a/examples/plugins/auth-admin/admin-grants.test.ts +++ b/examples/plugins/auth-admin/admin-grants.test.ts @@ -126,8 +126,6 @@ test("applyGrants sends the whole change as one Keto transaction, and a failure await assert.rejects(applyGrants(keto(true), userSubject("u1"), diff), /boom/); }); -// grantDiff never touches a held name nobody declares, so the picker lists them apart and offers a -// Remove for each. A reader is told what is held but offered no button. test("buildPermissionPicker lists direct grants no plugin declares, with a remove action only for a writer", () => { const direct = ["zed:read", "users:read", "old:write"]; const picker = buildPermissionPicker({ action: "/auth-admin/users/u1/permissions", declared, direct }); diff --git a/examples/plugins/auth-admin/views/partials/permission-picker.ejs b/examples/plugins/auth-admin/views/partials/permission-picker.ejs index a66fc91..9c7ae01 100644 --- a/examples/plugins/auth-admin/views/partials/permission-picker.ejs +++ b/examples/plugins/auth-admin/views/partials/permission-picker.ejs @@ -9,8 +9,6 @@ Locals: csrfToken, permissions ({ action, baseline, choices, empty, error, field, hint, inheritedNote, legend, notice, pending, readOnly, submit, undeclared }). - `undeclared` lists direct grants of names no plugin declares, outside the form above because grantDiff - never revokes one. Each has its own Remove form, and a reader gets the names without the button. %>

<%= permissions.legend %>

diff --git a/src/auth/keto-client.ts b/src/auth/keto-client.ts index 28420d8..2757d23 100644 --- a/src/auth/keto-client.ts +++ b/src/auth/keto-client.ts @@ -1,7 +1,7 @@ // Keto client: typed `fetch` wrappers over Ory Keto's relation-tuple APIs — // `check` a permission, `listRelations`/`expand` to inspect them (read API), `writeTuple`/ -// `deleteTuple`/`transactTuples` to grant/revoke them (write API). Built-in `fetch` only, no SDK dep (AGENTS.md); -// `fetchImpl`-injectable like the kratos clients. Read/write split onto the two ports config.ts +// `deleteTuple`/`transactTuples` to grant/revoke them (write API). Built-in `fetch` only, no SDK dep +// (AGENTS.md); `fetchImpl`-injectable like the kratos clients. Read/write split onto the two ports config.ts // targets (ketoReadUrl 4466 / ketoWriteUrl 4467). // A subject set: a relation on another object (e.g. Group:eng#members), resolved -- 2.52.0 From ed48c5a8fd4e751316ccfc6da6c845b6b9e54875 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 8 Nov 2026 22:16:56 +0100 Subject: [PATCH 5/7] File the group delete that can stop part-way as plan item 62 --- todo.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/todo.md b/todo.md index 9acc202..7249652 100644 --- a/todo.md +++ b/todo.md @@ -6,12 +6,13 @@ `Bar = 1` -`Next ID = 62` +`Next ID = 63` ## Items | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| +| 62 | 0.5.1 | | | **Delete a group's grants and its membership set in one Keto transaction, so a failure cannot leave it half-deleted.** | 1 | 2 | 2 | 3 | 2 | code health by 2, 2 by 3 | 16.2 | | 19 | 0.6.0 | | question | **Run the documented Docker commands on macOS and fix whatever misbehaves.** | 2 | 2 | 1 | 3 | 3 | 1 by 3 | 3.3 | | 16 | 0.6.0 | | question | **Decide whether the single Keto `Resource` namespace splits into per-domain namespaces.** | 3 | 2 | 3 | 6 | 6 | 7 by 2 | 0.5 | | 14 | 0.6.0 | | | **Drop a double-clicked duplicate submit, with no client-side JavaScript.** | 4 | 2 | 3 | 7 | 7 | 2 by 7, 1 by 4 | 1.4 | @@ -23,6 +24,14 @@ ## Details +### 62. Delete a group's grants and its membership set in one Keto transaction, so a failure cannot leave it half-deleted. + +`groupsDelete` in `examples/plugins/auth-admin/admin-groups.ts` drops each held grant with its own +`deleteTuple`, then the membership tuple, so a Keto failure part-way leaves a group with some grants +gone and its members still listed. Send all the deletes through `KetoClient.transactTuples`, as +`applyGrants` in `admin-grants.ts` does. The comment on delete order stays true inside one +transaction. + ### 19. Run the documented Docker commands on macOS and fix whatever misbehaves. Needs a Mac. Two suspects, both from `--user "$(id -u):$(id -g)"`: a macOS `id -g` is `20`, which is -- 2.52.0 From 0727d159648ff7fa57d0bb1e36205cd8edcd6ac3 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 8 Nov 2026 22:24:18 +0100 Subject: [PATCH 6/7] Trim two stale comments in the grants and Keto client headers --- examples/plugins/auth-admin/admin-grants.ts | 4 ++-- src/auth/keto-client.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/plugins/auth-admin/admin-grants.ts b/examples/plugins/auth-admin/admin-grants.ts index c90c79c..2111853 100644 --- a/examples/plugins/auth-admin/admin-grants.ts +++ b/examples/plugins/auth-admin/admin-grants.ts @@ -3,8 +3,8 @@ // resolves a group's grant transitively at login. // // The set of permissions that *exist* is `ctx.declaredPermissions` — the host's catalog, built from -// what the installed plugins declare in code. Nothing here invents a name, which is why the old -// Permissions screen is gone: a grant is a property of a user or a group, edited where they are. +// what the installed plugins declare in code. Nothing here invents a name: a grant is a property of a +// user or a group, edited where they are. import { createHash } from "node:crypto"; import type { KetoClient, PermissionDecl, RelationTuple, SubjectSet, Translate } from "@plainpages/plugin-api"; diff --git a/src/auth/keto-client.ts b/src/auth/keto-client.ts index 2757d23..2ffeca7 100644 --- a/src/auth/keto-client.ts +++ b/src/auth/keto-client.ts @@ -32,7 +32,7 @@ export interface RelationList { // Keto's expand tree: a node is a set operation (union/…) or a leaf. The resolved subject // (subject_id xor subject_set) rides on `tuple`, not the node itself — verified against Keto -// v26.2.0. A `subject_set` node carries its members as `children` ("effective access" view). +// v26.2.0. A `subject_set` node carries its members as `children`. export interface ExpandTree { children?: ExpandTree[]; tuple?: RelationTuple; -- 2.52.0 From 23b5f717d7b7f658d9ffe13c186292bae8b09862 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 8 Nov 2026 22:30:33 +0100 Subject: [PATCH 7/7] Record the comprehension check: two scoring runs, overall 6.63 and 6.63, under the floor of 7.0 --- docs/comprehension-history.md | 111 ++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/docs/comprehension-history.md b/docs/comprehension-history.md index 768bd02..70e2c77 100644 --- a/docs/comprehension-history.md +++ b/docs/comprehension-history.md @@ -396,3 +396,114 @@ Inherited architect, decided by: - Locality: `src/http/app.ts:213` `Host`: a 19-field bag wider than any stage needs - Shape: `src/ui/chrome.ts:29` `ShellUser.profileLinks`: no longer collides with `MenuConfig` and the `menu` partial - Navigation: `.gitea/workflows/release.yml:60` error pointer: names `docs/ci-cd.md`, one hop from the failure + +## 2026-11-08T21:24:33Z, PR #228 at ed48c5a + +| Seat | Navigation | Locality | Shape | Self-sufficiency | Overall | +|---|---|---|---|---|---| +| Junior A | 7.5 | 6 | 7 | 7 | 6.5 | +| Mid A | 7 | 6 | 6.5 | 6.5 | 6.5 | +| Maintainability senior | 7.5 | 6.5 | 7 | 6.5 | 7 | +| Inherited architect | 6 | 6 | 6.5 | 7 | 6.5 | +| Mean | 7.00 | 6.13 | 6.75 | 6.75 | 6.63 | + +Junior A, hardest first: + +- Locality, Overall: `src/http/app.ts:67` `createApp`: a closure bag whose order matters; `switchBase` uses `builtinRoutes` declared further down +- Locality, Overall: `src/http/app.ts:274` `prepareRequest`: builds the context, the factory and the memoized chrome together, and every page path must remember to set the CSRF cookie +- Locality, Self-sufficiency: `examples/plugins/auth-admin/admin-groups.ts:285` `lostToSelf`: the self-lockout rule is spread over four call sites and no single place states it +- Shape, Overall: `examples/plugins/auth-admin/admin-grants.ts:99` `buildPermissionPicker`: checked, inherited, read-only and undeclared rows interact and need a truth table +- Self-sufficiency: `src/auth/routes.ts:45` `flowReturnTo`: locale and return-to inputs interact, and no table gives the result per input +- Self-sufficiency: `src/auth/routes.ts:58` `flowFailure`: four Kratos error branches that need Ory knowledge the repo doesn't hold +- Locality: `src/ui/chrome.ts:55` `buildPluginChrome`: the locale rule is put into effect in three places with no single owner +- Self-sufficiency: `src/plugin-host/router.ts:4` header: promises order-independent specificity, but ties fall to discovery order + +Mid A, hardest first: + +- Locality, Overall: `examples/plugins/auth-admin/admin-groups.ts:285` `lostToSelf`: needs Keto's transitive group resolution rebuilt by hand, and over-reports +- Shape, Locality: `examples/plugins/auth-admin/admin-groups.ts:435` `groupsDelete`: deletes grants one at a time, so it can stop part-way +- Shape, Overall: `examples/plugins/auth-admin/admin-grants.ts:99` `buildPermissionPicker`: four overlapping ideas; `checked` means direct or effective +- Self-sufficiency: `src/auth/routes.ts:58` `flowFailure`: Kratos status codes map to different outcomes and need outside knowledge +- Shape, Locality: `src/http/app.ts:183` `handleRequest`: the 19-field `Host` bag and the state threaded between stages +- Shape: `src/test-tooling/stylesheet-floor.ts:122` `positive`: a hand-rolled parser for `@supports` conditions +- Self-sufficiency: `src/plugin-host/settings.ts:35` `ValueOfDecl`: type-level inference with a `const` generic +- Self-sufficiency: `src/auth/jwt-middleware.ts:75` `verifyToken`: a denylisted token is thrown as expired so it gets re-minted +- Self-sufficiency: `examples/plugins/auth-admin/plugin.ts:22` `on`: each write-intent GET states its `"write"` override in the route table and in the handler + +Maintainability senior, hardest first: + +- Locality, Overall: `examples/plugins/auth-admin/admin-groups.ts:285` `lostToSelf`: the self-lockout guards rebuild Keto's transitive membership and no sentence states the invariant +- Shape, Overall: `src/http/app.ts:67` `createApp`: a long closure factory that is safe to read but not safe to change +- Shape, Self-sufficiency: `examples/plugins/auth-admin/admin-grants.ts:99` `buildPermissionPicker`: direct, effective and undeclared grant sets with a staleness token on top +- Self-sufficiency: `src/plugin-host/settings.ts:35` `ValueOfDecl`: type-level inference whose comments say what it does, not how +- Self-sufficiency: `src/auth/routes.ts:45` `flowFailure`: a matrix of Kratos statuses and flow states +- Locality: `src/auth/jwks.ts:68` `cachingJwks`: subtle cache state, and the denylist reuses the `expired` flag +- Shape: `src/test-tooling/stylesheet-floor.ts:122` `positive`: a hand-written parser for `@supports` conditions +- Locality: `src/auth/bootstrap.ts:86` `seedAdmin`: calls Kratos and Keto with raw fetch instead of the client modules +- Self-sufficiency: `examples/plugins/auth-admin/views/partials/group-detail-body.ejs:2` locals header: documents `identity` and `del`, omits `canWrite` and `permissions` + +Inherited architect, would restructure first: + +- Navigation, Overall: `src/auth/routes.ts:1` `src/auth`: 19 modules holding sessions, Ory clients, flows, the OAuth2 provider and one-shot CLIs +- Navigation: `src/auth/bootstrap.ts:1` `bootstrap.ts`: a one-shot process that also provisions plugin storage, placed under auth +- Navigation, Locality: `src/ui/chrome.ts:55` `buildPluginChrome`: builds the chrome for every page, the host's own included +- Navigation: `views/index.ejs:1` `index.ejs`: the built-in /dashboard, and `home.ejs` is "/", and neither filename says so +- Locality: `public/css/styles.css:326` `.btn-primary`: one visual fact spans the partial, the rule, the tokens and the floor policy in `src/test-tooling/` +- Shape, Locality: `examples/plugins/auth-admin/admin-groups.ts:113` `admin-groups.ts`: pure view models, Keto queries, lockout policy and handlers in one 463-line file +- Shape: `src/http/app.ts:213` `Host`: a 19-field bag that every cross-cutting feature edits +- Self-sufficiency: `src/plugin-host/system.ts:1` header: calls the built-in admin screens the reference consumer, but they are a drop-in plugin + +## 2026-11-08T21:30:30Z, PR #228 rerun at 0727d15 + +| Seat | Navigation | Locality | Shape | Self-sufficiency | Overall | +|---|---|---|---|---|---| +| Junior A | 7.5 | 6 | 6.5 | 6.5 | 6.5 | +| Mid A | 7 | 6 | 6.5 | 7 | 6.5 | +| Maintainability senior | 7.5 | 6.5 | 7 | 7 | 7 | +| Inherited architect | 7 | 6.5 | 6 | 7.5 | 6.5 | +| Mean | 7.25 | 6.25 | 6.50 | 7.00 | 6.63 | + +Junior A, hardest first: + +- Locality, Shape, Overall: `src/http/app.ts:274` `prepareRequest`: a 19-field `Host` bag and a twice-memoized chrome threaded through one closure, and two return conventions (send and return null, or return a result) +- Locality, Self-sufficiency: `examples/plugins/auth-admin/admin-groups.ts:285` `lostToSelf`: the self-lockout rule differs from the users side and nothing says whether that is deliberate +- Shape, Self-sufficiency: `examples/plugins/auth-admin/admin-groups.ts:18` `LIST_FETCH_SIZE`: the comment says a Keto page but the call is Kratos `listIdentities` +- Self-sufficiency: `src/auth/routes.ts:58` `flowFailure`: branches on Kratos status and body text that need Ory knowledge +- Shape, Self-sufficiency: `src/auth/jwt-middleware.ts:75` `verifyToken`: a revoked token is reported as expired, with the effect in another file +- Locality: `src/i18n/load.ts:43` `loadI18n`: four catalog sources kept apart with a nested spread on one line +- Self-sufficiency: `src/server.ts:25` boot order: deleting `PLUGIN_DB_SECRET` is order-dependent and its reason sits in the decision log +- Self-sufficiency: `src/plugin-host/system.ts:1` header: calls the built-in admin screens the reference consumer + +Mid A, hardest first: + +- Locality, Shape, Overall: `src/http/app.ts:274` `prepareRequest`: the request order lives only in the sequence of a long closure, and `serveHome` skips `sendResult` +- Shape: `src/test-tooling/stylesheet-floor.ts:122` `positive`: a hand-written, regex-heavy reader of `@supports` conditions +- Locality: `src/auth/jwt-middleware.ts:75` `verifyToken`: revocation reported as expired works only through `resolveUser` in another file +- Self-sufficiency: `src/auth/routes.ts:58` `flowFailure`: Kratos statuses that need outside knowledge +- Locality: `src/auth/jwks.ts:68` `cachingJwks`: four interacting behaviours in one closure +- Shape: `src/i18n/load.ts:43` `loadI18n`: merges four catalog sources, with the chain order described in two files +- Locality: `src/i18n/locale.ts:81` `chosenLocale`: infers an explicit choice from the output of a URL builder +- Shape, Self-sufficiency: `examples/plugins/scheduling/plugin.ts:37` header: says "Roles" and `:` where the project has neither +- Self-sufficiency: `compose.yml:166` comment: says consent comes next, but `src/auth/oauth-consent.ts` exists + +Maintainability senior, hardest first: + +- Shape, Overall: `src/test-tooling/stylesheet-floor.ts:122` `positive`: a regex-driven mini-parser whose header states the rules but not how the helpers carry them +- Self-sufficiency: `src/auth/routes.ts:58` `flowFailure`: five branches that assume Kratos' error vocabulary +- Locality: `src/http/app.ts:274` `prepareRequest`: where the language picker points after a POST is spread over four units in three files +- Shape: `src/i18n/load.ts:43` `loadI18n`: three catalog sets, each with its own baseline, and a nested spread +- Locality: `src/auth/jwt-middleware.ts:84` `verifyToken`: the denylist travels as `expired`, so `resolveSession` can't show it +- Shape: `src/ui/chrome.ts:55` `buildPluginChrome`: labels are translated twice and `markCurrent` mutates the nodes +- Locality, Shape: `examples/plugins/auth-admin/admin-users.ts:348` `usersPermissions`: the stale-page, self-revoke and diff guards interact across two files, and `pickerOrNothing`'s promise is broken on the self-delete paths +- Shape: `src/plugin-host/router.ts:4` header: promises order-independent specificity, but ties fall to discovery order + +Inherited architect, would restructure first: + +- Shape, Navigation, Overall: `src/auth/bootstrap.ts:1` `src/auth`: 19 modules across the session, Kratos flows, OAuth2 and a second process entrypoint +- Navigation, Self-sufficiency: `src/auth/bootstrap.ts:1` header: lists the JWKS, admin and Keto steps but not the plugin storage step, and the file reads the environment outside `config.ts` +- Shape, Navigation: `views/index.ejs:1` `index.ejs`: the dashboard, with `home.ejs` as the landing page, and neither name says so +- Shape: `src/ui/chrome.ts:55` `buildPluginChrome`: builds the chrome for every page, not only plugin pages +- Locality, Navigation: `src/test-tooling/stylesheet-floor.ts:10` header: the custom-property footgun sits in test tooling instead of beside the tokens at `public/css/styles.css:9` +- Locality: `src/auth/bootstrap.ts:170` `provisionPluginStorage`: plugin storage lives in three places, across `storage.ts`, `storage-provisioning.ts` and `bootstrap.ts` +- Locality, Shape: `src/http/app.ts:183` `handleRequest`: every new manifest field or slot adds a stage here and a field to `Host` +- Self-sufficiency: `src/plugin-host/system.ts:1` header: calls the built-in admin screens the reference consumer -- 2.52.0