Merge branch 'master' of https://github.com/NodeBB/NodeBB

README.md

@@ -1,4 +1,4 @@
-# <img alt="NodeBB" src="http://i.imgur.com/mYxPPtB.png" />
+# <img alt="NodeBB" src="https://github.com/NodeBB/assets/raw/master/nodebb-svg/nodebb.svg" />
 
 [![Build Status](https://travis-ci.org/NodeBB/NodeBB.svg?branch=master)](https://travis-ci.org/NodeBB/NodeBB)
 [![Coverage Status](https://coveralls.io/repos/github/NodeBB/NodeBB/badge.svg?branch=master)](https://coveralls.io/github/NodeBB/NodeBB?branch=master)

src/api/users.js

@@ -1,5 +1,6 @@
 'use strict';
 
+const db = require('../database');
 const user = require('../user');
 const groups = require('../groups');
 const meta = require('../meta');
@@ -8,6 +9,8 @@ const privileges = require('../privileges');
 const notifications = require('../notifications');
 const plugins = require('../plugins');
 const events = require('../events');
+const translator = require('../translator');
+const sockets = require('../socket.io');
 
 const usersAPI = module.exports;
 
@@ -20,25 +23,28 @@ usersAPI.create = async function (caller, data) {
 };
 
 usersAPI.update = async function (caller, data) {
-	if (!data || !data.uid) {
-		throw new Error('[[error:invalid-data]]');
-	}
 	if (!caller.uid) {
 		throw new Error('[[error:invalid-uid]]');
 	}
+
+	if (!data || !data.uid) {
+		throw new Error('[[error:invalid-data]]');
+	}
+
 	const oldUserData = await user.getUserFields(data.uid, ['email', 'username']);
 	if (!oldUserData || !oldUserData.username) {
 		throw new Error('[[error:invalid-data]]');
 	}
 
-	const [isAdminOrGlobalMod, canEdit, passwordMatch] = await Promise.all([
+	const [isAdminOrGlobalMod, canEdit, hasPassword, passwordMatch] = await Promise.all([
 		user.isAdminOrGlobalMod(caller.uid),
 		privileges.users.canEdit(caller.uid, data.uid),
+		user.hasPassword(data.uid),
 		data.password ? user.isPasswordCorrect(data.uid, data.password, caller.ip) : false,
 	]);
 
 	// Changing own email/username requires password confirmation
-	if (['email', 'username'].some(prop => Object.keys(data).includes(prop)) && !isAdminOrGlobalMod && caller.uid === data.uid && !passwordMatch) {
+	if (['email', 'username'].some(prop => Object.keys(data).includes(prop)) && !isAdminOrGlobalMod && caller.uid === data.uid && hasPassword && !passwordMatch) {
 		throw new Error('[[error:invalid-password]]');
 	}
 
@@ -80,7 +86,6 @@ usersAPI.delete = async function (caller, data) {
 };
 
 usersAPI.deleteMany = async function (caller, data) {
-	console.log(data.uids);
 	if (await canDeleteUids(data.uids)) {
 		await Promise.all(data.uids.map(uid => processDeletion(uid, caller)));
 	}
@@ -127,6 +132,63 @@ usersAPI.unfollow = async function (caller, data) {
 	});
 };
 
+usersAPI.ban = async function (caller, data) {
+	if (!await privileges.users.hasBanPrivilege(caller.uid)) {
+		throw new Error('[[error:no-privileges]]');
+	} else if (await user.isAdministrator(data.uid)) {
+		throw new Error('[[error:cant-ban-other-admins]]');
+	}
+
+	const banData = await user.bans.ban(data.uid, data.until, data.reason);
+	await db.setObjectField('uid:' + data.uid + ':ban:' + banData.timestamp, 'fromUid', caller.uid);
+
+	if (!data.reason) {
+		data.reason = await translator.translate('[[user:info.banned-no-reason]]');
+	}
+
+	sockets.in('uid_' + data.uid).emit('event:banned', {
+		until: data.until,
+		reason: data.reason,
+	});
+
+	await flags.resolveFlag('user', data.uid, caller.uid);
+	await flags.resolveUserPostFlags(data.uid, caller.uid);
+	await events.log({
+		type: 'user-ban',
+		uid: caller.uid,
+		targetUid: data.uid,
+		ip: caller.ip,
+		reason: data.reason || undefined,
+	});
+	plugins.fireHook('action:user.banned', {
+		callerUid: caller.uid,
+		ip: caller.ip,
+		uid: data.uid,
+		until: data.until > 0 ? data.until : undefined,
+		reason: data.reason || undefined,
+	});
+	await user.auth.revokeAllSessions(data.uid);
+};
+
+usersAPI.unban = async function (caller, data) {
+	if (!await privileges.users.hasBanPrivilege(caller.uid)) {
+		throw new Error('[[error:no-privileges]]');
+	}
+
+	await user.bans.unban(data.uid);
+	await events.log({
+		type: 'user-unban',
+		uid: caller.uid,
+		targetUid: data.uid,
+		ip: caller.ip,
+	});
+	plugins.fireHook('action:user.unbanned', {
+		callerUid: caller.uid,
+		ip: caller.ip,
+		uid: data.uid,
+	});
+};
+
 async function processDeletion(uid, caller) {
 	const isTargetAdmin = await user.isAdministrator(uid);
 	const isSelf = parseInt(uid, 10) === caller.uid;

src/controllers/helpers.js

@@ -357,6 +357,13 @@ helpers.formatApiResponse = async (statusCode, res, payload) => {
 			message = payload.message;
 		}
 
+		// Update status code based on some common error codes
+		switch (payload.message) {
+			case '[[error:no-privileges]]':
+				statusCode = 403;
+				break;
+		}
+
 		const returnPayload = helpers.generateError(statusCode, message);
 
 		if (global.env === 'development') {

src/controllers/write/users.js

@@ -1,18 +1,10 @@
 'use strict';
 
 const api = require('../../api');
-const user = require('../../user');
-const plugins = require('../../plugins');
-const privileges = require('../../privileges');
-const flags = require('../../flags');
 const meta = require('../../meta');
-const events = require('../../events');
-const translator = require('../../translator');
 const utils = require('../../utils');
 
-const db = require('../../database');
 const helpers = require('../helpers');
-const sockets = require('../../socket.io');
 
 const Users = module.exports;
 
@@ -52,62 +44,12 @@ Users.unfollow = async (req, res) => {
 };
 
 Users.ban = async (req, res) => {
-	if (!await privileges.users.hasBanPrivilege(req.user.uid)) {
+	await api.users.ban(req, { ...req.body, uid: req.params.uid });
-		return helpers.formatApiResponse(403, res, new Error('[[error:no-privileges]]'));
-	} else if (await user.isAdministrator(req.params.uid)) {
-		return helpers.formatApiResponse(403, res, new Error('[[error:cant-ban-other-admins]]'));
-	}
-
-	const banData = await user.bans.ban(req.params.uid, req.body.until, req.body.reason);
-	await db.setObjectField('uid:' + req.params.uid + ':ban:' + banData.timestamp, 'fromUid', req.user.uid);
-
-	if (!req.body.reason) {
-		req.body.reason = await translator.translate('[[user:info.banned-no-reason]]');
-	}
-
-	sockets.in('uid_' + req.params.uid).emit('event:banned', {
-		until: req.body.until,
-		reason: req.body.reason,
-	});
-
-	await flags.resolveFlag('user', req.params.uid, req.user.uid);
-	await events.log({
-		type: 'user-ban',
-		uid: req.user.uid,
-		targetUid: req.params.uid,
-		ip: req.ip,
-		reason: req.body.reason || undefined,
-	});
-	plugins.fireHook('action:user.banned', {
-		callerUid: req.user.uid,
-		ip: req.ip,
-		uid: req.params.uid,
-		until: req.body.until > 0 ? req.body.until : undefined,
-		reason: req.body.reason || undefined,
-	});
-	await user.auth.revokeAllSessions(req.params.uid);
-
 	helpers.formatApiResponse(200, res);
 };
 
 Users.unban = async (req, res) => {
-	if (!await privileges.users.hasBanPrivilege(req.user.uid)) {
+	await api.users.unban(req, { ...req.body, uid: req.params.uid });
-		return helpers.formatApiResponse(403, res, new Error('[[error:no-privileges]]'));
-	}
-
-	await user.bans.unban(req.params.uid);
-	await events.log({
-		type: 'user-unban',
-		uid: req.user.uid,
-		targetUid: req.params.uid,
-		ip: req.ip,
-	});
-	plugins.fireHook('action:user.unbanned', {
-		callerUid: req.user.uid,
-		ip: req.ip,
-		uid: req.params.uid,
-	});
-
 	helpers.formatApiResponse(200, res);
 };
 

src/socket.io/user/ban.js

@@ -1,91 +1,23 @@
 'use strict';
 
-const db = require('../../database');
+const api = require('../../api');
-const user = require('../../user');
 const websockets = require('../index');
-const events = require('../../events');
-const privileges = require('../../privileges');
-const plugins = require('../../plugins');
-const translator = require('../../translator');
-const flags = require('../../flags');
 
 module.exports = function (SocketUser) {
 	SocketUser.banUsers = async function (socket, data) {
 		websockets.warnDeprecated(socket, 'PUT /api/v3/users/:uid/ban');
-
+		await Promise.all(data.uids.map(async (uid) => {
-		if (!data || !Array.isArray(data.uids)) {
+			const payload = { ...data };
-			throw new Error('[[error:invalid-data]]');
+			delete payload.uids;
-		}
+			payload.uid = uid;
-
+			await api.users.ban(socket, payload);
-		await toggleBan(socket.uid, data.uids, async function (uid) {
+		}));
-			await banUser(socket.uid, uid, data.until || 0, data.reason || '');
-			await flags.resolveFlag('user', uid, socket.uid);
-			await flags.resolveUserPostFlags(uid, socket.uid);
-			await events.log({
-				type: 'user-ban',
-				uid: socket.uid,
-				targetUid: uid,
-				ip: socket.ip,
-				reason: data.reason || undefined,
-			});
-			plugins.fireHook('action:user.banned', {
-				callerUid: socket.uid,
-				ip: socket.ip,
-				uid: uid,
-				until: data.until > 0 ? data.until : undefined,
-				reason: data.reason || undefined,
-			});
-			await user.auth.revokeAllSessions(uid);
-		});
 	};
 
 	SocketUser.unbanUsers = async function (socket, uids) {
 		websockets.warnDeprecated(socket, 'DELETE /api/v3/users/:uid/ban');
-
+		await Promise.all(uids.map(async (uid) => {
-		await toggleBan(socket.uid, uids, async function (uid) {
+			await api.users.unban(socket, { uid });
-			await user.bans.unban(uid);
+		}));
-			await events.log({
-				type: 'user-unban',
-				uid: socket.uid,
-				targetUid: uid,
-				ip: socket.ip,
-			});
-			plugins.fireHook('action:user.unbanned', {
-				callerUid: socket.uid,
-				ip: socket.ip,
-				uid: uid,
-			});
-		});
 	};
-
-	async function toggleBan(uid, uids, method) {
-		if (!Array.isArray(uids)) {
-			throw new Error('[[error:invalid-data]]');
-		}
-		const hasBanPrivilege = await privileges.users.hasBanPrivilege(uid);
-		if (!hasBanPrivilege) {
-			throw new Error('[[error:no-privileges]]');
-		}
-
-		await Promise.all(uids.map(uid => method(uid)));
-	}
-
-	async function banUser(callerUid, uid, until, reason) {
-		const isAdmin = await user.isAdministrator(uid);
-		if (isAdmin) {
-			throw new Error('[[error:cant-ban-other-admins]]');
-		}
-
-		const banData = await user.bans.ban(uid, until, reason);
-		await db.setObjectField('uid:' + uid + ':ban:' + banData.timestamp, 'fromUid', callerUid);
-
-		if (!reason) {
-			reason = await translator.translate('[[user:info.banned-no-reason]]');
-		}
-
-		websockets.in('uid_' + uid).emit('event:banned', {
-			until: until,
-			reason: reason,
-		});
-	}
 };

test/user.js

@@ -805,6 +805,7 @@ describe('User', function () {
 					groupTitle: 'testGroup',
 					birthday: '01/01/1980',
 					signature: 'nodebb is good',
+					password: '123456',
 				};
 				socketUser.updateProfile({ uid: uid }, { ...data, password: '123456' }, function (err, result) {
 					assert.ifError(err);
@@ -816,7 +817,11 @@ describe('User', function () {
 					db.getObject('user:' + uid, function (err, userData) {
 						assert.ifError(err);
 						Object.keys(data).forEach(function (key) {
-							assert.equal(data[key], userData[key]);
+							if (key !== 'password') {
+								assert.equal(data[key], userData[key]);
+							} else {
+								assert(userData[key].startsWith('$2a$'));
+							}
 						});
 						done();
 					});