From 430e7f5834fa86a7607ecc1f8381a1f7860b3541 Mon Sep 17 00:00:00 2001
From: Julian Lam <julian@nodebb.org>
Date: Thu, 15 Oct 2020 16:49:06 -0400
Subject: [PATCH] refactor: user deletion to use api lib

---
 src/api/users.js               | 49 ++++++++++++++++++++++++++++++++++
 src/controllers/write/users.js | 46 +++----------------------------
 src/socket.io/admin/user.js    |  6 +----
 3 files changed, 54 insertions(+), 47 deletions(-)

diff --git a/src/api/users.js b/src/api/users.js
index 1b7863b50b..054f59ef35 100644
--- a/src/api/users.js
+++ b/src/api/users.js
@@ -1,7 +1,9 @@
 'use strict';
 
 const user = require('../user');
+const groups = require('../groups');
 const meta = require('../meta');
+const flags = require('../flags');
 const privileges = require('../privileges');
 const events = require('../events');
 
@@ -60,3 +62,50 @@ usersAPI.update = async function (caller, data) {
 		await log('username-change', { oldUsername: oldUserData.username, newUsername: userData.username });
 	}
 };
+
+usersAPI.delete = async function (caller, data) {
+	processDeletion(data.uid, caller);
+};
+
+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)));
+	}
+};
+
+async function processDeletion(uid, caller) {
+	const isTargetAdmin = await user.isAdministrator(uid);
+	const isSelf = parseInt(uid, 10) === caller.uid;
+	const isAdmin = await user.isAdministrator(caller.uid);
+
+	if (!isSelf && !isAdmin) {
+		throw new Error('[[error:no-privileges]]');
+	} else if (!isSelf && isTargetAdmin) {
+		throw new Error('[[error:cant-delete-other-admins]]');
+	}
+
+	// TODO: clear user tokens for this uid
+	await flags.resolveFlag('user', uid, caller.uid);
+	const userData = await user.delete(caller.uid, uid);
+	await events.log({
+		type: 'user-delete',
+		uid: caller.uid,
+		targetUid: uid,
+		ip: caller.ip,
+		username: userData.username,
+		email: userData.email,
+	});
+}
+
+async function canDeleteUids(uids) {
+	if (!Array.isArray(uids)) {
+		throw new Error('[[error:invalid-data]]');
+	}
+	const isMembers = await groups.isMembers(uids, 'administrators');
+	if (isMembers.includes(true)) {
+		throw new Error('[[error:cant-delete-other-admins]]');
+	}
+
+	return true;
+}
diff --git a/src/controllers/write/users.js b/src/controllers/write/users.js
index 7102f42a33..35306472c3 100644
--- a/src/controllers/write/users.js
+++ b/src/controllers/write/users.js
@@ -2,7 +2,6 @@
 
 const api = require('../../api');
 const user = require('../../user');
-const groups = require('../../groups');
 const plugins = require('../../plugins');
 const privileges = require('../../privileges');
 const notifications = require('../../notifications');
@@ -24,57 +23,20 @@ Users.create = async (req, res) => {
 };
 
 Users.update = async (req, res) => {
-	const userObj = await api.users.update(req, { ...req.body, ...req.params });
+	const userObj = await api.users.update(req, { ...req.body, uid: req.params.uid });
 	helpers.formatApiResponse(200, res, userObj);
 };
 
 Users.delete = async (req, res) => {
-	processDeletion(req.params.uid, req, res);
+	await api.users.delete(req, req.params);
 	helpers.formatApiResponse(200, res);
 };
 
 Users.deleteMany = async (req, res) => {
-	if (await canDeleteUids(req.body.uids, res)) {
-		await Promise.all(req.body.uids.map(uid => processDeletion(uid, req, res)));
-		helpers.formatApiResponse(200, res);
-	}
+	await api.users.deleteMany(req, req.body);
+	helpers.formatApiResponse(200, res);
 };
 
-async function canDeleteUids(uids, res) {
-	if (!Array.isArray(uids)) {
-		helpers.formatApiResponse(400, res, new Error('[[error:invalid-data]]'));
-		return false;
-	}
-	const isMembers = await groups.isMembers(uids, 'administrators');
-	if (isMembers.includes(true)) {
-		helpers.formatApiResponse(403, res, new Error('[[error:cant-delete-other-admins]]'));
-		return false;
-	}
-
-	return true;
-}
-
-async function processDeletion(uid, req, res) {
-	const isTargetAdmin = await user.isAdministrator(uid);
-	if (!res.locals.privileges.isSelf && !res.locals.privileges.isAdmin) {
-		return helpers.formatApiResponse(403, res);
-	} else if (!res.locals.privileges.isSelf && isTargetAdmin) {
-		return helpers.formatApiResponse(403, res, new Error('[[error:cant-delete-other-admins]]'));
-	}
-
-	// TODO: clear user tokens for this uid
-	await flags.resolveFlag('user', uid, req.user.uid);
-	const userData = await user.delete(req.user.uid, uid);
-	await events.log({
-		type: 'user-delete',
-		uid: req.user.uid,
-		targetUid: uid,
-		ip: req.ip,
-		username: userData.username,
-		email: userData.email,
-	});
-}
-
 Users.changePassword = async (req, res) => {
 	req.body.uid = req.params.uid;
 	await user.changePassword(req.user.uid, Object.assign(req.body, { ip: req.ip }));
diff --git a/src/socket.io/admin/user.js b/src/socket.io/admin/user.js
index 44a8de4075..266e739db6 100644
--- a/src/socket.io/admin/user.js
+++ b/src/socket.io/admin/user.js
@@ -140,11 +140,7 @@ User.deleteUsersContent = async function (socket, uids) {
 
 User.deleteUsersAndContent = async function (socket, uids) {
 	sockets.warnDeprecated(socket, 'DELETE /api/v3/users or DELETE /api/v3/users/:uid');
-
-	await canDeleteUids(uids);
-	deleteUsers(socket, uids, async function (uid) {
-		return await user.delete(socket.uid, uid);
-	});
+	await api.users.deleteMany(socket, { uids });
 };
 
 async function canDeleteUids(uids) {