From 76845ab4626b52c2f33372998fa83dc435648c00 Mon Sep 17 00:00:00 2001 From: NGPixel Date: Thu, 17 Sep 2026 01:43:32 -0400 Subject: [PATCH] feat: rework the global permissions + make manage:navigation site and path aware instead of global --- CLAUDE.md | 95 ++- backend/api/groups.ts | 154 +++-- backend/api/hooks.ts | 59 +- backend/api/navigation.ts | 129 +++- backend/api/pages.ts | 3 +- backend/api/schemas/group.ts | 5 + backend/api/sites.ts | 12 +- backend/api/storage.ts | 12 +- backend/api/users.ts | 54 +- backend/locales/en.json | 4 + backend/models/groups.ts | 63 ++ backend/models/navigation.ts | 96 ++- backend/models/storage.ts | 2 +- frontend/src/assets/icons.generated.js | 12 +- frontend/src/components/GroupEditOverlay.vue | 318 +++++++-- frontend/src/components/NavEditMenu.vue | 23 +- frontend/src/components/UserCreateDialog.vue | 14 +- frontend/src/components/UserEditOverlay.vue | 50 +- frontend/src/components/UtilCodeEditor.vue | 24 +- frontend/src/components/WebhookEditDialog.vue | 34 +- frontend/src/helpers/sampleContent.js | 11 +- frontend/src/layouts/AdminLayout.vue | 91 +-- frontend/src/layouts/MainLayout.vue | 15 +- frontend/src/pages/AdminGroups.vue | 17 +- frontend/src/pages/AdminNavigation.vue | 647 ------------------ frontend/src/pages/AdminTheme.vue | 3 +- frontend/src/pages/AdminUsers.vue | 34 +- frontend/src/pages/AdminWebhooks.vue | 34 +- frontend/src/router/routes.js | 1 - 29 files changed, 1109 insertions(+), 907 deletions(-) delete mode 100644 frontend/src/pages/AdminNavigation.vue diff --git a/CLAUDE.md b/CLAUDE.md index 748b84d53..21d5b395e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -390,12 +390,68 @@ kind a name belongs to decides how it may be enforced, so it is the first thing any permission you touch. **Global permissions** are held site-wide, bound to no path: `access:admin`, `read:users`, -`manage:users`, `read:groups`, `manage:groups`, `read:audit`, `read:metrics`, -`manage:navigation`, `manage:theme`, `manage:sites`, `manage:system`. That is the list as it stands — the one offered by the group editor +`write:users`, `manage:users`, `read:groups`, `write:groups`, `manage:groups`, `read:audit`, +`read:metrics`, `manage:theme`, `manage:storage`, `manage:sites`, +`read:webhooks`, `manage:webhooks`, `manage:system`. That is the +list as it stands — the one offered by the group editor (`GroupEditOverlay.vue`). They live on a group's `permissions` column, are flattened onto `req.session.permissions` at login (`models/users.ts` → `updateSession`), and are what the per-route `config.permissions` hook checks. `manage:system` bypasses every check everywhere. +**Five of them are ELEVATED ADMIN PERMISSIONS**: `write:users`, `manage:users`, `write:groups`, +`manage:groups` and `manage:system`. `ELEVATED_PERMISSIONS` in `models/groups.ts` is the list that +decides, `isElevated()` is the test, and `groups.elevatedGroupIds()` answers which groups carry one. + +What makes them a category is that each is a route to every OTHER permission on the wiki: whoever +can rewrite who holds what can grant themselves anything, in one step or two. So **membership of a +group carrying one is itself a privilege**, and the guards are written against the whole list rather +than against `manage:system` alone — stopping at the root permission would leave `manage:users` +handing out `manage:groups`, and `manage:groups` handing back `manage:users`, with neither step +looking like an escalation on its own. + +Three rules follow, and they are enforced in `api/users.ts` and `api/groups.ts` rather than in the +models, since they are questions about the CALLER: + +- **Nobody but `manage:system` moves a user in or out of an elevated group** — on create as well as + on edit, and in both directions. Creating an account already inside one is the same act as + promoting an existing one. +- **`manage:users` may not touch an account that belongs to a `manage:system` group at all** + (`systemUserGuard`). That guard is `manage:system` only, not the whole list: a `manage:groups` + account is protected from being re-grouped, not from being renamed. +- **The two group-editing rungs stop at different places** (`elevatedGroupGuard`): `manage:groups` is + stopped only by `manage:system`, `write:groups` by any of the five. + +The list is exposed to clients as a single `isElevated` boolean on `GroupCore`, never as the +permissions themselves — a caller who may not read a group still has to know which ones its controls +must not offer. + +**A site's settings are split across three permissions that do not overlap**, so that the look of a +site, where its content is kept, and everything else about it are three separate grants: + +| Permission | Admin screens | +| ---------- | ------------- | +| `manage:sites` | General, Analytics, Approvals, Comments, Content Blocks, Editors, Locale, Login — plus creating, deleting and listing sites | +| `manage:theme` | Theme, and nothing else. `PUT /sites/:siteId/theme` takes this alone | +| `manage:storage` | Storage, and nothing else. Every route in `api/storage.ts` takes this alone | + +An administrator who is to change all of a site's settings therefore holds all three. + +**Webhooks are their own pair**, `read:webhooks` and `manage:webhooks` (`api/hooks.ts`), rather than +part of `manage:system` as they were. A webhook's `authHeader` is sent verbatim as the +`Authorization` header of every delivery, so it is a credential for somebody else's service: it +reads back as `SENSITIVE_MASK` for a caller who may not change webhooks, and the mask posted back +means "unchanged", the same contract module props use. Whoever may edit still sees the value — the +field is theirs to correct. Don't "helpfully" +accept `manage:sites` on a theme or storage route — the disjointness is the point, and the general +site update (`PUT /sites/:siteId`) refuses a `theme` key for the same reason. + +**The two `write:*` rungs sit between reading and managing:** + +| Permission | May | May not | +| ---------- | --- | ------- | +| `write:users` | create an account | change any existing one; see the list (that is `read:users`); create into an elevated group | +| `write:groups` | create a group, rename it, write its page rules, staff an ordinary one | change what a group is ALLOWED to do (the Permissions tab); delete a group; staff an elevated one | + **Adding a global permission is the maintainer's call, not yours.** The list is not frozen, but a new name reshapes who can do what across the whole instance and every existing group silently lacks it — so propose it and wait for a yes before writing any code that names it. Until then, express what a @@ -405,7 +461,7 @@ that is already on it needs no permission from anybody. **Page rule permissions** are bound to paths, and to locales and sites: `read:pages`, `write:pages`, `review:pages`, `manage:pages`, `delete:pages`, `write:styles`, `write:scripts`, `read:source`, `read:history`, `read:assets`, `write:assets`, `manage:assets`, `read:comments`, `write:comments`, -`manage:comments` (`PAGE_PERMISSIONS` in `api/pages.ts`). A group grants them through **rules**: +`manage:comments`, `manage:navigation` (`PAGE_PERMISSIONS` in `api/pages.ts`). A group grants them through **rules**: each rule names some of them (`roles`) plus how it addresses pages (`match` + `path`, or tags) and what it does with them (`mode`: ALLOW / DENY / FORCEALLOW). Nothing is granted by default, and when several rules match, the most specific one wins — `helpers/pageRules.ts` documents the ordering. @@ -426,6 +482,16 @@ Consequences worth knowing: treats `manage:system` as a wildcard, so it answers "may do this somewhere". Gate a control over the page in front of the reader on `pagePermissions` — that is what the endpoint behind the button will check. +- **`manage:navigation` asks about TWO paths.** It is the one page permission where holding it at the + page in front of you is not the whole answer. At the page it buys the navigation MODE — whether + this page inherits, overrides or hides its sidebar — which affects nothing above it. Editing the + menu's ITEMS additionally needs it on the entry the menu BELONGS to, since those items are shown to + every page under that entry: a page that inherits is editing its ancestor's menu, and a page with + no overriding ancestor is editing the site-wide one, which `navigation.menuOwnerRef` reports as the + home page's path. So a rule over `/guides` lets that section re-point its own pages without letting + it rewrite the menu handed down to it. `api/navigation.ts` has the pair of checks + (`mayManageNavAt`, `mayEditNavItems`), and the `inherited` route answers `canEditItems` so the + editor knows which of its two halves to offer. - **An anonymous request is the guests group**, not an absence of groups: that is how a wiki opens reading, and suggesting edits, to the public. Deny guests explicitly where an account is genuinely required (`reviewerFor` in `api/approvals.ts` is the worked example). @@ -784,7 +850,7 @@ store; no SVG is ever written into content. Components that take an `icon` prop go through it too, so every form works there. - Every Iconify reference written **literally in this repo's source** is inlined at build time by `scripts/generate-icons.mjs` into `src/assets/icons.generated.js` (committed) and drawn as an - inline ``. Run `npm run icons` after adding or removing one; `check-icons.mjs` fails if the + inline ``. Run `npm run icons` after adding or removing one; `npm run icons:check` fails if the bundle drifts. This is why the interface needs no icon webfont — and why nothing an administrator does to icon sets can blank it, which fetching at runtime could not promise: resolution is gated on the set being enabled, and deleting a set drops every icon stored for it. @@ -1121,14 +1187,13 @@ An earlier iteration of 3.x used GraphQL/Apollo. **All of it is deprecated** — server left in `backend/`, and `APOLLO_CLIENT` is not defined as a global, so any call still going through it throws. -**One call is left.** `pages/AdminNavigation.vue`'s `save()` sends the navigation tree and its mode -through `APOLLO_CLIENT.mutate`, so saving the navigation is broken until it is ported. Nothing else -under `frontend/src/` references the global. That handler needs more than the endpoint, mind: it also -calls `this.$store.commit(...)` nine times over, and the file is `
@@ -242,7 +248,11 @@ }}
- +
@@ -444,6 +459,7 @@ [`START`, `SUBTREE`, `REGEX`, `EXACT`].includes(rule.match) ? `/` : null " :suffix="rule.match === `REGEX` ? `/` : null" + :disable="!canManage" :aria-label="t(`admin.groups.rulePath`)" /> @@ -459,10 +475,20 @@
+
- + - {{ t(`admin.groups.permissions`) }} + {{ t(card.title) }} - @@ -78,6 +79,18 @@ const props = defineProps({ type: String, default: null }, + /** + * Show the code but refuse edits. + * + * `readonly` rather than `disabled`: the text stays selectable, copyable and at full contrast, and + * a screen reader still reads it out -- which is the whole point for somebody who may look at a + * setting but not change it. A disabled textarea dims its own content and drops out of the tab + * order, so the reader loses the thing they came for. + */ + readonly: { + type: Boolean, + default: false + }, /** * Sharp corners, for a field that spans its container edge to edge. * @@ -235,8 +248,17 @@ function onScroll(ev) { Tab indents by two, as the editor this replaces did. Shift+Tab is deliberately NOT handled, so it still moves focus and a keyboard user is never trapped in the field. + + `preventDefault` is called here rather than through the template's `.prevent` modifier, because a + readonly field must not swallow Tab -- there it is a key that moves focus on, and this handler has + to return before deciding. Guarding the emit matters on its own: `readonly` stops TYPING into a + textarea, not a keydown handler that writes the model itself. */ function onTab(ev) { + if (props.readonly) { + return + } + ev.preventDefault() const el = ev.target const { selectionStart: start, selectionEnd: end, value } = el emit('update:modelValue', `${value.slice(0, start)} ${value.slice(end)}`) diff --git a/frontend/src/components/WebhookEditDialog.vue b/frontend/src/components/WebhookEditDialog.vue index 2322a7b0c..f58586bbb 100644 --- a/frontend/src/components/WebhookEditDialog.vue +++ b/frontend/src/components/WebhookEditDialog.vue @@ -42,6 +42,7 @@ {{ t(`admin.webhooks.urlHint`) }} @@ -139,6 +143,7 @@ @@ -152,6 +157,7 @@ @@ -163,6 +169,7 @@ {{ t(`admin.webhooks.authHeaderHint`) }} + + userStore.can('manage:webhooks')) + // DATA const state = reactive({ diff --git a/frontend/src/helpers/sampleContent.js b/frontend/src/helpers/sampleContent.js index 3f532aa70..0de17f412 100644 --- a/frontend/src/helpers/sampleContent.js +++ b/frontend/src/helpers/sampleContent.js @@ -775,12 +775,17 @@ There are two kinds, granted separately and checked in different places. ## Global permissions -Held site-wide, bound to no path. \`access:admin\`, \`read:users\`, \`manage:users\`, \`read:groups\`, -\`manage:groups\`, \`read:audit\`, \`read:metrics\`, \`manage:navigation\`, \`manage:theme\`, -\`manage:sites\`, \`manage:system\`. That list is the whole of it. +Held site-wide, bound to no path. \`access:admin\`, \`read:users\`, \`write:users\`, +\`manage:users\`, \`read:groups\`, \`write:groups\`, \`manage:groups\`, \`read:audit\`, +\`read:metrics\`, \`manage:theme\`, \`manage:storage\`, \`manage:sites\`, \`read:webhooks\`, +\`manage:webhooks\`, \`manage:system\`. That list is the whole of it. \`manage:system\` bypasses every check everywhere. +A site's settings are split across three permissions that do not overlap: \`manage:sites\` for its +general settings, \`manage:theme\` for its appearance and \`manage:storage\` for where its content +is kept. Changing all of them takes all three. + ## Page rule permissions Bound to paths, and to locales and sites. A group grants them through **rules**: each rule names some diff --git a/frontend/src/layouts/AdminLayout.vue b/frontend/src/layouts/AdminLayout.vue index e245cfc84..59b0c6cfe 100644 --- a/frontend/src/layouts/AdminLayout.vue +++ b/frontend/src/layouts/AdminLayout.vue @@ -194,23 +194,10 @@ {{ t('admin.login.title') }} - - - - - {{ t('admin.navigation.title') }} - + v-if="userStore.can(`manage:storage`)"> @@ -227,7 +214,7 @@ + v-if="userStore.can(`manage:theme`)"> @@ -412,25 +399,31 @@ {{ t('admin.utilities.title') }} - - - - - {{ t('admin.webhooks.title') }} - - - - - - - - - {{ t('admin.flags.title') }} - + + + + + {{ t('admin.webhooks.title') }} + + + + + + + + + {{ t('admin.flags.title') }} + @@ -570,11 +563,18 @@ const leftDrawerOpen = computed({ */ const showSidebarBtn = computed(() => !isWideViewport.value && !narrowSidebarOpen.value) +/* + The site section is shown for any permission that reaches one of the screens inside it: a site's + settings are split across `manage:sites`, `manage:theme` and `manage:storage`, which do not overlap. + + `manage:navigation` is deliberately absent — it is a page rule now, granted per path, and the screen + it used to reach here has gone. Navigation is edited from the sidebar of the page it belongs to. +*/ const siteSectionShown = computed(() => { return ( userStore.can('manage:sites') || - userStore.can('manage:navigation') || - userStore.can('manage:theme') + userStore.can('manage:theme') || + userStore.can('manage:storage') ) }) /* @@ -583,10 +583,14 @@ const siteSectionShown = computed(() => { to pages nothing links to. */ const groupsAreVisible = computed(() => { - return userStore.can('read:groups') || userStore.can('manage:groups') + return ( + userStore.can('read:groups') || userStore.can('write:groups') || userStore.can('manage:groups') + ) }) const usersAreVisible = computed(() => { - return userStore.can('read:users') || userStore.can('manage:users') + return ( + userStore.can('read:users') || userStore.can('write:users') || userStore.can('manage:users') + ) }) const usersSectionShown = computed(() => { return groupsAreVisible.value || usersAreVisible.value @@ -594,8 +598,15 @@ const usersSectionShown = computed(() => { const auditIsVisible = computed(() => { return userStore.can('read:audit') }) +/* + Webhooks are their own pair of permissions rather than part of `manage:system`, so the item is + outside that block and the section opens for it too. +*/ +const webhooksAreVisible = computed(() => { + return userStore.can('read:webhooks') || userStore.can('manage:webhooks') +}) const systemSectionShown = computed(() => { - return userStore.can('manage:system') || auditIsVisible.value + return userStore.can('manage:system') || auditIsVisible.value || webhooksAreVisible.value }) const overlayIsShown = computed(() => { return Boolean(adminStore.overlay) @@ -657,7 +668,7 @@ watch( router.push({ params: { siteid: newValue } }) } // -> Storage is configured per site, so the light belongs to whichever one is selected - if (newValue && userStore.can('manage:sites')) { + if (newValue && userStore.can('manage:storage')) { adminStore.fetchStorageStatus(newValue) } } @@ -680,7 +691,7 @@ onMounted(async () => { } adminStore.fetchInfo() // -> Only for a role that can see the Storage item at all; anyone else would be asking for a 403 - if (adminStore.currentSiteId && userStore.can('manage:sites')) { + if (adminStore.currentSiteId && userStore.can('manage:storage')) { adminStore.fetchStorageStatus(adminStore.currentSiteId) } }) diff --git a/frontend/src/layouts/MainLayout.vue b/frontend/src/layouts/MainLayout.vue index 56e369bcf..6a0975959 100644 --- a/frontend/src/layouts/MainLayout.vue +++ b/frontend/src/layouts/MainLayout.vue @@ -353,8 +353,21 @@ const showSidebarActions = computed(() => siteStore.locales.showMenu || canBrows page. Same call as the page header's authoring actions, at the same breakpoint -- an editing control that needs a pointer is not offered on a screen that has none. */ +/* + `manage:navigation` is a page rule, so this is what the rules grant AT THIS PATH -- read off + `pagePermissions` rather than through `userStore.can()`, which also answers for the group-wide list + and would say "may manage navigation somewhere". Somewhere is how a button ends up leading to a 403. + + Holding it here buys the MODE at minimum; whether the menu's items are editable too depends on the + entry the menu belongs to, which only the server can resolve -- `canEditItems` on the inherited + response is what says so, and `NavEditMenu` reads it. +*/ const showEditNav = computed(() => { - return userStore.authenticated && userStore.can('manage:navigation') && isAtLeastSm.value + return ( + userStore.authenticated && + userStore.pagePermissions.includes('manage:navigation') && + isAtLeastSm.value + ) }) // WATCHERS diff --git a/frontend/src/pages/AdminGroups.vue b/frontend/src/pages/AdminGroups.vue index 5d11a271e..a4306f9ce 100644 --- a/frontend/src/pages/AdminGroups.vue +++ b/frontend/src/pages/AdminGroups.vue @@ -5,7 +5,9 @@
-
{{ t('admin.groups.title') }}
+
+ {{ t('admin.groups.title') }} +
{{ t('admin.groups.subtitle') }}
@@ -95,7 +97,7 @@ no-caps /> ({ // COMPUTED /* - `read:groups` reaches this page too (see the nav in `AdminLayout`), and everything that writes needs - `manage:groups` -- so the controls behind it are hidden rather than left to fail at the API. + `read:groups` reaches this page too (see the nav in `AdminLayout`), and writing needs one of the two + group-editing rungs -- so the controls behind them are hidden rather than left to fail at the API. + + `write:groups` creates and arranges groups; `manage:groups` additionally decides what a group is + ALLOWED to do and is the only one that may delete one. Hence two computeds rather than one: the + editor and the New button take either, the trash takes only the second. */ -const canManage = computed(() => userStore.can('manage:groups')) +const canManage = computed(() => userStore.can('manage:groups') || userStore.can('write:groups')) +const canDelete = computed(() => userStore.can('manage:groups')) // DATA diff --git a/frontend/src/pages/AdminNavigation.vue b/frontend/src/pages/AdminNavigation.vue deleted file mode 100644 index 5e06d61d9..000000000 --- a/frontend/src/pages/AdminNavigation.vue +++ /dev/null @@ -1,647 +0,0 @@ - - - - - diff --git a/frontend/src/pages/AdminTheme.vue b/frontend/src/pages/AdminTheme.vue index f2da17174..f3cf8c15d 100644 --- a/frontend/src/pages/AdminTheme.vue +++ b/frontend/src/pages/AdminTheme.vue @@ -736,7 +736,8 @@ async function save() { /* The theme's own endpoint rather than the general site update, because it is its own permission: `manage:theme` grants the look of a site without granting its hostname, locales or - authentication, and the general update asks for `manage:sites`. + authentication, and the general update asks for `manage:sites` -- which this route deliberately + does NOT accept, since the three site permissions do not overlap. */ const resp = await API_CLIENT.put(`sites/${adminStore.currentSiteId}/theme`, { json: patchTheme diff --git a/frontend/src/pages/AdminUsers.vue b/frontend/src/pages/AdminUsers.vue index dd464f615..a25e70ce4 100644 --- a/frontend/src/pages/AdminUsers.vue +++ b/frontend/src/pages/AdminUsers.vue @@ -12,6 +12,7 @@
- + + -
+
({ // COMPUTED /* - `read:users` reaches this page too (see the nav in `AdminLayout`), and everything that writes needs - `manage:users` -- so the controls behind it are hidden rather than left to fail at the API. + `read:users` reaches this page too (see the nav in `AdminLayout`), and everything that CHANGES an + existing user needs `manage:users` -- so the controls behind it are hidden rather than left to fail + at the API. */ const canManage = computed(() => userStore.can('manage:users')) +/* + Creating one is a rung of its own: `write:users` brings an account into existence without being + trusted with the accounts that already exist, so the New User button answers to either permission + while every row control above stays on `manage:users`. +*/ +const canCreate = computed(() => canManage.value || userStore.can('write:users')) + +/** Whether this user may be shown the accounts that already exist. */ +const canList = computed(() => canManage.value || userStore.can('read:users')) + // DATA const state = reactive({ @@ -289,6 +305,14 @@ watch( // METHODS async function load({ page } = {}) { + /* + `write:users` reaches this page to use the Create button and nothing else -- seeing the accounts + that already exist is `read:users`. Asking anyway would answer 403 and put a red toast over a + page that is working exactly as intended, so the listing is simply not requested. + */ + if (!canList.value) { + return + } state.loading++ loading.show() try { diff --git a/frontend/src/pages/AdminWebhooks.vue b/frontend/src/pages/AdminWebhooks.vue index 556af049f..30b40963a 100644 --- a/frontend/src/pages/AdminWebhooks.vue +++ b/frontend/src/pages/AdminWebhooks.vue @@ -7,7 +7,9 @@ src="/_assets/icons/fluent-lightning-bolt-animated.svg" />
-
{{ t('admin.webhooks.title') }}
+
+ {{ t('admin.webhooks.title') }} +
{{ t('admin.webhooks.subtitle') }}
@@ -34,6 +36,7 @@ {{ t(`common.actions.refresh`) }}