diff --git a/CHANGELOG.md b/CHANGELOG.md index e22368c..d22b83a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,31 +6,33 @@ at boot. Entries start at 0.3.0. ## Unreleased -### The admin example refuses a permission save made from an out-of-date page - -Two administrators editing one user's or group's permissions no longer overwrite each other. The -form carries a token for the grants it showed, and a save made after they changed is refused with -"These permissions changed while you had the page open", re-rendering the current state. A POST with -no token is refused the same way, so a script posting to `/admin/users/:id/permissions` or -`/admin/groups/:name/permissions` must first read the token from the 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 and shows what is stored, instead of the generic error page; the -changes applied before the failure still sign the user's live sessions out. - -### A read-only holder sees View, not Edit, on the users list - -A holder of `users:read` without `users:write` gets a "View" row action with an eye icon. The new -`common.view` catalog key is in `en-US` and `sv-SE`; a mounted core catalog must add it. - ### A language chosen on a sign-in link no longer drops an absolute return target `/login?return_to=&locale=sv-SE` passed the visitor to the dashboard after sign-in and dropped the OAuth2 login challenge in `return_to`. The target now reaches Kratos, and the chosen language is lost across that round-trip instead. +### The admin example refuses a permission save made from an out-of-date page + +Two administrators editing one user's or group's permissions no longer overwrite each other. The +form carries a token for the grants it showed. A save that would change something, made after those +grants changed, is refused with "These permissions changed while you had the page open", and the page +shows the current grants. A POST with no token is refused the same way, so a script posting to +`/admin/users/:id/permissions` or `/admin/groups/:name/permissions` must first read the token from the +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 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 +`admin.users.view`, so a translated catalog mounted under `locales/plugins/admin/` must add them. + ### Every page carries a smaller icon sprite `i-chart`, `i-copy` and `i-download` are gone from the inlined icon sprite, so a plugin that diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index 7e6996d..0f822f7 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -1,5 +1,5 @@ import type { Browser, Page } from "@playwright/test"; -import { expect, test, watchedPage } from "./console-guard.ts"; +import { allowConsole, expect, test, watchedPage } from "./console-guard.ts"; import { randomUUID } from "node:crypto"; // Full browser E2E: the real Playwright UI against the live stack via the same-origin @@ -136,7 +136,7 @@ test.describe.serial("authenticated admin journey", () => { await expect(page.locator("tr", { hasText: email })).toHaveCount(0); }); - test("groups CRUD: create a group (writes go to Keto), see it listed, then grant it a permission", async () => { + test("groups CRUD: create a group (writes go to Keto), see it listed, then grant it a permission", async ({ browser }) => { // A Keto set exists only while it has ≥1 member, so create needs a first member (the form // enforces it); pick the first option (a user) from the required picker. const group = `e2e-grp-${suffix}`; @@ -150,7 +150,9 @@ test.describe.serial("authenticated admin journey", () => { // Permissions are declared in plugin code, so the group's detail page offers them as a fixed // checkbox list rather than a create form — there is no Permissions screen to visit. await page.goto(`/admin/groups/${group}`); - const staleTab = await watchedPage(page.context()); // opened now, so it holds this page's grants + allowConsole(/status of 409 .*\/admin\/groups\/[^/]+\/permissions/); // the stale save below, refused on purpose + // A second browser signed in as the same admin, opened before the save below, so its form carries the old grants. + const staleTab = await watchedPage(await browser.newContext({ storageState: await page.context().storageState() })); await staleTab.goto(`/admin/groups/${group}`); const scheduling = page.locator('input[name="permission"][value="scheduling:read"]'); await expect(scheduling).toHaveCount(1); // declared by the reference plugin, so it's on offer @@ -168,7 +170,7 @@ test.describe.serial("authenticated admin journey", () => { await expect(staleTab.getByText("changed while you had the page open")).toBeVisible(); 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.close(); + await staleTab.context().close(); }); 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/admin/admin-grants.test.ts b/examples/plugins/admin/admin-grants.test.ts index 32899a1..27d9500 100644 --- a/examples/plugins/admin/admin-grants.test.ts +++ b/examples/plugins/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, userSubject } from "./admin-grants.ts"; +import { applyGrants, baselineToken, buildPermissionPicker, grantDiff, grantTuple, groupSubject, isStale, userSubject } from "./admin-grants.ts"; const declared: PermissionDecl[] = [ { description: "View users", name: "users:read" }, @@ -83,7 +83,6 @@ test("buildPermissionPicker says so when no plugin declares a permission, rather assert.ok(picker.empty); }); -// The concurrency token of the permission form: equal exactly when the direct grants it offered are. test("baselineToken depends on the offered grants held, not on their order or on undeclared ones", () => { const token = baselineToken(declared, ["users:read", "users:write"]); assert.match(token, /^[0-9a-f]+$/); @@ -95,6 +94,18 @@ test("baselineToken depends on the offered grants held, not on their order or on assert.equal(buildPermissionPicker({ action: "/x", declared, direct: ["users:read", "users:write"] }).baseline, token); }); +test("isStale refuses a missing or old token, but only for a save that would change something", () => { + const held = ["users:read"]; + const change = grantDiff(declared, held, ["users:read", "users:write"]); + const nothing = grantDiff(declared, held, ["users:read"]); + const current = new URLSearchParams({ baseline: baselineToken(declared, held) }); + const old = new URLSearchParams({ baseline: baselineToken(declared, []) }); + assert.equal(isStale(current, declared, held, change), false); + assert.equal(isStale(old, declared, held, change), true); + assert.equal(isStale(new URLSearchParams(), declared, held, change), true); // no token at all + 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 => ({ diff --git a/examples/plugins/admin/admin-grants.ts b/examples/plugins/admin/admin-grants.ts index 232decc..a6b7513 100644 --- a/examples/plugins/admin/admin-grants.ts +++ b/examples/plugins/admin/admin-grants.ts @@ -15,6 +15,9 @@ export const PERMISSIONS_FIELD = "permission"; // the checkbox name the two form export const BASELINE_FIELD = "baseline"; // the hidden field carrying baselineToken export const SAVED_NOTICE = "permissions-saved"; // the `info-msg` a successful save redirects with +// 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; + export type GrantSubject = { subject_id: string } | { subject_set: SubjectSet }; export const userSubject = (id: string): GrantSubject => ({ subject_id: `user:${id}` }); @@ -123,23 +126,25 @@ export function grantDiff(declared: readonly PermissionDecl[], held: string[], w }; } -// Proves which direct grants a form was rendered from, so a save made after someone else changed -// them is refused rather than silently undoing that change. Covers only the declared names the form -// offered, like grantDiff, and is a digest so an empty set is still a token and a missing field is not. +// 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. export function baselineToken(declared: readonly PermissionDecl[], held: string[]): string { const offered = new Set(declared.map((d) => d.name)); return createHash("sha256").update(held.filter((name) => offered.has(name)).sort().join("\n")).digest("hex").slice(0, 16); } -// True when the submitted token is missing or no longer matches Keto. -export const isStale = (form: { get(name: string): string | null }, declared: readonly PermissionDecl[], held: string[]): boolean => - form.get(BASELINE_FIELD) !== baselineToken(declared, held); +// True when the form's token is missing or no longer matches Keto *and* the save would change +// something. A save that changes nothing, such as a double-clicked Save whose first POST landed, is +// harmless, so it is not refused. +export const isStale = (form: { get(name: string): string | null }, declared: readonly PermissionDecl[], held: string[], diff: GrantChange): boolean => + (diff.grant.length > 0 || diff.revoke.length > 0) && form.get(BASELINE_FIELD) !== baselineToken(declared, held); export interface GrantChange { grant: string[]; revoke: string[] } -// Keto has no transaction, 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. +// 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 { diff --git a/examples/plugins/admin/admin-groups.ts b/examples/plugins/admin/admin-groups.ts index 230e8b3..81f1a3a 100644 --- a/examples/plugins/admin/admin-groups.ts +++ b/examples/plugins/admin/admin-groups.ts @@ -7,8 +7,8 @@ // each returning a RouteResult. import { can, type KetoClient, type KratosAdmin, paginate, parseListQuery, type RelationQuery, type RelationTuple, type RequestContext, type RouteHandler, type RouteResult, type SubjectSet, type Translate, type User } from "@plainpages/plugin-api"; -import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, grantTuple, groupSubject, heldPermissions, isStale, type PermissionPicker, PERMISSIONS_FIELD, SAVED_NOTICE } from "./admin-grants.ts"; -import { ADMIN_EN, type AdminAction, ADMIN_GROUPS_BASE, buildConfirmModel, guardedForm, notFound, permissionName, requirePermission, unavailable } from "./admin-shared.ts"; +import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, grantTuple, groupSubject, heldPermissions, isSavedNotice, isStale, type PermissionPicker, PERMISSIONS_FIELD, SAVED_NOTICE } from "./admin-grants.ts"; +import { ADMIN_EN, type AdminAction, ADMIN_GROUPS_BASE, buildConfirmModel, guardedForm, notFound, partialSave, permissionName, requirePermission, unavailable } from "./admin-shared.ts"; import type { FieldConfig } from "./admin-users.ts"; const GROUP_NS = "Group"; @@ -369,9 +369,9 @@ async function groupDetailPage({ ctx, keto, kratosAdmin }: GroupsDeps, name: str return { data: { chrome: ctx.chrome, model: buildGroupDetailModel({ canWrite: !permissions.readOnly, candidates: options, csrfToken: ctx.chrome.csrfToken, group: { name }, members, permissions, t: ctx.t }) }, view: "group-detail" }; } -// GET /admin/groups/:name — the detail + membership page; `info-msg` is read as on the user page. +// GET /admin/groups/:name — the detail + membership page. export const groupsDetail = withGroupName((deps, name) => - groupDetailPage(deps, name, deps.ctx.url.searchParams.get("info-msg") === SAVED_NOTICE ? { notice: deps.ctx.t("admin.grants.saved") } : {})); + groupDetailPage(deps, name, isSavedNotice(deps.ctx.url) ? { notice: deps.ctx.t("admin.grants.saved") } : {})); // POST /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 — @@ -381,14 +381,15 @@ export const groupsPermissions = withGroupName(async (deps, name) => { const form = (await guardedForm(ctx))!; const subject = groupSubject(name); const held = await heldPermissions(keto, subject); - if (isStale(form, ctx.declaredPermissions, held)) return { ...(await groupDetailPage(deps, name, { error: ctx.t("admin.grants.stale") })), status: 409 }; - const { applied, error } = await applyGrants(keto, subject, grantDiff(ctx.declaredPermissions, held, form.getAll(PERMISSIONS_FIELD))); + const diff = grantDiff(ctx.declaredPermissions, held, form.getAll(PERMISSIONS_FIELD)); + if (isStale(form, ctx.declaredPermissions, held, diff)) return { ...(await groupDetailPage(deps, name, { error: ctx.t("admin.grants.stale") })), status: 409 }; + 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 { ...(await groupDetailPage(deps, name, { error: ctx.t("admin.grants.partial") })), status: 502 }; + return partialSave(ctx, async () => ({ ...(await groupDetailPage(deps, name, { error: ctx.t("admin.grants.partial") })), status: 502 })); } return { redirect: `${detailHref(name)}?info-msg=${SAVED_NOTICE}` }; }); diff --git a/examples/plugins/admin/admin-shared.ts b/examples/plugins/admin/admin-shared.ts index 8e8b424..7aaaa74 100644 --- a/examples/plugins/admin/admin-shared.ts +++ b/examples/plugins/admin/admin-shared.ts @@ -83,6 +83,16 @@ 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/admin/admin-users.ts b/examples/plugins/admin/admin-users.ts index e60c2d6..1ba1778 100644 --- a/examples/plugins/admin/admin-users.ts +++ b/examples/plugins/admin/admin-users.ts @@ -4,8 +4,8 @@ // a shared `withUser` gate. import { 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, isStale, type PermissionPicker, PERMISSIONS_FIELD, 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"; +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"; const SCHEMA_ID = "default"; // matches kratos.yml identity.default_schema_id const DEFAULT_PAGE_SIZE = 25; @@ -153,7 +153,7 @@ function listTable(rows: UserView[], state: ListState, sort: { dir: "asc" | "des return { href: listHref(state, { page: 1, sort: next }), label: t(c.label), sort: dir, sortable: true }; }), rows: rows.map((u) => ({ - actions: [{ href: `${ADMIN_USERS_BASE}/${encodeURIComponent(u.id)}`, icon: canWrite ? "i-edit" : "i-eye", label: t(canWrite ? "common.edit" : "common.view") }], + actions: [{ href: `${ADMIN_USERS_BASE}/${encodeURIComponent(u.id)}`, icon: canWrite ? "i-edit" : "i-eye", label: t(canWrite ? "common.edit" : "admin.users.view") }], cells: [ { user: { initials: u.initials, name: u.name } }, u.email, @@ -329,9 +329,7 @@ export const usersNewForm = withUser(({ ctx }) => Promise.resolve(formResult(ctx // GET /admin/users/:id — the edit form, prefilled. export const usersEditForm = withTarget(async (deps, identity, id) => { - // `info-msg` is honored only as the exact value a save redirects with, so a crafted link can't put words on the page. - const saved = deps.ctx.url.searchParams.get("info-msg") === SAVED_NOTICE; - const permissions = await userPermissionPicker(deps, id, undefined, saved ? deps.ctx.t("admin.grants.saved") : undefined); + const permissions = await userPermissionPicker(deps, id, undefined, isSavedNotice(deps.ctx.url) ? deps.ctx.t("admin.grants.saved") : undefined); return formResult(deps.ctx, { canWrite: canWriteUsers(deps.ctx), identity, ...(permissions ? { permissions } : {}) }); }); @@ -374,8 +372,8 @@ export const usersPermissions = withTarget(async (deps, identity, id) => { const permissions = await userPermissionPicker(deps, id, error); return { ...formResult(ctx, { canWrite: canWriteUsers(ctx), identity, ...(permissions ? { permissions } : {}) }), status }; }; - if (isStale(form, ctx.declaredPermissions, held)) return refuse(ctx.t("admin.grants.stale"), 409); const diff = grantDiff(ctx.declaredPermissions, held, form.getAll(PERMISSIONS_FIELD)); + if (isStale(form, ctx.declaredPermissions, held, diff)) return refuse(ctx.t("admin.grants.stale"), 409); // Self-lockout guard, matching the self-deactivate/self-delete ones: revoking your own grants can // remove the last `users:write` on the deployment, and the instant-revoke hook lands it on the very // next request — leaving a `curl` against Keto as the only way back in. @@ -390,7 +388,7 @@ export const usersPermissions = withTarget(async (deps, identity, id) => { } if (error) { ctx.log.error("admin: user permissions only partly saved", { actor: user.id, error: String(error), target: id }); - return refuse(ctx.t("admin.grants.partial"), 502); + return partialSave(ctx, () => refuse(ctx.t("admin.grants.partial"), 502)); } return { redirect: `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}?info-msg=${SAVED_NOTICE}` }; }); diff --git a/examples/plugins/admin/i18n/en-US.ts b/examples/plugins/admin/i18n/en-US.ts index 52473cb..5c3d1ed 100644 --- a/examples/plugins/admin/i18n/en-US.ts +++ b/examples/plugins/admin/i18n/en-US.ts @@ -53,7 +53,7 @@ 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. The list shows what is stored now. Save again to finish.", + "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.save": "Save permissions", "admin.grants.saved": "Permissions saved.", @@ -152,6 +152,7 @@ const messages = { "admin.users.status.inactive": "Inactive", "admin.users.status.label": "Status", "admin.users.title": "Users", + "admin.users.view": "View", }; export type AdminMessages = typeof messages; diff --git a/examples/plugins/admin/i18n/sv-SE.ts b/examples/plugins/admin/i18n/sv-SE.ts index a170713..78bdd80 100644 --- a/examples/plugins/admin/i18n/sv-SE.ts +++ b/examples/plugins/admin/i18n/sv-SE.ts @@ -53,7 +53,7 @@ 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 fallerade, så bara en del av ändringarna genomfördes. Listan visar vad som är sparat nu. Spara igen för att slutföra.", + "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.save": "Spara behörigheter", "admin.grants.saved": "Behörigheterna är sparade.", @@ -152,6 +152,7 @@ const messages: AdminMessages = { "admin.users.status.inactive": "Inaktiv", "admin.users.status.label": "Status", "admin.users.title": "Användare", + "admin.users.view": "Visa", }; export default messages; diff --git a/examples/plugins/admin/views/partials/permission-picker.ejs b/examples/plugins/admin/views/partials/permission-picker.ejs index 9414aaf..8cb25df 100644 --- a/examples/plugins/admin/views/partials/permission-picker.ejs +++ b/examples/plugins/admin/views/partials/permission-picker.ejs @@ -7,9 +7,6 @@ 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. - The hidden `baseline` is a token for the direct grants this page showed: a save made after they changed - is refused (see baselineToken in admin-grants.ts). - Locals: csrfToken, permissions ({ action, baseline, choices, empty, error, field, hint, inheritedNote, legend, notice, pending, readOnly, submit }). %>
@@ -32,7 +29,8 @@ <% }) -%> <% } else { -%> -
+ <%# The fragment brings the card, and the notice in it, into view on a long page. %> +
diff --git a/src/http/app.test.ts b/src/http/app.test.ts index 13e5af8..72990c2 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -851,6 +851,9 @@ test("login return_to: a chosen language alone still routes through /auth/comple t.after(() => app.close()); const url = `http://localhost:${(app.address() as AddressInfo).port}`; + lastReturnTo = "unset"; + await fetch(url + "/login", { redirect: "manual" }); + assert.equal(lastReturnTo, undefined, "no target and no language: Kratos keeps its default"); await fetch(url + "/login?locale=sv-SE", { redirect: "manual" }); assert.match(lastReturnTo ?? "", /^http:\/\/[^/]+\/auth\/complete\?locale=sv-SE$/); await fetch(url + "/login?locale=sv-SE&return_to=" + encodeURIComponent("/admin/users"), { redirect: "manual" }); @@ -1388,8 +1391,7 @@ 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")); }); -// Two admins open the same page; the second save would silently undo the first. The form carries a -// token for the grants it was rendered from, and a save made from any other state is refused. +// 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(); const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }]; @@ -1428,8 +1430,7 @@ 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"); }); -// applyGrants is not a transaction: a Keto error midway leaves some changes applied. The save says -// so, instead of the generic error page, and still revokes the tokens of what did change. +// 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) => { const ada = randomUUID(); const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }]; @@ -1456,6 +1457,75 @@ test("admin permission grants: a Keto failure midway reports a partial save and assert.equal(denylist.isRevoked(ada, 0), true); // the half that was applied still revokes live tokens }); +// 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) => { + const ada = randomUUID(); + const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }]; + const tuples: RelationTuple[] = [{ namespace: "Permission", object: "users:read", relation: "granted", subject_id: `user:${ada}` }]; + 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 stale = baselineOf(await (await get(`/admin/users/${ada}`)).text()); + tuples.push({ namespace: "Permission", object: "users:write", relation: "granted", subject_id: `user:${ada}` }); + const before = JSON.stringify(tuples); + + const repeated = await post(`/admin/users/${ada}/permissions`, `_csrf=${token}&baseline=${stale}&permission=users%3Aread&permission=users%3Awrite`); + assert.equal(repeated.status, 303); + assert.equal(JSON.stringify(tuples), before); + 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) => { + 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 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(`/admin/users/${ada}`)).text()); + + const res = await post(`/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"]); + + // 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("/admin/groups/eng")).text()); + const groupRes = await post("/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"); + + // 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("/admin/groups/eng/permissions", `_csrf=${token}&baseline=${baselineOf(await (await get("/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/); +}); + // Revoking your own grants can remove the last users:write on the deployment, and the instant-revoke // hook lands it on the next request — recovery would be a curl against Keto. Guarded like // self-deactivate and self-delete are. (`admin1` is the harness's own sub.) diff --git a/src/i18n/locales/en-US.ts b/src/i18n/locales/en-US.ts index 37a85d7..471eb78 100644 --- a/src/i18n/locales/en-US.ts +++ b/src/i18n/locales/en-US.ts @@ -46,7 +46,6 @@ const messages = { "common.edit": "Edit", "common.new": "New", "common.remove": "Remove", - "common.view": "View", "consent.allow": "Allow", "consent.deny": "Deny", diff --git a/src/i18n/locales/sv-SE.ts b/src/i18n/locales/sv-SE.ts index 59a701d..85487eb 100644 --- a/src/i18n/locales/sv-SE.ts +++ b/src/i18n/locales/sv-SE.ts @@ -37,7 +37,6 @@ const messages: CoreMessages = { "common.edit": "Redigera", "common.new": "Ny", "common.remove": "Ta bort", - "common.view": "Visa", "consent.allow": "Tillåt", "consent.deny": "Neka",