refactor: how admins change emails (#11973)

* refactor: how admins change emails

ability for admins to change emails from acp
ability for admins to change passwords from acp
only users themselves can use /user/<slug>/edit/email
group actions in manage users dropdown
admins can use the same modal from profile page instead of interstitial to update email
add missing checks to addEmail, if email take throw error
add targetUid to email change event

* test: bunch of baloney

* test: remove old test

public/language/en-GB/admin/manage/users.json

@@ -4,16 +4,21 @@
 	"edit": "Actions",
 	"make-admin": "Make Admin",
 	"remove-admin": "Remove Admin",
+	"change-email": "Change Email",
+	"new-email": "New Email",
 	"validate-email": "Validate Email",
 	"send-validation-email": "Send Validation Email",
+	"change-password": "Change Password",
 	"password-reset-email": "Send Password Reset Email",
 	"force-password-reset": "Force Password Reset & Log User Out",
-	"ban": "Ban User(s)",
+	"ban": "Ban",
+	"ban-users": "Ban User(s)",
 	"temp-ban": "Ban User(s) Temporarily",
 	"unban": "Unban User(s)",
 	"reset-lockout": "Reset Lockout",
 	"reset-flags": "Reset Flags",
-	"delete": "Delete <strong>User(s)</strong>",
+	"delete": "Delete",
+	"delete-users": "Delete <strong>User(s)</strong>",
 	"delete-content": "Delete User(s) <strong>Content</strong>",
 	"purge": "Delete <strong>User(s)</strong> and <strong>Content</strong>",
 	"download-csv": "Download CSV",
@@ -81,6 +86,7 @@
 	"alerts.button-ban-x": "Ban %1 user(s)",
 	"alerts.unban-success": "User(s) unbanned!",
 	"alerts.lockout-reset-success": "Lockout(s) reset!",
+	"alerts.password-change-success": "Password(s) changed!",
 	"alerts.flag-reset-success": "Flags(s) reset!",
 	"alerts.no-remove-yourself-admin": "You can't remove yourself as Administrator!",
 	"alerts.make-admin-success": "User is now administrator.",
@@ -106,6 +112,7 @@
 	"alerts.create": "Create User",
 	"alerts.button-create": "Create",
 	"alerts.button-cancel": "Cancel",
+	"alerts.button-change": "Change",
 	"alerts.error-passwords-different": "Passwords must match!",
 	"alerts.error-x": "<strong>Error</strong><p>%1</p>",
 	"alerts.create-success": "User created!",
@@ -113,6 +120,10 @@
 	"alerts.prompt-email": "Emails: ",
 	"alerts.email-sent-to": "An invitation email has been sent to %1",
 	"alerts.x-users-found": "%1 user(s) found, (%2 seconds)",
+	"alerts.select-a-single-user-to-change-email": "Select a single user to change email",
 	"export-users-started": "Exporting users as csv, this might take a while. You will receive a notification when it is complete.",
-	"export-users-completed": "Users exported as csv, click here to download."
+	"export-users-completed": "Users exported as csv, click here to download.",
+	"email": "Email",
+	"password": "Password",
+	"manage": "Manage"
 }

public/language/en-GB/user.json

@@ -64,6 +64,7 @@
 	"change_picture": "Change Picture",
 	"change_username": "Change Username",
 	"change_email": "Change Email",
+	"email-updated": "Email Updated",
 	"email_same_as_password": "Please enter your current password to continue – you've entered your new email again",
 	"edit": "Edit",
 	"edit-profile": "Edit Profile",

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

@@ -82,3 +82,5 @@ post:
                     items:
                       type: string
                       description: An email address
+    '400':
+      description: error occured (aka email taken)

public/src/admin/manage/users.js

@@ -1,8 +1,8 @@
 'use strict';
 
 define('admin/manage/users', [
-	'translator', 'benchpress', 'autocomplete', 'api', 'slugify', 'bootbox', 'alerts', 'accounts/invite', 'helpers',
-], function (translator, Benchpress, autocomplete, api, slugify, bootbox, alerts, AccountInvite, helpers) {
+	'translator', 'benchpress', 'autocomplete', 'api', 'slugify', 'bootbox', 'alerts', 'accounts/invite', 'helpers', 'admin/modules/change-email',
+], function (translator, Benchpress, autocomplete, api, slugify, bootbox, alerts, AccountInvite, helpers, changeEmail) {
 	const Users = {};
 
 	Users.init = function () {
@@ -273,6 +273,26 @@ define('admin/manage/users', [
 			socket.emit('admin.user.resetLockouts', uids, done('[[admin/manage/users:alerts.lockout-reset-success]]'));
 		});
 
+		$('.change-email').on('click', function () {
+			const uids = getSelectedUids();
+			if (uids.length !== 1) {
+				return alerts.error('[[admin/manage/users:alerts.select-a-single-user-to-change-email]]');
+			}
+			changeEmail.init({
+				uid: uids[0],
+				onSuccess: function (newEmail) {
+					update('.notvalidated', false);
+					update('.pending', false);
+					update('.expired', false);
+					update('.validated', false);
+					update('.validated-by-admin', !!newEmail);
+					update('.no-email', !newEmail);
+					$('.users-table [component="user/select/single"]:checked').parents('.user-row').find('.validated-by-admin .email').text(newEmail);
+					// $('.users-table [component="user/select/single"]:checked').parents('.user-row').find('.no-email').
+				},
+			});
+		});
+
 		$('.validate-email').on('click', function () {
 			const uids = getSelectedUids();
 			if (!uids.length) {
@@ -311,6 +331,51 @@ define('admin/manage/users', [
 			});
 		});
 
+		$('.change-password').on('click', async function () {
+			const uids = getSelectedUids();
+			if (!uids.length) {
+				return;
+			}
+			async function changePassword(modal) {
+				const newPassword = modal.find('#newPassword').val();
+				const confirmPassword = modal.find('#confirmPassword').val();
+				if (newPassword !== confirmPassword) {
+					throw new Error('[[[user:change_password_error_match]]');
+				}
+				await Promise.all(uids.map(uid => api.put('/users/' + uid + '/password', {
+					currentPassword: '',
+					newPassword: newPassword,
+				})));
+			}
+
+			const modal = bootbox.dialog({
+				message: `<div class="d-flex flex-column gap-2">
+					<label class="form-label">[[user:new_password]]</label>
+					<input id="newPassword" class="form-control" type="text">
+					<label class="form-label">[[user:confirm_password]]</label>
+					<input id="confirmPassword" class="form-control" type="text">
+				</div>`,
+				title: '[[admin/manage/users:change-password]]',
+				onEscape: true,
+				buttons: {
+					cancel: {
+						label: '[[admin/manage/users:alerts.button-cancel]]',
+						className: 'btn-link',
+					},
+					change: {
+						label: '[[admin/manage/users:alerts.button-change]]',
+						className: 'btn-primary',
+						callback: function () {
+							changePassword(modal).then(() => {
+								modal.modal('hide');
+							}).catch(alerts.error);
+							return false;
+						},
+					},
+				},
+			});
+		});
+
 		$('.password-reset-email').on('click', function () {
 			const uids = getSelectedUids();
 			if (!uids.length) {

public/src/admin/modules/change-email.js

@@ -0,0 +1,41 @@
+'use strict';
+
+define('admin/modules/change-email', [
+	'api', 'bootbox', 'alerts',
+], function (api, bootbox, alerts) {
+	const ChangeEmail = {};
+
+	ChangeEmail.init = function (params) {
+		const modal = bootbox.dialog({
+			message: `
+				<label class="form-label">[[admin/manage/users:new-email]]</label>
+				<input id="newEmail" class="form-control" type="text" value="${utils.escapeHTML(params.email || '')}">
+			`,
+			title: '[[admin/manage/users:change-email]]',
+			onEscape: true,
+			buttons: {
+				cancel: {
+					label: '[[admin/manage/users:alerts.button-cancel]]',
+					className: 'btn-link',
+				},
+				change: {
+					label: '[[admin/manage/users:alerts.button-change]]',
+					className: 'btn-primary',
+					callback: function () {
+						const newEmail = modal.find('#newEmail').val();
+						api.post('/users/' + params.uid + '/emails', {
+							skipConfirmation: true,
+							email: newEmail,
+						}).then(() => {
+							modal.modal('hide');
+							params.onSuccess(newEmail);
+						}).catch(alerts.error);
+						return false;
+					},
+				},
+			},
+		});
+	};
+
+	return ChangeEmail;
+});

public/src/client/account/edit.js

@@ -8,7 +8,8 @@ define('forum/account/edit', [
 	'hooks',
 	'bootbox',
 	'alerts',
-], function (header, picture, translator, api, hooks, bootbox, alerts) {
+	'admin/modules/change-email',
+], function (header, picture, translator, api, hooks, bootbox, alerts, changeEmail) {
 	const AccountEdit = {};
 
 	AccountEdit.init = function () {
@@ -25,6 +26,19 @@ define('forum/account/edit', [
 		updateSignature();
 		updateAboutMe();
 		handleGroupSort();
+
+		if (!ajaxify.data.isSelf && app.user.isAdmin) {
+			$(`a[href="${config.relative_path}/user/${ajaxify.data.userslug}/edit/email"]`).on('click', () => {
+				changeEmail.init({
+					uid: ajaxify.data.uid,
+					email: ajaxify.data.email,
+					onSuccess: function () {
+						alerts.success('[[user:email-updated]]');
+					},
+				});
+				return false;
+			});
+		}
 	};
 
 	function updateProfile() {

public/src/client/account/profile.js

@@ -15,7 +15,10 @@ define('forum/account/profile', [
 		processPage();
 
 		if (parseInt(ajaxify.data.emailChanged, 10) === 1) {
-			bootbox.alert('[[user:emailUpdate.change-instructions]]');
+			bootbox.alert({
+				message: '[[user:emailUpdate.change-instructions]]',
+				closeButton: false,
+			});
 		}
 
 		socket.removeListener('event:user_status_change', onUserStatusChange);

src/api/users.js

@@ -417,8 +417,15 @@ usersAPI.addEmail = async (caller, { email, skipConfirmation, uid }) => {
 	skipConfirmation = canManageUsers && skipConfirmation;
 
 	if (skipConfirmation) {
+		if (!email.length) {
+			await user.email.remove(uid);
+		} else {
+			if (!await user.email.available(email)) {
+				throw new Error('[[error:email-taken]]');
+			}
 			await user.setUserField(uid, 'email', email);
 			await user.email.confirmByUid(uid);
+		}
 	} else {
 		await usersAPI.update(caller, { uid, email });
 	}

src/controllers/accounts/edit.js

@@ -90,16 +90,7 @@ editController.username = async function (req, res, next) {
 
 editController.email = async function (req, res, next) {
 	const targetUid = await user.getUidByUserslug(req.params.userslug);
-	if (!targetUid) {
-		return next();
-	}
-
-	const [isAdminOrGlobalMod, canEdit] = await Promise.all([
-		user.isAdminOrGlobalMod(req.uid),
-		privileges.users.canEdit(req.uid, targetUid),
-	]);
-
-	if (!isAdminOrGlobalMod && !canEdit) {
+	if (!targetUid || req.uid !== parseInt(targetUid, 10)) {
 		return next();
 	}
 

src/user/email.js

@@ -40,7 +40,12 @@ UserEmail.remove = async function (uid, sessionId) {
 		db.sortedSetRemove('email:sorted', `${email.toLowerCase()}:${uid}`),
 		user.email.expireValidation(uid),
 		sessionId ? user.auth.revokeAllSessions(uid, sessionId) : Promise.resolve(),
-		events.log({ type: 'email-change', email, newEmail: '' }),
+		events.log({
+			targetUid: uid,
+			type: 'email-change',
+			email,
+			newEmail: '',
+		}),
 	]);
 };
 

src/user/interstitials.js

@@ -28,8 +28,7 @@ Interstitials.email = async (data) => {
 		return data;
 	}
 
-	const [canManageUsers, hasPassword, hasPending] = await Promise.all([
-		privileges.admin.can('admin:users', data.req.uid),
+	const [hasPassword, hasPending] = await Promise.all([
 		user.hasPassword(data.userData.uid),
 		user.email.isValidationPending(data.userData.uid),
 	]);
@@ -44,12 +43,7 @@ Interstitials.email = async (data) => {
 		data: {
 			email,
 			requireEmailAddress: meta.config.requireEmailAddress,
-			issuePasswordChallenge:
-				hasPassword &&
-				(
-					(canManageUsers && data.userData.uid === data.req.uid) || // admin changing own email
-					(!canManageUsers && !!data.userData.uid) // non-admins changing own email
-				),
+			issuePasswordChallenge: hasPassword,
 			hasPending,
 		},
 		callback: async (userData, formData) => {
@@ -73,7 +67,7 @@ Interstitials.email = async (data) => {
 					}),
 				]);
 
-				if (!canManageUsers && !isPasswordCorrect) {
+				if (!isPasswordCorrect) {
 					await sleep(2000);
 				}
 
@@ -92,14 +86,7 @@ Interstitials.email = async (data) => {
 					}
 
 					// Admins editing will auto-confirm, unless editing their own email
-					if (canManageUsers && userData.uid !== data.req.uid) {
-						if (!await user.email.available(formData.email)) {
-							throw new Error('[[error:email-taken]]');
-						}
-						await user.email.remove(userData.uid);
-						await user.setUserField(userData.uid, 'email', formData.email);
-						await user.email.confirmByUid(userData.uid);
-					} else if (canEdit) {
+					if (canEdit) {
 						if (hasPassword && !isPasswordCorrect) {
 							throw new Error('[[error:invalid-password]]');
 						}
@@ -120,8 +107,8 @@ Interstitials.email = async (data) => {
 						throw new Error('[[error:invalid-email]]');
 					}
 
-					if (current.length && (!hasPassword || (hasPassword && isPasswordCorrect) || canManageUsers)) {
-						// User or admin explicitly clearing their email
+					if (current.length && (!hasPassword || (hasPassword && isPasswordCorrect))) {
+						// User explicitly clearing their email
 						await user.email.remove(userData.uid, isSelf ? data.req.session.id : null);
 					}
 				}

src/views/admin/manage/users.tpl

@@ -39,22 +39,40 @@
 				</div>
 				<div class="btn-group">
 					<button class="btn btn-primary btn-sm dropdown-toggle" id="action-dropdown" data-bs-toggle="dropdown" type="button" disabled="disabled">[[admin/manage/users:edit]] <span class="caret"></span></button>
-					<ul class="dropdown-menu dropdown-menu-end p-1">
-						<li><a href="#" class="dropdown-item rounded-1 validate-email"><i class="fa fa-fw fa-check"></i> [[admin/manage/users:validate-email]]</a></li>
-						<li><a href="#" class="dropdown-item rounded-1 send-validation-email"><i class="fa fa-fw fa-mail-forward"></i> [[admin/manage/users:send-validation-email]]</a></li>
-						<li><a href="#" class="dropdown-item rounded-1 password-reset-email"><i class="fa fa-fw fa-key"></i> [[admin/manage/users:password-reset-email]]</a></li>
-						<li><a href="#" class="dropdown-item rounded-1 force-password-reset"><i class="fa fa-fw fa-unlock-alt"></i> [[admin/manage/users:force-password-reset]]</a></li>
-						<li><a href="#" class="dropdown-item rounded-1 manage-groups"><i class="fa fa-fw fa-users"></i> [[admin/manage/users:manage-groups]]</a></li>
-						<li><a href="#" class="dropdown-item rounded-1 set-reputation"><i class="fa fa-fw fa-star"></i> [[admin/manage/users:set-reputation]]</a></li>
-						<li class="dropdown-divider"></li>
-						<li><a href="#" class="dropdown-item rounded-1 ban-user"><i class="fa fa-fw fa-gavel"></i> [[admin/manage/users:ban]]</a></li>
-						<li><a href="#" class="dropdown-item rounded-1 ban-user-temporary"><i class="fa fa-fw fa-clock-o"></i> [[admin/manage/users:temp-ban]]</a></li>
-						<li><a href="#" class="dropdown-item rounded-1 unban-user"><i class="fa fa-fw fa-comment-o"></i> [[admin/manage/users:unban]]</a></li>
-						<li><a href="#" class="dropdown-item rounded-1 reset-lockout"><i class="fa fa-fw fa-unlock"></i> [[admin/manage/users:reset-lockout]]</a></li>
-						<li class="divider"></li>
-						<li><a href="#" class="dropdown-item rounded-1 delete-user"><i class="fa fa-fw fa-trash-o"></i> [[admin/manage/users:delete]]</a></li>
-						<li><a href="#" class="dropdown-item rounded-1 delete-user-content"><i class="fa fa-fw fa-trash-o"></i> [[admin/manage/users:delete-content]]</a></li>
-						<li><a href="#" class="dropdown-item rounded-1 delete-user-and-content"><i class="fa fa-fw fa-trash-o"></i> [[admin/manage/users:purge]]</a></li>
+					<ul class="dropdown-menu dropdown-menu-end p-1 text-sm">
+
+						<li><h6 class="dropdown-header">[[admin/manage/users:email]]</h6></li>
+						<li><a href="#" class="dropdown-item rounded-1 change-email"><i class="text-secondary fa fa-fw fa-envelope text-start"></i> [[admin/manage/users:change-email]]</a></li>
+						<li><a href="#" class="dropdown-item rounded-1 validate-email"><i class="text-secondary fa fa-fw fa-envelope-circle-check"></i> [[admin/manage/users:validate-email]]</a></li>
+						<li><a href="#" class="dropdown-item rounded-1 send-validation-email"><i class="text-secondary fa fa-fw fa-mail-forward"></i> [[admin/manage/users:send-validation-email]]</a></li>
+
+						<li><hr class="dropdown-divider"></li>
+
+						<li><h6 class="dropdown-header">[[admin/manage/users:password]]</h6></li>
+						<li><a href="#" class="dropdown-item rounded-1 change-password"><i class="text-secondary fa fa-fw fa-key"></i> [[admin/manage/users:change-password]]</a></li>
+						<li><a href="#" class="dropdown-item rounded-1 password-reset-email"><i class="text-secondary fa fa-fw fa-envelope-open-text"></i> [[admin/manage/users:password-reset-email]]</a></li>
+						<li><a href="#" class="dropdown-item rounded-1 force-password-reset"><i class="text-secondary fa fa-fw fa-user-lock"></i> [[admin/manage/users:force-password-reset]]</a></li>
+						<li><a href="#" class="dropdown-item rounded-1 reset-lockout"><i class="text-secondary fa fa-fw fa-lock-open"></i> [[admin/manage/users:reset-lockout]]</a></li>
+
+						<li><hr class="dropdown-divider"></li>
+
+						<li><h6 class="dropdown-header">[[admin/manage/users:manage]]</h6></li>
+						<li><a href="#" class="dropdown-item rounded-1 manage-groups"><i class="text-secondary fa fa-fw fa-users"></i> [[admin/manage/users:manage-groups]]</a></li>
+						<li><a href="#" class="dropdown-item rounded-1 set-reputation"><i class="text-secondary fa fa-fw fa-star"></i> [[admin/manage/users:set-reputation]]</a></li>
+
+						<li><hr class="dropdown-divider"></li>
+
+						<li><h6 class="dropdown-header">[[admin/manage/users:ban]]</h6></li>
+						<li><a href="#" class="dropdown-item rounded-1 ban-user"><i class="text-secondary fa fa-fw fa-gavel"></i> [[admin/manage/users:ban-users]]</a></li>
+						<li><a href="#" class="dropdown-item rounded-1 ban-user-temporary"><i class="text-secondary fa fa-fw fa-clock-o"></i> [[admin/manage/users:temp-ban]]</a></li>
+						<li><a href="#" class="dropdown-item rounded-1 unban-user"><i class="text-secondary fa fa-fw fa-comment-o"></i> [[admin/manage/users:unban]]</a></li>
+
+						<li><hr class="dropdown-divider"></li>
+
+						<li><h6 class="dropdown-header">[[admin/manage/users:delete]]</h6></li>
+						<li><a href="#" class="dropdown-item rounded-1 delete-user"><i class="text-secondary fa fa-fw fa-trash-o"></i> [[admin/manage/users:delete-users]]</a></li>
+						<li><a href="#" class="dropdown-item rounded-1 delete-user-content"><i class="text-secondary fa fa-fw fa-trash-o"></i> [[admin/manage/users:delete-content]]</a></li>
+						<li><a href="#" class="dropdown-item rounded-1 delete-user-and-content"><i class="text-secondary fa fa-fw fa-trash-o"></i> [[admin/manage/users:purge]]</a></li>
 					</ul>
 				</div>
 				<div class="btn-group">
@@ -107,34 +125,32 @@
 							</td>
 							<td class="text-nowrap">
 								<div class="d-flex flex-column gap-1">
-									{{{ if (!./email && !./emailToConfirm) }}}
-									<em class="text-muted">[[admin/manage/users:users.no-email]]</em>
-									{{{ else }}}
+									<em class="text-muted no-email {{{ if (./email || ./emailToConfirm) }}}hidden{{{ end }}} ">[[admin/manage/users:users.no-email]]</em>
+
 									<span class="validated {{{ if !users.email:confirmed }}} hidden{{{ end }}}">
 										<i class="fa fa-fw fa-check text-success" title="[[admin/manage/users:users.validated]]" data-bs-toggle="tooltip"></i>
-										{{{ if ./email }}}{./email}{{{ end }}}
+										<span class="email">{{{ if ./email }}}{./email}{{{ end }}}</span>
 									</span>
 
 									<span class="validated-by-admin hidden">
 										<i class="fa fa-fw fa-check text-success" title="[[admin/manage/users:users.validated]]" data-bs-toggle="tooltip"></i>
-										{{{ if ./emailToConfirm }}}{./emailToConfirm}{{{ end }}}
+										<span class="email">{{{ if ./emailToConfirm }}}{./emailToConfirm}{{{ end }}}</span>
 									</span>
 
-									<span class="pending {{{ if !users.email:pending }}} hidden{{{ end }}}">
+									<span class="pending {{{ if (!./emailToConfirm || !users.email:pending) }}} hidden{{{ end }}}">
 										<i class="fa fa-fw fa-clock-o text-warning" title="[[admin/manage/users:users.validation-pending]]" data-bs-toggle="tooltip"></i>
-										{./emailToConfirm}
+										<span class="email">{./emailToConfirm}</span>
 									</span>
 
-									<span class="expired {{{ if !users.email:expired }}} hidden{{{ end }}}">
+									<span class="expired {{{ if (!./emailToConfirm || !users.email:expired) }}} hidden{{{ end }}}">
 										<i class="fa fa-fw fa-times text-danger" title="[[admin/manage/users:users.validation-expired]]" data-bs-toggle="tooltip"></i>
-										{./emailToConfirm}
+										<span class="email">{./emailToConfirm}</span>
 									</span>
 
-									<span class="notvalidated {{{ if (users.email:expired || (users.email:pending || users.email:confirmed)) }}} hidden{{{ end }}}">
+									<span class="notvalidated {{{ if (!./emailToConfirm || (users.email:expired || (users.email:pending || users.email:confirmed))) }}} hidden{{{ end }}}">
 										<i class="fa fa-fw fa-times text-danger" title="[[admin/manage/users:users.not-validated]]" data-bs-toggle="tooltip"></i>
-										{./emailToConfirm}
+										<span class="email">{./emailToConfirm}</span>
 									</span>
-									{{{ end }}}
 								</div>
 							</td>
 							<td>

test/api.js

@@ -547,6 +547,11 @@ describe('API', async () => {
 						return;
 					}
 
+					if (response.statusCode === 400 && context[method].responses['400']) {
+						// TODO: check 400 schema to response.body?
+						return;
+					}
+
 					const http200 = context[method].responses['200'];
 					if (!http200) {
 						return;

test/controllers.js

@@ -455,22 +455,6 @@ describe('Controllers', () => {
 				assert.strictEqual(result.req.session.emailChanged, 1);
 			});
 
-			it('should set email if admin is changing it', async () => {
-				const uid = await user.create({ username: 'interstiuser3' });
-				const result = await user.interstitials.email({
-					userData: { uid: uid, updateEmail: true },
-					req: { uid: adminUid },
-					interstitials: [],
-				});
-
-				await result.interstitials[0].callback({ uid: uid }, {
-					email: 'interstiuser3@nodebb.org',
-				});
-				const userData = await user.getUserData(uid);
-				assert.strictEqual(userData.email, 'interstiuser3@nodebb.org');
-				assert.strictEqual(userData['email:confirmed'], 1);
-			});
-
 			it('should throw error if user tries to edit other users email', async () => {
 				const uid = await user.create({ username: 'interstiuser4' });
 				try {