diff --git a/client/components/admin/admin-groups-edit-rules.vue b/client/components/admin/admin-groups-edit-rules.vue index f52d9265b..056b6e7a7 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 ec0dd720b..47c411cbe 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 9e99686ad..aa25ee7fc 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