[FS-1271] Do not throw 500 when listing conversations

changelog.d/3-bug-fixes/list-self-mls-not-configured

@@ -0,0 +1 @@
+Do not throw 500 when listing conversations and MLS is not configured

services/galley/src/Galley/API/Public/Conversation.hs

@@ -44,7 +44,7 @@ conversationAPI =
     <@> mkNamedAPI @"get-conversation-by-reusable-code" (getConversationByReusableCode @Cassandra)
     <@> mkNamedAPI @"create-group-conversation" createGroupConversation
     <@> mkNamedAPI @"create-self-conversation" createProteusSelfConversation
-    <@> mkNamedAPI @"get-mls-self-conversation" getMLSSelfConversation
+    <@> mkNamedAPI @"get-mls-self-conversation" getMLSSelfConversationWithError
     <@> mkNamedAPI @"create-one-to-one-conversation" createOne2OneConversation
     <@> mkNamedAPI @"add-members-to-conversation-unqualified" addMembersUnqualified
     <@> mkNamedAPI @"add-members-to-conversation-unqualified2" addMembersUnqualifiedV2

services/galley/src/Galley/API/Query.hs

@@ -37,6 +37,7 @@ module Galley.API.Query
     getConversationGuestLinksStatus,
     ensureConvAdmin,
     getMLSSelfConversation,
+    getMLSSelfConversationWithError,
   )
 where
 
@@ -450,7 +451,14 @@ conversationIdsPageFrom lusr state = do
   -- NOTE: Getting the MLS self-conversation creates it in case it does not
   -- exist yet. This is to ensure it is automatically listed without needing to
   -- create it separately.
-  void $ getMLSSelfConversation lusr
+  --
+  -- Make sure that in case MLS is not configured (the non-existance of the
+  -- backend removal key is a proxy for it) the self-conversation is not
+  -- returned or attempted to be created; in that case we skip anything related
+  -- to it.
+  whenM (isJust <$> getMLSRemovalKey)
+    . void
+    $ getMLSSelfConversation lusr
   conversationIdsPageFromV2 ListGlobalSelf lusr state
 
 getConversations ::
@@ -699,6 +707,29 @@ getConversationGuestLinksFeatureStatus mbTid = do
       mbLockStatus <- TeamFeatures.getFeatureLockStatus @db (Proxy @GuestLinksConfig) tid
       pure $ computeFeatureConfigForTeamUser mbConfigNoLock mbLockStatus defaultStatus
 
+-- | The same as 'getMLSSelfConversation', but it throws an error in case the
+-- backend is not configured for MLS (the proxy for it being the existance of
+-- the backend removal key).
+getMLSSelfConversationWithError ::
+  forall r.
+  Members
+    '[ ConversationStore,
+       Error InternalError,
+       Input Env,
+       P.TinyLog
+     ]
+    r =>
+  Local UserId ->
+  Sem r Conversation
+getMLSSelfConversationWithError lusr = do
+  unlessM (isJust <$> getMLSRemovalKey) $
+    throw (InternalErrorWithDescription noKeyMsg)
+  getMLSSelfConversation lusr
+  where
+    noKeyMsg =
+      "No backend removal key is configured (See 'mlsPrivateKeyPaths'"
+        <> "in galley's config). Refusing to create MLS conversation."
+
 -- | Get an MLS self conversation. In case it does not exist, it is partially
 -- created in the database. The part that is not written is the epoch number;
 -- the number is inserted only upon the first commit. With this we avoid race
@@ -717,20 +748,10 @@ getMLSSelfConversation ::
   Local UserId ->
   Sem r Conversation
 getMLSSelfConversation lusr = do
-  let selfConvId = mlsSelfConvId usr
+  let selfConvId = mlsSelfConvId . tUnqualified $ lusr
   mconv <- E.getConversation selfConvId
-  cnv <- maybe create pure mconv
+  cnv <- maybe (E.createMLSSelfConversation lusr) pure mconv
   conversationView lusr cnv
-  where
-    usr = tUnqualified lusr
-    create :: Sem r Data.Conversation
-    create = do
-      unlessM (isJust <$> getMLSRemovalKey) $
-        throw (InternalErrorWithDescription noKeyMsg)
-      E.createMLSSelfConversation lusr
-    noKeyMsg =
-      "No backend removal key is configured (See 'mlsPrivateKeyPaths'"
-        <> "in galley's config). Refusing to create MLS conversation."
 
 -------------------------------------------------------------------------------
 -- Helpers

services/galley/test/integration/API/MLS.hs

@@ -27,7 +27,7 @@ import Bilge.Assert
 import Cassandra
 -- import Control.Error.Util (hush)
 -- import Control.Lens (view, (^.))
-import Control.Lens (view)
+import Control.Lens (view, (%~), (.~))
 import qualified Control.Monad.State as State
 import Crypto.Error
 import qualified Crypto.PubKey.Ed25519 as Ed25519
@@ -48,6 +48,7 @@ import Data.String.Conversions
 import qualified Data.Text as T
 import Data.Time
 import Federator.MockServer hiding (withTempMockFederator)
+import qualified Galley.Options as Opts
 -- import Galley.Data.Conversation
 -- import Galley.Options
 import Imports
@@ -213,6 +214,7 @@ tests s =
         [ test s "create a self conversation" testSelfConversation,
           test s "do not list a self conversation below v3" $ testSelfConversationList True,
           test s "list a self conversation automatically from v3" $ testSelfConversationList False,
+          test s "listing conversations without MLS configured" testSelfConversationMLSNotConfigured,
           test s "attempt to add another user to a conversation fails" testSelfConversationOtherUser,
           test s "attempt to leave fails" testSelfConversationLeave
         ]
@@ -2416,6 +2418,16 @@ testSelfConversationList isBelowV3 = do
             <!! const 200 === statusCode
       pure $ foldr (<|>) Nothing $ guard . isMLSSelf u <$> mtpResults convIds
 
+testSelfConversationMLSNotConfigured :: TestM ()
+testSelfConversationMLSNotConfigured = do
+  alice <- randomUser
+  let paginationOpts = GetPaginatedConversationIds Nothing (toRange (Proxy @100))
+      noMLS = Opts.optSettings %~ Opts.setMlsPrivateKeyPaths .~ Nothing
+  runMLSTest
+    . liftTest
+    . withSettingsOverrides noMLS
+    $ listConvIds alice paginationOpts !!! const 200 === statusCode
+
 testSelfConversationOtherUser :: TestM ()
 testSelfConversationOtherUser = do
   users@[_alice, bob] <- createAndConnectUsers [Nothing, Nothing]