From 765f3490071a06cbab714c1681c2fe0d66227362 Mon Sep 17 00:00:00 2001 From: lilleman Date: Wed, 5 Aug 2026 14:47:51 +0200 Subject: [PATCH] Model the read/write split in the UI: read-only views, self-revoke and inherited-grant guards --- AGENTS.md | 10 +++ README.md | 8 +-- examples/config/menu.ts | 2 +- examples/plugins/admin/README.md | 10 +-- examples/plugins/admin/admin-grants.test.ts | 25 +++++++- examples/plugins/admin/admin-grants.ts | 43 ++++++++++--- examples/plugins/admin/admin-groups.ts | 23 ++++--- examples/plugins/admin/admin-shared.test.ts | 4 +- examples/plugins/admin/admin-shared.ts | 4 +- examples/plugins/admin/admin-users.ts | 62 +++++++++++++------ examples/plugins/admin/i18n/en-US.ts | 4 ++ examples/plugins/admin/i18n/sv-SE.ts | 4 ++ examples/plugins/admin/plugin.ts | 2 +- examples/plugins/admin/views/group-detail.ejs | 2 +- examples/plugins/admin/views/groups.ejs | 3 +- .../views/partials/group-detail-body.ejs | 6 +- .../views/partials/permission-picker.ejs | 40 +++++++++--- .../admin/views/partials/user-form-body.ejs | 4 +- examples/plugins/admin/views/user-form.ejs | 2 +- examples/plugins/admin/views/users.ejs | 3 +- public/css/styles.css | 8 +++ src/auth/bootstrap.test.ts | 7 +++ src/auth/bootstrap.ts | 13 +++- src/http/app.test.ts | 54 ++++++++++++++++ src/plugin-host/plugin.ts | 2 +- src/ui/chrome.test.ts | 4 +- src/ui/nav.test.ts | 12 ++-- todo.md | 5 ++ 28 files changed, 287 insertions(+), 79 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a3895a1..e89a152 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -134,6 +134,16 @@ them. Revisit only if the stated reason stops holding. example it buys one thing: the route table and the in-handler guard derive from one function, so 29 routes × 2 gate sites cannot drift. As a general mechanism it would make authorization a function of the transport verb, and a route table must answer "what does this need?" on its own. +- **A `:read`-only holder must never be shown a write affordance.** The split created a real read-only + operator (a helpdesk account with `users:read`), and the host's 403 is the backstop, not the UX: the + list/detail models carry `canWrite` and the views drop create/save/delete/add/remove, while the + permission picker still renders — disabled — because *seeing* who holds what is the point of `:read`. + Two grant-specific guards go with it, both restoring behaviour the deleted Permissions screen had: + you cannot revoke your own grants (self-lockout would need a `curl` against Keto to undo, which the + operator persona can't do — same shape as the self-deactivate/self-delete guards), and a permission + held *through a group* renders ticked-but-disabled rather than unticked, because showing it unticked + stated the opposite of the truth and unticking it wrote nothing while looking like a successful + revoke. Raised by the architecture + product reviews 2026-08-05. - **`users:write` and `groups:write` are equivalent to full administrative access**, and the split does not change that: `groups:write` adds you to any group, including one holding every permission; `users:write` mints a recovery code for any account. The containment the split buys is real on the diff --git a/README.md b/README.md index 659c213..dd28507 100644 --- a/README.md +++ b/README.md @@ -126,7 +126,7 @@ or gated**, so the same foundation serves a purely public site, a fully locked-d tool, or the common middle: a public front with an authenticated area behind it. Its **sweet spot** is the **back-office and operational tooling** you'd otherwise hand-roll for the tenth time, but nothing ties it to internal-only use. The core itself ships **no domain screens at -all** — even the screens for running the system (**users, groups, permissions**) are a **drop-in +all** — even the screens for running the system (**users, groups, OAuth2 clients**) are a **drop-in plugin** you opt into ([`examples/plugins/admin/`](examples/plugins/admin/)). Everything is a plugin. **Who it's for.** Experienced developers building server-rendered web products — back-office @@ -160,7 +160,7 @@ audience above, and three of them shape the design more than any feature request - **Included in the core:** themed sign-in / register / reset (Kratos-backed), the design system + app shell, the config-driven menu, sessions, and access control. No domain screens. -- **Opt-in admin plugin:** the **users, groups, permissions, and OAuth2-clients** screens (users via +- **Opt-in admin plugin:** the **users, groups, and OAuth2-clients** screens (users via Kratos, the relationship graph via Keto, OAuth2 clients via Hydra) ship as [`examples/plugins/admin/`](examples/plugins/admin/) — copy it into `plugins/` to get a GUI for user & group admin. It's an ordinary plugin, using the privileged @@ -735,7 +735,7 @@ are `ICON_NAMES` in `src/ui/icons.ts`, and adding one means registering its luci subtree disappears with it. When the children need *different* permissions, leave the header ungated and gate each child: `composeNav` drops a header whose children all filtered out. That second form only works while the header carries **no `href`** — give it one and it survives the filter as an -ungated leaf, visible to everyone. The admin example uses it (four screens, four permissions). +ungated leaf, visible to everyone. The admin example uses it (three screens, six permissions). #### Public pages & menu items @@ -1332,7 +1332,7 @@ deactivate the user, or use a direct user-permission change, for an instant effe is for. Reserve it for those; don't pay its tuple-sync cost for rules a service can already answer from its own data. -The admin plugin's users / groups / permissions screens write authorization **only to Keto** — coarse +The admin plugin's users / groups screens write authorization **only to Keto** — coarse permissions and fine-grained relationships alike. ### OAuth2 provider (Hydra) diff --git a/examples/config/menu.ts b/examples/config/menu.ts index b41bbe9..27b34e9 100644 --- a/examples/config/menu.ts +++ b/examples/config/menu.ts @@ -20,7 +20,7 @@ export default defineMenu({ // Operator override (rename → group → order → hide), keyed by node id. override: { // rename: { people: "Staff" }, // node id → new label (or a catalog key) - // groups: [{ id: "admin", label: "Admin", children: ["users", "permissions"] }], + // groups: [{ id: "admin", label: "Admin", children: ["users", "groups"] }], // order: ["people", "reports"], // top-level order by id // hide: ["teams"], // remove nodes (any depth) }, diff --git a/examples/plugins/admin/README.md b/examples/plugins/admin/README.md index f61d997..564854e 100644 --- a/examples/plugins/admin/README.md +++ b/examples/plugins/admin/README.md @@ -7,7 +7,7 @@ screens live at `/admin/*`) and restart: ```bash cp -r examples/plugins/admin plugins/admin -docker compose restart web +docker compose up -d ``` The bootstrap grants the seeded `admin@plainpages.local` every permission this plugin declares, so the @@ -46,8 +46,8 @@ property of a user or a group, edited as a checkbox list on those two screens (` ## Layout -- `plugin.ts` — the manifest: the Admin nav fragment, the eight permissions the plugin declares, and - the route table — one thin handler per method+path, gated via `adminPermission(resource, method)` +- `plugin.ts` — the manifest: the Admin nav fragment, the six permissions the plugin declares, and + the route table — one thin handler per method+path, gated via `permissionName(resource, actionForMethod(method))` so a GET needs `:read` and a POST `:write`. - `admin-grants.ts` — the permission picker and the grant diff, shared by the Users and Groups screens: what a submitted checkbox set grants and revokes, against the host's declared catalog. @@ -55,12 +55,12 @@ property of a user or a group, edited as a checkbox list on those two screens (` view-model builders (unit-tested in the matching `*.test.ts`) plus thin per-route handlers keyed on `ctx.params` (the host extracts `:id`/`:name`), sharing a small `withX` wrapper that resolves the screen's permission gate + the needed `ctx.system` clients once. -- `admin-shared.ts` — the permission naming (`adminPermission`), the shared gate +- `admin-shared.ts` — the permission naming (`permissionName` / `actionForMethod`), the shared gate (`requirePermission`), CSRF form reader (`guardedForm`), confirm model, nav fragment, and the not-found / unavailable helpers. - `views/` — the screens' EJS, plus the admin-specific body partials under `views/partials/`. They `include()` the core building-block partials (shell, data-table, filter-bar, field, …). -The four screens hold **no state** — everything lives in Ory. Handlers are thin, so their builders +The three screens hold **no state** — everything lives in Ory. Handlers are thin, so their builders unit-test as pure functions with no host; the HTTP routing/gate/CSRF is covered in `src/http/app.test.ts` (which mounts this plugin) and end-to-end in `e2e-tests/full-flow.spec.ts`. diff --git a/examples/plugins/admin/admin-grants.test.ts b/examples/plugins/admin/admin-grants.test.ts index 29acb5b..efba871 100644 --- a/examples/plugins/admin/admin-grants.test.ts +++ b/examples/plugins/admin/admin-grants.test.ts @@ -35,17 +35,38 @@ test("grantDiff ignores anything the plugins don't declare, in both directions", }); test("buildPermissionPicker ticks what is held and carries each declaration's description", () => { - const picker = buildPermissionPicker({ action: "/admin/users/u1/permissions", declared, held: ["users:write"] }); + const picker = buildPermissionPicker({ action: "/admin/users/u1/permissions", declared, direct: ["users:write"] }); assert.equal(picker.action, "/admin/users/u1/permissions"); assert.deepEqual(picker.choices.map((c) => c.name), ["users:read", "users:write", "groups:read"]); assert.deepEqual(picker.choices.map((c) => c.checked), [false, true, false]); assert.equal(picker.choices[0]?.description, "View users"); assert.equal(picker.choices[2]?.description, ""); // a declaration may omit one assert.equal(picker.empty, undefined); + assert.equal(picker.readOnly, false); + assert.equal(picker.inheritedNote, undefined); // nothing is group-held here +}); + +// The failure this prevents: a permission held through a group used to render unticked, so the page +// said "not held" about a grant that reaches the JWT — and unticking it wrote nothing, which read as +// a successful revoke. Inherited rows are ticked, disabled, and never posted. +test("buildPermissionPicker distinguishes a direct grant from one inherited through a group", () => { + const picker = buildPermissionPicker({ action: "/x", declared, direct: ["users:write"], effective: ["users:read", "users:write"] }); + assert.deepEqual(picker.choices.map((c) => [c.name, c.checked, c.inherited]), [ + ["users:read", true, true], // effective but not direct → shown as held, not editable here + ["users:write", true, false], // direct → editable + ["groups:read", false, false], + ]); + assert.ok(picker.inheritedNote, "the disabled row needs an explanation"); +}); + +test("buildPermissionPicker in read-only mode still shows the state, and marks itself unwritable", () => { + const picker = buildPermissionPicker({ action: "/x", declared, direct: ["users:read"], readOnly: true }); + assert.equal(picker.readOnly, true); + assert.deepEqual(picker.choices.map((c) => c.checked), [true, false, false]); // a reader still sees who holds what }); test("buildPermissionPicker says so when no plugin declares a permission, rather than rendering an empty box", () => { - const picker = buildPermissionPicker({ action: "/x", declared: [], held: [] }); + const picker = buildPermissionPicker({ action: "/x", declared: [], direct: [] }); assert.deepEqual(picker.choices, []); assert.ok(picker.empty); }); diff --git a/examples/plugins/admin/admin-grants.ts b/examples/plugins/admin/admin-grants.ts index 1687f43..a3d04ff 100644 --- a/examples/plugins/admin/admin-grants.ts +++ b/examples/plugins/admin/admin-grants.ts @@ -22,8 +22,7 @@ export function grantTuple(permission: string, subject: GrantSubject): RelationT } // The permissions this subject holds *directly* — one Keto read filtered by the subject, not one per -// declared name. A group's members hold them transitively; that expansion is Keto's job at login, -// and this screen edits the direct edge only. +// declared name. This is the edge the picker edits; `effectivePermissions` adds what a group confers. export async function heldPermissions(keto: KetoClient, subject: GrantSubject): Promise { const held = new Set(); let pageToken: string | undefined; @@ -35,9 +34,20 @@ export async function heldPermissions(keto: KetoClient, subject: GrantSubject): return [...held].sort(); } +// Every declared permission the subject effectively holds — direct grants *plus* anything reached +// through a group, which is what actually lands in their JWT. One Keto check per declared name; +// the catalog is small and this is an admin screen (login does the same walk). +export async function effectivePermissions(keto: KetoClient, subject: GrantSubject, declared: PermissionDecl[]): Promise { + const held = await Promise.all(declared.map((decl) => keto.check({ namespace: PERMISSION_NS, object: decl.name, relation: GRANTED, ...subject }))); + return declared.filter((_, i) => held[i]).map((decl) => decl.name); +} + export interface PermissionChoice { - checked: boolean; + checked: boolean; // held directly — the only state this form can change description: string; + // Effective through a group, not granted directly. Rendered ticked but disabled: the grant is real + // (it reaches the JWT), and it is removed by editing the group, not this subject. + inherited: boolean; name: string; } @@ -45,27 +55,44 @@ export interface PermissionPicker { action: string; choices: PermissionChoice[]; empty: string | undefined; // set when no plugin declares a permission — the picker has nothing to offer + error?: string; // a rejected save (e.g. the self-revoke guard), rendered above the list field: string; + hint: string; + inheritedNote: string | undefined; // set when at least one choice is group-held, to explain the disabled row legend: string; + readOnly: boolean; // the viewer holds :read but not :write — show the state, offer no save submit: string; } -// The checkbox list: every declared permission, ticked where this subject already holds it. A fixed -// list means the form is the whole truth — what it posts back *is* the desired set (applyGrants). +// The checkbox list: every declared permission, ticked where this subject holds it. A fixed list +// means the form is the whole truth — what it posts back *is* the desired set of *direct* grants +// (grantDiff). An inherited row is disabled, so it never posts and can never be diffed into a revoke. export function buildPermissionPicker(opts: { action: string; declared: PermissionDecl[]; - held: string[]; + direct: string[]; + effective?: string[]; // omit when the caller can't resolve group-held grants; then only direct shows + readOnly?: boolean; t?: Translate; }): PermissionPicker { const t = opts.t ?? ((k: string) => k); - const heldSet = new Set(opts.held); + const directSet = new Set(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), + description: decl.description ?? "", + inherited: !directSet.has(decl.name) && effectiveSet.has(decl.name), + name: decl.name, + })); return { action: opts.action, - choices: opts.declared.map((decl) => ({ checked: heldSet.has(decl.name), description: decl.description ?? "", name: decl.name })), + choices, empty: opts.declared.length === 0 ? t("admin.grants.none") : undefined, field: PERMISSIONS_FIELD, + hint: t("admin.grants.hint"), + inheritedNote: choices.some((c) => c.inherited) ? t("admin.grants.inherited") : undefined, legend: t("admin.grants.legend"), + readOnly: opts.readOnly === true, submit: t("admin.grants.save"), }; } diff --git a/examples/plugins/admin/admin-groups.ts b/examples/plugins/admin/admin-groups.ts index 0632e52..7fcf7ca 100644 --- a/examples/plugins/admin/admin-groups.ts +++ b/examples/plugins/admin/admin-groups.ts @@ -6,9 +6,9 @@ // per-route handlers (keyed on ctx.params) over a shared `withGroups` gate — admin-only, CSRF-guarded, // each returning a RouteResult. -import { type KetoClient, type KratosAdmin, paginate, parseListQuery, type RelationQuery, type RelationTuple, type RequestContext, type RouteHandler, type RouteResult, type SubjectSet, type Translate, type User } from "#plugin-api"; -import { applyGrants, buildPermissionPicker, grantDiff, groupSubject, heldPermissions, type PermissionPicker, PERMISSIONS_FIELD } from "./admin-grants.ts"; -import { ADMIN_EN, ADMIN_GROUPS_BASE, buildConfirmModel, guardedForm, notFound, requirePermission, unavailable } from "./admin-shared.ts"; +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 "#plugin-api"; +import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, groupSubject, heldPermissions, type PermissionPicker, PERMISSIONS_FIELD } from "./admin-grants.ts"; +import { ADMIN_EN, ADMIN_GROUPS_BASE, buildConfirmModel, guardedForm, notFound, permissionName, requirePermission, unavailable } from "./admin-shared.ts"; import type { FieldConfig } from "./admin-users.ts"; const GROUP_NS = "Group"; @@ -111,6 +111,7 @@ function listHref(state: ListState, overrides: Partial = {}): string } export function buildGroupsListModel(opts: { + canWrite?: boolean; csrfToken?: string; groups: GroupView[]; t?: Translate; @@ -140,6 +141,7 @@ export function buildGroupsListModel(opts: { return { breadcrumbs: [{ href: ADMIN_GROUPS_BASE, label: t("admin.nav.section") }, { label: t("admin.groups.title") }], + canWrite: opts.canWrite !== false, filterBar: listFilterBar(state, t), pagination: listPagination(state, page, t), table: listTable(rows, state, sort, t), @@ -225,6 +227,7 @@ export function buildGroupFormModel(opts: { } export function buildGroupDetailModel(opts: { + canWrite?: boolean; // false ⇒ a `groups:read` holder: show the members, offer no edit candidates: MemberOption[]; csrfToken?: string; error?: string; @@ -239,9 +242,11 @@ export function buildGroupDetailModel(opts: { const taken = new Set(opts.members.map((m) => m.subject)); const self = `group:${name}`; // a group can't be a member of itself const options = opts.candidates.filter((c) => c.value !== self && !taken.has(c.value)); + const canWrite = opts.canWrite !== false; return { add: { action: `${base}/members`, options }, breadcrumbs: [{ href: ADMIN_GROUPS_BASE, label: t("admin.groups.title") }, { label: name }], + canWrite, // the view drops add/remove/delete when false; the host already 403s those POSTs csrfToken: opts.csrfToken ?? "", delete: { action: `${base}/delete` }, error: opts.error, @@ -254,7 +259,7 @@ export function buildGroupDetailModel(opts: { // ---- request handler (imperative shell) ---- -// Drain every page of a relation-tuple query. (Reused by the Roles screen — same membership model.) +// Drain every page of a relation-tuple query. export async function pagedTuples(keto: KetoClient, query: RelationQuery): Promise { const out: RelationTuple[] = []; let pageToken: string | undefined; @@ -320,7 +325,7 @@ const groupFormResult = async (deps: GroupsDeps, extra: { error?: string; values // GET /admin/groups — the list. export const groupsList = withGroups(async ({ ctx, keto }) => { const groups = groupsFromTuples(await pagedTuples(keto, { namespace: GROUP_NS, relation: MEMBERS })); - return { data: { chrome: ctx.chrome, model: buildGroupsListModel({ csrfToken: ctx.chrome.csrfToken, groups, t: ctx.t, url: ctx.url }) }, view: "groups" }; + return { data: { chrome: ctx.chrome, model: buildGroupsListModel({ canWrite: can(ctx, permissionName("groups", "write")), csrfToken: ctx.chrome.csrfToken, groups, t: ctx.t, url: ctx.url }) }, view: "groups" }; }); // POST /admin/groups — create (a group exists once it has ≥1 member, so this writes the first tuple). @@ -346,13 +351,17 @@ export const groupsNewForm = withGroups((deps) => groupFormResult(deps, {})); export const groupsDetail = withGroupName(async ({ ctx, keto, kratosAdmin }, name) => { const { emailById, options } = await memberCandidates(keto, kratosAdmin); const members = (await pagedTuples(keto, { namespace: GROUP_NS, object: name, relation: MEMBERS })).map((t) => memberView(t, emailById)); + const subject = groupSubject(name); + const [direct, effective] = await Promise.all([heldPermissions(keto, subject), effectivePermissions(keto, subject, ctx.declaredPermissions)]); const permissions = buildPermissionPicker({ action: `${detailHref(name)}/permissions`, declared: ctx.declaredPermissions, - held: await heldPermissions(keto, groupSubject(name)), + direct, + effective, // a group nested in another group inherits its permissions too + readOnly: !can(ctx, permissionName("groups", "write")), t: ctx.t, }); - return { data: { chrome: ctx.chrome, model: buildGroupDetailModel({ candidates: options, csrfToken: ctx.chrome.csrfToken, group: { name }, members, permissions, t: ctx.t }) }, view: "group-detail" }; + 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" }; }); // POST /admin/groups/:name/permissions — the submitted checkboxes are the desired set. Members hold diff --git a/examples/plugins/admin/admin-shared.test.ts b/examples/plugins/admin/admin-shared.test.ts index 2455ffd..bc13d0d 100644 --- a/examples/plugins/admin/admin-shared.test.ts +++ b/examples/plugins/admin/admin-shared.test.ts @@ -1,5 +1,5 @@ // Direct units for the admin plugin's shared nav + auth helpers. They're security-critical -// (requirePermission/guardedForm gate every admin write) and reused across all four screens, so pin the +// (requirePermission/guardedForm gate every admin write) and reused across all three screens, so pin the // contract here in isolation; the HTTP routing/gate/CSRF is exercised end-to-end in src/http/app.test.ts. // Import only from the #plugin-api barrel — the same contract boundary the plugin code uses. import assert from "node:assert/strict"; @@ -30,7 +30,7 @@ function fakeCtx(opts: { body?: string; method?: string; user?: User | null; ver test("ADMIN_NAV: an ungated Admin header whose three screens each gate on their own read permission", () => { assert.equal(ADMIN_NAV.id, "admin"); // No gate on the header: a user may hold one screen's permission and not another's. composeNav - // drops a header left with no visible children, so holding none of the four hides the section. + // drops a header left with no visible children, so holding none of the three hides the section. // Both halves matter — give the header an `href` and it survives the filter as a visible leaf, // ungated, for anonymous visitors included. assert.equal(ADMIN_NAV.permission, undefined); diff --git a/examples/plugins/admin/admin-shared.ts b/examples/plugins/admin/admin-shared.ts index edd9ee1..22d5929 100644 --- a/examples/plugins/admin/admin-shared.ts +++ b/examples/plugins/admin/admin-shared.ts @@ -38,10 +38,10 @@ export function actionForMethod(method: string): AdminAction { return verb === "GET" || verb === "HEAD" ? "read" : "write"; } -// The plugin's nav fragment: an ungated "Admin" header + its four screens, each gated on its own +// The plugin's nav fragment: an ungated "Admin" header + its three screens, each gated on its own // read permission. The header carries no `permission` because a user may hold one screen's and not // another's; composeNav drops a header left with no visible children, so a user holding none of the -// four never sees the section. The host current-marks the active item — no `current`/`open` here. +// three never sees the section. The host current-marks the active item — no `current`/`open` here. export const ADMIN_NAV: NavNode = { children: [ { href: ADMIN_USERS_BASE, icon: "i-users", id: "users", label: "admin.nav.users", permission: permissionName("users", "read") }, diff --git a/examples/plugins/admin/admin-users.ts b/examples/plugins/admin/admin-users.ts index 80cb21c..4237c6d 100644 --- a/examples/plugins/admin/admin-users.ts +++ b/examples/plugins/admin/admin-users.ts @@ -4,9 +4,9 @@ // models; below them are thin per-route handlers (keyed on ctx.params) over a shared `withUser` gate // — admin-only, CSRF-guarded, each returning a RouteResult (a view, or a redirect after a write — PRG). -import { type Identity, type KetoClient, type KratosAdmin, KratosError, paginate, parseListQuery, type RecoveryCode, type RequestContext, type RouteHandler, type RouteResult, type Translate, type User } from "#plugin-api"; -import { applyGrants, buildPermissionPicker, grantDiff, heldPermissions, type PermissionPicker, PERMISSIONS_FIELD, userSubject } from "./admin-grants.ts"; -import { ADMIN_EN, ADMIN_USERS_BASE, buildConfirmModel, guardedForm, notFound, requirePermission, unavailable } from "./admin-shared.ts"; +import { can, type Identity, type KetoClient, type KratosAdmin, KratosError, paginate, parseListQuery, type RecoveryCode, type RequestContext, type RouteHandler, type RouteResult, type Translate, type User } from "#plugin-api"; +import { applyGrants, buildPermissionPicker, effectivePermissions, grantDiff, heldPermissions, type PermissionPicker, PERMISSIONS_FIELD, userSubject } from "./admin-grants.ts"; +import { ADMIN_EN, 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; @@ -106,6 +106,7 @@ function listHref(state: ListState, overrides: Partial = {}): string } export function buildUsersListModel(opts: { + canWrite?: boolean; csrfToken?: string; identities: Identity[]; t?: Translate; @@ -135,6 +136,7 @@ export function buildUsersListModel(opts: { return { breadcrumbs: [{ href: ADMIN_USERS_BASE, label: t("admin.nav.section") }, { label: t("admin.users.title") }], + canWrite: opts.canWrite !== false, filterBar: listFilterBar(state, all.length, t), pagination: listPagination(state, page, t), table: listTable(rows, state, sort, t), @@ -217,6 +219,7 @@ export interface FieldConfig { } export function buildUserFormModel(opts: { + canWrite?: boolean; // false ⇒ a `users:read` holder: show the state, render no write affordance csrfToken?: string; error?: string; identity?: Identity | null; @@ -240,8 +243,10 @@ export function buildUserFormModel(opts: { ]; if (!editing) fields.push({ autocomplete: "new-password", hint: t("admin.users.field.passwordHint"), icon: "i-lock", id: "password", label: t("admin.users.field.password"), name: "password", optional: true, type: "password" }); + const canWrite = opts.canWrite !== false; return { breadcrumbs: [{ href: ADMIN_USERS_BASE, label: t("admin.users.title") }, { label: editing ? t("common.edit") : t("common.new") }], + canWrite, // the view drops every write affordance when false; the host already 403s the POSTs edit: editing ? { deleteAction: `${idPath}/delete`, id: view!.id, @@ -304,7 +309,7 @@ const formResult = (ctx: RequestContext, extra: Parameters { const { identities } = await kratosAdmin.listIdentities({ pageSize: LIST_FETCH_SIZE }); - return { data: { chrome: ctx.chrome, model: buildUsersListModel({ csrfToken: ctx.chrome.csrfToken, identities, t: ctx.t, url: ctx.url }) }, view: "users" }; + return { data: { chrome: ctx.chrome, model: buildUsersListModel({ canWrite: canWriteUsers(ctx), csrfToken: ctx.chrome.csrfToken, identities, t: ctx.t, url: ctx.url }) }, view: "users" }; }); // POST /admin/users — create; a Kratos 4xx re-renders the form (400), keeping the input. @@ -326,31 +331,50 @@ 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) => { const permissions = await userPermissionPicker(deps, id); - return formResult(deps.ctx, { identity, ...(permissions ? { permissions } : {}) }); + return formResult(deps.ctx, { canWrite: canWriteUsers(deps.ctx), identity, ...(permissions ? { permissions } : {}) }); }); -// The checkbox list of declared permissions, ticked where this user holds one directly. Undefined -// when Keto isn't wired — the rest of the edit page still works. -async function userPermissionPicker(deps: UsersDeps, id: string): Promise { +const canWriteUsers = (ctx: RequestContext): boolean => can(ctx, permissionName("users", "write")); + +// The checkbox list of declared permissions: ticked where this user holds one, and disabled where +// the grant comes from a group (real, but removed on that group). Undefined when Keto isn't wired — +// the rest of the edit page still works. +async function userPermissionPicker(deps: UsersDeps, id: string, error?: string): Promise { if (!deps.keto) return undefined; - const held = await heldPermissions(deps.keto, userSubject(id)); - return buildPermissionPicker({ - action: `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}/permissions`, - declared: deps.ctx.declaredPermissions, - held, - t: deps.ctx.t, - }); + const subject = userSubject(id); + const [direct, effective] = await Promise.all([ + heldPermissions(deps.keto, subject), + effectivePermissions(deps.keto, subject, deps.ctx.declaredPermissions), + ]); + return { + ...buildPermissionPicker({ + action: `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}/permissions`, + declared: deps.ctx.declaredPermissions, + direct, + effective, + readOnly: !canWriteUsers(deps.ctx), + t: deps.ctx.t, + }), + ...(error ? { error } : {}), + }; } -// POST /admin/users/:id/permissions — the submitted checkboxes are the desired set; grant what's -// newly ticked, revoke what's newly unticked. A change to a user's own grants revokes their live -// tokens so it lands now rather than at the next re-mint. -export const usersPermissions = withTarget(async (deps, _identity, id) => { +// POST /admin/users/:id/permissions — the submitted checkboxes are the desired set of *direct* +// grants; grant what's newly ticked, revoke what's newly unticked. A change to a user's own grants +// revokes their live tokens so it lands now rather than at the next re-mint. +export const usersPermissions = withTarget(async (deps, identity, id) => { const { ctx, keto, revoke, user } = deps; const form = (await guardedForm(ctx))!; if (!keto) return unavailable(ctx, ctx.t("admin.capability.keto")); const subject = userSubject(id); const diff = grantDiff(ctx.declaredPermissions, await heldPermissions(keto, subject), form.getAll(PERMISSIONS_FIELD)); + // 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. Recovery would be a curl against Keto — not something the operator persona can do. + if (id === user.id && diff.revoke.length > 0) { + const permissions = await userPermissionPicker(deps, id, ctx.t("admin.grants.selfRevoke")); + return { ...formResult(ctx, { canWrite: canWriteUsers(ctx), identity, ...(permissions ? { permissions } : {}) }), status: 400 }; + } await applyGrants(keto, subject, diff); if (diff.grant.length > 0 || diff.revoke.length > 0) { revoke?.(id); diff --git a/examples/plugins/admin/i18n/en-US.ts b/examples/plugins/admin/i18n/en-US.ts index 654ac00..88ad576 100644 --- a/examples/plugins/admin/i18n/en-US.ts +++ b/examples/plugins/admin/i18n/en-US.ts @@ -48,9 +48,13 @@ const messages = { "admin.common.type": "Type", "admin.common.user": "User", + "admin.grants.hint": "Which permissions exist is set by the plugins installed on this system. Tick to grant, untick to revoke.", + "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.pending": "Members get this at their next sign-in (up to 10 minutes).", "admin.grants.save": "Save permissions", + "admin.grants.selfRevoke": "You can't revoke your own permissions — ask another administrator, so you can't lock yourself out.", "admin.groups.actions": "Group actions", "admin.groups.addMember": "Add a member", diff --git a/examples/plugins/admin/i18n/sv-SE.ts b/examples/plugins/admin/i18n/sv-SE.ts index 6bf7e0c..a5f2696 100644 --- a/examples/plugins/admin/i18n/sv-SE.ts +++ b/examples/plugins/admin/i18n/sv-SE.ts @@ -48,9 +48,13 @@ const messages: AdminMessages = { "admin.common.type": "Typ", "admin.common.user": "Användare", + "admin.grants.hint": "Vilka behörigheter som finns bestäms av de plugins som är installerade. Kryssa i för att tilldela, ur för att återkalla.", + "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.pending": "Medlemmar får detta vid nästa inloggning (upp till 10 minuter).", "admin.grants.save": "Spara behörigheter", + "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.groups.actions": "Gruppåtgärder", "admin.groups.addMember": "Lägg till en medlem", diff --git a/examples/plugins/admin/plugin.ts b/examples/plugins/admin/plugin.ts index d3125ad..7523f19 100644 --- a/examples/plugins/admin/plugin.ts +++ b/examples/plugins/admin/plugin.ts @@ -1,4 +1,4 @@ -// Admin example plugin: the Users / Groups / Roles / OAuth2-clients screens for running the system. +// Admin example plugin: the Users / Groups / OAuth2-clients screens for running the system. // These used to ship inside the core; they were extracted here so a fresh clone has no built-in admin // GUI. Copy this folder to plugins/admin (then restart) to enable it — see README → Quick start. // diff --git a/examples/plugins/admin/views/group-detail.ejs b/examples/plugins/admin/views/group-detail.ejs index 54b7c81..5de3371 100644 --- a/examples/plugins/admin/views/group-detail.ejs +++ b/examples/plugins/admin/views/group-detail.ejs @@ -2,7 +2,7 @@ Group admin detail / membership page: the group-detail body in the app shell. %><% const nav = include("partials/nav-tree", { nodes: chrome.nav }); - const body = include("partials/group-detail-body", { add: model.add, csrfToken: model.csrfToken, del: model.delete, error: model.error, group: model.group, members: model.members, permissions: model.permissions }); + const body = include("partials/group-detail-body", { add: model.add, canWrite: model.canWrite, csrfToken: model.csrfToken, del: model.delete, error: model.error, group: model.group, members: model.members, permissions: model.permissions }); -%> <%- include("partials/shell", { body, diff --git a/examples/plugins/admin/views/groups.ejs b/examples/plugins/admin/views/groups.ejs index 40a0738..4227cb6 100644 --- a/examples/plugins/admin/views/groups.ejs +++ b/examples/plugins/admin/views/groups.ejs @@ -6,7 +6,8 @@ const filters = include("partials/filter-bar", model.filterBar); const table = include("partials/data-table", model.table); const pager = include("partials/pagination", model.pagination); - const actions = '' + t("admin.groups.new") + ''; + // Only offer "New group" to a groups:write holder — a groups:read one would get the 403 page. + const actions = model.canWrite === false ? "" : '' + t("admin.groups.new") + ''; -%> <%- include("partials/shell", { actions, diff --git a/examples/plugins/admin/views/partials/group-detail-body.ejs b/examples/plugins/admin/views/partials/group-detail-body.ejs index 2fc1f06..d34d4c6 100644 --- a/examples/plugins/admin/views/partials/group-detail-body.ejs +++ b/examples/plugins/admin/views/partials/group-detail-body.ejs @@ -21,13 +21,14 @@ <% if (members.rows.length) { -%>
<% members.rows.forEach((m) => { -%> - + <% }) -%>
<%= t("admin.groups.membersOf", { name: group.name }) %>
<%= t("admin.common.member") %><%= t("admin.common.type") %><%= t("table.actions") %>
<%= m.label %><%= m.kind === "group" ? t("admin.common.group") : t("admin.common.user") %>
<%= m.label %><%= m.kind === "group" ? t("admin.common.group") : t("admin.common.user") %><% if (locals.canWrite !== false) { %>
<% } %>
<% } else { -%>

<%= t("admin.groups.noMembers") %>

<% } -%> +<% if (locals.canWrite !== false) { -%>

<%= t("admin.groups.addMember") %>

<% if (add.options.length) { -%> @@ -36,10 +37,13 @@

<%= t("admin.groups.allMembers") %>

<% } -%>
+<% } -%> <% if (locals.permissions) { -%> <%- include("partials/permission-picker", { csrfToken: csrf, permissions: locals.permissions }) %> <% } -%> +<% if (locals.canWrite !== false) { -%>
"> <%= t("admin.groups.delete") %>
+<% } -%> diff --git a/examples/plugins/admin/views/partials/permission-picker.ejs b/examples/plugins/admin/views/partials/permission-picker.ejs index ac47faa..bd09a2c 100644 --- a/examples/plugins/admin/views/partials/permission-picker.ejs +++ b/examples/plugins/admin/views/partials/permission-picker.ejs @@ -1,26 +1,46 @@ <%# The permission picker, shared by the user-edit and group-detail pages. A fieldset of checkboxes — - one per permission the installed plugins declare — ticked where this user/group already holds it. - The whole set posts back, so what is submitted IS the desired state (see admin-grants.ts). - Locals: csrfToken, permissions ({ action, choices, empty, field, legend, submit }). + one per permission the installed plugins declare — ticked where this user/group holds it. The whole + set posts back, so what is submitted IS the desired set of *direct* grants (see admin-grants.ts). + + Two rows never post, by design: an `inherited` one (the grant comes from a group, so it is changed + 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, choices, empty, error, field, hint, inheritedNote, legend, readOnly, submit }). %>

<%= permissions.legend %>

+<% if (permissions.error) { -%> +<%- include("partials/alert", { text: permissions.error, tone: "neg" }) %> +<% } -%> <% if (permissions.empty) { -%>

<%= permissions.empty %>

+<% } else { -%> +

<%= permissions.hint %>

+<% if (permissions.readOnly) { -%> +
+ <%= permissions.legend %> +<% permissions.choices.forEach((c) => { -%> + +<% }) -%> +
<% } else { -%>
-
+
<%= permissions.legend %> -<% permissions.choices.forEach((c, i) => { -%> -
- > - -
+<% permissions.choices.forEach((c) => { -%> + <% }) -%>
- +
+ +
+<% } -%> +<% if (permissions.inheritedNote) { -%> +

<%= permissions.inheritedNote %>

+<% } -%> <% } -%>
diff --git a/examples/plugins/admin/views/partials/user-form-body.ejs b/examples/plugins/admin/views/partials/user-form-body.ejs index d15b7b9..a9c3d78 100644 --- a/examples/plugins/admin/views/partials/user-form-body.ejs +++ b/examples/plugins/admin/views/partials/user-form-body.ejs @@ -23,13 +23,15 @@ <% }) -%>
<%= t("common.cancel") %> +<% if (locals.canWrite !== false) { -%> +<% } -%>
<% if (edit && locals.permissions) { -%> <%- include("partials/permission-picker", { csrfToken: form.csrfToken, permissions: locals.permissions }) %> <% } -%> -<% if (edit) { -%> +<% if (edit && locals.canWrite !== false) { -%>
">
diff --git a/examples/plugins/admin/views/user-form.ejs b/examples/plugins/admin/views/user-form.ejs index 8498e0b..d0378a1 100644 --- a/examples/plugins/admin/views/user-form.ejs +++ b/examples/plugins/admin/views/user-form.ejs @@ -2,7 +2,7 @@ Users admin create/edit page: the user-form body captured into the app shell. %><% const nav = include("partials/nav-tree", { nodes: chrome.nav }); - const body = include("partials/user-form-body", { edit: model.edit, error: model.error, form: model.form, permissions: model.permissions, recovery: model.recovery }); + const body = include("partials/user-form-body", { canWrite: model.canWrite, edit: model.edit, error: model.error, form: model.form, permissions: model.permissions, recovery: model.recovery }); -%> <%- include("partials/shell", { body, diff --git a/examples/plugins/admin/views/users.ejs b/examples/plugins/admin/views/users.ejs index ecef9b1..86a9350 100644 --- a/examples/plugins/admin/views/users.ejs +++ b/examples/plugins/admin/views/users.ejs @@ -6,7 +6,8 @@ const filters = include("partials/filter-bar", model.filterBar); const table = include("partials/data-table", model.table); const pager = include("partials/pagination", model.pagination); - const actions = '' + t("admin.users.new") + ''; + // Only offer "New user" to a users:write holder — a users:read one would get the 403 page. + const actions = model.canWrite === false ? "" : '' + t("admin.users.new") + ''; -%> <%- include("partials/shell", { actions, diff --git a/public/css/styles.css b/public/css/styles.css index 84618e6..d55a79f 100644 --- a/public/css/styles.css +++ b/public/css/styles.css @@ -474,6 +474,14 @@ span.nav-self { cursor: default; } /* static / non-clickable */ .check input, .radio input { width: 15px; height: 15px; accent-color: var(--accent); margin: 0; cursor: pointer; } .check:hover, .radio:hover { color: var(--text); } +/* A stacked group of .check rows in a
— the right element for related checkboxes, but the + UA gives it a groove border, so reset it like .filter-field and .menu-field do. A disabled row is + still readable: it states a fact (a permission held through a group) rather than offering an edit. */ +.check-group { border: 0; margin: 0; padding: 0; display: flex; flex-direction: column; gap: 10px; } +.check-group .check { align-items: baseline; } +.check-group .check input:disabled { cursor: default; } +.check-group .check:has(input:disabled) { opacity: .7; cursor: default; } +.check-group .check .cell-muted { margin-left: auto; padding-left: 12px; font-size: var(--fz-xs); } /* popover menu (language picker, profile, row kebab) — a