diff --git a/CHANGELOG.md b/CHANGELOG.md index d13060b..09a204d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,37 @@ follows [Semantic Versioning](https://semver.org). --- +## [Unreleased] + +### Fixed + +- **A third-party plugin's own `mount()`-registered route could never see + `ctx.state.adminContext` or `ctx.state.auth`**, no matter how the request + was authenticated — `withGuards()` always treated it as unauthenticated. + Root cause: Fresh 2 snapshots each route's middleware chain at the moment + the route is registered, and `@dune/core`'s `mountPlugins()` calls every + plugin's `mount()` in registration order — this plugin always registers + last (`bootstrap()` can't construct it until authz/hmac-key/history are + ready), so any other plugin's own route was compiled before this plugin's + `ctx.state.adminContext = …` and admin-auth middleware existed. +- **Implements `DunePlugin.mountEarly()`** (new in `@dune/core@0.34.4`) to + fix it: `mountDuneAdminEarly()` registers just the `ctx.state.adminContext` + and admin-auth middleware, before any plugin's `mount()` runs — the rest + of `mountDuneAdmin()` (admin panel routes, plugin admin pages, the + `registerPluginPublicRoutes()` call) stays exactly where it was. See + `mount.ts`'s doc comments for the full reasoning, and `@dune/core`'s own + changelog for why this couldn't be a plain reordering. +- A plugin's own guarded route must be registered **under the admin path + prefix** (default `/admin`, e.g. `/admin/my-plugin/…`) — the admin auth + middleware that populates `ctx.state.auth` only runs for requests under + that prefix, same as every built-in admin route. Documented in + `admin/guards.ts`'s own example and in the plugin-authoring guide + (duneorg/dune-docs#4). + +### Requires + +- `@dune/core@^0.34.4` or later (for `DunePlugin.mountEarly()`). + ## [3.0.0] — 2026-08-29 ### Breaking diff --git a/deno.json b/deno.json index a6dc0a0..2ea7765 100644 --- a/deno.json +++ b/deno.json @@ -1,6 +1,6 @@ { "name": "@dune/plugin-admin", - "version": "3.0.0", + "version": "3.1.0", "license": "MIT", "minimumDependencyAge": { "age": "P1D", @@ -18,7 +18,7 @@ "./admin/auth/provider": "./src/admin/auth/provider.ts" }, "imports": { - "@dune/core": "jsr:@dune/core@^0.34.2", + "@dune/core": "jsr:@dune/core@^0.34.4", "fresh": "jsr:@fresh/core@^2", "preact": "npm:preact@^10", "preact/hooks": "npm:preact@^10/hooks", diff --git a/mod.ts b/mod.ts index 8e84705..df030c7 100644 --- a/mod.ts +++ b/mod.ts @@ -44,7 +44,7 @@ import type { AuthzDbAdapter } from "@dune/core/auth/authz-adapter-db"; import { initAdminContext } from "./src/admin/context.ts"; import type { AdminContext } from "./src/admin/context.ts"; import { createBlockEditorPlugin } from "./src/admin/block-editor-plugin.tsx"; -import { mountDuneAdmin, getDuneAdminIslands } from "./src/admin/mount.ts"; +import { getDuneAdminIslands, mountDuneAdmin, mountDuneAdminEarly } from "./src/admin/mount.ts"; import { logger } from "@dune/core/logger"; import { sectionRegistry } from "@dune/core/sections"; import type { SectionRegistry } from "@dune/core/sections"; @@ -103,6 +103,12 @@ export function createAdminPlugin( opts: AdminPluginOptions, ): DunePlugin { let setupState: AdminSetupState | null = null; + // Set partway through mount(), once AdminContext is actually built. + // mountEarly() registers a middleware that reads this lazily (per + // request) rather than needing the built value up front — see + // mountDuneAdminEarly()'s doc comment for why that's necessary (it runs + // before mount() has built anything). + let liveAdminContext: AdminContext | null = null; return { name: "dune-admin", @@ -111,6 +117,12 @@ export function createAdminPlugin( hooks: {}, islandSpecifiers: getDuneAdminIslands(), + mountEarly({ app, bootstrap }: MountApi) { + if (!setupState) return; // admin disabled + // deno-lint-ignore no-explicit-any + mountDuneAdminEarly(app as any, bootstrap, () => liveAdminContext); + }, + async setup(_api) { const adminCfg = config.admin ?? { path: "/admin", @@ -336,6 +348,12 @@ export function createAdminPlugin( // Keep the singleton for single-site serve paths (serve.ts, dev.ts). initAdminContext(adminContext); + // Unblocks mountEarly()'s ctx.state.adminContext middleware — see its + // doc comment. Must happen before any other plugin's route could + // conceivably run, which it does: mountPlugins() finishes every + // plugin's mount() before the app starts serving requests. + liveAdminContext = adminContext; + // Expose adminContext on the bootstrap result so serve.ts can wire the // job scheduler into it after mount() returns. BootstrapResult types // this loosely (Record) since core has no concrete diff --git a/scripts/generate-core-imports.ts b/scripts/generate-core-imports.ts index 019df8b..3361689 100644 --- a/scripts/generate-core-imports.ts +++ b/scripts/generate-core-imports.ts @@ -43,15 +43,22 @@ * auto-tracks patch releases within that minor — confirmed via `deno cache`: * "0.28" resolved to the latest 0.28.x, not just 0.28.0. * - * Currently floored at the patch, not the minor (^0.34.2, not "0.34"): - * `./auth/authz-schema` (needed for 3.0.0's ROLE_PERMISSIONS removal) was - * added mid-minor at 0.34.2, not at 0.34.0 — a bare "0.34" floors JSR's - * publish-time subpath check at 0.34.0, which doesn't have the export yet. - * Reproduced live 2026-08-31: "invalid 'jsr:' dependency subpath: - * '@dune/core@0.34/auth/authz-schema', resolved to 0.34.0, has no export". - * Safe to widen back to a bare "0.35" (or whatever) at the next minor bump, - * once every subpath this package needs exists from that minor's .0. */ -const CORE_RANGE = "^0.34.2"; + * Currently floored at the patch, not the minor (^0.34.4, not "0.34"): + * `DunePlugin.mountEarly()` (needed for the mountEarly()-based adminContext + * fix) was added mid-minor at 0.34.4. `./auth/authz-schema` (needed + * earlier for 3.0.0's ROLE_PERMISSIONS removal) was itself already a + * mid-minor addition at 0.34.2 — a bare "0.34" floors JSR's publish-time + * check at 0.34.0, which has neither. Reproduced live 2026-08-31: "invalid + * 'jsr:' dependency subpath: '@dune/core@0.34/auth/authz-schema', resolved + * to 0.34.0, has no export". Whenever this package starts depending on + * something added at a *later* patch within the same minor, re-floor to + * that patch — this file's own CORE_RANGE update has been missed twice now + * (deno.json hand-edited without updating this constant to match) because + * nothing forces the two together outside of `check:core-imports` + * actually being run before merge. Safe to widen back to a bare "0.35" (or + * whatever) once every subpath/field this package needs exists from that + * minor's .0. */ +const CORE_RANGE = "^0.34.4"; const DENO_JSON = "deno.json"; const SCAN_ROOTS = ["mod.ts", "src", "tests"]; diff --git a/src/admin/mount.ts b/src/admin/mount.ts index 7bddb5c..ee4f199 100644 --- a/src/admin/mount.ts +++ b/src/admin/mount.ts @@ -53,11 +53,113 @@ import { handleIncomingWebhook, } from "./public-api.ts"; +/** + * `ctx` objects (`BootstrapResult`) for which the `ctx.state.adminContext` + * middleware has already been registered on `app` — makes a second + * registration for the same bootstrap a no-op instead of stacking a + * duplicate middleware. Both `mountDuneAdminEarly()` (called from the admin + * plugin's `mountEarly()` in the normal `createDuneApp()` path) and + * `mountDuneAdmin()` itself (headless-mode direct callers, who never call + * `mountEarly()` at all) can reach this — whichever runs first wins, and the + * other silently skips. + */ +const contextMiddlewareRegisteredFor = new WeakSet(); + +/** + * Register the `ctx.state.adminContext = …` middleware every admin route + * handler (and `@dune/plugin-admin`'s `withGuards()`, and any other + * plugin's own guarded route) reads. Takes a getter rather than the context + * value itself so it can be registered before the `AdminContext` object + * exists yet — see `mountDuneAdminEarly()`. + * + * Exported so a headless-mode caller with an unusual setup can register this + * independently of `mountDuneAdmin()`'s other route registration, but the + * normal paths (`mountDuneAdmin()`, `mountDuneAdminEarly()`) already call + * this — most callers should use one of those instead. + */ +export function registerAdminContextMiddleware( + // deno-lint-ignore no-explicit-any + app: App, + ctx: BootstrapResult, + getAdminContext: () => import("./context.ts").AdminContext | null, +): void { + if (contextMiddlewareRegisteredFor.has(ctx)) return; + contextMiddlewareRegisteredFor.add(ctx); + app.use(async (fc) => { + fc.state.adminContext = getAdminContext(); + return fc.next(); + }); +} + +/** `App` instances the admin auth middleware has already been registered on — see {@link registerAdminAuthMiddleware}. */ +const authMiddlewareRegisteredFor = new WeakSet(); + +/** + * Register the admin auth middleware (`routes/_middleware.ts`'s `handler` — + * authenticates the session cookie into `ctx.state.auth`, applies admin CSP + * headers). No-op if already registered on this `app` — see + * {@link mountDuneAdminEarly}, whose whole reason for existing is to + * register this (and {@link registerAdminContextMiddleware}) before other + * plugins' routes are compiled; `registerAdminRoutes()` calls this too, for + * headless-mode callers who never call `mountDuneAdminEarly()`. + */ +function registerAdminAuthMiddleware( + // deno-lint-ignore no-explicit-any + app: App, +): void { + if (authMiddlewareRegisteredFor.has(app)) return; + authMiddlewareRegisteredFor.add(app); + // deno-lint-ignore no-explicit-any + app.use(adminMiddleware.handler as Middleware); +} + +/** + * Register the middleware every other plugin's own routes need to already + * be in place before it runs: + * + * - `ctx.state.adminContext` (see {@link registerAdminContextMiddleware}) + * - `ctx.state.auth` (the admin auth middleware — sets it for any request + * under the admin path prefix; a plugin's own guarded route must be + * registered under that prefix too, e.g. `/admin/my-plugin/…`, to be + * reached by it — see `@dune/plugin-admin/admin/guards`' own doc comment) + * + * This is the admin plugin's `DunePlugin.mountEarly()` implementation (see + * that hook's doc comment in `@dune/core`'s `hooks/types.ts` for why this + * needs to be a separate pass from `mountDuneAdmin()`/`mount()`). Called + * before the `AdminContext` itself has been built — `getAdminContext` is + * read lazily, per-request, so it naturally resolves to the real value once + * `mount()` (which runs after every plugin's `mountEarly()`) finishes + * building it, well before the app starts serving requests. The auth + * middleware itself needs no such laziness — it's a static generated route + * module, not something built from `AdminContext`. + * + * Headless-mode callers using `mountDuneAdmin()` directly (see this + * module's own docstring) don't need this — they already have a built + * `AdminContext` before they call anything, and their own routes are + * registered by them, after `mountDuneAdmin()`, per that docstring's + * example — so `mountDuneAdmin()`'s own (immediate, non-lazy) registration + * already covers them. + * + * @since 3.1.0 + */ +export function mountDuneAdminEarly( + // deno-lint-ignore no-explicit-any + app: App, + ctx: BootstrapResult, + getAdminContext: () => import("./context.ts").AdminContext | null, +): void { + if (ctx.config.admin?.enabled === false) return; + registerAdminContextMiddleware(app, ctx, getAdminContext); + registerAdminAuthMiddleware(app); +} + /** * Mount Dune's admin panel and public API routes onto an existing Fresh app. * * Registers: - * - Per-site admin context middleware (required for admin route handlers) + * - Per-site admin context middleware (required for admin route handlers) — + * a no-op here if `mountDuneAdminEarly()` already registered it (see + * `contextMiddlewareRegisteredFor`). * - Admin file-system routes under `adminPrefix` (default `/admin`) * - Plugin admin pages (programmatic routes registered by plugins) * - Plugin public routes (registered by plugins) @@ -81,13 +183,11 @@ export async function mountDuneAdmin( // ── Admin panel ───────────────────────────────────────────────────────────── if (config.admin?.enabled !== false) { // Per-site admin context middleware — captures the correct context in - // closure so multisite setups don't suffer from the module-level singleton. + // closure so multisite setups don't suffer from the module-level + // singleton. No-op if mountDuneAdminEarly() already registered it for + // this ctx (the normal createDuneApp() + mountPlugins() path). if (adminContext) { - const adminCtx = adminContext; - app.use(async (fc) => { - fc.state.adminContext = adminCtx; - return fc.next(); - }); + registerAdminContextMiddleware(app, ctx, () => adminContext); } // Admin routes (login, pages, users, settings, …) — registered @@ -203,7 +303,10 @@ export function getDuneAdminIslands(): string[] { * The admin auth middleware and shell layout are registered app-wide; both * self-guard and pass through untouched on non-admin paths (they must, * because Fresh applies fs middleware/layouts globally too — this preserves - * the exact fsRoutes() behavior). + * the exact fsRoutes() behavior). The auth middleware registration itself + * is a no-op here if `mountDuneAdminEarly()` already registered it (see + * `registerAdminAuthMiddleware`) — the normal `createDuneApp()` + + * `mountPlugins()` path. */ export function registerAdminRoutes( // deno-lint-ignore no-explicit-any @@ -212,8 +315,7 @@ export function registerAdminRoutes( ): void { const prefix = adminPrefix === "/" ? "" : adminPrefix.replace(/\/+$/, ""); - // deno-lint-ignore no-explicit-any - app.use(adminMiddleware.handler as Middleware); + registerAdminAuthMiddleware(app); // deno-lint-ignore no-explicit-any app.layout("*", adminLayout.default as any); diff --git a/tests/admin/mount_early_test.ts b/tests/admin/mount_early_test.ts new file mode 100644 index 0000000..4849d13 --- /dev/null +++ b/tests/admin/mount_early_test.ts @@ -0,0 +1,82 @@ +/** + * Regression test for the third-party-plugin-guarded-route bug: a plugin's + * own `mount()`-registered route (registered under the admin prefix, e.g. + * `/admin/my-plugin/…` — see `admin/guards.ts`'s own doc comment for why it + * has to be under the prefix) previously read both `ctx.state.adminContext` + * and `ctx.state.auth` as `undefined`, no matter how the request was + * authenticated, whenever that route was registered before `mountDuneAdmin()` + * ran — which is the normal case: `@dune/core`'s `mountPlugins()` calls + * `mount()` in plugin registration order, and the built-in admin plugin + * always registers last. + * + * `mountDuneAdminEarly()` (this plugin's `mountEarly()` implementation, see + * `@dune/core`'s `hooks/types.ts` for why that hook exists) registers both + * pieces of middleware — `ctx.state.adminContext` and the admin auth + * middleware that sets `ctx.state.auth` — before any plugin's `mount()` + * runs. This test registers a fake plugin's route first, then calls + * `mountDuneAdminEarly()` and `mountDuneAdmin()` in the same relative order + * `mountPlugins()` uses (early hooks before any mount(), admin's own + * mount()-equivalent last), and confirms both are visible. + */ + +import { assertEquals } from "https://deno.land/std@0.224.0/assert/mod.ts"; +import { App } from "fresh"; +import type { BootstrapResult } from "@dune/core/bootstrap"; +import { mountDuneAdmin, mountDuneAdminEarly } from "../../src/admin/mount.ts"; +import type { AdminContext } from "../../src/admin/context.ts"; + +Deno.test( + "mountDuneAdminEarly: a plugin's route registered before mountDuneAdmin() still sees adminContext and auth", + { sanitizeOps: false, sanitizeResources: false }, + async () => { + // deno-lint-ignore no-explicit-any + const app = new App(); + + // Minimal fake AdminContext — enough for routes/_middleware.ts's handler + // to authenticate the request and call ctx.next() rather than + // redirecting/403ing before it reaches our probe route. + const adminContext = { + prefix: "/admin", + auth: { + authenticate: () => + Promise.resolve({ + authenticated: true, + user: { id: "u1", roles: ["admin"] }, + }), + }, + authz: { + check: () => Promise.resolve(true), + }, + // deno-lint-ignore no-explicit-any + } as any as AdminContext; + + // deno-lint-ignore no-explicit-any + const ctx = { config: { admin: { enabled: true, path: "/admin" } } } as any as BootstrapResult; + + // Phase 1 (mountEarly, admin first) — the fix under test. + mountDuneAdminEarly(app, ctx, () => adminContext); + + // A fake plugin's own mount()-registered route, added here to mirror + // Phase 2 registration order (this plugin runs before admin's own + // mount()-equivalent below, exactly as it would in the real + // mountPlugins() loop). + app.get("/admin/my-plugin/probe", (fc) => { + return Response.json({ + // deno-lint-ignore no-explicit-any + sawAdminContext: (fc.state as any).adminContext != null, + // deno-lint-ignore no-explicit-any + sawAuth: (fc.state as any).auth != null, + }); + }); + + // Phase 2, admin last — mirrors mountPlugins()'s own ordering. + await mountDuneAdmin(app, ctx, adminContext); + + const handler = app.handler(); + const res = await handler( + new Request("http://localhost:8000/admin/my-plugin/probe"), + ); + assertEquals(res.status, 200); + assertEquals(await res.json(), { sawAdminContext: true, sawAuth: true }); + }, +);