From 767b2688b606e5575102b68d02cfba0bdb94a58a Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 10 Mar 2022 16:33:56 -0500 Subject: [PATCH] refactor: privileges system to use a Map in the backend instead of separate objects for keys and labels (#10378) * refactor: privileges system to use a Map in the backend instead of separate objects for keys and labels - Existing hooks are preserved (to be deprecated at a later date, possibly) - New init hooks are called on NodeBB start, and provide a one-stop shop to add new privileges, instead of having to add to four different hooks * docs: fix typo in comment * test: spec changes --- .../read/admin/manage/privileges/cid.yaml | 12 +-- .../write/categories/cid/privileges.yaml | 14 +-- .../categories/cid/privileges/privilege.yaml | 28 ++---- src/privileges/admin.js | 66 +++++++------- src/privileges/categories.js | 89 ++++++++----------- src/privileges/global.js | 76 +++++++--------- src/privileges/index.js | 6 ++ .../admin/partials/privileges/category.tpl | 12 +-- .../admin/partials/privileges/global.tpl | 12 +-- src/webserver.js | 2 + test/posts.js | 3 +- 11 files changed, 141 insertions(+), 179 deletions(-) diff --git a/public/openapi/read/admin/manage/privileges/cid.yaml b/public/openapi/read/admin/manage/privileges/cid.yaml index 2625f3abef..2ef7c8d891 100644 --- a/public/openapi/read/admin/manage/privileges/cid.yaml +++ b/public/openapi/read/admin/manage/privileges/cid.yaml @@ -27,17 +27,13 @@ get: users: type: array items: - type: object - properties: - name: - type: string + type: string + description: Language key of the privilege name's user-friendly name groups: type: array items: - type: object - properties: - name: - type: string + type: string + description: Language key of the privilege name's user-friendly name keys: type: object properties: diff --git a/public/openapi/write/categories/cid/privileges.yaml b/public/openapi/write/categories/cid/privileges.yaml index 9cf239cbac..209b0ad051 100644 --- a/public/openapi/write/categories/cid/privileges.yaml +++ b/public/openapi/write/categories/cid/privileges.yaml @@ -30,19 +30,13 @@ get: users: type: array items: - type: object - properties: - name: - type: string - description: Language key of the privilege name's user-friendly name + type: string + description: Language key of the privilege name's user-friendly name groups: type: array items: - type: object - properties: - name: - type: string - description: Language key of the privilege name's user-friendly name + type: string + description: Language key of the privilege name's user-friendly name users: type: array items: diff --git a/public/openapi/write/categories/cid/privileges/privilege.yaml b/public/openapi/write/categories/cid/privileges/privilege.yaml index 9b00cc8573..1303640b19 100644 --- a/public/openapi/write/categories/cid/privileges/privilege.yaml +++ b/public/openapi/write/categories/cid/privileges/privilege.yaml @@ -47,19 +47,13 @@ put: users: type: array items: - type: object - properties: - name: - type: string - description: Language key of the privilege name's user-friendly name + type: string + description: Language key of the privilege name's user-friendly name groups: type: array items: - type: object - properties: - name: - type: string - description: Language key of the privilege name's user-friendly name + type: string + description: Language key of the privilege name's user-friendly name users: type: array items: @@ -164,19 +158,13 @@ delete: users: type: array items: - type: object - properties: - name: - type: string - description: Language key of the privilege name's user-friendly name + type: string + description: Language key of the privilege name's user-friendly name groups: type: array items: - type: object - properties: - name: - type: string - description: Language key of the privilege name's user-friendly name + type: string + description: Language key of the privilege name's user-friendly name users: type: array items: diff --git a/src/privileges/admin.js b/src/privileges/admin.js index 3cf42fa7a8..9c05749caa 100644 --- a/src/privileges/admin.js +++ b/src/privileges/admin.js @@ -11,34 +11,24 @@ const utils = require('../utils'); const privsAdmin = module.exports; -privsAdmin.privilegeLabels = [ - { name: '[[admin/manage/privileges:admin-dashboard]]' }, - { name: '[[admin/manage/privileges:admin-categories]]' }, - { name: '[[admin/manage/privileges:admin-privileges]]' }, - { name: '[[admin/manage/privileges:admin-admins-mods]]' }, - { name: '[[admin/manage/privileges:admin-users]]' }, - { name: '[[admin/manage/privileges:admin-groups]]' }, - { name: '[[admin/manage/privileges:admin-tags]]' }, - { name: '[[admin/manage/privileges:admin-settings]]' }, -]; +/** + * Looking to add a new admin privilege via plugin/theme? Attach a hook to + * `static:privileges.admin.init` and call .set() on the privilege map passed + * in to your listener. + */ +const _privilegeMap = new Map([ + ['admin:dashboard', { label: '[[admin/manage/privileges:admin-dashboard]]' }], + ['admin:categories', { label: '[[admin/manage/privileges:admin-categories]]' }], + ['admin:privileges', { label: '[[admin/manage/privileges:admin-privileges]]' }], + ['admin:admins-mods', { label: '[[admin/manage/privileges:admin-admins-mods]]' }], + ['admin:users', { label: '[[admin/manage/privileges:admin-users]]' }], + ['admin:groups', { label: '[[admin/manage/privileges:admin-groups]]' }], + ['admin:tags', { label: '[[admin/manage/privileges:admin-tags]]' }], + ['admin:settings', { label: '[[admin/manage/privileges:admin-settings]]' }], +]); -privsAdmin.userPrivilegeList = [ - 'admin:dashboard', - 'admin:categories', - 'admin:privileges', - 'admin:admins-mods', - 'admin:users', - 'admin:groups', - 'admin:tags', - 'admin:settings', -]; - -privsAdmin.groupPrivilegeList = privsAdmin.userPrivilegeList.map(privilege => `groups:${privilege}`); - -privsAdmin.privilegeList = privsAdmin.userPrivilegeList.concat(privsAdmin.groupPrivilegeList); - -privsAdmin.getUserPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.admin.list', privsAdmin.userPrivilegeList.slice()); -privsAdmin.getGroupPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.admin.groups.list', privsAdmin.groupPrivilegeList.slice()); +privsAdmin.getUserPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.admin.list', Array.from(_privilegeMap.keys())); +privsAdmin.getGroupPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.admin.groups.list', Array.from(_privilegeMap.keys()).map(privilege => `groups:${privilege}`)); privsAdmin.getPrivilegeList = async () => { const [user, group] = await Promise.all([ privsAdmin.getUserPrivilegeList(), @@ -47,6 +37,12 @@ privsAdmin.getPrivilegeList = async () => { return user.concat(group); }; +privsAdmin.init = async () => { + await plugins.hooks.fire('static:privileges.admin.init', { + privileges: _privilegeMap, + }); +}; + // Mapping for a page route (via direct match or regexp) to a privilege privsAdmin.routeMap = { dashboard: 'admin:dashboard', @@ -118,13 +114,13 @@ privsAdmin.resolve = (path) => { }; privsAdmin.list = async function (uid) { - const privilegeLabels = privsAdmin.privilegeLabels.slice(); - const userPrivilegeList = privsAdmin.userPrivilegeList.slice(); - const groupPrivilegeList = privsAdmin.groupPrivilegeList.slice(); + const privilegeLabels = Array.from(_privilegeMap.values()).map(data => data.label); + const userPrivilegeList = await privsAdmin.getUserPrivilegeList(); + const groupPrivilegeList = await privsAdmin.getGroupPrivilegeList(); // Restrict privileges column to superadmins if (!(await user.isAdministrator(uid))) { - const idx = privsAdmin.userPrivilegeList.indexOf('admin:privileges'); + const idx = Array.from(_privilegeMap.keys()).indexOf('admin:privileges'); privilegeLabels.splice(idx, 1); userPrivilegeList.splice(idx, 1); groupPrivilegeList.splice(idx, 1); @@ -135,10 +131,10 @@ privsAdmin.list = async function (uid) { groups: plugins.hooks.fire('filter:privileges.admin.groups.list_human', privilegeLabels.slice()), }); - const keys = await utils.promiseParallel({ - users: plugins.hooks.fire('filter:privileges.admin.list', userPrivilegeList.slice()), - groups: plugins.hooks.fire('filter:privileges.admin.groups.list', groupPrivilegeList.slice()), - }); + const keys = { + users: userPrivilegeList, + groups: groupPrivilegeList, + }; const payload = await utils.promiseParallel({ labels, diff --git a/src/privileges/categories.js b/src/privileges/categories.js index f38291cb7b..92b0edd849 100644 --- a/src/privileges/categories.js +++ b/src/privileges/categories.js @@ -12,50 +12,32 @@ const utils = require('../utils'); const privsCategories = module.exports; -privsCategories.privilegeLabels = [ - { name: '[[admin/manage/privileges:find-category]]' }, - { name: '[[admin/manage/privileges:access-category]]' }, - { name: '[[admin/manage/privileges:access-topics]]' }, - { name: '[[admin/manage/privileges:create-topics]]' }, - { name: '[[admin/manage/privileges:reply-to-topics]]' }, - { name: '[[admin/manage/privileges:schedule-topics]]' }, - { name: '[[admin/manage/privileges:tag-topics]]' }, - { name: '[[admin/manage/privileges:edit-posts]]' }, - { name: '[[admin/manage/privileges:view-edit-history]]' }, - { name: '[[admin/manage/privileges:delete-posts]]' }, - { name: '[[admin/manage/privileges:upvote-posts]]' }, - { name: '[[admin/manage/privileges:downvote-posts]]' }, - { name: '[[admin/manage/privileges:delete-topics]]' }, - { name: '[[admin/manage/privileges:view_deleted]]' }, - { name: '[[admin/manage/privileges:purge]]' }, - { name: '[[admin/manage/privileges:moderate]]' }, -]; +/** + * Looking to add a new category privilege via plugin/theme? Attach a hook to + * `static:privileges.category.init` and call .set() on the privilege map passed + * in to your listener. + */ +const _privilegeMap = new Map([ + ['find', { label: '[[admin/manage/privileges:find-category]]' }], + ['read', { label: '[[admin/manage/privileges:access-category]]' }], + ['topics:read', { label: '[[admin/manage/privileges:access-topics]]' }], + ['topics:create', { label: '[[admin/manage/privileges:create-topics]]' }], + ['topics:reply', { label: '[[admin/manage/privileges:reply-to-topics]]' }], + ['topics:schedule', { label: '[[admin/manage/privileges:schedule-topics]]' }], + ['topics:tag', { label: '[[admin/manage/privileges:tag-topics]]' }], + ['posts:edit', { label: '[[admin/manage/privileges:edit-posts]]' }], + ['posts:history', { label: '[[admin/manage/privileges:view-edit-history]]' }], + ['posts:delete', { label: '[[admin/manage/privileges:delete-posts]]' }], + ['posts:upvote', { label: '[[admin/manage/privileges:upvote-posts]]' }], + ['posts:downvote', { label: '[[admin/manage/privileges:downvote-posts]]' }], + ['topics:delete', { label: '[[admin/manage/privileges:delete-topics]]' }], + ['posts:view_deleted', { label: '[[admin/manage/privileges:view_deleted]]' }], + ['purge', { label: '[[admin/manage/privileges:purge]]' }], + ['moderate', { label: '[[admin/manage/privileges:moderate]]' }], +]); -privsCategories.userPrivilegeList = [ - 'find', - 'read', - 'topics:read', - 'topics:create', - 'topics:reply', - 'topics:schedule', - 'topics:tag', - 'posts:edit', - 'posts:history', - 'posts:delete', - 'posts:upvote', - 'posts:downvote', - 'topics:delete', - 'posts:view_deleted', - 'purge', - 'moderate', -]; - -privsCategories.groupPrivilegeList = privsCategories.userPrivilegeList.map(privilege => `groups:${privilege}`); - -privsCategories.privilegeList = privsCategories.userPrivilegeList.concat(privsCategories.groupPrivilegeList); - -privsCategories.getUserPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.list', privsCategories.userPrivilegeList.slice()); -privsCategories.getGroupPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.groups.list', privsCategories.groupPrivilegeList.slice()); +privsCategories.getUserPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.list', Array.from(_privilegeMap.keys())); +privsCategories.getGroupPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.groups.list', Array.from(_privilegeMap.keys()).map(privilege => `groups:${privilege}`)); privsCategories.getPrivilegeList = async () => { const [user, group] = await Promise.all([ privsCategories.getUserPrivilegeList(), @@ -64,16 +46,23 @@ privsCategories.getPrivilegeList = async () => { return user.concat(group); }; +privsCategories.init = async () => { + await plugins.hooks.fire('static:privileges.categories.init', { + privileges: _privilegeMap, + }); +}; + // Method used in admin/category controller to show all users/groups with privs in that given cid privsCategories.list = async function (cid) { - const labels = await utils.promiseParallel({ - users: plugins.hooks.fire('filter:privileges.list_human', privsCategories.privilegeLabels.slice()), - groups: plugins.hooks.fire('filter:privileges.groups.list_human', privsCategories.privilegeLabels.slice()), + let labels = Array.from(_privilegeMap.values()).map(data => data.label); + labels = await utils.promiseParallel({ + users: plugins.hooks.fire('filter:privileges.list_human', labels.slice()), + groups: plugins.hooks.fire('filter:privileges.groups.list_human', labels.slice()), }); const keys = await utils.promiseParallel({ - users: plugins.hooks.fire('filter:privileges.list', privsCategories.userPrivilegeList.slice()), - groups: plugins.hooks.fire('filter:privileges.groups.list', privsCategories.groupPrivilegeList.slice()), + users: privsCategories.getUserPrivilegeList(), + groups: privsCategories.getGroupPrivilegeList(), }); const payload = await utils.promiseParallel({ @@ -83,8 +72,8 @@ privsCategories.list = async function (cid) { }); payload.keys = keys; - payload.columnCountUserOther = payload.labels.users.length - privsCategories.privilegeLabels.length; - payload.columnCountGroupOther = payload.labels.groups.length - privsCategories.privilegeLabels.length; + payload.columnCountUserOther = payload.labels.users.length - labels.users.length; + payload.columnCountGroupOther = payload.labels.groups.length - labels.groups.length; return payload; }; diff --git a/src/privileges/global.js b/src/privileges/global.js index 00cf0cce05..734f242b1d 100644 --- a/src/privileges/global.js +++ b/src/privileges/global.js @@ -11,48 +11,31 @@ const utils = require('../utils'); const privsGlobal = module.exports; -privsGlobal.privilegeLabels = [ - { name: '[[admin/manage/privileges:chat]]' }, - { name: '[[admin/manage/privileges:upload-images]]' }, - { name: '[[admin/manage/privileges:upload-files]]' }, - { name: '[[admin/manage/privileges:signature]]' }, - { name: '[[admin/manage/privileges:invite]]' }, - { name: '[[admin/manage/privileges:allow-group-creation]]' }, - { name: '[[admin/manage/privileges:search-content]]' }, - { name: '[[admin/manage/privileges:search-users]]' }, - { name: '[[admin/manage/privileges:search-tags]]' }, - { name: '[[admin/manage/privileges:view-users]]' }, - { name: '[[admin/manage/privileges:view-tags]]' }, - { name: '[[admin/manage/privileges:view-groups]]' }, - { name: '[[admin/manage/privileges:allow-local-login]]' }, - { name: '[[admin/manage/privileges:ban]]' }, - { name: '[[admin/manage/privileges:view-users-info]]' }, -]; +/** + * Looking to add a new global privilege via plugin/theme? Attach a hook to + * `static:privileges.global.init` and call .set() on the privilege map passed + * in to your listener. + */ +const _privilegeMap = new Map([ + ['chat', { label: '[[admin/manage/privileges:chat]]' }], + ['upload:post:image', { label: '[[admin/manage/privileges:upload-images]]' }], + ['upload:post:file', { label: '[[admin/manage/privileges:upload-files]]' }], + ['signature', { label: '[[admin/manage/privileges:signature]]' }], + ['invite', { label: '[[admin/manage/privileges:invite]]' }], + ['group:create', { label: '[[admin/manage/privileges:allow-group-creation]]' }], + ['search:content', { label: '[[admin/manage/privileges:search-content]]' }], + ['search:users', { label: '[[admin/manage/privileges:search-users]]' }], + ['search:tags', { label: '[[admin/manage/privileges:search-tags]]' }], + ['view:users', { label: '[[admin/manage/privileges:view-users]]' }], + ['view:tags', { label: '[[admin/manage/privileges:view-tags]]' }], + ['view:groups', { label: '[[admin/manage/privileges:view-groups]]' }], + ['local:login', { label: '[[admin/manage/privileges:allow-local-login]]' }], + ['ban', { label: '[[admin/manage/privileges:ban]]' }], + ['view:users:info', { label: '[[admin/manage/privileges:view-users-info]]' }], +]); -privsGlobal.userPrivilegeList = [ - 'chat', - 'upload:post:image', - 'upload:post:file', - 'signature', - 'invite', - 'group:create', - 'search:content', - 'search:users', - 'search:tags', - 'view:users', - 'view:tags', - 'view:groups', - 'local:login', - 'ban', - 'view:users:info', -]; - -privsGlobal.groupPrivilegeList = privsGlobal.userPrivilegeList.map(privilege => `groups:${privilege}`); - -privsGlobal.privilegeList = privsGlobal.userPrivilegeList.concat(privsGlobal.groupPrivilegeList); - -privsGlobal.getUserPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.global.list', privsGlobal.userPrivilegeList.slice()); -privsGlobal.getGroupPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.global.groups.list', privsGlobal.groupPrivilegeList.slice()); +privsGlobal.getUserPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.global.list', Array.from(_privilegeMap.keys())); +privsGlobal.getGroupPrivilegeList = async () => await plugins.hooks.fire('filter:privileges.global.groups.list', Array.from(_privilegeMap.keys()).map(privilege => `groups:${privilege}`)); privsGlobal.getPrivilegeList = async () => { const [user, group] = await Promise.all([ privsGlobal.getUserPrivilegeList(), @@ -61,11 +44,18 @@ privsGlobal.getPrivilegeList = async () => { return user.concat(group); }; +privsGlobal.init = async () => { + await plugins.hooks.fire('static:privileges.global.init', { + privileges: _privilegeMap, + }); +}; + privsGlobal.list = async function () { async function getLabels() { + const labels = Array.from(_privilegeMap.values()).map(data => data.label); return await utils.promiseParallel({ - users: plugins.hooks.fire('filter:privileges.global.list_human', privsGlobal.privilegeLabels.slice()), - groups: plugins.hooks.fire('filter:privileges.global.groups.list_human', privsGlobal.privilegeLabels.slice()), + users: plugins.hooks.fire('filter:privileges.global.list_human', labels.slice()), + groups: plugins.hooks.fire('filter:privileges.global.groups.list_human', labels.slice()), }); } diff --git a/src/privileges/index.js b/src/privileges/index.js index 6445b07b97..e399e25c9d 100644 --- a/src/privileges/index.js +++ b/src/privileges/index.js @@ -8,4 +8,10 @@ privileges.topics = require('./topics'); privileges.posts = require('./posts'); privileges.users = require('./users'); +privileges.init = async () => { + await privileges.global.init(); + await privileges.admin.init(); + await privileges.categories.init(); +}; + require('../promisify')(privileges); diff --git a/src/views/admin/partials/privileges/category.tpl b/src/views/admin/partials/privileges/category.tpl index 19a21a9032..6e4dbdf0cf 100644 --- a/src/views/admin/partials/privileges/category.tpl +++ b/src/views/admin/partials/privileges/category.tpl @@ -14,9 +14,9 @@ [[admin/manage/categories:privileges.section-group]] [[admin/manage/privileges:select-clear-all]] - - {privileges.labels.groups.name} - + {{{ each privileges.labels.groups }}} + {@value} + {{{ end }}} @@ -97,9 +97,9 @@ [[admin/manage/categories:privileges.section-user]] [[admin/manage/privileges:select-clear-all]] - - {privileges.labels.users.name} - + {{{ each privileges.labels.users }}} + {@value} + {{{ end }}} diff --git a/src/views/admin/partials/privileges/global.tpl b/src/views/admin/partials/privileges/global.tpl index 28271a215d..a6f3e6f4ad 100644 --- a/src/views/admin/partials/privileges/global.tpl +++ b/src/views/admin/partials/privileges/global.tpl @@ -18,9 +18,9 @@ [[admin/manage/categories:privileges.section-group]] [[admin/manage/privileges:select-clear-all]] - - {privileges.labels.groups.name} - + {{{ each privileges.labels.groups }}} + {@value} + {{{ end }}} @@ -71,9 +71,9 @@ [[admin/manage/categories:privileges.section-user]] [[admin/manage/privileges:select-clear-all]] - - {privileges.labels.users.name} - + {{{ each privileges.labels.users }}} + {@value} + {{{ end }}} diff --git a/src/webserver.js b/src/webserver.js index df7b9c3456..6e75ac1fba 100644 --- a/src/webserver.js +++ b/src/webserver.js @@ -31,6 +31,7 @@ const logger = require('./logger'); const plugins = require('./plugins'); const flags = require('./flags'); const topicEvents = require('./topics/events'); +const privileges = require('./privileges'); const routes = require('./routes'); const auth = require('./routes/authentication'); @@ -103,6 +104,7 @@ async function initializeNodeBB() { middleware: middleware, }); await routes(app, middleware); + await privileges.init(); await meta.blacklist.load(); await flags.init(); await analytics.init(); diff --git a/test/posts.js b/test/posts.js index fc9b89f44b..4858167336 100644 --- a/test/posts.js +++ b/test/posts.js @@ -704,7 +704,8 @@ describe('Post\'s', () => { const cat2 = await categories.create({ name: 'Test Category', description: 'Test category created by testing script' }); const result = await apiTopics.create({ uid: globalModUid }, { title: 'target topic', content: 'queued topic', cid: cat2.cid }); const modUid = await user.create({ username: 'modofcat1' }); - await privileges.categories.give(privileges.categories.userPrivilegeList, cat1.cid, modUid); + const userPrivilegeList = await privileges.categories.getUserPrivilegeList(); + await privileges.categories.give(userPrivilegeList, cat1.cid, modUid); let err; try { await apiPosts.move({ uid: modUid }, { pid: replyPid, tid: result.tid });