From 035f624758dc4d88b32e5aecebc881d1d0fbb073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Thu, 30 May 2019 19:30:47 -0400 Subject: [PATCH] Remove implicit mod privs. closes #6345 (#7648) * feat: add upgrade script to give mods privs * feat: give all privileges when making a moderator * feat: remove implicit privs * feat: give global mods default privs * feat: more priv fixes * feat: use lodash * fix: remove implicit mod priv from topic delete * fix: more privs * fix: posts.canEdit * fix: canDelete and canEdit * fix: tests, remove console.log * feat: shorter functions * feat: add tests * fix: uids * fix: redis random test fail --- public/src/admin/manage/admins-mods.js | 4 +- src/categories/create.js | 8 +- src/controllers/admin/admins-mods.js | 2 + src/install.js | 3 + src/posts/category.js | 8 +- src/privileges/categories.js | 63 +++------ src/privileges/global.js | 25 ++-- src/privileges/helpers.js | 43 +++---- src/privileges/posts.js | 143 ++++++++++----------- src/privileges/topics.js | 52 ++++---- src/privileges/users.js | 14 +- src/upgrades/1.12.3/give_mod_privileges.js | 88 +++++++++++++ test/categories.js | 4 +- test/posts.js | 26 ++++ test/topics.js | 63 +++++++-- 15 files changed, 319 insertions(+), 227 deletions(-) create mode 100644 src/upgrades/1.12.3/give_mod_privileges.js diff --git a/public/src/admin/manage/admins-mods.js b/public/src/admin/manage/admins-mods.js index 1c7022339a..c2522aeea3 100644 --- a/public/src/admin/manage/admins-mods.js +++ b/public/src/admin/manage/admins-mods.js @@ -89,7 +89,7 @@ define('admin/manage/admins-mods', ['translator', 'benchpress', 'autocomplete'], var cid = $(ev.target).attr('data-cid'); socket.emit('admin.categories.setPrivilege', { cid: cid, - privilege: ['moderate'], + privilege: ajaxify.data.allPrivileges, set: true, member: ui.item.user.uid, }, function (err) { @@ -120,7 +120,7 @@ define('admin/manage/admins-mods', ['translator', 'benchpress', 'autocomplete'], if (confirm) { socket.emit('admin.categories.setPrivilege', { cid: cid, - privilege: ['moderate'], + privilege: ajaxify.data.allPrivileges, set: false, member: uid, }, function (err) { diff --git a/src/categories/create.js b/src/categories/create.js index 3c1c23a354..e736a40acb 100644 --- a/src/categories/create.js +++ b/src/categories/create.js @@ -68,6 +68,11 @@ module.exports = function (Categories) { 'posts:downvote', 'topics:delete', ]; + const modPrivileges = defaultPrivileges.concat([ + 'posts:view_deleted', + 'purge', + 'moderate', + ]); async.series([ async.apply(db.setObject, 'category:' + category.cid, category), @@ -78,7 +83,8 @@ module.exports = function (Categories) { Categories.parseDescription(category.cid, category.description, next); }, async.apply(db.sortedSetsAdd, ['categories:cid', 'cid:' + parentCid + ':children'], category.order, category.cid), - async.apply(privileges.categories.give, defaultPrivileges, category.cid, ['administrators', 'registered-users']), + async.apply(privileges.categories.give, defaultPrivileges, category.cid, 'registered-users'), + async.apply(privileges.categories.give, modPrivileges, category.cid, ['administrators', 'Global Moderators']), async.apply(privileges.categories.give, ['find', 'read', 'topics:read'], category.cid, ['guests', 'spiders']), ], next); }, diff --git a/src/controllers/admin/admins-mods.js b/src/controllers/admin/admins-mods.js index 97d5828c16..92d4a4bf20 100644 --- a/src/controllers/admin/admins-mods.js +++ b/src/controllers/admin/admins-mods.js @@ -4,6 +4,7 @@ var async = require('async'); var groups = require('../../groups'); var categories = require('../../categories'); +var privileges = require('../../privileges'); var AdminsMods = module.exports; @@ -23,6 +24,7 @@ AdminsMods.get = function (req, res, next) { }, next); }, function (results) { + results.allPrivileges = privileges.userPrivilegeList; res.render('admin/manage/admins-mods', results); }, ], next); diff --git a/src/install.js b/src/install.js index fdb53c0e1e..bbfecf1b05 100644 --- a/src/install.js +++ b/src/install.js @@ -397,6 +397,9 @@ function giveGlobalPrivileges(next) { function (next) { privileges.global.give(defaultPrivileges, 'registered-users', next); }, + function (next) { + privileges.global.give(defaultPrivileges.concat(['ban', 'upload:post:file']), 'Global Moderators', next); + }, function (next) { privileges.global.give(['view:users', 'view:tags', 'view:groups'], 'guests', next); }, diff --git a/src/posts/category.js b/src/posts/category.js index d1de31850e..3cfc3e4d09 100644 --- a/src/posts/category.js +++ b/src/posts/category.js @@ -28,9 +28,7 @@ module.exports = function (Posts) { }, function (_postData, next) { postData = _postData; - tids = _.uniq(postData.map(function (post) { - return post && post.tid; - }).filter(Boolean)); + tids = _.uniq(postData.map(post => post && post.tid).filter(Boolean)); topics.getTopicsFields(tids, ['cid'], next); }, @@ -42,9 +40,7 @@ module.exports = function (Posts) { } }); - var cids = postData.map(function (post) { - return map[post.tid]; - }); + var cids = postData.map(post => map[post.tid]); next(null, cids); }, ], callback); diff --git a/src/privileges/categories.js b/src/privileges/categories.js index 9eda36d211..36d09dab25 100644 --- a/src/privileges/categories.js +++ b/src/privileges/categories.js @@ -64,10 +64,10 @@ module.exports = function (privileges) { var isAdminOrMod = results.isAdministrator || results.isModerator; plugins.fireHook('filter:privileges.categories.get', { - 'topics:create': privData['topics:create'] || isAdminOrMod, - 'topics:read': privData['topics:read'] || isAdminOrMod, - 'topics:tag': privData['topics:tag'] || isAdminOrMod, - read: privData.read || isAdminOrMod, + 'topics:create': privData['topics:create'] || results.isAdministrator, + 'topics:read': privData['topics:read'] || results.isAdministrator, + 'topics:tag': privData['topics:tag'] || results.isAdministrator, + read: privData.read || results.isAdministrator, cid: cid, uid: uid, editable: isAdminOrMod, @@ -94,7 +94,7 @@ module.exports = function (privileges) { privileges.categories.isUserAllowedTo = function (privilege, cid, uid, callback) { if (!cid) { - return callback(null, false); + return setImmediate(callback, null, false); } if (Array.isArray(cid)) { helpers.isUserAllowedTo(privilege, uid, cid, function (err, results) { @@ -109,30 +109,18 @@ module.exports = function (privileges) { privileges.categories.can = function (privilege, cid, uid, callback) { if (!cid) { - return callback(null, false); + return setImmediate(callback, null, false); } - async.waterfall([ function (next) { - categories.getCategoryField(cid, 'disabled', next); + async.parallel({ + disabled: async.apply(categories.getCategoryField, cid, 'disabled'), + isAdmin: async.apply(user.isAdministrator, uid), + isAllowed: async.apply(privileges.categories.isUserAllowedTo, privilege, cid, uid), + }, next); }, - function (disabled, next) { - if (disabled) { - return callback(null, false); - } - helpers.some([ - function (next) { - helpers.isUserAllowedTo(privilege, uid, [cid], function (err, results) { - next(err, Array.isArray(results) && results.length ? results[0] : false); - }); - }, - function (next) { - user.isModerator(uid, cid, next); - }, - function (next) { - user.isAdministrator(uid, next); - }, - ], next); + function (results, next) { + next(null, !results.disabled && (results.isAllowed || results.isAdmin)); }, ], callback); }; @@ -151,7 +139,7 @@ module.exports = function (privileges) { function (results, next) { cids = cids.filter(function (cid, index) { return !results.categories[index].disabled && - (results.allowedTo[index] || results.isAdmin || results.isModerators[index]); + (results.allowedTo[index] || results.isAdmin); }); next(null, cids.filter(Boolean)); @@ -167,9 +155,6 @@ module.exports = function (privileges) { allowedTo: function (next) { helpers.isUserAllowedTo(privilege, uid, cids, next); }, - isModerators: function (next) { - user.isModerator(uid, cids, next); - }, isAdmin: function (next) { user.isAdministrator(uid, next); }, @@ -178,7 +163,7 @@ module.exports = function (privileges) { privileges.categories.filterUids = function (privilege, cid, uids, callback) { if (!uids.length) { - return callback(null, []); + return setImmediate(callback, null, []); } uids = _.uniq(uids); @@ -189,9 +174,6 @@ module.exports = function (privileges) { allowedTo: function (next) { helpers.isUsersAllowedTo(privilege, uids, cid, next); }, - isModerators: function (next) { - user.isModerator(uids, cid, next); - }, isAdmins: function (next) { user.isAdministrator(uids, next); }, @@ -199,7 +181,7 @@ module.exports = function (privileges) { }, function (results, next) { uids = uids.filter(function (uid, index) { - return results.allowedTo[index] || results.isModerators[index] || results.isAdmins[index]; + return results.allowedTo[index] || results.isAdmins[index]; }); next(null, uids); }, @@ -218,19 +200,12 @@ module.exports = function (privileges) { async.waterfall([ function (next) { async.parallel({ - isAdministrator: function (next) { - user.isAdministrator(uid, next); - }, - moderatorOfCurrent: function (next) { - user.isModerator(uid, currentCid, next); - }, - moderatorOfTarget: function (next) { - user.isModerator(uid, targetCid, next); - }, + isAdmin: async.apply(user.isAdministrator, uid), + isModerators: async.apply(user.isModerator, uid, [currentCid, targetCid]), }, next); }, function (results, next) { - next(null, results.isAdministrator || (results.moderatorOfCurrent && results.moderatorOfTarget)); + next(null, results.isAdmin || !results.isModerators.includes(false)); }, ], callback); }; diff --git a/src/privileges/global.js b/src/privileges/global.js index 275aa73275..7edb32ef12 100644 --- a/src/privileges/global.js +++ b/src/privileges/global.js @@ -84,25 +84,21 @@ module.exports = function (privileges) { isAdministrator: function (next) { user.isAdministrator(uid, next); }, - isGlobalModerator: function (next) { - user.isGlobalModerator(uid, next); - }, }, next); }, function (results, next) { var privData = _.zipObject(privileges.global.userPrivilegeList, results.privileges); - var isAdminOrMod = results.isAdministrator || results.isGlobalModerator; plugins.fireHook('filter:privileges.global.get', { - chat: privData.chat || isAdminOrMod, - 'upload:post:image': privData['upload:post:image'] || isAdminOrMod, - 'upload:post:file': privData['upload:post:file'] || isAdminOrMod, - 'search:content': privData['search:content'] || isAdminOrMod, - 'search:users': privData['search:users'] || isAdminOrMod, - 'search:tags': privData['search:tags'] || isAdminOrMod, - 'view:users': privData['view:users'] || isAdminOrMod, - 'view:tags': privData['view:tags'] || isAdminOrMod, - 'view:groups': privData['view:groups'] || isAdminOrMod, + chat: privData.chat || results.isAdministrator, + 'upload:post:image': privData['upload:post:image'] || results.isAdministrator, + 'upload:post:file': privData['upload:post:file'] || results.isAdministrator, + 'search:content': privData['search:content'] || results.isAdministrator, + 'search:users': privData['search:users'] || results.isAdministrator, + 'search:tags': privData['search:tags'] || results.isAdministrator, + 'view:users': privData['view:users'] || results.isAdministrator, + 'view:tags': privData['view:tags'] || results.isAdministrator, + 'view:groups': privData['view:groups'] || results.isAdministrator, }, next); }, ], callback); @@ -115,9 +111,6 @@ module.exports = function (privileges) { next(err, Array.isArray(results) && results.length ? results[0] : false); }); }, - function (next) { - user.isGlobalModerator(uid, next); - }, function (next) { user.isAdministrator(uid, next); }, diff --git a/src/privileges/helpers.js b/src/privileges/helpers.js index 49002c8680..a0933c5c5e 100644 --- a/src/privileges/helpers.js +++ b/src/privileges/helpers.js @@ -106,18 +106,12 @@ helpers.isUsersAllowedTo = function (privilege, uids, cid, callback) { }; function isSystemGroupAllowedToCids(privilege, uid, cids, callback) { - var groupKeys = cids.map(function (cid) { - return 'cid:' + cid + ':privileges:groups:' + privilege; - }); - + const groupKeys = cids.map(cid => 'cid:' + cid + ':privileges:groups:' + privilege); groups.isMemberOfGroups(uidToSystemGroup[uid], groupKeys, callback); } function isSystemGroupAllowedToPrivileges(privileges, uid, cid, callback) { - var groupKeys = privileges.map(function (privilege) { - return 'cid:' + cid + ':privileges:groups:' + privilege; - }); - + const groupKeys = privileges.map(privilege => 'cid:' + cid + ':privileges:groups:' + privilege); groups.isMemberOfGroups(uidToSystemGroup[uid], groupKeys, callback); } @@ -128,15 +122,11 @@ helpers.getUserPrivileges = function (cid, hookName, userPrivilegeList, callback async.apply(plugins.fireHook, hookName, userPrivilegeList.slice()), function (_privs, next) { userPrivileges = _privs; - groups.getMembersOfGroups(userPrivileges.map(function (privilege) { - return 'cid:' + cid + ':privileges:' + privilege; - }), next); + groups.getMembersOfGroups(userPrivileges.map(privilege => 'cid:' + cid + ':privileges:' + privilege), next); }, function (_memberSets, next) { memberSets = _memberSets.map(function (set) { - return set.map(function (uid) { - return parseInt(uid, 10); - }); + return set.map(uid => parseInt(uid, 10)); }); var members = _.uniq(_.flatten(memberSets)); @@ -164,9 +154,7 @@ helpers.getGroupPrivileges = function (cid, hookName, groupPrivilegeList, callba groupPrivileges = _privs; async.parallel({ memberSets: function (next) { - groups.getMembersOfGroups(groupPrivileges.map(function (privilege) { - return 'cid:' + cid + ':privileges:' + privilege; - }), next); + groups.getMembersOfGroups(groupPrivileges.map(privilege => 'cid:' + cid + ':privileges:' + privilege), next); }, groupNames: function (next) { groups.getGroups('groups:createtime', 0, -1, next); @@ -177,17 +165,11 @@ helpers.getGroupPrivileges = function (cid, hookName, groupPrivilegeList, callba var memberSets = results.memberSets; var uniqueGroups = _.uniq(_.flatten(memberSets)); - var groupNames = results.groupNames.filter(function (groupName) { - return !groupName.includes(':privileges:') && uniqueGroups.includes(groupName); - }); + var groupNames = results.groupNames.filter(groupName => !groupName.includes(':privileges:') && uniqueGroups.includes(groupName)); groupNames = groups.ephemeralGroups.concat(groupNames); - var registeredUsersIndex = groupNames.indexOf('registered-users'); - if (registeredUsersIndex !== -1) { - groupNames.splice(0, 0, groupNames.splice(registeredUsersIndex, 1)[0]); - } else { - groupNames = ['registered-users'].concat(groupNames); - } + moveToFront(groupNames, 'Global Moderators'); + moveToFront(groupNames, 'registered-users'); var adminIndex = groupNames.indexOf('administrators'); if (adminIndex !== -1) { @@ -227,6 +209,15 @@ helpers.getGroupPrivileges = function (cid, hookName, groupPrivilegeList, callba ], callback); }; +function moveToFront(groupNames, groupToMove) { + const index = groupNames.indexOf(groupToMove); + if (index !== -1) { + groupNames.splice(0, 0, groupNames.splice(index, 1)[0]); + } else { + groupNames.unshift(groupToMove); + } +} + helpers.giveOrRescind = function (method, privileges, cids, groupNames, callback) { groupNames = Array.isArray(groupNames) ? groupNames : [groupNames]; cids = Array.isArray(cids) ? cids : [cids]; diff --git a/src/privileges/posts.js b/src/privileges/posts.js index a8983c3234..9da75c769f 100644 --- a/src/privileges/posts.js +++ b/src/privileges/posts.js @@ -16,39 +16,49 @@ module.exports = function (privileges) { privileges.posts.get = function (pids, uid, callback) { if (!Array.isArray(pids) || !pids.length) { - return callback(null, []); + return setImmediate(callback, null, []); } - + let uniqueCids; + let cids; async.waterfall([ function (next) { posts.getCidsByPids(pids, next); }, - function (cids, next) { + function (_cids, next) { + cids = _cids; + uniqueCids = _.uniq(cids); async.parallel({ isAdmin: async.apply(user.isAdministrator, uid), - isModerator: async.apply(user.isModerator, uid, cids), + isModerator: async.apply(user.isModerator, uid, uniqueCids), isOwner: async.apply(posts.isOwner, pids, uid), - 'topics:read': async.apply(helpers.isUserAllowedTo, 'topics:read', uid, cids), - read: async.apply(helpers.isUserAllowedTo, 'read', uid, cids), - 'posts:edit': async.apply(helpers.isUserAllowedTo, 'posts:edit', uid, cids), - 'posts:history': async.apply(helpers.isUserAllowedTo, 'posts:history', uid, cids), - 'posts:view_deleted': async.apply(helpers.isUserAllowedTo, 'posts:view_deleted', uid, cids), + 'topics:read': async.apply(helpers.isUserAllowedTo, 'topics:read', uid, uniqueCids), + read: async.apply(helpers.isUserAllowedTo, 'read', uid, uniqueCids), + 'posts:edit': async.apply(helpers.isUserAllowedTo, 'posts:edit', uid, uniqueCids), + 'posts:history': async.apply(helpers.isUserAllowedTo, 'posts:history', uid, uniqueCids), + 'posts:view_deleted': async.apply(helpers.isUserAllowedTo, 'posts:view_deleted', uid, uniqueCids), }, next); }, function (results, next) { - var privileges = pids.map(function (pid, i) { - var isAdminOrMod = results.isAdmin || results.isModerator[i]; - var editable = isAdminOrMod || (results.isOwner[i] && results['posts:edit'][i]); - var viewDeletedPosts = isAdminOrMod || results.isOwner[i] || results['posts:view_deleted'][i]; - var viewHistory = isAdminOrMod || results.isOwner[i] || results['posts:history'][i]; + const isModerator = _.zipObject(uniqueCids, results.isModerator); + const privData = {}; + privData['topics:read'] = _.zipObject(uniqueCids, results['topics:read']); + privData.read = _.zipObject(uniqueCids, results.read); + privData['posts:edit'] = _.zipObject(uniqueCids, results['posts:edit']); + privData['posts:history'] = _.zipObject(uniqueCids, results['posts:history']); + privData['posts:view_deleted'] = _.zipObject(uniqueCids, results['posts:view_deleted']); + + var privileges = cids.map(function (cid, i) { + var isAdminOrMod = results.isAdmin || isModerator[cid]; + var editable = (privData['posts:edit'][cid] && (results.isOwner[i] || results.isModerator)) || results.isAdmin; + var viewDeletedPosts = results.isOwner[i] || privData['posts:view_deleted'][cid] || results.isAdmin; + var viewHistory = results.isOwner[i] || privData['posts:history'][cid] || results.isAdmin; return { editable: editable, - view_deleted: editable, move: isAdminOrMod, isAdminOrMod: isAdminOrMod, - 'topics:read': results['topics:read'][i] || isAdminOrMod, - read: results.read[i] || isAdminOrMod, + 'topics:read': privData['topics:read'][cid] || results.isAdmin, + read: privData.read[cid] || results.isAdmin, 'posts:history': viewHistory, 'posts:view_deleted': viewDeletedPosts, }; @@ -72,7 +82,7 @@ module.exports = function (privileges) { privileges.posts.filter = function (privilege, pids, uid, callback) { if (!Array.isArray(pids) || !pids.length) { - return callback(null, []); + return setImmediate(callback, null, []); } var cids; var postData; @@ -112,18 +122,16 @@ module.exports = function (privileges) { privileges.categories.getBase(privilege, cids, uid, next); }, function (results, next) { - var isModOf = {}; cids = cids.filter(function (cid, index) { - isModOf[cid] = results.isModerators[index]; return !results.categories[index].disabled && - (results.allowedTo[index] || results.isAdmin || results.isModerators[index]); + (results.allowedTo[index] || results.isAdmin); }); const cidsSet = new Set(cids); pids = postData.filter(function (post) { return post.topic && cidsSet.has(post.topic.cid) && - ((!post.topic.deleted && !post.deleted) || results.isAdmin || isModOf[post.cid]); + ((!post.topic.deleted && !post.deleted) || results.isAdmin); }).map(post => post.pid); plugins.fireHook('filter:privileges.posts.filter', { @@ -138,19 +146,41 @@ module.exports = function (privileges) { }; privileges.posts.canEdit = function (pid, uid, callback) { + let results; async.waterfall([ function (next) { async.parallel({ - isEditable: async.apply(isPostEditable, pid, uid), - isAdminOrMod: async.apply(isAdminOrMod, pid, uid), + isAdmin: async.apply(privileges.users.isAdministrator, uid), + isMod: async.apply(posts.isModerator, [pid], uid), + owner: async.apply(posts.isOwner, pid, uid), + edit: async.apply(privileges.posts.can, 'posts:edit', pid, uid), + postData: async.apply(posts.getPostFields, pid, ['tid', 'timestamp']), }, next); }, - function (results, next) { - if (results.isAdminOrMod) { - return next(null, { flag: true }); + function (_results, next) { + results = _results; + results.isMod = results.isMod[0]; + if (results.isAdmin) { + return callback(null, { flag: true }); + } + + if (!results.isMod && meta.config.postEditDuration && (Date.now() - results.postData.timestamp > meta.config.postEditDuration * 1000)) { + return callback(null, { flag: false, message: '[[error:post-edit-duration-expired, ' + meta.config.postEditDuration + ']]' }); + } + topics.isLocked(results.postData.tid, next); + }, + function (isLocked, next) { + if (!results.isMod && isLocked) { + return callback(null, { flag: false, message: '[[error:topic-locked]]' }); } - next(null, results.isEditable); + results.pid = parseInt(pid, 10); + results.uid = uid; + + plugins.fireHook('filter:privileges.posts.edit', results, next); + }, + function (result, next) { + next(null, { flag: result.edit && (result.owner || result.isMod), message: '[[error:no-privileges]]' }); }, ], callback); }; @@ -164,31 +194,29 @@ module.exports = function (privileges) { function (_postData, next) { postData = _postData; async.parallel({ - isAdminOrMod: async.apply(isAdminOrMod, pid, uid), + isAdmin: async.apply(privileges.users.isAdministrator, uid), + isMod: async.apply(posts.isModerator, [pid], uid), isLocked: async.apply(topics.isLocked, postData.tid), isOwner: async.apply(posts.isOwner, pid, uid), 'posts:delete': async.apply(privileges.posts.can, 'posts:delete', pid, uid), }, next); }, function (results, next) { - if (results.isAdminOrMod) { + results.isMod = results.isMod[0]; + if (results.isAdmin) { return next(null, { flag: true }); } - if (results.isLocked) { + if (!results.isMod && results.isLocked) { return next(null, { flag: false, message: '[[error:topic-locked]]' }); } - if (!results['posts:delete']) { - return next(null, { flag: false, message: '[[error:no-privileges]]' }); - } - var postDeleteDuration = meta.config.postDeleteDuration; - if (postDeleteDuration && (Date.now() - postData.timestamp > postDeleteDuration * 1000)) { + if (!results.isMod && postDeleteDuration && (Date.now() - postData.timestamp > postDeleteDuration * 1000)) { return next(null, { flag: false, message: '[[error:post-delete-duration-expired, ' + meta.config.postDeleteDuration + ']]' }); } var deleterUid = postData.deleterUid; - var flag = results.isOwner && (deleterUid === 0 || deleterUid === postData.uid); + var flag = results['posts:delete'] && ((results.isOwner && (deleterUid === 0 || deleterUid === postData.uid)) || results.isMod); next(null, { flag: flag, message: '[[error:no-privileges]]' }); }, ], callback); @@ -233,51 +261,16 @@ module.exports = function (privileges) { async.parallel({ purge: async.apply(privileges.categories.isUserAllowedTo, 'purge', cid, uid), owner: async.apply(posts.isOwner, pid, uid), - isAdminOrMod: async.apply(privileges.categories.isAdminOrMod, cid, uid), + isAdmin: async.apply(privileges.users.isAdministrator, uid), + isModerator: async.apply(privileges.users.isModerator, uid, cid), }, next); }, function (results, next) { - next(null, results.isAdminOrMod || (results.purge && results.owner)); + next(null, (results.purge && (results.owner || results.isModerator)) || results.isAdmin); }, ], callback); }; - function isPostEditable(pid, uid, callback) { - async.waterfall([ - function (next) { - posts.getPostFields(pid, ['tid', 'timestamp'], next); - }, - function (postData, next) { - var postEditDuration = meta.config.postEditDuration; - if (postEditDuration && (Date.now() - postData.timestamp > postEditDuration * 1000)) { - return callback(null, { flag: false, message: '[[error:post-edit-duration-expired, ' + meta.config.postEditDuration + ']]' }); - } - topics.isLocked(postData.tid, next); - }, - function (isLocked, next) { - if (isLocked) { - return callback(null, { flag: false, message: '[[error:topic-locked]]' }); - } - - async.parallel({ - owner: async.apply(posts.isOwner, pid, uid), - edit: async.apply(privileges.posts.can, 'posts:edit', pid, uid), - }, next); - }, - (result, next) => { - Object.assign(result, { - pid: parseInt(pid, 10), - uid: uid, - }); - - plugins.fireHook('filter:privileges.posts.edit', result, next); - }, - function (result, next) { - next(null, { flag: result.owner && result.edit, message: '[[error:no-privileges]]' }); - }, - ], callback); - } - function isAdminOrMod(pid, uid, callback) { helpers.some([ function (next) { diff --git a/src/privileges/topics.js b/src/privileges/topics.js index 4b56f22fad..247447b031 100644 --- a/src/privileges/topics.js +++ b/src/privileges/topics.js @@ -38,23 +38,23 @@ module.exports = function (privileges) { var isOwner = uid > 0 && uid === topic.uid; var isAdminOrMod = results.isAdministrator || results.isModerator; var editable = isAdminOrMod; - var deletable = isAdminOrMod || (isOwner && privData['topics:delete']); - var purge = results.isAdministrator || privData.purge; + var deletable = (privData['topics:delete'] && (isOwner || results.isModerator)) || results.isAdministrator; plugins.fireHook('filter:privileges.topics.get', { - 'topics:reply': (privData['topics:reply'] && !topic.locked && !topic.deleted) || isAdminOrMod, - 'topics:read': privData['topics:read'] || isAdminOrMod, - 'topics:tag': privData['topics:tag'] || isAdminOrMod, - 'topics:delete': (isOwner && privData['topics:delete']) || isAdminOrMod, - 'posts:edit': (privData['posts:edit'] && !topic.locked) || isAdminOrMod, - 'posts:history': privData['posts:history'] || isAdminOrMod, - 'posts:delete': (privData['posts:delete'] && !topic.locked) || isAdminOrMod, - 'posts:view_deleted': privData['posts:view_deleted'] || isAdminOrMod, - read: privData.read || isAdminOrMod, + 'topics:reply': (privData['topics:reply'] && ((!topic.locked && !topic.deleted) || results.isModerator)) || results.isAdministrator, + 'topics:read': privData['topics:read'] || results.isAdministrator, + 'topics:tag': privData['topics:tag'] || results.isAdministrator, + 'topics:delete': (privData['topics:delete'] && (isOwner || results.isModerator)) || results.isAdministrator, + 'posts:edit': (privData['posts:edit'] && (!topic.locked || results.isModerator)) || results.isAdministrator, + 'posts:history': privData['posts:history'] || results.isAdministrator, + 'posts:delete': (privData['posts:delete'] && (!topic.locked || results.isModerator)) || results.isAdministrator, + 'posts:view_deleted': privData['posts:view_deleted'] || results.isAdministrator, + read: privData.read || results.isAdministrator, + purge: (privData.purge && (isOwner || results.isModerator)) || results.isAdministrator, + view_thread_tools: editable || deletable, editable: editable, deletable: deletable, - purge: purge, view_deleted: isAdminOrMod || isOwner, isAdminOrMod: isAdminOrMod, disabled: results.disabled, @@ -93,18 +93,16 @@ module.exports = function (privileges) { privileges.categories.getBase(privilege, cids, uid, next); }, function (results, next) { - var isModOf = {}; cids = cids.filter(function (cid, index) { - isModOf[cid] = results.isModerators[index]; return !results.categories[index].disabled && - (results.allowedTo[index] || results.isAdmin || results.isModerators[index]); + (results.allowedTo[index] || results.isAdmin); }); const cidsSet = new Set(cids); tids = topicsData.filter(function (topic) { return cidsSet.has(topic.cid) && - (!topic.deleted || results.isAdmin || isModOf[topic.cid]); + (!topic.deleted || results.isAdmin); }).map(topic => topic.tid); plugins.fireHook('filter:privileges.topics.filter', { @@ -120,7 +118,7 @@ module.exports = function (privileges) { privileges.topics.filterUids = function (privilege, tid, uids, callback) { if (!Array.isArray(uids) || !uids.length) { - return callback(null, []); + return setImmediate(callback, null, []); } uids = _.uniq(uids); @@ -138,9 +136,6 @@ module.exports = function (privileges) { allowedTo: function (next) { helpers.isUsersAllowedTo(privilege, uids, topicData.cid, next); }, - isModerators: function (next) { - user.isModerator(uids, topicData.cid, next); - }, isAdmins: function (next) { user.isAdministrator(uids, next); }, @@ -149,7 +144,7 @@ module.exports = function (privileges) { function (results, next) { uids = uids.filter(function (uid, index) { return !results.disabled && - ((results.allowedTo[index] && !topicData.deleted) || results.isAdmins[index] || results.isModerators[index]); + ((results.allowedTo[index] && !topicData.deleted) || results.isAdmins[index]); }); next(null, uids); @@ -166,11 +161,12 @@ module.exports = function (privileges) { async.parallel({ purge: async.apply(privileges.categories.isUserAllowedTo, 'purge', cid, uid), owner: async.apply(topics.isOwner, tid, uid), - isAdminOrMod: async.apply(privileges.categories.isAdminOrMod, cid, uid), + isAdmin: async.apply(privileges.users.isAdministrator, uid), + isModerator: async.apply(privileges.users.isModerator, uid, cid), }, next); }, function (results, next) { - next(null, results.isAdminOrMod || (results.purge && results.owner)); + next(null, (results.purge && (results.owner || results.isModerator)) || results.isAdmin); }, ], callback); }; @@ -191,23 +187,19 @@ module.exports = function (privileges) { }, next); }, function (results, next) { - if (results.isModerator || results.isAdministrator) { + if (results.isAdministrator) { return next(null, true); } var preventTopicDeleteAfterReplies = meta.config.preventTopicDeleteAfterReplies; - if (preventTopicDeleteAfterReplies && (topicData.postcount - 1) >= preventTopicDeleteAfterReplies) { + if (!results.isModerator && preventTopicDeleteAfterReplies && (topicData.postcount - 1) >= preventTopicDeleteAfterReplies) { var langKey = preventTopicDeleteAfterReplies > 1 ? '[[error:cant-delete-topic-has-replies, ' + meta.config.preventTopicDeleteAfterReplies + ']]' : '[[error:cant-delete-topic-has-reply]]'; return next(new Error(langKey)); } - if (!results['topics:delete'][0]) { - return next(null, false); - } - - next(null, results.isOwner); + next(null, results['topics:delete'][0] && (results.isOwner || results.isModerator)); }, ], callback); }; diff --git a/src/privileges/users.js b/src/privileges/users.js index 7cdfef0bf9..b73c1dd033 100644 --- a/src/privileges/users.js +++ b/src/privileges/users.js @@ -48,7 +48,7 @@ module.exports = function (privileges) { }, function (isGlobalModerator, next) { if (isGlobalModerator) { - return filterIsModerator(cids, uid, cids.map(function () { return true; }), callback); + return filterIsModerator(cids, uid, cids.map(() => true), callback); } uniqueCids = _.uniq(cids); @@ -56,16 +56,8 @@ module.exports = function (privileges) { helpers.isUserAllowedTo('moderate', uid, uniqueCids, next); }, function (isAllowed, next) { - var map = {}; - - uniqueCids.forEach(function (cid, index) { - map[cid] = isAllowed[index]; - }); - - var isModerator = cids.map(function (cid) { - return map[cid]; - }); - + const map = _.zipObject(uniqueCids, isAllowed); + const isModerator = cids.map(cid => map[cid]); filterIsModerator(cids, uid, isModerator, next); }, ], callback); diff --git a/src/upgrades/1.12.3/give_mod_privileges.js b/src/upgrades/1.12.3/give_mod_privileges.js new file mode 100644 index 0000000000..7cf113803d --- /dev/null +++ b/src/upgrades/1.12.3/give_mod_privileges.js @@ -0,0 +1,88 @@ +'use strict'; + +var async = require('async'); +var privileges = require('../../privileges'); +var groups = require('../../groups'); +var db = require('../../database'); + +module.exports = { + name: 'Give mods explicit privileges', + timestamp: Date.UTC(2019, 4, 28), + method: function (callback) { + var defaultPrivileges = [ + 'find', + 'read', + 'topics:read', + 'topics:create', + 'topics:reply', + 'topics:tag', + 'posts:edit', + 'posts:history', + 'posts:delete', + 'posts:upvote', + 'posts:downvote', + 'topics:delete', + ]; + const modPrivileges = defaultPrivileges.concat([ + 'posts:view_deleted', + 'purge', + 'moderate', + ]); + + const globalModPrivs = [ + 'chat', + 'upload:post:image', + 'upload:post:file', + 'signature', + 'ban', + 'search:content', + 'search:users', + 'search:tags', + 'view:users', + 'view:tags', + 'view:groups', + 'local:login', + ]; + + async.waterfall([ + function (next) { + db.getSortedSetRevRange('categories:cid', 0, -1, next); + }, + function (cids, next) { + async.eachSeries(cids, function (cid, next) { + async.waterfall([ + function (next) { + givePrivsToModerators(cid, '', next); + }, + function (next) { + givePrivsToModerators(cid, 'groups:', next); + }, + function (next) { + privileges.categories.give(modPrivileges, cid, ['Global Moderators'], next); + }, + ], next); + }, next); + }, + function (next) { + privileges.global.give(globalModPrivs, 'Global Moderators', next); + }, + ], callback); + + function givePrivsToModerators(cid, groupPrefix, callback) { + const privGroups = modPrivileges.map(function (priv) { + return 'cid:' + cid + ':privileges:' + groupPrefix + priv; + }); + + async.waterfall([ + function (next) { + db.getSortedSetRevRange('group:cid:' + cid + ':privileges:' + groupPrefix + 'moderate:members', 0, -1, next); + }, + function (members, next) { + async.eachSeries(members, function (member, next) { + groups.join(privGroups, member, next); + }, next); + }, + ], callback); + } + }, +}; diff --git a/test/categories.js b/test/categories.js index 644ce6adb1..16db481f4f 100644 --- a/test/categories.js +++ b/test/categories.js @@ -850,8 +850,8 @@ describe('Categories', function () { Categories.getModeratorUids([1, 2], function (err, uids) { assert.ifError(err); assert.strictEqual(2, uids.length); - assert.strictEqual(1, parseInt(uids[0], 10)); - assert.strictEqual(0, uids[1].length); + assert(uids[0].includes('1')); + assert.strictEqual(1, uids[1].length); done(); }); }); diff --git a/test/posts.js b/test/posts.js index 1d20bb303f..cca10542e0 100644 --- a/test/posts.js +++ b/test/posts.js @@ -265,6 +265,32 @@ describe('Post\'s', function () { }); }); + it('should not see post content if global mod does not have posts:view_deleted privilege', function (done) { + async.waterfall([ + function (next) { + user.create({ username: 'global mod', password: '123456' }, next); + }, + function (uid, next) { + groups.join('Global Moderators', uid, next); + }, + function (next) { + privileges.categories.rescind(['posts:view_deleted'], cid, 'Global Moderators', next); + }, + function (next) { + helpers.loginUser('global mod', '123456', function (err, _jar) { + assert.ifError(err); + var jar = _jar; + + request(nconf.get('url') + '/api/topic/' + tid, { jar: jar, json: true }, function (err, res, body) { + assert.ifError(err); + assert.equal(body.posts[1].content, '[[topic:post_is_deleted]]'); + privileges.categories.give(['posts:view_deleted'], cid, 'Global Moderators', next); + }); + }); + }, + ], done); + }); + it('should restore a post', function (done) { socketPosts.restore({ uid: voterUid }, { pid: replyPid, tid: tid }, function (err) { assert.ifError(err); diff --git a/test/topics.js b/test/topics.js index 2a1dd87a95..a8c26cb038 100644 --- a/test/topics.js +++ b/test/topics.js @@ -342,7 +342,7 @@ describe('Topic\'s', function () { }); it('should load topic tools', function (done) { - socketTopics.loadTopicTools({ uid: 1 }, { tid: newTopic.tid }, function (err, data) { + socketTopics.loadTopicTools({ uid: adminUid }, { tid: newTopic.tid }, function (err, data) { assert.ifError(err); assert(data); done(); @@ -350,21 +350,21 @@ describe('Topic\'s', function () { }); it('should delete the topic', function (done) { - socketTopics.delete({ uid: 1 }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { + socketTopics.delete({ uid: adminUid }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { assert.ifError(err); done(); }); }); it('should restore the topic', function (done) { - socketTopics.restore({ uid: 1 }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { + socketTopics.restore({ uid: adminUid }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { assert.ifError(err); done(); }); }); it('should lock topic', function (done) { - socketTopics.lock({ uid: 1 }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { + socketTopics.lock({ uid: adminUid }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { assert.ifError(err); topics.isLocked(newTopic.tid, function (err, isLocked) { assert.ifError(err); @@ -375,7 +375,7 @@ describe('Topic\'s', function () { }); it('should unlock topic', function (done) { - socketTopics.unlock({ uid: 1 }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { + socketTopics.unlock({ uid: adminUid }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { assert.ifError(err); topics.isLocked(newTopic.tid, function (err, isLocked) { assert.ifError(err); @@ -386,7 +386,7 @@ describe('Topic\'s', function () { }); it('should pin topic', function (done) { - socketTopics.pin({ uid: 1 }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { + socketTopics.pin({ uid: adminUid }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { assert.ifError(err); topics.getTopicField(newTopic.tid, 'pinned', function (err, pinned) { assert.ifError(err); @@ -397,7 +397,7 @@ describe('Topic\'s', function () { }); it('should unpin topic', function (done) { - socketTopics.unpin({ uid: 1 }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { + socketTopics.unpin({ uid: adminUid }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { assert.ifError(err); topics.getTopicField(newTopic.tid, 'pinned', function (err, pinned) { assert.ifError(err); @@ -408,7 +408,7 @@ describe('Topic\'s', function () { }); it('should move all topics', function (done) { - socketTopics.moveAll({ uid: 1 }, { cid: moveCid, currentCid: categoryObj.cid }, function (err) { + socketTopics.moveAll({ uid: adminUid }, { cid: moveCid, currentCid: categoryObj.cid }, function (err) { assert.ifError(err); topics.getTopicField(newTopic.tid, 'cid', function (err, cid) { assert.ifError(err); @@ -419,7 +419,7 @@ describe('Topic\'s', function () { }); it('should move a topic', function (done) { - socketTopics.move({ uid: 1 }, { cid: categoryObj.cid, tids: [newTopic.tid] }, function (err) { + socketTopics.move({ uid: adminUid }, { cid: categoryObj.cid, tids: [newTopic.tid] }, function (err) { assert.ifError(err); topics.getTopicField(newTopic.tid, 'cid', function (err, cid) { assert.ifError(err); @@ -543,8 +543,40 @@ describe('Topic\'s', function () { ], done); }); + it('should fail to purge topic if user does not have privilege', function (done) { + var globalModUid; + var tid; + async.waterfall([ + function (next) { + topics.post({ + uid: adminUid, + title: 'topic for purge test', + content: 'topic content', + cid: categoryObj.cid, + }, next); + }, + function (result, next) { + tid = result.topicData.tid; + User.create({ username: 'global mod' }, next); + }, + function (uid, next) { + globalModUid = uid; + groups.join('Global Moderators', uid, next); + }, + function (next) { + privileges.categories.rescind(['purge'], categoryObj.cid, 'Global Moderators', next); + }, + function (next) { + socketTopics.purge({ uid: globalModUid }, { tids: [tid], cid: categoryObj.cid }, function (err) { + assert.equal(err.message, '[[error:no-privileges]]'); + privileges.categories.give(['purge'], categoryObj.cid, 'Global Moderators', next); + }); + }, + ], done); + }); + it('should purge the topic', function (done) { - socketTopics.purge({ uid: 1 }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { + socketTopics.purge({ uid: adminUid }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) { assert.ifError(err); db.isSortedSetMember('uid:' + followerUid + ':followed_tids', newTopic.tid, function (err, isMember) { assert.ifError(err); @@ -588,7 +620,10 @@ describe('Topic\'s', function () { topics.tools.pin(tid1, adminUid, next); }, function (next) { - topics.tools.pin(tid2, adminUid, next); + // artificial timeout so pin time is different on redis sometimes scores are indentical + setTimeout(function () { + topics.tools.pin(tid2, adminUid, next); + }, 5); }, ], done); }); @@ -822,7 +857,7 @@ describe('Topic\'s', function () { }); it('should fail with invalid data', function (done) { - socketTopics.createTopicFromPosts({ uid: 1 }, null, function (err) { + socketTopics.createTopicFromPosts({ uid: adminUid }, null, function (err) { assert.equal(err.message, '[[error:invalid-data]]'); done(); }); @@ -939,8 +974,8 @@ describe('Topic\'s', function () { request(nconf.get('url') + '/api/topic/' + topicData.slug + '/-1', { json: true }, function (err, res, body) { assert.ifError(err); assert.equal(res.statusCode, 200); - assert.equal(res.headers['x-redirect'], '/topic/15/topic-for-controller-test'); - assert.equal(body, '/topic/15/topic-for-controller-test'); + assert.equal(res.headers['x-redirect'], '/topic/' + topicData.tid + '/topic-for-controller-test'); + assert.equal(body, '/topic/' + topicData.tid + '/topic-for-controller-test'); done(); }); });