Merge ed9ac9ac5d into 6f042e97cc

2 weeks ago · 3dfbe53c12
parent 6f042e97cc ed9ac9ac5d
commit 3dfbe53c12
3 changed files with 99 additions and 32 deletions
--- 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
--- 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
    }
  },

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