diff --git a/changelog.d/3-bug-fixes/WPB-21359 b/changelog.d/3-bug-fixes/WPB-21359 new file mode 100644 index 00000000000..4c0a7892df3 --- /dev/null +++ b/changelog.d/3-bug-fixes/WPB-21359 @@ -0,0 +1 @@ +Fix: Inconsistent removal messages across local and federated conversation members diff --git a/integration/test/MLS/Util.hs b/integration/test/MLS/Util.hs index a30031f6362..cdbd6fcee2c 100644 --- a/integration/test/MLS/Util.hs +++ b/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 diff --git a/libs/wire-subsystems/src/Wire/ConversationStore/MLS/Types.hs b/libs/wire-subsystems/src/Wire/ConversationStore/MLS/Types.hs index 2573cfdd0fd..f3e620ed1de 100644 --- a/libs/wire-subsystems/src/Wire/ConversationStore/MLS/Types.hs +++ b/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. diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs index 1059711effd..dde9267c7e8 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs +++ b/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,11 +341,16 @@ 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 -> @@ -353,11 +358,8 @@ postMLSCommitBundleToLocalConv qusr c conn bundle ctype lConvOrSubId = do -- 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