feat(api): account deletion routes for the Write API (#8881)

* feat(api): account deletion routes for the Write API

* refactor: rewrite client-side calls to account deletion to use api

* style: apply DRY

public/openapi/write.yaml

@@ -32,6 +32,10 @@ paths:
     $ref: 'write/users.yaml'
   /users/{uid}:
     $ref: 'write/users/uid.yaml'
+  /users/{uid}/content:
+    $ref: 'write/users/uid/content.yaml'
+  /users/{uid}/account:
+    $ref: 'write/users/uid/account.yaml'
   /users/{uid}/settings:
     $ref: 'write/users/uid/settings.yaml'
   /users/{uid}/password:

public/openapi/write/users/uid/account.yaml

@@ -0,0 +1,25 @@
+delete:
+  tags:
+    - users
+  summary: delete a single user account (preserve content)
+  description: This route deletes a single user's account, but preserves the content (posts, bookmarks, etc.)
+  parameters:
+    - in: path
+      name: uid
+      schema:
+        type: integer
+      required: true
+      description: uid of the user to delete
+      example: 7
+  responses:
+    '200':
+      description: user account deleted
+      content:
+        application/json:
+          schema:
+            type: object
+            properties:
+              status:
+                $ref: ../../../components/schemas/Status.yaml#/Status
+              response:
+                type: object

public/openapi/write/users/uid/content.yaml

@@ -0,0 +1,25 @@
+delete:
+  tags:
+    - users
+  summary: delete a single user account's content (preserve account)
+  description: This route deletes a single user's account content (posts, bookmarks, etc.) but preserves the account itself
+  parameters:
+    - in: path
+      name: uid
+      schema:
+        type: integer
+      required: true
+      description: uid of the user's content to delete
+      example: 7
+  responses:
+    '200':
+      description: user account content deleted
+      content:
+        application/json:
+          schema:
+            type: object
+            properties:
+              status:
+                $ref: ../../../components/schemas/Status.yaml#/Status
+              response:
+                type: object

public/src/admin/manage/users.js

@@ -36,8 +36,12 @@ define('admin/manage/users', [
 			$('.users-table [component="user/select/all"]').prop('checked', false);
 		}
 
-		function removeSelected() {
+		function removeRow(uid) {
-			$('.users-table [component="user/select/single"]:checked').parents('.user-row').remove();
+			const checkboxEl = document.querySelector(`.users-table [component="user/select/single"][data-uid="${uid}"]`);
+			if (checkboxEl) {
+				const rowEl = checkboxEl.closest('.user-row');
+				rowEl.parentNode.removeChild(rowEl);
+			}
 		}
 
 		// use onSuccess instead
@@ -253,71 +257,46 @@ define('admin/manage/users', [
 			});
 		});
 
-		$('.delete-user').on('click', function () {
+		$('.delete-user').on('click', () => {
+			handleDelete('[[admin/manage/users:alerts.confirm-delete]]', '/account');
+		});
+
+		$('.delete-user-content').on('click', () => {
+			handleDelete('[[admin/manage/users:alerts.confirm-delete-content]]', '/content');
+		});
+
+		$('.delete-user-and-content').on('click', () => {
+			handleDelete('[[admin/manage/users:alerts.confirm-purge]]', '');
+		});
+
+		function handleDelete(confirmMsg, path) {
 			var uids = getSelectedUids();
 			if (!uids.length) {
 				return;
 			}
 
-			bootbox.confirm('[[admin/manage/users:alerts.confirm-delete]]', function (confirm) {
+			bootbox.confirm(confirmMsg, function (confirm) {
 				if (confirm) {
-					socket.emit('admin.user.deleteUsers', uids, function (err) {
+					Promise.all(uids.map(uid => api.del(`/users/${uid}${path}`, {})
-						if (err) {
+						.then(() => {
-							return app.alertError(err.message);
+							if (path !== '/content') {
+								removeRow(uid);
 							}
-
+						})
+					)).then(() => {
+						if (path !== '/content') {
 							app.alertSuccess('[[admin/manage/users:alerts.delete-success]]');
-						removeSelected();
+						} else {
-						unselectAll();
-						if (!$('.users-table [component="user/select/single"]').length) {
-							ajaxify.refresh();
-						}
-					});
-				}
-			});
-		});
-
-		$('.delete-user-content').on('click', function () {
-			var uids = getSelectedUids();
-			if (!uids.length) {
-				return;
-			}
-
-			bootbox.confirm('[[admin/manage/users:alerts.confirm-delete-content]]', function (confirm) {
-				if (confirm) {
-					socket.emit('admin.user.deleteUsersContent', uids, function (err) {
-						if (err) {
-							return app.alertError(err.message);
-						}
-
 							app.alertSuccess('[[admin/manage/users:alerts.delete-content-success]]');
-					});
 						}
-			});
-		});
-
-		$('.delete-user-and-content').on('click', function () {
-			var uids = getSelectedUids();
-			if (!uids.length) {
-				return;
-			}
-			bootbox.confirm('[[admin/manage/users:alerts.confirm-purge]]', function (confirm) {
-				if (confirm) {
-					socket.emit('admin.user.deleteUsersAndContent', uids, function (err) {
-						if (err) {
-							return app.alertError(err.message);
-						}
-
-						app.alertSuccess('[[admin/manage/users:alerts.delete-success]]');
-						removeSelected();
 						unselectAll();
 						if (!$('.users-table [component="user/select/single"]').length) {
 							ajaxify.refresh();
 						}
-					});
+					}).catch(app.alertError);
 				}
 			});
-		});
+		}
 
 		function handleUserCreate() {
 			$('[data-action="create"]').on('click', function () {

public/src/modules/accounts/delete.js

@@ -1,13 +1,13 @@
 'use strict';
 
-define('accounts/delete', [], function () {
+define('accounts/delete', ['api', 'bootbox'], function (api) {
 	var Delete = {};
 
 	Delete.account = function (uid, callback) {
 		executeAction(
 			uid,
 			'[[user:delete_this_account_confirm]]',
-			'admin.user.deleteUsers',
+			'/account',
 			'[[user:account-deleted]]',
 			callback
 		);
@@ -17,7 +17,7 @@ define('accounts/delete', [], function () {
 		executeAction(
 			uid,
 			'[[user:delete_account_content_confirm]]',
-			'admin.user.deleteUsersContent',
+			'/content',
 			'[[user:account-content-deleted]]',
 			callback
 		);
@@ -27,22 +27,19 @@ define('accounts/delete', [], function () {
 		executeAction(
 			uid,
 			'[[user:delete_all_confirm]]',
-			'admin.user.deleteUsersAndContent',
+			'',
 			'[[user:account-deleted]]',
 			callback
 		);
 	};
 
-	function executeAction(uid, confirmText, action, successText, callback) {
+	function executeAction(uid, confirmText, path, successText, callback) {
 		bootbox.confirm(confirmText, function (confirm) {
 			if (!confirm) {
 				return;
 			}
 
-			socket.emit(action, [uid], function (err) {
+			api.del(`/users/${uid}${path}`, {}).then(() => {
-				if (err) {
-					return app.alertError(err.message);
-				}
 				app.alertSuccess(successText);
 
 				if (typeof callback === 'function') {

src/api/users.js

@@ -81,13 +81,21 @@ usersAPI.update = async function (caller, data) {
 	return userData;
 };
 
-usersAPI.delete = async function (caller, data) {
+usersAPI.delete = async function (caller, { uid, password }) {
-	processDeletion(data.uid, caller);
+	processDeletion({ uid: uid, method: 'delete', password, caller });
+};
+
+usersAPI.deleteContent = async function (caller, { uid, password }) {
+	processDeletion({ uid, method: 'deleteContent', password, caller });
+};
+
+usersAPI.deleteAccount = async function (caller, { uid, password }) {
+	processDeletion({ uid, method: 'deleteAccount', password, caller });
 };
 
 usersAPI.deleteMany = async function (caller, data) {
 	if (await canDeleteUids(data.uids)) {
-		await Promise.all(data.uids.map(uid => processDeletion(uid, caller)));
+		await Promise.all(data.uids.map(uid => processDeletion({ uid, method: 'delete', caller })));
 	}
 };
 
@@ -229,22 +237,56 @@ async function isPrivilegedOrSelfAndPasswordMatch(caller, data) {
 	}
 }
 
-async function processDeletion(uid, caller) {
+async function processDeletion({ uid, method, password, caller }) {
 	const isTargetAdmin = await user.isAdministrator(uid);
 	const isSelf = parseInt(uid, 10) === caller.uid;
 	const isAdmin = await user.isAdministrator(caller.uid);
 
-	if (!isSelf && !isAdmin) {
+	if (meta.config.allowAccountDelete !== 1) {
 		throw new Error('[[error:no-privileges]]');
-	} else if (!isSelf && isTargetAdmin) {
+	} else if (!isSelf && !isAdmin) {
+		throw new Error('[[error:no-privileges]]');
+	} else if (isTargetAdmin) {
 		throw new Error('[[error:cant-delete-other-admins]]');
 	}
 
+	// Privilege checks -- only deleteAccount is available for non-admins
+	const hasAdminPrivilege = await privileges.admin.can('admin:users', caller.uid);
+	if (!hasAdminPrivilege && ['delete', 'deleteContent'].includes(method)) {
+		throw new Error('[[error:no-privileges]]');
+	}
+
+	// Self-deletions require a password
+	const hasPassword = await user.hasPassword(uid);
+	if (isSelf && hasPassword) {
+		const ok = await user.isPasswordCorrect(uid, password, caller.ip);
+		if (!ok) {
+			throw new Error('[[error:invalid-password]]');
+		}
+	}
+
 	// TODO: clear user tokens for this uid
 	await flags.resolveFlag('user', uid, caller.uid);
-	const userData = await user.delete(caller.uid, uid);
+
+	let userData;
+	if (method === 'deleteAccount') {
+		userData = await user[method](uid);
+	} else {
+		userData = await user[method](caller.uid, uid);
+	}
+	userData = userData || {};
+
+	sockets.server.sockets.emit('event:user_status_change', { uid: caller.uid, status: 'offline' });
+
+	plugins.fireHook('action:user.delete', {
+		callerUid: caller.uid,
+		uid: uid,
+		ip: caller.ip,
+		user: userData,
+	});
+
 	await events.log({
-		type: 'user-delete',
+		type: `user-${method}`,
 		uid: caller.uid,
 		targetUid: uid,
 		ip: caller.ip,

src/controllers/write/users.js

@@ -49,7 +49,17 @@ Users.update = async (req, res) => {
 };
 
 Users.delete = async (req, res) => {
-	await api.users.delete(req, req.params);
+	await api.users.delete(req, { ...req.params, password: req.body.password });
+	helpers.formatApiResponse(200, res);
+};
+
+Users.deleteContent = async (req, res) => {
+	await api.users.deleteContent(req, { ...req.params, password: req.body.password });
+	helpers.formatApiResponse(200, res);
+};
+
+Users.deleteAccount = async (req, res) => {
+	await api.users.deleteAccount(req, { ...req.params, password: req.body.password });
 	helpers.formatApiResponse(200, res);
 };
 

src/routes/write/users.js

@@ -20,7 +20,9 @@ function authenticatedRoutes() {
 
 	setupApiRoute(router, 'head', '/:uid', [middleware.assert.user], controllers.write.users.exists);
 	setupApiRoute(router, 'put', '/:uid', [...middlewares, middleware.assert.user], controllers.write.users.update);
-	setupApiRoute(router, 'delete', '/:uid', [...middlewares, middleware.assert.user, middleware.exposePrivileges], controllers.write.users.delete);
+	setupApiRoute(router, 'delete', '/:uid', [...middlewares, middleware.assert.user], controllers.write.users.delete);
+	setupApiRoute(router, 'delete', '/:uid/content', [...middlewares, middleware.assert.user], controllers.write.users.deleteContent);
+	setupApiRoute(router, 'delete', '/:uid/account', [...middlewares, middleware.assert.user], controllers.write.users.deleteAccount);
 
 	setupApiRoute(router, 'put', '/:uid/settings', [...middlewares, middleware.checkRequired.bind(null, ['settings'])], controllers.write.users.updateSettings);
 

src/socket.io/admin/user.js

@@ -1,7 +1,6 @@
 'use strict';
 
 const async = require('async');
-const winston = require('winston');
 
 const db = require('../../database');
 const api = require('../../api');
@@ -9,9 +8,7 @@ const groups = require('../../groups');
 const user = require('../../user');
 const events = require('../../events');
 const meta = require('../../meta');
-const plugins = require('../../plugins');
 const translator = require('../../translator');
-const flags = require('../../flags');
 const sockets = require('..');
 
 const User = module.exports;
@@ -125,16 +122,16 @@ User.forcePasswordReset = async function (socket, uids) {
 };
 
 User.deleteUsers = async function (socket, uids) {
-	await canDeleteUids(uids);
+	sockets.warnDeprecated(socket, 'DELETE /api/v3/users/:uid/account');
-	deleteUsers(socket, uids, async function (uid) {
+	await Promise.all(uids.map(async (uid) => {
-		return await user.deleteAccount(uid);
+		await api.users.deleteAccount(socket, { uid });
-	});
+	}));
 };
 
 User.deleteUsersContent = async function (socket, uids) {
-	await canDeleteUids(uids);
+	sockets.warnDeprecated(socket, 'DELETE /api/v3/users/:uid/content');
 	await Promise.all(uids.map(async (uid) => {
-		await user.deleteContent(socket.uid, uid);
+		await api.users.deleteContent(socket, { uid });
 	}));
 };
 
@@ -143,42 +140,6 @@ User.deleteUsersAndContent = async function (socket, uids) {
 	await api.users.deleteMany(socket, { uids });
 };
 
-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]]');
-	}
-}
-
-async function deleteUsers(socket, uids, method) {
-	async function doDelete(uid) {
-		await flags.resolveFlag('user', uid, socket.uid);
-		const userData = await method(uid);
-		await events.log({
-			type: 'user-delete',
-			uid: socket.uid,
-			targetUid: uid,
-			ip: socket.ip,
-			username: userData.username,
-			email: userData.email,
-		});
-		plugins.fireHook('action:user.delete', {
-			callerUid: socket.uid,
-			uid: uid,
-			ip: socket.ip,
-			user: userData,
-		});
-	}
-	try {
-		await Promise.all(uids.map(uid => doDelete(uid)));
-	} catch (err) {
-		winston.error(err.stack);
-	}
-}
-
 User.restartJobs = async function () {
 	user.startJobs();
 };

src/socket.io/user.js

@@ -15,7 +15,6 @@ const db = require('../database');
 const userController = require('../controllers/user');
 const privileges = require('../privileges');
 const utils = require('../utils');
-const flags = require('../flags');
 const sockets = require('.');
 
 const SocketUser = module.exports;
@@ -37,37 +36,8 @@ SocketUser.exists = async function (socket, data) {
 };
 
 SocketUser.deleteAccount = async function (socket, data) {
-	if (!socket.uid) {
+	sockets.warnDeprecated(socket, 'DELETE /api/v3/users/:uid/account');
-		throw new Error('[[error:no-privileges]]');
+	await api.users.deleteAccount(socket, data);
-	}
-	const hasPassword = await user.hasPassword(socket.uid);
-	if (hasPassword) {
-		const ok = await user.isPasswordCorrect(socket.uid, data.password, socket.ip);
-		if (!ok) {
-			throw new Error('[[error:invalid-password]]');
-		}
-	}
-	const isAdmin = await user.isAdministrator(socket.uid);
-	if (isAdmin) {
-		throw new Error('[[error:cant-delete-admin]]');
-	}
-	if (meta.config.allowAccountDelete !== 1) {
-		throw new Error('[[error:no-privileges]]');
-	}
-
-	await flags.resolveFlag('user', socket.uid, socket.uid);
-	const userData = await user.deleteAccount(socket.uid);
-
-	require('./index').server.sockets.emit('event:user_status_change', { uid: socket.uid, status: 'offline' });
-
-	await events.log({
-		type: 'user-delete',
-		uid: socket.uid,
-		targetUid: socket.uid,
-		ip: socket.ip,
-		username: userData.username,
-		email: userData.email,
-	});
 };
 
 SocketUser.emailExists = async function (socket, data) {

src/user/delete.js

@@ -7,6 +7,7 @@ const nconf = require('nconf');
 
 const db = require('../database');
 const posts = require('../posts');
+const flags = require('../flags');
 const topics = require('../topics');
 const groups = require('../groups');
 const messaging = require('../messaging');
@@ -149,6 +150,7 @@ module.exports = function (User) {
 			deleteUserFromFollowers(uid),
 			deleteImages(uid),
 			groups.leaveAllGroups(uid),
+			flags.resolveFlag('user', uid, uid),
 		]);
 		await db.deleteAll(['followers:' + uid, 'following:' + uid, 'user:' + uid]);
 		delete deletesInProgress[uid];

test/api.js

@@ -80,9 +80,9 @@ describe('API', async () => {
 		// Create sample users
 		const adminUid = await user.create({ username: 'admin', password: '123456', email: 'test@example.org' });
 		const unprivUid = await user.create({ username: 'unpriv', password: '123456', email: 'unpriv@example.org' });
-		for (let x = 0; x < 3; x++) {
+		for (let x = 0; x < 4; x++) {
 			// eslint-disable-next-line no-await-in-loop
-			await user.create({ username: 'deleteme', password: '123456' });	// for testing of user deletion routes (uids 4-6)
+			await user.create({ username: 'deleteme', password: '123456' });	// for testing of DELETE /users (uids 5, 6) and DELETE /user/:uid/account (uid 7)
 		}
 		await groups.join('administrators', adminUid);