From f30d1f40ce41e786bcdaaa93149a0aed656c9607 Mon Sep 17 00:00:00 2001 From: zumbrunn Date: Wed, 11 Nov 2026 10:02:31 +0100 Subject: [PATCH] fix: list pages contributed to admin.pages in the sidebar (#27) Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 9 ++++++++ src/admin/contributions.ts | 4 ++-- src/admin/nav.ts | 4 +--- src/admin/routes/_layout.tsx | 12 ++++++++-- src/admin/routes/_middleware.ts | 36 +++++++++++++++++++++-------- src/admin/types.ts | 6 +++-- tests/admin/layout_test.tsx | 36 +++++++++++++++++++++++++++++ tests/admin/nav_permissions_test.ts | 26 +++++++++++++++++++++ 8 files changed, 114 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7faa295..98e1643 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,15 @@ follows [Semantic Versioning](https://semver.org). ## [Unreleased] +### Fixed + +- **Pages contributed to `admin.pages` now appear in the sidebar (#27).** They were mounted as routes but never + listed. They show under a "plugins" group to the users who hold the page's `permission`. The request's permission + set now also covers the permissions that contributed pages declare, so a custom permission (one that is not a core + `pages.*` / `media.*` style action) works too; a permission authz cannot evaluate counts as denied. `icon` is + accepted but not rendered, as for core items. `AdminState.permissions` and `NavItem.permission` are now plain + strings. + ## [4.0.0] — 2026-11-10 Pairs with `@dune/core` 0.36 and needs it. The admin is a plugin you list under `plugins:` in `site.yaml` diff --git a/src/admin/contributions.ts b/src/admin/contributions.ts index 955f1ad..c98f665 100644 --- a/src/admin/contributions.ts +++ b/src/admin/contributions.ts @@ -35,9 +35,9 @@ export interface AdminPageRegistration { * Example: `/my-plugin` registers at `/admin/my-plugin`. */ path: string; - /** Human-readable nav label shown in the admin sidebar */ + /** Human-readable nav label shown in the admin sidebar, under "plugins", to users who hold `permission` */ label: string; - /** Optional icon — emoji or inline SVG */ + /** Optional icon. Accepted but not rendered: the sidebar shows labels only. */ icon?: string; /** * Admin permission required to view this page. Required, like diff --git a/src/admin/nav.ts b/src/admin/nav.ts index 16dd6a7..7215627 100644 --- a/src/admin/nav.ts +++ b/src/admin/nav.ts @@ -3,8 +3,6 @@ * Core items are defined here; plugins append via registerNavItem() at bootstrap. */ -import type { AdminPermission } from "./types.ts"; - export interface NavItem { label: string; /** Path relative to the admin prefix, e.g. "/pages" */ @@ -15,7 +13,7 @@ export interface NavItem { /** If true, only admin-role users see this item */ adminOnly?: boolean; /** Required permission to see this item */ - permission?: AdminPermission; + permission?: string; /** Display order within the group (lower = higher) */ order?: number; } diff --git a/src/admin/routes/_layout.tsx b/src/admin/routes/_layout.tsx index 258c007..a8c55a2 100644 --- a/src/admin/routes/_layout.tsx +++ b/src/admin/routes/_layout.tsx @@ -6,7 +6,7 @@ import type { h } from "preact"; import type { AdminState } from "../types.ts"; -import { getNavItems } from "../nav.ts"; +import { getNavItems, type NavItem } from "../nav.ts"; import { isRtl } from "@dune/core/i18n"; import { highestValidRole } from "../auth/role-utils.ts"; import { @@ -94,7 +94,15 @@ export default function AdminLayout( // would actually decide for a route. const userPermissions = state.permissions ?? []; - const allNavItems = getNavItems(); + // Pages that plugins contributed to `admin.pages`, in their own group after + // the core ones. Each is shown to the users who hold its declared permission. + const pluginNavItems: NavItem[] = (state.adminContext.pluginPages ?? []).map((page) => ({ + label: page.label, + path: page.path, + group: "plugins", + permission: page.permission, + })); + const allNavItems = [...getNavItems(), ...pluginNavItems]; const navItems = allNavItems.filter((item) => { if (item.adminOnly && role !== "admin") return false; if (item.permission && !userPermissions.includes(item.permission)) { diff --git a/src/admin/routes/_middleware.ts b/src/admin/routes/_middleware.ts index 595d1a5..f38f36f 100644 --- a/src/admin/routes/_middleware.ts +++ b/src/admin/routes/_middleware.ts @@ -169,20 +169,35 @@ const NAV_PERMISSIONS: readonly AdminPermission[] = [ export async function computeNavPermissions( authz: DuneAuthSystem | undefined, userId: string, -): Promise { + /** + * Permissions that plugin-contributed pages declare (`ADMIN_PAGES`). They are + * checked in addition to the core list so the sidebar can show a contributed + * page to the users who may open it. A permission name authz cannot evaluate + * counts as denied; it must not break the page for everyone. + */ + extraPermissions: readonly string[] = [], +): Promise { if (!authz) return []; + const all = [...new Set([...NAV_PERMISSIONS, ...extraPermissions])]; + const coreSet = new Set(NAV_PERMISSIONS); const checks = await Promise.all( - NAV_PERMISSIONS.map(async (permission) => { - const allowed = await authz.check({ - who: { type: "user", id: userId }, - // deno-lint-ignore no-explicit-any - canThey: permission as any, - onWhat: { type: "app", id: "site" }, - }); - return allowed ? permission : null; + all.map(async (permission) => { + try { + const allowed = await authz.check({ + who: { type: "user", id: userId }, + // deno-lint-ignore no-explicit-any + canThey: permission as any, + onWhat: { type: "app", id: "site" }, + }); + return allowed ? permission : null; + } catch (err) { + // A core permission failing is a real fault; a plugin's custom one is the plugin's. + if (coreSet.has(permission)) throw err; + return null; + } }), ); - return checks.filter((p): p is AdminPermission => p !== null); + return checks.filter((p): p is string => p !== null); } export async function handler( @@ -240,6 +255,7 @@ export async function handler( ctx.state.permissions = await computeNavPermissions( adminCtx.authz, authResult.user.id, + (adminCtx.pluginPages ?? []).map((page) => page.permission), ); } diff --git a/src/admin/types.ts b/src/admin/types.ts index 6890004..2810802 100644 --- a/src/admin/types.ts +++ b/src/admin/types.ts @@ -101,9 +101,11 @@ export interface AdminState { * once per request by `routes/_middleware.ts` (`computeNavPermissions()`) * and read synchronously by `routes/_layout.tsx` for sidebar nav * filtering. Undefined for an unauthenticated request. Replaces the flat - * `ROLE_PERMISSIONS[role]` lookup removed in 3.0.0. + * `ROLE_PERMISSIONS[role]` lookup removed in 3.0.0. Includes the permissions + * that plugin-contributed pages declare, so it is a list of strings, not only + * `AdminPermission`s. */ - permissions?: AdminPermission[]; + permissions?: string[]; } /** Convert User to safe API response */ diff --git a/tests/admin/layout_test.tsx b/tests/admin/layout_test.tsx index 9fe3ad2..3af6496 100644 --- a/tests/admin/layout_test.tsx +++ b/tests/admin/layout_test.tsx @@ -74,3 +74,39 @@ Deno.test("_layout: wraps a normal authenticated admin route in the sidebar/topb assertStringIncludes(html, "admin-topbar"); assertStringIncludes(html, 'name="csrf-token"'); }); + +function sidebarFor(permissions: string[], roles = ["editor"]) { + return render( + new Response("ok") }, + { path: "/reports", label: "Reports", permission: "reports.view", handler: () => new Response("ok") }, + ], + }, + auth: { user: { name: "Ed", roles }, session: { id: "s" } }, + permissions, + // deno-lint-ignore no-explicit-any + } as any} + url={new URL("http://localhost/admin/")} + />, + ); +} + +Deno.test("_layout: a contributed page is listed for a user holding its permission", () => { + const html = sidebarFor(["billing.manage"]); + assertStringIncludes(html, "Ausgaben"); + assertStringIncludes(html, 'href="/admin/ausgaben"'); + assertStringIncludes(html, ">plugins<"); +}); + +Deno.test("_layout: a contributed page is not listed without its permission, and only the permitted ones show", () => { + const html = sidebarFor(["reports.view"]); + assertEquals(html.includes("Ausgaben"), false); + assertStringIncludes(html, "Reports"); + assertEquals(sidebarFor([]).includes(">plugins<"), false); +}); diff --git a/tests/admin/nav_permissions_test.ts b/tests/admin/nav_permissions_test.ts index e6639f4..eaf507f 100644 --- a/tests/admin/nav_permissions_test.ts +++ b/tests/admin/nav_permissions_test.ts @@ -103,3 +103,29 @@ Deno.test("_middleware handler: authenticated request with no authz gets 403 (fa assertEquals(res.status, 403); assertEquals(nextRan, false); }); + +// ── permissions declared by plugin-contributed pages ── + +Deno.test("computeNavPermissions: also checks the permissions plugin pages declare", async () => { + const authz = makeAuthz(["billing.manage"]); + const result = await computeNavPermissions( + authz as unknown as DuneAuthSystem, + "u1", + ["billing.manage", "pages.read", "reports.view"], + ); + assertEquals(result, ["billing.manage"]); + // pages.read is both a core and a declared permission: checked once. + assertEquals(authz.calls.filter((c) => c === "pages.read").length, 1); +}); + +Deno.test("computeNavPermissions: a plugin permission authz cannot evaluate is denied, not fatal", async () => { + const authz = { + // deno-lint-ignore no-explicit-any + check(args: any): Promise { + if (args.canThey === "weird.action") return Promise.reject(new Error("unknown action")); + return Promise.resolve(args.canThey === "pages.read"); + }, + }; + const result = await computeNavPermissions(authz as unknown as DuneAuthSystem, "u1", ["weird.action"]); + assertEquals(result, ["pages.read"]); +});