From db99e3d6ef5e618fc69d1d89a00b96865ba46047 Mon Sep 17 00:00:00 2001 From: NGPixel Date: Sat, 1 Aug 2026 19:02:52 -0400 Subject: [PATCH] fix: revise and harden permissions system + various fixes --- backend/api/approvals.ts | 125 ++++++++++- backend/api/assets.ts | 86 ++++++-- backend/api/authentication.ts | 21 ++ backend/api/locales.ts | 6 + backend/api/pages.ts | 197 ++++++++++++++---- backend/api/schemas/site.ts | 3 - backend/api/sites.ts | 3 + backend/api/system.ts | 3 + backend/api/tags.ts | 14 +- backend/api/tree.ts | 134 ++++++++++-- backend/helpers/pageRules.ts | 179 ++++++++++++++++ backend/index.ts | 51 ++++- backend/locales/en.json | 7 +- backend/models/approvals.ts | 95 +++++++-- backend/models/groups.ts | 90 ++++++++ backend/models/search.ts | 56 ++++- backend/models/sites.ts | 6 +- backend/models/tags.ts | 61 +++++- backend/types/fastify.d.ts | 9 + frontend/src/assets/icons.generated.js | 3 +- .../src/components/ApprovalRuleDialog.vue | 29 ++- frontend/src/components/FileManager.vue | 55 +++++ frontend/src/components/GroupEditOverlay.vue | 131 ++++++++---- frontend/src/components/PageActionsCol.vue | 169 +++++++++++++-- .../src/components/PageHistoryOverlay.vue | 2 +- frontend/src/components/PageSourceOverlay.vue | 2 +- frontend/src/components/PageTags.vue | 31 ++- .../src/components/SuggestionGuestDialog.vue | 9 +- frontend/src/components/shared/WSelect.vue | 13 +- frontend/src/pages/AdminGeneral.vue | 15 -- frontend/src/pages/ErrorGeneric.vue | 39 +++- frontend/src/pages/InboxReview.vue | 67 +++++- frontend/src/pages/Index.vue | 4 + frontend/src/pages/Search.vue | 13 +- frontend/src/router/routes.js | 7 +- frontend/src/stores/page.js | 8 + frontend/src/stores/site.js | 13 ++ 37 files changed, 1510 insertions(+), 246 deletions(-) create mode 100644 backend/helpers/pageRules.ts diff --git a/backend/api/approvals.ts b/backend/api/approvals.ts index 8000442d0..45740f33a 100644 --- a/backend/api/approvals.ts +++ b/backend/api/approvals.ts @@ -1,6 +1,6 @@ import { CustomError } from '../helpers/common.ts' -import { actorFrom, mayBypassPassword, unlockedFor } from './pages.ts' -import type { ApprovalPageRef, ApprovalRulePatch } from '../models/approvals.ts' +import { actorFrom, mayBypassPassword, mayOnPage, unlockedFor } from './pages.ts' +import type { ApprovalPageRef, ApprovalRulePatch, ReviewerScope } from '../models/approvals.ts' import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify' /** @@ -14,7 +14,7 @@ import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify' */ async function loadSuggestablePage(req: FastifyRequest, siteId: string, pageId: string) { const actor = actorFrom(req) - return WIKI.models.pages.getPage({ + const page = await WIKI.models.pages.getPage({ siteId, id: pageId, withContent: true, @@ -22,16 +22,38 @@ async function loadSuggestablePage(req: FastifyRequest, siteId: string, pageId: unlocked: (id: string) => unlockedFor(req, id), withPassword: mayBypassPassword(req) }) + /* + Reading the page comes first, for suggesting an edit to it and for reviewing one alike: neither is + something to be done to a page the caller may not see, and answering as though it were not there + is how every other page-scoped route treats that. + */ + if (!page || !mayOnPage(req, 'read:pages', page)) { + return null + } + return page } /** - * Who is reviewing, as the rules see them: the groups on their session, plus whether they hold - * `manage:system` — which sees every queue, here as everywhere else. + * Who is reviewing, as the approval rules see them: the groups on their session, plus whether they + * review everything regardless of which groups a rule names. + * + * Two different kinds of rule meet here. An APPROVAL rule says which pages take suggestions and who + * reviews them; a group's PAGE rules say what a member may do to a page, `review:pages` among them. + * Holding that permission is the second way of being a reviewer, because reviewing is the entire + * content of it — a group granted it and named in no approval rule could otherwise review nothing. + * + * Page permissions are per page, so `reviewsAll` is answered for a page when there is one. Without + * one — the site-wide queue in the inbox — it is answered at the site root, which is the only thing + * a queue spanning every page could ask about; the per-page check then still applies to each entry + * through the approval rules that produced it. */ -function reviewerFor(req: FastifyRequest): { groupIds: string[]; isAdmin: boolean } { +function reviewerFor(req: FastifyRequest, page?: { path: string; tags?: string[] }): ReviewerScope { + const actor = WIKI.models.groups.actorForRequest(req) return { groupIds: WIKI.models.approvals.getActorGroupIds(req), - isAdmin: Boolean(req.session?.permissions?.includes('manage:system')) + reviewsAll: + actor.permissions.includes('manage:system') || + WIKI.models.groups.checkAccess(actor, 'review:pages', page ?? { path: '' }) } } @@ -59,7 +81,15 @@ function validateRule({ if (!name || name.trim().length < 1) { return new CustomError('approvalRuleEmptyName', 'A rule name is required.') } - if (!path || path.trim().length < 1) { + /* + Empty is only meaningful for `START`, where it is every path and therefore the whole site -- which + is how a rule covers a site without naming a folder. + + Every other mode still needs something. An empty `EXACT` matches no page at all; an empty `END` or + `REGEX` matches every one of them, but by accident of the operator rather than by intent, and a + rule whose reach nobody meant to write is exactly what this refuses. + */ + if (match !== 'START' && (!path || path.trim().length < 1)) { return new CustomError( 'approvalRuleEmptyPath', match === 'TAG' || match === 'TAGALL' @@ -548,6 +578,71 @@ async function routes(app: FastifyInstance) { } ) + /** + * PENDING SUBMISSIONS FOR A PAGE + */ + app.get<{ Params: { siteId: string; pageId: string } }>( + '/sites/:siteId/pages/:pageId/submissions', + { + /* + No route-level `permissions`: those are page permissions, granted by a group's rules rather + than group-wide. `canReview` below is the real answer, and an ineligible caller gets `false` + rather than a refusal — the button simply does not appear. + */ + schema: { + summary: "Edit suggestions waiting on a page, for that page's reviewers", + description: + 'What the review button on a page view is drawn from. `canReview` says whether this caller reviews this page at all — an enabled rule covers it and either names one of their groups or they hold `review:pages` or `manage:system` — and is what decides whether the button is shown; `submissions` is what is waiting, oldest first, and is empty for everybody else.\n\nA reviewer with an empty queue still gets `canReview: true`: the button belongs to the page, not to whatever happens to be pending on it.', + tags: ['Approvals'], + params: { + type: 'object', + properties: { + siteId: { type: 'string', format: 'uuid' }, + pageId: { type: 'string', format: 'uuid' } + }, + required: ['siteId', 'pageId'] + }, + response: { + 200: { + description: 'Whether the caller reviews this page, and what is waiting on it', + type: 'object', + properties: { + canReview: { type: 'boolean' }, + submissions: { + type: 'array', + items: { $ref: 'PageEditSubmission#' } + } + } + } + } + } + }, + async (req, reply) => { + reply.preventCache() + const page = await loadSuggestablePage(req, req.params.siteId, req.params.pageId) + if (!page) { + return reply.notFound('This page does not exist.') + } + + const scope = reviewerFor(req, { path: page.path, tags: page.tags ?? [] }) + const canReview = await WIKI.models.approvals.canReviewPage( + req.params.siteId, + { path: page.path, tags: page.tags ?? [] }, + scope + ) + if (!canReview) { + return { canReview: false, submissions: [] } + } + return { + canReview: true, + submissions: await WIKI.models.approvals.getReviewableSubmissions(req.params.siteId, { + ...scope, + pageId: req.params.pageId + }) + } + } + ) + /** * GET OWN SUGGESTION STATE FOR A PAGE * @@ -616,7 +711,12 @@ async function routes(app: FastifyInstance) { const actor = actorFrom(req) const groupIds = WIKI.models.approvals.getActorGroupIds(req) - const pageRef: ApprovalPageRef = { id: page.id, path: page.path, tags: page.tags ?? [] } + const pageRef: ApprovalPageRef = { + id: page.id, + path: page.path, + tags: page.tags ?? [], + allowContributions: page.allowContributions + } const rule = await WIKI.models.approvals.findSubmitRule(req.params.siteId, pageRef, groupIds) if (!rule) { return { canSubmit: false, isGuest: !actor, submission: null } @@ -691,7 +791,12 @@ async function routes(app: FastifyInstance) { const actor = actorFrom(req) const groupIds = WIKI.models.approvals.getActorGroupIds(req) - const pageRef: ApprovalPageRef = { id: page.id, path: page.path, tags: page.tags ?? [] } + const pageRef: ApprovalPageRef = { + id: page.id, + path: page.path, + tags: page.tags ?? [], + allowContributions: page.allowContributions + } const rule = await WIKI.models.approvals.findSubmitRule(req.params.siteId, pageRef, groupIds) if (!rule) { return reply.forbidden('This page does not accept edit suggestions from you.') diff --git a/backend/api/assets.ts b/backend/api/assets.ts index 8235b7eff..1302037cf 100644 --- a/backend/api/assets.ts +++ b/backend/api/assets.ts @@ -1,4 +1,6 @@ -import type { FastifyInstance } from 'fastify' +import type { FastifyInstance, FastifyRequest } from 'fastify' + +import { decodeTreePath } from '../helpers/common.ts' /** Extensions a browser may render inline. Everything else is sent as a download. */ const INLINE_EXTS = new Set(['png', 'apng', 'jpg', 'jpeg', 'gif', 'bmp', 'webp', 'svg']) @@ -21,6 +23,24 @@ const assetIdParam = { /** * Assets API Routes */ +/** + * Whether the caller holds an asset permission on an asset, judged on where it sits. + * + * Assets live in the same tree as pages and are addressed by the same rules — a rule over a branch + * covers the files in it as well as the pages, which is why the asset permissions are offered + * alongside the page ones in the group editor. + */ +function mayOnAsset( + req: FastifyRequest, + permission: string, + asset: { folderPath?: string | null; fileName: string } +): boolean { + const folder = asset.folderPath ?? '' + return WIKI.models.groups.checkAccess(WIKI.models.groups.actorForRequest(req), permission, { + path: folder ? `${folder}/${asset.fileName}` : asset.fileName + }) +} + async function routes(app: FastifyInstance) { // -> An upload is the raw file rather than a multipart form: one file per request, with the name and // the destination in the query string. The catch-all only claims content types nothing else @@ -46,9 +66,10 @@ async function routes(app: FastifyInstance) { }>( '/sites/:siteId/assets', { - config: { - permissions: ['write:assets', 'manage:assets'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and asset permissions come + from a group's RULES, which address the folder the file is in. Checked below. + */ schema: { summary: 'Upload an asset', description: `The body is the file itself, not a multipart form — send the bytes with their \`Content-Type\`. At most ${Math.round((WIKI.config.security?.uploadMaxFileSize ?? 10485760) / 1024 / 1024)} MB. The file name is sanitized, so the stored name in the response may differ from the one sent; the type served back later comes from that name's extension rather than from the request. Images get a thumbnail when the Sharp extension is installed.`, @@ -113,6 +134,16 @@ async function routes(app: FastifyInstance) { return reply.badRequest('No file was sent.') } + const folder = req.query.folderId + ? await WIKI.models.tree.getFolderById(req.query.folderId) + : null + const folderPath = folder ? (decodeTreePath(folder.folderPath ?? '') ?? '') : '' + const destination = folder ? [folderPath, folder.fileName].filter(Boolean).join('/') : '' + if ( + !mayOnAsset(req, 'write:assets', { folderPath: destination, fileName: req.query.fileName }) + ) { + return reply.forbidden('You are not allowed to upload a file here.') + } const asset = await WIKI.models.assets.upload({ siteId: req.params.siteId, locale: req.query.locale ?? WIKI.sites[req.params.siteId]?.config?.locales?.primary ?? 'en', @@ -137,9 +168,10 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string; assetId: string } }>( '/sites/:siteId/assets/:assetId', { - config: { - permissions: ['read:assets', 'manage:assets'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and asset permissions come + from a group's RULES, which address the folder the file is in. Checked below. + */ schema: { summary: 'Get a single asset', description: 'Metadata only. `/content` serves the file itself.', @@ -152,7 +184,8 @@ async function routes(app: FastifyInstance) { }, async (req, reply) => { const asset = await WIKI.models.assets.getAsset(req.params.siteId, req.params.assetId) - if (!asset) { + // -> Not readable is answered as not there, so the endpoint cannot be used to probe for files + if (!asset || !mayOnAsset(req, 'read:assets', asset)) { return reply.notFound('This asset does not exist.') } return asset @@ -165,9 +198,10 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string; assetId: string } }>( '/sites/:siteId/assets/:assetId/content', { - config: { - permissions: ['read:assets', 'manage:assets'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and asset permissions come + from a group's RULES, which address the folder the file is in. Checked below. + */ schema: { summary: 'Download an asset', description: @@ -191,7 +225,7 @@ async function routes(app: FastifyInstance) { }, async (req, reply) => { const asset = await WIKI.models.assets.getAsset(req.params.siteId, req.params.assetId) - if (!asset) { + if (!asset || !mayOnAsset(req, 'read:assets', asset)) { return reply.notFound('This asset does not exist.') } const content = await WIKI.models.assets.getContent(req.params.assetId) @@ -218,9 +252,10 @@ async function routes(app: FastifyInstance) { app.patch<{ Params: { siteId: string; assetId: string }; Body: { fileName: string } }>( '/sites/:siteId/assets/:assetId', { - config: { - permissions: ['manage:assets'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and asset permissions come + from a group's RULES, which address the folder the file is in. Checked below. + */ schema: { summary: 'Rename an asset', description: @@ -257,6 +292,13 @@ async function routes(app: FastifyInstance) { } }, async (req, reply) => { + const existing = await WIKI.models.assets.getAsset(req.params.siteId, req.params.assetId) + if (!existing) { + return reply.notFound('This asset does not exist.') + } + if (!mayOnAsset(req, 'manage:assets', existing)) { + return reply.forbidden('You are not allowed to rename this file.') + } const asset = await WIKI.models.assets.renameAsset( req.params.siteId, req.params.assetId, @@ -279,9 +321,10 @@ async function routes(app: FastifyInstance) { app.delete<{ Params: { siteId: string; assetId: string } }>( '/sites/:siteId/assets/:assetId', { - config: { - permissions: ['manage:assets'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and asset permissions come + from a group's RULES, which address the folder the file is in. Checked below. + */ schema: { summary: 'Delete an asset', tags: ['Assets'], @@ -294,6 +337,13 @@ async function routes(app: FastifyInstance) { } }, async (req, reply) => { + const doomed = await WIKI.models.assets.getAsset(req.params.siteId, req.params.assetId) + if (!doomed) { + return reply.notFound('This asset does not exist.') + } + if (!mayOnAsset(req, 'manage:assets', doomed)) { + return reply.forbidden('You are not allowed to delete this file.') + } if (!(await WIKI.models.assets.deleteAsset(req.params.siteId, req.params.assetId))) { return reply.notFound('This asset does not exist.') } diff --git a/backend/api/authentication.ts b/backend/api/authentication.ts index 0af5c4d95..b089987c5 100644 --- a/backend/api/authentication.ts +++ b/backend/api/authentication.ts @@ -10,6 +10,9 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string }; Querystring: { visibleOnly?: boolean } }>( '/sites/:siteId/auth/strategies', { + config: { + publicAccess: true + }, schema: { summary: 'List all site authentication strategies', description: @@ -135,6 +138,9 @@ async function routes(app: FastifyInstance) { }>( '/sites/:siteId/auth/login', { + config: { + publicAccess: true + }, schema: { summary: 'Login', tags: ['Authentication'], @@ -214,6 +220,9 @@ async function routes(app: FastifyInstance) { }>( '/sites/:siteId/auth/changePassword', { + config: { + publicAccess: true + }, schema: { summary: 'Change Password From Login', tags: ['Authentication'], @@ -305,6 +314,9 @@ async function routes(app: FastifyInstance) { }>( '/sites/:siteId/auth/tfa', { + config: { + publicAccess: true + }, schema: { summary: 'Submit a 2FA Security Code From Login', description: @@ -390,6 +402,9 @@ async function routes(app: FastifyInstance) { app.post<{ Params: { siteId: string } }>( '/sites/:siteId/auth/passkey/challenge', { + config: { + publicAccess: true + }, schema: { summary: 'Get the options for logging in with a passkey', description: @@ -449,6 +464,9 @@ async function routes(app: FastifyInstance) { app.put<{ Params: { siteId: string }; Body: { authResponse: Record } }>( '/sites/:siteId/auth/passkey/login', { + config: { + publicAccess: true + }, schema: { summary: 'Login With a Passkey', description: @@ -515,6 +533,9 @@ async function routes(app: FastifyInstance) { app.post<{ Params: { siteId: string } }>( '/sites/:siteId/auth/logout', { + config: { + publicAccess: true + }, schema: { summary: 'Logout', description: diff --git a/backend/api/locales.ts b/backend/api/locales.ts index be579f436..a4eef1408 100644 --- a/backend/api/locales.ts +++ b/backend/api/locales.ts @@ -7,6 +7,9 @@ async function routes(app: FastifyInstance) { app.get( '/', { + config: { + publicAccess: true + }, schema: { summary: 'List all locales', tags: ['Locales'] @@ -20,6 +23,9 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { code: string } }>( '/:code/strings', { + config: { + publicAccess: true + }, schema: { summary: 'Get locale strings', tags: ['Locales'] diff --git a/backend/api/pages.ts b/backend/api/pages.ts index dca294271..ea9b43e60 100644 --- a/backend/api/pages.ts +++ b/backend/api/pages.ts @@ -69,15 +69,25 @@ export function actorFrom(req: FastifyRequest): PageActor | null { const PASSWORD_BYPASS = ['write:pages', 'manage:pages', 'manage:system'] /** - * Every page permission a group can be granted, i.e. the whole set `manage:system` amounts to. Mirrors - * the page rules offered in the group editor. + * Every page permission a rule can grant, i.e. the whole set `manage:system` amounts to. Mirrors the + * page rules offered in the group editor, and is what the interface asks about per path. */ const PAGE_PERMISSIONS = [ 'read:pages', 'write:pages', 'review:pages', 'manage:pages', - 'delete:pages' + 'delete:pages', + 'write:styles', + 'write:scripts', + 'read:source', + 'read:history', + 'read:assets', + 'write:assets', + 'manage:assets', + 'read:comments', + 'write:comments', + 'manage:comments' ] export function mayBypassPassword(req: FastifyRequest): boolean { @@ -95,6 +105,21 @@ export function unlockedFor(req: FastifyRequest, pageId: string): boolean { return mayBypassPassword(req) || Boolean(req.session?.unlockedPages?.includes(pageId)) } +/** + * Whether this requester holds a page permission ON THIS PAGE. + * + * Page permissions are granted by a group's rules, not by the group-wide permission list, so this is + * a different question from the one the route-level `config.permissions` hook answers — and the only + * correct one for anything page-scoped. `helpers/pageRules.ts` sets out how a rule is chosen. + */ +export function mayOnPage( + req: FastifyRequest, + permission: string, + page: { path: string; locale?: string; tags?: string[] } +): boolean { + return WIKI.models.groups.checkAccess(WIKI.models.groups.actorForRequest(req), permission, page) +} + /** * A page, as this requester is allowed to see it — or null when they are not allowed to see it at all. * @@ -104,12 +129,17 @@ export function unlockedFor(req: FastifyRequest, pageId: string): boolean { */ async function loadReadablePage(req: FastifyRequest, siteId: string, pageId: string) { const actor = actorFrom(req) - return WIKI.models.pages.getPage({ + const page = await WIKI.models.pages.getPage({ siteId, id: pageId, publicOnly: !actor, unlocked: (id: string) => unlockedFor(req, id) }) + // -> Not readable is indistinguishable from not there, for anything hanging off the page + if (!page || !mayOnPage(req, 'read:pages', page)) { + return null + } + return page } /** @@ -122,13 +152,14 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string } }>( '/sites/:siteId/pages', { - config: { - permissions: ['read:pages', 'manage:pages'] - }, + /* + No route-level `permissions`: page permissions come from a group's RULES, and this would have + to filter per page against them. It has nothing to filter yet — see the description. + */ schema: { summary: 'List all pages', description: - 'Not implemented yet — always answers with an empty list. Browse the tree instead, which is what the file manager and the navigation use.', + 'Not implemented yet — always answers with an empty list. Browse the tree instead, which is what the file manager and the navigation use, and which filters what it lists by the page rules.', tags: ['Pages'], params: siteIdParam, response: { @@ -276,6 +307,8 @@ async function routes(app: FastifyInstance) { offset: req.query.offset, limit: req.query.limit, publicOnly: !actor, + // -> So that a page the caller could not open never shows up as a result + actor: WIKI.models.groups.actorForRequest(req), // -> An unpublished page is only of interest to someone who could have written it includeDrafts: ['write:pages', 'manage:pages', 'manage:system'].some((p) => permissions.includes(p) @@ -336,6 +369,9 @@ async function routes(app: FastifyInstance) { if (!page) { return reply.notFound('This page does not exist.') } + if (!mayOnPage(req, 'read:pages', page)) { + return reply.forbidden('You are not allowed to read this page.') + } return { path: page.path, locale: page.locale, @@ -410,6 +446,9 @@ async function routes(app: FastifyInstance) { if (!page) { return reply.notFound('This page does not exist.') } + if (!mayOnPage(req, 'read:pages', page)) { + return reply.forbidden('You are not allowed to read this page.') + } return page } ) @@ -500,9 +539,11 @@ async function routes(app: FastifyInstance) { app.post<{ Params: { siteId: string }; Body: PageInput }>( '/sites/:siteId/pages', { - config: { - permissions: ['write:pages', 'manage:pages'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and page permissions are + granted by a group's RULES. Checked against the page in question below instead — which is + also what lets a rule open one branch to somebody the group as a whole cannot write to. + */ schema: { summary: 'Create a page', description: @@ -533,6 +574,10 @@ async function routes(app: FastifyInstance) { if (!actor) { return reply.unauthorized('Saving a page requires a logged in user.') } + // -> Against where the page is going: there is no page to ask about yet + if (!mayOnPage(req, 'write:pages', { path: req.body.path, locale: req.body.locale })) { + return reply.forbidden('You are not allowed to create a page here.') + } const page = await WIKI.models.pages.createPage(req.params.siteId, req.body, actor) return { ok: true, @@ -548,9 +593,11 @@ async function routes(app: FastifyInstance) { app.patch<{ Params: { siteId: string; pageId: string }; Body: Partial }>( '/sites/:siteId/pages/:pageId', { - config: { - permissions: ['write:pages', 'manage:pages'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and page permissions are + granted by a group's RULES. Checked against the page in question below instead — which is + also what lets a rule open one branch to somebody the group as a whole cannot write to. + */ schema: { summary: 'Update a page', description: @@ -576,6 +623,16 @@ async function routes(app: FastifyInstance) { if (!actor) { return reply.unauthorized('Saving a page requires a logged in user.') } + const target = await WIKI.models.pages.getPage({ + siteId: req.params.siteId, + id: req.params.pageId + }) + if (!target) { + return reply.notFound('This page does not exist.') + } + if (!mayOnPage(req, 'write:pages', target)) { + return reply.forbidden('You are not allowed to edit this page.') + } const page = await WIKI.models.pages.updatePage( req.params.siteId, req.params.pageId, @@ -602,9 +659,11 @@ async function routes(app: FastifyInstance) { }>( '/sites/:siteId/pages/:pageId/path', { - config: { - permissions: ['manage:pages'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and page permissions are + granted by a group's RULES. Checked against the page in question below instead — which is + also what lets a rule open one branch to somebody the group as a whole cannot write to. + */ schema: { summary: 'Move a page to another path', description: @@ -645,6 +704,16 @@ async function routes(app: FastifyInstance) { if (!actor) { return reply.unauthorized('Moving a page requires a logged in user.') } + const target = await WIKI.models.pages.getPage({ + siteId: req.params.siteId, + id: req.params.pageId + }) + if (!target) { + return reply.notFound('This page does not exist.') + } + if (!mayOnPage(req, 'manage:pages', target)) { + return reply.forbidden('You are not allowed to move this page.') + } const page = await WIKI.models.pages.movePage( req.params.siteId, req.params.pageId, @@ -668,9 +737,11 @@ async function routes(app: FastifyInstance) { app.post<{ Params: { siteId: string; pageId: string } }>( '/sites/:siteId/pages/:pageId/render', { - config: { - permissions: ['write:pages', 'manage:pages'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and page permissions are + granted by a group's RULES. Checked against the page in question below instead — which is + also what lets a rule open one branch to somebody the group as a whole cannot write to. + */ schema: { summary: 'Render a page again from its source', description: @@ -695,6 +766,17 @@ async function routes(app: FastifyInstance) { if (!actor) { return reply.unauthorized('Rendering a page requires a logged in user.') } + const target = await WIKI.models.pages.getPage({ + siteId: req.params.siteId, + id: req.params.pageId + }) + if (!target) { + return reply.notFound('This page does not exist.') + } + // -> Rewrites what the page shows, so it is an edit and takes the same permission as one + if (!mayOnPage(req, 'write:pages', target)) { + return reply.forbidden('You are not allowed to edit this page.') + } const page = await WIKI.models.pages.rerenderPage(req.params.siteId, req.params.pageId, actor) if (!page) { return reply.notFound('This page does not exist.') @@ -713,9 +795,11 @@ async function routes(app: FastifyInstance) { app.delete<{ Params: { siteId: string; pageId: string } }>( '/sites/:siteId/pages/:pageId', { - config: { - permissions: ['delete:pages', 'manage:pages'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and page permissions are + granted by a group's RULES. Checked against the page in question below instead — which is + also what lets a rule open one branch to somebody the group as a whole cannot write to. + */ schema: { summary: 'Delete a page', tags: ['Pages'], @@ -732,6 +816,16 @@ async function routes(app: FastifyInstance) { if (!actor) { return reply.unauthorized('Deleting a page requires a logged in user.') } + const target = await WIKI.models.pages.getPage({ + siteId: req.params.siteId, + id: req.params.pageId + }) + if (!target) { + return reply.notFound('This page does not exist.') + } + if (!mayOnPage(req, 'delete:pages', target)) { + return reply.forbidden('You are not allowed to delete this page.') + } if (!(await WIKI.models.pages.deletePage(req.params.siteId, req.params.pageId, actor))) { return reply.notFound('This page does not exist.') } @@ -745,10 +839,14 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string; pageId: string } }>( '/sites/:siteId/pages/:pageId/history', { + /* + No route-level `permissions`: that hook reads the group-wide list, and `read:history` is a + page permission granted by a rule. Checked against this page below instead. + */ schema: { summary: "Get a page's version history", description: - 'Every recorded version of the page, newest first — the first entry is the page as it stands now.\n\nGated on being able to read the page, no more: history is part of a page, so whoever may read the page may read what it used to say. That means an anonymous reader sees the history of a published page and nothing of a draft, and that a password-protected page answers only once the session has satisfied `POST …/unlock`.', + 'Every recorded version of the page, newest first — the first entry is the page as it stands now.\n\nNeeds `read:history` ON THIS PAGE, granted by a group rule — the permission that says who may see what a page used to contain. Reading the page itself is required on top, so a page the caller could not open answers 404 and a password-protected one answers only once the session has satisfied `POST …/unlock`.', tags: ['Pages'], params: pageIdParam, response: { @@ -765,6 +863,9 @@ async function routes(app: FastifyInstance) { if (!page) { return reply.notFound('This page does not exist.') } + if (!mayOnPage(req, 'read:history', page)) { + return reply.forbidden("You are not allowed to read this page's history.") + } if (page.isLocked) { return reply.forbidden('This page is password protected.') } @@ -778,10 +879,11 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string; pageId: string; versionId: string } }>( '/sites/:siteId/pages/:pageId/history/:versionId', { + // -> Checked per page below, for the same reason as the history list above schema: { summary: 'Get a single version of a page', description: - 'One version in full, source included — one side of a comparison. Readable by whoever may read the page, on the same terms as the history list.', + 'One version in full, source included — one side of a comparison. Needs `read:history` and the ability to read the page, on the same terms as the history list.', tags: ['Pages'], params: { type: 'object', @@ -811,6 +913,9 @@ async function routes(app: FastifyInstance) { if (!page) { return reply.notFound('This page does not exist.') } + if (!mayOnPage(req, 'read:history', page)) { + return reply.forbidden("You are not allowed to read this page's history.") + } if (page.isLocked) { return reply.forbidden('This page is password protected.') } @@ -832,9 +937,11 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string; alias: string } }>( '/sites/:siteId/pages/alias/:alias', { - config: { - permissions: ['read:pages', 'manage:pages'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and page permissions are + granted by a group's RULES. Checked against the page in question below instead — which is + also what lets a rule open one branch to somebody the group as a whole cannot write to. + */ schema: { summary: 'Resolve a page alias to its path', tags: ['Pages'], @@ -870,6 +977,11 @@ async function routes(app: FastifyInstance) { if (!target) { return reply.notFound('No page uses this alias.') } + // -> Resolving an alias tells the caller a page exists and where it is, which is only theirs + // to know if they may read it + if (!mayOnPage(req, 'read:pages', { path: target.path })) { + return reply.notFound('No page uses this alias.') + } return target } ) @@ -883,7 +995,7 @@ async function routes(app: FastifyInstance) { schema: { summary: 'Get page user permissions', description: - "The current user's page permissions, which are not yet scoped per path — every page in the site answers the same.", + "Which page permissions the caller holds AT THIS PATH, as their groups' rules decide. This is what the interface hides its controls by, so it answers the same question the endpoints themselves do rather than a broader one.\n\nAn administrator holds all of them. Everybody else gets whatever their rules grant, which for a path nobody wrote a rule for is nothing at all.", tags: ['Pages'], params: siteIdParam, body: { @@ -912,21 +1024,26 @@ async function routes(app: FastifyInstance) { } }, async (req) => { - const actor = actorFrom(req) - if (!actor) { - return [] - } /* - An administrator holds all of them, and holds them here too. Filtering their permissions by - name the way the line below does would answer `manage:system` → nothing ending in `:pages` → - that an administrator has no rights over any page, which is the opposite of true. + Anonymous included: the guests group has rules of its own, and what the public may do is + exactly what they say. Answering an empty list for a reader without a session would hide + controls a wiki had deliberately opened to everyone. */ - if (actor.permissions.includes('manage:system')) { + const accessActor = WIKI.models.groups.actorForRequest(req) + /* + An administrator holds all of them, and holds them here too. Deriving the list from their + permissions instead would answer `manage:system` → nothing ending in `:pages` → that an + administrator has no rights over any page, which is the opposite of true. + */ + if (accessActor.permissions.includes('manage:system')) { return PAGE_PERMISSIONS } - // FIXME: per-path permission rules are not implemented — a group's page permissions apply to - // the whole site, so this returns what the user holds anywhere rather than here. - return actor.permissions.filter((p) => p.endsWith(':pages')) + // -> Resolved per permission against this path, since each one may be decided by a different + // rule — a branch can be readable but not writable, and one page within it neither + const page = { path: req.body.path.replace(/^\/+/, '') } + return PAGE_PERMISSIONS.filter((permission) => + WIKI.models.groups.checkAccess(accessActor, permission, page) + ) } ) } diff --git a/backend/api/schemas/site.ts b/backend/api/schemas/site.ts index 548906038..4bb60b8d0 100644 --- a/backend/api/schemas/site.ts +++ b/backend/api/schemas/site.ts @@ -78,9 +78,6 @@ export async function registerSchemas(app: FastifyInstance): Promise { comments: { type: 'boolean' }, - contributions: { - type: 'boolean' - }, profile: { type: 'boolean' }, diff --git a/backend/api/sites.ts b/backend/api/sites.ts index 9437cabf3..fe572b99c 100644 --- a/backend/api/sites.ts +++ b/backend/api/sites.ts @@ -64,6 +64,9 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteIdorHostname: string }; Querystring: { strict?: boolean } }>( '/:siteIdorHostname', { + config: { + publicAccess: true + }, schema: { summary: 'Get site info', tags: ['Sites'], diff --git a/backend/api/system.ts b/backend/api/system.ts index 95b829ac0..908735528 100644 --- a/backend/api/system.ts +++ b/backend/api/system.ts @@ -150,6 +150,9 @@ async function routes(app: FastifyInstance) { app.get( '/flags', { + config: { + publicAccess: true + }, schema: { summary: 'System Flags', description: diff --git a/backend/api/tags.ts b/backend/api/tags.ts index a9185c778..8a3c28bb5 100644 --- a/backend/api/tags.ts +++ b/backend/api/tags.ts @@ -13,13 +13,14 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string }; Querystring: { limit?: number } }>( '/sites/:siteId/tags', { - config: { - permissions: ['read:pages', 'write:pages', 'manage:pages'] - }, + /* + No route-level `permissions`: a tag exists because a readable page carries it, so the answer + is filtered per page below rather than refused outright. + */ schema: { summary: 'List the tags in use on a site', description: - 'Every tag carried by at least one page, most used first. This is what the tag field offers as suggestions while a page is being edited.', + 'Every tag carried by at least one page the caller may read, most used first, counted over those pages only. This is what the tag field offers as suggestions while a page is being edited, and what the search screen filters by.', tags: ['Pages'], params: { type: 'object', @@ -63,7 +64,10 @@ async function routes(app: FastifyInstance) { } }, async (req) => { - return WIKI.models.tags.getTags(req.params.siteId, { limit: req.query.limit }) + return WIKI.models.tags.getTags(req.params.siteId, { + limit: req.query.limit, + actor: WIKI.models.groups.actorForRequest(req) + }) } ) } diff --git a/backend/api/tree.ts b/backend/api/tree.ts index b0b535b4a..b1d54b858 100644 --- a/backend/api/tree.ts +++ b/backend/api/tree.ts @@ -1,4 +1,4 @@ -import type { FastifyInstance } from 'fastify' +import type { FastifyInstance, FastifyRequest } from 'fastify' import { TREE_ORDER_BY, type TreeItemType, type TreeOrderBy } from '../models/tree.ts' import { decodeTreePath } from '../helpers/common.ts' @@ -77,6 +77,52 @@ const folderIdParam = { * pages and assets. Folders are the only kind created here — a page or an asset gets its tree entry * from whatever created it. */ +/** + * The entries of a tree listing this caller may see, and the folders leading to them. + * + * Filtered here rather than in the query for the same reason as everywhere else: a page rule can be a + * regular expression or a set of tags, so which rule decides an entry is only knowable per entry. + * + * A folder is judged on its own path, so a DENY over a branch hides the branch itself rather than + * leaving an empty folder to walk into. The consequence worth knowing is the other way round: a + * folder stays listed when the rules deny everything inside it but say nothing about the folder, and + * a reader opening it finds it empty. Hiding those would mean resolving every descendant of every + * folder on every listing, which is not worth what it costs. + */ +function visibleTreeItems( + req: FastifyRequest, + items: T[] +): T[] { + const actor = WIKI.models.groups.actorForRequest(req) + return items.filter((item) => { + const path = item.folderPath ? `${item.folderPath}/${item.fileName}` : (item.fileName ?? '') + const permission = item.type === 'asset' ? 'read:assets' : 'read:pages' + return WIKI.models.groups.checkAccess(actor, permission, { + path, + tags: (item as any).tags ?? [] + }) + }) +} + +/** A folder's own slash-separated path, which is what a rule over that branch addresses. */ +function folderPathOf(folder: { folderPath?: string | null; fileName: string }): string { + const parent = decodeTreePath(folder.folderPath ?? '') ?? '' + return parent ? `${parent}/${folder.fileName}` : folder.fileName +} + +/** + * Whether the caller holds a page permission over a folder, judged on the folder's own path. + * + * A folder is not a page and has no permissions of its own, so what governs it is what governs the + * branch it opens: a rule denying `read:pages` under `geography` hides the folder as well as the + * pages in it, and only somebody who may reorganise pages there may rename or remove it. + */ +function mayOnFolder(req: FastifyRequest, permission: string, path: string): boolean { + return WIKI.models.groups.checkAccess(WIKI.models.groups.actorForRequest(req), permission, { + path + }) +} + async function routes(app: FastifyInstance) { /** * BROWSE THE TREE @@ -84,9 +130,11 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string }; Querystring: TreeQuery }>( '/sites/:siteId/tree', { - config: { - permissions: ['read:pages', 'read:assets', 'manage:pages', 'manage:assets'] - }, + /* + No route-level `permissions`: page permissions come from a group's RULES, and every entry is + filtered against them below — a caller allowed nowhere gets an empty listing rather than a + refusal, which is the same thing the tree would look like if the pages were not there. + */ schema: { summary: 'Browse the tree', description: @@ -168,7 +216,7 @@ async function routes(app: FastifyInstance) { }, async (req) => { const q = req.query - return WIKI.models.tree.getTree({ + const items = await WIKI.models.tree.getTree({ siteId: req.params.siteId, parentId: q.parentId, parentPath: q.parentPath, @@ -183,6 +231,7 @@ async function routes(app: FastifyInstance) { includeAncestors: q.includeAncestors, includeRootFolders: q.includeRootFolders }) + return visibleTreeItems(req, items) } ) @@ -258,7 +307,18 @@ async function routes(app: FastifyInstance) { if (!level) { return reply.notFound('This folder does not exist.') } - return level + /* + A browse row carries a whole path rather than a folder/name pair, and stands for a page, a + folder, or both at once. Judged on that path either way: for the page it IS the page, and for + a folder it is the branch, which is what a rule over the branch is talking about. + */ + const actor = WIKI.models.groups.actorForRequest(req) + return { + ...level, + items: level.items.filter((item) => + WIKI.models.groups.checkAccess(actor, 'read:pages', { path: item.path }) + ) + } } ) @@ -340,7 +400,7 @@ async function routes(app: FastifyInstance) { if (!WIKI.sites[req.params.siteId]) { return reply.notFound('This site does not exist.') } - return WIKI.models.tree.listPages({ + const pages = await WIKI.models.tree.listPages({ siteId: req.params.siteId, path: req.query.path, locale: req.query.locale ?? defaultLocale(req.params.siteId), @@ -351,6 +411,15 @@ async function routes(app: FastifyInstance) { depth: req.query.depth, publicOnly: !req.session?.authenticated }) + // -> An index block is drawn inside a page, but it lists other pages: each one still has to be + // the reader's to see + const actor = WIKI.models.groups.actorForRequest(req) + return pages.filter((page) => + WIKI.models.groups.checkAccess(actor, 'read:pages', { + path: page.path, + locale: req.query.locale ?? defaultLocale(req.params.siteId) + }) + ) } ) @@ -360,9 +429,7 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string; folderId: string } }>( '/sites/:siteId/tree/folders/:folderId', { - config: { - permissions: ['read:pages', 'read:assets', 'manage:pages', 'manage:assets'] - }, + // -> Checked against the folder's own path below, not against the group-wide list schema: { summary: 'Get a single folder', tags: ['Tree'], @@ -377,6 +444,11 @@ async function routes(app: FastifyInstance) { if (!folder || folder.siteId !== req.params.siteId) { return reply.notFound('This folder does not exist.') } + const folderPath = folderPathOf(folder) + // -> Not visible is the same as not there, so it answers as the id had matched nothing + if (!mayOnFolder(req, 'read:pages', folderPath)) { + return reply.notFound('This folder does not exist.') + } return { ...folder, folderPath: decodeTreePath(folder.folderPath ?? '') ?? '', @@ -391,9 +463,10 @@ async function routes(app: FastifyInstance) { app.post<{ Params: { siteId: string }; Body: FolderBody }>( '/sites/:siteId/tree/folders', { - config: { - permissions: ['write:pages', 'write:assets', 'manage:pages', 'manage:assets'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and page permissions come + from a group's RULES. Checked against the folder's own path below. + */ schema: { summary: 'Create a folder', description: @@ -443,7 +516,20 @@ async function routes(app: FastifyInstance) { } } }, - async (req) => { + async (req, reply) => { + /* + Against where the folder is going. `parentPath` is the slash-separated path when given; with + `parentId` the parent has to be looked up, and a missing one is left to the model to report. + */ + let parentPath = req.body.parentPath ?? '' + if (req.body.parentId) { + const parent = await WIKI.models.tree.getFolderById(req.body.parentId) + parentPath = parent ? folderPathOf(parent) : parentPath + } + const target = [parentPath, req.body.pathName].filter(Boolean).join('/') + if (!mayOnFolder(req, 'manage:pages', target)) { + return reply.forbidden('You are not allowed to create a folder here.') + } const folder = await WIKI.models.tree.createFolder({ siteId: req.params.siteId, locale: req.body.locale ?? defaultLocale(req.params.siteId), @@ -470,9 +556,10 @@ async function routes(app: FastifyInstance) { app.patch<{ Params: { siteId: string; folderId: string }; Body: FolderBody }>( '/sites/:siteId/tree/folders/:folderId', { - config: { - permissions: ['manage:pages', 'manage:assets'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and page permissions come + from a group's RULES. Checked against the folder's own path below. + */ schema: { summary: 'Rename a folder', description: @@ -504,6 +591,9 @@ async function routes(app: FastifyInstance) { if (!existing || existing.siteId !== req.params.siteId) { return reply.notFound('This folder does not exist.') } + if (!mayOnFolder(req, 'manage:pages', folderPathOf(existing))) { + return reply.forbidden('You are not allowed to rename this folder.') + } const folder = await WIKI.models.tree.renameFolder({ folderId: req.params.folderId, pathName: req.body.pathName, @@ -527,9 +617,10 @@ async function routes(app: FastifyInstance) { app.delete<{ Params: { siteId: string; folderId: string } }>( '/sites/:siteId/tree/folders/:folderId', { - config: { - permissions: ['manage:pages', 'manage:assets'] - }, + /* + No route-level `permissions`: that hook reads the group-wide list, and page permissions come + from a group's RULES. Checked against the folder's own path below. + */ schema: { summary: 'Delete a folder', description: @@ -548,6 +639,9 @@ async function routes(app: FastifyInstance) { if (!existing || existing.siteId !== req.params.siteId) { return reply.notFound('This folder does not exist.') } + if (!mayOnFolder(req, 'manage:pages', folderPathOf(existing))) { + return reply.forbidden('You are not allowed to delete this folder.') + } const removed = await WIKI.models.tree.deleteFolder(req.params.folderId) await WIKI.models.assets.deleteOrphaned(removed.assets) return reply.code(204).send() diff --git a/backend/helpers/pageRules.ts b/backend/helpers/pageRules.ts new file mode 100644 index 000000000..7a26aac84 --- /dev/null +++ b/backend/helpers/pageRules.ts @@ -0,0 +1,179 @@ +import type { GroupRule, GroupRuleMatch, GroupRuleMode } from '../models/groups.ts' + +/** + * How a page rule is matched against a page, and which rule wins when several match. + * + * --------------------------------------------------------------------------------------------- + * THE RULES OF PAGE PERMISSIONS + * --------------------------------------------------------------------------------------------- + * + * A group grants page permissions through rules, never as a blanket. Every rule names a set of + * permissions (`roles`), a way of addressing pages (`match` + `path`), and what it does with them + * (`mode`). A user's rules are all of their groups' rules pooled together — belonging to a second + * group can therefore both widen and narrow what the first one said. + * + * **Nothing is granted by default.** A permission nobody wrote a rule for is denied: no rules at all + * is the same as one DENY rule covering the whole site. This is why an empty group can read nothing. + * + * When more than one rule names the permission being asked about and matches the page, exactly one + * of them decides the answer — the most specific. Order in the array means nothing. + * + * 1. SPECIFICITY, highest first. A rule addressing `geography/countries` beats one addressing + * `geography`, because it says something about a smaller part of the site. Measured as the + * length of the path the rule addresses, so the deeper of two paths always wins, and a rule for + * the whole site (empty path) is the least specific thing there is. Tag rules address no path + * at all and are therefore never more specific than a path rule. + * + * 2. MATCH TYPE, when two rules are equally specific. From weakest to strongest: + * + * Has Any Tag < Has All Tags < Path Starts With < Path Ends With < + * Path Matches Regex < Path Is Exactly + * + * The order runs from the vaguest way of naming pages to the most precise: a tag is a property + * a page happens to have, a prefix is a whole branch of the tree, and an exact path is one page + * and nothing else. + * + * 3. MODE, when two rules are equally specific and of the same kind: + * + * ALLOW < DENY < FORCE ALLOW + * + * An ALLOW grants the permission. A DENY overrides any ALLOW. A FORCE ALLOW overrides any DENY, + * which is what makes a hole in an otherwise closed branch possible. + * + * The three are applied in that order: mode only settles a tie between rules of the same kind at the + * same specificity, so a DENY on `geography` does NOT override an ALLOW on `geography/countries` — + * the deeper rule was more specific and had already won. + * + * --------------------------------------------------------------------------------------------- + * + * `manage:system` is not evaluated here: it bypasses this entirely, and does so before any rule is + * read. See `models/groups.ts`. + */ + +/** A page as a rule sees it. `locale` and `path` place it; `tags` are what tag rules match on. */ +export interface RulePageRef { + path: string + locale?: string + tags?: string[] +} + +/** + * Match kinds from weakest to strongest, used to break a tie between equally specific rules. The + * index IS the priority, so the order of this array is the order documented above. + */ +const MATCH_PRIORITY: GroupRuleMatch[] = ['TAG', 'TAGALL', 'START', 'END', 'REGEX', 'EXACT'] + +/** Modes from weakest to strongest, used to break a tie between rules of the same kind. */ +const MODE_PRIORITY: GroupRuleMode[] = ['ALLOW', 'DENY', 'FORCEALLOW'] + +/** Tags are written on a rule as a comma-separated list, in the field a path would otherwise use. */ +function ruleTags(rule: GroupRule): string[] { + return rule.path + .split(',') + .map((tag) => tag.trim().toLowerCase()) + .filter(Boolean) +} + +/** Compared without leading slashes on either side, since neither is stored with one. */ +function normalizePath(value: string): string { + return value.replace(/^\/+/, '') +} + +/** + * How much of the site a rule is talking about, as a number where higher is narrower. + * + * The length of the path it addresses. A tag rule addresses no path, so it scores zero and can never + * out-specify a rule that names one — matching the ordering above, where tags are the vaguest way of + * naming a page. + */ +function specificityOf(rule: GroupRule): number { + if (rule.match === 'TAG' || rule.match === 'TAGALL') { + return 0 + } + return normalizePath(rule.path).length +} + +/** Whether a rule addresses this page at all, ignoring what it then says about it. */ +export function ruleMatchesPage(rule: GroupRule, page: RulePageRef): boolean { + // -> A rule may be limited to particular locales; an empty list means every one of them + if (rule.locales?.length > 0 && page.locale && !rule.locales.includes(page.locale)) { + return false + } + + const pagePath = normalizePath(page.path) + const rulePath = normalizePath(rule.path) + const pageTags = (page.tags ?? []).map((tag) => tag.toLowerCase()) + + switch (rule.match) { + case 'START': + return pagePath.startsWith(rulePath) + case 'EXACT': + return pagePath === rulePath + case 'END': + return pagePath.endsWith(rulePath) + case 'REGEX': + try { + return new RegExp(rulePath).test(pagePath) + } catch { + // -> A rule that cannot compile addresses nothing, rather than everything + return false + } + case 'TAG': + return ruleTags(rule).some((tag) => pageTags.includes(tag)) + case 'TAGALL': { + const tags = ruleTags(rule) + return tags.length > 0 && tags.every((tag) => pageTags.includes(tag)) + } + default: + return false + } +} + +/** + * The rule that decides a permission for a page, out of everything the caller's groups say. + * + * @param rules Every rule from every group the caller belongs to, pooled + * @param permission The single permission being asked about, e.g. `read:pages` + * @returns The deciding rule, or null when nothing addresses this — which means denied + */ +export function resolvePageRule( + rules: GroupRule[], + permission: string, + page: RulePageRef +): GroupRule | null { + let winner: GroupRule | null = null + let winnerRank: [number, number, number] = [-1, -1, -1] + + for (const rule of rules) { + if (!rule.roles?.includes(permission) || !ruleMatchesPage(rule, page)) { + continue + } + const rank: [number, number, number] = [ + specificityOf(rule), + MATCH_PRIORITY.indexOf(rule.match), + MODE_PRIORITY.indexOf(rule.mode) + ] + // -> Strictly greater, so the first rule of an otherwise identical pair wins and the outcome + // does not depend on the order they happen to arrive in + if ( + rank[0] > winnerRank[0] || + (rank[0] === winnerRank[0] && + (rank[1] > winnerRank[1] || (rank[1] === winnerRank[1] && rank[2] > winnerRank[2]))) + ) { + winner = rule + winnerRank = rank + } + } + + return winner +} + +/** + * Whether the caller's rules grant a permission on a page. + * + * @returns False when no rule addresses it, which is the default for everything. + */ +export function rulesAllow(rules: GroupRule[], permission: string, page: RulePageRef): boolean { + const rule = resolvePageRule(rules, permission, page) + return rule ? rule.mode !== 'DENY' : false +} diff --git a/backend/index.ts b/backend/index.ts index b9415147d..b89886d0b 100644 --- a/backend/index.ts +++ b/backend/index.ts @@ -137,6 +137,8 @@ async function postBoot() { await WIKI.models.authentication.activateStrategies() await WIKI.models.locales.reloadCache() await WIKI.models.sites.reloadCache() + // -> Page access is decided from these on every request, so they are in memory from the start + await WIKI.models.groups.reloadCache() // -> Must follow the sites cache: every site gets a row per installed block await WIKI.models.blocks.refreshFromDisk() @@ -357,10 +359,10 @@ async function initHTTPServer() { app.register(fastifySwagger, { hideUntagged: true, openapi: { - openapi: '3.0.0', + openapi: '3.1.0', info: { title: 'Wiki.js API', - version: WIKI.config.version + version: WIKI.version }, components: { securitySchemes: { @@ -397,9 +399,18 @@ async function initHTTPServer() { transformedSchema.description = `${currentDescription}\n\n**Required Permissions:** ${uniq(nestedPermissions).join(' or ')}`.trim() transformedSchema['x-permissions'] = permissions - } else { + } else if (route?.config?.publicAccess) { transformedSchema.description = `${currentDescription}\n\n**This API is public.** No special permissions required.`.trim() + } else { + /* + No fixed permission is not the same as public, and saying so was wrong for most of these. + A route without one is usually a route whose answer depends on the caller: the page rules of + their groups, their own account, or the queue they happen to be a reviewer for. What it + serves is scoped, not unrestricted. + */ + transformedSchema.description = + `${currentDescription}\n\n**No fixed permission.** What this returns, and what it acts on, is limited to what the caller is entitled to — their session, their groups' page rules, or their own account. A request that is entitled to nothing gets an empty answer or a refusal rather than an error about permissions.`.trim() } return { schema: transformedSchema, url } @@ -407,7 +418,39 @@ async function initHTTPServer() { }) app.register(fastifySwaggerUi, { routePrefix: '/_api', - logo: {} as any + // -> Left empty so the plugin inlines neither its own logo nor one of ours; the stylesheet below + // is what puts the site's logo in the topbar + logo: {} as any, + theme: { + css: [ + { + filename: 'wiki.css', + /* + The site's own logo in the topbar, as a background on the link swagger draws its wordmark + in. + + A stylesheet rather than the plugin's `logo` option, which takes a buffer and base64-inlines + it into the page when the server boots. This documentation is served for whichever site the + request arrived at, and an administrator can change that site's logo at any time — a URL + resolves both of those per request, and a buffer chosen at boot resolves neither. + + `contain` in a box wider than it is tall, so a square mark and a wordmark both sit sensibly + without the logo being distorted to fit. + */ + content: ` + .swagger-ui .topbar-wrapper a.link > * { + display: none; + } + .swagger-ui .topbar-wrapper a.link { + display: block; + width: 160px; + height: 40px; + background: url('/_site/current/logo') left center / contain no-repeat; + } + ` + } + ] + } }) // ---------------------------------------- diff --git a/backend/locales/en.json b/backend/locales/en.json index 271fdb83b..3cf2c3177 100644 --- a/backend/locales/en.json +++ b/backend/locales/en.json @@ -95,9 +95,10 @@ "admin.approval.nameHint": "How this rule is identified in the list, e.g. Documentation suggestions", "admin.approval.nameRequired": "A rule name is required.", "admin.approval.newRule": "New Rule", - "admin.approval.noRules": "No approval rules yet. Pages accept no edit suggestions until a rule covers them.", + "admin.approval.noRules": "No approval rules yet. Pages accept no edit suggestions from read-only users until a rule covers them.", "admin.approval.path": "Path", "admin.approval.pathHint": "Without the leading slash, e.g. docs/getting-started", + "admin.approval.pathHintStart": "Without the leading slash, e.g. docs/. Leave empty to cover the whole site.", "admin.approval.pathInvalidRegex": "Not a valid regular expression: {reason}", "admin.approval.pathRequired": "A path is required.", "admin.approval.reviewers": "Reviews submissions", @@ -107,7 +108,7 @@ "admin.approval.submitters": "Can submit edits", "admin.approval.submittersHint": "Members of these groups can submit edit suggestions for matching pages.", "admin.approval.submittersRequired": "Select at least one group that can submit edits.", - "admin.approval.subtitle": "Define which pages accept edit suggestions, and who reviews them", + "admin.approval.subtitle": "Define which pages accept edit suggestions from read-only users, and who reviews them", "admin.approval.tags": "Tags", "admin.approval.tagsHint": "Comma-separated list of tags.", "admin.approval.tagsRequired": "At least one tag is required.", @@ -281,8 +282,6 @@ "admin.general.allowBrowseHint": "Can users browse using the tree structure of the site to pages they have access to?", "admin.general.allowComments": "Allow Comments", "admin.general.allowCommentsHint": "Can users leave comments on pages? Can be restricted using Page Rules.", - "admin.general.allowContributions": "Allow Contributions", - "admin.general.allowContributionsHint": "Can users with read access permissions propose changes for pages? Can be restricted using Page Rules.", "admin.general.allowProfile": "Allow Profile Editing", "admin.general.allowProfileHint": "Can users edit their own profile? If profile data is managed by an external identity provider, you should turn this off.", "admin.general.allowRatings": "Allow Ratings", diff --git a/backend/models/approvals.ts b/backend/models/approvals.ts index 37e319793..28a4214e6 100644 --- a/backend/models/approvals.ts +++ b/backend/models/approvals.ts @@ -18,12 +18,35 @@ export const approvalMatchModes = ['START', 'EXACT', 'END', 'REGEX', 'TAG', 'TAG export type ApprovalMatchMode = (typeof approvalMatchModes)[number] /** The part of a page a rule is matched against. */ -export interface ApprovalPageRef { - id: string +/** What a rule is matched against: where the page is, and what it is tagged with. */ +export interface ApprovalPageMatch { path: string tags: string[] } +export interface ApprovalPageRef extends ApprovalPageMatch { + id: string + /** + * The page's own switch, from its properties. A page with contributions turned off takes no + * suggestions whatever the rules say — which is how a single page is exempted without writing a + * rule around it. + */ + allowContributions: boolean +} + +/** + * Who is reviewing, as the rules see them. + * + * `reviewsAll` covers the two ways of being a reviewer without a rule naming your group: the + * `manage:system` permission, which sees everything everywhere, and `review:pages`, which is granted + * to review pages and would be worth nothing if it could not. Neither widens WHICH pages take + * suggestions -- a page still needs a rule -- only who may answer them. + */ +export interface ReviewerScope { + groupIds: string[] + reviewsAll?: boolean +} + /** An edit suggested against a page, as the author's own view of it. */ export interface PageEditSubmission { id: string @@ -184,7 +207,10 @@ class Approvals { name: patch.name ?? '', isEnabled: patch.isEnabled ?? true, match: patch.match ?? 'START', - path: patch.path ?? '', + // -> Trimmed, so a pattern typed with a stray space still matches what it reads as -- and so + // that a `START` path of nothing but spaces is the whole site rather than a rule that + // quietly covers no page at all + path: (patch.path ?? '').trim(), submitterGroups: patch.submitterGroups ?? [], reviewerGroups: patch.reviewerGroups ?? [] }) @@ -212,7 +238,8 @@ class Approvals { 'reviewerGroups' ] as const) { if (patch[key] !== undefined) { - values[key] = patch[key] + // -> Trimmed for the same reason it is on create + values[key] = key === 'path' ? String(patch[key]).trim() : patch[key] } } @@ -232,7 +259,7 @@ class Approvals { * throwing: the rule is already refused at the API, so this is only reached by one that was valid * when it was written and stopped being so. */ - matchesPage(rule: ApprovalRule, page: ApprovalPageRef): boolean { + matchesPage(rule: ApprovalRule, page: ApprovalPageMatch): boolean { const pagePath = page.path.replace(/^\/+/, '') const rulePath = rule.path.replace(/^\/+/, '') switch (rule.match) { @@ -277,6 +304,13 @@ class Approvals { /** * The enabled rule that lets these groups suggest an edit to this page, if there is one. * + * The page's own `allowContributions` is a veto rather than another condition to match: a rule says + * which pages MAY take suggestions, and turning the switch off on one page says that this one does + * not — no rule has to be rewritten, narrowed or excluded around it. + * + * Everything asking whether a page takes a suggestion asks this, which is why the check lives here + * rather than at either route. + * * @returns The first matching rule, or null when the page takes no suggestions from them */ async findSubmitRule( @@ -284,7 +318,7 @@ class Approvals { page: ApprovalPageRef, groupIds: string[] ): Promise { - if (groupIds.length < 1) { + if (groupIds.length < 1 || !page.allowContributions) { return null } const rules = await this.getRules(siteId) @@ -298,6 +332,31 @@ class Approvals { ) } + /** + * Whether this reviewer has any business reviewing this page at all. + * + * What decides whether the page view offers a review button, so it is about the page rather than + * about what happens to be waiting on it: a reviewer of a page with an empty queue is still its + * reviewer. A page no rule covers takes no suggestions, so nobody reviews it -- not even an + * administrator, who would only be offered a button that could never have anything behind it. + */ + async canReviewPage( + siteId: string, + page: ApprovalPageMatch, + { groupIds, reviewsAll = false }: ReviewerScope + ): Promise { + if (!reviewsAll && groupIds.length < 1) { + return false + } + const rules = await this.getRules(siteId) + return rules.some( + (rule) => + rule.isEnabled && + (reviewsAll || rule.reviewerGroups.some((id) => groupIds.includes(id))) && + this.matchesPage(rule, page) + ) + } + /** * The suggestion this user already has open on this page, if any. * @@ -413,14 +472,14 @@ class Approvals { */ async getReviewableSubmissions( siteId: string, - { groupIds, isAdmin = false }: { groupIds: string[]; isAdmin?: boolean } + { groupIds, reviewsAll = false, pageId }: ReviewerScope & { pageId?: string } ): Promise { - if (!isAdmin && groupIds.length < 1) { + if (!reviewsAll && groupIds.length < 1) { return [] } const rules = (await this.getRules(siteId)).filter( (rule) => - rule.isEnabled && (isAdmin || rule.reviewerGroups.some((id) => groupIds.includes(id))) + rule.isEnabled && (reviewsAll || rule.reviewerGroups.some((id) => groupIds.includes(id))) ) if (rules.length < 1) { return [] @@ -447,7 +506,11 @@ class Approvals { .from(submissionsTable) .innerJoin(pagesTable, eq(pagesTable.id, submissionsTable.pageId)) .leftJoin(usersTable, eq(usersTable.id, submissionsTable.authorId)) - .where(eq(submissionsTable.siteId, siteId)) + .where( + pageId + ? and(eq(submissionsTable.siteId, siteId), eq(submissionsTable.pageId, pageId)) + : eq(submissionsTable.siteId, siteId) + ) .orderBy(asc(submissionsTable.createdAt)) // -> Matched in memory rather than in SQL: a rule can be a regular expression or a set of tags, @@ -455,7 +518,13 @@ class Approvals { return rows .filter((row: any) => rules.some((rule) => - this.matchesPage(rule, { id: row.pageId, path: row.pagePath, tags: row.pageTags ?? [] }) + /* + No `allowContributions` here, deliberately: that switch governs whether a suggestion may + be MADE. One already sent stays in its reviewers' queue if the page is later closed to + contributions -- otherwise turning the switch off would silently strand work somebody had + submitted in good faith, with nobody able to accept or decline it. + */ + this.matchesPage(rule, { path: row.pagePath, tags: row.pageTags ?? [] }) ) ) .map((row: any) => this.toReviewable(row)) @@ -469,10 +538,10 @@ class Approvals { async getSubmissionForReview( siteId: string, submissionId: string, - { groupIds, isAdmin = false }: { groupIds: string[]; isAdmin?: boolean } + { groupIds, reviewsAll = false }: ReviewerScope ): Promise { // -> Reuses the queue rather than re-deriving who may see what: one definition of reviewable - const reviewable = await this.getReviewableSubmissions(siteId, { groupIds, isAdmin }) + const reviewable = await this.getReviewableSubmissions(siteId, { groupIds, reviewsAll }) if (!reviewable.some((s) => s.id === submissionId)) { return null } diff --git a/backend/models/groups.ts b/backend/models/groups.ts index e3aeae70a..132985028 100644 --- a/backend/models/groups.ts +++ b/backend/models/groups.ts @@ -1,7 +1,9 @@ import { v4 as uuid } from 'uuid' import { and, count, eq, ilike, or, sql } from 'drizzle-orm' import { groups as groupsTable, userGroups, users as usersTable } from '../db/schema.ts' +import { resolvePageRule, type RulePageRef } from '../helpers/pageRules.ts' import type { SystemIds } from './types.ts' +import type { FastifyRequest } from 'fastify' /** How a rule's `path` is compared against the page path. */ export type GroupRuleMatch = 'START' | 'END' | 'REGEX' | 'TAG' | 'TAGALL' | 'EXACT' @@ -94,10 +96,95 @@ const groupSelection = { userCount: count(userGroups.userId) } +/** + * Who is asking, and what they hold outside the page rules. + * + * `permissions` is the group-wide list — `manage:system`, `access:admin` and the rest — which is a + * different thing from the page permissions the rules decide. + */ +export interface AccessActor { + groupIds: string[] + permissions: string[] +} + +/** + * Every group's rules, by group id. + * + * Cached because a page permission is checked on every page read, and reading three rows out of the + * database to answer it would put a query in front of every request. Reloaded whenever a group + * changes, the same way the site configurations are. + */ +let rulesCache: Record = {} + /** * Groups model */ class Groups { + /** + * Reload the page rules of every group into memory. + * + * Called at boot and after any change to a group. A group edit therefore takes effect on the next + * request rather than on the next login, which matters: rules are the whole of page access, and a + * revoked permission that waits for a logout is not revoked. + */ + async reloadCache(): Promise { + const rows = await WIKI.db + .select({ id: groupsTable.id, rules: groupsTable.rules }) + .from(groupsTable) + rulesCache = {} + for (const row of rows) { + rulesCache[row.id] = (row.rules ?? []) as GroupRule[] + } + WIKI.logger.info(`Loaded page rules for ${rows.length} groups [ OK ]`) + } + + /** The pooled rules of a set of groups, which is what a permission is decided against. */ + rulesForGroups(groupIds: string[]): GroupRule[] { + return groupIds.flatMap((id) => rulesCache[id] ?? []) + } + + /** + * Which groups a request speaks for. + * + * An anonymous request is not group-less: it is the guests group, whose rules are how a wiki says + * what the public may see. Treating it as no groups at all would deny everything, which is a + * different answer from the one the administrator configured. + */ + groupIdsForRequest(req: FastifyRequest): string[] { + if (req.session?.authenticated && req.session.user?.id) { + return req.session.groups ?? [] + } + return [WIKI.data.systemIds.guestsGroupId] + } + + /** The actor a request speaks for: its groups, and the group-wide permissions it holds. */ + actorForRequest(req: FastifyRequest): AccessActor { + return { + groupIds: this.groupIdsForRequest(req), + // -> An API key stands in for a session and carries its own permissions, as it does in the + // route-level check + permissions: req.apiKey?.permissions ?? req.session?.permissions ?? [] + } + } + + /** + * Whether this caller may do this to this page. + * + * The one place page permissions are decided. Everything page-scoped asks this rather than reading + * the session's permission list, because that list says what a group was granted GLOBALLY and page + * permissions are not granted that way — see `helpers/pageRules.ts` for how a rule is chosen. + * + * @param permission A single page permission, e.g. `read:pages` or `read:history` + */ + checkAccess(actor: AccessActor, permission: string, page: RulePageRef): boolean { + // -> Above the rules entirely: an administrator is not something a rule can lock out, and a + // wiki whose only administrator had denied themselves would have nobody left to fix it + if (actor.permissions.includes('manage:system')) { + return true + } + const rule = resolvePageRule(this.rulesForGroups(actor.groupIds), permission, page) + return rule ? rule.mode !== 'DENY' : false + } async init(ids: SystemIds): Promise { WIKI.logger.info('Inserting default groups...') @@ -177,6 +264,7 @@ class Groups { isSystem: false }) .returning({ id: groupsTable.id }) + await this.reloadCache() return result[0].id } @@ -222,6 +310,7 @@ class Groups { .update(groupsTable) .set({ ...patch, updatedAt: sql`now()` }) .where(eq(groupsTable.id, id)) + await this.reloadCache() return (result.rowCount ?? 0) > 0 } @@ -233,6 +322,7 @@ class Groups { */ async deleteGroup(id: string): Promise { const result = await WIKI.db.delete(groupsTable).where(eq(groupsTable.id, id)) + await this.reloadCache() return (result.rowCount ?? 0) > 0 } diff --git a/backend/models/search.ts b/backend/models/search.ts index 3ac824163..33affcb14 100644 --- a/backend/models/search.ts +++ b/backend/models/search.ts @@ -1,4 +1,5 @@ import { sql } from 'drizzle-orm' +import type { AccessActor } from './groups.ts' /** * Locale to PostgreSQL text search dictionary, for the languages postgres ships a snowball stemmer @@ -90,6 +91,13 @@ export interface SearchPagesParams { publicOnly?: boolean /** Whether unpublished pages belong in the results, which is an editor's view of the wiki. */ includeDrafts?: boolean + /** + * Who is searching, so that a result they could not open never reaches them. + * + * Applied to the rows rather than in the query: which pages a rule covers can depend on a regular + * expression or on a page's tags, neither of which a `WHERE` clause here could express. + */ + actor?: AccessActor /** * Keep a password-protected page's *body* out of the results, for a searcher who would have to enter * the password to read it. The page itself still appears — its title and description are not what @@ -219,7 +227,8 @@ class Search { limit = 25, publicOnly = false, includeDrafts = false, - hideProtectedContent = true + hideProtectedContent = true, + actor }: SearchPagesParams): Promise { const terms = query.trim() const hasQuery = terms.length > 0 @@ -227,10 +236,17 @@ class Search { // -> Only the locales in play need an arm in the dictionary CASE const siteLocales: string[] = WIKI.sites[siteId]?.config?.locales?.active ?? ['en'] const searchedLocales = locales.length > 0 ? locales : siteLocales - const dict = this.dictionaryExpression( - searchedLocales, - hasQuery ? await this.getAvailableDictionaries() : [] - ) + /* + No terms means no query to parse, and therefore no dictionary to parse it with. + + Both arguments are withheld together on purpose. Passing the locales while claiming nothing is + installed -- which is what an empty `available` says -- made every locale resolve to the + fallback and warn that its dictionary was missing, on a code path that never uses the answer. + That warning was the one in the logs: `english` is installed, nobody had looked. + */ + const dict = hasQuery + ? this.dictionaryExpression(searchedLocales, await this.getAvailableDictionaries()) + : this.dictionaryExpression([], []) const tsQuery = sql`websearch_to_tsquery(${dict}, ${terms})` const conditions = [sql`p."siteId" = ${siteId}`, sql`p."isSearchable" = true`] @@ -321,7 +337,22 @@ class Search { LIMIT ${limit} OFFSET ${offset} `) - const result = ((rows.rows ?? rows) as any[]).map((row) => ({ + /* + Filtered here rather than in SQL: a page rule can be a regular expression or a set of tags, so + the deciding rule is only knowable per row. Search must not be a way around page permissions — + a title and an excerpt are content too. + */ + const visible = actor + ? ((rows.rows ?? rows) as any[]).filter((row) => + WIKI.models.groups.checkAccess(actor, 'read:pages', { + path: row.path as string, + locale: row.locale as string, + tags: (row.tags ?? []) as string[] + }) + ) + : ((rows.rows ?? rows) as any[]) + + const result = visible.map((row) => ({ id: row.id as string, path: row.path as string, locale: row.locale as string, @@ -341,7 +372,18 @@ class Search { return { results: result, - totalHits: Number((rows.rows ?? rows)[0]?.totalHits ?? 0) + /* + The count postgres reported, less whatever the rules just removed from this page of results. + Not exact when rows are dropped -- the window function counted every match, including ones on + later pages this reader may not see -- but a total that ignored the filtering entirely would + promise results that do not exist. + */ + totalHits: Math.max( + 0, + Number((rows.rows ?? rows)[0]?.totalHits ?? 0) - + ((rows.rows ?? rows) as any[]).length + + visible.length + ) } } diff --git a/backend/models/sites.ts b/backend/models/sites.ts index 7bcff096a..afa2fb403 100644 --- a/backend/models/sites.ts +++ b/backend/models/sites.ts @@ -86,9 +86,8 @@ class Sites { ratings: false, ratingsMode: 'off', comments: false, - contributions: false, profile: true, - reasonForChange: 'required', + reasonForChange: 'optional', search: true }, logoUrl: '', @@ -279,9 +278,8 @@ class Sites { ratings: false, ratingsMode: 'off', comments: false, - contributions: false, profile: true, - reasonForChange: 'required', + reasonForChange: 'optional', search: true }, logoText: true, diff --git a/backend/models/tags.ts b/backend/models/tags.ts index 793e8aca2..a6b189b05 100644 --- a/backend/models/tags.ts +++ b/backend/models/tags.ts @@ -1,4 +1,5 @@ import { sql } from 'drizzle-orm' +import type { AccessActor } from './groups.ts' export interface Tag { tag: string @@ -21,20 +22,58 @@ class Tags { * * @param siteId Site the pages belong to * @param limit Ceiling on how many distinct tags come back, most used first + * @param actor Who is asking. Given one, the list is built only from the pages they may read — + * a tag is the name of something on a page, and the set of tags in use tells a + * reader what a wiki is about. Counted over readable pages too, so the numbers agree + * with what a search for the tag would return. */ - async getTags(siteId: string, { limit = 1000 }: { limit?: number } = {}): Promise { + async getTags( + siteId: string, + { limit = 1000, actor }: { limit?: number; actor?: AccessActor } = {} + ): Promise { + if (!actor) { + const result = await WIKI.db.execute(sql` + SELECT tag, COUNT(*)::int AS "usageCount" + FROM pages, unnest(tags) AS tag + WHERE "siteId" = ${siteId} + GROUP BY tag + ORDER BY COUNT(*) DESC, tag ASC + LIMIT ${limit} + `) + return ((result.rows ?? result) as any[]).map((row) => ({ + tag: row.tag as string, + usageCount: row.usageCount as number + })) + } + + /* + Aggregated here rather than in postgres, because which pages count depends on the page rules and + a rule can be a regular expression or a set of tags — neither of which a `GROUP BY` could take + into account. Only tagged pages are read, and only their path, locale and tags. + */ const result = await WIKI.db.execute(sql` - SELECT tag, COUNT(*)::int AS "usageCount" - FROM pages, unnest(tags) AS tag - WHERE "siteId" = ${siteId} - GROUP BY tag - ORDER BY COUNT(*) DESC, tag ASC - LIMIT ${limit} + SELECT path, locale, tags + FROM pages + WHERE "siteId" = ${siteId} AND array_length(tags, 1) > 0 `) - return ((result.rows ?? result) as any[]).map((row) => ({ - tag: row.tag as string, - usageCount: row.usageCount as number - })) + const counts = new Map() + for (const row of (result.rows ?? result) as any[]) { + const page = { + path: row.path as string, + locale: row.locale as string, + tags: (row.tags ?? []) as string[] + } + if (!WIKI.models.groups.checkAccess(actor, 'read:pages', page)) { + continue + } + for (const tag of page.tags) { + counts.set(tag, (counts.get(tag) ?? 0) + 1) + } + } + return [...counts.entries()] + .map(([tag, usageCount]) => ({ tag, usageCount })) + .sort((a, b) => b.usageCount - a.usageCount || a.tag.localeCompare(b.tag)) + .slice(0, limit) } } diff --git a/backend/types/fastify.d.ts b/backend/types/fastify.d.ts index a85691a38..42c205942 100644 --- a/backend/types/fastify.d.ts +++ b/backend/types/fastify.d.ts @@ -63,5 +63,14 @@ declare module 'fastify' { * The outer array is OR-ed; a nested array is AND-ed. `manage:system` bypasses the check. */ permissions?: (string | string[])[] + /** + * Whether this route genuinely serves everybody the same thing. + * + * Only affects the API documentation. A route with no `permissions` is not thereby public: most + * of them answer according to who is asking — the caller's session, their groups' page rules, or + * their own account — and the docs say so. This marks the few where a guest and an administrator + * really do get the same reply, so that the difference is stated rather than assumed. + */ + publicAccess?: boolean } } diff --git a/frontend/src/assets/icons.generated.js b/frontend/src/assets/icons.generated.js index 2f20ded62..69850f5e8 100644 --- a/frontend/src/assets/icons.generated.js +++ b/frontend/src/assets/icons.generated.js @@ -5,7 +5,7 @@ never waits on (or depends on) the icon service. Regenerate with `npm run icons` after adding or removing an icon; `check-icons.mjs` fails the build if this drifts. - 256 icons. + 257 icons. */ export const BUNDLED_ICONS = { "la:angle-double-right": {"body":"","width":32,"height":32}, @@ -77,6 +77,7 @@ export const BUNDLED_ICONS = { "la:icons": {"body":"","width":32,"height":32}, "la:id-card": {"body":"","width":32,"height":32}, "la:image": {"body":"","width":32,"height":32}, + "la:inbox": {"body":"","width":32,"height":32}, "la:infinity": {"body":"","width":32,"height":32}, "la:info-circle": {"body":"","width":32,"height":32}, "la:js-square": {"body":"","width":32,"height":32}, diff --git a/frontend/src/components/ApprovalRuleDialog.vue b/frontend/src/components/ApprovalRuleDialog.vue index 9bc5a8ac4..acae1a30a 100644 --- a/frontend/src/components/ApprovalRuleDialog.vue +++ b/frontend/src/components/ApprovalRuleDialog.vue @@ -6,8 +6,14 @@ {{ isEdit ? t('admin.approval.editRule') : t('admin.approval.newRule') }} + - + - + - + {{ opt.title }} - {{opt.hint}} + {{ opt.hint }} @@ -300,7 +314,11 @@ option-label="title" multiple behavior="dialog" - :display-value="t(`admin.groups.selectedSites`, rule.sites.length, { count: rule.sites.length })"> + :display-value=" + t(`admin.groups.selectedSites`, rule.sites.length, { + count: rule.sites.length + }) + "> + - + - + - + @@ -126,8 +126,12 @@ - {{ displayText }} + {{ showsChips ? '' : displayText }} { const hasSelection = computed(() => selectedValues.value.length > 0) +/** Whether the selection is being drawn as chips, which is a different thing from being able to. */ +const showsChips = computed(() => props.useChips && hasSelection.value) + const displayText = computed(() => { if (props.displayValue !== null) { return props.displayValue diff --git a/frontend/src/pages/AdminGeneral.vue b/frontend/src/pages/AdminGeneral.vue index f548a9b8e..f7d71748d 100644 --- a/frontend/src/pages/AdminGeneral.vue +++ b/frontend/src/pages/AdminGeneral.vue @@ -183,19 +183,6 @@ - - - - {{ t(`admin.general.allowContributions`) }} - {{ t(`admin.general.allowContributionsHint`) }} - - - - - - @@ -588,7 +575,6 @@ function defaultConfig() { ratings: false, ratingsMode: 'off', comments: false, - contributions: false, reasonForChange: 'required', profile: false }, @@ -708,7 +694,6 @@ async function save() { browse: state.config.features?.browse ?? false, comments: state.config.features?.comments ?? false, ratingsMode: state.config.features?.ratingsMode ?? 'off', - contributions: state.config.features?.contributions ?? false, profile: state.config.features?.profile ?? false, reasonForChange: state.config.features?.reasonForChange ?? 'required', search: state.config.features?.search ?? false diff --git a/frontend/src/pages/ErrorGeneric.vue b/frontend/src/pages/ErrorGeneric.vue index 049d7ebd0..e9dc5069e 100644 --- a/frontend/src/pages/ErrorGeneric.vue +++ b/frontend/src/pages/ErrorGeneric.vue @@ -2,9 +2,9 @@
-
{{error.code}}
-
{{error.title}}
-
{{error.hint}}
+
{{ error.code }}
+
{{ error.title }}
+
{{ error.hint }}
import { useI18n } from 'vue-i18n' -import { computed } from 'vue' -import { useRoute } from 'vue-router' +import { computed, onMounted } from 'vue' +import { useRoute, useRouter } from 'vue-router' import { useMeta } from '@/composables/meta' +import { useSiteStore } from '@/stores/site' +import { useUserStore } from '@/stores/user' + const actions = { unauthorized: { code: 403, @@ -53,6 +56,12 @@ const actions = { // ROUTER const route = useRoute() +const router = useRouter() + +// STORES + +const siteStore = useSiteStore() +const userStore = useUserStore() // I18N @@ -64,6 +73,26 @@ useMeta({ title: t('common.error.title') }) +// MOUNTED + +/* + A site can choose to skip this screen entirely for a visitor who is not logged in: with + `bypassUnauthorized` on, being refused a page sends them to sign in rather than to a page whose only + purpose is to offer them a login button. + + Only when nobody is logged in. Somebody who IS signed in and still refused has nothing to gain from + the login screen, and sending them there would bounce them straight back. +*/ +onMounted(() => { + if ( + route.params.action === 'unauthorized' && + siteStore.auth.bypassUnauthorized && + !userStore.authenticated + ) { + router.replace('/login') + } +}) + // COMPUTED const error = computed(() => { diff --git a/frontend/src/pages/InboxReview.vue b/frontend/src/pages/InboxReview.vue index 9c1c7c123..74090cb8a 100644 --- a/frontend/src/pages/InboxReview.vue +++ b/frontend/src/pages/InboxReview.vue @@ -143,6 +143,7 @@ diff --git a/frontend/src/pages/Index.vue b/frontend/src/pages/Index.vue index 06a99ed3c..a425e4b00 100644 --- a/frontend/src/pages/Index.vue +++ b/frontend/src/pages/Index.vue @@ -514,6 +514,10 @@ watch( message: 'This page does not exist (yet)!' }) } + } else if (err.message === 'ERR_PAGE_UNAUTHORIZED') { + // -> `replace`, so the back button leaves the wiki the way it came rather than bouncing off + // the same refusal again + router.replace('/_error/unauthorized') } else { notify({ type: 'negative', diff --git a/frontend/src/pages/Search.vue b/frontend/src/pages/Search.vue index e2f5b4b5d..899c7849c 100644 --- a/frontend/src/pages/Search.vue +++ b/frontend/src/pages/Search.vue @@ -172,14 +172,21 @@ {{ item.title }} {{ item.description }} + /{{ item.path }} -
+
{{ humanizeDate(item.updatedAt) }}
+ +
{{ tag }}
-
-
/{{ item.path }}
-
{{ humanizeDate(item.updatedAt) }}
-
diff --git a/frontend/src/router/routes.js b/frontend/src/router/routes.js index d89b647fa..66820a854 100644 --- a/frontend/src/router/routes.js +++ b/frontend/src/router/routes.js @@ -37,7 +37,12 @@ const routes = [ { path: '', redirect: '/_inbox/messages' }, { path: 'messages', component: () => import('@/pages/InboxMessages.vue') }, { path: 'watching', component: () => import('@/pages/InboxWatching.vue') }, - { path: 'review', component: () => import('@/pages/InboxReview.vue') } + /* + The submission being reviewed is in the URL, so a review can be linked to -- which is what a + notification about one will have to do. Optional, since the same screen without it is the + queue. + */ + { path: 'review/:submissionId?', component: () => import('@/pages/InboxReview.vue') } ] }, { diff --git a/frontend/src/stores/page.js b/frontend/src/stores/page.js index 5137fb13e..8a84079fa 100644 --- a/frontend/src/stores/page.js +++ b/frontend/src/stores/page.js @@ -165,6 +165,14 @@ export const usePageStore = defineStore('page', { if (err.response?.status === 404) { throw new Error('ERR_PAGE_NOT_FOUND') } + /* + Nor is a page the reader may not open: the group rules say so deliberately, and the reader + is owed the unauthorized screen -- which offers signing in as somebody else -- rather than + an error banner over an empty page view. + */ + if (err.response?.status === 403) { + throw new Error('ERR_PAGE_UNAUTHORIZED') + } console.warn(err) throw err } diff --git a/frontend/src/stores/site.js b/frontend/src/stores/site.js index bcc52797b..8afcc694d 100644 --- a/frontend/src/stores/site.js +++ b/frontend/src/stores/site.js @@ -67,6 +67,15 @@ export const useSiteStore = defineStore('site', { reasonForChange: 'required', search: false }, + /** How this site handles signing in. Set in the admin area's Login section. */ + auth: { + /** + * Send a visitor who is not logged in straight to the login screen instead of showing them + * the unauthorized page. For a wiki that is closed to the public, that screen is a dead end + * with a login button on it, and this skips the step. + */ + bypassUnauthorized: false + }, editors: { asciidoc: false, markdown: false, @@ -165,6 +174,10 @@ export const useSiteStore = defineStore('site', { ...this.features, ...siteInfo.features }, + auth: { + ...this.auth, + ...siteInfo.auth + }, editors: { asciidoc: siteInfo.editors.asciidoc.isActive, markdown: siteInfo.editors.markdown.isActive,