[FS-1249] Do Not List MLS Self-conversation in client API v1 and v2

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

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)

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

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 =>

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
-        <!! const 200 === statusCode
-  convs <-
-    forM (mtpResults convIds) (responseJsonError <=< getConvQualified alice)
-  let mMLSSelf = foldr (<|>) 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
+            <!! const 200 === statusCode
+      pure $ foldr (<|>) Nothing $ guard . isMLSSelf u <$> mtpResults convIds
 
 testSelfConversationOtherUser :: TestM ()
 testSelfConversationOtherUser = do