diff --git a/apps/meteor/app/federation-v2/server/Federation.ts b/apps/meteor/app/federation-v2/server/Federation.ts index 9b29c9ad620f..f45bacc2bbe3 100644 --- a/apps/meteor/app/federation-v2/server/Federation.ts +++ b/apps/meteor/app/federation-v2/server/Federation.ts @@ -14,6 +14,8 @@ const allowedActionsInFederatedRooms: ValueOf[] = [ RoomMemberActions.LEAVE, ]; +const allowedActionsForModerators = allowedActionsInFederatedRooms.filter((action) => action !== RoomMemberActions.SET_AS_OWNER); + const allowedRoomSettingsChangesInFederatedRooms: ValueOf[] = [RoomSettingsEnum.NAME, RoomSettingsEnum.TOPIC]; export class Federation { @@ -33,10 +35,19 @@ export class Federation { return true; } - return Boolean( - (userSubscription.roles?.includes('owner') || userSubscription.roles?.includes('moderator')) && - allowedActionsInFederatedRooms.includes(action), - ); + if (action === RoomMemberActions.LEAVE) { + return true; + } + + if (userSubscription.roles?.includes('owner')) { + return allowedActionsInFederatedRooms.includes(action); + } + + if (userSubscription.roles?.includes('moderator')) { + return allowedActionsForModerators.includes(action); + } + + return false; } public static isAFederatedUsername(username: string): boolean { diff --git a/apps/meteor/tests/unit/app/federation-v2/server/unit/Federation.spec.ts b/apps/meteor/tests/unit/app/federation-v2/server/unit/Federation.spec.ts index 9553b2bac12f..7a55121fc4cd 100644 --- a/apps/meteor/tests/unit/app/federation-v2/server/unit/Federation.spec.ts +++ b/apps/meteor/tests/unit/app/federation-v2/server/unit/Federation.spec.ts @@ -39,6 +39,11 @@ describe('Federation[Server] - Federation', () => { expect(Federation.actionAllowed({ t: 'c', federated: true } as any, RoomMemberActions.INVITE, 'userId')).to.be.true; }); + it('should return true if the action is equal to Leave (since any user can leave a channel)', () => { + findOneByRoomIdAndUserIdStub.returns({}); + expect(Federation.actionAllowed({ t: 'c', federated: true } as any, RoomMemberActions.LEAVE, 'userId')).to.be.true; + }); + const allowedActions = [ RoomMemberActions.REMOVE_USER, RoomMemberActions.SET_AS_OWNER, @@ -62,11 +67,23 @@ describe('Federation[Server] - Federation', () => { findOneByRoomIdAndUserIdStub.returns({ roles: ['owner'] }); expect(Federation.actionAllowed({ t: 'c', federated: true } as any, action, 'userId')).to.be.true; }); + }); + + const allowedActionsForModerators = allowedActions.filter((action) => action !== RoomMemberActions.SET_AS_OWNER); + allowedActionsForModerators.forEach((action) => { it('should return true if the action is allowed within the federation context for regular channels and the user is a room moderator', () => { findOneByRoomIdAndUserIdStub.returns({ roles: ['moderator'] }); expect(Federation.actionAllowed({ t: 'c', federated: true } as any, action, 'userId')).to.be.true; }); - it('should return false if the action is allowed within the federation context for regular channels and the user is nor a moderator or owner', () => { + }); + it('should return false if the action is equal to set owner and the user is a room moderator', () => { + findOneByRoomIdAndUserIdStub.returns({ roles: ['moderator'] }); + expect(Federation.actionAllowed({ t: 'c', federated: true } as any, RoomMemberActions.SET_AS_OWNER, 'userId')).to.be.false; + }); + + const disallowedActionForRegularUsers = allowedActions.filter((action) => action !== RoomMemberActions.LEAVE); + disallowedActionForRegularUsers.forEach((action) => { + it('should return false if the for all other actions (excluding LEAVE) for regular users', () => { findOneByRoomIdAndUserIdStub.returns({}); expect(Federation.actionAllowed({ t: 'c', federated: true } as any, action, 'userId')).to.be.false; });