From 4d2e7b59fde073751ca0d213f6f67f836d6e684a Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 8 Nov 2026 22:04:52 +0100 Subject: [PATCH] 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