From 48a9b5d66ef9d2d14ee94c22e140c7feccc79e50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Thu, 15 Oct 2020 17:31:03 -0400 Subject: [PATCH 1/5] update logo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index a606da88e6..86e435d345 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# NodeBB +# NodeBB [![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) From 2d252f2fa455613210a6041f5ba89e77751f0b95 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 15 Oct 2020 19:05:31 -0400 Subject: [PATCH 2/5] refactor: user bans to use api lib --- src/api/users.js | 60 ++++++++++++++++++++++++ src/controllers/write/users.js | 62 +------------------------ src/socket.io/user/ban.js | 85 +++------------------------------- 3 files changed, 69 insertions(+), 138 deletions(-) diff --git a/src/api/users.js b/src/api/users.js index df816961e5..54b26d1845 100644 --- a/src/api/users.js +++ b/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; @@ -120,6 +123,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; diff --git a/src/controllers/write/users.js b/src/controllers/write/users.js index c74b823497..c76a6d0b5b 100644 --- a/src/controllers/write/users.js +++ b/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)) { - 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); - + await api.users.ban(req, { ...req.body, uid: req.params.uid }); helpers.formatApiResponse(200, res); }; Users.unban = async (req, res) => { - if (!await privileges.users.hasBanPrivilege(req.user.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, - }); - + await api.users.unban(req, { ...req.body, uid: req.params.uid }); helpers.formatApiResponse(200, res); }; diff --git a/src/socket.io/user/ban.js b/src/socket.io/user/ban.js index 8f021cb258..f2a5b447a6 100644 --- a/src/socket.io/user/ban.js +++ b/src/socket.io/user/ban.js @@ -1,91 +1,20 @@ 'use strict'; -const db = require('../../database'); -const user = require('../../user'); +const api = require('../../api'); 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'); - - if (!data || !Array.isArray(data.uids)) { - throw new Error('[[error:invalid-data]]'); - } - - 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); - }); + await Promise.all(data.uids.map(async (uid) => { + await api.users.ban(socket, { uid }); + })); }; SocketUser.unbanUsers = async function (socket, uids) { websockets.warnDeprecated(socket, 'DELETE /api/v3/users/:uid/ban'); - - await toggleBan(socket.uid, uids, async function (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, - }); - }); + await Promise.all(uids.map(async (uid) => { + await api.users.unban(socket, { 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, - }); - } }; From 222b4c95338ebfa025ecec106a63780bbbfe066a Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 15 Oct 2020 19:33:20 -0400 Subject: [PATCH 3/5] fix: broken tests from api change --- src/api/users.js | 15 +++++++++++++-- test/user.js | 7 ++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/api/users.js b/src/api/users.js index 54b26d1845..9b16717aac 100644 --- a/src/api/users.js +++ b/src/api/users.js @@ -23,19 +23,28 @@ usersAPI.create = async function (caller, data) { }; usersAPI.update = async function (caller, 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]]'); } @@ -69,6 +78,8 @@ usersAPI.update = async function (caller, data) { if (userData.username !== oldUserData.username) { await log('username-change', { oldUsername: oldUserData.username, newUsername: userData.username }); } + + return userData; }; usersAPI.delete = async function (caller, data) { diff --git a/test/user.js b/test/user.js index 060137e3d4..d0a9d337e6 100644 --- a/test/user.js +++ b/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, 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(); }); From 14f9d8b0e58549e37ee0268bb8c5511eff1266ec Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 15 Oct 2020 20:23:19 -0400 Subject: [PATCH 4/5] feat: send back 403 on no-privileges error --- src/controllers/helpers.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/controllers/helpers.js b/src/controllers/helpers.js index 76dc5f6fa4..4f1a8c6d81 100644 --- a/src/controllers/helpers.js +++ b/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') { From 3f347baadb74bf27097d87b21f429ee6072054f9 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 15 Oct 2020 20:31:25 -0400 Subject: [PATCH 5/5] fix: socket user bans --- src/api/users.js | 1 - src/socket.io/user/ban.js | 5 ++++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/api/users.js b/src/api/users.js index 9b16717aac..b695d1f848 100644 --- a/src/api/users.js +++ b/src/api/users.js @@ -87,7 +87,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))); } diff --git a/src/socket.io/user/ban.js b/src/socket.io/user/ban.js index f2a5b447a6..77df1d78a9 100644 --- a/src/socket.io/user/ban.js +++ b/src/socket.io/user/ban.js @@ -7,7 +7,10 @@ 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) => { - await api.users.ban(socket, { uid }); + const payload = { ...data }; + delete payload.uids; + payload.uid = uid; + await api.users.ban(socket, payload); })); };