diff --git a/public/language/en-GB/admin/manage/categories.json b/public/language/en-GB/admin/manage/categories.json index eef2e00529..68a79e78aa 100644 --- a/public/language/en-GB/admin/manage/categories.json +++ b/public/language/en-GB/admin/manage/categories.json @@ -47,6 +47,8 @@ "privileges.no-users": "No user-specific privileges in this category.", "privileges.section-group": "Group", "privileges.group-private": "This group is private", + "privileges.inheritance-exception": "This group does not inherit privileges from registered-users group", + "privileges.banned-user-inheritance": "Banned users inherit privileges from banned-users group", "privileges.search-group": "Add Group", "privileges.copy-to-children": "Copy to Children", "privileges.copy-from-category": "Copy from Category", diff --git a/public/language/en-GB/global.json b/public/language/en-GB/global.json index b5b9600449..639d45bb75 100644 --- a/public/language/en-GB/global.json +++ b/public/language/en-GB/global.json @@ -57,7 +57,10 @@ "alert.error": "Error", "alert.banned": "Banned", - "alert.banned.message": "You have just been banned, you will now be logged out.", + "alert.banned.message": "You have just been banned, your access is now restricted.", + + "alert.unbanned": "Unbanned", + "alert.unbanned.message": "Your ban is just lifted, you may continue participating in the forum as usual.", "alert.unfollow": "You are no longer following %1!", "alert.follow": "You are now following %1!", diff --git a/public/src/admin/manage/privileges.js b/public/src/admin/manage/privileges.js index 49757db41e..85da47f370 100644 --- a/public/src/admin/manage/privileges.js +++ b/public/src/admin/manage/privileges.js @@ -2,11 +2,12 @@ define('admin/manage/privileges', [ 'autocomplete', + 'bootbox', 'translator', 'categorySelector', 'mousetrap', 'admin/modules/checkboxRowSelector', -], function (autocomplete, translator, categorySelector, mousetrap, checkboxRowSelector) { +], function (autocomplete, bootbox, translator, categorySelector, mousetrap, checkboxRowSelector) { var Privileges = {}; var cid; @@ -38,6 +39,7 @@ define('admin/manage/privileges', [ var member = rowEl.attr('data-group-name') || rowEl.attr('data-uid'); var isPrivate = parseInt(rowEl.attr('data-private') || 0, 10); var isGroup = rowEl.attr('data-group-name') !== undefined; + var isBanned = (isGroup && rowEl.attr('data-group-name') === 'banned-users') || rowEl.attr('data-banned') !== undefined; var delta = checkboxEl.prop('checked') === (wrapperEl.attr('data-value') === 'true') ? null : state; if (member) { @@ -45,7 +47,7 @@ define('admin/manage/privileges', [ bootbox.confirm('[[admin/manage/privileges:alert.confirm-moderate]]', function (confirm) { if (confirm) { wrapperEl.attr('data-delta', delta); - Privileges.exposeAssumedPrivileges(); + Privileges.exposeAssumedPrivileges(isBanned); } else { checkboxEl.prop('checked', !checkboxEl.prop('checked')); } @@ -61,7 +63,7 @@ define('admin/manage/privileges', [ }); } else { wrapperEl.attr('data-delta', delta); - Privileges.exposeAssumedPrivileges(); + Privileges.exposeAssumedPrivileges(isBanned); } checkboxRowSelector.updateState(checkboxEl); } else { @@ -165,36 +167,27 @@ define('admin/manage/privileges', [ }); }; - Privileges.exposeAssumedPrivileges = function () { + Privileges.exposeAssumedPrivileges = function (isBanned) { /* If registered-users has a privilege enabled, then all users and groups of that privilege should be assumed to have that privilege as well, even if not set in the db, so reflect this arrangement in the table */ - var privs = []; - $('.privilege-table tr[data-group-name="registered-users"] td input[type="checkbox"]:not(.checkbox-helper)').parent().each(function (idx, el) { - if ($(el).find('input').prop('checked')) { - privs.push(el.getAttribute('data-privilege')); - } - }); - // Also apply to non-group privileges - privs = privs.concat(privs.map(function (priv) { - if (priv.startsWith('groups:')) { - return priv.slice(7); + // As such, individual banned users inherits privileges from banned-users group + // Running this block only when needed + if (isBanned === undefined || isBanned === true) { + const getBannedUsersInputSelector = (privs, i) => `.privilege-table tr[data-banned] td[data-privilege="${privs[i]}"] input`; + const bannedUsersPrivs = getPrivilegesFromRow('banned-users'); + applyPrivileges(bannedUsersPrivs, getBannedUsersInputSelector); + if (isBanned === true) { + return; } - - return false; - })).filter(Boolean); - - for (var x = 0, numPrivs = privs.length; x < numPrivs; x += 1) { - var inputs = $('.privilege-table tr[data-group-name]:not([data-group-name="registered-users"],[data-group-name="guests"],[data-group-name="spiders"]) td[data-privilege="' + privs[x] + '"] input, .privilege-table tr[data-uid] td[data-privilege="' + privs[x] + '"] input'); - inputs.each(function (idx, el) { - if (!el.checked) { - el.indeterminate = true; - } - }); } + + const getRegisteredUsersInputSelector = (privs, i) => `.privilege-table tr[data-group-name]:not([data-group-name="registered-users"],[data-group-name="banned-users"],[data-group-name="guests"],[data-group-name="spiders"]) td[data-privilege="${privs[i]}"] input, .privilege-table tr[data-uid]:not([data-banned]) td[data-privilege="${privs[i]}"] input`; + const registeredUsersPrivs = getPrivilegesFromRow('registered-users'); + applyPrivileges(registeredUsersPrivs, getRegisteredUsersInputSelector); }; Privileges.setPrivilege = function (member, privilege, state) { @@ -288,6 +281,37 @@ define('admin/manage/privileges', [ }); }; + function getPrivilegesFromRow(sourceGroupName) { + const privs = []; + $(`.privilege-table tr[data-group-name="${sourceGroupName}"] td input[type="checkbox"]:not(.checkbox-helper)`) + .parent() + .each(function (idx, el) { + if ($(el).find('input').prop('checked')) { + privs.push(el.getAttribute('data-privilege')); + } + }); + + // Also apply to non-group privileges + return privs.concat(privs.map(function (priv) { + if (priv.startsWith('groups:')) { + return priv.slice(7); + } + + return false; + })).filter(Boolean); + } + + function applyPrivileges(privs, inputSelectorFn) { + for (let x = 0, numPrivs = privs.length; x < numPrivs; x += 1) { + const inputs = $(inputSelectorFn(privs, x)); + inputs.each(function (idx, el) { + if (!el.checked) { + el.indeterminate = true; + } + }); + } + } + function hightlightRowByDataAttr(attrName, attrValue) { if (attrValue) { var el = $('[' + attrName + ']').filter(function () { @@ -363,6 +387,7 @@ define('admin/manage/privileges', [ { picture: user.picture, username: user.username, + banned: user.banned, uid: user.uid, 'icon:text': user['icon:text'], 'icon:bgColor': user['icon:bgColor'], diff --git a/public/src/modules/autocomplete.js b/public/src/modules/autocomplete.js index 404abcc219..7aa1d7744e 100644 --- a/public/src/modules/autocomplete.js +++ b/public/src/modules/autocomplete.js @@ -41,6 +41,7 @@ define('autocomplete', ['api'], function (api) { username: user.username, userslug: user.userslug, picture: user.picture, + banned: user.banned, 'icon:text': user['icon:text'], 'icon:bgColor': user['icon:bgColor'], }, diff --git a/public/src/sockets.js b/public/src/sockets.js index 80b639b1d1..6c08e91935 100644 --- a/public/src/sockets.js +++ b/public/src/sockets.js @@ -83,6 +83,7 @@ socket = window.socket; }); socket.on('event:banned', onEventBanned); + socket.on('event:unbanned', onEventUnbanned); socket.on('event:logout', function () { app.logout(); }); @@ -214,6 +215,17 @@ socket = window.socket; }); } + function onEventUnbanned() { + bootbox.alert({ + title: '[[global:alert.unbanned]]', + message: '[[global:alert.unbanned.message]]', + closeButton: false, + callback: function () { + window.location.href = config.relative_path + '/'; + }, + }); + } + if ( config.socketioOrigins && config.socketioOrigins !== '*:*' && diff --git a/src/api/users.js b/src/api/users.js index d65f450d29..f159ed226a 100644 --- a/src/api/users.js +++ b/src/api/users.js @@ -200,7 +200,10 @@ usersAPI.ban = async function (caller, data) { until: data.until > 0 ? data.until : undefined, reason: data.reason || undefined, }); - await user.auth.revokeAllSessions(data.uid); + const canLoginIfBanned = await user.bans.canLoginIfBanned(data.uid); + if (!canLoginIfBanned) { + await user.auth.revokeAllSessions(data.uid); + } }; usersAPI.unban = async function (caller, data) { @@ -209,6 +212,9 @@ usersAPI.unban = async function (caller, data) { } await user.bans.unban(data.uid); + + sockets.in('uid_' + data.uid).emit('event:unbanned'); + await events.log({ type: 'user-unban', uid: caller.uid, diff --git a/src/controllers/accounts/helpers.js b/src/controllers/accounts/helpers.js index 08a9fd260d..9932707914 100644 --- a/src/controllers/accounts/helpers.js +++ b/src/controllers/accounts/helpers.js @@ -91,7 +91,6 @@ helpers.getUserDataByUserSlug = async function (userslug, callerUID) { }); userData.sso = results.sso.associations; - userData.banned = userData.banned === 1; userData.website = validator.escape(String(userData.website || '')); userData.websiteLink = !userData.website.startsWith('http') ? 'http://' + userData.website : userData.website; userData.websiteName = userData.website.replace(validator.escape('http://'), '').replace(validator.escape('https://'), ''); diff --git a/src/controllers/admin/groups.js b/src/controllers/admin/groups.js index 2f557a87a9..cf46a6bdaa 100644 --- a/src/controllers/admin/groups.js +++ b/src/controllers/admin/groups.js @@ -41,7 +41,7 @@ groupsController.get = async function (req, res, next) { categories.buildForSelectAll(), ]); - if (!group) { + if (!group || groupName === groups.BANNED_USERS) { return next(); } group.isOwner = true; @@ -69,6 +69,7 @@ async function getGroupNames() { return groupNames.filter(name => name !== 'registered-users' && name !== 'verified-users' && name !== 'unverified-users' && + name !== groups.BANNED_USERS && !groups.isPrivilegeGroup(name) ); } diff --git a/src/controllers/authentication.js b/src/controllers/authentication.js index 53de38b08c..5c16fc4022 100644 --- a/src/controllers/authentication.js +++ b/src/controllers/authentication.js @@ -382,24 +382,25 @@ authenticationController.localLogin = async function (req, username, password, n const userslug = slugify(username); const uid = await user.getUidByUserslug(userslug); try { - const [userData, isAdminOrGlobalMod, banned, hasLoginPrivilege] = await Promise.all([ + const [userData, isAdminOrGlobalMod, canLoginIfBanned] = await Promise.all([ user.getUserFields(uid, ['uid', 'passwordExpiry']), user.isAdminOrGlobalMod(uid), - user.bans.isBanned(uid), - privileges.global.can('local:login', uid), + user.bans.canLoginIfBanned(uid), ]); userData.isAdminOrGlobalMod = isAdminOrGlobalMod; - if (parseInt(uid, 10) && !hasLoginPrivilege) { - return next(new Error('[[error:local-login-disabled]]')); - } - - if (banned) { + if (!canLoginIfBanned) { const banMesage = await getBanInfo(uid); return next(new Error(banMesage)); } + // Doing this after the ban check, because user's privileges might change after a ban expires + const hasLoginPrivilege = await privileges.global.can('local:login', uid); + if (parseInt(uid, 10) && !hasLoginPrivilege) { + return next(new Error('[[error:local-login-disabled]]')); + } + const passwordMatch = await user.isPasswordCorrect(uid, password, req.ip); if (!passwordMatch) { return next(new Error('[[error:invalid-login-credentials]]')); diff --git a/src/groups/index.js b/src/groups/index.js index f98ef0e7a8..3ea83f9c4d 100644 --- a/src/groups/index.js +++ b/src/groups/index.js @@ -23,6 +23,7 @@ require('./join')(Groups); require('./leave')(Groups); require('./cache')(Groups); +Groups.BANNED_USERS = 'banned-users'; Groups.ephemeralGroups = ['guests', 'spiders']; @@ -30,6 +31,7 @@ Groups.systemGroups = [ 'registered-users', 'verified-users', 'unverified-users', + Groups.BANNED_USERS, 'administrators', 'Global Moderators', ]; diff --git a/src/groups/join.js b/src/groups/join.js index b2e1edc153..9107ce6146 100644 --- a/src/groups/join.js +++ b/src/groups/join.js @@ -90,7 +90,7 @@ module.exports = function (Groups) { } async function setGroupTitleIfNotSet(groupNames, uid) { - const ignore = ['registered-users', 'verified-users', 'unverified-users']; + const ignore = ['registered-users', 'verified-users', 'unverified-users', Groups.BANNED_USERS]; groupNames = groupNames.filter( groupName => !ignore.includes(groupName) && !Groups.isPrivilegeGroup(groupName) ); diff --git a/src/groups/search.js b/src/groups/search.js index cab203b9b3..3cda01a30f 100644 --- a/src/groups/search.js +++ b/src/groups/search.js @@ -13,7 +13,9 @@ module.exports = function (Groups) { if (!options.hideEphemeralGroups) { groupNames = Groups.ephemeralGroups.concat(groupNames); } - groupNames = groupNames.filter(name => name.toLowerCase().includes(query) && !Groups.isPrivilegeGroup(name)); + groupNames = groupNames.filter(name => name.toLowerCase().includes(query) && + name !== Groups.BANNED_USERS && // hide banned-users in searches + !Groups.isPrivilegeGroup(name)); groupNames = groupNames.slice(0, 100); let groupsData; diff --git a/src/messaging/index.js b/src/messaging/index.js index f5202d236f..fc02f7b217 100644 --- a/src/messaging/index.js +++ b/src/messaging/index.js @@ -202,10 +202,6 @@ Messaging.canMessageUser = async (uid, toUid) => { throw new Error('[[error:no-user]]'); } - const userData = await user.getUserFields(uid, ['banned']); - if (userData.banned) { - throw new Error('[[error:user-banned]]'); - } const canChat = await privileges.global.can('chat', uid); if (!canChat) { throw new Error('[[error:no-privileges]]'); @@ -238,10 +234,6 @@ Messaging.canMessageRoom = async (uid, roomId) => { throw new Error('[[error:not-in-room]]'); } - const userData = await user.getUserFields(uid, ['banned']); - if (userData.banned) { - throw new Error('[[error:user-banned]]'); - } const canChat = await privileges.global.can('chat', uid); if (!canChat) { throw new Error('[[error:no-privileges]]'); diff --git a/src/middleware/header.js b/src/middleware/header.js index c2f6594e80..0de5b79470 100644 --- a/src/middleware/header.js +++ b/src/middleware/header.js @@ -32,13 +32,13 @@ const relative_path = nconf.get('relative_path'); middleware.buildHeader = helpers.try(async function buildHeader(req, res, next) { res.locals.renderHeader = true; res.locals.isAPI = false; - const [config, isBanned] = await Promise.all([ + const [config, canLoginIfBanned] = await Promise.all([ controllers.api.loadConfig(req), - user.bans.isBanned(req.uid), + user.bans.canLoginIfBanned(req.uid), plugins.hooks.fire('filter:middleware.buildHeader', { req: req, locals: res.locals }), ]); - if (isBanned) { + if (!canLoginIfBanned && req.loggedIn) { req.logout(); return res.redirect('/'); } diff --git a/src/privileges/helpers.js b/src/privileges/helpers.js index 7c85598159..ca2b0e0788 100644 --- a/src/privileges/helpers.js +++ b/src/privileges/helpers.js @@ -110,7 +110,7 @@ helpers.getUserPrivileges = async function (cid, userPrivileges) { }); const members = _.uniq(_.flatten(memberSets)); - const memberData = await user.getUsersFields(members, ['picture', 'username']); + const memberData = await user.getUsersFields(members, ['picture', 'username', 'banned']); memberData.forEach(function (member) { member.privileges = {}; @@ -133,6 +133,7 @@ helpers.getGroupPrivileges = async function (cid, groupPrivileges) { let groupNames = allGroupNames.filter(groupName => !groupName.includes(':privileges:') && uniqueGroups.includes(groupName)); groupNames = groups.ephemeralGroups.concat(groupNames); + moveToFront(groupNames, groups.BANNED_USERS); moveToFront(groupNames, 'Global Moderators'); moveToFront(groupNames, 'unverified-users'); moveToFront(groupNames, 'verified-users'); diff --git a/src/socket.io/admin/user.js b/src/socket.io/admin/user.js index 8e1c91a2ab..a6628fdbb9 100644 --- a/src/socket.io/admin/user.js +++ b/src/socket.io/admin/user.js @@ -18,12 +18,10 @@ User.makeAdmins = async function (socket, uids) { if (!Array.isArray(uids)) { throw new Error('[[error:invalid-data]]'); } - const userData = await user.getUsersFields(uids, ['banned']); - userData.forEach((userData) => { - if (userData && userData.banned) { - throw new Error('[[error:cant-make-banned-users-admin]]'); - } - }); + const isMembersOfBanned = await groups.isMembers(uids, groups.BANNED_USERS); + if (isMembersOfBanned.includes(true)) { + throw new Error('[[error:cant-make-banned-users-admin]]'); + } for (const uid of uids) { /* eslint-disable no-await-in-loop */ await groups.join('administrators', uid); diff --git a/src/upgrades/1.16.0/banned_users_group.js b/src/upgrades/1.16.0/banned_users_group.js new file mode 100644 index 0000000000..0a6b690402 --- /dev/null +++ b/src/upgrades/1.16.0/banned_users_group.js @@ -0,0 +1,63 @@ +'use strict'; + +const batch = require('../../batch'); +const db = require('../../database'); +const groups = require('../../groups'); + +const now = Date.now(); + +module.exports = { + name: 'Move banned users to banned-users group', + timestamp: Date.UTC(2020, 11, 13), + method: async function () { + const progress = this.progress; + const timestamp = await db.getObjectField('group:administrators', 'timestamp'); + const bannedExists = await groups.exists('banned-users'); + if (!bannedExists) { + await groups.create({ + name: 'banned-users', + hidden: 1, + private: 1, + system: 1, + disableLeave: 1, + disableJoinRequests: 1, + timestamp: timestamp + 1, + }); + } + + await batch.processSortedSet('users:banned', async function (uids) { + progress.incr(uids.length); + + await db.sortedSetAdd( + 'group:banned-users:members', + uids.map(() => now), + uids.map(uid => uid) + ); + + await db.sortedSetRemove( + [ + 'group:registered-users:members', + 'group:verified-users:members', + 'group:unverified-users:members', + 'group:Global Moderators:members', + ], + uids.map(uid => uid) + ); + }, { + batch: 500, + progress: this.progress, + }); + + + const bannedCount = await db.sortedSetCard('group:banned-users:members'); + const registeredCount = await db.sortedSetCard('group:registered-users:members'); + const verifiedCount = await db.sortedSetCard('group:verified-users:members'); + const unverifiedCount = await db.sortedSetCard('group:unverified-users:members'); + const globalModCount = await db.sortedSetCard('group:Global Moderators:members'); + await db.setObjectField('group:banned-users', 'memberCount', bannedCount); + await db.setObjectField('group:registered-users', 'memberCount', registeredCount); + await db.setObjectField('group:verified-users', 'memberCount', verifiedCount); + await db.setObjectField('group:unverified-users', 'memberCount', unverifiedCount); + await db.setObjectField('group:Global Moderators', 'memberCount', globalModCount); + }, +}; diff --git a/src/user/bans.js b/src/user/bans.js index 617ecce3bc..424d120cac 100644 --- a/src/user/bans.js +++ b/src/user/bans.js @@ -5,10 +5,14 @@ const winston = require('winston'); const meta = require('../meta'); const emailer = require('../emailer'); const db = require('../database'); +const groups = require('../groups'); +const privileges = require('../privileges'); module.exports = function (User) { User.bans = {}; + const systemGroups = groups.systemGroups.filter(group => group !== groups.BANNED_USERS); + User.bans.ban = async function (uid, until, reason) { // "until" (optional) is unix timestamp in milliseconds // "reason" (optional) is a string @@ -32,7 +36,9 @@ module.exports = function (User) { banData.reason = reason; } - await User.setUserField(uid, 'banned', 1); + // Leaving all other system groups to have privileges constrained to the "banned-users" group + await groups.leave(systemGroups, uid); + await groups.join(groups.BANNED_USERS, uid); await db.sortedSetAdd('users:banned', now, uid); await db.sortedSetAdd('uid:' + uid + ':bans:timestamp', now, banKey); await db.setObject(banKey, banData); @@ -59,10 +65,20 @@ module.exports = function (User) { }; User.bans.unban = async function (uids) { - if (Array.isArray(uids)) { - await db.setObject(uids.map(uid => 'user:' + uid), { banned: 0, 'banned:expire': 0 }); - } else { - await User.setUserFields(uids, { banned: 0, 'banned:expire': 0 }); + uids = Array.isArray(uids) ? uids : [uids]; + const userData = await User.getUsersFields(uids, ['email:confirmed']); + + await db.setObject(uids.map(uid => 'user:' + uid), { 'banned:expire': 0 }); + + /* eslint-disable no-await-in-loop */ + for (const user of userData) { + const systemGroupsToJoin = [ + 'registered-users', + (parseInt(user['email:confirmed'], 10) === 1 ? 'verified-users' : 'unverified-users'), + ]; + await groups.leave(groups.BANNED_USERS, user.uid); + // An unbanned user would lost its previous "Global Moderator" status + await groups.join(systemGroupsToJoin, user.uid); } await db.sortedSetRemove(['users:banned', 'users:banned:expire'], uids); @@ -75,22 +91,39 @@ module.exports = function (User) { return isArray ? result.map(r => r.banned) : result[0].banned; }; + User.bans.canLoginIfBanned = async function (uid) { + let canLogin = true; + + const banned = (await User.bans.unbanIfExpired([uid]))[0].banned; + // Group privilege overshadows individual one + if (banned) { + canLogin = await privileges.global.canGroup('local:login', groups.BANNED_USERS); + } + if (banned && !canLogin) { + // Checking a single privilege of user + canLogin = await groups.isMember(uid, 'cid:0:privileges:local:login'); + } + + return canLogin; + }; + User.bans.unbanIfExpired = async function (uids) { // loading user data will unban if it has expired -barisu - const userData = await User.getUsersFields(uids, ['banned', 'banned:expire']); + const userData = await User.getUsersFields(uids, ['banned:expire']); return User.bans.calcExpiredFromUserData(userData); }; - User.bans.calcExpiredFromUserData = function (userData) { + User.bans.calcExpiredFromUserData = async function (userData) { const isArray = Array.isArray(userData); userData = isArray ? userData : [userData]; - userData = userData.map(function (userData) { + userData = await Promise.all(userData.map(async function (userData) { + const banned = await groups.isMember(userData.uid, groups.BANNED_USERS); return { - banned: userData && !!userData.banned, + banned: banned, 'banned:expire': userData && userData['banned:expire'], banExpired: userData && userData['banned:expire'] <= Date.now() && userData['banned:expire'] !== 0, }; - }); + })); return isArray ? userData : userData[0]; }; diff --git a/src/user/data.js b/src/user/data.js index 6bc1f9055b..dee2f5a7a0 100644 --- a/src/user/data.js +++ b/src/user/data.js @@ -219,7 +219,8 @@ module.exports = function (User) { } if (user.hasOwnProperty('banned') || user.hasOwnProperty('banned:expire')) { - const result = User.bans.calcExpiredFromUserData(user); + const result = await User.bans.calcExpiredFromUserData(user); + user.banned = result.banned; const unban = result.banned && result.banExpired; user.banned_until = unban ? 0 : user['banned:expire']; user.banned_until_readable = user.banned_until && !unban ? utils.toISOString(user.banned_until) : 'Not Banned'; diff --git a/src/user/search.js b/src/user/search.js index c7f3c7897e..2201e204ca 100644 --- a/src/user/search.js +++ b/src/user/search.js @@ -12,8 +12,6 @@ const utils = require('../utils'); module.exports = function (User) { const filterFnMap = { online: user => user.status !== 'offline' && (Date.now() - user.lastonline < 300000), - banned: user => user.banned, - notbanned: user => !user.banned, flagged: user => parseInt(user.flags, 10) > 0, verified: user => !!user['email:confirmed'], unverified: user => !user['email:confirmed'], @@ -21,8 +19,6 @@ module.exports = function (User) { const filterFieldMap = { online: ['status', 'lastonline'], - banned: ['banned'], - notbanned: ['banned'], flagged: ['flags'], verified: ['email:confirmed'], unverified: ['email:confirmed'], @@ -111,6 +107,12 @@ module.exports = function (User) { return uids; } + if (filters.includes('banned') || filters.includes('notbanned')) { + const isMembersOfBanned = await groups.isMembers(uids, groups.BANNED_USERS); + const checkBanned = filters.includes('banned'); + uids = uids.filter((uid, index) => (checkBanned ? isMembersOfBanned[index] : !isMembersOfBanned[index])); + } + fields.push('uid'); let userData = await User.getUsersFields(uids, fields); diff --git a/src/views/admin/partials/privileges/category.tpl b/src/views/admin/partials/privileges/category.tpl index 8cc7af2de0..6e75d5ed5e 100644 --- a/src/views/admin/partials/privileges/category.tpl +++ b/src/views/admin/partials/privileges/category.tpl @@ -30,9 +30,13 @@ - - - + {{{ if privileges.groups.isPrivate }}} + {{{ if (privileges.groups.name == "banned-users") }}} + + {{{ else }}} + + {{{ end }}} + {{{ end }}} {privileges.groups.name} @@ -109,7 +113,7 @@ - + @@ -117,7 +121,12 @@
{../icon:text}
- {privileges.users.username} + + {{{ if privileges.users.banned }}} + + {{{ end }}} + {privileges.users.username} + {function.spawnPrivilegeStates, privileges.users.username, ../privileges} diff --git a/src/views/admin/partials/privileges/global.tpl b/src/views/admin/partials/privileges/global.tpl index ba2106230b..ed462a7ff1 100644 --- a/src/views/admin/partials/privileges/global.tpl +++ b/src/views/admin/partials/privileges/global.tpl @@ -13,9 +13,13 @@ - - - + {{{ if privileges.groups.isPrivate }}} + {{{ if (privileges.groups.name == "banned-users") }}} + + {{{ else }}} + + {{{ end }}} + {{{ end }}} {privileges.groups.name} @@ -55,7 +59,7 @@ - + @@ -63,7 +67,12 @@
{../icon:text}
- {privileges.users.username} + + {{{ if privileges.users.banned }}} + + {{{ end }}} + {privileges.users.username} + {function.spawnPrivilegeStates, privileges.users.username, ../privileges} diff --git a/test/authentication.js b/test/authentication.js index b2f9ff90ba..4698cb2bcd 100644 --- a/test/authentication.js +++ b/test/authentication.js @@ -2,9 +2,10 @@ var assert = require('assert'); +var async = require('async'); var nconf = require('nconf'); var request = require('request'); -var async = require('async'); +const util = require('util'); var db = require('./mocks/databasemock'); var user = require('../src/user'); @@ -40,6 +41,7 @@ describe('authentication', function () { }); }); } + const loginUserPromisified = util.promisify(loginUser); function registerUser(email, username, password, callback) { var jar = request.jar(); @@ -453,21 +455,30 @@ describe('authentication', function () { }); }); - it('should prevent banned user from logging in', function (done) { - user.create({ username: 'banme', password: '123456', email: 'ban@me.com' }, function (err, uid) { - assert.ifError(err); - user.bans.ban(uid, 0, 'spammer', function (err) { + describe('banned user authentication', function () { + const bannedUser = { + username: 'banme', + pw: '123456', + uid: null, + }; + + before(async function () { + bannedUser.uid = await user.create({ username: 'banme', password: '123456', email: 'ban@me.com' }); + }); + + it('should prevent banned user from logging in', function (done) { + user.bans.ban(bannedUser.uid, 0, 'spammer', function (err) { assert.ifError(err); - loginUser('banme', '123456', function (err, res, body) { + loginUser(bannedUser.username, bannedUser.pw, function (err, res, body) { assert.ifError(err); assert.equal(res.statusCode, 403); assert.equal(body, '[[error:user-banned-reason, spammer]]'); - user.bans.unban(uid, function (err) { + user.bans.unban(bannedUser.uid, function (err) { assert.ifError(err); var expiry = Date.now() + 10000; - user.bans.ban(uid, expiry, '', function (err) { + user.bans.ban(bannedUser.uid, expiry, '', function (err) { assert.ifError(err); - loginUser('banme', '123456', function (err, res, body) { + loginUser(bannedUser.username, bannedUser.pw, function (err, res, body) { assert.ifError(err); assert.equal(res.statusCode, 403); assert.equal(body, '[[error:user-banned-reason-until, ' + utils.toISOString(expiry) + ', No reason given.]]'); @@ -478,6 +489,19 @@ describe('authentication', function () { }); }); }); + + it('should allow banned user to log in if the "banned-users" group has "local-login" privilege', async function () { + await privileges.global.give(['groups:local:login'], 'banned-users'); + const res = await loginUserPromisified(bannedUser.username, bannedUser.pw); + assert.strictEqual(res.statusCode, 200); + }); + + it('should allow banned user to log in if the user herself has "local-login" privilege', async function () { + await privileges.global.rescind(['groups:local:login'], 'banned-users'); + await privileges.categories.give(['local:login'], 0, bannedUser.uid); + const res = await loginUserPromisified(bannedUser.username, bannedUser.pw); + assert.strictEqual(res.statusCode, 200); + }); }); it('should lockout account on 3 failed login attempts', function (done) { diff --git a/test/user.js b/test/user.js index 303afdcb20..6492e44847 100644 --- a/test/user.js +++ b/test/user.js @@ -80,7 +80,7 @@ describe('User', function () { assert.strictEqual(data.postcount, 0); assert.strictEqual(data.topiccount, 0); assert.strictEqual(data.lastposttime, 0); - assert.strictEqual(data.banned, 0); + assert.strictEqual(data.banned, false); }); it('should have a valid email, if using an email', function (done) { @@ -441,15 +441,18 @@ describe('User', function () { it('should filter users', function (done) { User.create({ username: 'ipsearch_filter' }, function (err, uid) { assert.ifError(err); - User.setUserFields(uid, { banned: 1, flags: 10 }, function (err) { + User.bans.ban(uid, 0, '', function (err) { assert.ifError(err); - socketUser.search({ uid: adminUid }, { - query: 'ipsearch', - filters: ['online', 'banned', 'flagged'], - }, function (err, data) { + User.setUserFields(uid, { flags: 10 }, function (err) { assert.ifError(err); - assert.equal(data.users[0].username, 'ipsearch_filter'); - done(); + socketUser.search({ uid: adminUid }, { + query: 'ipsearch', + filters: ['online', 'banned', 'flagged'], + }, function (err, data) { + assert.ifError(err); + assert.equal(data.users[0].username, 'ipsearch_filter'); + done(); + }); }); }); }); @@ -1303,6 +1306,16 @@ describe('User', function () { }); describe('user info', function () { + let testUserUid; + let verifiedTestUserUid; + + before(async function () { + // Might be the first user thus a verified one if this test part is ran alone + verifiedTestUserUid = await User.create({ username: 'bannedUser', password: '123456', email: 'banneduser@example.com' }); + await User.setUserField(verifiedTestUserUid, 'email:confirmed', 1); + testUserUid = await User.create({ username: 'bannedUser2', password: '123456', email: 'banneduser2@example.com' }); + }); + it('should return error if there is no ban reason', function (done) { User.getLatestBanInfo(123, function (err) { assert.equal(err.message, 'no-ban-info'); @@ -1310,11 +1323,10 @@ describe('User', function () { }); }); - it('should get history from set', async function () { const now = Date.now(); - await db.sortedSetAdd('user:' + testUid + ':usernames', now, 'derp:' + now); - const data = await User.getHistory('user:' + testUid + ':usernames'); + await db.sortedSetAdd('user:' + testUserUid + ':usernames', now, 'derp:' + now); + const data = await User.getHistory('user:' + testUserUid + ':usernames'); assert.equal(data[0].value, 'derp'); assert.equal(data[0].timestamp, now); }); @@ -1322,13 +1334,13 @@ describe('User', function () { it('should return the correct ban reason', function (done) { async.series([ function (next) { - User.bans.ban(testUid, 0, '', function (err) { + User.bans.ban(testUserUid, 0, '', function (err) { assert.ifError(err); next(err); }); }, function (next) { - User.getModerationHistory(testUid, function (err, data) { + User.getModerationHistory(testUserUid, function (err, data) { assert.ifError(err); assert.equal(data.bans.length, 1, 'one ban'); assert.equal(data.bans[0].reason, '[[user:info.banned-no-reason]]', 'no ban reason'); @@ -1338,7 +1350,7 @@ describe('User', function () { }, ], function (err) { assert.ifError(err); - User.bans.unban(testUid, function (err) { + User.bans.unban(testUserUid, function (err) { assert.ifError(err); done(); }); @@ -1346,28 +1358,28 @@ describe('User', function () { }); it('should ban user permanently', function (done) { - User.bans.ban(testUid, function (err) { + User.bans.ban(testUserUid, function (err) { assert.ifError(err); - User.bans.isBanned(testUid, function (err, isBanned) { + User.bans.isBanned(testUserUid, function (err, isBanned) { assert.ifError(err); assert.equal(isBanned, true); - User.bans.unban(testUid, done); + User.bans.unban(testUserUid, done); }); }); }); it('should ban user temporarily', function (done) { - User.bans.ban(testUid, Date.now() + 2000, function (err) { + User.bans.ban(testUserUid, Date.now() + 2000, function (err) { assert.ifError(err); - User.bans.isBanned(testUid, function (err, isBanned) { + User.bans.isBanned(testUserUid, function (err, isBanned) { assert.ifError(err); assert.equal(isBanned, true); setTimeout(function () { - User.bans.isBanned(testUid, function (err, isBanned) { + User.bans.isBanned(testUserUid, function (err, isBanned) { assert.ifError(err); assert.equal(isBanned, false); - User.bans.unban(testUid, done); + User.bans.unban(testUserUid, done); }); }, 3000); }); @@ -1375,11 +1387,49 @@ describe('User', function () { }); it('should error if until is NaN', function (done) { - User.bans.ban(testUid, 'asd', function (err) { + User.bans.ban(testUserUid, 'asd', function (err) { assert.equal(err.message, '[[error:ban-expiry-missing]]'); done(); }); }); + + it('should be member of "banned-users" system group only after a ban', async function () { + await User.bans.ban(testUserUid); + + const systemGroups = groups.systemGroups.filter(group => group !== groups.BANNED_USERS); + const isMember = await groups.isMember(testUserUid, groups.BANNED_USERS); + const isMemberOfAny = await groups.isMemberOfAny(testUserUid, systemGroups); + + assert.strictEqual(isMember, true); + assert.strictEqual(isMemberOfAny, false); + }); + + it('should restore system group memberships after an unban (for an unverified user)', async function () { + await User.bans.unban(testUserUid); + + const isMemberOfGroups = await groups.isMemberOfGroups(testUserUid, groups.systemGroups); + const membership = new Map(groups.systemGroups.map((item, index) => [item, isMemberOfGroups[index]])); + + assert.strictEqual(membership.get('registered-users'), true); + assert.strictEqual(membership.get('verified-users'), false); + assert.strictEqual(membership.get('unverified-users'), true); + assert.strictEqual(membership.get(groups.BANNED_USERS), false); + // administrators cannot be banned + assert.strictEqual(membership.get('administrators'), false); + // This will not restored + assert.strictEqual(membership.get('Global Moderators'), false); + }); + + it('should restore system group memberships after an unban (for a verified user)', async function () { + await User.bans.ban(verifiedTestUserUid); + await User.bans.unban(verifiedTestUserUid); + + const isMemberOfGroups = await groups.isMemberOfGroups(verifiedTestUserUid, groups.systemGroups); + const membership = new Map(groups.systemGroups.map((item, index) => [item, isMemberOfGroups[index]])); + + assert.strictEqual(membership.get('verified-users'), true); + assert.strictEqual(membership.get('unverified-users'), false); + }); }); describe('Digest.getSubscribers', function (done) {