[NEW] Assign oldest active user as owner when deleting last room owner (

#16088)

Co-authored-by: Diego Sampaio <chinello@gmail.com>

app/api/server/v1/users.js

@@ -76,9 +76,10 @@ API.v1.addRoute('users.delete', { authRequired: true }, {
 		}
 
 		const user = this.getUserFromParams();
+		const { confirmRelinquish = false } = this.requestParams();
 
 		Meteor.runAsUser(this.userId, () => {
-			Meteor.call('deleteUser', user._id);
+			Meteor.call('deleteUser', user._id, confirmRelinquish);
 		});
 
 		return API.v1.success();
@@ -122,14 +123,15 @@ API.v1.addRoute('users.setActiveStatus', { authRequired: true }, {
 		check(this.bodyParams, {
 			userId: String,
 			activeStatus: Boolean,
+			confirmRelinquish: Match.Maybe(Boolean),
 		});
 
 		if (!hasPermission(this.userId, 'edit-other-user-active-status')) {
 			return API.v1.unauthorized();
 		}
 
 		Meteor.runAsUser(this.userId, () => {
-			Meteor.call('setUserActiveStatus', this.bodyParams.userId, this.bodyParams.activeStatus);
+			Meteor.call('setUserActiveStatus', this.bodyParams.userId, this.bodyParams.activeStatus, this.bodyParams.confirmRelinquish);
 		});
 		return API.v1.success({ user: Users.findOneById(this.bodyParams.userId, { fields: { active: 1 } }) });
 	},

app/authorization/server/functions/hasRole.js

@@ -6,3 +6,5 @@ export const hasRoleAsync = async (userId, roleNames, scope) => {
 };
 
 export const hasRole = (userId, roleNames, scope) => Promise.await(hasRoleAsync(userId, roleNames, scope));
+
+export const subscriptionHasRole = (sub, role) => sub.roles && sub.roles.includes(role);

app/authorization/server/index.js

@@ -12,7 +12,7 @@ import {
 	hasAtLeastOnePermission,
 	hasPermission,
 } from './functions/hasPermission';
-import { hasRole } from './functions/hasRole';
+import { hasRole, subscriptionHasRole } from './functions/hasRole';
 import { removeUserFromRoles } from './functions/removeUserFromRoles';
 import { AuthorizationUtils } from '../lib/AuthorizationUtils';
 import './methods/addPermissionToRole';
@@ -28,6 +28,7 @@ export {
 	getRoles,
 	getUsersInRole,
 	hasRole,
+	subscriptionHasRole,
 	removeUserFromRoles,
 	canSendMessage,
 	addRoomAccessValidator,

app/lib/server/functions/deleteUser.js

@@ -3,23 +3,14 @@ import { TAPi18n } from 'meteor/rocketchat:tap-i18n';
 
 import { FileUpload } from '../../../file-upload/server';
 import { Users, Subscriptions, Messages, Rooms, Integrations, FederationServers } from '../../../models/server';
-import { hasRole, getUsersInRole } from '../../../authorization/server';
 import { settings } from '../../../settings/server';
 import { Notifications } from '../../../notifications/server';
 import { updateGroupDMsName } from './updateGroupDMsName';
+import { relinquishRoomOwnerships } from './relinquishRoomOwnerships';
+import { getSubscribedRoomsForUserWithDetails, shouldRemoveOrChangeOwner } from './getRoomsWithSingleOwner';
+import { getUserSingleOwnedRooms } from './getUserSingleOwnedRooms';
 
-const bulkRoomCleanUp = (rids) => {
-	// no bulk deletion for files
-	rids.forEach((rid) => FileUpload.removeFilesByRoomId(rid));
-
-	return Promise.await(Promise.all([
-		Subscriptions.removeByRoomIds(rids),
-		Messages.removeByRoomIds(rids),
-		Rooms.removeByIds(rids),
-	]));
-};
-
-export const deleteUser = function(userId) {
+export const deleteUser = function(userId, confirmRelinquish = false) {
 	const user = Users.findOneById(userId, {
 		fields: { username: 1, avatarOrigin: 1, federation: 1 },
 	});
@@ -36,47 +27,16 @@ export const deleteUser = function(userId) {
 		}
 	}
 
+	const subscribedRooms = getSubscribedRoomsForUserWithDetails(userId);
+
+	if (shouldRemoveOrChangeOwner(subscribedRooms) && !confirmRelinquish) {
+		const rooms = getUserSingleOwnedRooms(subscribedRooms);
+		throw new Meteor.Error('user-last-owner', '', rooms);
+	}
+
 	// Users without username can't do anything, so there is nothing to remove
 	if (user.username != null) {
-		const roomCache = [];
-
-		// Iterate through all the rooms the user is subscribed to, to check if they are the last owner of any of them.
-		Subscriptions.db.findByUserId(userId).forEach((subscription) => {
-			const roomData = {
-				rid: subscription.rid,
-				t: subscription.t,
-				subscribers: null,
-			};
-
-			// DMs can always be deleted, so let's ignore it on this check
-			if (roomData.t !== 'd') {
-				// If the user is an owner on this room
-				if (hasRole(user._id, 'owner', subscription.rid)) {
-					// Fetch the number of owners
-					const numOwners = getUsersInRole('owner', subscription.rid).fetch().length;
-					// If it's only one, then this user is the only owner.
-					if (numOwners === 1) {
-						// If the user is the last owner of a public channel, then we need to abort the deletion
-						if (roomData.t === 'c') {
-							throw new Meteor.Error('error-user-is-last-owner', `To delete this user you'll need to set a new owner to the following room: ${ subscription.name }.`, {
-								method: 'deleteUser',
-							});
-						}
-
-						// For private groups, let's check how many subscribers it has. If the user is the only subscriber, then it will be eliminated and doesn't need to abort the deletion
-						roomData.subscribers = Subscriptions.findByRoomId(subscription.rid).count();
-
-						if (roomData.subscribers > 1) {
-							throw new Meteor.Error('error-user-is-last-owner', `To delete this user you'll need to set a new owner to the following room: ${ subscription.name }.`, {
-								method: 'deleteUser',
-							});
-						}
-					}
-				}
-			}
-
-			roomCache.push(roomData);
-		});
+		relinquishRoomOwnerships(userId, subscribedRooms);
 
 		const messageErasureType = settings.get('Message_ErasureType');
 		switch (messageErasureType) {
@@ -94,19 +54,6 @@ export const deleteUser = function(userId) {
 				break;
 		}
 
-		const roomIds = roomCache.filter((roomData) => {
-			if (roomData.subscribers === null && roomData.t !== 'd' && roomData.t !== 'c') {
-				roomData.subscribers = Subscriptions.findByRoomId(roomData.rid).count();
-			}
-
-			// Remove non-channel rooms with only 1 user (the one being deleted)
-			return roomData.t !== 'c' && roomData.subscribers === 1;
-		}).map(({ rid }) => rid);
-
-		Rooms.find1On1ByUserId(user._id, { fields: { _id: 1 } }).forEach(({ _id }) => roomIds.push(_id));
-
-		bulkRoomCleanUp(roomIds);
-
 		Rooms.updateGroupDMsRemovingUsernamesByUsername(user.username); // Remove direct rooms with the user
 		Rooms.removeDirectRoomContainingUsername(user.username); // Remove direct rooms with the user
 

app/lib/server/functions/getRoomsWithSingleOwner.js

@@ -0,0 +1,60 @@
+import { subscriptionHasRole } from '../../../authorization/server';
+import { Users, Subscriptions } from '../../../models/server';
+
+export function shouldRemoveOrChangeOwner(subscribedRooms) {
+	return subscribedRooms
+		.some(({ shouldBeRemoved, shouldChangeOwner }) => shouldBeRemoved || shouldChangeOwner);
+}
+
+export function getSubscribedRoomsForUserWithDetails(userId) {
+	const subscribedRooms = [];
+
+	// Iterate through all the rooms the user is subscribed to, to check if he is the last owner of any of them.
+	Subscriptions.findByUserIdExceptType(userId, 'd').forEach((subscription) => {
+		const roomData = {
+			rid: subscription.rid,
+			t: subscription.t,
+			shouldBeRemoved: false,
+			shouldChangeOwner: false,
+			newOwner: null,
+		};
+
+		if (subscriptionHasRole(subscription, 'owner')) {
+			// Fetch the number of owners
+			const numOwners = Subscriptions.findByRoomIdAndRoles(subscription.rid, ['owner']).count();
+
+			// If it's only one, then this user is the only owner.
+			if (numOwners === 1) {
+				// Let's check how many subscribers the room has.
+				const options = { fields: { 'u._id': 1 }, sort: { ts: 1 } };
+				const subscribersCursor = Subscriptions.findByRoomId(subscription.rid, options);
+
+				subscribersCursor.forEach(({ u: { _id: uid } }) => {
+					// If we already changed the owner or this subscription is for the user we are removing, then don't try to give it ownership
+					if (roomData.shouldChangeOwner || uid === userId) {
+						return;
+					}
+					const newOwner = Users.findOneActiveById(uid, { fields: { _id: 1 } });
+					if (!newOwner) {
+						return;
+					}
+
+					roomData.newOwner = uid;
+					roomData.shouldChangeOwner = true;
+				});
+
+				// If there's no subscriber available to be the new owner and it's not a public room, we can remove it.
+				if (!roomData.shouldChangeOwner && roomData.t !== 'c') {
+					roomData.shouldBeRemoved = true;
+				}
+			}
+		} else if (roomData.t !== 'c') {
+			// If the user is not an owner, remove the room if the user is the only subscriber
+			roomData.shouldBeRemoved = Subscriptions.findByRoomId(roomData.rid).count() === 1;
+		}
+
+		subscribedRooms.push(roomData);
+	});
+
+	return subscribedRooms;
+}

app/lib/server/functions/getUserSingleOwnedRooms.js

@@ -0,0 +1,25 @@
+import { Rooms } from '../../../models/server';
+
+export const getUserSingleOwnedRooms = function(subscribedRooms) {
+	const roomsThatWillChangeOwner = subscribedRooms.filter(({ shouldChangeOwner }) => shouldChangeOwner).map(({ rid }) => rid);
+	const roomsThatWillBeRemoved = subscribedRooms.filter(({ shouldBeRemoved }) => shouldBeRemoved).map(({ rid }) => rid);
+
+	const roomIds = roomsThatWillBeRemoved.concat(roomsThatWillChangeOwner);
+	const rooms = Rooms.findByIds(roomIds, { fields: { _id: 1, name: 1, fname: 1 } });
+
+	const result = {
+		shouldBeRemoved: [],
+		shouldChangeOwner: [],
+	};
+
+	rooms.forEach((room) => {
+		const name = room.fname || room.name;
+		if (roomsThatWillBeRemoved.includes(room._id)) {
+			result.shouldBeRemoved.push(name);
+		} else {
+			result.shouldChangeOwner.push(name);
+		}
+	});
+
+	return result;
+};

app/lib/server/functions/index.js

@@ -12,12 +12,14 @@ export { deleteRoom } from './deleteRoom';
 export { deleteUser } from './deleteUser';
 export { getFullUserData } from './getFullUserData';
 export { getRoomByNameOrIdWithOptionToJoin } from './getRoomByNameOrIdWithOptionToJoin';
+export { getUserSingleOwnedRooms } from './getUserSingleOwnedRooms';
 export { generateUsernameSuggestion } from './getUsernameSuggestion';
 export { insertMessage } from './insertMessage';
 export { isTheLastMessage } from './isTheLastMessage';
 export { loadMessageHistory } from './loadMessageHistory';
 export { processWebhookMessage } from './processWebhookMessage';
 export { removeUserFromRoom } from './removeUserFromRoom';
+export { relinquishRoomOwnerships } from './relinquishRoomOwnerships';
 export { saveCustomFields } from './saveCustomFields';
 export { saveCustomFieldsWithoutValidation } from './saveCustomFieldsWithoutValidation';
 export { saveUser } from './saveUser';

app/lib/server/functions/relinquishRoomOwnerships.js

@@ -0,0 +1,30 @@
+import { FileUpload } from '../../../file-upload/server';
+import { Subscriptions, Messages, Rooms, Roles } from '../../../models/server';
+
+const bulkRoomCleanUp = (rids) => {
+	// no bulk deletion for files
+	rids.forEach((rid) => FileUpload.removeFilesByRoomId(rid));
+
+	return Promise.await(Promise.all([
+		Subscriptions.removeByRoomIds(rids),
+		Messages.removeByRoomIds(rids),
+		Rooms.removeByIds(rids),
+	]));
+};
+
+export const relinquishRoomOwnerships = function(userId, subscribedRooms, removeDirectMessages = true) {
+	// change owners
+	subscribedRooms
+		.filter(({ shouldChangeOwner }) => shouldChangeOwner)
+		.forEach(({ newOwner, rid }) => Roles.addUserRoles(newOwner, ['owner'], rid));
+
+	const roomIdsToRemove = subscribedRooms.filter(({ shouldBeRemoved }) => shouldBeRemoved).map(({ rid }) => rid);
+
+	if (removeDirectMessages) {
+		Rooms.find1On1ByUserId(userId, { fields: { _id: 1 } }).forEach(({ _id }) => roomIdsToRemove.push(_id));
+	}
+
+	bulkRoomCleanUp(roomIdsToRemove);
+
+	return subscribedRooms;
+};

app/lib/server/functions/setUserActiveStatus.js

@@ -1,11 +1,15 @@
+import { Meteor } from 'meteor/meteor';
 import { check } from 'meteor/check';
 import { Accounts } from 'meteor/accounts-base';
 
 import * as Mailer from '../../../mailer';
 import { Users, Subscriptions } from '../../../models';
 import { settings } from '../../../settings';
+import { relinquishRoomOwnerships } from './relinquishRoomOwnerships';
+import { shouldRemoveOrChangeOwner, getSubscribedRoomsForUserWithDetails } from './getRoomsWithSingleOwner';
+import { getUserSingleOwnedRooms } from './getUserSingleOwnedRooms';
 
-export function setUserActiveStatus(userId, active) {
+export function setUserActiveStatus(userId, active, confirmRelinquish = false) {
 	check(userId, String);
 	check(active, Boolean);
 
@@ -15,6 +19,18 @@ export function setUserActiveStatus(userId, active) {
 		return false;
 	}
 
+	// Users without username can't do anything, so there is no need to check for owned rooms
+	if (user.username != null && !active) {
+		const subscribedRooms = getSubscribedRoomsForUserWithDetails(userId);
+
+		if (shouldRemoveOrChangeOwner(subscribedRooms) && !confirmRelinquish) {
+			const rooms = getUserSingleOwnedRooms(subscribedRooms);
+			throw new Meteor.Error('user-last-owner', '', rooms);
+		}
+
+		relinquishRoomOwnerships(user._id, subscribedRooms, false);
+	}
+
 	Users.setUserActive(userId, active);
 
 	if (user.username) {

app/lib/server/methods/deleteUserOwnAccount.js

@@ -8,7 +8,7 @@ import { Users } from '../../../models';
 import { deleteUser } from '../functions';
 
 Meteor.methods({
-	deleteUserOwnAccount(password) {
+	deleteUserOwnAccount(password, confirmRelinquish) {
 		check(password, String);
 
 		if (!Meteor.userId()) {
@@ -38,9 +38,7 @@ Meteor.methods({
 			throw new Meteor.Error('error-invalid-username', 'Invalid username', { method: 'deleteUserOwnAccount' });
 		}
 
-		Meteor.defer(function() {
-			deleteUser(userId);
-		});
+		deleteUser(userId, confirmRelinquish);
 
 		return true;
 	},

app/models/server/models/Subscriptions.js

@@ -472,6 +472,15 @@ export class Subscriptions extends Base {
 		return this.find(query, options);
 	}
 
+	findByUserIdExceptType(userId, typeException, options) {
+		const query = {
+			'u._id': userId,
+			t: { $ne: typeException },
+		};
+
+		return this.find(query, options);
+	}
+
 	findByUserIdAndType(userId, type, options) {
 		const query = {
 			'u._id': userId,

app/models/server/models/Users.js

@@ -549,6 +549,15 @@ export class Users extends Base {
 		return this.findOne(query, options);
 	}
 
+	findOneActiveById(userId, options) {
+		const query = {
+			_id: userId,
+			active: true,
+		};
+
+		return this.findOne(query, options);
+	}
+
 	findOneByIdOrUsername(idOrUsername, options) {
 		const query = {
 			$or: [{