From 0b200d1504e59110438de690dff4a8cfd2817ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Thu, 1 Dec 2022 14:29:57 +0100 Subject: [PATCH 1/3] Test: don't fail /list-ids when MLS not configured --- services/galley/test/integration/API/MLS.hs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) 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] From 7fec2af91ce7532fbdaefce6b6c9a3b5684024ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Thu, 1 Dec 2022 14:30:51 +0100 Subject: [PATCH 2/3] Do not error for /list-ids when MLS not configured for --- .../src/Galley/API/Public/Conversation.hs | 2 +- services/galley/src/Galley/API/Query.hs | 47 ++++++++++++++----- 2 files changed, 35 insertions(+), 14 deletions(-) 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 From ca32b64bdb6e8cf36bbde477b3b465978a38f293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Thu, 1 Dec 2022 14:49:29 +0100 Subject: [PATCH 3/3] Add a changelog --- changelog.d/3-bug-fixes/list-self-mls-not-configured | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3-bug-fixes/list-self-mls-not-configured 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