diff --git a/changelog.d/3-bug-fixes/mls-self-conv-not-listed-below-v3 b/changelog.d/3-bug-fixes/mls-self-conv-not-listed-below-v3 new file mode 100644 index 0000000000..d656f28b45 --- /dev/null +++ b/changelog.d/3-bug-fixes/mls-self-conv-not-listed-below-v3 @@ -0,0 +1 @@ +Do not list MLS self-conversation in client API v1 and v2 if it exists diff --git a/services/galley/src/Galley/API/MLS/Types.hs b/services/galley/src/Galley/API/MLS/Types.hs index f9b6cefb8e..935702c3a5 100644 --- a/services/galley/src/Galley/API/MLS/Types.hs +++ b/services/galley/src/Galley/API/MLS/Types.hs @@ -19,6 +19,7 @@ module Galley.API.MLS.Types ( ClientMap, mkClientMap, cmAssocs, + ListGlobalSelfConvs (..), ) where @@ -41,3 +42,9 @@ mkClientMap = foldr addEntry mempty cmAssocs :: ClientMap -> [(Qualified UserId, (ClientId, KeyPackageRef))] cmAssocs cm = Map.assocs cm >>= traverse toList + +-- | 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. +data ListGlobalSelfConvs = ListGlobalSelf | DoNotListGlobalSelf + deriving (Eq) diff --git a/services/galley/src/Galley/API/Public/Conversation.hs b/services/galley/src/Galley/API/Public/Conversation.hs index 9512508c51..223cd96d46 100644 --- a/services/galley/src/Galley/API/Public/Conversation.hs +++ b/services/galley/src/Galley/API/Public/Conversation.hs @@ -19,6 +19,7 @@ module Galley.API.Public.Conversation where import Galley.API.Create import Galley.API.MLS.GroupInfo +import Galley.API.MLS.Types import Galley.API.Query import Galley.API.Update import Galley.App @@ -35,7 +36,7 @@ conversationAPI = <@> mkNamedAPI @"get-conversation-roles" getConversationRoles <@> mkNamedAPI @"get-group-info" getGroupInfo <@> mkNamedAPI @"list-conversation-ids-unqualified" conversationIdsPageFromUnqualified - <@> mkNamedAPI @"list-conversation-ids-v2" conversationIdsPageFromV2 + <@> mkNamedAPI @"list-conversation-ids-v2" (conversationIdsPageFromV2 DoNotListGlobalSelf) <@> mkNamedAPI @"list-conversation-ids" conversationIdsPageFrom <@> mkNamedAPI @"get-conversations" getConversations <@> mkNamedAPI @"list-conversations-v1" listConversations diff --git a/services/galley/src/Galley/API/Query.hs b/services/galley/src/Galley/API/Query.hs index 4643c462fd..9262ee1d3e 100644 --- a/services/galley/src/Galley/API/Query.hs +++ b/services/galley/src/Galley/API/Query.hs @@ -70,6 +70,7 @@ import Data.Range import qualified Data.Set as Set import Galley.API.Error import Galley.API.MLS.Keys +import Galley.API.MLS.Types import Galley.API.Mapping import qualified Galley.API.Mapping as Mapping import Galley.API.Util @@ -351,10 +352,11 @@ conversationIdsPageFromV2 :: ] r ) => + ListGlobalSelfConvs -> Local UserId -> Public.GetPaginatedConversationIds -> Sem r Public.ConvIdsPage -conversationIdsPageFromV2 lusr Public.GetMultiTablePageRequest {..} = do +conversationIdsPageFromV2 listGlobalSelf lusr Public.GetMultiTablePageRequest {..} = do let localDomain = tDomain lusr case gmtprState of Just (Public.ConversationPagingState Public.PagingRemotes stateBS) -> @@ -375,11 +377,17 @@ conversationIdsPageFromV2 lusr Public.GetMultiTablePageRequest {..} = do <$> E.listItems (tUnqualified lusr) pagingState size let remainingSize = fromRange size - fromIntegral (length (Public.mtpResults localPage)) if Public.mtpHasMore localPage || remainingSize <= 0 - then pure localPage {Public.mtpHasMore = True} -- We haven't checked the remotes yet, so has_more must always be True here. + then -- We haven't checked the remotes yet, so has_more must always be True here. + pure (filterOut localPage) {Public.mtpHasMore = True} else do -- remainingSize <= size and remainingSize >= 1, so it is safe to convert to Range remotePage <- remotesOnly Nothing (unsafeRange remainingSize) - pure $ remotePage {Public.mtpResults = Public.mtpResults localPage <> Public.mtpResults remotePage} + pure $ + remotePage + { Public.mtpResults = + Public.mtpResults (filterOut localPage) + <> Public.mtpResults remotePage + } remotesOnly :: Maybe C.PagingState -> @@ -398,6 +406,22 @@ conversationIdsPageFromV2 lusr Public.GetMultiTablePageRequest {..} = do mtpPagingState = Public.ConversationPagingState table (LBS.toStrict . C.unPagingState <$> pwsState) } + -- MLS self-conversation of this user + selfConvId = mlsSelfConvId (tUnqualified lusr) + isNotSelfConv = (/= selfConvId) . qUnqualified + + -- If this is an old client making a request (i.e., a V1 or V2 client), make + -- sure to filter out the MLS global team conversation and the MLS + -- self-conversation. + -- + -- FUTUREWORK: This is yet to be implemented for global team conversations. + filterOut :: ConvIdsPage -> ConvIdsPage + filterOut page | listGlobalSelf == ListGlobalSelf = page + filterOut page = + page + { Public.mtpResults = filter isNotSelfConv $ Public.mtpResults page + } + -- | Lists conversation ids for the logged in user in a paginated way. -- -- Pagination requires an order, in this case the order is defined as: @@ -427,7 +451,7 @@ conversationIdsPageFrom lusr state = do -- exist yet. This is to ensure it is automatically listed without needing to -- create it separately. void $ getMLSSelfConversation lusr - conversationIdsPageFromV2 lusr state + conversationIdsPageFromV2 ListGlobalSelf lusr state getConversations :: Members '[Error InternalError, ListItems LegacyPaging ConvId, ConversationStore, P.TinyLog] r => diff --git a/services/galley/test/integration/API/MLS.hs b/services/galley/test/integration/API/MLS.hs index 37d95a1866..660ea3b3bf 100644 --- a/services/galley/test/integration/API/MLS.hs +++ b/services/galley/test/integration/API/MLS.hs @@ -2346,7 +2346,7 @@ testSelfConversation = do commit <- createAddCommit creator [alice] welcome <- assertJust (mpWelcome commit) mlsBracket others $ \wss -> do - void $ sendAndConsumeCommit commit + void $ sendAndConsumeCommitBundle commit WS.assertMatchN_ (5 # Second) wss $ wsAssertMLSWelcome alice welcome WS.assertNoEvent (1 # WS.Second) wss @@ -2361,19 +2361,26 @@ testSelfConversationList isBelowV3 = do then ("The MLS self-conversation is listed", isNothing, listConvIdsV2) else ("The MLS self-conversation is not listed", isJust, listConvIds) alice <- randomUser - let paginationOpts = GetPaginatedConversationIds Nothing (toRange (Proxy @100)) - convIds :: ConvIdsPage <- - responseJsonError - =<< listCnvs alice paginationOpts - ) Nothing $ guard . isMLSSelf <$> convs - liftIO $ assertBool errMsg (justOrNothing mMLSSelf) + do + mMLSSelf <- findSelfConv alice listCnvs + liftIO $ assertBool errMsg (justOrNothing mMLSSelf) + + -- make sure that the self-conversation is not listed below V3 even once it + -- has been created. + unless isBelowV3 $ do + mMLSSelf <- findSelfConv alice listConvIdsV2 + liftIO $ assertBool errMsg (isNothing mMLSSelf) where - isMLSSelf conv = - cnvType conv == SelfConv - && protocolTag (cnvProtocol conv) == ProtocolMLSTag + paginationOpts = GetPaginatedConversationIds Nothing (toRange (Proxy @100)) + + isMLSSelf u conv = mlsSelfConvId u == qUnqualified conv + + findSelfConv u listEndpoint = do + convIds :: ConvIdsPage <- + responseJsonError + =<< listEndpoint u paginationOpts + ) Nothing $ guard . isMLSSelf u <$> mtpResults convIds testSelfConversationOtherUser :: TestM () testSelfConversationOtherUser = do