Regression: Do not allow to remove users from federated DMs

apps/meteor/app/federation-v2/client/Federation.ts

@@ -1,4 +1,5 @@
-import { ValueOf } from '@rocket.chat/core-typings';
+import { RoomType } from '@rocket.chat/apps-engine/definition/rooms';
+import { IRoom, ValueOf } from '@rocket.chat/core-typings';
 
 import { RoomMemberActions } from '../../../definition/IRoomTypeConfig';
 
@@ -9,6 +10,8 @@ const allowedActionsInFederatedRooms: ValueOf<typeof RoomMemberActions>[] = [
 	RoomMemberActions.LEAVE,
 ];
 
-export const actionAllowed = (action: ValueOf<typeof RoomMemberActions>): boolean => {
-	return allowedActionsInFederatedRooms.includes(action);
+export const actionAllowed = (room: Partial<IRoom>, action: ValueOf<typeof RoomMemberActions>): boolean => {
+	return room.t === RoomType.DIRECT_MESSAGE && action === RoomMemberActions.REMOVE_USER
+		? false
+		: allowedActionsInFederatedRooms.includes(action);
 };

apps/meteor/app/federation-v2/server/infrastructure/rocket-chat/Federation.ts

@@ -1,3 +1,4 @@
+import { RoomType } from '@rocket.chat/apps-engine/definition/rooms';
 import { IRoom, ValueOf } from '@rocket.chat/core-typings';
 
 import { RoomMemberActions } from '../../../../../definition/IRoomTypeConfig';
@@ -14,8 +15,10 @@ export class Federation {
 		return room.federated === true;
 	}
 
-	public static federationActionAllowed(action: ValueOf<typeof RoomMemberActions>): boolean {
-		return allowedActionsInFederatedRooms.includes(action);
+	public static actionAllowed(room: IRoom, action: ValueOf<typeof RoomMemberActions>): boolean {
+		return room.t === RoomType.DIRECT_MESSAGE && action === RoomMemberActions.REMOVE_USER
+			? false
+			: allowedActionsInFederatedRooms.includes(action);
 	}
 
 	public static isAFederatedUsername(username: string): boolean {

apps/meteor/client/lib/rooms/roomTypes/direct.ts

@@ -38,7 +38,7 @@ roomCoordinator.add(DirectMessageRoomType, {
 
 	allowMemberAction(room, action) {
 		if (isRoomFederated(room as IRoom)) {
-			return Federation.actionAllowed(action);
+			return Federation.actionAllowed(room, action);
 		}
 		switch (action) {
 			case RoomMemberActions.BLOCK:

apps/meteor/client/lib/rooms/roomTypes/private.ts

@@ -35,7 +35,7 @@ roomCoordinator.add(PrivateRoomType, {
 
 	allowMemberAction(_room, action) {
 		if (isRoomFederated(_room as IRoom)) {
-			return Federation.actionAllowed(action);
+			return Federation.actionAllowed(_room, action);
 		}
 		switch (action) {
 			case RoomMemberActions.BLOCK:

apps/meteor/client/lib/rooms/roomTypes/public.ts

@@ -33,7 +33,7 @@ roomCoordinator.add(PublicRoomType, {
 
 	allowMemberAction(_room, action) {
 		if (isRoomFederated(_room as IRoom)) {
-			return Federation.actionAllowed(action);
+			return Federation.actionAllowed(_room, action);
 		}
 		switch (action) {
 			case RoomMemberActions.BLOCK:

apps/meteor/server/lib/rooms/roomTypes/direct.ts

@@ -40,7 +40,7 @@ roomCoordinator.add(DirectMessageRoomType, {
 
 	allowMemberAction(room: IRoom, action) {
 		if (isRoomFederated(room)) {
-			return Federation.federationActionAllowed(action);
+			return Federation.actionAllowed(room, action);
 		}
 		switch (action) {
 			case RoomMemberActions.BLOCK:

apps/meteor/server/lib/rooms/roomTypes/private.ts

@@ -29,7 +29,7 @@ roomCoordinator.add(PrivateRoomType, {
 
 	allowMemberAction(_room, action) {
 		if (isRoomFederated(_room as IRoom)) {
-			return Federation.federationActionAllowed(action);
+			return Federation.actionAllowed(_room, action);
 		}
 		switch (action) {
 			case RoomMemberActions.BLOCK:

apps/meteor/server/lib/rooms/roomTypes/public.ts

@@ -28,7 +28,7 @@ roomCoordinator.add(PublicRoomType, {
 
 	allowMemberAction(_room, action) {
 		if (isRoomFederated(_room as IRoom)) {
-			return Federation.federationActionAllowed(action);
+			return Federation.actionAllowed(_room, action);
 		}
 		switch (action) {
 			case RoomMemberActions.BLOCK:

apps/meteor/tests/unit/app/federation-v2/client/unit/Federation.spec.ts

@@ -10,15 +10,23 @@ describe('Federation[Client] - Federation', () => {
 		Object.values(RoomMemberActions)
 			.filter((action) => !allowedActions.includes(action as any))
 			.forEach((action) => {
-				it('should return false if the action is NOT allowed within the federation context', () => {
-					expect(Federation.actionAllowed(action)).to.be.false;
+				it('should return false if the action is NOT allowed within the federation context for regular channels', () => {
+					expect(Federation.actionAllowed({ t: 'c' } as any, action)).to.be.false;
 				});
 			});
 
 		allowedActions.forEach((action) => {
-			it('should return true if the action is allowed within the federation context', () => {
-				expect(Federation.actionAllowed(action)).to.be.true;
+			it('should return true if the action is allowed within the federation context for regular channels', () => {
+				expect(Federation.actionAllowed({ t: 'c' } as any, action)).to.be.true;
 			});
 		});
+
+		it('should return false if the action is equal to REMOVE_USER and the channel is a DM', () => {
+			expect(Federation.actionAllowed({ t: 'd' } as any, RoomMemberActions.REMOVE_USER)).to.be.false;
+		});
+
+		it('should return true if the action is equal to any other action except REMOVE_USER and the channel is a DM', () => {
+			expect(Federation.actionAllowed({ t: 'd' } as any, RoomMemberActions.INVITE)).to.be.true;
+		});
 	});
 });

apps/meteor/tests/unit/app/federation-v2/server/unit/Federation.spec.ts

@@ -18,21 +18,29 @@ describe('Federation[Server] - Federation', () => {
 		});
 	});
 
-	describe('#federationActionAllowed()', () => {
+	describe('#actionAllowed()', () => {
 		const allowedActions = [RoomMemberActions.REMOVE_USER, RoomMemberActions.INVITE, RoomMemberActions.JOIN, RoomMemberActions.LEAVE];
 
 		Object.values(RoomMemberActions)
 			.filter((action) => !allowedActions.includes(action as any))
 			.forEach((action) => {
-				it('should return false if the action is NOT allowed within the federation context', () => {
-					expect(Federation.federationActionAllowed(action)).to.be.false;
+				it('should return false if the action is NOT allowed within the federation context for regular channels', () => {
+					expect(Federation.actionAllowed({ t: 'c' } as any, action)).to.be.false;
 				});
 			});
 
 		allowedActions.forEach((action) => {
-			it('should return true if the action is allowed within the federation context', () => {
-				expect(Federation.federationActionAllowed(action)).to.be.true;
+			it('should return true if the action is allowed within the federation context for regular channels', () => {
+				expect(Federation.actionAllowed({ t: 'c' } as any, action)).to.be.true;
 			});
 		});
+
+		it('should return false if the action is equal to REMOVE_USER and the channel is a DM', () => {
+			expect(Federation.actionAllowed({ t: 'd' } as any, RoomMemberActions.REMOVE_USER)).to.be.false;
+		});
+
+		it('should return true if the action is equal to any other action except REMOVE_USER and the channel is a DM', () => {
+			expect(Federation.actionAllowed({ t: 'd' } as any, RoomMemberActions.INVITE)).to.be.true;
+		});
 	});
 });