From ed9ac9ac5dbdf689fdcf52ce12cf9bcc54dc984a Mon Sep 17 00:00:00 2001 From: lucaci32u4 Date: Sat, 16 May 2026 03:09:39 +0300 Subject: [PATCH] Fix page rule evaluation Squashed commit of the following, omitting all environment-related setups: commit e0c04f8d47db8b7589cef9acfc5633835624ea24 Author: lucaci32u4 Date: Sat May 16 02:36:04 2026 +0300 Fix forking release again commit 4b21b8229c58bc59263f074b72326f297dd6e115 Author: lucaci32u4 Date: Sat May 16 02:19:46 2026 +0300 Fix forking release commit cac209b0afe897a83d3c8f8094d5085d28d27335 Author: lucaci32u4 Date: Sat May 16 01:57:19 2026 +0300 Remove windows build commit b3e6b37d077ec1ca0895a6f3e8ae278049092035 Author: lucaci32u4 Date: Sat May 16 01:42:21 2026 +0300 Fix page tree responding to all permissions rules commit 7606393de6f074fedd1b36acb7f085307304e8e1 Author: lucaci32u4 Date: Tue May 12 05:12:42 2026 +0300 Fix build process and improve permission handling commit 1b6dbed46b6eaa9f34e22b1b222fd3bd7a021575 Author: lucaci32u4 Date: Tue May 12 05:00:51 2026 +0300 Add explanation of fork reason to README commit a7a7460f8b7a7053e2e8145ea2852989a3ead99f Merge: 3d63b55d 67d69fe9 Author: lucaci32u4 Date: Tue May 12 04:54:13 2026 +0300 Merge branch 'feat-permissions' commit 67d69fe90453c2d3998bf4d204eed3a4fe801157 Author: lucaci32u4 Date: Tue May 12 04:53:39 2026 +0300 Update inline hint about permission evaluation commit 64d20cb58c713df517d4f92b79cbceb900ae90e9 Author: lucaci32u4 Date: Tue May 12 04:04:40 2026 +0300 Rework permissions path matching to better support tags commit 3d63b55d257309b97ef2ba787c5d62980a065b33 Author: lucaci32u4 Date: Tue May 12 04:03:17 2026 +0300 Publish to form repo --- .../admin/admin-groups-edit-rules.vue | 17 +++-- server/core/auth.js | 50 +++++++++------ server/graph/resolvers/page.js | 64 ++++++++++++++++++- 3 files changed, 99 insertions(+), 32 deletions(-) diff --git a/client/components/admin/admin-groups-edit-rules.vue b/client/components/admin/admin-groups-edit-rules.vue index f52d9265b4..056b6e7a79 100644 --- a/client/components/admin/admin-groups-edit-rules.vue +++ b/client/components/admin/admin-groups-edit-rules.vue @@ -171,23 +171,22 @@ v-divider.mt-3 .overline.py-3 Rules Order - .body-2.pl-3 Rules are applied in order of path specificity. A more precise path will always override a less defined path. - .body-2.pl-5 For example, #[span.teal--text /geography/countries] will override #[span.teal--text /geography]. - .body-2.pl-3.pt-2 When 2 rules have the same specificity, the priority is given from lowest to highest as follows: + .body-2.pl-3 Rules are applied in order of path specificity. A more precise rule will always override a less defined rule. + .body-2.pl-3 The specificity is evaluated as such, in order: .body-2.pl-3.pt-1 ul li - strong Path Starts With... - em.caption.pl-1 (lowest) + strong Path is Exactly... + em.caption.pl-1 (highest) li - strong Path Ends With... + strong Tag Matches... li strong Path Matches Regex... li - strong Tag Matches... + span #[strong Path Starts With...] and #[strong Path Ends With...] + em.caption.pl-1 (lowest, variable based on path) li - strong Path Is Exactly... - em.caption.pl-1 (highest) + span For example, #[span.teal--text /geo/areas/north] will override #[span.teal--text /geography/countries] which will override #[span.teal--text /geography]. .body-2.pl-3.pt-2 When 2 rules have the same path specificity AND the same match type, #[strong.red--text DENY] will always override an #[strong.green--text ALLOW] rule. v-divider.mt-3 .overline.py-3 Regular Expressions diff --git a/server/core/auth.js b/server/core/auth.js index ec0dd720b7..47c411cbe2 100644 --- a/server/core/auth.js +++ b/server/core/auth.js @@ -241,7 +241,7 @@ module.exports = { let checkState = { deny: false, match: false, - specificity: '' + specificity: 0 } user.groups.forEach(grp => { const grpId = _.isObject(grp) ? _.get(grp, 'id', 0) : grp @@ -253,34 +253,30 @@ module.exports = { switch (rule.match) { case 'START': if (_.startsWith(`/${page.path}`, `/${rule.path}`)) { - checkState = this._applyPageRuleSpecificity({ rule, checkState, higherPriority: ['END', 'REGEX', 'EXACT', 'TAG'] }) + checkState = this._applyPageRuleSpecificity({ rule, checkState }) } break case 'END': if (_.endsWith(page.path, rule.path)) { - checkState = this._applyPageRuleSpecificity({ rule, checkState, higherPriority: ['REGEX', 'EXACT', 'TAG'] }) + checkState = this._applyPageRuleSpecificity({ rule, checkState }) } break case 'REGEX': const reg = new RegExp(rule.path) if (reg.test(page.path)) { - checkState = this._applyPageRuleSpecificity({ rule, checkState, higherPriority: ['EXACT', 'TAG'] }) + checkState = this._applyPageRuleSpecificity({ rule, checkState }) } break case 'TAG': _.get(page, 'tags', []).forEach(tag => { if (tag.tag === rule.path) { - checkState = this._applyPageRuleSpecificity({ - rule, - checkState, - higherPriority: ['EXACT'] - }) + checkState = this._applyPageRuleSpecificity({ rule, checkState }) } }) break case 'EXACT': if (`/${page.path}` === `/${rule.path}`) { - checkState = this._applyPageRuleSpecificity({ rule, checkState, higherPriority: [] }) + checkState = this._applyPageRuleSpecificity({ rule, checkState }) } break } @@ -365,25 +361,39 @@ module.exports = { * * @access private */ - _applyPageRuleSpecificity ({ rule, checkState, higherPriority = [] }) { - if (rule.path.length === checkState.specificity.length) { - // Do not override higher priority rules - if (_.includes(higherPriority, checkState.match)) { - return checkState + _getPathSpecificity (matchType, rulePath) { + switch (matchType) { + case 'EXACT': + return Number.MAX_SAFE_INTEGER + case 'TAG': + return Number.MAX_SAFE_INTEGER - 1 + case 'REGEX': + return Number.MAX_SAFE_INTEGER - 2 + default: { + const segments = rulePath.split('/').filter(s => s.length > 0).length + const separators = (rulePath.match(/\//g) || []).length + const base = segments + separators + return matchType === 'END' ? base + 0.5 : base } - // Do not override a previous DENY rule with same match - if (rule.match === checkState.match && checkState.deny && !rule.deny) { + } + }, + + _applyPageRuleSpecificity ({ rule, checkState }) { + const ruleSpecificity = this._getPathSpecificity(rule.match, rule.path) + + if (ruleSpecificity === checkState.specificity) { + // Do not override a previous DENY rule with same specificity + if (checkState.deny && !rule.deny) { return checkState } - } else if (rule.path.length < checkState.specificity.length) { - // Do not override higher specificity rules + } else if (ruleSpecificity < checkState.specificity) { return checkState } return { deny: rule.deny, match: rule.match, - specificity: rule.path + specificity: ruleSpecificity } }, diff --git a/server/graph/resolvers/page.js b/server/graph/resolvers/page.js index 9e99686ad6..aa25ee7fc9 100644 --- a/server/graph/resolvers/page.js +++ b/server/graph/resolvers/page.js @@ -282,12 +282,70 @@ module.exports = { } } }).orderBy([{ column: 'isFolder', order: 'desc' }, 'title']) - return results.filter(r => { + + const pageIds = results.filter(r => r.pageId).map(r => r.pageId) + let tagsByPageId = {} + if (pageIds.length > 0) { + const tagRows = await WIKI.models.knex('pageTags') + .join('tags', 'pageTags.tagId', 'tags.id') + .whereIn('pageTags.pageId', pageIds) + .select('pageTags.pageId', 'tags.tag') + tagRows.forEach(row => { + if (!tagsByPageId[row.pageId]) { tagsByPageId[row.pageId] = [] } + tagsByPageId[row.pageId].push({ tag: row.tag }) + }) + } + + const visibleItems = results.filter(r => { return WIKI.auth.checkAccess(context.req.user, ['read:pages'], { path: r.path, - locale: r.localeCode + locale: r.localeCode, + tags: r.pageId ? (tagsByPageId[r.pageId] || []) : [] }) - }).map(r => ({ + }) + + const visibleIds = new Set(visibleItems.map(r => r.id)) + const deniedFolders = results.filter(r => r.isFolder && !visibleIds.has(r.id)) + + if (deniedFolders.length > 0) { + const descendantPages = await WIKI.models.knex('pages') + .select('id', 'path', 'localeCode') + .where('localeCode', args.locale) + .where(builder => { + deniedFolders.forEach((folder, i) => { + if (i === 0) { builder.where('path', 'like', `${folder.path}/%`) } + else { builder.orWhere('path', 'like', `${folder.path}/%`) } + }) + }) + + const descPageIds = descendantPages.map(p => p.id) + const descTagsByPageId = {} + if (descPageIds.length > 0) { + const descTagRows = await WIKI.models.knex('pageTags') + .join('tags', 'pageTags.tagId', 'tags.id') + .whereIn('pageTags.pageId', descPageIds) + .select('pageTags.pageId', 'tags.tag') + descTagRows.forEach(row => { + if (!descTagsByPageId[row.pageId]) { descTagsByPageId[row.pageId] = [] } + descTagsByPageId[row.pageId].push({ tag: row.tag }) + }) + } + + for (const folder of deniedFolders) { + const hasVisibleDescendant = descendantPages.some(p => { + return p.path.startsWith(folder.path + '/') && WIKI.auth.checkAccess(context.req.user, ['read:pages'], { + path: p.path, + locale: p.localeCode, + tags: descTagsByPageId[p.id] || [] + }) + }) + if (hasVisibleDescendant) { + visibleItems.push(folder) + } + } + } + + return visibleItems.map(r => ({ ...r, parent: r.parent || 0, locale: r.localeCode