diff --git a/CHANGELOG.md b/CHANGELOG.md index 10aee7c1..48ef4d4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,52 @@ exactly what "breaking" means and what doesn't count. ## [Unreleased] +### Changed + +- **Dropped the `ROLE_PERMISSIONS`-table fallback from every core-side admin + permission decision — `authz.check()` is now the sole authority, no + exceptions.** `checkInlineEditPermission()` and + `runPluginResponseTransforms()`'s `pages.update` gate previously fell back + to a flat role table when `authz` was undefined (authz creation failing at + startup — an in-process object construction that essentially never fails + in practice). Both now fail closed (deny) instead of silently degrading to + a separate, less-audited mechanism — dec-identity-unification Phase 5c's + second half, finally closed. The site-user path to `/api/inline-edit/ws` + (`checkInlineEditPermissionForSiteUser()`) had the same kind of fallback — + a raw `roles[]` check for an editor-equivalent role when `authz` was + undefined — and now fails closed too. `runPluginResponseTransforms()`'s + published `auth.role` / `hasPermission()` now also read the *highest* + admin-tier role in the session's `roles[]` (`highestAdminRole()`, new, + `@dune/core/auth/authz-schema`) instead of `roles[0]` — a merged User's + `roles[]` can hold content-gating tags alongside the admin role in any + order, and `roles[0]` could be a tag, silently under-privileging the + synchronous approximation relative to what `authz.check()` had just + decided. The one place that genuinely can't + call the real async `authz.check()` — + `ResponseTransformContext.auth.hasPermission()`, + a published *synchronous* hook API — now reads `roleHasPermission()` + (new, `@dune/core/auth/authz-schema`), a synchronous lookup against the + same canonical `actionToRelations` schema `authz.check()` itself uses, + instead of a hand-maintained mirror. `@dune/plugin-admin` no longer needs + to keep its own `ROLE_PERMISSIONS` table in sync with this schema at all — + see its own 3.0.0 changelog entry for the corresponding removal there. +- **`ADMIN_ROLE_RANK`** exported alongside `highestAdminRole()` + (`@dune/core/auth/authz-schema`, now also reachable via a new + `"./auth/authz-schema"` export map entry — it previously had none, so + nothing outside this package could actually import from it despite + `roleHasPermission()`'s own doc already describing it as importable). + `@dune/plugin-admin`'s `role-utils.ts` now derives its `ROLE_RANK`/ + `highestValidRole()` from this instead of keeping its own copy of the + same three numbers — the exact "two tables kept in sync by convention" + pattern the `ROLE_PERMISSIONS` removal was about eliminating, just + spotted a second time during review before it could take hold. +- **Built-in admin plugin pin is now `@dune/plugin-admin@^3.0`** + (`deno.json` imports and `src/plugins/builtin.ts`). 3.0.0 is the + companion that removes `ROLE_PERMISSIONS` / `hasPermission()`; a 2.x + plugin-admin still runs against this core, but the two must be released + together so sites that pick up 3.0.0 do not call a method that no + longer exists. + ### Added - **`dune plugin:update`, `plugin:install`, and `plugin:remove` now accept diff --git a/deno.json b/deno.json index 10b6f144..824d911c 100644 --- a/deno.json +++ b/deno.json @@ -22,6 +22,7 @@ "./auth/api-guard": "./src/auth/api-guard.ts", "./auth/mount": "./src/auth/mount.ts", "./auth/authz": "./src/auth/authz.ts", + "./auth/authz-schema": "./src/auth/authz-schema.ts", "./auth/authz-adapter-local": "./src/auth/authz-adapter-local.ts", "./auth/authz-adapter-db": "./src/auth/authz-adapter-db.ts", "./auth/types": "./src/auth/types.ts", @@ -56,12 +57,12 @@ "./fresh-app": "./src/runtime/server.ts" }, "imports": { - "@dune/plugin-admin": "jsr:@dune/plugin-admin@^2.0", - "@dune/plugin-admin/admin/mount": "jsr:@dune/plugin-admin@^2.0/admin/mount", - "@dune/plugin-admin/admin/context": "jsr:@dune/plugin-admin@^2.0/admin/context", - "@dune/plugin-admin/admin/types": "jsr:@dune/plugin-admin@^2.0/admin/types", - "@dune/plugin-admin/admin/auth/middleware": "jsr:@dune/plugin-admin@^2.0/admin/auth/middleware", - "@dune/plugin-admin/admin/auth/provider": "jsr:@dune/plugin-admin@^2.0/admin/auth/provider", + "@dune/plugin-admin": "jsr:@dune/plugin-admin@^3.0", + "@dune/plugin-admin/admin/mount": "jsr:@dune/plugin-admin@^3.0/admin/mount", + "@dune/plugin-admin/admin/context": "jsr:@dune/plugin-admin@^3.0/admin/context", + "@dune/plugin-admin/admin/types": "jsr:@dune/plugin-admin@^3.0/admin/types", + "@dune/plugin-admin/admin/auth/middleware": "jsr:@dune/plugin-admin@^3.0/admin/auth/middleware", + "@dune/plugin-admin/admin/auth/provider": "jsr:@dune/plugin-admin@^3.0/admin/auth/provider", "fresh": "jsr:@fresh/core@^2", "preact": "npm:preact@^10", "preact/hooks": "npm:preact@^10/hooks", diff --git a/skills/dune-authz.md b/skills/dune-authz.md index 362cc5a0..9b64654b 100644 --- a/skills/dune-authz.md +++ b/skills/dune-authz.md @@ -73,7 +73,7 @@ const canAdmin = await getAuthz().check({ }); ``` -In practice, `plugin-admin`'s own route guards don't call this directly for you — `requirePermission(ctx, perm)` (internal to `@dune/plugin-admin`) already does: it calls `authz.check()` when polizy is wired (`auth.mode: dune`, `authzStore: local`), and falls back to a simpler `ROLE_PERMISSIONS` table lookup when it isn't. You don't need to duplicate this in an admin route. +In practice, `plugin-admin`'s own route guards don't call this directly for you — `requirePermission(ctx, perm)` (internal to `@dune/plugin-admin`) already does: it calls `authz.check()` and denies if `authz` is undefined. There is no role-table fallback. You don't need to duplicate this in an admin route. ### Group membership (content gating, membership sites) diff --git a/skills/dune-plugin-authoring.md b/skills/dune-plugin-authoring.md index 3e4f0573..fbf58241 100644 --- a/skills/dune-plugin-authoring.md +++ b/skills/dune-plugin-authoring.md @@ -288,7 +288,7 @@ export default { } satisfies DunePlugin; ``` -A route registered via `mount()` is **not** automatically authenticated the way `adminPages` is — you must wrap it in `withGuards()` (or call `csrfCheck()`/`requirePermission()` from the same module directly) yourself. Reimplementing the guard sequence by hand instead of using `withGuards()` is exactly what caused three real security regressions in Dune's own admin routes (the module's own doc comment names them) — the CSRF check's Origin/Sec-Fetch-Site/Referer fallback chain in particular is not trivial to get right, and `requirePermission()` checks the polizy-backed `authz` system first when configured, falling back to the role table only when it's not — a detail that's easy to miss if you reach for `AdminContext.auth.hasPermission()` directly instead. If your plugin only needs to *display* data or trigger something a human clicks through in the admin UI, prefer `adminPages` anyway — you get all of this for free, no `withGuards()` needed. +A route registered via `mount()` is **not** automatically authenticated the way `adminPages` is — you must wrap it in `withGuards()` (or call `csrfCheck()`/`requirePermission()` from the same module directly) yourself. Reimplementing the guard sequence by hand instead of using `withGuards()` is exactly what caused three real security regressions in Dune's own admin routes (the module's own doc comment names them) — the CSRF check's Origin/Sec-Fetch-Site/Referer fallback chain in particular is not trivial to get right, and `requirePermission()` consults `authz.check()` only (missing `authz` denies; there is no role-table fallback). `AdminContext.auth.hasPermission()` was removed in `@dune/plugin-admin` 3.0.0 — do not reimplement it. If your plugin only needs to *display* data or trigger something a human clicks through in the admin UI, prefer `adminPages` anyway — you get all of this for free, no `withGuards()` needed. ### `publicRoutes` — declarative public-facing routes @@ -321,7 +321,7 @@ Headless-mode developers who call `mountDuneAdmin(app, ctx)` directly (never goi ### Permission reference -Real values of the `AdminPermission` union (`plugin-admin/src/admin/types.ts`) — `adminPages[].permission` accepts any string but only these are meaningful against the built-in role table: +Real values of the `AdminPermission` union (`plugin-admin/src/admin/types.ts`) — `adminPages[].permission` accepts any string but only these map to actions in `@dune/core`'s canonical `actionToRelations` schema (`authz.check()` is the sole authority; unknown names deny): | Permission string | Grants | |--------------------|--------| @@ -330,7 +330,8 @@ Real values of the `AdminPermission` union (`plugin-admin/src/admin/types.ts`) | `"users.create"` / `"users.read"` / `"users.update"` / `"users.delete"` | User management | | `"config.read"` / `"config.update"` | Site config | | `"submissions.read"` / `"submissions.delete"` | Form submissions | -| `"admin.access"` | Any authenticated admin (no finer check) | + +Panel access itself is `authz.check({ canThey: "access", onWhat: { type: "app", id: "admin" } })`, not a permission string. There is no `"admin.access"` — that name existed only on the removed `ROLE_PERMISSIONS` table and is now rejected as unknown. There is no `"pages.view"`, `"users.manage"`, or `"plugins.manage"` — use the split create/read/update/delete permissions above. diff --git a/src/auth/authz-schema.ts b/src/auth/authz-schema.ts index 9c9a750c..c137108d 100644 --- a/src/auth/authz-schema.ts +++ b/src/auth/authz-schema.ts @@ -17,16 +17,19 @@ * ## Admin panel access (top-level) * authz.check({ who: adminUser, canThey: "access", onWhat: { type: "app", id: "admin" } }) * - * ## Granular admin permissions (mirror of ROLE_PERMISSIONS) + * ## Granular admin permissions * authz.check({ who: adminUser, canThey: "pages.create", onWhat: { type: "app", id: "admin" } }) * - * The admin permission actions below are the canonical permission definition. - * ROLE_PERMISSIONS in src/admin/types.ts is kept for reference only and must - * stay in sync with this schema. + * The admin permission actions below are the sole, canonical permission + * definition — `@dune/plugin-admin` no longer keeps its own `ROLE_PERMISSIONS` + * mirror (removed alongside dec-identity-unification Phase 5c/6, 3.0.0); see + * {@link roleHasPermission} for the one place that still needs a synchronous + * read of this same data instead of `authz.check()`. */ import { defineSchema } from "polizy"; import type { AuthSchema } from "polizy"; +import type { Role } from "../config/admin-config.ts"; /** * The polizy authorization schema for Dune — defines relations and action-to-relation mappings @@ -65,7 +68,7 @@ export const duneAuthzSchema: AuthSchema = defineSchema // ── Admin panel — granular permissions (maps 1:1 with AdminPermission) ─ // - // These mirror ROLE_PERMISSIONS in src/admin/types.ts. Change one → change both. + // Sole definition — nothing else mirrors this (see the module doc above). // "pages.create": ["admin", "editor", "author"], "pages.read": ["admin", "editor", "author"], @@ -93,3 +96,71 @@ export const duneAuthzSchema: AuthSchema = defineSchema /** TypeScript type of the Dune authorization schema — pass to `AuthSystem` generics. */ export type DuneAuthzSchema = typeof duneAuthzSchema; + +/** + * Synchronous, role-only approximation of an admin permission check, sourced + * directly from this schema's `actionToRelations` — the single canonical + * permission definition, not a hand-maintained mirror of it. + * + * `authz.check()` is the real, async, sole authority for every admin route. + * This exists only for the one place in Dune that cannot call it: a + * published, synchronous hook API (`ResponseTransformContext.auth + * .hasPermission()`, `src/cli/response-transforms.ts`) that plugin authors + * already depend on. It answers exactly for the permission that gated the + * hook context into existing in the first place, and is a reasonable + * approximation for any other permission a plugin might ask about — the + * same tradeoff a flat role table always made, just without a second table + * to keep in sync with this one. + * + * ⚠️ **Not for access decisions.** This is an *approximation*: it sees only + * the role vocabulary in `actionToRelations` and knows nothing of what + * `authz.check()` knows beyond roles — revoked tuples, per-user direct + * grants (`owner`/`editor` relations), or group membership. Its answer can + * therefore diverge from the sole authority in either direction. Do NOT use + * it (or copy it) to gate access anywhere — use `authz.check()`. If you + * need a permission decision in a synchronous context, restructure the + * context so the decision is made once, asynchronously, and passed in. + */ +export function roleHasPermission(role: string, permission: string): boolean { + const relations = duneAuthzSchema.actionToRelations as Record; + return relations[permission]?.includes(role) ?? false; +} + +/** + * Canonical admin-tier role ranking (`"admin" > "editor" > "author"`) — + * the single source of truth `highestAdminRole()` below uses, and the one + * `@dune/plugin-admin`'s `role-utils.ts` imports for its own direct rank + * comparisons (`provisioner.ts`'s role-escalation guards) instead of + * keeping a second, hand-maintained copy of the same three numbers. This + * is the exact "two tables that must be kept in sync by convention" + * pattern the `ROLE_PERMISSIONS` removal (3.0.0) was about eliminating — + * codified once, here, rather than repeated. + */ +export const ADMIN_ROLE_RANK: Record = { + admin: 3, + editor: 2, + author: 1, +}; + +/** + * Pick the highest-ranked admin-tier role out of a generic `roles: string[]` + * array, or `""` if none is present. + * + * A merged `User`'s `roles[]` mixes admin-tier roles with content-gating + * tags (e.g. `["member", "admin"]`) in no guaranteed order, so `roles[0]` + * is not necessarily an admin role at all. Permission answers sourced from + * a single `role` string must use the highest admin-tier entry — matching + * what `authz.check()` (which unions all the user's relations) would + * decide — or the synchronous approximation under-privileges relative to + * the sole authority. Used for `ResponseTransformContext.auth.role` / + * `.hasPermission()`, the one place that can't call the real async check. + */ +export function highestAdminRole(roles: string[] | undefined): string { + let best = ""; + for (const r of roles ?? []) { + const rr = ADMIN_ROLE_RANK[r as Role] ?? 0; + if (rr === 0) continue; // unknown / content-gating tag — never wins + if (!best || rr > (ADMIN_ROLE_RANK[best as Role] ?? 0)) best = r; + } + return best; +} diff --git a/src/cli/response-transforms.ts b/src/cli/response-transforms.ts index 2cab76bd..6d9134bb 100644 --- a/src/cli/response-transforms.ts +++ b/src/cli/response-transforms.ts @@ -8,11 +8,12 @@ * - Resolve the auth context once per request, enforcing the documented * contract: `auth` is non-null only for a valid admin session that holds * the `pages.update` permission. Plugins can rely on this and must not - * need to re-authenticate. The permission check itself goes through the - * polizy `authz` system (`authz.check()`) when configured — the same - * sole-authority contract every other admin permission check follows - * (dec-identity-unification Phase 5c/7) — not the flat `ROLE_PERMISSIONS` - * table `auth.hasPermission()` alone would consult. + * need to re-authenticate. The permission check itself is `authz.check()` — + * the sole authority every admin permission check follows + * (dec-identity-unification Phase 5c/6/7); if `authz` is somehow + * undefined (authz creation itself failed at startup — already logged + * loudly there), this fails closed rather than falling back to a + * separate, less-audited mechanism. * - Never run transforms on admin-panel paths (defense in depth — admin * routes are mounted separately and should never reach the content * catch-all, but the pipeline must not depend on routing order alone). @@ -31,18 +32,16 @@ /** * Minimal auth middleware interface — concrete type is from `@dune/plugin-admin`. * `authenticate()` is still the only way to validate an admin session cookie - * (core doesn't own admin session cookie config), but `hasPermission()` is - * used only as the fallback when `authz` (below) isn't configured — see the - * doc comment on `runPluginResponseTransforms` for why. + * (core doesn't own admin session cookie config). */ interface AdminAuthMiddleware { authenticate(req: Request): Promise; - hasPermission(result: unknown, permission: string): boolean; } import type { DuneConfig } from "../config/types.ts"; import type { DunePlugin, ResponseTransformContext } from "../hooks/types.ts"; import type { DuneAuthSystem } from "../auth/authz.ts"; import type { PageIndex } from "../content/types.ts"; +import { highestAdminRole, roleHasPermission } from "../auth/authz-schema.ts"; import { applyResponseTransforms } from "../plugins/loader.ts"; import { hasAdminSessionCookie } from "./admin-bar-inject.ts"; import { isAdminPath } from "./serve-utils.ts"; @@ -72,11 +71,10 @@ export interface RunResponseTransformsOptions { */ auth: AdminAuthMiddleware | null; /** - * Polizy authz system — when present, is the sole authority for the - * `pages.update` check below (dec-identity-unification Phase 7). Falls - * back to `auth.hasPermission()` only in the narrow, exceptional case - * where authz creation itself failed at startup (already logged loudly - * there — see `src/runtime/bootstrap.ts`). + * Polizy authz system — the sole authority for the `pages.update` check + * below (dec-identity-unification Phase 5c/7). Undefined only when authz + * creation itself failed at startup (already logged loudly there — see + * `src/runtime/bootstrap.ts`), in which case the check fails closed. */ authz?: DuneAuthSystem; /** @@ -134,22 +132,26 @@ export async function runPluginResponseTransforms( canThey: "pages.update" as any, onWhat: { type: "app", id: "admin" }, }) - : auth.hasPermission(result, "pages.update") + : false ); if (canUpdatePages) { const user = r.user as Record; - // dec-identity-unification Phase 5b: the merged User type has no - // single `role` field, only a generic `roles: string[]` — an admin - // session always has exactly one entry in it (set by - // @dune/plugin-admin's UserManager), so the first entry is the - // session's admin role. This context field keeps its original - // `role: string` shape (a published hook type, ResponseTransformContext - // — minimize churn for plugin authors) rather than switching to `roles`. + // This context field keeps its original `role: string` shape (a + // published hook type, ResponseTransformContext — minimize churn for + // plugin authors) rather than switching to `roles`. const roles = user.roles as string[] | undefined; + // The merged User type mixes admin-tier roles with content-gating + // tags in roles[] (e.g. ["member", "admin"]) with no guaranteed + // order — roles[0] is not necessarily an admin role. The published + // role string and hasPermission() answers sourced from it must use + // the highest admin-tier entry, or the synchronous approximation + // would under-privilege relative to what authz.check() (which + // unions all the user's relations) decided just above. + const role = highestAdminRole(roles); transformAuth = { username: user.username as string, - role: roles?.[0] ?? "", - hasPermission: (perm) => auth.hasPermission(result, perm as string), + role, + hasPermission: (perm) => roleHasPermission(role, perm), }; } } catch { /* invalid session — treat as unauthenticated */ } diff --git a/src/cli/validate.ts b/src/cli/validate.ts index 2a437786..d31f38ca 100644 --- a/src/cli/validate.ts +++ b/src/cli/validate.ts @@ -58,7 +58,6 @@ const VALID_PERMISSIONS = new Set([ "users.create", "users.read", "users.update", "users.delete", "config.read", "config.update", "submissions.read", "submissions.delete", - "admin.access", ]); /** @@ -111,8 +110,12 @@ function scanCodeBlock(code: string): { unknownHooks: string[]; unknownPerms: st if (!VALID_PERMISSIONS.has(m[1])) unknownPerms.push(m[1]); } - // Pattern 4: hasPermission(authResult, "perm.name") - const hasPerm = /hasPermission\([^,]+,\s*["']([^"']+)["']/g; + // Pattern 4: hasPermission("perm.name") — the synchronous hook API + // (ResponseTransformContext.auth.hasPermission). The old two-argument + // form — hasPermission(authResult, "perm") — was removed alongside + // ROLE_PERMISSIONS (3.0.0); a two-arg call now means old example code + // and is flagged via the unknown-permission vocabulary below too. + const hasPerm = /hasPermission\(\s*["']([^"']+)["']/g; while ((m = hasPerm.exec(code)) !== null) { if (!VALID_PERMISSIONS.has(m[1])) unknownPerms.push(m[1]); } diff --git a/src/config/admin-config.ts b/src/config/admin-config.ts index 3622e3b4..9f023ca6 100644 --- a/src/config/admin-config.ts +++ b/src/config/admin-config.ts @@ -243,9 +243,9 @@ export interface AdminConfig { * site-user tuples, and previously creation of this authz instance was * accidentally gated on the public-auth config instead of its own * setting — a site running `site.auth.mode: external-jwt` (or - * `authzStore: "db"`) would silently lose real admin-panel permission - * checking too, falling back to the flat `ROLE_PERMISSIONS` table with - * no warning. + * `authzStore: "db"`) would silently lose the admin-panel authz instance. + * That path now fails closed (deny) when `authz` is missing; there is no + * role-table fallback. * * Only `"local"` is implemented today — a `"db"` tier (mirroring * `site.auth.authzStore`'s db-backed option) isn't wired up for the diff --git a/src/hooks/types.ts b/src/hooks/types.ts index e6d18d21..152536a6 100644 --- a/src/hooks/types.ts +++ b/src/hooks/types.ts @@ -693,14 +693,22 @@ export interface ResponseTransformContext { role: string; /** * Check whether this user has a specific admin permission. `auth` being - * non-null already means the session holds `pages.update` — checked - * against the polizy `authz` system when configured (dec-identity- - * unification Phase 7), not the flat role table alone. This method - * itself stays synchronous (a published hook API) and so cannot - * re-consult `authz.check()` per call; it answers from the flat role - * table, which is exact for `pages.update` (already gated above) and a - * reasonable approximation for any other permission a plugin might ask - * about. + * non-null already means the session holds `pages.update`, checked via + * `authz.check()` — the sole authority (dec-identity-unification Phase + * 5c/7). This method itself stays synchronous (a published hook API) + * and so cannot re-consult `authz.check()` per call; it answers from + * `roleHasPermission()` (`@dune/core/auth/authz-schema`), a synchronous + * role-only read of the same canonical `actionToRelations` schema + * `authz.check()` itself uses — exact for `pages.update` (already gated + * above) and a reasonable approximation for any other permission a + * plugin might ask about. + * + * ⚠️ Trust accordingly: this is a role-only *approximation*, not the + * authority. It ignores everything `authz.check()` knows beyond roles + * (revoked tuples, per-user direct grants, group membership), so its + * answer can diverge from a real `authz.check()` in either direction. + * Fine for UI affordances and informational gating inside a plugin; + * do not treat it as an access-control decision for anything sensitive. */ hasPermission(permission: string): boolean; } | null; diff --git a/src/plugins/builtin.ts b/src/plugins/builtin.ts index 0bdafaf2..e23e7f44 100644 --- a/src/plugins/builtin.ts +++ b/src/plugins/builtin.ts @@ -18,10 +18,10 @@ import { CORE_INSTANCE, CORE_VERSION } from "./mod.ts"; /** The built-in admin plugin package (bootstrap.ts's dynamic import). */ -export const ADMIN_PLUGIN_SPECIFIER = "jsr:@dune/plugin-admin@^2.0"; +export const ADMIN_PLUGIN_SPECIFIER = "jsr:@dune/plugin-admin@^3.0"; /** The admin mount subpath used by the multisite manager. */ -export const ADMIN_MOUNT_SPECIFIER = "jsr:@dune/plugin-admin@^2.0/admin/mount"; +export const ADMIN_MOUNT_SPECIFIER = "jsr:@dune/plugin-admin@^3.0/admin/mount"; /** * Core-instance handshake for the dynamically imported admin plugin. diff --git a/src/runtime/server.ts b/src/runtime/server.ts index bc79fb08..2001a0da 100644 --- a/src/runtime/server.ts +++ b/src/runtime/server.ts @@ -184,25 +184,25 @@ export function isIndexedInlineEditPath( /** * Whether an authenticated admin session may perform inline edits - * (`pages.update`). Checked against `authz` when configured — the sole - * authority every other admin permission check follows - * (dec-identity-unification Phase 5c/7) — falling back to - * `adminAuth.hasPermission()` (the flat `ROLE_PERMISSIONS` table) only when - * authz creation itself failed at startup. Extracted from the `/api/ - * inline-edit/ws` handler below and exported so this decision is directly - * unit-testable without a full app + WebSocket-upgrade harness. + * (`pages.update`). `authz` is the sole authority — the flat + * `ROLE_PERMISSIONS` fallback this used to have was removed + * (dec-identity-unification Phase 5c, second half): `admin.authzStore` + * defaults to `"local"` and is created automatically whenever the admin + * panel is enabled, regardless of `site.auth`'s mode, so `authz` being + * undefined here means its creation itself failed at startup (already + * logged loudly there) — an exceptional condition that should fail closed + * (deny), not silently degrade to a materially different, less-audited + * authorization mechanism. Extracted from the `/api/inline-edit/ws` handler + * below and exported so this decision is directly unit-testable without a + * full app + WebSocket-upgrade harness. */ export async function checkInlineEditPermission( authz: DuneAuthSystem | undefined, // deno-lint-ignore no-explicit-any - adminAuth: any, - // deno-lint-ignore no-explicit-any authResult: any, ): Promise { - if (authz) { - return await checkPagesUpdateViaAuthz(authz, authResult.user.id); - } - return adminAuth.hasPermission(authResult, "pages.update"); + if (!authz) return false; + return await checkPagesUpdateViaAuthz(authz, authResult.user.id); } function checkPagesUpdateViaAuthz( @@ -221,20 +221,18 @@ function checkPagesUpdateViaAuthz( * Same `pages.update` decision as {@link checkInlineEditPermission}, for a * user resolved via the public-auth session (`src/auth/`, `getUser(req)`) * rather than an admin session — this is what lets `/api/inline-edit/ws` - * work for a site that has `@dune/plugin-admin` decoupled entirely. There is - * no `ROLE_PERMISSIONS` table for public-auth users (that fallback exists - * only for admin sessions, whose roles are a closed admin-panel vocabulary), - * so the no-authz fallback here is a direct check against the user's own - * `roles: string[]` for an editor-equivalent role instead. + * work for a site that has `@dune/plugin-admin` decoupled entirely. Like + * every other admin permission check, this fails closed (deny) when `authz` + * is undefined — there is no role-table fallback for public-auth users (and + * after 3.0.0, none for admin sessions either): `authz.check()` is the sole + * authority, no exceptions. */ export async function checkInlineEditPermissionForSiteUser( authz: DuneAuthSystem | undefined, user: { id: string; roles: string[] }, ): Promise { - if (authz) { - return await checkPagesUpdateViaAuthz(authz, user.id); - } - return user.roles.includes("editor") || user.roles.includes("admin"); + if (!authz) return false; + return await checkPagesUpdateViaAuthz(authz, user.id); } // ── Factory ──────────────────────────────────────────────────────────────────── @@ -485,7 +483,6 @@ export async function createDuneApp( username = authResult.user.username; permitted = await checkInlineEditPermission( ctx.authz, - adminAuth, authResult, ); } diff --git a/tests/auth/authz_schema_test.ts b/tests/auth/authz_schema_test.ts new file mode 100644 index 00000000..6cf816db --- /dev/null +++ b/tests/auth/authz_schema_test.ts @@ -0,0 +1,48 @@ +/** + * Tests for authz-schema.ts's roleHasPermission() — the synchronous, + * role-only read of the canonical actionToRelations schema that replaced + * plugin-admin's hand-maintained ROLE_PERMISSIONS mirror (dec-identity- + * unification Phase 5c/6). Only used where a published, synchronous hook + * API can't call the real, async authz.check() (response-transforms.ts). + */ + +import { assertEquals } from "https://deno.land/std@0.224.0/assert/mod.ts"; +import { highestAdminRole, roleHasPermission } from "../../src/auth/authz-schema.ts"; + +Deno.test("roleHasPermission: admin has every admin-tier permission", () => { + assertEquals(roleHasPermission("admin", "pages.update"), true); + assertEquals(roleHasPermission("admin", "users.manage"), true); + assertEquals(roleHasPermission("admin", "config.update"), true); +}); + +Deno.test("roleHasPermission: editor has pages/media/config-read but not admin-only actions", () => { + assertEquals(roleHasPermission("editor", "pages.update"), true); + assertEquals(roleHasPermission("editor", "config.read"), true); + assertEquals(roleHasPermission("editor", "config.update"), false); + assertEquals(roleHasPermission("editor", "users.manage"), false); +}); + +Deno.test("roleHasPermission: author is more restricted than editor", () => { + assertEquals(roleHasPermission("author", "pages.update"), true); + assertEquals(roleHasPermission("author", "media.delete"), false); + assertEquals(roleHasPermission("author", "config.read"), false); +}); + +Deno.test("roleHasPermission: unknown role or unknown permission both deny", () => { + assertEquals(roleHasPermission("not-a-real-role", "pages.update"), false); + assertEquals(roleHasPermission("admin", "not-a-real-permission"), false); +}); + +Deno.test("highestAdminRole: picks the highest admin-tier role regardless of order", () => { + assertEquals(highestAdminRole(["member", "admin"]), "admin"); + assertEquals(highestAdminRole(["editor", "admin"]), "admin"); + assertEquals(highestAdminRole(["member", "editor", "author"]), "editor"); + assertEquals(highestAdminRole(["author"]), "author"); +}); + +Deno.test("highestAdminRole: ignores content-gating tags and unknown roles", () => { + assertEquals(highestAdminRole(["member", "subscriber"]), ""); + assertEquals(highestAdminRole(["member"]), ""); + assertEquals(highestAdminRole(undefined), ""); + assertEquals(highestAdminRole([]), ""); +}); diff --git a/tests/cli/response_transforms_test.ts b/tests/cli/response_transforms_test.ts index 4f890b88..b1202dc6 100644 --- a/tests/cli/response_transforms_test.ts +++ b/tests/cli/response_transforms_test.ts @@ -18,7 +18,6 @@ import type { } from "../../src/hooks/types.ts"; import type { DuneConfig } from "../../src/config/types.ts"; import type { - AdminPermission, AuthResult, User, } from "jsr:@dune/plugin-admin/admin/types"; @@ -41,10 +40,14 @@ function makeUser(role: string): User { }; } -/** Fake auth middleware with a fixed outcome and call counter. */ +/** + * Fake auth middleware with a fixed outcome and call counter. `authenticate()` + * is the only method the real interface has since ROLE_PERMISSIONS/ + * hasPermission() were removed (dec-identity-unification Phase 5c/6) — + * permission decisions go through `makeAuthz()` below instead. + */ function makeAuth(opts: { result?: AuthResult; - permissions?: AdminPermission[]; throws?: boolean; }) { const calls = { authenticate: 0 }; @@ -57,11 +60,6 @@ function makeAuth(opts: { opts.result ?? { authenticated: false, error: "No session cookie" }, ); }, - hasPermission(authResult: unknown, permission: string): boolean { - const r = authResult as AuthResult; - if (!r.authenticated || !r.user) return false; - return (opts.permissions ?? []).includes(permission as AdminPermission); - }, }; } @@ -174,10 +172,9 @@ Deno.test("runPluginResponseTransforms: anonymous request — plugin runs with a assertEquals(auth.calls.authenticate, 0); }); -Deno.test("runPluginResponseTransforms: valid session WITHOUT pages.update — auth is null (F1)", async () => { +Deno.test("runPluginResponseTransforms: valid session, no authz — auth is null (F1, fails closed)", async () => { const auth = makeAuth({ result: { authenticated: true, user: makeUser("author") }, - permissions: ["pages.read"] as AdminPermission[], }); const { plugin, seen } = makeRecordingPlugin(); await runPluginResponseTransforms({ @@ -196,14 +193,16 @@ Deno.test("runPluginResponseTransforms: valid session WITHOUT pages.update — a Deno.test("runPluginResponseTransforms: valid session WITH pages.update — auth populated", async () => { const auth = makeAuth({ result: { authenticated: true, user: makeUser("editor") }, - permissions: ["pages.read", "pages.update"] as AdminPermission[], }); + // deno-lint-ignore no-explicit-any + const authz = makeAuthz(true) as any; const { plugin, seen } = makeRecordingPlugin(); await runPluginResponseTransforms({ req: makeReq("/about", SESSION_COOKIE), response: new Response("hello"), plugins: [plugin], auth, + authz, resolve, config, adminPrefix: "/admin", @@ -211,14 +210,47 @@ Deno.test("runPluginResponseTransforms: valid session WITH pages.update — auth const ctxAuth = seen[0].auth; assertEquals(ctxAuth?.username, "alice"); assertEquals(ctxAuth?.role, "editor"); + // hasPermission() is a synchronous read of the canonical actionToRelations + // schema (roleHasPermission()), not a re-consultation of authz.check() — + // "editor" grants pages.update but not the admin-only users.manage. assertEquals(ctxAuth?.hasPermission("pages.update"), true); assertEquals(ctxAuth?.hasPermission("users.manage"), false); }); +Deno.test("runPluginResponseTransforms: roles[] mixing tags and admin roles — highest admin-tier role wins", async () => { + // A merged User's roles[] can hold content-gating tags alongside the + // admin-tier role, in no guaranteed order. roles[0] ("member") used to + // become ctx.auth.role — under-privileging hasPermission() relative to + // what authz.check() decided just above. + const auth = makeAuth({ + result: { + authenticated: true, + // deno-lint-ignore no-explicit-any + user: { ...makeUser("admin"), roles: ["member", "admin"] } as any, + }, + }); + // deno-lint-ignore no-explicit-any + const authz = makeAuthz(true) as any; + const { plugin, seen } = makeRecordingPlugin(); + await runPluginResponseTransforms({ + req: makeReq("/about", SESSION_COOKIE), + response: new Response("hello"), + plugins: [plugin], + auth, + authz, + resolve, + config, + adminPrefix: "/admin", + }); + const ctxAuth = seen[0].auth; + assertEquals(ctxAuth?.role, "admin"); + assertEquals(ctxAuth?.hasPermission("pages.update"), true); + assertEquals(ctxAuth?.hasPermission("users.manage"), true); +}); + Deno.test("runPluginResponseTransforms: admin paths are never transformed (F4)", async () => { const auth = makeAuth({ result: { authenticated: true, user: makeUser("admin") }, - permissions: ["pages.update"] as AdminPermission[], }); const { plugin, seen } = makeRecordingPlugin(); for (const path of ["/admin", "/admin/pages", "/admin/api/content/x"]) { @@ -281,11 +313,10 @@ Deno.test("runPluginResponseTransforms: non-content route — page is null", asy // ── authz-first permission check (dec-identity-unification Phase 7) ───────────── // -// The `pages.update` gate must consult `authz.check()` when `authz` is -// configured, not the flat ROLE_PERMISSIONS table `auth.hasPermission()` -// alone would consult — same sole-authority contract as every other admin -// permission check (Phase 5c). These prove authz's answer wins over what -// auth.hasPermission() would say in either direction. +// The `pages.update` gate is `authz.check()`, full stop — same sole-authority +// contract as every other admin permission check (Phase 5c/6). No fallback +// table to compare against anymore; these just prove authz's answer is what +// actually governs `auth` in the returned context. function makeAuthz(allowed: boolean) { const calls = { check: 0 }; @@ -299,10 +330,9 @@ function makeAuthz(allowed: boolean) { }; } -Deno.test("runPluginResponseTransforms: authz.check() allowing wins even when ROLE_PERMISSIONS would deny", async () => { +Deno.test("runPluginResponseTransforms: authz.check() allowing grants access", async () => { const auth = makeAuth({ result: { authenticated: true, user: makeUser("author") }, - permissions: [] as AdminPermission[], // ROLE_PERMISSIONS alone would deny }); // deno-lint-ignore no-explicit-any const authz = makeAuthz(true) as any; @@ -321,10 +351,9 @@ Deno.test("runPluginResponseTransforms: authz.check() allowing wins even when RO assertEquals(seen[0].auth?.username, "alice"); }); -Deno.test("runPluginResponseTransforms: authz.check() denying wins even when ROLE_PERMISSIONS would allow", async () => { +Deno.test("runPluginResponseTransforms: authz.check() denying blocks access", async () => { const auth = makeAuth({ result: { authenticated: true, user: makeUser("editor") }, - permissions: ["pages.update"] as AdminPermission[], // ROLE_PERMISSIONS alone would allow }); // deno-lint-ignore no-explicit-any const authz = makeAuthz(false) as any; @@ -343,10 +372,9 @@ Deno.test("runPluginResponseTransforms: authz.check() denying wins even when ROL assertEquals(seen[0].auth, null); }); -Deno.test("runPluginResponseTransforms: falls back to ROLE_PERMISSIONS when authz is not passed", async () => { +Deno.test("runPluginResponseTransforms: fails closed (auth stays null) when authz is not passed — no ROLE_PERMISSIONS fallback", async () => { const auth = makeAuth({ result: { authenticated: true, user: makeUser("editor") }, - permissions: ["pages.update"] as AdminPermission[], }); const { plugin, seen } = makeRecordingPlugin(); await runPluginResponseTransforms({ @@ -358,13 +386,12 @@ Deno.test("runPluginResponseTransforms: falls back to ROLE_PERMISSIONS when auth config, adminPrefix: "/admin", }); - assertEquals(seen[0].auth?.username, "alice"); + assertEquals(seen[0].auth, null); }); -Deno.test("runPluginResponseTransforms: authz.check() denying scrubs markers even though ROLE_PERMISSIONS would allow", async () => { +Deno.test("runPluginResponseTransforms: authz.check() denying scrubs markers", async () => { const auth = makeAuth({ result: { authenticated: true, user: makeUser("editor") }, - permissions: ["pages.update"] as AdminPermission[], }); // deno-lint-ignore no-explicit-any const authz = makeAuthz(false) as any; @@ -438,7 +465,6 @@ Deno.test("marker scrub: forged/invalid session cookie still gets scrubbed", asy Deno.test("marker scrub: valid session WITHOUT pages.update gets scrubbed", async () => { const auth = makeAuth({ result: { authenticated: true, user: makeUser("author") }, - permissions: ["pages.read"] as AdminPermission[], }); const result = await runPluginResponseTransforms({ req: makeReq("/about", SESSION_COOKIE), @@ -455,13 +481,15 @@ Deno.test("marker scrub: valid session WITHOUT pages.update gets scrubbed", asyn Deno.test("marker scrub: valid editing session keeps markers", async () => { const auth = makeAuth({ result: { authenticated: true, user: makeUser("editor") }, - permissions: ["pages.update"] as AdminPermission[], }); + // deno-lint-ignore no-explicit-any + const authz = makeAuthz(true) as any; const result = await runPluginResponseTransforms({ req: makeReq("/about", SESSION_COOKIE), response: htmlResponse(MARKED_HTML), plugins: [], auth, + authz, resolve, config, adminPrefix: "/admin", @@ -502,7 +530,6 @@ Deno.test("marker scrub: runs after plugin transforms for anonymous requests", a Deno.test("runPluginResponseTransforms: ctx.plugins lets a bar-owning plugin collect another plugin's adminBarActions", async () => { const auth = makeAuth({ result: { authenticated: true, user: makeUser("admin") }, - permissions: ["pages.update"], }); // A plugin that only contributes adminBarActions — no transformResponse diff --git a/tests/runtime/server_test.ts b/tests/runtime/server_test.ts index c043e91b..1827ed32 100644 --- a/tests/runtime/server_test.ts +++ b/tests/runtime/server_test.ts @@ -32,17 +32,6 @@ import { clientIp, rememberSocketAddr } from "../../src/security/rate-limit.ts"; import type { DuneAuthSystem } from "../../src/auth/authz.ts"; import { USER_HEADER } from "../../src/auth/types.ts"; -function makeAdminAuth(allowed: boolean) { - const calls = { hasPermission: 0 }; - return { - calls, - hasPermission(_authResult: unknown, _permission: string): boolean { - calls.hasPermission++; - return allowed; - }, - }; -} - function makeAuthz(allowed: boolean) { const calls = { check: 0 }; return { @@ -57,50 +46,28 @@ function makeAuthz(allowed: boolean) { const authResult = { authenticated: true, user: { id: "u1" } }; -Deno.test("checkInlineEditPermission: authz.check() allowing wins even when ROLE_PERMISSIONS would deny", async () => { - const adminAuth = makeAdminAuth(false); +Deno.test("checkInlineEditPermission: allows when authz.check() allows", async () => { const authz = makeAuthz(true); const result = await checkInlineEditPermission( authz as unknown as DuneAuthSystem, - adminAuth, authResult, ); assertEquals(result, true); assertEquals(authz.calls.check, 1); - assertEquals(adminAuth.calls.hasPermission, 0); }); -Deno.test("checkInlineEditPermission: authz.check() denying wins even when ROLE_PERMISSIONS would allow", async () => { - const adminAuth = makeAdminAuth(true); +Deno.test("checkInlineEditPermission: denies when authz.check() denies", async () => { const authz = makeAuthz(false); const result = await checkInlineEditPermission( authz as unknown as DuneAuthSystem, - adminAuth, authResult, ); assertEquals(result, false); assertEquals(authz.calls.check, 1); - assertEquals(adminAuth.calls.hasPermission, 0); -}); - -Deno.test("checkInlineEditPermission: falls back to ROLE_PERMISSIONS when authz is undefined", async () => { - const adminAuth = makeAdminAuth(true); - const result = await checkInlineEditPermission( - undefined, - adminAuth, - authResult, - ); - assertEquals(result, true); - assertEquals(adminAuth.calls.hasPermission, 1); }); -Deno.test("checkInlineEditPermission: falls back to ROLE_PERMISSIONS denial when authz is undefined", async () => { - const adminAuth = makeAdminAuth(false); - const result = await checkInlineEditPermission( - undefined, - adminAuth, - authResult, - ); +Deno.test("checkInlineEditPermission: fails closed (denies) when authz is undefined, no ROLE_PERMISSIONS fallback", async () => { + const result = await checkInlineEditPermission(undefined, authResult); assertEquals(result, false); }); @@ -126,28 +93,27 @@ Deno.test("checkInlineEditPermissionForSiteUser: authz.check() denial wins even assertEquals(authz.calls.check, 1); }); -Deno.test("checkInlineEditPermissionForSiteUser: falls back to roles[] when authz is undefined — editor allowed", async () => { +Deno.test("checkInlineEditPermissionForSiteUser: fails closed (denies) when authz is undefined, no roles[] fallback", async () => { + // An editor role used to grant inline edit access here when authz was + // undefined — that fallback was removed: authz.check() is the sole + // authority, no exceptions. const result = await checkInlineEditPermissionForSiteUser(undefined, { id: "u1", roles: ["editor"], }); - assertEquals(result, true); -}); + assertEquals(result, false); -Deno.test("checkInlineEditPermissionForSiteUser: falls back to roles[] when authz is undefined — admin allowed", async () => { - const result = await checkInlineEditPermissionForSiteUser(undefined, { + const adminResult = await checkInlineEditPermissionForSiteUser(undefined, { id: "u1", roles: ["admin"], }); - assertEquals(result, true); -}); + assertEquals(adminResult, false); -Deno.test("checkInlineEditPermissionForSiteUser: falls back to roles[] when authz is undefined — no matching role denied", async () => { - const result = await checkInlineEditPermissionForSiteUser(undefined, { + const unknownRoles = await checkInlineEditPermissionForSiteUser(undefined, { id: "u1", roles: ["member", "subscriber"], }); - assertEquals(result, false); + assertEquals(unknownRoles, false); }); // ── stripUserHeader ────────────────────────────────────────────────────