runfix: handle a crash during change to mixed protocol (#17044)

* runfix: handle a crash during mixed mls group creation

* test: handle a crash during mixed mls group creation

* refactor: join mixed conversations

src/script/mls/MLSConversations.ts

@@ -62,23 +62,49 @@ export async function initMLSGroupConversations(
   );
 
   for (const mlsConversation of mlsGroupConversations) {
-    try {
-      const {groupId, qualifiedId} = mlsConversation;
+    await initMLSGroupConversation(mlsConversation, {core, onSuccessfulJoin, onError});
+  }
+}
 
-      const doesMLSGroupExist = await conversationService.mlsGroupExistsLocally(groupId);
+/**
+ * Will initialize a single MLS conversation that the user is member of but that are not yet locally established.
+ *
+ * @param mlsConversation - the conversation to initialize
+ * @param core - the instance of the core
+ */
+export async function initMLSGroupConversation(
+  mlsConversation: MLSCapableConversation,
+  {
+    core,
+    onSuccessfulJoin,
+    onError,
+  }: {
+    core: Account;
+    onSuccessfulJoin?: (conversation: Conversation) => void;
+    onError?: (conversation: Conversation, error: unknown) => void;
+  },
+): Promise<void> {
+  const {mls: mlsService, conversation: conversationService} = core.service || {};
+  if (!mlsService || !conversationService) {
+    throw new Error('MLS or Conversation service is not available!');
+  }
 
-      //if group is already established, we just schedule periodic key material updates
-      if (doesMLSGroupExist) {
-        await mlsService.scheduleKeyMaterialRenewal(groupId);
-        continue;
-      }
+  try {
+    const {groupId, qualifiedId} = mlsConversation;
 
-      //otherwise we should try joining via external commit
-      await conversationService.joinByExternalCommit(qualifiedId);
-      onSuccessfulJoin?.(mlsConversation);
-    } catch (error) {
-      onError?.(mlsConversation, error);
+    const doesMLSGroupExist = await conversationService.mlsGroupExistsLocally(groupId);
+
+    //if group is already established, we just schedule periodic key material updates
+    if (doesMLSGroupExist) {
+      await mlsService.scheduleKeyMaterialRenewal(groupId);
+      return;
     }
+
+    //otherwise we should try joining via external commit
+    await conversationService.joinByExternalCommit(qualifiedId);
+    onSuccessfulJoin?.(mlsConversation);
+  } catch (error) {
+    onError?.(mlsConversation, error);
   }
 }
 

src/script/mls/MLSMigration/MLSMigration.ts

@@ -169,7 +169,10 @@ const migrateConversationsToMLS = async ({
   await initialiseMigrationOfProteusConversations(selfTeamGroupConversations, selfUserId, conversationHandler);
 
   const allGroupConversations = conversationHandler.getAllGroupConversations();
-  await joinUnestablishedMixedConversations(allGroupConversations, {core});
+  await joinUnestablishedMixedConversations(allGroupConversations, selfUserId, {
+    core,
+    conversationHandler,
+  });
 
   await finaliseMigrationOfMixedConversations(
     selfTeamGroupConversations,

src/script/mls/MLSMigration/migrationInitialiser/joinUnestablishedMixedConversations/joinUnestablishedMixedConversations.test.ts

@@ -22,35 +22,50 @@ import {container} from 'tsyringe';
 
 import {MixedConversation} from 'src/script/conversation/ConversationSelectors';
 import {Conversation} from 'src/script/entity/Conversation';
+import {User} from 'src/script/entity/User';
 import {Core} from 'src/script/service/CoreSingleton';
+import {TestFactory} from 'test/helper/TestFactory';
 import {createUuid} from 'Util/uuid';
 
 import {joinUnestablishedMixedConversations} from '.';
 
-const createMixedConversation = (mockGroupId: string): MixedConversation => {
+const createMixedConversation = (mockGroupId: string, epoch = 1): MixedConversation => {
   const conversation = new Conversation(createUuid(), '', ConversationProtocol.MIXED);
   conversation.groupId = mockGroupId;
   conversation.type(CONVERSATION_TYPE.REGULAR);
+  conversation.epoch = epoch;
   return conversation as MixedConversation;
 };
 
 const mockCore = container.resolve(Core);
 
 describe('joinUnestablishedMixedConversations', () => {
+  const testFactory = new TestFactory();
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
   it('Should join known "mixed" conversations with unestablished MLS groups', async () => {
     const mixedConversation1 = createMixedConversation('groupId1');
     const mixedConversation2 = createMixedConversation('groupId2');
     const mixedConversation3 = createMixedConversation('unestablishedGroup3');
     const mixedConversation4 = createMixedConversation('unestablishedGroup4');
 
+    const selfUser = new User(createUuid(), 'domain');
+
     jest.spyOn(mockCore.service!.conversation!, 'mlsGroupExistsLocally').mockImplementation(groupId => {
       return Promise.resolve(!groupId.includes('unestablished'));
     });
 
+    const mockedConversationRepository = await testFactory.exposeConversationActors();
+
     await joinUnestablishedMixedConversations(
       [mixedConversation1, mixedConversation2, mixedConversation3, mixedConversation4],
+      selfUser.qualifiedId,
       {
         core: mockCore,
+        conversationHandler: mockedConversationRepository,
       },
     );
 
@@ -60,4 +75,34 @@ describe('joinUnestablishedMixedConversations', () => {
     expect(mockCore.service?.conversation?.joinByExternalCommit).toHaveBeenCalledWith(mixedConversation3.qualifiedId);
     expect(mockCore.service?.conversation?.joinByExternalCommit).toHaveBeenCalledWith(mixedConversation4.qualifiedId);
   });
+
+  it('Should establish not initialised (epoch 0) mls groups for known "mixed" conversations', async () => {
+    const mixedConversation1 = createMixedConversation('groupId1');
+    const mixedConversation2 = createMixedConversation('groupId2');
+    const mixedConversation3 = createMixedConversation('unestablishedGroup3', 0);
+    const mixedConversation4 = createMixedConversation('unestablishedGroup4', 0);
+
+    const selfUser = new User(createUuid(), 'domain');
+
+    jest.spyOn(mockCore.service!.conversation!, 'mlsGroupExistsLocally').mockImplementation(groupId => {
+      return Promise.resolve(!groupId.includes('unestablished'));
+    });
+
+    const mockedConversationRepository = await testFactory.exposeConversationActors();
+    jest.spyOn(mockedConversationRepository, 'tryEstablishingMLSGroup');
+
+    await joinUnestablishedMixedConversations(
+      [mixedConversation1, mixedConversation2, mixedConversation3, mixedConversation4],
+      selfUser.qualifiedId,
+      {
+        core: mockCore,
+        conversationHandler: mockedConversationRepository,
+      },
+    );
+
+    expect(mockCore.service?.conversation?.mlsGroupExistsLocally).toHaveBeenCalledTimes(2);
+    expect(mockCore.service?.conversation?.joinByExternalCommit).not.toHaveBeenCalled();
+
+    expect(mockedConversationRepository.tryEstablishingMLSGroup).toHaveBeenCalledTimes(2);
+  });
 });

src/script/mls/MLSMigration/migrationInitialiser/joinUnestablishedMixedConversations/joinUnestablishedMixedConversations.ts

@@ -17,28 +17,80 @@
  *
  */
 
+import {QualifiedId} from '@wireapp/api-client/lib/user';
+
 import {Account} from '@wireapp/core';
 
-import {isMixedConversation} from 'src/script/conversation/ConversationSelectors';
+import {isMixedConversation, MixedConversation} from 'src/script/conversation/ConversationSelectors';
 import {Conversation} from 'src/script/entity/Conversation';
-import {initMLSGroupConversations} from 'src/script/mls/MLSConversations';
+import {initMLSGroupConversation} from 'src/script/mls/MLSConversations';
 
 import {mlsMigrationLogger} from '../../MLSMigrationLogger';
 
+interface MigrationJoinMixedConversationHandler {
+  tryEstablishingMLSGroup: (params: {
+    groupId: string;
+    conversationId: QualifiedId;
+    selfUserId: QualifiedId;
+    qualifiedUsers: QualifiedId[];
+  }) => Promise<void>;
+}
+
 interface JoinUnestablishedMixedConversationsParams {
   core: Account;
+  conversationHandler: MigrationJoinMixedConversationHandler;
 }
 
 export const joinUnestablishedMixedConversations = async (
   conversations: Conversation[],
-  {core}: JoinUnestablishedMixedConversationsParams,
+  selfUserId: QualifiedId,
+  {core, conversationHandler}: JoinUnestablishedMixedConversationsParams,
 ) => {
   const mixedConversations = conversations.filter(isMixedConversation);
   mlsMigrationLogger.info(`Found ${mixedConversations.length} "mixed" conversations, joining unestablished ones...`);
 
-  await initMLSGroupConversations(mixedConversations, {
-    core,
-    onError: ({id}, error) =>
-      mlsMigrationLogger.error(`Failed when joining a mls group of mixed conversation with id ${id}, error: `, error),
-  });
+  for (const conversation of mixedConversations) {
+    await joinUnestablishedMixedConversation(conversation, selfUserId, {core, conversationHandler});
+  }
+};
+
+export const joinUnestablishedMixedConversation = async (
+  mixedConversation: MixedConversation,
+  selfUserId: QualifiedId,
+  {core, conversationHandler}: JoinUnestablishedMixedConversationsParams,
+  shouldRetry = true,
+) => {
+  if (mixedConversation.epoch > 0) {
+    return initMLSGroupConversation(mixedConversation, {
+      core,
+      onError: ({id}, error) =>
+        mlsMigrationLogger.error(`Failed when joining a mls group of mixed conversation with id ${id}, error: `, error),
+    });
+  }
+
+  // It's possible that the client has updated the protocol to mixed and then crashed, before it has established the MLS group.
+  // In this case, we should try to establish the MLS group again.
+  mlsMigrationLogger.info(`Trying to establish MLS group for mixed conversation with id ${mixedConversation.id}`);
+
+  const otherUsersToAdd = mixedConversation.participating_user_ids();
+
+  try {
+    await conversationHandler.tryEstablishingMLSGroup({
+      conversationId: mixedConversation.qualifiedId,
+      groupId: mixedConversation.groupId,
+      qualifiedUsers: otherUsersToAdd,
+      selfUserId: selfUserId,
+    });
+  } catch (error) {
+    mlsMigrationLogger.error(
+      `Failed to establish MLS group for mixed conversation with id ${mixedConversation.id}, error: `,
+      error,
+    );
+    if (!shouldRetry) {
+      return;
+    }
+
+    mlsMigrationLogger.info(`Retrying to join unestablished mixed conversation with id ${mixedConversation.id}`);
+    await joinUnestablishedMixedConversation(mixedConversation, selfUserId, {core, conversationHandler}, false);
+  }
 };