From 1105cf4b5e0d6cc9741c3812319dd73823d1dbe5 Mon Sep 17 00:00:00 2001 From: NGPixel Date: Sun, 2 Aug 2026 16:04:17 -0400 Subject: [PATCH] fix: permissions + optimize network calls per page --- CLAUDE.md | 48 ++++++- backend/api/approvals.ts | 22 ++++ backend/api/blocks.ts | 68 +++++++++- backend/api/bootstrap.ts | 78 ++++++++++++ backend/api/index.ts | 1 + backend/api/pages.ts | 71 ++++++++--- backend/api/schemas/page.ts | 34 ++++- backend/api/users.ts | 38 ++++-- backend/index.ts | 2 + backend/models/approvals.ts | 123 ++++++++++++++++-- frontend/src/App.vue | 59 ++++++--- frontend/src/components/FileManager.vue | 2 +- frontend/src/components/HeaderNav.vue | 8 +- frontend/src/components/PageActionsCol.vue | 63 +++------ frontend/src/components/PageHeader.vue | 2 +- frontend/src/components/UserEditOverlay.vue | 2 +- frontend/src/components/WelcomeOverlay.vue | 9 +- frontend/src/pages/Index.vue | 134 +++++++++++++++++--- frontend/src/stores/flags.js | 18 ++- frontend/src/stores/page.js | 131 ++++++++++++------- frontend/src/stores/site.js | 82 ++++++------ frontend/src/stores/user.js | 84 +++++++----- 22 files changed, 822 insertions(+), 257 deletions(-) create mode 100644 backend/api/bootstrap.ts diff --git a/CLAUDE.md b/CLAUDE.md index 6e4b425c0..8f845beec 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -278,6 +278,49 @@ These apply to **every workspace**, `frontend/` included — not just the backen mid-2026) and is awaited first in `main.js`. The polyfill is a lazy chunk (~21 kB gzipped) that browsers with native `Temporal` never download. +### Permissions + +There are **two kinds of permission**, granted separately and checked in different places. Which +kind a name belongs to decides how it may be enforced, so it is the first thing to establish about +any permission you touch. + +**Global permissions** are held site-wide, bound to no path: `access:admin`, `manage:users`, +`manage:groups`, `manage:navigation`, `manage:theme`, `manage:sites`, `manage:system`. That list is +the whole of it — 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. + +**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**: +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. +Ask `WIKI.models.groups.checkAccess(actor, permission, page)`, or `mayOnPage(req, permission, page)` +in `api/pages.ts`. + +Consequences worth knowing: + +- **A page permission cannot be enforced by `config.permissions`.** That hook reads the group-wide + list only, so `permissions: ['write:pages']` refuses everybody. A route that turns on a page + permission declares no route permission and checks in the handler instead — say so with a + `No route-level permissions:` comment, as `api/pages.ts`, `api/assets.ts` and `api/blocks.ts` do. +- **The two names are not interchangeable.** `manage:pages` does not imply `write:pages`: a rule + grants the exact strings in its `roles`. +- **On the frontend**, `userStore.permissions` is the global list (from `users/whoami`) and + `userStore.pagePermissions` is what the session holds AT THE CURRENT PATH (from + `pages/userPermissions`, refreshed per route in `App.vue`). `userStore.can()` ORs the two and + 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. +- **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). +- **Never invent a permission name.** Both lists above are closed; `can('browse:fileman')` and + friends matched nothing and silently hid the controls they guarded. + ### Backend patterns - **The `WIKI` global.** Set up in `index.ts`, typed in `types/global.d.ts`, available everywhere @@ -288,9 +331,10 @@ These apply to **every workspace**, `frontend/` included — not just the backen - **Routes** are Fastify plugins: `async function routes(app) { ... }` with a default export. - **Permissions** are declared per-route in `config.permissions`, and enforced by a single `preHandler` hook in `index.ts`. The array is OR-ed; a nested array is AND-ed - (`permissions: ['read:sites', ['manage:pages', 'write:pages']]`). `manage:system` bypasses every + (`permissions: ['read:sites', ['manage:users', 'manage:groups']]`). `manage:system` bypasses every check. `@fastify/swagger`'s `transform` folds these into the OpenAPI description automatically — - so declaring them is also how they get documented. + so declaring them is also how they get documented. Only **global** permissions belong here; see + [Permissions](#permissions) for the other kind and how they are checked. - **Every route needs a `schema`** with `summary`, `tags`, and response schemas. `hideUntagged` is on, so an untagged route is invisible in the API docs. Reuse `$ref` schemas from `api/schemas/`. - **Errors** via `@fastify/sensible` helpers (`reply.notFound()`, `reply.badRequest()`, diff --git a/backend/api/approvals.ts b/backend/api/approvals.ts index 45740f33a..5160e15f3 100644 --- a/backend/api/approvals.ts +++ b/backend/api/approvals.ts @@ -46,8 +46,17 @@ async function loadSuggestablePage(req: FastifyRequest, siteId: string, pageId: * 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. + * + * Nobody reviews anything without an account. A guest is treated as a member of the guests group, + * which is right for SUBMITTING — anonymous suggestions are a feature — but a review is an act with + * an author: accepting one writes the page and records who accepted it. So a rule that named the + * guests group among its reviewers, or a page rule granting them `review:pages`, would otherwise hand + * the queue to the public. An empty scope reviews nothing, whatever the rules say. */ function reviewerFor(req: FastifyRequest, page?: { path: string; tags?: string[] }): ReviewerScope { + if (!isReviewerSession(req)) { + return { groupIds: [], reviewsAll: false } + } const actor = WIKI.models.groups.actorForRequest(req) return { groupIds: WIKI.models.approvals.getActorGroupIds(req), @@ -57,6 +66,11 @@ function reviewerFor(req: FastifyRequest, page?: { path: string; tags?: string[] } } +/** Shorthand for the model's own check; see `isReviewerSession` there for why reviewing needs one. */ +function isReviewerSession(req: FastifyRequest): boolean { + return WIKI.models.approvals.isReviewerSession(req) +} + /** * Everything a rule has to satisfy beyond what the JSON Schema already enforces. * @@ -619,6 +633,14 @@ async function routes(app: FastifyInstance) { }, async (req, reply) => { reply.preventCache() + /* + Answered before the page is even looked up. Every reader loading any page asks this, so the + one case that can be settled from the session alone is settled there: a guest reviews nothing, + and the wiki has no reason to read a page and a rule set to say so again on every page view. + */ + if (!isReviewerSession(req)) { + return { canReview: false, submissions: [] } + } const page = await loadSuggestablePage(req, req.params.siteId, req.params.pageId) if (!page) { return reply.notFound('This page does not exist.') diff --git a/backend/api/blocks.ts b/backend/api/blocks.ts index 54179e5bb..8455be953 100644 --- a/backend/api/blocks.ts +++ b/backend/api/blocks.ts @@ -1,4 +1,57 @@ -import type { FastifyInstance } from 'fastify' +import type { FastifyInstance, FastifyRequest } from 'fastify' + +/** + * Group-wide permissions that carry the block list on their own. + * + * Only the ones a group really is granted as a blanket. Writing a page is NOT among them, however much + * it sounds like it belongs: page permissions come from a group's rules, and are read below. + */ +const LIST_PERMISSIONS = ['read:sites', 'manage:sites', 'manage:system'] + +/** The page rules that make somebody an author, i.e. able to put a block into a page directly. */ +const AUTHOR_ROLES = ['write:pages', 'manage:pages'] + +/** + * Whether this caller has any business seeing which blocks a site has. + * + * The list is what the editor's block picker is built from, so it belongs to whoever may put a block + * into a page. Three ways of being that person: + * + * - an administrator, from the group-wide list above; + * - an author, from a page rule that lets them write somewhere on this site; + * - anyone an enabled approval rule lets SUGGEST an edit — the guests group included, when a wiki + * has opened suggestions to the public. A suggestion is written in the same editor, with the same + * picker in it, and refusing the list there leaves the button throwing an error at a reader who + * was invited to use it. + * + * Asked of the site rather than of a page, because that is what the answer is about: which blocks + * exist here. Nothing in the reply is page-specific, so a rule anywhere on the site settles it — what + * may be written WHERE is decided by the page and suggestion routes, as it is for everything else. + * + * The route-level permission hook cannot answer any of this: it reads the group-wide list alone, and + * both writing a page and suggesting an edit are granted by rules instead. + */ +async function mayListBlocks(req: FastifyRequest, siteId: string): Promise { + const actor = WIKI.models.groups.actorForRequest(req) + if (LIST_PERMISSIONS.some((permission) => actor.permissions.includes(permission))) { + return true + } + // -> Both of these read cached group rules; only the last resort goes to the database + if ( + WIKI.models.groups + .rulesForGroups(actor.groupIds) + .some( + (rule) => rule.mode !== 'DENY' && AUTHOR_ROLES.some((role) => rule.roles?.includes(role)) + ) + ) { + return true + } + const groupIds = WIKI.models.approvals.getActorGroupIds(req) + const rules = await WIKI.models.approvals.getRules(siteId) + return rules.some( + (rule) => rule.isEnabled && rule.submitterGroups.some((id) => groupIds.includes(id)) + ) +} /** * Blocks API Routes @@ -10,13 +63,15 @@ async function routes(app: FastifyInstance) { app.get<{ Params: { siteId: string } }>( '/sites/:siteId/blocks', { - config: { - permissions: ['read:sites', 'manage:sites'] - }, + /* + No route-level `permissions`: who may see this list comes down to a group's rules, which that + hook does not read — and it would refuse an anonymous reader outright, when a wiki that takes + public suggestions has invited exactly that reader to use the picker. See `mayListBlocks`. + */ schema: { summary: 'List the blocks available to a site', description: - 'Built-in blocks are registered from the compiled block manifest, so the list reflects what is actually installed.', + 'Built-in blocks are registered from the compiled block manifest, so the list reflects what is actually installed. This is what the editor builds its block picker from, so it is available to page authors and to anyone an approval rule lets suggest an edit — guests included, where a site takes public suggestions — as well as to site administrators.', tags: ['Blocks'], params: { type: 'object', @@ -42,6 +97,9 @@ async function routes(app: FastifyInstance) { if (!site) { return reply.notFound('Site does not exist.') } + if (!(await mayListBlocks(req, req.params.siteId))) { + return reply.forbidden('You are not allowed to list the blocks of this site.') + } return WIKI.models.blocks.getSiteBlocks(req.params.siteId) } ) diff --git a/backend/api/bootstrap.ts b/backend/api/bootstrap.ts new file mode 100644 index 000000000..5fe2a3616 --- /dev/null +++ b/backend/api/bootstrap.ts @@ -0,0 +1,78 @@ +import { whoAmI } from './users.ts' +import type { FastifyInstance } from 'fastify' + +/** + * Bootstrap API Route + * + * The three things the app has to know before it can draw anything: which site it is on, which system + * flags are set, and who is asking. Each has an endpoint of its own — the admin area reads the flags, + * the login flow asks who is logged in once that has changed — but a full load needs all three at + * once, and asking for them one at a time is three round trips before the first pixel. + * + * None of them touches the database: the site configurations and the flags are in memory, and the + * session carries the user. So what this saves is the round trips, which is the whole cost. + */ +async function routes(app: FastifyInstance) { + app.get<{ Querystring: { hostname?: string } }>( + '/', + { + config: { + publicAccess: true + }, + schema: { + summary: 'Everything the app needs to start', + description: + 'The site for the hostname, the system flags, and the current session — the same answers `sites/{hostname}`, `system/flags` and `users/whoami` give, in one request.\n\nCarries the session, so it is never cached.', + tags: ['System'], + querystring: { + type: 'object', + properties: { + hostname: { + type: 'string', + maxLength: 255, + description: "The host the browser is on. The request's own hostname when absent." + } + } + }, + response: { + 200: { + description: 'Site, flags and session', + type: 'object', + properties: { + site: { $ref: 'Site#' }, + flags: { $ref: 'SystemFlags#' }, + user: { + type: 'object', + description: + 'As `users/whoami` answers it: `authenticated: false` alone for a guest, otherwise the account and its group-wide permissions.', + additionalProperties: true + } + } + } + } + } + }, + async (req, reply) => { + // -> The session decides part of the answer, so no shared cache may hold on to it + reply.preventCache() + const site = await WIKI.models.sites.getSiteByHostname({ + hostname: req.query.hostname ?? req.hostname + }) + if (!site) { + return reply.notFound('There is no wiki site at this hostname.') + } + return { + site: { + ...site.config, + id: site.id, + hostname: site.hostname, + isEnabled: site.isEnabled + }, + flags: WIKI.models.flags.getFlags(), + user: whoAmI(req) + } + } + ) +} + +export default routes diff --git a/backend/api/index.ts b/backend/api/index.ts index 59f4739de..a57749ebf 100644 --- a/backend/api/index.ts +++ b/backend/api/index.ts @@ -30,6 +30,7 @@ async function routes(app: FastifyInstance) { app.register(import('./assets.ts')) app.register(import('./authentication.ts')) app.register(import('./blocks.ts')) + app.register(import('./bootstrap.ts'), { prefix: '/bootstrap' }) app.register(import('./groups.ts'), { prefix: '/groups' }) app.register(import('./hooks.ts'), { prefix: '/hooks' }) app.register(import('./icons.ts'), { prefix: '/icons' }) diff --git a/backend/api/pages.ts b/backend/api/pages.ts index 47ea6cb14..b4899e7af 100644 --- a/backend/api/pages.ts +++ b/backend/api/pages.ts @@ -120,6 +120,35 @@ export function mayOnPage( return WIKI.models.groups.checkAccess(WIKI.models.groups.actorForRequest(req), permission, page) } +/** + * Every page permission this requester holds at a path. + * + * What the interface hides its controls by, and the reason it is a list rather than a question: each + * permission may be decided by a different rule — a branch can be readable but not writable, and one + * page within it neither — so they are resolved one at a time. + * + * 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. + */ +export function pagePermissionsFor( + req: FastifyRequest, + page: { path: string; locale?: string; tags?: string[] } +): string[] { + const actor = 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 (actor.permissions.includes('manage:system')) { + return PAGE_PERMISSIONS + } + return PAGE_PERMISSIONS.filter((permission) => + WIKI.models.groups.checkAccess(actor, permission, page) + ) +} + /** * A page, as this requester is allowed to see it — or null when they are not allowed to see it at all. * @@ -449,7 +478,26 @@ async function routes(app: FastifyInstance) { if (!mayOnPage(req, 'read:pages', page)) { return reply.forbidden('You are not allowed to read this page.') } - return page + /* + The reader's own standing on this page, carried back with it. + + Three questions the page view used to ask as three more requests — what may I do here, may I + suggest an edit, do I review this page — each of which had to load the page again to answer. + They are answered here from the page already in hand, against rules already in memory, which + is what makes a page view one request instead of four. + */ + return { + ...page, + viewer: { + permissions: pagePermissionsFor(req, page), + ...(await WIKI.models.approvals.pageViewerState(req, req.params.siteId, { + id: page.id, + path: page.path, + tags: page.tags ?? [], + allowContributions: page.allowContributions + })) + } + } } ) @@ -1034,26 +1082,7 @@ async function routes(app: FastifyInstance) { } }, async (req) => { - /* - 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. - */ - 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 - } - // -> 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) - ) + return pagePermissionsFor(req, { path: req.body.path.replace(/^\/+/, '') }) } ) } diff --git a/backend/api/schemas/page.ts b/backend/api/schemas/page.ts index ce3d9c38a..51ee14f51 100644 --- a/backend/api/schemas/page.ts +++ b/backend/api/schemas/page.ts @@ -202,7 +202,39 @@ export async function registerSchemas(app: FastifyInstance): Promise { authorId: { type: 'string', format: 'uuid' }, authorName: { type: 'string' }, createdAt: { type: 'string', format: 'date-time' }, - updatedAt: { type: 'string', format: 'date-time' } + updatedAt: { type: 'string', format: 'date-time' }, + viewer: { + type: 'object', + description: + 'Where the requester stands on this page: what they may do to it, whether they may suggest an edit, and whether they review it. Present when a page is fetched on its own — the page view draws its controls from this, rather than asking three further endpoints about a page it already has. Absent from a page returned by a save.', + properties: { + permissions: { + type: 'array', + items: { type: 'string' }, + description: + 'The page permissions held AT THIS PATH, as this reader’s groups’ rules decide. The same answer `pages/userPermissions` gives for the path.' + }, + canSuggestEdits: { + type: 'boolean', + description: + 'An enabled approval rule covers this page and names a group the requester is in, and the page allows contributions.' + }, + hasOpenSuggestion: { + type: 'boolean', + description: + 'The requester already has a suggestion waiting on this page, which they would carry on with rather than start again. Always false for a guest, whose suggestions are attributed to nobody.' + }, + canReview: { + type: 'boolean', + description: 'The requester reviews this page. Always false without an account.' + }, + pendingSubmissions: { + type: 'array', + items: { $ref: 'PageEditSubmission#' }, + description: 'What is waiting on this page, oldest first. Empty unless `canReview`.' + } + } + } } }) diff --git a/backend/api/users.ts b/backend/api/users.ts index f9a78c3bf..5fbd42e39 100644 --- a/backend/api/users.ts +++ b/backend/api/users.ts @@ -27,6 +27,30 @@ function sessionUserId(req: FastifyRequest): string | null { return req.session?.authenticated && req.session.user?.id ? req.session.user.id : null } +/** + * Who is asking, as the interface needs to know it: the account on the session and the group-wide + * permissions it holds, or nothing at all for a guest. + * + * Exported because `bootstrap` answers the same question as part of the one call an app load makes, + * and two versions of "who is this" would be one too many. + */ +export function whoAmI(req: FastifyRequest): Record { + if (!req.session?.authenticated) { + return { authenticated: false } + } + return { + authenticated: true, + ...req.session.user, + /* + The same list the route permission hook checks against — written onto the session at login from + the groups the user belongs to. Nothing is added for the interface's benefit: a control it shows + on a permission the session does not hold leads to a button that gets a 403 from the endpoint + behind it. + */ + permissions: req.session.permissions ?? [] + } +} + /** * Whether self-service profile editing is enabled on the site being browsed. * @@ -120,22 +144,14 @@ async function routes(app: FastifyInstance) { { schema: { summary: 'Get currently logged in user info', + description: + 'Includes the group-wide permissions of the session, which is what the interface hides its own controls by. Permissions ON A PAGE are a different question, answered by `pages/userPermissions`.\n\nThe app itself gets this from `bootstrap` on load, together with the site and the flags; this endpoint is what asks again once a login or a logout has changed the answer.', tags: ['Users'] } }, async (req, reply) => { reply.preventCache() - if (req.session?.authenticated) { - return { - authenticated: true, - ...req.session.user, - permissions: ['manage:system'] // TODO: pull actual permissions - } - } else { - return { - authenticated: false - } - } + return whoAmI(req) } ) diff --git a/backend/index.ts b/backend/index.ts index a99f601ce..f9454165e 100644 --- a/backend/index.ts +++ b/backend/index.ts @@ -142,6 +142,8 @@ async function postBoot() { 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() + // -> Likewise: every page view asks whether the page takes suggestions and who reviews it + await WIKI.models.approvals.reloadCache() // -> Must follow the sites cache: every site gets a row per installed block await WIKI.models.blocks.refreshFromDisk() diff --git a/backend/models/approvals.ts b/backend/models/approvals.ts index 28a4214e6..6099a81e3 100644 --- a/backend/models/approvals.ts +++ b/backend/models/approvals.ts @@ -136,6 +136,19 @@ const ruleSelection = { updatedAt: approvalRulesTable.updatedAt } +/** + * Every site's rules, by site id, in the order `getRules` promises. + * + * Cached for the reason the group rules are (`models/groups.ts`): whether a page takes suggestions + * and who reviews it are questions the page view asks about every page it draws, and answering them + * from the database would put two queries in front of every page read. Rules change from one admin + * screen, and the cache is reloaded there. + * + * A single instance's memory, like the group and site caches beside it: a rule changed on one node of + * a cluster reaches the others when they next reload. + */ +let rulesCache: Record = {} + /** * Approvals model * @@ -143,22 +156,41 @@ const ruleSelection = { * submissions themselves are a separate concern and are not stored yet. */ class Approvals { + /** + * Reload every site's rules into memory. + * + * Called at boot and after any change to a rule, so that an administrator's edit takes effect on the + * next request — the same contract `models/groups.ts` gives page rules. + */ + async reloadCache(): Promise { + const rows = (await WIKI.db + .select({ ...ruleSelection, siteId: approvalRulesTable.siteId }) + .from(approvalRulesTable) + .orderBy( + asc(sql`lower(${approvalRulesTable.name})`), + asc(approvalRulesTable.createdAt) + )) as (ApprovalRule & { siteId: string })[] + rulesCache = {} + for (const { siteId, ...rule } of rows) { + rulesCache[siteId] ??= [] + rulesCache[siteId].push(rule as ApprovalRule) + } + WIKI.logger.info(`Loaded ${rows.length} approval rules [ OK ]`) + } + /** * Every rule configured for a site, by name. * * Order carries no meaning — a page is covered if any enabled rule matches it — so the list is * sorted for the reader: alphabetically, ignoring case, since `Zoo` sorting before `apple` is not * what alphabetical means to anyone. Two rules sharing a name keep a stable order by age. + * + * From `rulesCache`, so this costs nothing to ask; async because every caller awaits it and because + * where the rules come from is this model's business. The array is the cached one — read it, do not + * sort or splice it. */ async getRules(siteId: string): Promise { - return WIKI.db - .select(ruleSelection) - .from(approvalRulesTable) - .where(eq(approvalRulesTable.siteId, siteId)) - .orderBy( - asc(sql`lower(${approvalRulesTable.name})`), - asc(approvalRulesTable.createdAt) - ) as Promise + return rulesCache[siteId] ?? [] } /** @@ -215,6 +247,8 @@ class Approvals { reviewerGroups: patch.reviewerGroups ?? [] }) .returning(ruleSelection) + // -> Every rule read afterwards comes from the cache, so it has to know about this one + await this.reloadCache() return rows[0] as ApprovalRule } @@ -248,6 +282,7 @@ class Approvals { .set(values) .where(and(eq(approvalRulesTable.siteId, siteId), eq(approvalRulesTable.id, id))) .returning(ruleSelection) + await this.reloadCache() return (rows[0] as ApprovalRule) ?? null } @@ -357,6 +392,77 @@ class Approvals { ) } + /** + * Whether this request could review anything at all, i.e. it is a logged in user. + * + * Reads the session and nothing else, so a guest can be turned away before a single query is made on + * their behalf. A guest counts as a member of the guests group everywhere else, which is right for + * SUBMITTING — anonymous suggestions are a feature — but a review is an act with an author. + */ + isReviewerSession(req: any): boolean { + return Boolean(req.session?.authenticated && req.session.user?.id) + } + + /** + * Where this reader stands on this page: may they suggest an edit to it, and do they review it. + * + * Answered here, in one place, because it is answered on EVERY page view — the page route carries it + * back with the page rather than leaving the browser to ask two more questions about a page it has + * just been given. The cost is kept to what is actually needed: the rules are in memory, and neither + * of the two queries below is reached by a reader the rules say nothing about. + * + * @param req The request, for its session; both answers are about who is asking + */ + async pageViewerState( + req: any, + siteId: string, + page: ApprovalPageRef + ): Promise<{ + canSuggestEdits: boolean + hasOpenSuggestion: boolean + canReview: boolean + pendingSubmissions: ReviewableSubmission[] + }> { + const actorId = req.session?.authenticated ? (req.session.user?.id ?? null) : null + const groupIds = this.getActorGroupIds(req) + + const submitRule = await this.findSubmitRule(siteId, page, groupIds) + /* + Only a logged in author can have one waiting: a guest suggestion is attributed to nobody, so + there is nothing to look up and nothing to carry on from. `getOwnSubmission` says the same, and + this keeps the query from being made at all. + */ + const hasOpenSuggestion = Boolean( + submitRule && actorId && (await this.getOwnSubmission(page.id, actorId)) + ) + + const reviewerScope = this.isReviewerSession(req) + ? { + groupIds, + reviewsAll: + (req.session?.permissions ?? []).includes('manage:system') || + WIKI.models.groups.checkAccess( + WIKI.models.groups.actorForRequest(req), + 'review:pages', + { + path: page.path, + tags: page.tags + } + ) + } + : { groupIds: [], reviewsAll: false } + const canReview = await this.canReviewPage(siteId, page, reviewerScope) + + return { + canSuggestEdits: Boolean(submitRule), + hasOpenSuggestion, + canReview, + pendingSubmissions: canReview + ? await this.getReviewableSubmissions(siteId, { ...reviewerScope, pageId: page.id }) + : [] + } + } + /** * The suggestion this user already has open on this page, if any. * @@ -685,6 +791,7 @@ class Approvals { const result = await WIKI.db .delete(approvalRulesTable) .where(and(eq(approvalRulesTable.siteId, siteId), eq(approvalRulesTable.id, id))) + await this.reloadCache() return (result.rowCount ?? 0) > 0 } } diff --git a/frontend/src/App.vue b/frontend/src/App.vue index 5630228e6..fe8a97671 100644 --- a/frontend/src/App.vue +++ b/frontend/src/App.vue @@ -184,6 +184,31 @@ if (typeof siteConfig !== 'undefined') { applyTheme() } +/** + * Everything the app has to know before it can draw: which site it is on, which system flags are set, + * and who is asking. + * + * The three have endpoints of their own, and are still asked separately where they change on their + * own — the admin area saves flags, a login changes who is asking. This is the load, where all three + * are wanted at once and none of them is known yet. + * + * A failure leaves the stores at their defaults and says so in the console, which is what the three + * calls did before: there is no interface yet to put an error in front of. + */ +async function loadBootstrap() { + try { + const data = await API_CLIENT.get('bootstrap', { + searchParams: { hostname: window.location.hostname }, + cache: 'no-store' + }).json() + siteStore.applySiteInfo(data.site) + flagsStore.apply(data.flags) + userStore.applyProfile(data.user) + } catch (err) { + console.warn(`Could not load the site configuration: ${err.message}`) + } +} + // ROUTE GUARDS router.beforeEach(async (to, from) => { @@ -194,16 +219,14 @@ router.beforeEach(async (to, from) => { // userStore.loadToken() // } - // -> System Flags - if (!flagsStore.loaded) { - flagsStore.load() - } - - // -> Site Info - if (!siteStore.id) { - console.info('No pre-cached site config. Loading site info...') - await siteStore.loadSite(window.location.hostname) - console.info(`Using Site ID ${siteStore.id}`) + /* + -> Site info, system flags and the session + One request for the three of them: none touches the database, so what they cost is the round trip, + and a full load paid it three times over before it could draw anything. Asked once — a guest is an + answer like any other, so this does not run again on the way to the next page. + */ + if (!siteStore.id || !flagsStore.loaded || !userStore.profileLoaded) { + await loadBootstrap() } // -> Locale @@ -215,14 +238,16 @@ router.beforeEach(async (to, from) => { } applyLocale(commonStore.desiredLocale) - // -> User Profile - if (!userStore.profileLoaded) { - console.info(`Refreshing user profile...`) - await userStore.refreshProfile() + /* + -> Page Permissions + Not fetched here any more: what this reader may do at a path comes back with the page itself, so + a page view is one request rather than two. What is left is the routes that are not a page — + dropping the last page's permissions on the way out of the page view, which takes no request at + all. A path with no page behind it has nothing to carry them, and asks in `pages/Index.vue`. + */ + if (to.path.startsWith('/_')) { + userStore.$patch({ pagePermissions: [] }) } - - // -> Page Permissions - await userStore.fetchPagePermissions(to.path) }) // GLOBAL EVENTS HANDLERS diff --git a/frontend/src/components/FileManager.vue b/frontend/src/components/FileManager.vue index c841fb889..3bb98ceef 100644 --- a/frontend/src/components/FileManager.vue +++ b/frontend/src/components/FileManager.vue @@ -266,7 +266,7 @@
- + Fetching folder contents...
diff --git a/frontend/src/components/HeaderNav.vue b/frontend/src/components/HeaderNav.vue index 4fede5759..ad5181f4c 100644 --- a/frontend/src/components/HeaderNav.vue +++ b/frontend/src/components/HeaderNav.vue @@ -30,8 +30,14 @@ Create New Page + -