Gate a route or nav node on a session, not only a permission
CI / full-gate (push) Successful in 3m6s
CI / full-gate (push) Successful in 3m6s
This commit is contained in:
@@ -62,6 +62,9 @@ const badCases: Array<{ name: string; files: Record<string, string>; match: RegE
|
||||
{ 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 permission is contradictory", files: { "contra/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", public: true, permission: "x:read", 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: "${HOST_API_VERSION}", nav: [{ id: "n", label: "N", public: true, permission: "x:read" }] };` }, match: /contranav.*public.*permission/s },
|
||||
{ name: "a route marked session AND permission is contradictory", files: { "contrasess/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", session: true, permission: "x:read", handler: () => ({ html: "x" }) }] };` }, match: /contrasess.*session.*permission/s },
|
||||
{ name: "a route marked public AND session is contradictory", files: { "contrapub/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", public: true, session: true, handler: () => ({ html: "x" }) }] };` }, match: /contrapub.*public.*session/s },
|
||||
{ name: "a nav node marked session AND permission is contradictory", files: { "contrasessnav/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ id: "n", label: "N", session: true, permission: "x:read" }] };` }, match: /contrasessnav.*session.*permission/s },
|
||||
// A permission name is <resource>:<action> wherever the manifest mentions one. Enforced here, not
|
||||
// only in the admin GUI, so it holds for a plugin installed without that GUI.
|
||||
{ name: "a route gating on a bare word", files: { "bare/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", routes: [{ method: "GET", path: "/", permission: "admin", handler: () => ({ html: "x" }) }] };` }, match: /bare.*admin.*<resource>:<action>/s },
|
||||
@@ -96,12 +99,19 @@ test("a discovery failure tells the operator their plugins/ copy may just be out
|
||||
});
|
||||
});
|
||||
|
||||
test("a route + nav node may be marked public and load fine", async (t) => {
|
||||
const dir = scaffold(t, { "pub/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ href: "/pub", id: "n", label: "N", public: true }], routes: [{ method: "GET", path: "/", public: true, handler: () => ({ html: "x" }) }] };` });
|
||||
test("a route + nav node may be marked public, or session, and load fine", async (t) => {
|
||||
const dir = scaffold(t, {
|
||||
"pub/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ href: "/pub", id: "n", label: "N", public: true }], routes: [{ method: "GET", path: "/", public: true, handler: () => ({ html: "x" }) }] };`,
|
||||
"sess/plugin.ts": `export default { apiVersion: "${HOST_API_VERSION}", nav: [{ href: "/sess", id: "s", label: "S", session: true }], routes: [{ method: "GET", path: "/", session: true, handler: () => ({ html: "x" }) }] };`,
|
||||
});
|
||||
const plugins = await discoverPlugins({ dir });
|
||||
assert.equal(plugins.length, 1);
|
||||
assert.equal(plugins[0]?.routes?.[0]?.public, true);
|
||||
assert.equal(plugins[0]?.nav?.[0]?.public, true);
|
||||
assert.equal(plugins.length, 2);
|
||||
const pub = plugins.find((p) => p.id === "pub");
|
||||
const sess = plugins.find((p) => p.id === "sess");
|
||||
assert.equal(pub?.routes?.[0]?.public, true);
|
||||
assert.equal(pub?.nav?.[0]?.public, true);
|
||||
assert.equal(sess?.routes?.[0]?.session, true);
|
||||
assert.equal(sess?.nav?.[0]?.session, true);
|
||||
});
|
||||
|
||||
test("`admin` is not reserved — the admin screens ship as a drop-in plugin mounted at /admin", async (t) => {
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
import { existsSync, readdirSync, readFileSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { gatesSet } from "../auth/gate.ts";
|
||||
import { checkApiVersion, findConflicts, isValidPermissionName, isValidPluginId, RESERVED_PLUGIN_IDS, type Plugin, type PluginManifest } from "./plugin.ts";
|
||||
import { settingsDeclError } from "./settings.ts";
|
||||
import { isValidStoragePluginId, MAX_STORAGE_PLUGIN_ID_LENGTH } from "./storage.ts";
|
||||
@@ -146,12 +147,13 @@ function shapeError(manifest: PluginManifest): string | null {
|
||||
const settings = settingsDeclError(manifest.settings);
|
||||
if (settings) return settings;
|
||||
}
|
||||
// `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.
|
||||
// Two gates on one route or nav node contradict each other — "open to all" vs "needs a session"
|
||||
// vs "needs this permission". Refuse rather than silently pick one, so intent stays unambiguous.
|
||||
for (const route of Array.isArray(manifest.routes) ? manifest.routes : []) {
|
||||
if (route?.public === true && route.permission != null) return `route "${route.method} ${route.path}" sets both public and permission — they are mutually exclusive`;
|
||||
const gates = gatesSet(route);
|
||||
if (gates.length > 1) return `route "${route?.method} ${route?.path}" sets ${gates.join(" and ")}; a route names exactly one gate — public, session or permission`;
|
||||
}
|
||||
const navContradiction = findPublicNavContradiction(manifest.nav);
|
||||
const navContradiction = findNavGateContradiction(manifest.nav);
|
||||
if (navContradiction) return navContradiction;
|
||||
// Every permission name the manifest mentions — gated on or declared — must be `<resource>:<action>`.
|
||||
// A bare word names a role, and roles are groups here (README → Naming a permission).
|
||||
@@ -170,11 +172,12 @@ function shapeError(manifest: PluginManifest): string | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Recurse the nav fragment: a node that is both `public` and `permission`-gated is contradictory.
|
||||
function findPublicNavContradiction(nodes: PluginManifest["nav"]): string | null {
|
||||
// Recurse the nav fragment: a node naming more than one gate is contradictory, same as a route.
|
||||
function findNavGateContradiction(nodes: PluginManifest["nav"]): string | null {
|
||||
for (const node of Array.isArray(nodes) ? nodes : []) {
|
||||
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);
|
||||
const gates = gatesSet(node);
|
||||
if (gates.length > 1) return `nav node "${node?.label ?? node?.id ?? "?"}" sets ${gates.join(" and ")}; a node names exactly one gate — public, session or permission`;
|
||||
const inChild = findNavGateContradiction(node?.children);
|
||||
if (inChild) return inChild;
|
||||
}
|
||||
return null;
|
||||
|
||||
@@ -32,6 +32,9 @@ export interface Route {
|
||||
// Same as omitting `permission`, but stated outright so public is a deliberate choice rather than
|
||||
// a forgotten gate. Mutually exclusive with `permission` (discovery refuses both).
|
||||
public?: boolean;
|
||||
// Any signed-in user, no grant to hold — for a plugin whose data is the visitor's own. Anonymous
|
||||
// is bounced to /login, never 403. Mutually exclusive with the other two (discovery refuses both).
|
||||
session?: boolean;
|
||||
}
|
||||
|
||||
// A Keto Permission this plugin gates on — declared for docs/seeding. Names are a shared global
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { test } from "node:test";
|
||||
import type { Plugin, Route } from "./plugin.ts";
|
||||
import { allowedMethods, isAuthorized, matchRoute } from "./router.ts";
|
||||
import { allowedMethods, matchRoute } from "./router.ts";
|
||||
|
||||
const noop: Route["handler"] = () => ({ html: "x" });
|
||||
|
||||
@@ -54,14 +54,3 @@ test("allowedMethods lists methods at a path (GET implies HEAD); empty when the
|
||||
assert.deepEqual(allowedMethods(plugins, "/x/a"), ["GET", "HEAD", "POST"]);
|
||||
assert.deepEqual(allowedMethods(plugins, "/x/missing"), []);
|
||||
});
|
||||
|
||||
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: "/", 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 permission — but stated outright
|
||||
});
|
||||
|
||||
@@ -73,10 +73,3 @@ export function allowedMethods(plugins: Plugin[], pathname: string): string[] {
|
||||
}
|
||||
return [...methods].sort();
|
||||
}
|
||||
|
||||
// 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