diff --git a/CHANGELOG.md b/CHANGELOG.md index fbe1b75..0aa71c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ 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`. +### The core dashboard view is `dashboard`, not `index` + +`views/index.ejs` is now `views/dashboard.ejs`. The `apiVersion` check can't see this: a plugin view +that still does `include("index")` fails when it renders, with the 500 page. Change it to +`include("dashboard")`. + ### `KetoClient.transactTuples` applies inserts and deletes as one transaction `transactTuples({ delete, insert })` sends both lists to Keto in one `PATCH /admin/relation-tuples`, @@ -19,6 +25,15 @@ so all of them apply or none does. A `KetoClient` you build by hand, such as a t 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 deletes a group's grants and members in one Keto transaction + +A Keto failure while deleting a group now leaves the group as it was, with its grants and members, +and answers with the error page. It used to leave some grants gone and the members still listed. + +### The admin example keeps a user's permissions section when it refuses a self-deactivate or self-delete + +The refusal page used to drop the permissions section of the user form. + ### 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, diff --git a/README.md b/README.md index 74625c2..86c2694 100644 --- a/README.md +++ b/README.md @@ -1789,7 +1789,7 @@ src/ The app — strict tsc, no build step. *.test.ts sit beside stylesheet-floor (the browser-floor check) plugin-api/ The `@plainpages/plugin-api` package — the author barrel, linked into /node_modules -views/ Core EJS in the one app shell: home, index, auth, oauth-consent, error, 403/404/500/503, +views/ Core EJS in the one app shell: home, dashboard, auth, oauth-consent, error, 403/404/500/503, and partials/ (shell, nav tree, filter bar, data table, pagination, field, auth card, alert, menu/popover, theme switch, language picker, icon sprite). Domain screens live in plugins. diff --git a/docs/reading-cost.md b/docs/reading-cost.md index 89bfff6..012edf9 100644 --- a/docs/reading-cost.md +++ b/docs/reading-cost.md @@ -103,7 +103,7 @@ Read most (opens and searches): - `src/http/app.test.ts` 1 - `src/ui/chrome.test.ts` 1 - `src/ui/chrome.ts` 1 -- `views/index.ejs` 1 +- `views/dashboard.ejs` 1 - `views/partials/shell.ejs` 1 ## Reading cost, chunk e4d3bb0ac092, finished 2026-10-31T08:02:27Z @@ -485,3 +485,31 @@ Read most (opens and searches): - `src/http/app.test.ts` 2 - `todo.md` 2 - `src/plugin-host/plugin.ts` 1 + +## Reading cost, chunk 8d5c5ad2ab7c, finished 2026-11-08T21:39:34Z + +145906 new input tokens before the first commit. + +Read most (opens and searches): + +- `src/http/app-admin.test.ts` 8 +- `README.md` 6 +- `CHANGELOG.md` 5 +- `compose.yml` 5 +- `docs/decisions.md` 5 +- `examples/plugins/auth-admin/admin-grants.test.ts` 4 +- `examples/plugins/auth-admin/admin-users.ts` 4 +- `src/auth/keto-client.ts` 4 +- `todo.md` 4 +- `examples/plugins/auth-admin/README.md` 3 + +## Reading cost, chunk ef984469151f, finished 2026-11-08T22:01:41Z + +38668 new input tokens before the first commit. + +Read most (opens and searches): + +- `todo.md` 7 +- `AGENTS.md` 1 +- `README.md` 1 +- `docs/decisions.md` 1 diff --git a/examples/plugins/auth-admin/admin-groups.ts b/examples/plugins/auth-admin/admin-groups.ts index d767aa8..94eb0c1 100644 --- a/examples/plugins/auth-admin/admin-groups.ts +++ b/examples/plugins/auth-admin/admin-groups.ts @@ -431,19 +431,20 @@ export const groupsDeleteConfirm = withGroupName(async (deps, name) => { }) }, view: "confirm" }; }, "write"); -// POST /auth-admin/groups/:name/delete — remove every member tuple (the group ceases to exist). +// POST /auth-admin/groups/:name/delete — one Keto transaction removes the group's member tuples +// and the grants it holds, so a Keto failure leaves the group as it was. export const groupsDelete = withGroupName(async (deps, name) => { const { ctx, keto, user } = deps; await guardedForm(ctx); // CSRF-verify the POST const lost = await lostOnDelete(deps, name); if (lost.length > 0) return refuseSelfLockout(deps, name, "delete", lost); - // Drop what the group *holds* before what it *contains*: a Keto set exists only through its - // tuples, so leaving the grants behind would resurrect every permission the moment someone - // re-created a group with the same name. + // A Keto set exists only through its tuples, so leaving the grants behind would resurrect every + // permission the moment someone re-created a group with the same name. const subject = groupSubject(name); const held = await heldPermissions(keto, subject); - for (const permission of held) await keto.deleteTuple(grantTuple(permission, subject)); - await keto.deleteTuple({ namespace: GROUP_NS, object: name, relation: MEMBERS }); + const grants = held.map((permission) => grantTuple(permission, subject)); + const members = await pagedTuples(keto, { namespace: GROUP_NS, object: name, relation: MEMBERS }); + await keto.transactTuples({ delete: [...grants, ...members], insert: [] }); ctx.log.info("admin: group deleted", { actor: user.id, group: name, revoked: held.join(",") }); return { redirect: ADMIN_GROUPS_BASE }; }); diff --git a/examples/plugins/auth-admin/admin-users.ts b/examples/plugins/auth-admin/admin-users.ts index 4a4ac9f..c405f8b 100644 --- a/examples/plugins/auth-admin/admin-users.ts +++ b/examples/plugins/auth-admin/admin-users.ts @@ -391,8 +391,7 @@ export const usersUpdate = withTarget(async (deps, identity, id) => { try { await kratosAdmin.updateIdentity(id, updateIdentityPayload(identity, input)); } catch (err) { - // Re-render with the picker, or the permissions section vanishes off the page on a failed save. - if (err instanceof KratosError) return { ...formResult(ctx, { canWrite: canWriteUsers(ctx), error: ctx.t("admin.users.error.save"), identity, ...(await pickerOrNothing(deps, id)) }), status: 400 }; + if (err instanceof KratosError) return refuseForm(deps, identity, id, ctx.t("admin.users.error.save")); throw err; } return { redirect: `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}` }; @@ -400,9 +399,10 @@ export const usersUpdate = withTarget(async (deps, identity, id) => { // POST /auth-admin/users/:id/state — toggle active/inactive; a deactivation revokes the target's live // tokens now (not after the JWT TTL). Self-protection: an admin can't deactivate their own account. -export const usersState = withTarget(async ({ ctx, kratosAdmin, revoke, user }, identity, id) => { +export const usersState = withTarget(async (deps, identity, id) => { + const { ctx, kratosAdmin, revoke, user } = deps; await guardedForm(ctx); // CSRF-verify the POST (no fields read) - if (id === user.id) return { ...formResult(ctx, { error: ctx.t("admin.users.error.selfDeactivate"), identity }), status: 400 }; + if (id === user.id) return refuseForm(deps, identity, id, ctx.t("admin.users.error.selfDeactivate")); const nextState = identity.state === "inactive" ? "active" : "inactive"; await kratosAdmin.updateIdentity(id, setStatePayload(identity, nextState)); if (nextState === "inactive") revoke?.(id); @@ -412,7 +412,7 @@ export const usersState = withTarget(async ({ ctx, kratosAdmin, revoke, user }, // GET /auth-admin/users/:id/delete — the deliberate confirm step (zero-JS). Refuses self-delete. export const usersDeleteConfirm = withTarget((deps, identity, id) => { - if (id === deps.user.id) return Promise.resolve({ ...formResult(deps.ctx, { error: deps.ctx.t("admin.users.error.selfDelete"), identity }), status: 400 }); + if (id === deps.user.id) return refuseForm(deps, identity, id, deps.ctx.t("admin.users.error.selfDelete")); const back = `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}`; const view = toUserView(identity); const tt = deps.ctx.t; @@ -424,9 +424,10 @@ export const usersDeleteConfirm = withTarget((deps, identity, id) => { }, "write"); // POST /auth-admin/users/:id/delete — perform it; revoke the gone account's live tokens. Refuses self-delete. -export const usersDelete = withTarget(async ({ ctx, kratosAdmin, revoke, user }, identity, id) => { +export const usersDelete = withTarget(async (deps, identity, id) => { + const { ctx, kratosAdmin, revoke, user } = deps; await guardedForm(ctx); // CSRF-verify the POST - if (id === user.id) return { ...formResult(ctx, { error: ctx.t("admin.users.error.selfDelete"), identity }), status: 400 }; + if (id === user.id) return refuseForm(deps, identity, id, ctx.t("admin.users.error.selfDelete")); await kratosAdmin.deleteIdentity(id); revoke?.(id); ctx.log.info("admin: user deleted", { actor: user.id, target: id }); @@ -441,7 +442,13 @@ export const usersRecovery = withTarget(async (deps, identity, id) => { return formResult(ctx, { canWrite: canWriteUsers(ctx), identity, recovery, ...(await pickerOrNothing(deps, id)) }); }); -// The picker as a spreadable fragment, so a re-render never silently drops the section. +// The 400 re-render of the user's form with `error` above it. `usersPermissions` refuses through its +// own `refuse`, which puts the error in the permissions section instead. +async function refuseForm(deps: UsersDeps, identity: Identity, id: string, error: string): Promise { + return { ...formResult(deps.ctx, { canWrite: canWriteUsers(deps.ctx), error, identity, ...(await pickerOrNothing(deps, id)) }), status: 400 }; +} + +// The picker as a spreadable fragment, so every form re-render keeps the section. async function pickerOrNothing(deps: UsersDeps, id: string): Promise<{ permissions?: PermissionPicker }> { const permissions = await userPermissionPicker(deps, id); return permissions ? { permissions } : {}; diff --git a/release-tooling/contract-version.test.ts b/release-tooling/contract-version.test.ts index 53e6050..ad87823 100644 --- a/release-tooling/contract-version.test.ts +++ b/release-tooling/contract-version.test.ts @@ -26,7 +26,7 @@ test("every author-facing apiVersion sample matches the shipped contract", () => "examples/plugins/auth-admin/plugin.ts", "examples/plugins/scheduling/plugin.ts", "release-tooling/dockerhub-overview.md.tmpl", - "views/index.ejs", + "views/dashboard.ejs", ]) { const found = [...readFileSync(file, "utf8").matchAll(/apiVersion: "(\d+\.\d+\.\d+)"/g)].map((m) => m[1]); assert.ok(found.length > 0, `${file} should carry at least one apiVersion sample`); diff --git a/src/http/app-admin.test.ts b/src/http/app-admin.test.ts index 02a7d4c..f866aaf 100644 --- a/src/http/app-admin.test.ts +++ b/src/http/app-admin.test.ts @@ -11,6 +11,7 @@ import { staticJwks } from "../auth/jwks.ts"; import type { OAuth2Client } from "../auth/hydra-admin.ts"; import type { RelationTuple } from "../auth/keto-client.ts"; import type { Identity } from "../auth/kratos-admin.ts"; +import { KratosError } from "../auth/kratos-public.ts"; import { SESSION_COOKIE } from "../auth/login.ts"; import type { Plugin } from "../plugin-host/plugin.ts"; import adminManifest from "../../examples/plugins/auth-admin/plugin.ts"; @@ -213,6 +214,11 @@ test("admin Groups screen: gate, list, create, detail/membership, delete (CSRF-g // Delete the group: a confirm step (GET) then the POST removes every member tuple, back to the list. assert.match(await (await get("/auth-admin/groups/eng/delete")).text(), /Cancel/); + const bystanders: RelationTuple[] = [ + { namespace: "Group", object: "ops", relation: "members", subject_id: `user:${ada}` }, + { namespace: "Permission", object: "users:read", relation: "granted", subject_id: `user:${ada}` }, + ]; + tuples.push(...bystanders); const del = await post("/auth-admin/groups/eng/delete", `_csrf=${token}`); assert.equal(del.status, 303); assert.equal(del.headers.get("location"), "/auth-admin/groups"); @@ -220,6 +226,7 @@ test("admin Groups screen: gate, list, create, detail/membership, delete (CSRF-g // …and the permissions it held go with it. A Keto set exists only through its tuples, so an // orphaned grant would resurrect the moment someone re-created a group with the same name. assert.ok(!tuples.some((tp) => tp.namespace === "Permission" && tp.subject_set?.object === "eng")); + assert.ok(bystanders.every((tp) => tuples.includes(tp)), "only the deleted group's own tuples go"); // An invalid group name in the path → 404; malformed %-encoding doesn't 500. assert.equal((await get("/auth-admin/groups/Bad%20Name")).status, 404); @@ -368,6 +375,58 @@ test("admin permission grants: a Keto failure applies none of the changes and re assert.equal(tuples.filter((tp) => tp.subject_set?.object === "eng").length, 0); }); +test("admin users: a refused self-deactivate or self-delete keeps the permissions section", async (t) => { + const identities: Identity[] = [{ id: "admin1", traits: { email: "admin@x" } }]; + 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: fakeKeto(), kratosAdmin }); + assert.notEqual(baselineOf(await (await get("/auth-admin/users/admin1")).text()), "", "the plain form carries the section"); + + const refusals: [Response, RegExp][] = [ + [await post("/auth-admin/users/admin1/state", `_csrf=${token}`), /deactivate your own account/], + [await get("/auth-admin/users/admin1/delete"), /delete your own account/], + [await post("/auth-admin/users/admin1/delete", `_csrf=${token}`), /delete your own account/], + ]; + for (const [res, reason] of refusals) { + assert.equal(res.status, 400); + const html = await res.text(); + assert.match(html, reason); + assert.notEqual(baselineOf(html), ""); + } +}); + +test("admin users: a Kratos rejection of a save re-renders the form with its permissions, any other failure is a 500", async (t) => { + const identities: Identity[] = [{ id: "ada", traits: { email: "ada@example.com" } }]; + let failure: Error = new KratosError("rejected", 400, ""); + const kratosAdmin = stubAdmin({ + getIdentity: async (id) => identities.find((i) => i.id === id) ?? null, + listIdentities: async () => ({ identities, nextPageToken: null }), + updateIdentity: async () => { throw failure; }, + }); + const { post, token } = await adminHarness(t, { keto: fakeKeto(), kratosAdmin }); + + const rejected = await post("/auth-admin/users/ada", `_csrf=${token}&first=Ada`); + assert.equal(rejected.status, 400); + assert.notEqual(baselineOf(await rejected.text()), ""); + failure = new Error("kratos unreachable"); + assert.equal((await post("/auth-admin/users/ada", `_csrf=${token}&first=Ada`)).status, 500); +}); + +test("admin group delete: a Keto failure leaves the group's grants and members in place", async (t) => { + const ada = randomUUID(); + const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }]; + const tuples: RelationTuple[] = [ + { namespace: "Group", object: "eng", relation: "members", subject_id: `user:${ada}` }, + { namespace: "Permission", object: "users:read", relation: "granted", subject_set: { namespace: "Group", object: "eng", relation: "members" } }, + ]; + 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 { post, token } = await adminHarness(t, { keto, kratosAdmin }); + + const res = await post("/auth-admin/groups/eng/delete", `_csrf=${token}`); + assert.equal(res.status, 500); + assert.equal(tuples.length, 2); +}); + // A double-clicked Save posts twice from one page; the second post finds the first one's result. // A save that changes nothing has nothing to refuse and nothing to revoke. test("admin permission grants: a save that changes nothing is not refused for a stale page and leaves live tokens alone", async (t) => { diff --git a/src/http/app.test.ts b/src/http/app.test.ts index cff1dbe..c9ef9bb 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -406,7 +406,7 @@ test("/error renders a themed sign-in error page (Kratos' flow error sink), not test("renders the 500 HTML page when a handler throws", async () => { const dir = mkdtempSync(join(tmpdir(), "pp-views-")); cpSync(viewsDir, dir, { recursive: true }); // the real views: 500.ejs includes the language picker - writeFileSync(join(dir, "index.ejs"), "<% throw new Error('boom'); %>"); // …but the dashboard view throws + writeFileSync(join(dir, "dashboard.ejs"), "<% throw new Error('boom'); %>"); // …but the dashboard view throws const app = createApp({ jwks: staticJwks([ecJwk]), viewsDir: dir }); try { await new Promise((resolve) => app.listen(0, resolve)); diff --git a/src/http/app.ts b/src/http/app.ts index 80bb6c5..7987776 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -165,7 +165,7 @@ export function createApp(options: AppOptions = {}): Server { await sendResult(ctx.res, result, pluginViewsFor(pluginCtx, dashboardPlugin.id), pluginCtx.localeHref); return null; } - return { data: { chrome: ctx.chrome }, view: "index" }; + return { data: { chrome: ctx.chrome }, view: "dashboard" }; }; // The internal route table, matched after plugin routes: the auth/OAuth2 group (src/auth/ diff --git a/todo.md b/todo.md index 3a6ecfb..cb69dce 100644 --- a/todo.md +++ b/todo.md @@ -6,19 +6,17 @@ `Bar = 1` -`Next ID = 71` +`Next ID = 72` ## Items | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| -| 62 | 0.5.0 | | | **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 | | 63 | 0.5.0 | | | **Correct the stale and false comments and docs the comprehension seats flagged, so each states what the code does today.** | 2 | 2 | 1 | 3 | 3 | code health by 3, 9 by 2 | 15.4 | | 68 | 0.5.0 | | | **Make `serveHome` and `serveDashboard` return their result to the request pipeline, as every other stage does, instead of sending it themselves.** | 2 | 1 | 3 | 4 | 3 | code health by 3 | 13.5 | -| 69 | 0.5.0 | | | **Rename `views/index.ejs` to `views/dashboard.ejs`, so the core view's file names the page it renders.** | 1 | 2 | 2 | 3 | 2 | code health by 2 | 12.0 | -| 64 | 0.5.0 | | | **Keep the permissions section on the user form when a self-deactivate or self-delete is refused.** | 1 | 2 | 1 | 2 | 2 | 2 by 3, code health by 1 | 10.2 | | 66 | 0.5.0 | | | **Make `src/auth/bootstrap.ts` read its settings through `src/config.ts` and call Kratos and Keto through their client modules.** | 2 | 1 | 3 | 4 | 4 | code health by 3 | 9.0 | | 70 | 0.5.0 | | | **Give the duplicated paging loop, Link-header parser and `FieldConfig` type one home each, and rename `buildPluginChrome` to `buildPageChrome`.** | 2 | 1 | 1 | 2 | 3 | code health by 2 | 9.0 | +| 71 | 0.5.0 | | | **Delete, with a group, every membership tuple in another group that names it, so re-creating the name restores no nested membership.** | 1 | 2 | 2 | 3 | 2 | 2 by 3 | 4.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 | | 65 | 0.6.0 | | | **Export `KetoError` from `@plainpages/plugin-api`, so a plugin can tell a Keto failure apart with `instanceof`.** | 1 | 2 | 1 | 2 | 1 | 7 by 3, 1 by 1 | 6.7 | @@ -32,14 +30,6 @@ ## 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. - ### 63. Correct the stale and false comments and docs the comprehension seats flagged, so each states what the code does today. Prose only; no behaviour changes. Make each listed place easier to read, since the seats named @@ -80,24 +70,6 @@ result with whatever names its renderer and let the one send stage render it. Ma `prepareRequest` and `handleRequest` easier to read, places the seats found hard. The 19-field `Host` bag is left alone: no checkable target for it came out of the scoring run. -### 69. Rename `views/index.ejs` to `views/dashboard.ejs`, so the core view's file names the page it renders. - -`index.ejs` renders `/dashboard` (`src/http/app.ts` ~L168); `home.ejs` renders `/` and matches the -manifest's `home` slot, so it keeps its name. Core `views/` is an `include()` root for plugin views -(`src/plugin-host/view-resolver.ts`), so a plugin may include `index` today: the rename changes the -contract surface (AGENTS.md → Rules) and ships in a minor. Update `docs/reading-cost.md` and the -README file map. Make `views/index.ejs` easier to find, a place the seats found hard. - -### 64. Keep the permissions section on the user form when a self-deactivate or self-delete is refused. - -In `examples/plugins/auth-admin/admin-users.ts`, the self-refusals in `usersState` (~L405), -`usersDeleteConfirm` (~L415) and `usersDelete` (~L429) call `formResult` without -`...(await pickerOrNothing(deps, id))` or `canWrite`, so the re-rendered form loses its permissions -section; `usersUpdate` and `usersRecovery` keep it, and `pickerOrNothing`'s comment promises it. -The groups side re-renders through `groupDetailPage`, which keeps it. Make `usersPermissions` and -the `pickerOrNothing` contract easier to read, a place the seats found hard. Not exempt: a -comprehension finding, not a 🔴, and no maintain goal is broken. - ### 66. Make `src/auth/bootstrap.ts` read its settings through `src/config.ts` and call Kratos and Keto through their client modules. `seedAdminAndPermissions` (~L221-233) reads `ADMIN_EMAIL`, `ADMIN_PASSWORD`, `KETO_WRITE_URL`, @@ -121,6 +93,15 @@ easier to read, a place the seats found hard. included; rename it, its test and the README ~L1027 mention. Make `buildPluginChrome` easier to read, a place the seats found hard. +### 71. Delete, with a group, every membership tuple in another group that names it, so re-creating the name restores no nested membership. + +Groups nest: a parent holds `Group:#members` with `subject_set` `Group:#members`. +`groupsDelete` in `examples/plugins/auth-admin/admin-groups.ts` drops the child's own member tuples +and held grants, but not those parent tuples, so a group re-created with the deleted name is back +in every parent, the orphan class its comment already guards against for grants. List them with +`pagedTuples` filtered on `subject_set` (`namespace: GROUP_NS`, `object: name`, `relation: MEMBERS`) +and add them to the same `transactTuples` delete; extend the comment to say so. + ### 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 diff --git a/views/index.ejs b/views/dashboard.ejs similarity index 100% rename from views/index.ejs rename to views/dashboard.ejs