fix(admin): restrict admin-only routes to administrators - #3020
fix(admin): restrict admin-only routes to administrators#3020jmirchandani wants to merge 4 commits into
Conversation
The user menu showed a Settings link to every role, and admin-only routes mounted for anyone who typed their URL. The sidebar and command palette already hide these entries below Admin, but that is navigation, not a guard: a non-admin could still open the /settings hub and its site-level pages, /users, /import/wordpress and a plugin's settings page, where requests the server rejects left screens that failed to load or could not save. Add a RequireAdmin route guard, using the same in-component pattern the byline schema page already uses, around those routes, and show the user menu's Settings link only to admins. /settings/security stays open to every role because it manages the signed-in user's own passkeys. Editors lose read access to General, Social Links and SEO, which the server allows at settings:read, and non-admins lose the language switcher on the Settings hub. Both are deliberate and described in the changeset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AznEK6RvKGboBkFg4XqadT
🦋 Changeset detectedLatest commit: fd94ab0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Scope checkThis PR changes 599 lines across 7 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the right fix for the right problem. Non-admin users could indeed land on admin-only UI routes by typing URLs, and the in-component RequireAdmin guard matches the existing BylineSchemaPage pattern while respecting the server’s actual RBAC tiers: it gates the settings hub and its admin-only sub-pages, /users, /import/wordpress, and auto-generated plugin settings, but deliberately leaves /content-types, /plugins-manager, and the marketplaces unwrapped because the server already allows Editor reads there. I confirmed against packages/auth/src/rbac.ts, packages/core/src/astro/routes/api/settings.ts, the plugin settings endpoint, Sidebar.tsx, MediaUsageSettings.tsx, and Settings.tsx that the role checks and the language-switcher claim line up. Tests are behavior-oriented and provide good regression coverage.
The only real issues are large-scale violations of the repo’s comment-discipline conventions. Several new files and route definitions carry essay-length comments that justify the change, reference internal cards (emdb-023), cite specific line numbers in other files, and narrate rejected alternatives. Per AGENTS.md, comments are for future readers, not reviewers; justification, cross-references, and issue/PR numbers should live in the PR description and commit message. These should be pruned before merge. The code underneath is sound.
What I checked:
- Full diff and changed files (
RequireAdmin.tsx,Header.tsx,router.tsx, tests, changeset) - Auth RBAC and the relevant server-side permission checks
Sidebar.tsxnav gating for consistencyMediaUsageSettings.tsxown admin gateSettings.tsxlanguage switcher location- Changeset against
.changeset/README.md— the changeset is user-facing and accurate, no issue found
Trim review-narrative comments from the admin gate change down to short factual ones per house style, and shorten two describe() titles in router-admin-gate.test.tsx. No executable code changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AznEK6RvKGboBkFg4XqadT
github-actions
Bot
added
review/needs-rereview
# Conflicts: # packages/admin/tests/components/Header.test.tsx
1 participant
What does this PR do?
Non-admin users could reach admin-only screens in the admin UI. The sidebar and command palette already hide these entries below Admin, but the user menu showed a Settings link to every role, and the admin-only routes mounted for anyone who typed their URL. There, requests the server rejects left screens that failed to load or could not save.
This PR:
RequireAdminroute guard, the same in-component pattern the byline schema page already uses: a loader while the current user loads, then either the page or an "Access denied" screen;/settingshub and its site-level pages (General, Social Links, SEO, Self-Signup Domains, API Tokens, Email, Backups),/users,/import/wordpressand/plugins-manager/$pluginId/settingsin that guard;Deliberately left open:
/settings/security(each user's own passkeys) and/settings/media-usage(it already has its own admin gate)./content-types,/plugins-manager,/plugins/marketplaceand/themes/marketplacestay reachable for Editors because the server allows their reads at Editor.Behaviour change for Editors, also stated in the changeset: Editors can no longer open Settings → General, Social Links and SEO. The server allows reading those at
settings:read, but saving already required Admin. Non-admins also lose the language switcher that lives on the Settings hub; they can still choose a language on the sign-in screen.There is no existing issue; this was found while auditing role access across the admin UI.
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
Screenshots: captured with Playwright from
demos/simplerunning this branch (b873df21), signed in as the dev user, first with the Admin role and then with the Editor role.Admin, user menu: "Security Settings" and "Settings" (unchanged).
Editor, user menu: "Security Settings" only; the "Settings" link is gone.
Editor, opening
/_emdash/admin/settingsby URL: "Access denied" with "You need admin permissions to view this page." instead of the page.Editor, opening
/_emdash/admin/settings/security: unchanged, still works.The "Plugin error (403)" card on the Editor dashboard comes from the demo's audit history plugin widget; this PR does not touch plugins.
Tests (
packages/admin):npx vitest run router-admin-gate RequireAdmin Header tests/components/settings/→ 13 test files, 136 tests passed.router-admin-gate.test.tsxmounts every wrapped route as an Editor and asserts the "Access denied" screen (and the page itself for an Admin), with a mirror table asserting the four Editor-reachable routes stay reachable; removing any wrap makes its case fail.Checks:
pnpm build,pnpm typecheckandpnpm lintpass. Formatting was checked on the changed files with bothoxfmtandprettier; a repo-widepnpm format:checkon this Windows checkout flags nearly every file because of CRLF line endings, which is unrelated to this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01AznEK6RvKGboBkFg4XqadT