Extend the read-only treatment to OAuth2 clients and write-intent GETs
This commit is contained in:
@@ -138,12 +138,20 @@ them. Revisit only if the stated reason stops holding.
|
|||||||
operator (a helpdesk account with `users:read`), and the host's 403 is the backstop, not the UX: the
|
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
|
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`.
|
permission picker still renders — disabled — because *seeing* who holds what is the point of `:read`.
|
||||||
|
A *write-intent GET* — a create form or a delete-confirm page — is the exception to
|
||||||
|
`actionForMethod`: it gates on `:write` (declared in the route table and passed to the handler's
|
||||||
|
guard, so the two still agree), because a page whose only purpose is to start a write should refuse
|
||||||
|
a reader rather than render a form whose submit 403s.
|
||||||
Two grant-specific guards go with it, both restoring behaviour the deleted Permissions screen had:
|
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
|
you cannot revoke your own **direct** grants on the Users screen (self-lockout would need a `curl`
|
||||||
operator persona can't do — same shape as the self-deactivate/self-delete guards), and a permission
|
against Keto to undo, which the operator persona can't do — same shape as the self-deactivate/
|
||||||
held *through a group* renders ticked-but-disabled rather than unticked, because showing it unticked
|
self-delete guards), and a permission held *through a group* renders ticked-but-disabled rather than
|
||||||
stated the opposite of the truth and unticking it wrote nothing while looking like a successful
|
unticked, because showing it unticked stated the opposite of the truth and unticking it wrote
|
||||||
revoke. Raised by the architecture + product reviews 2026-08-05.
|
nothing while looking like a successful revoke. **Known gap, same scope the deleted screen had:**
|
||||||
|
the group paths are unguarded — unticking a permission on a group you belong to, removing yourself
|
||||||
|
from it, or deleting it can all still strip your own effective access. The robust "last effective
|
||||||
|
holder" check needs a reverse Keto query and is deferred. Raised by the architecture + product +
|
||||||
|
stability reviews 2026-08-05.
|
||||||
- **`users:write` and `groups:write` are equivalent to full administrative access**, and the split
|
- **`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;
|
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
|
`users:write` mints a recovery code for any account. The containment the split buys is real on the
|
||||||
|
|||||||
@@ -5,8 +5,8 @@
|
|||||||
// PRG redirect (mirrors the Users "trigger recovery" one-time code). Below the builders are thin
|
// PRG redirect (mirrors the Users "trigger recovery" one-time code). Below the builders are thin
|
||||||
// per-route handlers (keyed on ctx.params) over a shared `withClients` gate — admin-only, CSRF-guarded.
|
// per-route handlers (keyed on ctx.params) over a shared `withClients` gate — admin-only, CSRF-guarded.
|
||||||
|
|
||||||
import { type HydraAdmin, HydraError, type OAuth2Client, paginate, parseListQuery, type RequestContext, type RouteHandler, type RouteResult, type Translate, type User } from "#plugin-api";
|
import { can, type HydraAdmin, HydraError, type OAuth2Client, paginate, parseListQuery, type RequestContext, type RouteHandler, type RouteResult, type Translate, type User } from "#plugin-api";
|
||||||
import { ADMIN_CLIENTS_BASE, ADMIN_EN, buildConfirmModel, guardedForm, notFound, requirePermission, unavailable } from "./admin-shared.ts";
|
import { ADMIN_CLIENTS_BASE, ADMIN_EN, type AdminAction, buildConfirmModel, guardedForm, notFound, permissionName, requirePermission, unavailable } from "./admin-shared.ts";
|
||||||
import type { FieldConfig } from "./admin-users.ts";
|
import type { FieldConfig } from "./admin-users.ts";
|
||||||
|
|
||||||
const DEFAULT_PAGE_SIZE = 25;
|
const DEFAULT_PAGE_SIZE = 25;
|
||||||
@@ -100,6 +100,7 @@ function listHref(state: ListState, overrides: Partial<ListState> = {}): string
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function buildClientsListModel(opts: {
|
export function buildClientsListModel(opts: {
|
||||||
|
canWrite?: boolean;
|
||||||
clients: OAuth2Client[];
|
clients: OAuth2Client[];
|
||||||
csrfToken?: string;
|
csrfToken?: string;
|
||||||
t?: Translate;
|
t?: Translate;
|
||||||
@@ -119,6 +120,7 @@ export function buildClientsListModel(opts: {
|
|||||||
|
|
||||||
return {
|
return {
|
||||||
breadcrumbs: [{ href: ADMIN_CLIENTS_BASE, label: t("admin.nav.section") }, { label: t("admin.clients.title") }],
|
breadcrumbs: [{ href: ADMIN_CLIENTS_BASE, label: t("admin.nav.section") }, { label: t("admin.clients.title") }],
|
||||||
|
canWrite: opts.canWrite !== false,
|
||||||
filterBar: listFilterBar(state, t),
|
filterBar: listFilterBar(state, t),
|
||||||
pagination: listPagination(state, page, t),
|
pagination: listPagination(state, page, t),
|
||||||
table: listTable(rows, t),
|
table: listTable(rows, t),
|
||||||
@@ -208,6 +210,7 @@ export function buildClientFormModel(opts: {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function buildClientDetailModel(opts: {
|
export function buildClientDetailModel(opts: {
|
||||||
|
canWrite?: boolean;
|
||||||
client: ClientView;
|
client: ClientView;
|
||||||
created?: boolean; // just registered → success banner + the one-time secret (if any)
|
created?: boolean; // just registered → success banner + the one-time secret (if any)
|
||||||
csrfToken?: string;
|
csrfToken?: string;
|
||||||
@@ -218,6 +221,7 @@ export function buildClientDetailModel(opts: {
|
|||||||
const base = detailHref(opts.client.id);
|
const base = detailHref(opts.client.id);
|
||||||
return {
|
return {
|
||||||
breadcrumbs: [{ href: ADMIN_CLIENTS_BASE, label: t("admin.clients.title") }, { label: opts.client.name }],
|
breadcrumbs: [{ href: ADMIN_CLIENTS_BASE, label: t("admin.clients.title") }, { label: opts.client.name }],
|
||||||
|
canWrite: opts.canWrite !== false,
|
||||||
client: opts.client,
|
client: opts.client,
|
||||||
created: opts.created ?? false,
|
created: opts.created ?? false,
|
||||||
csrfToken: opts.csrfToken ?? "",
|
csrfToken: opts.csrfToken ?? "",
|
||||||
@@ -243,9 +247,9 @@ function readClientInput(form: URLSearchParams): ClientInput {
|
|||||||
// Hydra capability (else a themed 503). Each route below is a thin handler over these.
|
// Hydra capability (else a themed 503). Each route below is a thin handler over these.
|
||||||
interface ClientsDeps { ctx: RequestContext; hydra: HydraAdmin; user: User; }
|
interface ClientsDeps { ctx: RequestContext; hydra: HydraAdmin; user: User; }
|
||||||
|
|
||||||
function withClients(inner: (deps: ClientsDeps) => Promise<RouteResult>): RouteHandler {
|
function withClients(inner: (deps: ClientsDeps) => Promise<RouteResult>, action?: AdminAction): RouteHandler {
|
||||||
return async (ctx) => {
|
return async (ctx) => {
|
||||||
const user = requirePermission(ctx, "oauth2-clients");
|
const user = requirePermission(ctx, "oauth2-clients", action);
|
||||||
const hydra = ctx.system?.hydra;
|
const hydra = ctx.system?.hydra;
|
||||||
if (!hydra) return unavailable(ctx, ctx.t("admin.capability.hydra"));
|
if (!hydra) return unavailable(ctx, ctx.t("admin.capability.hydra"));
|
||||||
return inner({ ctx, hydra, user });
|
return inner({ ctx, hydra, user });
|
||||||
@@ -253,24 +257,26 @@ function withClients(inner: (deps: ClientsDeps) => Promise<RouteResult>): RouteH
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Same, plus the target client from ctx.params.id (unknown → themed 404).
|
// Same, plus the target client from ctx.params.id (unknown → themed 404).
|
||||||
function withClient(inner: (deps: ClientsDeps, client: OAuth2Client, id: string) => Promise<RouteResult>): RouteHandler {
|
function withClient(inner: (deps: ClientsDeps, client: OAuth2Client, id: string) => Promise<RouteResult>, action?: AdminAction): RouteHandler {
|
||||||
return withClients(async (deps) => {
|
return withClients(async (deps) => {
|
||||||
const id = deps.ctx.params["id"] ?? "";
|
const id = deps.ctx.params["id"] ?? "";
|
||||||
const client = await deps.hydra.getClient(id);
|
const client = await deps.hydra.getClient(id);
|
||||||
if (!client) return notFound(deps.ctx);
|
if (!client) return notFound(deps.ctx);
|
||||||
return inner(deps, client, id);
|
return inner(deps, client, id);
|
||||||
});
|
}, action);
|
||||||
}
|
}
|
||||||
|
|
||||||
const clientFormResult = (ctx: RequestContext, extra: { error?: string; values?: Partial<ClientInput> }): RouteResult =>
|
const clientFormResult = (ctx: RequestContext, extra: { error?: string; values?: Partial<ClientInput> }): RouteResult =>
|
||||||
({ data: { chrome: ctx.chrome, model: buildClientFormModel({ csrfToken: ctx.chrome.csrfToken, t: ctx.t, ...extra }) }, view: "client-form" });
|
({ data: { chrome: ctx.chrome, model: buildClientFormModel({ csrfToken: ctx.chrome.csrfToken, t: ctx.t, ...extra }) }, view: "client-form" });
|
||||||
|
const canWriteClients = (ctx: RequestContext): boolean => can(ctx, permissionName("oauth2-clients", "write"));
|
||||||
|
|
||||||
const clientDetailResult = (ctx: RequestContext, client: OAuth2Client, extra: { created?: boolean; secret?: string } = {}): RouteResult =>
|
const clientDetailResult = (ctx: RequestContext, client: OAuth2Client, extra: { created?: boolean; secret?: string } = {}): RouteResult =>
|
||||||
({ data: { chrome: ctx.chrome, model: buildClientDetailModel({ client: toClientView(client), csrfToken: ctx.chrome.csrfToken, t: ctx.t, ...extra }) }, view: "client-detail" });
|
({ data: { chrome: ctx.chrome, model: buildClientDetailModel({ canWrite: canWriteClients(ctx), client: toClientView(client), csrfToken: ctx.chrome.csrfToken, t: ctx.t, ...extra }) }, view: "client-detail" });
|
||||||
|
|
||||||
// GET /admin/clients — the list.
|
// GET /admin/clients — the list.
|
||||||
export const clientsList = withClients(async ({ ctx, hydra }) => {
|
export const clientsList = withClients(async ({ ctx, hydra }) => {
|
||||||
const { clients } = await hydra.listClients({ pageSize: LIST_FETCH_SIZE });
|
const { clients } = await hydra.listClients({ pageSize: LIST_FETCH_SIZE });
|
||||||
return { data: { chrome: ctx.chrome, model: buildClientsListModel({ clients, csrfToken: ctx.chrome.csrfToken, t: ctx.t, url: ctx.url }) }, view: "clients" };
|
return { data: { chrome: ctx.chrome, model: buildClientsListModel({ canWrite: canWriteClients(ctx), clients, csrfToken: ctx.chrome.csrfToken, t: ctx.t, url: ctx.url }) }, view: "clients" };
|
||||||
});
|
});
|
||||||
|
|
||||||
// POST /admin/clients — register; on success show the one-time secret directly (no PRG, Hydra never
|
// POST /admin/clients — register; on success show the one-time secret directly (no PRG, Hydra never
|
||||||
@@ -291,7 +297,7 @@ export const clientsCreate = withClients(async ({ ctx, hydra, user }) => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// GET /admin/clients/new — the register form.
|
// GET /admin/clients/new — the register form.
|
||||||
export const clientsNewForm = withClients(({ ctx }) => Promise.resolve(clientFormResult(ctx, {})));
|
export const clientsNewForm = withClients(({ ctx }) => Promise.resolve(clientFormResult(ctx, {})), "write");
|
||||||
|
|
||||||
// GET /admin/clients/:id — the detail (read-only; the secret is shown only once, at creation).
|
// GET /admin/clients/:id — the detail (read-only; the secret is shown only once, at creation).
|
||||||
export const clientsDetail = withClient((deps, client) => Promise.resolve(clientDetailResult(deps.ctx, client)));
|
export const clientsDetail = withClient((deps, client) => Promise.resolve(clientDetailResult(deps.ctx, client)));
|
||||||
@@ -306,7 +312,7 @@ export const clientsDeleteConfirm = withClient((deps, client, id) => {
|
|||||||
cancelHref: base, confirmAction: `${base}/delete`, confirmLabel: tt("admin.clients.delete"),
|
cancelHref: base, confirmAction: `${base}/delete`, confirmLabel: tt("admin.clients.delete"),
|
||||||
message: tt("admin.clients.deleteMessage", { name }), title: tt("admin.clients.delete"),
|
message: tt("admin.clients.deleteMessage", { name }), title: tt("admin.clients.delete"),
|
||||||
}) }, view: "confirm" });
|
}) }, view: "confirm" });
|
||||||
});
|
}, "write");
|
||||||
|
|
||||||
// POST /admin/clients/:id/delete — perform it.
|
// POST /admin/clients/:id/delete — perform it.
|
||||||
export const clientsDelete = withClient(async ({ ctx, hydra, user }, _client, id) => {
|
export const clientsDelete = withClient(async ({ ctx, hydra, user }, _client, id) => {
|
||||||
|
|||||||
@@ -37,7 +37,7 @@ export async function heldPermissions(keto: KetoClient, subject: GrantSubject):
|
|||||||
// Every declared permission the subject effectively holds — direct grants *plus* anything reached
|
// 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;
|
// 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).
|
// 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<string[]> {
|
export async function effectivePermissions(keto: KetoClient, subject: GrantSubject, declared: readonly PermissionDecl[]): Promise<string[]> {
|
||||||
const held = await Promise.all(declared.map((decl) => keto.check({ namespace: PERMISSION_NS, object: decl.name, relation: GRANTED, ...subject })));
|
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);
|
return declared.filter((_, i) => held[i]).map((decl) => decl.name);
|
||||||
}
|
}
|
||||||
@@ -60,6 +60,9 @@ export interface PermissionPicker {
|
|||||||
hint: string;
|
hint: string;
|
||||||
inheritedNote: string | undefined; // set when at least one choice is group-held, to explain the disabled row
|
inheritedNote: string | undefined; // set when at least one choice is group-held, to explain the disabled row
|
||||||
legend: string;
|
legend: string;
|
||||||
|
// Set for a group: its members hold these transitively, so a change reaches them at their next
|
||||||
|
// re-mint rather than at once. The user picker revokes live tokens, so it says nothing.
|
||||||
|
pending: string | undefined;
|
||||||
readOnly: boolean; // the viewer holds :read but not :write — show the state, offer no save
|
readOnly: boolean; // the viewer holds :read but not :write — show the state, offer no save
|
||||||
submit: string;
|
submit: string;
|
||||||
}
|
}
|
||||||
@@ -69,11 +72,12 @@ export interface PermissionPicker {
|
|||||||
// (grantDiff). An inherited row is disabled, so it never posts and can never be diffed into a revoke.
|
// (grantDiff). An inherited row is disabled, so it never posts and can never be diffed into a revoke.
|
||||||
export function buildPermissionPicker(opts: {
|
export function buildPermissionPicker(opts: {
|
||||||
action: string;
|
action: string;
|
||||||
declared: PermissionDecl[];
|
declared: readonly PermissionDecl[];
|
||||||
direct: string[];
|
direct: string[];
|
||||||
effective?: string[]; // omit when the caller can't resolve group-held grants; then only direct shows
|
effective?: string[]; // omit when the caller can't resolve group-held grants; then only direct shows
|
||||||
readOnly?: boolean;
|
readOnly?: boolean;
|
||||||
t?: Translate;
|
t?: Translate;
|
||||||
|
transitive?: boolean; // a group: its members inherit, so the change lands at their next re-mint
|
||||||
}): PermissionPicker {
|
}): PermissionPicker {
|
||||||
const t = opts.t ?? ((k: string) => k);
|
const t = opts.t ?? ((k: string) => k);
|
||||||
const directSet = new Set(opts.direct);
|
const directSet = new Set(opts.direct);
|
||||||
@@ -92,6 +96,7 @@ export function buildPermissionPicker(opts: {
|
|||||||
hint: t("admin.grants.hint"),
|
hint: t("admin.grants.hint"),
|
||||||
inheritedNote: choices.some((c) => c.inherited) ? t("admin.grants.inherited") : undefined,
|
inheritedNote: choices.some((c) => c.inherited) ? t("admin.grants.inherited") : undefined,
|
||||||
legend: t("admin.grants.legend"),
|
legend: t("admin.grants.legend"),
|
||||||
|
pending: opts.transitive === true ? t("admin.grants.pending") : undefined,
|
||||||
readOnly: opts.readOnly === true,
|
readOnly: opts.readOnly === true,
|
||||||
submit: t("admin.grants.save"),
|
submit: t("admin.grants.save"),
|
||||||
};
|
};
|
||||||
@@ -100,7 +105,7 @@ export function buildPermissionPicker(opts: {
|
|||||||
// What a submitted set changes. Pure so the diff is testable without Keto: only declared names are
|
// 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
|
// 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.
|
// name (left over from an uninstalled plugin) is never silently revoked by an unrelated save.
|
||||||
export function grantDiff(declared: PermissionDecl[], held: string[], wanted: string[]): { grant: string[]; revoke: string[] } {
|
export function grantDiff(declared: readonly PermissionDecl[], held: string[], wanted: string[]): { grant: string[]; revoke: string[] } {
|
||||||
const offered = new Set(declared.map((d) => d.name));
|
const offered = new Set(declared.map((d) => d.name));
|
||||||
const heldSet = new Set(held);
|
const heldSet = new Set(held);
|
||||||
const wantedSet = new Set(wanted.filter((name) => offered.has(name)));
|
const wantedSet = new Set(wanted.filter((name) => offered.has(name)));
|
||||||
|
|||||||
@@ -8,7 +8,7 @@
|
|||||||
|
|
||||||
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 { 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 { 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 { ADMIN_EN, type AdminAction, ADMIN_GROUPS_BASE, buildConfirmModel, guardedForm, notFound, permissionName, requirePermission, unavailable } from "./admin-shared.ts";
|
||||||
import type { FieldConfig } from "./admin-users.ts";
|
import type { FieldConfig } from "./admin-users.ts";
|
||||||
|
|
||||||
const GROUP_NS = "Group";
|
const GROUP_NS = "Group";
|
||||||
@@ -298,9 +298,9 @@ async function groupExists(keto: KetoClient, name: string): Promise<boolean> {
|
|||||||
// below is a thin handler over these.
|
// below is a thin handler over these.
|
||||||
interface GroupsDeps { ctx: RequestContext; keto: KetoClient; kratosAdmin: KratosAdmin; user: User; }
|
interface GroupsDeps { ctx: RequestContext; keto: KetoClient; kratosAdmin: KratosAdmin; user: User; }
|
||||||
|
|
||||||
function withGroups(inner: (deps: GroupsDeps) => Promise<RouteResult>): RouteHandler {
|
function withGroups(inner: (deps: GroupsDeps) => Promise<RouteResult>, action?: AdminAction): RouteHandler {
|
||||||
return async (ctx) => {
|
return async (ctx) => {
|
||||||
const user = requirePermission(ctx, "groups");
|
const user = requirePermission(ctx, "groups", action);
|
||||||
const keto = ctx.system?.keto;
|
const keto = ctx.system?.keto;
|
||||||
const kratosAdmin = ctx.system?.kratosAdmin;
|
const kratosAdmin = ctx.system?.kratosAdmin;
|
||||||
if (!keto || !kratosAdmin) return unavailable(ctx, ctx.t("admin.capability.keto"));
|
if (!keto || !kratosAdmin) return unavailable(ctx, ctx.t("admin.capability.keto"));
|
||||||
@@ -309,12 +309,12 @@ function withGroups(inner: (deps: GroupsDeps) => Promise<RouteResult>): RouteHan
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Same, plus the validated :name from ctx.params (an invalid group name → themed 404).
|
// Same, plus the validated :name from ctx.params (an invalid group name → themed 404).
|
||||||
function withGroupName(inner: (deps: GroupsDeps, name: string) => Promise<RouteResult>): RouteHandler {
|
function withGroupName(inner: (deps: GroupsDeps, name: string) => Promise<RouteResult>, action?: AdminAction): RouteHandler {
|
||||||
return withGroups((deps) => {
|
return withGroups((deps) => {
|
||||||
const name = deps.ctx.params["name"] ?? "";
|
const name = deps.ctx.params["name"] ?? "";
|
||||||
if (!isValidGroupName(name)) return Promise.resolve(notFound(deps.ctx));
|
if (!isValidGroupName(name)) return Promise.resolve(notFound(deps.ctx));
|
||||||
return inner(deps, name);
|
return inner(deps, name);
|
||||||
});
|
}, action);
|
||||||
}
|
}
|
||||||
|
|
||||||
const groupFormResult = async (deps: GroupsDeps, extra: { error?: string; values?: { member?: string; name?: string } }): Promise<RouteResult> => {
|
const groupFormResult = async (deps: GroupsDeps, extra: { error?: string; values?: { member?: string; name?: string } }): Promise<RouteResult> => {
|
||||||
@@ -345,7 +345,7 @@ export const groupsCreate = withGroups(async (deps) => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// GET /admin/groups/new — the create form.
|
// GET /admin/groups/new — the create form.
|
||||||
export const groupsNewForm = withGroups((deps) => groupFormResult(deps, {}));
|
export const groupsNewForm = withGroups((deps) => groupFormResult(deps, {}), "write");
|
||||||
|
|
||||||
// GET /admin/groups/:name — the detail + membership page.
|
// GET /admin/groups/:name — the detail + membership page.
|
||||||
export const groupsDetail = withGroupName(async ({ ctx, keto, kratosAdmin }, name) => {
|
export const groupsDetail = withGroupName(async ({ ctx, keto, kratosAdmin }, name) => {
|
||||||
@@ -360,6 +360,7 @@ export const groupsDetail = withGroupName(async ({ ctx, keto, kratosAdmin }, nam
|
|||||||
effective, // a group nested in another group inherits its permissions too
|
effective, // a group nested in another group inherits its permissions too
|
||||||
readOnly: !can(ctx, permissionName("groups", "write")),
|
readOnly: !can(ctx, permissionName("groups", "write")),
|
||||||
t: ctx.t,
|
t: ctx.t,
|
||||||
|
transitive: true, // members inherit, so a change here lands at their next re-mint, not at once
|
||||||
});
|
});
|
||||||
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" };
|
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" };
|
||||||
});
|
});
|
||||||
@@ -395,7 +396,7 @@ export const groupsDeleteConfirm = withGroupName((deps, name) => {
|
|||||||
cancelHref: base, confirmAction: `${base}/delete`, confirmLabel: tt("admin.groups.delete"),
|
cancelHref: base, confirmAction: `${base}/delete`, confirmLabel: tt("admin.groups.delete"),
|
||||||
message: tt("admin.groups.deleteMessage", { name }), title: tt("admin.groups.delete"),
|
message: tt("admin.groups.deleteMessage", { name }), title: tt("admin.groups.delete"),
|
||||||
}) }, view: "confirm" });
|
}) }, view: "confirm" });
|
||||||
});
|
}, "write");
|
||||||
|
|
||||||
// POST /admin/groups/:name/delete — remove every member tuple (the group ceases to exist).
|
// POST /admin/groups/:name/delete — remove every member tuple (the group ceases to exist).
|
||||||
export const groupsDelete = withGroupName(async ({ ctx, keto, user }, name) => {
|
export const groupsDelete = withGroupName(async ({ ctx, keto, user }, name) => {
|
||||||
|
|||||||
@@ -57,9 +57,13 @@ export const ADMIN_NAV: NavNode = {
|
|||||||
// declares the same permission, so the host enforces it before the handler runs; this is
|
// declares the same permission, so the host enforces it before the handler runs; this is
|
||||||
// defence-in-depth and what a direct unit test relies on. Returns the (non-null) user for the
|
// defence-in-depth and what a direct unit test relies on. Returns the (non-null) user for the
|
||||||
// handler to thread on. GuardError → /login or 403.
|
// handler to thread on. GuardError → /login or 403.
|
||||||
export function requirePermission(ctx: RequestContext, resource: AdminResource): User {
|
// `action` defaults to the method's, and is passed explicitly by a *write-intent GET* — a create form
|
||||||
|
// or a delete-confirm page, whose only purpose is to start a write. Those refuse a reader honestly
|
||||||
|
// instead of rendering a form whose submit would 403; the route table declares the same override, so
|
||||||
|
// the two still cannot disagree.
|
||||||
|
export function requirePermission(ctx: RequestContext, resource: AdminResource, action?: AdminAction): User {
|
||||||
const user = requireSession(ctx); // anonymous → GuardError → /login (return_to kept)
|
const user = requireSession(ctx); // anonymous → GuardError → /login (return_to kept)
|
||||||
const permission = permissionName(resource, actionForMethod(ctx.req.method ?? "GET"));
|
const permission = permissionName(resource, action ?? actionForMethod(ctx.req.method ?? "GET"));
|
||||||
if (!can(ctx, permission)) throw new GuardError(403, `${permission} required`);
|
if (!can(ctx, permission)) throw new GuardError(403, `${permission} required`);
|
||||||
return user;
|
return user;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,7 +6,7 @@
|
|||||||
|
|
||||||
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 { 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 { 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";
|
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 SCHEMA_ID = "default"; // matches kratos.yml identity.default_schema_id
|
||||||
const DEFAULT_PAGE_SIZE = 25;
|
const DEFAULT_PAGE_SIZE = 25;
|
||||||
@@ -283,9 +283,9 @@ interface UsersDeps { ctx: RequestContext; keto: KetoClient | undefined; kratosA
|
|||||||
|
|
||||||
// Resolve the shared deps, then run `inner`. The route's own `permission` already gated at the host;
|
// Resolve the shared deps, then run `inner`. The route's own `permission` already gated at the host;
|
||||||
// `requirePermission` is defence-in-depth and yields the user. GuardError (auth/CSRF) → host maps it.
|
// `requirePermission` is defence-in-depth and yields the user. GuardError (auth/CSRF) → host maps it.
|
||||||
function withUser(inner: (deps: UsersDeps) => Promise<RouteResult>): RouteHandler {
|
function withUser(inner: (deps: UsersDeps) => Promise<RouteResult>, action?: AdminAction): RouteHandler {
|
||||||
return async (ctx) => {
|
return async (ctx) => {
|
||||||
const user = requirePermission(ctx, "users");
|
const user = requirePermission(ctx, "users", action);
|
||||||
const kratosAdmin = ctx.system?.kratosAdmin;
|
const kratosAdmin = ctx.system?.kratosAdmin;
|
||||||
if (!kratosAdmin) return unavailable(ctx, ctx.t("admin.capability.kratos"));
|
if (!kratosAdmin) return unavailable(ctx, ctx.t("admin.capability.kratos"));
|
||||||
return inner({ ctx, keto: ctx.system?.keto, kratosAdmin, revoke: ctx.system?.revoke, user });
|
return inner({ ctx, keto: ctx.system?.keto, kratosAdmin, revoke: ctx.system?.revoke, user });
|
||||||
@@ -294,13 +294,13 @@ function withUser(inner: (deps: UsersDeps) => Promise<RouteResult>): RouteHandle
|
|||||||
|
|
||||||
// Same, plus the target identity from ctx.params.id (unknown id → themed 404). The router already
|
// Same, plus the target identity from ctx.params.id (unknown id → themed 404). The router already
|
||||||
// decoded the id and 404s malformed %-encoding, so no manual decode is needed here.
|
// decoded the id and 404s malformed %-encoding, so no manual decode is needed here.
|
||||||
function withTarget(inner: (deps: UsersDeps, identity: Identity, id: string) => Promise<RouteResult>): RouteHandler {
|
function withTarget(inner: (deps: UsersDeps, identity: Identity, id: string) => Promise<RouteResult>, action?: AdminAction): RouteHandler {
|
||||||
return withUser(async (deps) => {
|
return withUser(async (deps) => {
|
||||||
const id = deps.ctx.params["id"] ?? "";
|
const id = deps.ctx.params["id"] ?? "";
|
||||||
const identity = await deps.kratosAdmin.getIdentity(id);
|
const identity = await deps.kratosAdmin.getIdentity(id);
|
||||||
if (!identity) return notFound(deps.ctx);
|
if (!identity) return notFound(deps.ctx);
|
||||||
return inner(deps, identity, id);
|
return inner(deps, identity, id);
|
||||||
});
|
}, action);
|
||||||
}
|
}
|
||||||
|
|
||||||
const formResult = (ctx: RequestContext, extra: Parameters<typeof buildUserFormModel>[0]): RouteResult =>
|
const formResult = (ctx: RequestContext, extra: Parameters<typeof buildUserFormModel>[0]): RouteResult =>
|
||||||
@@ -326,7 +326,7 @@ export const usersCreate = withUser(async ({ ctx, kratosAdmin, user }) => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// GET /admin/users/new — the empty create form.
|
// GET /admin/users/new — the empty create form.
|
||||||
export const usersNewForm = withUser(({ ctx }) => Promise.resolve(formResult(ctx, {})));
|
export const usersNewForm = withUser(({ ctx }) => Promise.resolve(formResult(ctx, {})), "write");
|
||||||
|
|
||||||
// GET /admin/users/:id — the edit form, prefilled.
|
// GET /admin/users/:id — the edit form, prefilled.
|
||||||
export const usersEditForm = withTarget(async (deps, identity, id) => {
|
export const usersEditForm = withTarget(async (deps, identity, id) => {
|
||||||
@@ -372,6 +372,7 @@ export const usersPermissions = withTarget(async (deps, identity, id) => {
|
|||||||
// remove the last `users:write` on the deployment, and the instant-revoke hook lands it on the very
|
// 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.
|
// next request. Recovery would be a curl against Keto — not something the operator persona can do.
|
||||||
if (id === user.id && diff.revoke.length > 0) {
|
if (id === user.id && diff.revoke.length > 0) {
|
||||||
|
ctx.log.warn("admin: refused a self-revoke of permissions", { actor: user.id, refused: diff.revoke.join(",") });
|
||||||
const permissions = await userPermissionPicker(deps, id, ctx.t("admin.grants.selfRevoke"));
|
const permissions = await userPermissionPicker(deps, id, ctx.t("admin.grants.selfRevoke"));
|
||||||
return { ...formResult(ctx, { canWrite: canWriteUsers(ctx), identity, ...(permissions ? { permissions } : {}) }), status: 400 };
|
return { ...formResult(ctx, { canWrite: canWriteUsers(ctx), identity, ...(permissions ? { permissions } : {}) }), status: 400 };
|
||||||
}
|
}
|
||||||
@@ -384,12 +385,14 @@ export const usersPermissions = withTarget(async (deps, identity, id) => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// POST /admin/users/:id — save edits; a Kratos 4xx re-renders the form (400).
|
// POST /admin/users/:id — save edits; a Kratos 4xx re-renders the form (400).
|
||||||
export const usersUpdate = withTarget(async ({ ctx, kratosAdmin }, identity, id) => {
|
export const usersUpdate = withTarget(async (deps, identity, id) => {
|
||||||
|
const { ctx, kratosAdmin } = deps;
|
||||||
const input = readUserInput((await guardedForm(ctx))!);
|
const input = readUserInput((await guardedForm(ctx))!);
|
||||||
try {
|
try {
|
||||||
await kratosAdmin.updateIdentity(id, updateIdentityPayload(identity, input));
|
await kratosAdmin.updateIdentity(id, updateIdentityPayload(identity, input));
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (err instanceof KratosError) return { ...formResult(ctx, { error: ctx.t("admin.users.error.save"), identity }), status: 400 };
|
// Re-render with the picker, or the permissions section vanishes off the page on a failed save.
|
||||||
|
if (err instanceof KratosError) return { ...formResult(ctx, { canWrite: canWriteUsers(ctx), error: ctx.t("admin.users.error.save"), identity, ...(await pickerOrNothing(deps, id)) }), status: 400 };
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
return { redirect: `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}` };
|
return { redirect: `${ADMIN_USERS_BASE}/${encodeURIComponent(id)}` };
|
||||||
@@ -418,7 +421,7 @@ export const usersDeleteConfirm = withTarget((deps, identity, id) => {
|
|||||||
cancelHref: back, confirmAction: `${back}/delete`, confirmLabel: tt("admin.users.delete"),
|
cancelHref: back, confirmAction: `${back}/delete`, confirmLabel: tt("admin.users.delete"),
|
||||||
message: tt("admin.users.deleteMessage", { email: view.email }), title: tt("admin.users.delete"),
|
message: tt("admin.users.deleteMessage", { email: view.email }), title: tt("admin.users.delete"),
|
||||||
}) }, view: "confirm" });
|
}) }, view: "confirm" });
|
||||||
});
|
}, "write");
|
||||||
|
|
||||||
// POST /admin/users/:id/delete — perform it; revoke the gone account's live tokens. Refuses self-delete.
|
// POST /admin/users/:id/delete — perform it; revoke the gone account's live tokens. Refuses self-delete.
|
||||||
export const usersDelete = withTarget(async ({ ctx, kratosAdmin, revoke, user }, identity, id) => {
|
export const usersDelete = withTarget(async ({ ctx, kratosAdmin, revoke, user }, identity, id) => {
|
||||||
@@ -431,12 +434,19 @@ export const usersDelete = withTarget(async ({ ctx, kratosAdmin, revoke, user },
|
|||||||
});
|
});
|
||||||
|
|
||||||
// POST /admin/users/:id/recovery — mint a one-time recovery code, shown on the edit page.
|
// POST /admin/users/:id/recovery — mint a one-time recovery code, shown on the edit page.
|
||||||
export const usersRecovery = withTarget(async ({ ctx, kratosAdmin }, identity, id) => {
|
export const usersRecovery = withTarget(async (deps, identity, id) => {
|
||||||
|
const { ctx, kratosAdmin } = deps;
|
||||||
await guardedForm(ctx); // CSRF-verify the POST
|
await guardedForm(ctx); // CSRF-verify the POST
|
||||||
const recovery = await kratosAdmin.createRecoveryCode(id);
|
const recovery = await kratosAdmin.createRecoveryCode(id);
|
||||||
return formResult(ctx, { identity, recovery });
|
return formResult(ctx, { canWrite: canWriteUsers(ctx), identity, recovery, ...(await pickerOrNothing(deps, id)) });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The picker as a spreadable fragment, so a re-render never silently drops the section.
|
||||||
|
async function pickerOrNothing(deps: UsersDeps, id: string): Promise<{ permissions?: PermissionPicker }> {
|
||||||
|
const permissions = await userPermissionPicker(deps, id);
|
||||||
|
return permissions ? { permissions } : {};
|
||||||
|
}
|
||||||
|
|
||||||
function createError(err: KratosError, t: Translate): string {
|
function createError(err: KratosError, t: Translate): string {
|
||||||
return err.status === 409
|
return err.status === 409
|
||||||
? t("admin.users.error.duplicate")
|
? t("admin.users.error.duplicate")
|
||||||
|
|||||||
@@ -51,8 +51,12 @@ test("every declared permission is <resource>:<action>, and reads and writes are
|
|||||||
});
|
});
|
||||||
|
|
||||||
test("GET routes gate on read and mutations on write, so a reader can open a screen but not change it", () => {
|
test("GET routes gate on read and mutations on write, so a reader can open a screen but not change it", () => {
|
||||||
|
// …except a write-intent GET — a create form or a delete-confirm page, which exists only to start a
|
||||||
|
// write. Those gate on `:write` so a reader is refused there rather than at the submit.
|
||||||
|
const writeIntent = (path: string): boolean => path.endsWith("/new") || path.endsWith("/delete");
|
||||||
for (const route of routes) {
|
for (const route of routes) {
|
||||||
const action = route.method === "GET" ? "read" : "write";
|
const action = route.method === "GET" && !writeIntent(route.path) ? "read" : "write";
|
||||||
assert.ok(route.permission?.endsWith(`:${action}`), `${route.method} ${route.path} → ${route.permission}`);
|
assert.ok(route.permission?.endsWith(`:${action}`), `${route.method} ${route.path} → ${route.permission}`);
|
||||||
}
|
}
|
||||||
|
assert.equal(routes.filter((r) => r.method === "GET" && writeIntent(r.path)).length, 6); // 2 per screen
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -10,15 +10,18 @@ import { definePlugin, type HttpMethod, type Route, type RouteHandler } from "#p
|
|||||||
import { clientsCreate, clientsDeleteConfirm, clientsDelete, clientsDetail, clientsList, clientsNewForm } from "./admin-clients.ts";
|
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, groupsRemoveMember } from "./admin-groups.ts";
|
||||||
import { usersCreate, usersDeleteConfirm, usersDelete, usersEditForm, usersList, usersNewForm, usersPermissions, usersRecovery, usersState, usersUpdate } from "./admin-users.ts";
|
import { usersCreate, usersDeleteConfirm, usersDelete, usersEditForm, usersList, usersNewForm, usersPermissions, usersRecovery, usersState, usersUpdate } from "./admin-users.ts";
|
||||||
import { ADMIN_NAV, actionForMethod, type AdminResource, permissionName } from "./admin-shared.ts";
|
import { ADMIN_NAV, actionForMethod, type AdminAction, type AdminResource, permissionName } from "./admin-shared.ts";
|
||||||
|
|
||||||
// One route factory per screen: a GET gates on `<resource>:read` and a POST on `<resource>:write`,
|
// One route factory per screen: a GET gates on `<resource>:read` and a POST on `<resource>:write`,
|
||||||
// derived through the same two helpers the in-handler guard uses, so the table below cannot drift
|
// derived through the same two helpers the in-handler guard uses, so the table below cannot drift
|
||||||
// from it. The host redirects an anonymous visitor to /login, gives a signed-in user missing the
|
// from it. The host redirects an anonymous visitor to /login, gives a signed-in user missing the
|
||||||
// permission the 403 page, and filters the nav the same way. Handlers are thin and keyed on
|
// permission the 403 page, and filters the nav the same way. Handlers are thin and keyed on
|
||||||
// ctx.params (the host extracts :id / :name), the idiomatic per-route style.
|
// ctx.params (the host extracts :id / :name), the idiomatic per-route style.
|
||||||
const on = (resource: AdminResource) => (method: HttpMethod, path: string, handler: RouteHandler): Route =>
|
// `action` overrides the method's default for a *write-intent GET* — a create form or a
|
||||||
({ handler, method, path, permission: permissionName(resource, actionForMethod(method)) });
|
// delete-confirm page, which exists only to start a write and so refuses a reader rather than
|
||||||
|
// rendering a form whose submit would 403. The handler's own guard takes the same override.
|
||||||
|
const on = (resource: AdminResource) => (method: HttpMethod, path: string, handler: RouteHandler, action?: AdminAction): Route =>
|
||||||
|
({ handler, method, path, permission: permissionName(resource, action ?? actionForMethod(method)) });
|
||||||
|
|
||||||
const users = on("users");
|
const users = on("users");
|
||||||
const groups = on("groups");
|
const groups = on("groups");
|
||||||
@@ -42,30 +45,30 @@ export default definePlugin({
|
|||||||
// Users
|
// Users
|
||||||
users("GET", "/users", usersList),
|
users("GET", "/users", usersList),
|
||||||
users("POST", "/users", usersCreate),
|
users("POST", "/users", usersCreate),
|
||||||
users("GET", "/users/new", usersNewForm),
|
users("GET", "/users/new", usersNewForm, "write"),
|
||||||
users("GET", "/users/:id", usersEditForm),
|
users("GET", "/users/:id", usersEditForm),
|
||||||
users("POST", "/users/:id", usersUpdate),
|
users("POST", "/users/:id", usersUpdate),
|
||||||
users("POST", "/users/:id/state", usersState),
|
users("POST", "/users/:id/state", usersState),
|
||||||
users("GET", "/users/:id/delete", usersDeleteConfirm),
|
users("GET", "/users/:id/delete", usersDeleteConfirm, "write"),
|
||||||
users("POST", "/users/:id/delete", usersDelete),
|
users("POST", "/users/:id/delete", usersDelete),
|
||||||
users("POST", "/users/:id/recovery", usersRecovery),
|
users("POST", "/users/:id/recovery", usersRecovery),
|
||||||
users("POST", "/users/:id/permissions", usersPermissions),
|
users("POST", "/users/:id/permissions", usersPermissions),
|
||||||
// Groups
|
// Groups
|
||||||
groups("GET", "/groups", groupsList),
|
groups("GET", "/groups", groupsList),
|
||||||
groups("POST", "/groups", groupsCreate),
|
groups("POST", "/groups", groupsCreate),
|
||||||
groups("GET", "/groups/new", groupsNewForm),
|
groups("GET", "/groups/new", groupsNewForm, "write"),
|
||||||
groups("GET", "/groups/:name", groupsDetail),
|
groups("GET", "/groups/:name", groupsDetail),
|
||||||
groups("POST", "/groups/:name/members", groupsAddMember),
|
groups("POST", "/groups/:name/members", groupsAddMember),
|
||||||
groups("GET", "/groups/:name/delete", groupsDeleteConfirm),
|
groups("GET", "/groups/:name/delete", groupsDeleteConfirm, "write"),
|
||||||
groups("POST", "/groups/:name/delete", groupsDelete),
|
groups("POST", "/groups/:name/delete", groupsDelete),
|
||||||
groups("POST", "/groups/:name/members/delete", groupsRemoveMember),
|
groups("POST", "/groups/:name/members/delete", groupsRemoveMember),
|
||||||
groups("POST", "/groups/:name/permissions", groupsPermissions),
|
groups("POST", "/groups/:name/permissions", groupsPermissions),
|
||||||
// OAuth2 clients
|
// OAuth2 clients
|
||||||
clients("GET", "/clients", clientsList),
|
clients("GET", "/clients", clientsList),
|
||||||
clients("POST", "/clients", clientsCreate),
|
clients("POST", "/clients", clientsCreate),
|
||||||
clients("GET", "/clients/new", clientsNewForm),
|
clients("GET", "/clients/new", clientsNewForm, "write"),
|
||||||
clients("GET", "/clients/:id", clientsDetail),
|
clients("GET", "/clients/:id", clientsDetail),
|
||||||
clients("GET", "/clients/:id/delete", clientsDeleteConfirm),
|
clients("GET", "/clients/:id/delete", clientsDeleteConfirm, "write"),
|
||||||
clients("POST", "/clients/:id/delete", clientsDelete),
|
clients("POST", "/clients/:id/delete", clientsDelete),
|
||||||
],
|
],
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -3,7 +3,7 @@
|
|||||||
shell. Doubles as the post-register page when `created`/`secret` are set.
|
shell. Doubles as the post-register page when `created`/`secret` are set.
|
||||||
%><%
|
%><%
|
||||||
const nav = include("partials/nav-tree", { nodes: chrome.nav });
|
const nav = include("partials/nav-tree", { nodes: chrome.nav });
|
||||||
const body = include("partials/client-detail-body", { client: model.client, created: model.created, csrfToken: chrome.csrfToken, del: model.delete, secret: model.secret });
|
const body = include("partials/client-detail-body", { canWrite: model.canWrite, client: model.client, created: model.created, csrfToken: chrome.csrfToken, del: model.delete, secret: model.secret });
|
||||||
-%>
|
-%>
|
||||||
<%- include("partials/shell", {
|
<%- include("partials/shell", {
|
||||||
body,
|
body,
|
||||||
|
|||||||
@@ -6,7 +6,8 @@
|
|||||||
const filters = include("partials/filter-bar", model.filterBar);
|
const filters = include("partials/filter-bar", model.filterBar);
|
||||||
const table = include("partials/data-table", model.table);
|
const table = include("partials/data-table", model.table);
|
||||||
const pager = include("partials/pagination", model.pagination);
|
const pager = include("partials/pagination", model.pagination);
|
||||||
const actions = '<a class="btn btn-primary" href="' + localeHref("/admin/clients/new") + '"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-plus"/></svg>' + t("admin.clients.registerClient") + '</a>';
|
// Only offer "Register client" to an oauth2-clients:write holder — a :read one would get the 403 page.
|
||||||
|
const actions = model.canWrite === false ? "" : '<a class="btn btn-primary" href="' + localeHref("/admin/clients/new") + '"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-plus"/></svg>' + t("admin.clients.registerClient") + '</a>';
|
||||||
-%>
|
-%>
|
||||||
<%- include("partials/shell", {
|
<%- include("partials/shell", {
|
||||||
actions,
|
actions,
|
||||||
|
|||||||
@@ -31,8 +31,10 @@
|
|||||||
<dt><%= t("admin.clients.field.redirectUris") %></dt><dd><% if (c.redirectUris.length) { %><ul class="plain-list"><% c.redirectUris.forEach((u) => { %><li><%= u %></li><% }) %></ul><% } else { %>—<% } %></dd>
|
<dt><%= t("admin.clients.field.redirectUris") %></dt><dd><% if (c.redirectUris.length) { %><ul class="plain-list"><% c.redirectUris.forEach((u) => { %><li><%= u %></li><% }) %></ul><% } else { %>—<% } %></dd>
|
||||||
</dl>
|
</dl>
|
||||||
</section>
|
</section>
|
||||||
|
<% if (locals.canWrite !== false) { -%>
|
||||||
<section class="form-card admin-actions" aria-label="<%= t("admin.clients.title") %>">
|
<section class="form-card admin-actions" aria-label="<%= t("admin.clients.title") %>">
|
||||||
<p class="field-hint"><%= t("admin.clients.rereg") %></p>
|
<p class="field-hint"><%= t("admin.clients.rereg") %></p>
|
||||||
<a class="btn btn-danger" href="<%= localeHref(del.action) %>"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-trash"/></svg><%= t("admin.clients.delete") %></a>
|
<a class="btn btn-danger" href="<%= localeHref(del.action) %>"><svg class="ico ico-sm" aria-hidden="true"><use href="#i-trash"/></svg><%= t("admin.clients.delete") %></a>
|
||||||
</section>
|
</section>
|
||||||
|
<% } -%>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -42,5 +42,8 @@
|
|||||||
<% if (permissions.inheritedNote) { -%>
|
<% if (permissions.inheritedNote) { -%>
|
||||||
<p class="cell-muted"><%= permissions.inheritedNote %></p>
|
<p class="cell-muted"><%= permissions.inheritedNote %></p>
|
||||||
<% } -%>
|
<% } -%>
|
||||||
|
<% if (permissions.pending) { -%>
|
||||||
|
<p class="cell-muted"><%= permissions.pending %></p>
|
||||||
|
<% } -%>
|
||||||
<% } -%>
|
<% } -%>
|
||||||
</section>
|
</section>
|
||||||
|
|||||||
+19
-2
@@ -1110,7 +1110,7 @@ test("admin Users screen: gate, list/filter, create, edit, deactivate, delete, r
|
|||||||
|
|
||||||
// Nav: the admin plugin's section composes into the one global menu, and each screen is filtered
|
// Nav: the admin plugin's section composes into the one global menu, and each screen is filtered
|
||||||
// by its own read permission — proving the drop-in nav fragment. A user holding only users:read
|
// by its own read permission — proving the drop-in nav fragment. A user holding only users:read
|
||||||
// sees Users and nothing else; holding none of the four, composeNav drops the emptied header.
|
// sees Users and nothing else; holding none of the three, composeNav drops the emptied header.
|
||||||
assert.match(await (await get("/dashboard")).text(), /href="\/admin\/users"/);
|
assert.match(await (await get("/dashboard")).text(), /href="\/admin\/users"/);
|
||||||
const usersOnlyNav = await (await get("/dashboard", ["users:read"])).text();
|
const usersOnlyNav = await (await get("/dashboard", ["users:read"])).text();
|
||||||
assert.match(usersOnlyNav, /href="\/admin\/users"/);
|
assert.match(usersOnlyNav, /href="\/admin\/users"/);
|
||||||
@@ -1317,7 +1317,10 @@ test("admin screens render no write affordance for a read-only holder", async (t
|
|||||||
const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }];
|
const identities: Identity[] = [{ id: ada, traits: { email: "ada@example.com" } }];
|
||||||
const keto = fakeKeto([{ namespace: "Group", object: "eng", relation: "members", subject_id: `user:${ada}` }]);
|
const keto = fakeKeto([{ 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 kratosAdmin = stubAdmin({ getIdentity: async (id) => identities.find((i) => i.id === id) ?? null, listIdentities: async () => ({ identities, nextPageToken: null }) });
|
||||||
const { get } = await adminHarness(t, { keto, kratosAdmin });
|
// Hydra is wired so the clients screen renders for real — without it the page is a 503 and the
|
||||||
|
// "no Register button" assertion below would pass without proving anything.
|
||||||
|
const hydra = stubHydra({ listClients: async () => ({ clients: [{ client_id: "existing", client_name: "Reporting" }], nextPageToken: null }) });
|
||||||
|
const { get } = await adminHarness(t, { hydra, keto, kratosAdmin });
|
||||||
const readOnly = ["users:read", "groups:read"];
|
const readOnly = ["users:read", "groups:read"];
|
||||||
|
|
||||||
const list = await (await get("/admin/users", readOnly)).text();
|
const list = await (await get("/admin/users", readOnly)).text();
|
||||||
@@ -1337,6 +1340,20 @@ test("admin screens render no write affordance for a read-only holder", async (t
|
|||||||
assert.doesNotMatch(group, /Delete group/);
|
assert.doesNotMatch(group, /Delete group/);
|
||||||
assert.doesNotMatch(group, /Save permissions/);
|
assert.doesNotMatch(group, /Save permissions/);
|
||||||
|
|
||||||
|
// The OAuth2-clients screen is held to the same rule (it was the one this test was written to catch).
|
||||||
|
const clientsRes = await get("/admin/clients", ["oauth2-clients:read"]);
|
||||||
|
assert.equal(clientsRes.status, 200); // a real render, not the capability-missing 503
|
||||||
|
const clients = await clientsRes.text();
|
||||||
|
assert.match(clients, /Reporting/); // the list is there — that's what :read buys
|
||||||
|
assert.doesNotMatch(clients, /href="\/admin\/clients\/new"/);
|
||||||
|
|
||||||
|
// A write-intent GET — a create form or a delete-confirm — refuses a reader outright rather than
|
||||||
|
// rendering a form whose submit would 403.
|
||||||
|
for (const path of ["/admin/users/new", "/admin/groups/new", `/admin/users/${ada}/delete`, "/admin/groups/eng/delete"]) {
|
||||||
|
assert.equal((await get(path, readOnly)).status, 403, path);
|
||||||
|
}
|
||||||
|
assert.equal((await get("/admin/clients/new", ["oauth2-clients:read"])).status, 403);
|
||||||
|
|
||||||
// A writer sees the affordances the reader didn't.
|
// A writer sees the affordances the reader didn't.
|
||||||
const writable = await (await get(`/admin/users/${ada}`, ["users:read", "users:write"])).text();
|
const writable = await (await get(`/admin/users/${ada}`, ["users:read", "users:write"])).text();
|
||||||
assert.match(writable, /Save changes/);
|
assert.match(writable, /Save changes/);
|
||||||
|
|||||||
+2
-2
@@ -41,7 +41,7 @@ export interface RequestContext {
|
|||||||
// Every permission the installed plugins declare, deduped and sorted — the fixed list an admin
|
// Every permission the installed plugins declare, deduped and sorted — the fixed list an admin
|
||||||
// screen offers when granting one. Pairs with `permissions` below: this is what *exists*, that is
|
// screen offers when granting one. Pairs with `permissions` below: this is what *exists*, that is
|
||||||
// what *this user holds*. Empty when no installed plugin declares any.
|
// what *this user holds*. Empty when no installed plugin declares any.
|
||||||
declaredPermissions: PermissionDecl[];
|
declaredPermissions: readonly PermissionDecl[];
|
||||||
params: Record<string, string>; // path params from the route match, e.g. /users/:id → { id }
|
params: Record<string, string>; // path params from the route match, e.g. /users/:id → { id }
|
||||||
permissions: string[]; // user?.permissions ?? [] — coarse gate without a null-check
|
permissions: string[]; // user?.permissions ?? [] — coarse gate without a null-check
|
||||||
query: URLSearchParams; // alias of url.searchParams, for ctx.query.get("q")
|
query: URLSearchParams; // alias of url.searchParams, for ctx.query.get("q")
|
||||||
@@ -66,7 +66,7 @@ export interface BuildContextOptions {
|
|||||||
// ctx.chrome (a json/redirect handler, or the public "/" with a standalone home, pays nothing).
|
// ctx.chrome (a json/redirect handler, or the public "/" with a standalone home, pays nothing).
|
||||||
// The host's factory is memoised, so the menu composes at most once per request across contexts.
|
// The host's factory is memoised, so the menu composes at most once per request across contexts.
|
||||||
chrome?: () => PageChrome;
|
chrome?: () => PageChrome;
|
||||||
declaredPermissions?: PermissionDecl[];
|
declaredPermissions?: readonly PermissionDecl[];
|
||||||
user?: User | null;
|
user?: User | null;
|
||||||
locale?: string;
|
locale?: string;
|
||||||
localeHref?: (href: string) => string;
|
localeHref?: (href: string) => string;
|
||||||
|
|||||||
@@ -4,6 +4,7 @@
|
|||||||
|
|
||||||
- [ ] Add a way to configure plugins directly when installing. Most reasonable is an .env file in the plugin folder, I think, but I am open to suggestions.
|
- [ ] Add a way to configure plugins directly when installing. Most reasonable is an .env file in the plugin folder, I think, but I am open to suggestions.
|
||||||
- [ ] Rename the plugin "admin" to something less generic, like "auth-admin" or "users-groups-admin".
|
- [ ] Rename the plugin "admin" to something less generic, like "auth-admin" or "users-groups-admin".
|
||||||
|
- [ ] Guard the group paths to self-lockout, or accept them explicitly. The self-revoke guard covers only your own *direct* grants on the Users screen; unticking a permission on a group you belong to, removing yourself from that group, or deleting it can all still strip your own effective access with no warning. Same scope the deleted Permissions screen had, and recorded in AGENTS.md as a known gap — the robust fix is a "last effective holder" check, which needs a reverse Keto query. Raised by the stability review 2026-08-05.
|
||||||
- [ ] The permission picker has no concurrency baseline, so two operators editing the same user/group silently discard each other's change (standard lost-update on a set-based form — and the natural "two of us are onboarding the new hire" workflow produces exactly it). Sketch: post the rendered set as a hidden baseline; if it no longer matches Keto, re-render with "this changed while you had the page open" rather than applying. Fits the existing "the form is the whole truth" model instead of fighting it. Raised by the product review 2026-08-05.
|
- [ ] The permission picker has no concurrency baseline, so two operators editing the same user/group silently discard each other's change (standard lost-update on a set-based form — and the natural "two of us are onboarding the new hire" workflow produces exactly it). Sketch: post the rendered set as a hidden baseline; if it no longer matches Keto, re-render with "this changed while you had the page open" rather than applying. Fits the existing "the form is the whole truth" model instead of fighting it. Raised by the product review 2026-08-05.
|
||||||
- [ ] A grant whose plugin was uninstalled is invisible and unremovable in the GUI. `grantDiff` deliberately never revokes an undeclared name (so an unrelated save can't drop it), but nothing *shows* it either — so it can't be audited or cleaned, and reinstalling that plugin silently reactivates access nobody remembers granting. Sketch: a read-only "held, but no installed plugin offers this" list with a remove action. Raised by the product review 2026-08-05.
|
- [ ] A grant whose plugin was uninstalled is invisible and unremovable in the GUI. `grantDiff` deliberately never revokes an undeclared name (so an unrelated save can't drop it), but nothing *shows* it either — so it can't be audited or cleaned, and reinstalling that plugin silently reactivates access nobody remembers granting. Sketch: a read-only "held, but no installed plugin offers this" list with a remove action. Raised by the product review 2026-08-05.
|
||||||
- [ ] A plugin may gate a route on a permission it never declares — declaring stays optional on purpose (mandatory declaration would warn on the legitimate cross-plugin sharing case). The cost is a dead end: the picker is built from declarations only, so that route is ungrantable from the GUI with no boot error, no warning, and a permanent 403 as the operator's only clue. Sketch: a discovery *warning* (not an error) naming the gated-but-undeclared permission. Raised by the product review 2026-08-05.
|
- [ ] A plugin may gate a route on a permission it never declares — declaring stays optional on purpose (mandatory declaration would warn on the legitimate cross-plugin sharing case). The cost is a dead end: the picker is built from declarations only, so that route is ungrantable from the GUI with no boot error, no warning, and a permanent 403 as the operator's only clue. Sketch: a discovery *warning* (not an error) naming the gated-but-undeclared permission. Raised by the product review 2026-08-05.
|
||||||
|
|||||||
Reference in New Issue
Block a user