feat: establish 1on1 mls conversation with new removal key [WPB-10744] (

#6502) * feat: establish 1on1 mls conversation with new removal key [10744] * remove federated SFT 1:1 call temporary fix * type data structure for 1:1 MLS conversations * remove public_keys from NewConversation type * use nullish coalescing instead of ternary for removalKey logic * implement typeguard for the specific MLS1to1Conversation response * use union types for api-client getMLS1to1Conversation return value * use type guard and return MLS1to1Conversation for getMLS1to1Conversation in core * address tests * Update packages/api-client/src/conversation/Conversation.ts Co-authored-by: Przemysław Jóźwik <[email protected]> * rename variable in type guard * Update packages/core/src/conversation/ConversationService/ConversationService.ts Co-authored-by: Immad Abdul Jabbar <[email protected]> * address CR issue * simplify typeguard's logic * address CR --------- Co-authored-by: Przemysław Jóźwik <[email protected]> Co-authored-by: Immad Abdul Jabbar <[email protected]>
wireapp · Sep 16, 2024 · 55db2a1 · 55db2a1
1 parent d279165
commit 55db2a1
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 53 deletions.
diff --git a/packages/api-client/src/conversation/Conversation.ts b/packages/api-client/src/conversation/Conversation.ts
@@ -19,6 +19,7 @@
 
 import {RECEIPT_MODE} from './data';
 
+import {MLSPublicKeyRecord} from '../client';
 import {QualifiedId} from '../user';
 
 import {ConversationMembers, ConversationProtocol} from './';
@@ -107,3 +108,18 @@ export interface MLSConversation extends Conversation {
   cipher_suite: number;
   protocol: ConversationProtocol.MLS;
 }
+
+export interface MLS1to1Conversation {
+  conversation: MLSConversation;
+  public_keys?: {
+    removal: MLSPublicKeyRecord;
+  };
+}
+
+export function isMLS1to1Conversation(response: unknown): response is MLS1to1Conversation {
+  if (typeof response === 'object' && response !== null && 'conversation' in response && 'public_keys' in response) {
+    return true;
+  }
+
+  return false;
+}
diff --git a/packages/api-client/src/conversation/ConversationAPI/ConversationAPI.ts b/packages/api-client/src/conversation/ConversationAPI/ConversationAPI.ts
@@ -31,6 +31,7 @@ import {
   JoinConversationByCodePayload,
   Member,
   MessageSendingStatus,
+  MLS1to1Conversation,
   MLSConversation,
   NewConversation,
   QualifiedConversationIds,
@@ -420,13 +421,13 @@ export class ConversationAPI {
    * Get a MLS 1:1-conversation with a given user.
    * @param userId - qualified user id
    */
-  public async getMLS1to1Conversation({domain, id}: QualifiedId): Promise<MLSConversation> {
+  public async getMLS1to1Conversation({domain, id}: QualifiedId): Promise<MLS1to1Conversation | MLSConversation> {
     const config: AxiosRequestConfig = {
       method: 'get',
       url: `${ConversationAPI.URL.CONVERSATIONS}/${ConversationAPI.URL.ONE_2_ONE}/${domain}/${id}`,
     };
 
-    const response = await this.client.sendJSON<MLSConversation>(config);
+    const response = await this.client.sendJSON<MLS1to1Conversation | MLSConversation>(config);
     return response.data;
   }
 

diff --git a/packages/core/src/conversation/ConversationService/ConversationService.test.ts b/packages/core/src/conversation/ConversationService/ConversationService.test.ts
@@ -426,7 +426,7 @@ describe('ConversationService', () => {
 
       expect(mlsService.wipeConversation).toHaveBeenCalledWith(mockGroupId);
       expect(mlsService.register1to1Conversation).toHaveBeenCalledTimes(1);
-      expect(mlsService.register1to1Conversation).toHaveBeenCalledWith(mockGroupId, otherUserId, selfUser);
+      expect(mlsService.register1to1Conversation).toHaveBeenCalledWith(mockGroupId, otherUserId, selfUser, undefined);
       expect(conversationService.joinByExternalCommit).not.toHaveBeenCalled();
       expect(establishedConversation.epoch).toEqual(updatedEpoch);
     });
@@ -477,7 +477,7 @@ describe('ConversationService', () => {
 
       expect(mlsService.wipeConversation).toHaveBeenCalledWith(mockGroupId);
       expect(mlsService.register1to1Conversation).toHaveBeenCalledTimes(2);
-      expect(mlsService.register1to1Conversation).toHaveBeenCalledWith(mockGroupId, otherUserId, selfUser);
+      expect(mlsService.register1to1Conversation).toHaveBeenCalledWith(mockGroupId, otherUserId, selfUser, undefined);
       expect(conversationService.joinByExternalCommit).not.toHaveBeenCalled();
       expect(establishedConversation.epoch).toEqual(updatedEpoch);
     });

diff --git a/packages/core/src/conversation/ConversationService/ConversationService.ts b/packages/core/src/conversation/ConversationService/ConversationService.ts
@@ -29,6 +29,7 @@ import {
   MLSConversation,
   SUBCONVERSATION_ID,
   Subconversation,
+  isMLS1to1Conversation,
 } from '@wireapp/api-client/lib/conversation';
 import {CONVERSATION_TYPING, ConversationMemberUpdateData} from '@wireapp/api-client/lib/conversation/data';
 import {
@@ -536,7 +537,12 @@ export class ConversationService extends TypedEventEmitter<Events> {
    * @param userId - qualified user id
    */
   async getMLS1to1Conversation(userId: QualifiedId) {
-    return this.apiClient.api.conversation.getMLS1to1Conversation(userId);
+    const conversation = await this.apiClient.api.conversation.getMLS1to1Conversation(userId);
+
+    if (isMLS1to1Conversation(conversation)) {
+      return conversation;
+    }
+    return {conversation};
   }
 
   /**
@@ -558,7 +564,8 @@ export class ConversationService extends TypedEventEmitter<Events> {
     // Before trying to register a group, check if the group is already established o backend.
     // If remote epoch is higher than 0, it means that the group was already established.
     // It's possible that we've already received a welcome message.
-    const mlsConversation = await this.getMLS1to1Conversation(otherUserId);
+
+    const {conversation: mlsConversation, public_keys} = await this.getMLS1to1Conversation(otherUserId);
 
     if (mlsConversation.epoch > 0) {
       this.logger.info(
@@ -580,19 +587,21 @@ export class ConversationService extends TypedEventEmitter<Events> {
       );
 
       await this.joinByExternalCommit(mlsConversation.qualified_id);
-      return this.getMLS1to1Conversation(otherUserId);
+      const {conversation: updatedMLSConversation} = await this.getMLS1to1Conversation(otherUserId);
+      return updatedMLSConversation;
     }
 
     // If group is not established on backend,
     // we wipe the it locally (in case it exsits in the local store) and try to register it.
     await this.mlsService.wipeConversation(groupId);
 
     try {
-      await this.mlsService.register1to1Conversation(groupId, otherUserId, selfUser);
+      await this.mlsService.register1to1Conversation(groupId, otherUserId, selfUser, public_keys?.removal);
 
       this.logger.info(`Conversation (id ${mlsConversation.qualified_id.id}) established successfully.`);
 
-      return this.getMLS1to1Conversation(otherUserId);
+      const {conversation: updatedMLSConversation} = await this.getMLS1to1Conversation(otherUserId);
+      return updatedMLSConversation;
     } catch (error) {
       this.logger.info(`Could not register MLS group with id ${groupId}: `, error);
 

diff --git a/packages/core/src/conversation/SubconversationService/SubconversationService.ts b/packages/core/src/conversation/SubconversationService/SubconversationService.ts
@@ -151,46 +151,6 @@ export class SubconversationService extends TypedEventEmitter<Events> {
     await this.clearSubconversationGroupId(conversationId, SUBCONVERSATION_ID.CONFERENCE);
   }
 
-  /**
-   * Short term solution to an issues with 1:1 MLS call, see
-   * https://wearezeta.atlassian.net/wiki/spaces/PAD/pages/1314750477/2024-07-29+1+1+calls+over+SFT#Do-not-leave-the-subconversation
-   * Will delete a conference subconversation when hanging up on a 1:1 call instead of leaving.
-   *
-   * @param conversationId Id of the parent conversation which subconversation we want to leave
-   */
-  public async leave1on1ConferenceSubconversation(conversationId: QualifiedId): Promise<void> {
-    const subconversationGroupId = await this.getSubconversationGroupId(conversationId, SUBCONVERSATION_ID.CONFERENCE);
-
-    if (!subconversationGroupId) {
-      return;
-    }
-
-    const doesGroupExistLocally = await this.mlsService.conversationExists(subconversationGroupId);
-    if (!doesGroupExistLocally) {
-      // If the subconversation was known by a client but is does not exist locally, we can remove it from the store.
-      return this.clearSubconversationGroupId(conversationId, SUBCONVERSATION_ID.CONFERENCE);
-    }
-
-    try {
-      const epochInfo = await this.getSubconversationEpochInfo(conversationId, subconversationGroupId);
-
-      if (!epochInfo) {
-        throw new Error('Failed to get epoch info for conference subconversation');
-      }
-      await this.apiClient.api.conversation.deleteSubconversation(conversationId, SUBCONVERSATION_ID.CONFERENCE, {
-        groupId: subconversationGroupId,
-        epoch: epochInfo.epoch,
-      });
-    } catch (error) {
-      this.logger.error(`Failed to delete conference subconversation:`, error);
-    }
-
-    await this.mlsService.wipeConversation(subconversationGroupId);
-
-    // once we've deleted the subconversation, we can remove it from the store
-    await this.clearSubconversationGroupId(conversationId, SUBCONVERSATION_ID.CONFERENCE);
-  }
-
   public async leaveStaleConferenceSubconversations(): Promise<void> {
     const conversationIds = await this.getAllGroupIdsBySubconversationId(SUBCONVERSATION_ID.CONFERENCE);
 

diff --git a/packages/core/src/messagingProtocols/mls/MLSService/MLSService.ts b/packages/core/src/messagingProtocols/mls/MLSService/MLSService.ts
@@ -17,7 +17,7 @@
  *
  */
 
-import type {ClaimedKeyPackages, RegisteredClient} from '@wireapp/api-client/lib/client';
+import type {ClaimedKeyPackages, MLSPublicKeyRecord, RegisteredClient} from '@wireapp/api-client/lib/client';
 import {PostMlsMessageResponse, SUBCONVERSATION_ID} from '@wireapp/api-client/lib/conversation';
 import {ConversationMLSMessageAddEvent, ConversationMLSWelcomeEvent} from '@wireapp/api-client/lib/event';
 import {BackendError, StatusCode} from '@wireapp/api-client/lib/http';
@@ -471,7 +471,11 @@ export class MLSService extends TypedEventEmitter<Events> {
    * @param groupId the id of the group to create inside of coreCrypto
    * @param parentGroupId in case the conversation is a subconversation, the id of the parent conversation
    */
-  public async registerEmptyConversation(groupId: string, parentGroupId?: string): Promise<void> {
+  public async registerEmptyConversation(
+    groupId: string,
+    parentGroupId?: string,
+    removalKeyFor1to1Signature?: MLSPublicKeyRecord,
+  ): Promise<void> {
     const groupIdBytes = Decoder.fromBase64(groupId).asBytes;
 
     let externalSenders: Uint8Array[] = [];
@@ -481,7 +485,8 @@ export class MLSService extends TypedEventEmitter<Events> {
     } else {
       const mlsKeys = (await this.apiClient.api.client.getPublicKeys()).removal;
       const ciphersuiteSignature = getSignatureAlgorithmForCiphersuite(this.config.defaultCiphersuite);
-      const removalKeyForSignature = mlsKeys[ciphersuiteSignature];
+      const removalKeyForSignature =
+        removalKeyFor1to1Signature?.[ciphersuiteSignature] ?? mlsKeys[ciphersuiteSignature];
       if (!removalKeyForSignature) {
         throw new Error(
           `Cannot create conversation: No backend removal key found for the signature ${ciphersuiteSignature}`,
@@ -559,9 +564,10 @@ export class MLSService extends TypedEventEmitter<Events> {
     groupId: string,
     userId: QualifiedId,
     selfUser: {user: QualifiedId; client: string},
+    removalKeyFor1to1Signature?: MLSPublicKeyRecord,
   ): Promise<PostMlsMessageResponse & {failures: AddUsersFailure[]}> {
     try {
-      await this.registerEmptyConversation(groupId);
+      await this.registerEmptyConversation(groupId, undefined, removalKeyFor1to1Signature);
 
       // We fist fetch key packages for the user we want to add
       const {keyPackages: otherUserKeyPackages, failures: otherUserKeysClaimingFailures} =