From f6895ca7438303ab9db4f9afc5966938ccecfc42 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Fri, 10 Apr 2026 19:51:43 -0300 Subject: [PATCH 1/5] test: add failing test for federation second re-invite error (PRD-313) When a user undergoes multiple Ban -> Unban -> Re-invite cycles in a federated room, the leave event from the unban can arrive after a new INVITED subscription has been created. The handleLeave function currently treats any non-banned subscription as a regular leave, deleting the new INVITED subscription and locking the user out with "You've been removed from room" error. --- .../src/events/member.spec.ts | 221 ++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 ee/packages/federation-matrix/src/events/member.spec.ts diff --git a/ee/packages/federation-matrix/src/events/member.spec.ts b/ee/packages/federation-matrix/src/events/member.spec.ts new file mode 100644 index 0000000000000..ec86ff56ec678 --- /dev/null +++ b/ee/packages/federation-matrix/src/events/member.spec.ts @@ -0,0 +1,221 @@ +import type { IRoom, ISubscription, IUser } from '@rocket.chat/core-typings'; +import { Room } from '@rocket.chat/core-services'; +import { Rooms, Subscriptions, Users } from '@rocket.chat/models'; +import { federationSDK } from '@rocket.chat/federation-sdk'; +import { Emitter } from '@rocket.chat/emitter'; + +import { member } from './member'; + +// Mock external dependencies +jest.mock('@rocket.chat/models', () => ({ + Rooms: { + findOneFederatedByMrid: jest.fn(), + findOne: jest.fn(), + }, + Subscriptions: { + findOneByRoomIdAndUserId: jest.fn(), + }, + Users: { + findOneByUsername: jest.fn(), + setName: jest.fn(), + }, +})); + +jest.mock('@rocket.chat/core-services', () => ({ + Room: { + performUserRemoval: jest.fn(), + performUserUnban: jest.fn(), + updateDirectMessageRoomName: jest.fn(), + performAcceptRoomInvite: jest.fn(), + createUserSubscription: jest.fn(), + create: jest.fn(), + }, + Upload: { + resetUserAvatar: jest.fn(), + setUserAvatar: jest.fn(), + }, +})); + +jest.mock('@rocket.chat/logger', () => ({ + Logger: jest.fn().mockImplementation(() => ({ + info: jest.fn(), + debug: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + })), +})); + +// Create a real emitter for our tests +const testEmitter = new Emitter(); + +jest.mock('@rocket.chat/federation-sdk', () => ({ + federationSDK: { + eventEmitterService: { + on: jest.fn((event: string, handler: (...args: any[]) => any) => { + testEmitter.on(event, handler); + }), + }, + getConfig: jest.fn().mockReturnValue('rc.local'), + }, +})); + +jest.mock('../helpers/createOrUpdateFederatedUser'); +jest.mock('../helpers/extractDomainFromMatrixUserId', () => ({ + extractDomainFromMatrixUserId: jest.fn().mockReturnValue('remote.server'), +})); +jest.mock('../helpers/getUsernameServername', () => ({ + getUsernameServername: jest.fn().mockImplementation((mxid: string, _serverName: string) => { + // For @user:rc.local -> local user + if (mxid.includes('rc.local')) { + const username = mxid.substring(1, mxid.indexOf(':')); + return [username, 'rc.local', true]; + } + // For remote users, return the full mxid + return [mxid, 'remote.server', false]; + }), +})); +jest.mock('../services/MatrixMediaService'); + +const mockRooms = Rooms as jest.Mocked; +const mockSubscriptions = Subscriptions as jest.Mocked; +const mockUsers = Users as jest.Mocked; +const mockRoom = Room as jest.Mocked; + +describe('federation member event: handleLeave', () => { + const fakeRoom: IRoom = { + _id: 'room1', + t: 'p', + name: 'test-room', + federation: { mrid: '!room:remote.server', origin: 'remote.server', version: 1 }, + msgs: 0, + usersCount: 2, + u: { _id: 'admin', username: 'admin' }, + ts: new Date(), + _updatedAt: new Date(), + } as any; + + const fakeUser: IUser = { + _id: 'user1', + username: 'testuser', + name: 'Test User', + roles: ['user'], + type: 'user', + } as any; + + const fakeSenderUser: IUser = { + _id: 'admin1', + username: 'admin', + name: 'Admin', + roles: ['admin'], + type: 'user', + } as any; + + beforeAll(() => { + // Register member event handlers + member(); + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + function emitLeaveEvent(userId: string, senderId: string, roomId: string): Promise { + return new Promise((resolve, reject) => { + testEmitter.emit('homeserver.matrix.membership', { + event: { + type: 'm.room.member', + room_id: roomId, + sender: senderId, + state_key: userId, + content: { + membership: 'leave', + }, + unsigned: {}, + }, + }); + // Give the async handler time to run + setTimeout(resolve, 100); + }); + } + + it('should unban user when subscription is BANNED on leave event', async () => { + const bannedSubscription: Partial = { + _id: 'sub1', + rid: 'room1', + u: { _id: 'user1', username: 'testuser', name: 'Test User' }, + status: 'BANNED', + }; + + mockUsers.findOneByUsername.mockResolvedValueOnce(fakeUser).mockResolvedValueOnce(fakeSenderUser); + (mockRooms.findOneFederatedByMrid as jest.Mock).mockResolvedValueOnce(fakeRoom); + mockSubscriptions.findOneByRoomIdAndUserId.mockResolvedValueOnce(bannedSubscription as ISubscription); + mockRoom.performUserUnban.mockResolvedValueOnce(undefined as any); + + await emitLeaveEvent('@testuser:rc.local', '@admin:rc.local', '!room:remote.server'); + + expect(mockRoom.performUserUnban).toHaveBeenCalledWith(fakeRoom, fakeUser, fakeSenderUser); + expect(mockRoom.performUserRemoval).not.toHaveBeenCalled(); + }); + + it('should remove user when subscription exists and is NOT banned on leave event', async () => { + const normalSubscription: Partial = { + _id: 'sub1', + rid: 'room1', + u: { _id: 'user1', username: 'testuser', name: 'Test User' }, + // no status field = normal active subscription + }; + + mockUsers.findOneByUsername.mockResolvedValueOnce(fakeUser).mockResolvedValueOnce(fakeSenderUser); + (mockRooms.findOneFederatedByMrid as jest.Mock).mockResolvedValueOnce(fakeRoom); + mockSubscriptions.findOneByRoomIdAndUserId.mockResolvedValueOnce(normalSubscription as ISubscription); + mockRoom.performUserRemoval.mockResolvedValueOnce(undefined as any); + + await emitLeaveEvent('@testuser:rc.local', '@admin:rc.local', '!room:remote.server'); + + expect(mockRoom.performUserRemoval).toHaveBeenCalledWith(fakeRoom, fakeUser); + expect(mockRoom.performUserUnban).not.toHaveBeenCalled(); + }); + + /** + * PRD-313: "Removed from Room" Error on Second Re-invite + * + * This test reproduces the scenario where a leave event from a previous unban + * arrives AFTER a new INVITED subscription has been created for the re-invite. + * + * The sequence is: + * 1. User is banned (subscription status = BANNED) + * 2. Admin re-invites user, which triggers unban + new invite: + * - Banned subscription is removed + * - Unban event sent to Matrix (kick/leave event) + * - New invite sent to Matrix + * - New INVITED subscription created locally + * 3. Matrix processes the unban and delivers the leave event back + * 4. handleLeave runs: finds the NEW INVITED subscription (not banned) + * 5. BUG: Falls through to performUserRemoval, deleting the INVITED subscription + * 6. User sees "You've been removed from room" error + * + * Expected: handleLeave should NOT remove a subscription in INVITED status + * when processing a leave event, because the leave event is stale (from the unban) + * and the user has already been re-invited. + */ + it('should NOT remove a user with INVITED subscription status on a leave event (PRD-313)', async () => { + const invitedSubscription: Partial = { + _id: 'sub2', + rid: 'room1', + u: { _id: 'user1', username: 'testuser', name: 'Test User' }, + status: 'INVITED', + inviter: { _id: 'admin1', username: 'admin', name: 'Admin' }, + }; + + mockUsers.findOneByUsername.mockResolvedValueOnce(fakeUser).mockResolvedValueOnce(fakeSenderUser); + (mockRooms.findOneFederatedByMrid as jest.Mock).mockResolvedValueOnce(fakeRoom); + mockSubscriptions.findOneByRoomIdAndUserId.mockResolvedValueOnce(invitedSubscription as ISubscription); + + await emitLeaveEvent('@testuser:rc.local', '@admin:rc.local', '!room:remote.server'); + + // The INVITED subscription must NOT be removed - it represents a valid pending invite + expect(mockRoom.performUserRemoval).not.toHaveBeenCalled(); + // And it's not a banned subscription either + expect(mockRoom.performUserUnban).not.toHaveBeenCalled(); + }); +}); From 4f5fa8c002e21c9905140fd845677ba5a0ae6940 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Fri, 10 Apr 2026 20:05:25 -0300 Subject: [PATCH 2/5] fix: skip stale leave events for users with pending invites (PRD-313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a federated user is unbanned and re-invited in a single operation, the unban sends a leave event to Matrix. If this leave event arrives after a new INVITED subscription has already been created, handleLeave was incorrectly treating it as a regular leave and calling performUserRemoval — deleting the valid INVITED subscription and showing "You've been removed from room" to the user. Now handleLeave checks for INVITED status before removing, and skips the stale leave event so the pending invite is preserved. --- ee/packages/federation-matrix/src/events/member.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ee/packages/federation-matrix/src/events/member.ts b/ee/packages/federation-matrix/src/events/member.ts index ce834f7bf5df9..5789544728041 100644 --- a/ee/packages/federation-matrix/src/events/member.ts +++ b/ee/packages/federation-matrix/src/events/member.ts @@ -1,5 +1,5 @@ import { Room, Upload } from '@rocket.chat/core-services'; -import { isBannedSubscription } from '@rocket.chat/core-typings'; +import { isBannedSubscription, isInviteSubscription } from '@rocket.chat/core-typings'; import type { IRoomNativeFederated, IRoom, IUser, RoomType } from '@rocket.chat/core-typings'; import { federationSDK, type HomeserverEventSignatures, type PduForType } from '@rocket.chat/federation-sdk'; import { Logger } from '@rocket.chat/logger'; @@ -358,6 +358,15 @@ async function handleLeave({ return; } + // PRD-313: When a user is unbanned and re-invited in a single operation, the unban sends + // a leave event to Matrix. If that leave event arrives after the new INVITED subscription + // has been created, we must NOT remove the subscription — the leave is stale, and the user + // has a valid pending invite. + if (subscription && isInviteSubscription(subscription)) { + logger.info({ msg: 'Ignoring stale leave event for user with pending invite', userId: leavingUser._id, roomId: room._id }); + return; + } + await Room.performUserRemoval(room, leavingUser); // update room name for DMs From cc0b84fdb6acfa0e990bccd56fb6cb482d35d341 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Sat, 11 Apr 2026 02:54:27 -0300 Subject: [PATCH 3/5] fix lint --- ee/packages/federation-matrix/src/events/member.spec.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ee/packages/federation-matrix/src/events/member.spec.ts b/ee/packages/federation-matrix/src/events/member.spec.ts index ec86ff56ec678..b9d93f4985bdc 100644 --- a/ee/packages/federation-matrix/src/events/member.spec.ts +++ b/ee/packages/federation-matrix/src/events/member.spec.ts @@ -1,8 +1,7 @@ -import type { IRoom, ISubscription, IUser } from '@rocket.chat/core-typings'; import { Room } from '@rocket.chat/core-services'; -import { Rooms, Subscriptions, Users } from '@rocket.chat/models'; -import { federationSDK } from '@rocket.chat/federation-sdk'; +import type { IRoom, ISubscription, IUser } from '@rocket.chat/core-typings'; import { Emitter } from '@rocket.chat/emitter'; +import { Rooms, Subscriptions, Users } from '@rocket.chat/models'; import { member } from './member'; From 46a2f0c6f6e48806f13eb4402b56d4c493ed09b5 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Mon, 13 Apr 2026 12:29:12 -0300 Subject: [PATCH 4/5] fix lint --- ee/packages/federation-matrix/src/events/member.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/packages/federation-matrix/src/events/member.spec.ts b/ee/packages/federation-matrix/src/events/member.spec.ts index b9d93f4985bdc..9ffd5f61f6d54 100644 --- a/ee/packages/federation-matrix/src/events/member.spec.ts +++ b/ee/packages/federation-matrix/src/events/member.spec.ts @@ -119,7 +119,7 @@ describe('federation member event: handleLeave', () => { }); function emitLeaveEvent(userId: string, senderId: string, roomId: string): Promise { - return new Promise((resolve, reject) => { + return new Promise((resolve, _reject) => { testEmitter.emit('homeserver.matrix.membership', { event: { type: 'm.room.member', From f6a14731ccc589efb96f181cc27998ad7454007f Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Mon, 13 Apr 2026 19:04:17 -0300 Subject: [PATCH 5/5] handle invite for banned subs --- apps/meteor/server/services/room/service.ts | 9 +++++++++ ee/packages/federation-matrix/src/events/member.ts | 4 ++++ packages/core-services/src/types/IRoomService.ts | 1 + 3 files changed, 14 insertions(+) diff --git a/apps/meteor/server/services/room/service.ts b/apps/meteor/server/services/room/service.ts index 47d3dba8f49c9..d1b8c340312eb 100644 --- a/apps/meteor/server/services/room/service.ts +++ b/apps/meteor/server/services/room/service.ts @@ -375,4 +375,13 @@ export class RoomService extends ServiceClassInternal implements IRoomService { tmid, }); } + + async inviteUserAfterBan(subscription: ISubscription, inviterUser: Pick): Promise { + await Subscriptions.updateOne( + { _id: subscription._id }, + { $set: { status: 'INVITED', open: true, unread: 1, userMentions: 1, groupMentions: 0, alert: true, inviter: inviterUser } }, + ); + + void notifyOnSubscriptionChangedById(subscription._id, 'updated'); + } } diff --git a/ee/packages/federation-matrix/src/events/member.ts b/ee/packages/federation-matrix/src/events/member.ts index 5789544728041..a279badd7075a 100644 --- a/ee/packages/federation-matrix/src/events/member.ts +++ b/ee/packages/federation-matrix/src/events/member.ts @@ -242,6 +242,10 @@ async function handleInvite({ const subscription = await Subscriptions.findOneByRoomIdAndUserId(room._id, inviteeUser._id); if (subscription) { + // if subscription state says the user is banned, it means the user was previously banned and is now being re-invited, so we need to unban the user instead of creating a new invite + if (isBannedSubscription(subscription)) { + await Room.inviteUserAfterBan(subscription, inviterUser); + } return; } diff --git a/packages/core-services/src/types/IRoomService.ts b/packages/core-services/src/types/IRoomService.ts index 57c15e101a4c6..ffce6d17d0fa6 100644 --- a/packages/core-services/src/types/IRoomService.ts +++ b/packages/core-services/src/types/IRoomService.ts @@ -79,4 +79,5 @@ export interface IRoomService { ): Promise; markAsRead(room: IRoom, userId: string, readThreads?: boolean): Promise; readThread(params: { user: IUser; room: IRoom; tmid: string }): Promise; + inviteUserAfterBan(subscription: ISubscription, inviterUser: Pick): Promise; }