diff --git a/changelog.d/3-bug-fixes/list-self-mls-not-configured b/changelog.d/3-bug-fixes/list-self-mls-not-configured new file mode 100644 index 0000000000..74e6066571 --- /dev/null +++ b/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 diff --git a/services/galley/src/Galley/API/Public/Conversation.hs b/services/galley/src/Galley/API/Public/Conversation.hs index 78f90daf18..ec8d06bbed 100644 --- a/services/galley/src/Galley/API/Public/Conversation.hs +++ b/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 diff --git a/services/galley/src/Galley/API/Query.hs b/services/galley/src/Galley/API/Query.hs index 56bbd62661..aefb08a810 100644 --- a/services/galley/src/Galley/API/Query.hs +++ b/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 diff --git a/services/galley/test/integration/API/MLS.hs b/services/galley/test/integration/API/MLS.hs index 61e28abb3c..84629a03e3 100644 --- a/services/galley/test/integration/API/MLS.hs +++ b/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 ) 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]