refactor(auth)!: remove ROLE_PERMISSIONS, hasPermission() entirely (3.0.0) - #3
Merged
Merged
Conversation
….0.0) dec-identity-unification Phase 5c/6, finally closed. This was supposed to be fully replaced by the polizy authz system when it was first introduced and instead survived as a parallel, hand-maintained table kept in sync with @dune/core's canonical actionToRelations schema by convention only, not by anything enforcing it. - Deleted ROLE_PERMISSIONS (types.ts) and AuthMiddleware.hasPermission() (auth/middleware.ts) entirely, including the public mod.ts re-export. - checkPermission()/requirePermission() (routes/api/_utils.ts) now fail closed (deny) when authz is undefined -- an in-process object construction failing at startup, essentially never hit in practice -- instead of silently degrading to the removed table. - routes/_layout.tsx's sidebar nav filtering, the one place that used ROLE_PERMISSIONS unconditionally rather than as a fallback, now reads a real permission set computed via authz.check() once per request (routes/_middleware.ts's new computeNavPermissions(), threaded through AdminState.permissions) instead of a table that could silently drift from what a route's own check would actually decide. No first-party or third-party plugin found calling the removed surface directly (checked @dune/plugin-inline-edit, @dune/plugin-meilisearch, @dune/plugin-orama, @dune/plugin-pdf) -- @dune/plugin-inline-edit's own auth.hasPermission(permission) call goes through @dune/core's published, synchronous HookContext.auth.hasPermission() hook API, whose contract is unchanged (companion @dune/core change: it now sources its answer from roleHasPermission(), a synchronous read of @dune/core's own canonical schema, instead of this package's table). 227 tests pass (5 new: computeNavPermissions()); deno check/lint clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…defined The _middleware.ts gate skipped the access check when adminCtx.authz was undefined (authz creation failed at startup), relying on every route's own checkPermission()/requirePermission() call to fail closed. But the PR claim is "authz.check() is the sole authority, no exceptions" — and any route that relies on this gate alone would be exposed to any authenticated user (of any role) during that state. The gate now returns 403 in that state, the same policy as every route-level check. csp_test's fixture needed an always-allowing authz mock: without one the authenticated render it tests never happens (403 before render). Found during security audit of dune#15 / plugin-admin#3 (Finding 4).
role-utils.ts's ROLE_RANK/highestValidRole() reimplemented the identical admin-tier rank table @dune/core's new highestAdminRole() (bdeb897, added to fix roles[0] under-privileging ResponseTransformContext) already has -- exactly the "two tables kept in sync by convention" pattern this release's ROLE_PERMISSIONS removal was about eliminating, just running in the other direction. Now derives ROLE_RANK/highestValidRole() from @dune/core's ADMIN_ROLE_RANK/highestAdminRole() (@dune/core/auth/authz-schema, companion @dune/core change adds the export) instead of keeping a second copy. VALID_ROLES/sanitizeRole()/withRole() unaffected. No behavior change; 4 new tests lock in the delegation. 232 tests pass, deno check/lint clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…llback authz is still up when bootstrapAdminTuples() throws; the access gate denies. The old warn told operators the panel still worked via a table that no longer exists. Co-authored-by: Cursor <cursoragent@cursor.com>
The name only existed on ROLE_PERMISSIONS. The schema action is "access" on app:admin, already enforced by the middleware gate. Leaving it on the union invited plugin pages that would 403 after the fallback was removed. Co-authored-by: Cursor <cursoragent@cursor.com>
AdminContext.auth.hasPermission() is gone; missing authz denies. Co-authored-by: Cursor <cursoragent@cursor.com>
role-utils.ts already imports ADMIN_ROLE_RANK from that subpath. Keep the 0.34 pin (per-minor); 0.34.1 on JSR does not have the export yet. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
dec-identity-unification Phase 5c/6, finally closed. This was supposed to be fully replaced by the polizy
authzsystem when it was first introduced and instead survived as a parallel, hand-maintained table kept in sync with@dune/core's canonicalactionToRelationsschema by convention only, not by anything enforcing it.ROLE_PERMISSIONS(types.ts) andAuthMiddleware.hasPermission()(auth/middleware.ts) entirely, including the publicmod.tsre-export.checkPermission()/requirePermission()(routes/api/_utils.ts) now fail closed (deny) whenauthzis undefined — an in-process object construction failing at startup, essentially never hit in practice — instead of silently degrading to the removed table.routes/_layout.tsx's sidebar nav filtering, the one place that usedROLE_PERMISSIONSunconditionally rather than as a fallback, now reads a real permission set computed viaauthz.check()once per request (routes/_middleware.ts's newcomputeNavPermissions(), threaded throughAdminState.permissions) instead of a table that could silently drift from what a route's own check would actually decide.Blast-radius check, not just an assumption: searched
@dune/plugin-inline-edit,@dune/plugin-meilisearch,@dune/plugin-orama,@dune/plugin-pdffor direct calls to the removed surface — none found.@dune/plugin-inline-edit's ownauth.hasPermission(permission)call goes through@dune/core's published, synchronousHookContext.auth.hasPermission()hook API, whose contract is unchanged (companion@dune/corePR: it now sources its answer fromroleHasPermission(), a synchronous read of@dune/core's own canonical schema, instead of this package's table). See duneorg/dune#15.227 tests pass (5 new:
computeNavPermissions());deno check/deno lintclean.Depends on / pairs with duneorg/dune#15 — merge that first or alongside this one.