fix: handle connection requests with deleted users [WPB-4356] (#16815)

* runfix: filter out connection requests with deleted users

* test: fix tests

* runfix: handle canceling a connection req with deleted user

* test: fix test

* runfix: handle incoming request case

* test: add scenario for deleting local req when loading conversations

* test: add scenario for trying to accept/ignore incoming req

* refactor: improve callback naming

src/script/connection/ConnectionRepository.test.ts

@@ -18,7 +18,10 @@
  */
 
 import {ConnectionStatus} from '@wireapp/api-client/lib/connection';
+import {BackendError, BackendErrorLabel} from '@wireapp/api-client/lib/http';
+import {QualifiedId} from '@wireapp/api-client/lib/user';
 import {amplify} from 'amplify';
+import {StatusCodes} from 'http-status-codes';
 
 import {WebAppEvents} from '@wireapp/webapp-events';
 
@@ -36,7 +39,7 @@ import {UserRepository} from '../user/UserRepository';
 function buildConnectionRepository() {
   const connectionState = new ConnectionState();
   const connectionService = new ConnectionService();
-  const userRepository = {} as UserRepository;
+  const userRepository = {refreshUser: jest.fn()} as unknown as UserRepository;
   return [
     new ConnectionRepository(connectionService, userRepository, connectionState),
     {connectionState, userRepository, connectionService},
@@ -54,7 +57,7 @@ function createConnection() {
 
 describe('ConnectionRepository', () => {
   describe('cancelRequest', () => {
-    const [connectionRepository, {connectionService}] = buildConnectionRepository();
+    const [connectionRepository, {connectionService, userRepository}] = buildConnectionRepository();
 
     it('sets the connection status to cancelled', () => {
       const user = createConnection();
@@ -76,6 +79,25 @@ describe('ConnectionRepository', () => {
         expect(amplifySpy).toHaveBeenCalled();
       });
     });
+
+    it('deletes connection request if the other user does not exist on backend anymore', async () => {
+      const user = createConnection();
+
+      connectionRepository.addConnectionEntity(user.connection()!);
+
+      jest.spyOn(userRepository, 'refreshUser').mockImplementationOnce(async (uid: QualifiedId) => {
+        user.isDeleted = true;
+        return user;
+      });
+      jest
+        .spyOn(connectionService, 'putConnections')
+        .mockRejectedValueOnce(new BackendError('', BackendErrorLabel.NOT_CONNECTED, StatusCodes.FORBIDDEN));
+      await connectionRepository.cancelRequest(user);
+
+      expect(connectionService.putConnections).toHaveBeenCalled();
+      expect(connectionRepository['connectionState'].connections()).not.toContain(user.connection());
+      expect(user.connection()).toEqual(null);
+    });
   });
 
   describe('getConnectionByConversationId', () => {

src/script/connection/ConnectionRepository.ts

@@ -52,6 +52,7 @@ export class ConnectionRepository {
   private readonly connectionService: ConnectionService;
   private readonly userRepository: UserRepository;
   private readonly logger: Logger;
+  private onDeleteConnectionRequestConversation?: (userId: QualifiedId) => Promise<void>;
 
   static get CONFIG(): Record<string, BackendEventType[]> {
     return {
@@ -115,6 +116,12 @@ export class ConnectionRepository {
       connectionEntity = ConnectionMapper.mapConnectionFromJson(connectionData);
     }
 
+    const user = await this.userRepository.getUserById(connectionEntity.userId);
+    // If this is the connection request, but the user does not exist anymore, no need to attach a connection to a user or a conversation
+    if (user?.isDeleted && connectionEntity.status() === ConnectionStatus.SENT) {
+      return;
+    }
+
     // Attach connection to user
     await this.attachConnectionToUser(connectionEntity);
 
@@ -363,9 +370,39 @@ export class ConnectionRepository {
           break;
         }
       }
+
+      const user = await this.userRepository.refreshUser(userEntity.qualifiedId);
+
+      const isNotConnectedError = isBackendError(error) && error.label === BackendErrorLabel.NOT_CONNECTED;
+
+      // If connection failed because the user is deleted, delete the conversation representing the connection request (type 3 - CONNECT)
+      if (isNotConnectedError && user.isDeleted) {
+        await this.deleteConnectionWithUser(user);
+      }
     }
   }
 
+  private async deleteConnectionWithUser(user: User) {
+    const connection = this.connectionState
+      .connections()
+      .find(connection => matchQualifiedIds(connection.userId, user.qualifiedId));
+
+    if (connection) {
+      this.connectionState.connections.remove(connection);
+      user.connection(null);
+    }
+
+    await this.onDeleteConnectionRequestConversation?.(user.qualifiedId);
+  }
+
+  /**
+   * Set callback for deleting a connection request conversation.
+   * @param callback Callback function
+   */
+  public setDeleteConnectionRequestConversationHandler(callback: (userId: QualifiedId) => Promise<void>): void {
+    this.onDeleteConnectionRequestConversation = callback;
+  }
+
   /**
    * Send the user connection notification.
    *

src/script/conversation/ConversationRepository.test.ts

@@ -2020,7 +2020,7 @@ describe('ConversationRepository', () => {
           .mockResolvedValue(localConversations as unknown as ConversationDatabaseData[]);
         jest.spyOn(conversationService, 'saveConversationsInDb').mockImplementation(data => Promise.resolve(data));
 
-        const conversations = await conversationRepository.loadConversations();
+        const conversations = await conversationRepository.loadConversations([]);
 
         expect(conversations).toHaveLength(remoteConversations.found.length);
       });
@@ -2065,7 +2065,7 @@ describe('ConversationRepository', () => {
           .mockResolvedValue(localConversations as unknown as ConversationDatabaseData[]);
         jest.spyOn(conversationService, 'saveConversationsInDb').mockImplementation(data => Promise.resolve(data));
 
-        const conversations = await conversationRepository.loadConversations();
+        const conversations = await conversationRepository.loadConversations([]);
 
         expect(conversations).toHaveLength(1);
       });
@@ -2110,11 +2110,49 @@ describe('ConversationRepository', () => {
           .mockResolvedValue(localConversations as unknown as ConversationDatabaseData[]);
         jest.spyOn(conversationService, 'saveConversationsInDb').mockImplementation(data => Promise.resolve(data));
 
-        const conversations = await conversationRepository.loadConversations();
+        const conversations = await conversationRepository.loadConversations([]);
 
         expect(conversations).toHaveLength(remoteConversations.found.length);
       });
 
+      it('does not load connection request (type 3) conversations if their users were deleted on backend', async () => {
+        const conversationRepository = testFactory.conversation_repository!;
+        const conversationService = conversationRepository['conversationService'];
+        const userId = {id: '05d0f240-bfe9-40d7-b6cb-602dac89fa1b', domain: 'staging.zinfra.io'};
+
+        const connectionReq = generateConversation(
+          {
+            domain: 'staging.zinfra.io',
+            id: '05d0f240-bfe9-1234-b6cb-602dac89fa1b',
+          },
+          'conv2',
+          [userId],
+          ConversationProtocol.PROTEUS,
+          CONVERSATION_TYPE.CONNECT,
+        );
+
+        const remoteConversations = {
+          found: [connectionReq],
+        };
+        const localConversations: any = [connectionReq];
+
+        jest.spyOn(conversationService, 'deleteConversationFromDb');
+        jest.spyOn(conversationService, 'blacklistConversation');
+        jest
+          .spyOn(conversationService, 'getAllConversations')
+          .mockResolvedValue(remoteConversations as unknown as RemoteConversations);
+        jest
+          .spyOn(conversationService, 'loadConversationStatesFromDb')
+          .mockResolvedValue(localConversations as unknown as ConversationDatabaseData[]);
+        jest.spyOn(conversationService, 'saveConversationsInDb').mockImplementation(data => Promise.resolve(data));
+
+        const conversations = await conversationRepository.loadConversations([]);
+
+        expect(conversations).toHaveLength(0);
+        expect(conversationService.deleteConversationFromDb).toHaveBeenCalledWith(connectionReq.qualified_id.id);
+        expect(conversationService.blacklistConversation).toHaveBeenCalledWith(connectionReq.qualified_id);
+      });
+
       it('keeps track of missing conversations', async () => {
         const conversationRepository = testFactory.conversation_repository!;
         const conversationService = conversationRepository['conversationService'];
@@ -2152,7 +2190,7 @@ describe('ConversationRepository', () => {
           .spyOn(conversationService, 'getAllConversations')
           .mockResolvedValue(remoteConversations as unknown as RemoteConversations);
 
-        await conversationRepository.loadConversations();
+        await conversationRepository.loadConversations([]);
 
         expect(conversationState.missingConversations).toHaveLength(remoteConversations.failed.length);
       });

src/script/conversation/ConversationRepository.ts

@@ -280,6 +280,8 @@ export class ConversationRepository {
         : this.messageRepository.requestUserSendingPermission(conversation, shouldWarnLegalHold, consentType);
     });
 
+    this.connectionRepository.setDeleteConnectionRequestConversationHandler(this.deleteConnectionRequestConversation);
+
     this.logger = getLogger('ConversationRepository');
 
     this.event_mapper = new EventMapper();
@@ -570,12 +572,12 @@ export class ConversationRepository {
    * Will load all the conversations in memory
    * @returns all the conversations from backend merged with the locally stored conversations and loaded into memory
    */
-  public async loadConversations(): Promise<Conversation[]> {
+  public async loadConversations(connections: ConnectionEntity[]): Promise<Conversation[]> {
     const remoteConversations = await this.conversationService.getAllConversations().catch(error => {
       this.logger.error(`Failed to get all conversations from backend: ${error.message}`);
       return {found: []} as RemoteConversations;
     });
-    return this.loadRemoteConversations(remoteConversations);
+    return this.loadRemoteConversations(remoteConversations, connections);
   }
 
   /**
@@ -593,7 +595,9 @@ export class ConversationRepository {
         this.logger.error(`Failed to get all conversations from backend: ${error.message}`);
         return {found: [], failed: missingConversations} as RemoteConversations;
       });
-    return this.loadRemoteConversations(remoteConversations);
+
+    const connections = this.connectionState.connections();
+    return this.loadRemoteConversations(remoteConversations, connections);
   }
 
   /**
@@ -671,28 +675,89 @@ export class ConversationRepository {
     return {...remoteConversations, found: allConversations};
   }
 
+  private async filterDeletedConnectionRequests(
+    localConversations: ConversationDatabaseData[],
+    remoteConversations: RemoteConversations,
+    connections: ConnectionEntity[],
+  ): Promise<{localConversations: ConversationDatabaseData[]; remoteConversations: RemoteConversations}> {
+    //If there's any local conversation of type 3 (CONNECT), but the connection doesn't exist anymore (user was deleted),
+    // we delete the conversation and blacklist it so it's never refetched from the backend
+
+    const deletedConnectionRequests = [] as ConversationDatabaseData[];
+    const filteredLocalConversations = [] as ConversationDatabaseData[];
+
+    for (const conversation of localConversations) {
+      const {type, qualified_id, id, domain} = conversation;
+
+      const isDeletedConnectionRequest =
+        type === CONVERSATION_TYPE.CONNECT &&
+        !connections.find(connection => matchQualifiedIds(connection.conversationId, qualified_id || {id, domain}));
+
+      if (isDeletedConnectionRequest) {
+        deletedConnectionRequests.push(conversation);
+      } else {
+        filteredLocalConversations.push(conversation);
+      }
+    }
+
+    const filteredRemoteConversations = remoteConversations.found?.filter(
+      remoteConversation =>
+        !deletedConnectionRequests.find(({qualified_id, id, domain}) =>
+          matchQualifiedIds(qualified_id || {id, domain}, remoteConversation.qualified_id),
+        ),
+    );
+
+    await Promise.all(
+      deletedConnectionRequests.map(async ({id, domain, qualified_id}) => {
+        const qualifiedId = qualified_id || {id, domain};
+        await this.conversationService.deleteConversationFromDb(qualifiedId.id);
+        await this.blacklistConversation(qualifiedId);
+      }),
+    );
+
+    return {
+      localConversations: filteredLocalConversations,
+      remoteConversations: {...remoteConversations, found: filteredRemoteConversations},
+    };
+  }
+
+  private async filterLoadedConversations(
+    localConversations: ConversationDatabaseData[],
+    remoteConversations: RemoteConversations,
+    connections: ConnectionEntity[],
+  ): Promise<{localConversations: ConversationDatabaseData[]; remoteConversations: RemoteConversations}> {
+    const filteredAbandonedRemoteConversations = await this.filterAbandonedProteus1to1Conversations(
+      remoteConversations,
+      localConversations,
+    );
+
+    return this.filterDeletedConnectionRequests(localConversations, filteredAbandonedRemoteConversations, connections);
+  }
+
   /**
    * Will append the new conversations from backend to the locally stored conversations in memory
    * @param remoteConversations new conversations fetched from backend
    * @returns the new conversations from backend merged with the locally stored conversations
    */
-  private async loadRemoteConversations(remoteConversations: RemoteConversations): Promise<Conversation[]> {
+  private async loadRemoteConversations(
+    remoteConversations: RemoteConversations,
+    connections: ConnectionEntity[],
+  ): Promise<Conversation[]> {
     const localConversations = await this.conversationService.loadConversationStatesFromDb<ConversationDatabaseData>();
 
     let conversationsData: any[];
 
+    const {localConversations: filteredLocalConversations, remoteConversations: filteredRemoteConversations} =
+      await this.filterLoadedConversations(localConversations, remoteConversations, connections);
+
     if (!remoteConversations.found?.length) {
-      conversationsData = localConversations;
+      conversationsData = filteredLocalConversations;
     } else {
-      const filteredRemoteConversations = await this.filterAbandonedProteus1to1Conversations(
-        remoteConversations,
-        localConversations,
-      );
-      const data = ConversationMapper.mergeConversations(localConversations, filteredRemoteConversations);
+      const data = ConversationMapper.mergeConversations(filteredLocalConversations, filteredRemoteConversations);
       conversationsData = (await this.conversationService.saveConversationsInDb(data)) as any[];
     }
 
-    const allConversationEntities = this.mapConversations(conversationsData);
+    const allConversationEntities = conversationsData.length ? this.mapConversations(conversationsData) : [];
     const newConversationEntities = allConversationEntities.filter(
       allConversations =>
         !this.conversationState
@@ -2293,6 +2358,18 @@ export class ConversationRepository {
       .catch(error => this.handleAddToConversationError(error, conversationEntity, [{domain: '', id: serviceId}]));
   }
 
+  private deleteConnectionRequestConversation = async (userId: QualifiedId) => {
+    const connection = this.connectionState
+      .connections()
+      .find(connection => matchQualifiedIds(connection.userId, userId));
+
+    if (!connection) {
+      return;
+    }
+
+    return this.deleteConversationLocally(connection.conversationId, true);
+  };
+
   private handleAddToConversationError(error: BackendError, conversationEntity: Conversation, userIds: QualifiedId[]) {
     switch (error.label) {
       case BackendErrorLabel.NOT_CONNECTED: {

src/script/main/app.ts

@@ -445,7 +445,7 @@ export class App {
 
       telemetry.addStatistic(AppInitStatisticsValue.CONNECTIONS, connections.length, 50);
 
-      const conversations = await conversationRepository.loadConversations();
+      const conversations = await conversationRepository.loadConversations(connections);
 
       // We load all the users the self user is connected with
       await userRepository.loadUsers(selfUser, connections, conversations, teamMembers);