WPB-21359 fix: Inconsistent notifications are sent to local and remote members when they're removed from MLS conversations

changelog.d/3-bug-fixes/WPB-21359

@@ -0,0 +1 @@
+Fix: Inconsistent removal messages across local and federated conversation members

integration/test/MLS/Util.hs

@@ -682,7 +682,8 @@ consumingMessages mlsProtocol mp = Codensity $ \k -> do
 
     -- at this point we know that every new user has been added to the
     -- conversation
-    for_ (zip clients wss) $ \((cid, t), ws) -> case t of
+    let cssNoToBeRemoved = filter (\((ci, _), _) -> ci `Set.notMember` conv.membersToBeRemoved) (zip clients wss)
+    for_ cssNoToBeRemoved $ \((cid, t), ws) -> case t of
       MLSNotificationMessageTag ->
         when (conv.epoch > 0) $
           void $
@@ -717,7 +718,7 @@ consumeMessageNoExternal cs cid mp = consumeMessageWithPredicate isNewMLSMessage
   where
     -- the backend (correctly) reacts to a commit removing someone from a parent conversation with a
     -- remove proposal, however, we don't want to consume this here
-    isNewMLSMessageNotifButNoProposal :: Value -> App Bool
+    isNewMLSMessageNotifButNoProposal :: (HasCallStack) => Value -> App Bool
     isNewMLSMessageNotifButNoProposal n = do
       isRelevantNotif <- isNewMLSMessageNotif n &&~ isNotifConvId mp.convId n
       if isRelevantNotif

libs/wire-subsystems/src/Wire/ConversationStore/MLS/Types.hs

@@ -174,6 +174,10 @@ cmSingleton cid idx =
       (cidQualifiedUser cid)
       (Map.singleton (ciClient cid) idx)
 
+-- | Construct the subset of the first client map consisting of users that are present in the second.
+cmIntersect :: ClientMap a -> ClientMap b -> ClientMap a
+cmIntersect (ClientMap m1) (ClientMap m2) = ClientMap (Map.intersection m1 m2)
+
 -- | Inform a handler for 'POST /conversations/list-ids' if the MLS global team
 -- conversation and the MLS self-conversation should be included in the
 -- response.

libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs

@@ -297,7 +297,7 @@ postMLSCommitBundleToLocalConv qusr c conn bundle ctype lConvOrSubId = do
 
   senderIdentity <- getSenderIdentity qusr c bundle.sender lConvOrSub
 
-  (events, newClients) <- handleGroupInfoMismatch lConvOrSubId bundle $ lowerCodensity $ do
+  (events, newClients, lConvOrSub') <- handleGroupInfoMismatch lConvOrSubId bundle $ lowerCodensity $ do
     (events, newClients) <- case senderIdentity.index of
       Just _ -> do
         -- extract added/removed clients from bundle
@@ -341,23 +341,25 @@ postMLSCommitBundleToLocalConv qusr c conn bundle ctype lConvOrSubId = do
           action
           bundle.commit.value.path
         pure ([], mempty)
-    lift $ do
+    lConvOrSub' <- lift $ do
       updateOutOfSyncFlag senderIdentity.client lConvOrSub
       storeGroupInfo convOrSub.id (GroupInfoData bundle.groupInfo.raw)
-      propagateMessage qusr (Just c) lConvOrSub conn bundle.rawMessage convOrSub.members
-    pure (events, newClients)
+      -- reload conversation from db to make sure we have an up-to-date list of members
+      lConvOrSub' <- fetchConvOrSub qusr bundle.groupId ctype lConvOrSubId
+      let convOrSub' = tUnqualified lConvOrSub'
+          mems = cmIntersect (void convOrSub.members) convOrSub'.members
+      propagateMessage qusr (Just c) lConvOrSub conn bundle.rawMessage mems
+      pure lConvOrSub'
+    pure (events, newClients, lConvOrSub')
 
   -- send welcome messages
   for_ bundle.welcome $ \welcome ->
     sendWelcomes lConvOrSubId qusr conn (cmIdentities newClients) welcome
 
   -- send application message
   for_ bundle.appMessage $ \msg -> do
-    -- reload conversation from db to make sure we have an up-to-date list of members
-    lConvOrSub' <- fetchConvOrSub qusr bundle.groupId ctype lConvOrSubId
     let convOrSub' = tUnqualified lConvOrSub'
-    propagateMessage qusr (Just c) lConvOrSub' conn msg.rawMessage $
-      void convOrSub'.members
+    propagateMessage qusr (Just c) lConvOrSub' conn msg.rawMessage convOrSub'.members
 
   pure events