feat: banned-users group

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

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!",

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'],

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

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 !== '*:*' &&

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,

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://'), '');

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)
 	);
 }

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]]'));

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',
 ];

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

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;

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]]');

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('/');
 	}

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');

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

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);
+	},
+};

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];
 	};
 

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

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

src/views/admin/partials/privileges/category.tpl

@@ -30,9 +30,13 @@
 							<!-- BEGIN privileges.groups -->
 							<tr data-group-name="{privileges.groups.nameEscaped}" data-private="<!-- IF privileges.groups.isPrivate -->1<!-- ELSE -->0<!-- ENDIF privileges.groups.isPrivate -->">
 								<td>
-									<!-- IF privileges.groups.isPrivate -->
-									<i class="fa fa-lock text-muted" title="[[admin/manage/categories:privileges.group-private]]"></i>
-									<!-- ENDIF privileges.groups.isPrivate -->
+									{{{ if privileges.groups.isPrivate }}}
+										{{{ if (privileges.groups.name == "banned-users") }}}
+										<i class="fa fa-exclamation-triangle text-muted" title="[[admin/manage/categories:privileges.inheritance-exception]]"></i>
+										{{{ else }}}
+										<i class="fa fa-lock text-muted" title="[[admin/manage/categories:privileges.group-private]]"></i>
+										{{{ end }}}
+									{{{ end }}}
 									{privileges.groups.name}
 								</td>
 								<td>
@@ -109,7 +113,7 @@
 						</thead>
 						<tbody>
 							<!-- BEGIN privileges.users -->
-							<tr data-uid="{privileges.users.uid}">
+							<tr data-uid="{privileges.users.uid}"{{{ if privileges.users.banned }}} data-banned{{{ end }}}>
 								<td>
 									<!-- IF ../picture -->
 									<img class="avatar avatar-sm" src="{privileges.users.picture}" title="{privileges.users.username}" />
@@ -117,7 +121,12 @@
 									<div class="avatar avatar-sm" style="background-color: {../icon:bgColor};">{../icon:text}</div>
 									<!-- ENDIF ../picture -->
 								</td>
-								<td>{privileges.users.username}</td>
+								<td>
+									{{{ if privileges.users.banned }}}
+									<i class="ban fa fa-gavel text-danger" title="[[admin/manage/categories:privileges.banned-user-inheritance]]"></i>
+									{{{ end }}}
+									{privileges.users.username}
+								</td>
 								<td class="text-center"><input autocomplete="off" type="checkbox" class="checkbox-helper"></td>
 								{function.spawnPrivilegeStates, privileges.users.username, ../privileges}
 							</tr>

src/views/admin/partials/privileges/global.tpl

@@ -13,9 +13,13 @@
 							<!-- BEGIN privileges.groups -->
 							<tr data-group-name="{privileges.groups.nameEscaped}" data-private="<!-- IF privileges.groups.isPrivate -->1<!-- ELSE -->0<!-- ENDIF privileges.groups.isPrivate -->">
 								<td>
-									<!-- IF privileges.groups.isPrivate -->
-									<i class="fa fa-lock text-muted" title="[[admin/manage/categories:privileges.group-private]]"></i>
-									<!-- ENDIF privileges.groups.isPrivate -->
+									{{{ if privileges.groups.isPrivate }}}
+										{{{ if (privileges.groups.name == "banned-users") }}}
+										<i class="fa fa-exclamation-triangle text-muted" title="[[admin/manage/categories:privileges.inheritance-exception]]"></i>
+										{{{ else }}}
+										<i class="fa fa-lock text-muted" title="[[admin/manage/categories:privileges.group-private]]"></i>
+										{{{ end }}}
+									{{{ end }}}
 									{privileges.groups.name}
 								</td>
 								<td></td>
@@ -55,7 +59,7 @@
 						</thead>
 						<tbody>
 							<!-- BEGIN privileges.users -->
-							<tr data-uid="{privileges.users.uid}">
+							<tr data-uid="{privileges.users.uid}"{{{ if privileges.users.banned }}} data-banned{{{ end }}}>
 								<td>
 									<!-- IF ../picture -->
 									<img class="avatar avatar-sm" src="{privileges.users.picture}" title="{privileges.users.username}" />
@@ -63,7 +67,12 @@
 									<div class="avatar avatar-sm" style="background-color: {../icon:bgColor};">{../icon:text}</div>
 									<!-- ENDIF ../picture -->
 								</td>
-								<td>{privileges.users.username}</td>
+								<td>
+									{{{ if privileges.users.banned }}}
+										<i class="ban fa fa-gavel text-danger" title="[[admin/manage/categories:privileges.banned-user-inheritance]]"></i>
+									{{{ end }}}
+									{privileges.users.username}
+								</td>
 								<td class="text-center"><input autocomplete="off" type="checkbox" class="checkbox-helper"></td>
 								{function.spawnPrivilegeStates, privileges.users.username, ../privileges}
 							</tr>

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

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