Rename the coarse gate from role to permission, matching RBAC
This commit is contained in:
@@ -50,8 +50,8 @@ const badCases: Array<{ name: string; files: Record<string, string>; match: RegE
|
||||
{ name: "non-function dashboard", files: { "weirddash/plugin.ts": `export default { apiVersion: "1.0.0", dashboard: "nope" };` }, match: /weirddash.*dashboard.*function/s },
|
||||
{ name: "reserved dashboard id shadows the gated dashboard", files: { "dashboard/plugin.ts": full("dashboard") }, match: /dashboard.*reserved/s },
|
||||
{ name: "duplicate nav id across plugins", files: { "a/plugin.ts": full("a").replace("a:root", "dup"), "b/plugin.ts": full("b").replace("b:root", "dup") }, match: /nav id "dup"/ },
|
||||
{ name: "a route marked public AND role is contradictory", files: { "contra/plugin.ts": `export default { apiVersion: "1.0.0", routes: [{ method: "GET", path: "/", public: true, role: "x", handler: () => ({ html: "x" }) }] };` }, match: /contra.*public.*role/s },
|
||||
{ name: "a nav node marked public AND role is contradictory", files: { "contranav/plugin.ts": `export default { apiVersion: "1.0.0", nav: [{ id: "n", label: "N", public: true, role: "x" }] };` }, match: /contranav.*public.*role/s },
|
||||
{ name: "a route marked public AND permission is contradictory", files: { "contra/plugin.ts": `export default { apiVersion: "1.0.0", routes: [{ method: "GET", path: "/", public: true, permission: "x", handler: () => ({ html: "x" }) }] };` }, match: /contra.*public.*permission/s },
|
||||
{ name: "a nav node marked public AND permission is contradictory", files: { "contranav/plugin.ts": `export default { apiVersion: "1.0.0", nav: [{ id: "n", label: "N", public: true, permission: "x" }] };` }, match: /contranav.*public.*permission/s },
|
||||
{ name: "two plugins claim the public home", files: { "a/plugin.ts": `export default { apiVersion: "1.0.0", home: () => ({ html: "a" }) };`, "b/plugin.ts": `export default { apiVersion: "1.0.0", home: () => ({ html: "b" }) };` }, match: /home/ },
|
||||
{ name: "two plugins claim the gated dashboard", files: { "a/plugin.ts": `export default { apiVersion: "1.0.0", dashboard: () => ({ html: "a" }) };`, "b/plugin.ts": `export default { apiVersion: "1.0.0", dashboard: () => ({ html: "b" }) };` }, match: /dashboard/ },
|
||||
];
|
||||
@@ -85,12 +85,12 @@ test("a plugin may declare `home` (public /) and `dashboard` (gated /dashboard)
|
||||
assert.equal(typeof plugins[0]?.dashboard, "function");
|
||||
});
|
||||
|
||||
test("a shared role name only warns — both plugins still load", async (t) => {
|
||||
const shared = `export default { apiVersion: "1.0.0", roles: [{ name: "shared:read" }] };`;
|
||||
test("a shared permission name only warns — both plugins still load", async (t) => {
|
||||
const shared = `export default { apiVersion: "1.0.0", permissions: [{ name: "shared:read" }] };`;
|
||||
const dir = scaffold(t, { "x/plugin.ts": shared, "y/plugin.ts": shared });
|
||||
const warnings: string[] = [];
|
||||
const plugins = await discoverPlugins({ dir, logger: { warn: (m) => warnings.push(String(m)) } });
|
||||
|
||||
assert.equal(plugins.length, 2);
|
||||
assert.ok(warnings.some((w) => /shared:read/.test(w)), "expected a role-conflict warning");
|
||||
assert.ok(warnings.some((w) => /shared:read/.test(w)), "expected a permission-conflict warning");
|
||||
});
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
// validate it, assemble the loaded Plugin[]. The imperative shell over plugin.ts's pure rules
|
||||
// (isValidPluginId, checkApiVersion, findConflicts). Fails loud: every per-plugin problem and
|
||||
// error-level conflict is collected into one boot-stopping Error; warn-level diagnostics
|
||||
// (older-minor apiVersion, shared role name) log and load continues. Folder name = id.
|
||||
// (older-minor apiVersion, shared permission name) log and load continues. Folder name = id.
|
||||
|
||||
import { existsSync, readdirSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
@@ -85,7 +85,7 @@ function asManifest(value: unknown): PluginManifest | null {
|
||||
|
||||
// The collection fields feed findConflicts, which iterates them — a non-array crashes it opaquely.
|
||||
function shapeError(manifest: PluginManifest): string | null {
|
||||
for (const field of ["nav", "roles", "routes"] as const) {
|
||||
for (const field of ["nav", "permissions", "routes"] as const) {
|
||||
if (manifest[field] !== undefined && !Array.isArray(manifest[field])) return `"${field}" must be an array`;
|
||||
}
|
||||
// `home` / `dashboard` (the landing-page overrides) are route handlers; the host calls them, so
|
||||
@@ -93,20 +93,20 @@ function shapeError(manifest: PluginManifest): string | null {
|
||||
for (const slot of ["home", "dashboard"] as const) {
|
||||
if (manifest[slot] !== undefined && typeof manifest[slot] !== "function") return `"${slot}" must be a function (a route handler)`;
|
||||
}
|
||||
// `public` and `role` are contradictory on the same route/nav node — "open to all" vs
|
||||
// "needs this role". Refuse rather than silently pick one, so the author's intent is unambiguous.
|
||||
// `public` and `permission` are contradictory on the same route/nav node — "open to all" vs
|
||||
// "needs this permission". Refuse rather than silently pick one, so the author's intent is unambiguous.
|
||||
for (const route of Array.isArray(manifest.routes) ? manifest.routes : []) {
|
||||
if (route?.public === true && route.role != null) return `route "${route.method} ${route.path}" sets both public and role — they are mutually exclusive`;
|
||||
if (route?.public === true && route.permission != null) return `route "${route.method} ${route.path}" sets both public and permission — they are mutually exclusive`;
|
||||
}
|
||||
const navContradiction = findPublicNavContradiction(manifest.nav);
|
||||
if (navContradiction) return navContradiction;
|
||||
return null;
|
||||
}
|
||||
|
||||
// Recurse the nav fragment: a node that is both `public` and `role`-gated is contradictory.
|
||||
// Recurse the nav fragment: a node that is both `public` and `permission`-gated is contradictory.
|
||||
function findPublicNavContradiction(nodes: PluginManifest["nav"]): string | null {
|
||||
for (const node of Array.isArray(nodes) ? nodes : []) {
|
||||
if (node?.public === true && node.role != null) return `nav node "${node.label ?? node.id ?? "?"}" sets both public and role — they are mutually exclusive`;
|
||||
if (node?.public === true && node.permission != null) return `nav node "${node.label ?? node.id ?? "?"}" sets both public and permission — they are mutually exclusive`;
|
||||
const inChild = findPublicNavContradiction(node?.children);
|
||||
if (inChild) return inChild;
|
||||
}
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
// a plugin should import from here, never reach into deeper modules. See README.md → Building plugins.
|
||||
|
||||
export { definePlugin } from "./plugin.ts";
|
||||
export type { HttpMethod, Plugin, PluginHooks, PluginManifest, RoleDecl, Route, RouteHandler, RouteResult } from "./plugin.ts";
|
||||
export type { HttpMethod, Plugin, PluginHooks, PluginManifest, PermissionDecl, Route, RouteHandler, RouteResult } from "./plugin.ts";
|
||||
export type { RequestContext, SessionIdentity } from "../http/context.ts";
|
||||
export type { PageChrome } from "../ui/chrome.ts";
|
||||
export type { NavNode } from "../ui/nav.ts";
|
||||
|
||||
@@ -21,13 +21,13 @@ const scheduling: PluginManifest = definePlugin({
|
||||
apiVersion: "1.0.0",
|
||||
hooks: { onBoot: () => {} },
|
||||
nav: [{
|
||||
children: [{ href: "/scheduling/shifts", id: "scheduling:shifts", label: "Shifts", role: "scheduling:read" }],
|
||||
children: [{ href: "/scheduling/shifts", id: "scheduling:shifts", label: "Shifts", permission: "scheduling:read" }],
|
||||
icon: "i-cal", id: "scheduling:root", label: "Scheduling",
|
||||
}],
|
||||
roles: [{ description: "View shifts", name: "scheduling:read" }],
|
||||
permissions: [{ description: "View shifts", name: "scheduling:read" }],
|
||||
routes: [
|
||||
{ handler: () => ({ data: { rows: [] }, view: "shifts" }), method: "GET", path: "/shifts", role: "scheduling:read" },
|
||||
{ handler: () => ({ redirect: "/scheduling/shifts" }), method: "POST", path: "/shifts", role: "scheduling:write" },
|
||||
{ handler: () => ({ data: { rows: [] }, view: "shifts" }), method: "GET", path: "/shifts", permission: "scheduling:read" },
|
||||
{ handler: () => ({ redirect: "/scheduling/shifts" }), method: "POST", path: "/shifts", permission: "scheduling:write" },
|
||||
{ handler: (ctx) => void ctx.res.end("raw"), method: "GET", path: "/raw" }, // void = handler wrote res itself
|
||||
],
|
||||
});
|
||||
@@ -87,19 +87,19 @@ test("findConflicts: a duplicate id and a colliding route are loud errors", () =
|
||||
assert.ok(dupRoute.some((c) => c.kind === "route" && c.level === "error" && c.message.includes("/a/t")));
|
||||
});
|
||||
|
||||
test("findConflicts: duplicate nav id is an error, a shared role name only warns", () => {
|
||||
test("findConflicts: duplicate nav id is an error, a shared permission name only warns", () => {
|
||||
const navDup = findConflicts([
|
||||
p({ id: "a", nav: [{ id: "dup", label: "A" }] }),
|
||||
p({ id: "b", nav: [{ id: "dup", label: "B" }] }),
|
||||
]);
|
||||
assert.ok(navDup.some((c) => c.kind === "nav-id" && c.level === "error" && c.plugins.includes("a") && c.plugins.includes("b")));
|
||||
|
||||
// Sharing a role across plugins is legitimate → warn, not error.
|
||||
const roleDup = findConflicts([
|
||||
p({ id: "a", roles: [{ name: "shared:read" }] }),
|
||||
p({ id: "b", roles: [{ name: "shared:read" }] }),
|
||||
// Sharing a permission across plugins is legitimate → warn, not error.
|
||||
const permissionDup = findConflicts([
|
||||
p({ id: "a", permissions: [{ name: "shared:read" }] }),
|
||||
p({ id: "b", permissions: [{ name: "shared:read" }] }),
|
||||
]);
|
||||
assert.ok(roleDup.some((c) => c.kind === "role" && c.level === "warn"));
|
||||
assert.ok(permissionDup.some((c) => c.kind === "permission" && c.level === "warn"));
|
||||
});
|
||||
|
||||
test("findConflicts: each single slot (`home`/`dashboard`) may have one owner — two is a loud error", () => {
|
||||
|
||||
+10
-10
@@ -29,16 +29,16 @@ export interface Route {
|
||||
handler: RouteHandler;
|
||||
method: HttpMethod;
|
||||
path: string; // relative to the plugin's mount path `/<id>`; ":name" segments → ctx.params.name
|
||||
role?: string; // coarse gate — the Keto Role the caller must hold; checked before the handler runs
|
||||
// Mark the page reachable by anyone, signed in or not. The same as omitting `role`
|
||||
permission?: string; // coarse gate — the Keto Permission the caller must hold; checked before the handler runs
|
||||
// Mark the page reachable by anyone, signed in or not. The same as omitting `permission`
|
||||
// — an ungated route is already open — but stated outright, so "public" is a deliberate
|
||||
// choice, not an accident. Mutually exclusive with `role` (discovery refuses both).
|
||||
// choice, not an accident. Mutually exclusive with `permission` (discovery refuses both).
|
||||
public?: boolean;
|
||||
}
|
||||
|
||||
// A Keto Role this plugin gates on — declared for docs/seeding. Role names are a shared
|
||||
// A Keto Permission this plugin gates on — declared for docs/seeding. Permission names are a shared
|
||||
// global namespace (so an operator grants them once in Keto); namespace as `<id>:<action>`.
|
||||
export interface RoleDecl {
|
||||
export interface PermissionDecl {
|
||||
description?: string;
|
||||
name: string;
|
||||
}
|
||||
@@ -63,7 +63,7 @@ export interface PluginManifest {
|
||||
home?: RouteHandler;
|
||||
hooks?: PluginHooks;
|
||||
nav?: NavNode[]; // fragment merged into the menu (composeNav); node `icon` is a Lucide sprite id (src/ui/icons.ts), node ids must be globally unique
|
||||
roles?: RoleDecl[];
|
||||
permissions?: PermissionDecl[];
|
||||
routes?: Route[];
|
||||
}
|
||||
|
||||
@@ -147,7 +147,7 @@ export function checkApiVersion(pluginVersion: unknown, hostVersion: string = HO
|
||||
}
|
||||
|
||||
export interface PluginConflict {
|
||||
kind: "dashboard" | "home" | "id" | "nav-id" | "role" | "route";
|
||||
kind: "dashboard" | "home" | "id" | "nav-id" | "permission" | "route";
|
||||
level: "error" | "warn";
|
||||
message: string;
|
||||
plugins: string[]; // unique ids involved
|
||||
@@ -155,7 +155,7 @@ export interface PluginConflict {
|
||||
|
||||
// The conflict rules: defined, loud resolution — never last-write-wins. Pure over the discovered
|
||||
// plugins; discovery throws on any "error" and logs every "warn". Mount-path (`/<id>`) uniqueness
|
||||
// is structural — it follows from the id check, so it needs no rule of its own. Shared role
|
||||
// is structural — it follows from the id check, so it needs no rule of its own. Shared permission
|
||||
// names are the one intentional overlap, so they warn rather than error.
|
||||
export function findConflicts(plugins: Plugin[]): PluginConflict[] {
|
||||
const out: PluginConflict[] = [];
|
||||
@@ -184,9 +184,9 @@ export function findConflicts(plugins: Plugin[]): PluginConflict[] {
|
||||
});
|
||||
|
||||
collect(plugins, (plugin, push) => {
|
||||
for (const decl of plugin.roles ?? []) push(decl.name);
|
||||
for (const decl of plugin.permissions ?? []) push(decl.name);
|
||||
}).forEach((owners, name) => {
|
||||
if (owners.length > 1) out.push({ kind: "role", level: "warn", message: `role "${name}" declared by ${uniq(owners).length} plugins; namespace as "<id>:<action>" unless shared on purpose`, plugins: uniq(owners) });
|
||||
if (owners.length > 1) out.push({ kind: "permission", level: "warn", message: `permission "${name}" declared by ${uniq(owners).length} plugins; namespace as "<id>:<action>" unless shared on purpose`, plugins: uniq(owners) });
|
||||
});
|
||||
|
||||
return out;
|
||||
|
||||
@@ -55,13 +55,13 @@ test("allowedMethods lists methods at a path (GET implies HEAD); empty when the
|
||||
assert.deepEqual(allowedMethods(plugins, "/x/missing"), []);
|
||||
});
|
||||
|
||||
test("isAuthorized: open routes pass; gated routes require the role token; public is explicitly open", () => {
|
||||
test("isAuthorized: open routes pass; gated routes require the permission token; public is explicitly open", () => {
|
||||
const open: Route = { handler: noop, method: "GET", path: "/" };
|
||||
const gated: Route = { handler: noop, method: "GET", path: "/", role: "x:read" };
|
||||
const gated: Route = { handler: noop, method: "GET", path: "/", permission: "x:read" };
|
||||
const pub: Route = { handler: noop, method: "GET", path: "/", public: true }; // blessed public alias
|
||||
assert.equal(isAuthorized(open, []), true);
|
||||
assert.equal(isAuthorized(gated, []), false);
|
||||
assert.equal(isAuthorized(gated, ["x:read"]), true);
|
||||
assert.equal(isAuthorized(gated, ["other"]), false);
|
||||
assert.equal(isAuthorized(pub, []), true); // open to anonymous, like omitting role — but stated outright
|
||||
assert.equal(isAuthorized(pub, []), true); // open to anonymous, like omitting permission — but stated outright
|
||||
});
|
||||
|
||||
@@ -74,9 +74,9 @@ export function allowedMethods(plugins: Plugin[], pathname: string): string[] {
|
||||
return [...methods].sort();
|
||||
}
|
||||
|
||||
// Coarse role gate: a route marked `public` (or one with no `role`) is open; otherwise
|
||||
// the user's roles (from the session JWT) must include the token. The same rule composeNav uses
|
||||
// for the menu. `public` and `role` are mutually exclusive (discovery refuses both).
|
||||
export function isAuthorized(route: Route, roles: string[]): boolean {
|
||||
return route.public === true || route.role == null || roles.includes(route.role);
|
||||
// Coarse permission gate: a route marked `public` (or one with no `permission`) is open; otherwise
|
||||
// the user's permissions (from the session JWT) must include the token. The same rule composeNav uses
|
||||
// for the menu. `public` and `permission` are mutually exclusive (discovery refuses both).
|
||||
export function isAuthorized(route: Route, permissions: string[]): boolean {
|
||||
return route.public === true || route.permission == null || permissions.includes(route.permission);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user