diff --git a/changelog.d/3-bug-fixes/fix-subconv-client-check b/changelog.d/3-bug-fixes/fix-subconv-client-check new file mode 100644 index 0000000000..fd8a929b54 --- /dev/null +++ b/changelog.d/3-bug-fixes/fix-subconv-client-check @@ -0,0 +1 @@ +Remove client check for subconversations diff --git a/integration/test/Test/MLS.hs b/integration/test/Test/MLS.hs index f9c966f78e..cbf41adcf0 100644 --- a/integration/test/Test/MLS.hs +++ b/integration/test/Test/MLS.hs @@ -389,13 +389,14 @@ testRemoteRemoveClient = do testCreateSubConv :: HasCallStack => Ciphersuite -> App () testCreateSubConv suite = do setMLSCiphersuite suite - alice <- randomUser OwnDomain def - alice1 <- createMLSClient def alice - (_, conv) <- createNewGroup alice1 - bindResponse (getSubConversation alice conv "conference") $ \resp -> do - resp.status `shouldMatchInt` 200 - let tm = resp.json %. "epoch_timestamp" - tm `shouldMatch` Null + [alice, bob] <- createAndConnectUsers [OwnDomain, OwnDomain] + aliceClients@(alice1 : _) <- replicateM 5 $ createMLSClient def alice + replicateM_ 3 $ traverse_ uploadNewKeyPackage aliceClients + [bob1, bob2] <- replicateM 2 $ createMLSClient def bob + replicateM_ 3 $ traverse_ uploadNewKeyPackage [bob1, bob2] + void $ createNewGroup alice1 + void $ createAddCommit alice1 [alice, bob] >>= sendAndConsumeCommitBundle + createSubConv alice1 "conference" testCreateSubConvProteus :: App () testCreateSubConvProteus = do diff --git a/services/galley/src/Galley/API/MLS/Commit/InternalCommit.hs b/services/galley/src/Galley/API/MLS/Commit/InternalCommit.hs index b7ac03592c..07bc721538 100644 --- a/services/galley/src/Galley/API/MLS/Commit/InternalCommit.hs +++ b/services/galley/src/Galley/API/MLS/Commit/InternalCommit.hs @@ -133,39 +133,47 @@ processInternalCommit senderIdentity con lConvOrSub epoch action commit = do pure qtarget - -- for each user, we compare their clients with the ones being added to the conversation - failedAddFetching <- fmap catMaybes . forM newUserClients $ - \(qtarget, newclients) -> case Map.lookup qtarget cm of - -- user is already present, skip check in this case - Just _ -> do - -- new user - pure Nothing - Nothing -> do - -- final set of clients in the conversation - let clients = Map.keysSet (newclients <> Map.findWithDefault mempty qtarget cm) - -- get list of mls clients from Brig (local or remote) - getClientInfo lConvOrSub qtarget suite >>= \case - Left _e -> pure (Just qtarget) - Right clientInfo -> do - let allClients = Set.map ciId clientInfo - let allMLSClients = Set.map ciId (Set.filter ciMLS clientInfo) - -- We check the following condition: - -- allMLSClients ⊆ clients ⊆ allClients - -- i.e. - -- - if a client has at least 1 key package, it has to be added - -- - if a client is being added, it has to still exist - -- - -- The reason why we can't simply check that clients == allMLSClients is - -- that a client with no remaining key packages might be added by a user - -- who just fetched its last key package. - unless - ( Set.isSubsetOf allMLSClients clients - && Set.isSubsetOf clients allClients - ) - $ do - -- FUTUREWORK: turn this error into a proper response - throwS @'MLSClientMismatch + -- For each user, we compare their clients with the ones being added + -- to the conversation, and return a list of users for of which we + -- were unable to get a list of MLS-capable clients. + -- + -- Again, for subconversations there is no need to check anything + -- here, so we simply return the empty list. + failedAddFetching <- case convOrSub.id of + SubConv _ _ -> pure [] + Conv _ -> + fmap catMaybes . forM newUserClients $ + \(qtarget, newclients) -> case Map.lookup qtarget cm of + -- user is already present, skip check in this case + Just _ -> do + -- new user pure Nothing + Nothing -> do + -- final set of clients in the conversation + let clients = Map.keysSet (newclients <> Map.findWithDefault mempty qtarget cm) + -- get list of mls clients from Brig (local or remote) + getClientInfo lConvOrSub qtarget suite >>= \case + Left _e -> pure (Just qtarget) + Right clientInfo -> do + let allClients = Set.map ciId clientInfo + let allMLSClients = Set.map ciId (Set.filter ciMLS clientInfo) + -- We check the following condition: + -- allMLSClients ⊆ clients ⊆ allClients + -- i.e. + -- - if a client has at least 1 key package, it has to be added + -- - if a client is being added, it has to still exist + -- + -- The reason why we can't simply check that clients == allMLSClients is + -- that a client with no remaining key packages might be added by a user + -- who just fetched its last key package. + unless + ( Set.isSubsetOf allMLSClients clients + && Set.isSubsetOf clients allClients + ) + $ + -- FUTUREWORK: turn this error into a proper response + throwS @'MLSClientMismatch + pure Nothing for_ (unreachableFromList failedAddFetching) (throw . unreachableUsersToUnreachableBackends) @@ -225,7 +233,8 @@ processInternalCommit senderIdentity con lConvOrSub epoch action commit = do cjRole = roleNameWireMember } pure [update] - _ -> do + SubConv _ _ -> pure [] + Conv _ -> do -- remove users from the conversation and send events removeEvents <- foldMap