From f739bf3dc4b42f7cea55d10fe7d1c8e9fc5ce2f0 Mon Sep 17 00:00:00 2001 From: Amir Ghezelbash Date: Tue, 3 Mar 2026 12:52:13 +0100 Subject: [PATCH 1/6] fix(core): Do not reset & re-establish conversations if they do not belong to self backend [WPB-22511] --- .../ConversationService.ts | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/libraries/core/src/conversation/ConversationService/ConversationService.ts b/libraries/core/src/conversation/ConversationService/ConversationService.ts index 4f701945ec4..92e81351484 100644 --- a/libraries/core/src/conversation/ConversationService/ConversationService.ts +++ b/libraries/core/src/conversation/ConversationService/ConversationService.ts @@ -394,6 +394,12 @@ export class ConversationService extends TypedEventEmitter { selfClientId: string; conversationQualifiedId: QualifiedId; }): Promise { + if (selfUserId.domain !== conversationQualifiedId.domain) { + const errorMessage = `Self user domain (${selfUserId.domain}) does not match conversation domain (${conversationQualifiedId.domain}), cannot establish MLS group conversation`; + this.logger.error(errorMessage, {conversationQualifiedId, selfUserId}); + throw new Error(errorMessage); + } + const failures = await this.mlsService.registerConversation(groupId, userIdsToAdd.concat(selfUserId), { creator: { user: selfUserId, @@ -619,9 +625,25 @@ export class ConversationService extends TypedEventEmitter { private async resetMLSConversation(conversationId: QualifiedId): Promise { this.logger.info(`Resetting MLS conversation with id ${conversationId.id}`); - // STEP 1: Fetch the conversation to retrieve the group ID & epoch + // STEP 1: fetch self user info + this.logger.info( + `Re-establishing the conversation by re-adding all members (conversation_id: ${conversationId.id})`, + ); + const {validatedClientId: clientId, userId, domain: selfUserDomain} = this.apiClient; + + // STEP 2: Fetch the conversation to retrieve the group ID & epoch const conversation = await this.apiClient.api.conversation.getConversation(conversationId); - const {group_id: groupId, epoch} = conversation; + const { + group_id: groupId, + epoch, + qualified_id: {domain: conversationDomain}, + } = conversation; + + if (selfUserDomain !== conversationDomain) { + const errorMessage = `Self user domain (${selfUserDomain}) does not match conversation domain (${conversationDomain}), cannot reset MLS conversation`; + this.logger.error(errorMessage, {conversationId}); + throw new Error(errorMessage); + } if (!groupId || !epoch) { const errorMessage = 'Could not find group id or epoch for the conversation'; @@ -629,26 +651,20 @@ export class ConversationService extends TypedEventEmitter { throw new Error(errorMessage); } - // STEP 2: Request backend to reset the conversation + // STEP 3: Request backend to reset the conversation this.logger.info(`Requesting backend to reset the conversation (group_id: ${groupId}, epoch: ${String(epoch)})`); await this.apiClient.api.conversation.resetMLSConversation({ epoch, groupId, }); - // STEP 3: fetch self user info - this.logger.info( - `Re-establishing the conversation by re-adding all members (conversation_id: ${conversationId.id})`, - ); - const {validatedClientId: clientId, userId, domain} = this.apiClient; - - if (!userId || !domain) { + if (!userId || !selfUserDomain) { const errorMessage = 'Could not find userId or domain of the self user'; this.logger.error(errorMessage, {conversationId}); throw new Error(errorMessage); } - const selfUserQualifiedId = {id: userId, domain}; + const selfUserQualifiedId = {id: userId, domain: selfUserDomain}; // STEP 4: Fetch the updated conversation data from backend to retrieve the new group ID const updatedConversation = await this.apiClient.api.conversation.getConversation(conversationId); @@ -912,6 +928,12 @@ export class ConversationService extends TypedEventEmitter { qualifiedUsers: QualifiedId[]; }): Promise { try { + if (selfUserId.domain !== conversationId.domain) { + const errorMessage = `Self user domain (${selfUserId.domain}) does not match conversation domain (${conversationId.domain}), cannot try to establish MLS group conversation`; + this.logger.error(errorMessage, {conversationId, selfUserId}); + throw new Error(errorMessage); + } + const wasGroupEstablishedBySelfClient = await this.mlsService.tryEstablishingMLSGroup(groupId); if (!wasGroupEstablishedBySelfClient) { From 7de946d3d9c618ca2c0a9f40e16660ad50044bd5 Mon Sep 17 00:00:00 2001 From: Amir Ghezelbash Date: Tue, 3 Mar 2026 12:55:04 +0100 Subject: [PATCH 2/6] update tests --- .../ConversationService.test.ts | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/libraries/core/src/conversation/ConversationService/ConversationService.test.ts b/libraries/core/src/conversation/ConversationService/ConversationService.test.ts index d8df2e92738..552e0fcbfaf 100644 --- a/libraries/core/src/conversation/ConversationService/ConversationService.test.ts +++ b/libraries/core/src/conversation/ConversationService/ConversationService.test.ts @@ -681,6 +681,49 @@ describe('ConversationService', () => { }); }); + describe('domain mismatch guards', () => { + it('throws when establishing MLS group conversation with mismatched self and conversation domains', async () => { + const [conversationService, {mlsService}] = await buildConversationService(); + + const groupId = 'group-domain-mismatch-establish'; + const selfUserId = {id: 'self-user-id', domain: 'local.wire.com'}; + const conversationQualifiedId = {id: PayloadHelper.getUUID(), domain: 'staging.zinfra.io'}; + + await expect( + conversationService.establishMLSGroupConversation( + groupId, + [], + selfUserId, + 'self-client-id', + conversationQualifiedId, + ), + ).rejects.toThrow('does not match conversation domain'); + + expect(mlsService.registerConversation).not.toHaveBeenCalled(); + }); + + it('throws when resetting MLS conversation if self user domain mismatches conversation domain', async () => { + const [conversationService, {apiClient}] = await buildConversationService(); + + const conversationId = {id: PayloadHelper.getUUID(), domain: 'staging.zinfra.io'}; + + jest.spyOn(apiClient.api.conversation, 'getConversation').mockResolvedValueOnce({ + qualified_id: {id: conversationId.id, domain: 'local.wire.com'}, + protocol: CONVERSATION_PROTOCOL.MLS, + epoch: 1, + group_id: 'group-domain-mismatch-reset', + } as unknown as Conversation); + + const resetSpy = jest.spyOn(apiClient.api.conversation, 'resetMLSConversation'); + + await expect((conversationService as any).resetMLSConversation(conversationId)).rejects.toThrow( + 'does not match conversation domain', + ); + + expect(resetSpy).not.toHaveBeenCalled(); + }); + }); + describe('handleEvent', () => { it('rejoins a MLS conversation if epoch mismatch detected when decrypting mls message', async () => { const [conversationService, {apiClient, mlsService}] = await buildConversationService(); @@ -1075,6 +1118,31 @@ describe('ConversationService', () => { expect(conversationService.addUsersToMLSConversation).not.toHaveBeenCalled(); }); + + it('throws when self user domain does not match conversation domain', async () => { + const [conversationService, {mlsService}] = await buildConversationService(); + const selfUserId = {id: 'self-user-id', domain: 'local.wire.com'}; + + const mockConversationId = {id: PayloadHelper.getUUID(), domain: 'staging.zinfra.io'}; + const mockGroupId = 'groupId'; + const otherUsersToAdd = Array(3) + .fill(0) + .map(() => ({id: PayloadHelper.getUUID(), domain: 'local.wire.com'})); + + const addUsersSpy = jest.spyOn(conversationService, 'addUsersToMLSConversation'); + + await expect( + conversationService.tryEstablishingMLSGroup({ + conversationId: mockConversationId, + groupId: mockGroupId, + qualifiedUsers: otherUsersToAdd, + selfUserId, + }), + ).rejects.toThrow('does not match conversation domain'); + + expect(mlsService.tryEstablishingMLSGroup).not.toHaveBeenCalled(); + expect(addUsersSpy).not.toHaveBeenCalled(); + }); }); describe('reactToKeyMaterialUpdateFailure', () => { From 9450bc4427be220c9576d5e83fd059978a9445ac Mon Sep 17 00:00:00 2001 From: Amir Ghezelbash Date: Tue, 3 Mar 2026 13:12:39 +0100 Subject: [PATCH 3/6] explicitly allow one to ones --- .../ConversationService.test.ts | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/libraries/core/src/conversation/ConversationService/ConversationService.test.ts b/libraries/core/src/conversation/ConversationService/ConversationService.test.ts index 552e0fcbfaf..b8decd1f2fd 100644 --- a/libraries/core/src/conversation/ConversationService/ConversationService.test.ts +++ b/libraries/core/src/conversation/ConversationService/ConversationService.test.ts @@ -679,6 +679,46 @@ describe('ConversationService', () => { expect(conversationService.joinByExternalCommit).not.toHaveBeenCalled(); expect(establishedConversation.epoch).toEqual(updatedEpoch); }); + + it('allows establishing cross-domain MLS 1:1 conversations', async () => { + const [conversationService, {apiClient, mlsService}] = await buildConversationService(); + + const mockConversationId = {id: 'mock-conversation-id', domain: 'remote.wire.com'}; + const mockGroupId = 'mock-group-id'; + + const selfUser = {user: {id: 'self-user-id', domain: 'local.wire.com'}, client: 'self-user-client-id'}; + const otherUserId = {id: 'other-user-id', domain: 'remote.wire.com'}; + + const remoteEpoch = 0; + const updatedEpoch = 1; + + jest.spyOn(apiClient.api.conversation, 'getMLS1to1Conversation').mockResolvedValueOnce({ + qualified_id: mockConversationId, + protocol: CONVERSATION_PROTOCOL.MLS, + epoch: remoteEpoch, + group_id: mockGroupId, + } as unknown as MLSConversation); + + jest.spyOn(apiClient.api.conversation, 'getMLS1to1Conversation').mockResolvedValueOnce({ + qualified_id: mockConversationId, + protocol: CONVERSATION_PROTOCOL.MLS, + epoch: updatedEpoch, + group_id: mockGroupId, + } as unknown as MLSConversation); + + jest.spyOn(mlsService, 'wipeConversation'); + + const establishedConversation = await conversationService.establishMLS1to1Conversation( + mockGroupId, + selfUser, + otherUserId, + ); + + expect(mlsService.register1to1Conversation).toHaveBeenCalledTimes(1); + expect(mlsService.register1to1Conversation).toHaveBeenCalledWith(mockGroupId, otherUserId, selfUser, undefined); + expect(conversationService.joinByExternalCommit).not.toHaveBeenCalled(); + expect(establishedConversation.epoch).toEqual(updatedEpoch); + }); }); describe('domain mismatch guards', () => { From ce61b988914a226a55d7db7cd3fab8e74f0e189f Mon Sep 17 00:00:00 2001 From: Amir Ghezelbash Date: Wed, 4 Mar 2026 10:34:40 +0100 Subject: [PATCH 4/6] refactor --- .../ConversationService.ts | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/libraries/core/src/conversation/ConversationService/ConversationService.ts b/libraries/core/src/conversation/ConversationService/ConversationService.ts index 92e81351484..04c86951fd9 100644 --- a/libraries/core/src/conversation/ConversationService/ConversationService.ts +++ b/libraries/core/src/conversation/ConversationService/ConversationService.ts @@ -138,6 +138,16 @@ export class ConversationService extends TypedEventEmitter { return this._mlsService; } + private validateDomainMatch(selfUserDomain: string, conversationDomain: string): void { + if (selfUserDomain === conversationDomain) { + return; + } + + const errorMessage = `Self user domain (${selfUserDomain}) does not match conversation domain (${conversationDomain})`; + this.logger.error(errorMessage); + throw new Error(errorMessage); + } + /** * Get a fresh list from backend of clients for all the participants of the conversation. * @fixme there are some case where this method is not enough to detect removed devices @@ -394,11 +404,7 @@ export class ConversationService extends TypedEventEmitter { selfClientId: string; conversationQualifiedId: QualifiedId; }): Promise { - if (selfUserId.domain !== conversationQualifiedId.domain) { - const errorMessage = `Self user domain (${selfUserId.domain}) does not match conversation domain (${conversationQualifiedId.domain}), cannot establish MLS group conversation`; - this.logger.error(errorMessage, {conversationQualifiedId, selfUserId}); - throw new Error(errorMessage); - } + this.validateDomainMatch(selfUserId.domain, conversationQualifiedId.domain); const failures = await this.mlsService.registerConversation(groupId, userIdsToAdd.concat(selfUserId), { creator: { @@ -639,11 +645,7 @@ export class ConversationService extends TypedEventEmitter { qualified_id: {domain: conversationDomain}, } = conversation; - if (selfUserDomain !== conversationDomain) { - const errorMessage = `Self user domain (${selfUserDomain}) does not match conversation domain (${conversationDomain}), cannot reset MLS conversation`; - this.logger.error(errorMessage, {conversationId}); - throw new Error(errorMessage); - } + this.validateDomainMatch(selfUserDomain, conversationDomain); if (!groupId || !epoch) { const errorMessage = 'Could not find group id or epoch for the conversation'; @@ -928,11 +930,7 @@ export class ConversationService extends TypedEventEmitter { qualifiedUsers: QualifiedId[]; }): Promise { try { - if (selfUserId.domain !== conversationId.domain) { - const errorMessage = `Self user domain (${selfUserId.domain}) does not match conversation domain (${conversationId.domain}), cannot try to establish MLS group conversation`; - this.logger.error(errorMessage, {conversationId, selfUserId}); - throw new Error(errorMessage); - } + this.validateDomainMatch(selfUserId.domain, conversationId.domain); const wasGroupEstablishedBySelfClient = await this.mlsService.tryEstablishingMLSGroup(groupId); From e26a8b16e225ae58a29bec15b110d4eda782e96e Mon Sep 17 00:00:00 2001 From: Amir Ghezelbash Date: Wed, 4 Mar 2026 10:39:31 +0100 Subject: [PATCH 5/6] check for self user --- .../conversation/ConversationService/ConversationService.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libraries/core/src/conversation/ConversationService/ConversationService.ts b/libraries/core/src/conversation/ConversationService/ConversationService.ts index 04c86951fd9..a59464ffce5 100644 --- a/libraries/core/src/conversation/ConversationService/ConversationService.ts +++ b/libraries/core/src/conversation/ConversationService/ConversationService.ts @@ -637,6 +637,12 @@ export class ConversationService extends TypedEventEmitter { ); const {validatedClientId: clientId, userId, domain: selfUserDomain} = this.apiClient; + if (!selfUserDomain) { + const errorMessage = 'Could not find domain of the self user'; + this.logger.error(errorMessage, {conversationId}); + throw new Error(errorMessage); + } + // STEP 2: Fetch the conversation to retrieve the group ID & epoch const conversation = await this.apiClient.api.conversation.getConversation(conversationId); const { From 95ab48b3923d78431548a170b22cf5799b1a40e9 Mon Sep 17 00:00:00 2001 From: Amir Ghezelbash Date: Wed, 4 Mar 2026 10:47:07 +0100 Subject: [PATCH 6/6] fix tests --- .../conversation/ConversationService/ConversationService.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/core/src/conversation/ConversationService/ConversationService.test.ts b/libraries/core/src/conversation/ConversationService/ConversationService.test.ts index b8decd1f2fd..47d09bf2528 100644 --- a/libraries/core/src/conversation/ConversationService/ConversationService.test.ts +++ b/libraries/core/src/conversation/ConversationService/ConversationService.test.ts @@ -746,6 +746,7 @@ describe('ConversationService', () => { const [conversationService, {apiClient}] = await buildConversationService(); const conversationId = {id: PayloadHelper.getUUID(), domain: 'staging.zinfra.io'}; + jest.spyOn(apiClient, 'domain', 'get').mockReturnValue('staging.zinfra.io'); jest.spyOn(apiClient.api.conversation, 'getConversation').mockResolvedValueOnce({ qualified_id: {id: conversationId.id, domain: 'local.wire.com'},