Fix page tree to respect TAG and REGEX permission rules, improve specificity ranking

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

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
     }
   },
 

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