fix(federation): enhance user authorization checks and update end-to-end tests for federated room creation

Author: ggazzo

apps/meteor/app/lib/server/functions/createRoom.ts

@@ -2,7 +2,7 @@ import { AppEvents, Apps } from '@rocket.chat/apps';
 import { AppsEngineException } from '@rocket.chat/apps-engine/definition/exceptions';
 import { FederationMatrix, Message, Room, Team } from '@rocket.chat/core-services';
 import type { ICreateRoomParams, ISubscriptionExtraData } from '@rocket.chat/core-services';
-import type { ICreatedRoom, IUser, IRoom, RoomType } from '@rocket.chat/core-typings';
+import { type ICreatedRoom, type IUser, type IRoom, type RoomType, isUserNativeFederated } from '@rocket.chat/core-typings';
 import { Rooms, Subscriptions, Users } from '@rocket.chat/models';
 import { Meteor } from 'meteor/meteor';
 
@@ -184,7 +184,12 @@ export const createRoom = async <T extends RoomType>(
 
 	const shouldBeHandledByFederation = extraData.federated === true;
 
-	if (shouldBeHandledByFederation && owner && !(await hasPermissionAsync(owner._id, 'access-federation'))) {
+	if (
+		shouldBeHandledByFederation &&
+		owner &&
+		!isUserNativeFederated(owner) &&
+		!(await hasPermissionAsync(owner._id, 'access-federation'))
+	) {
 		throw new Meteor.Error('error-not-authorized-federation', 'Not authorized to access federation', {
 			method: 'createRoom',
 		});

ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts

@@ -1,11 +1,12 @@
-import type { IUser } from '@rocket.chat/core-typings';
+import type { ISubscription, IUser } from '@rocket.chat/core-typings';
 
 import type {} from '../../../../../apps/meteor/app/api/server/v1/permissions.ts';
 import { api } from '../../../../../apps/meteor/tests/data/api-data';
-import { addUserToRoom, createRoom } from '../../../../../apps/meteor/tests/data/rooms.helper';
+import { addUserToRoom, createRoom, getSubscriptions } from '../../../../../apps/meteor/tests/data/rooms.helper';
 import { createUser, deleteUser, getRequestConfig } from '../../../../../apps/meteor/tests/data/users.helper';
 import type { IRequestConfig, TestUser } from '../../../../../apps/meteor/tests/data/users.helper';
 import { IS_EE } from '../../../../../apps/meteor/tests/e2e/config/constants';
+import { retry } from '../../../../../apps/meteor/tests/end-to-end/api/helpers/retry.ts';
 import { federationConfig } from '../helper/config';
 import { SynapseClient } from '../helper/synapse-client';
 
@@ -40,88 +41,135 @@ import { SynapseClient } from '../helper/synapse-client';
 			.expect(200);
 	});
 
-	afterAll(async () => {
+	afterAll(async () =>
 		// Add permissions for access-federation to any user but admin
-		await rc1AdminRequestConfig.request
+		rc1AdminRequestConfig.request
 			.post(api('permissions.update'))
 			.set(rc1AdminRequestConfig.credentials)
 			.send({ permissions: [{ _id: 'access-federation', roles: ['admin', 'user'] }] })
 			.expect('Content-Type', 'application/json')
-			.expect(200);
-	});
+			.expect(200),
+	);
 
-	afterAll(async () => {
-		await hs1AdminApp.close();
-	});
+	afterAll(async () => hs1AdminApp.close());
 
 	describe('Access Federation Permission', () => {
-		it('should throw an error if a user without access-federation permission tries to create a federated room', async () => {
-			const channelName = `federated-room-${Date.now()}`;
-			const createResponse = await createRoom({
-				type: 'p',
-				name: channelName,
-				members: [],
-				extraData: {
-					federated: true,
-				},
-				config: rc1User1RequestConfig,
+		describe('Users without access-federation permission', () => {
+			beforeAll(async () => {
+				await rc1AdminRequestConfig.request
+					.post(api('permissions.update'))
+					.set(rc1AdminRequestConfig.credentials)
+					.send({ permissions: [{ _id: 'access-federation', roles: ['admin'] }] })
+					.expect('Content-Type', 'application/json')
+					.expect(200);
 			});
 
-			expect(createResponse.status).toBe(400);
-			expect(createResponse.body).toHaveProperty('success', false);
-			expect(createResponse.body).toHaveProperty('errorType', 'error-not-authorized-federation');
-		});
-
-		it('should be able to create a federated room if the user has access-federation permission', async () => {
-			const channelName = `federated-room-${Date.now()}`;
-			const createResponse = await createRoom({
-				type: 'p',
-				name: channelName,
-				members: [],
-				extraData: {
-					federated: true,
-				},
-				config: rc1AdminRequestConfig,
+			afterAll(async () => {
+				// Add permissions for access-federation to any user but admin
+				await rc1AdminRequestConfig.request
+					.post(api('permissions.update'))
+					.set(rc1AdminRequestConfig.credentials)
+					.send({ permissions: [{ _id: 'access-federation', roles: ['admin', 'user'] }] })
+					.expect('Content-Type', 'application/json')
+					.expect(200);
 			});
 
-			expect(createResponse.status).toBe(200);
-			expect(createResponse.body).toHaveProperty('success', true);
-			expect(createResponse.body).toHaveProperty('group');
-			expect(createResponse.body.group).toHaveProperty('_id');
-			expect(createResponse.body.group).toHaveProperty('t', 'p');
-			expect(createResponse.body.group).toHaveProperty('federated', true);
-		});
+			describe('Inviting from a remote server', () => {
+				let user: TestUser<IUser>;
 
-		it('should not be able to add a user without access-federation permission to a room', async () => {
-			const createResponse = await createRoom({
-				type: 'p',
-				name: `federated-room-${Date.now()}`,
-				members: [],
-				extraData: {
-					federated: true,
-				},
-				config: rc1AdminRequestConfig,
+				let matrixRoomId: string;
+
+				beforeAll(async () => {
+					user = await createUser(
+						{
+							username: `g3-${Date.now()}`,
+							password: '1',
+							roles: ['user'],
+						},
+						rc1AdminRequestConfig,
+					);
+				});
+
+				afterAll(async () => {
+					await deleteUser(user, {}, rc1AdminRequestConfig);
+				});
+
+				let channelName: string;
+
+				beforeAll(async () => {
+					channelName = `federated-room-${Date.now()}`;
+					matrixRoomId = await hs1AdminApp.createRoom(channelName);
+				});
+
+				it('should throw an error if a user without access-federation permission tries to invite a user to a room', async () => {
+					await expect(hs1AdminApp.matrixClient.invite(matrixRoomId, `@${user.username}:${federationConfig.rc1.url}`)).rejects.toThrow();
+					const subscriptions = await getSubscriptions(rc1AdminRequestConfig);
+					const invitedSub = subscriptions.update.find((sub) => sub.fname?.includes(channelName));
+					expect(invitedSub).toBeUndefined();
+				});
+
+				it('should be able to invite a user to a room if the user has access-federation permission', async () => {
+					await expect(hs1AdminApp.matrixClient.invite(matrixRoomId, federationConfig.rc1.adminMatrixUserId)).resolves.not.toThrow();
+
+					retry('waiting for invitation to be processed', async () => {
+						const subscriptions = await getSubscriptions(rc1AdminRequestConfig);
+
+						const pendingInvitation = subscriptions.update.find(
+							(subscription) => subscription.status === 'INVITED' && subscription.fname?.includes(channelName),
+						);
+						expect(pendingInvitation).not.toBeUndefined();
+					});
+				});
 			});
 
-			expect(createResponse.status).toBe(200);
-			expect(createResponse.body).toHaveProperty('success', true);
-			expect(createResponse.body).toHaveProperty('group');
-			expect(createResponse.body.group).toHaveProperty('_id');
-			expect(createResponse.body.group).toHaveProperty('t', 'p');
-			expect(createResponse.body.group).toHaveProperty('federated', true);
-
-			const addUserResponse = await addUserToRoom({
-				usernames: [federationConfig.hs1.adminMatrixUserId],
-				rid: createResponse.body.group._id,
-				config: rc1User1RequestConfig,
+			it('should throw an error if a user without access-federation permission tries to create a federated room', async () => {
+				const channelName = `federated-room-${Date.now()}`;
+				const createResponse = await createRoom({
+					type: 'p',
+					name: channelName,
+					members: [],
+					extraData: {
+						federated: true,
+					},
+					config: rc1User1RequestConfig,
+				});
+
+				expect(createResponse.status).toBe(400);
+				expect(createResponse.body).toHaveProperty('success', false);
+				expect(createResponse.body).toHaveProperty('errorType', 'error-not-authorized-federation');
 			});
 
-			expect(addUserResponse.status).toBe(200);
-			expect(addUserResponse.body).toHaveProperty('success', true);
-			expect(addUserResponse.body.message).toMatch(/error-not-allowed/);
+			it('should not be able to add a user without access-federation permission to a room', async () => {
+				const createResponse = await createRoom({
+					type: 'p',
+					name: `federated-room-${Date.now()}`,
+					members: [],
+					extraData: {
+						federated: true,
+					},
+					config: rc1AdminRequestConfig,
+				});
+
+				expect(createResponse.status).toBe(200);
+				expect(createResponse.body).toHaveProperty('success', true);
+				expect(createResponse.body).toHaveProperty('group');
+				expect(createResponse.body.group).toHaveProperty('_id');
+				expect(createResponse.body.group).toHaveProperty('t', 'p');
+				expect(createResponse.body.group).toHaveProperty('federated', true);
+
+				const addUserResponse = await addUserToRoom({
+					usernames: [federationConfig.hs1.adminMatrixUserId],
+					rid: createResponse.body.group._id,
+					config: rc1User1RequestConfig,
+				});
+
+				expect(addUserResponse.status).toBe(200);
+				expect(addUserResponse.body).toHaveProperty('success', true);
+				expect(addUserResponse.body.message).toMatch(/error-not-allowed/);
+			});
 		});
 
-		describe('Add a user with access-federation permission to a room', () => {
+		describe('Users with access-federation permission', () => {
 			let user: TestUser<IUser>;
 
 			beforeAll(async () => {
@@ -133,23 +181,17 @@ import { SynapseClient } from '../helper/synapse-client';
 					},
 					rc1AdminRequestConfig,
 				);
-
-				await rc1AdminRequestConfig.request
-					.post(api('permissions.update'))
-					.set(rc1AdminRequestConfig.credentials)
-					.send({ permissions: [{ _id: 'access-federation', roles: ['admin', 'user'] }] })
-					.expect('Content-Type', 'application/json')
-					.expect(200);
 			});
 
 			afterAll(async () => {
 				await deleteUser(user, {}, rc1AdminRequestConfig);
 			});
 
-			it('should be able to add a user with access-federation permission to a room', async () => {
+			it('should be able to create a federated room if the user has access-federation permission', async () => {
+				const channelName = `federated-room-${Date.now()}`;
 				const createResponse = await createRoom({
 					type: 'p',
-					name: `federated-room-${Date.now()}`,
+					name: channelName,
 					members: [],
 					extraData: {
 						federated: true,
@@ -158,16 +200,46 @@ import { SynapseClient } from '../helper/synapse-client';
 				});
 
 				expect(createResponse.status).toBe(200);
-				const addUserResponse = await addUserToRoom({
-					usernames: [user.username],
-					rid: createResponse.body.group._id,
-					config: rc1AdminRequestConfig,
-				});
+				expect(createResponse.body).toHaveProperty('success', true);
+				expect(createResponse.body).toHaveProperty('group');
+				expect(createResponse.body.group).toHaveProperty('_id');
+				expect(createResponse.body.group).toHaveProperty('t', 'p');
+				expect(createResponse.body.group).toHaveProperty('federated', true);
+			});
 
-				expect(addUserResponse.status).toBe(200);
-				expect(addUserResponse.body).toHaveProperty('success', true);
-				expect(addUserResponse.body).toHaveProperty('message');
-				expect(addUserResponse.body.message).toMatch('{"msg":"result","id":"id","result":true}');
+			describe('Add a user with access-federation permission to a room', () => {
+				beforeAll(async () =>
+					rc1AdminRequestConfig.request
+						.post(api('permissions.update'))
+						.set(rc1AdminRequestConfig.credentials)
+						.send({ permissions: [{ _id: 'access-federation', roles: ['admin', 'user'] }] })
+						.expect('Content-Type', 'application/json')
+						.expect(200),
+				);
+
+				it('should be able to add a user with access-federation permission to a room', async () => {
+					const createResponse = await createRoom({
+						type: 'p',
+						name: `federated-room-${Date.now()}`,
+						members: [],
+						extraData: {
+							federated: true,
+						},
+						config: rc1AdminRequestConfig,
+					});
+
+					expect(createResponse.status).toBe(200);
+					const addUserResponse = await addUserToRoom({
+						usernames: [user.username],
+						rid: createResponse.body.group._id,
+						config: rc1AdminRequestConfig,
+					});
+
+					expect(addUserResponse.status).toBe(200);
+					expect(addUserResponse.body).toHaveProperty('success', true);
+					expect(addUserResponse.body).toHaveProperty('message');
+					expect(addUserResponse.body.message).toMatch('{"msg":"result","id":"id","result":true}');
+				});
 			});
 		});
 	});