From 80f1a9e6cd4a8b2083612d4277730c9263bb4c6a Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Wed, 23 Nov 2022 10:48:27 +0000 Subject: [PATCH 1/7] Improve dealing with global team convo --- services/galley/src/Galley/API/Action.hs | 14 +---- services/galley/src/Galley/API/MLS/Message.hs | 34 ++++++----- services/galley/src/Galley/API/MLS/Util.hs | 58 ++++++++----------- services/galley/src/Galley/API/Query.hs | 6 +- services/galley/src/Galley/API/Util.hs | 42 ++++++++++++-- .../src/Galley/Cassandra/Conversation.hs | 20 ------- .../src/Galley/Effects/ConversationStore.hs | 2 - services/galley/test/integration/API/MLS.hs | 27 ++++++++- 8 files changed, 112 insertions(+), 91 deletions(-) diff --git a/services/galley/src/Galley/API/Action.hs b/services/galley/src/Galley/API/Action.hs index 2acdd7858c6..44e9d9d4338 100644 --- a/services/galley/src/Galley/API/Action.hs +++ b/services/galley/src/Galley/API/Action.hs @@ -52,7 +52,6 @@ import Data.Singletons import Data.Time.Clock import Galley.API.Error import Galley.API.MLS.Removal -import Galley.API.MLS.Util (globalTeamConvToConversation) import Galley.API.Util import Galley.App import Galley.Data.Conversation @@ -90,7 +89,6 @@ import Wire.API.Event.Conversation import Wire.API.Federation.API (Component (Galley), fedClient) import Wire.API.Federation.API.Galley import Wire.API.Federation.Error -import Wire.API.MLS.GlobalTeamConversation import Wire.API.Team.LegalHold import Wire.API.Team.Member import qualified Wire.API.User as User @@ -597,17 +595,7 @@ updateLocalConversation lcnv qusr con action = do let tag = sing @tag -- retrieve conversation - conv <- do - -- Check if global or not, if global, map it to conversation - E.getGlobalTeamConversationById lcnv >>= \case - Just gtc -> - let c = gtcCreator gtc - in case c of - Nothing -> - throwS @'ConvNotFound - Just creator -> - pure $ globalTeamConvToConversation gtc creator mempty - Nothing -> getConversationWithError lcnv + conv <- getConversationWithError lcnv (qUnqualified qusr) -- check that the action does not bypass the underlying protocol unless (protocolValidAction (convProtocol conv) (fromSing tag)) $ diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index 7bd31569e0a..24d6ea0c0b6 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -793,20 +793,23 @@ processCommitWithAction qusr senderClient con lconv mlsMeta cm epoch action send processInternalCommit :: forall r. ( HasProposalEffects r, - Member (Error FederationError) r, - Member (Error InternalError) r, - Member (ErrorS 'ConvNotFound) r, - Member (ErrorS 'MLSClientSenderUserMismatch) r, - Member (ErrorS 'MLSCommitMissingReferences) r, - Member (ErrorS 'MLSMissingSenderClient) r, - Member (ErrorS 'MLSProposalNotFound) r, - Member (ErrorS 'MLSSelfRemovalNotAllowed) r, - Member (ErrorS 'MLSStaleMessage) r, - Member (ErrorS 'MissingLegalholdConsent) r, - Member (Input (Local ())) r, - Member ProposalStore r, - Member BrigAccess r, - Member Resource r + Members + [ Error FederationError, + Error InternalError, + ErrorS 'ConvNotFound, + ErrorS 'MLSClientSenderUserMismatch, + ErrorS 'MLSCommitMissingReferences, + ErrorS 'MLSMissingSenderClient, + ErrorS 'MLSProposalNotFound, + ErrorS 'MLSSelfRemovalNotAllowed, + ErrorS 'MLSStaleMessage, + ErrorS 'MissingLegalholdConsent, + Input (Local ()), + ProposalStore, + BrigAccess, + Resource + ] + r ) => Qualified UserId -> Maybe ClientId -> @@ -858,6 +861,9 @@ processInternalCommit qusr senderClient con lconv mlsMeta cm epoch action sender . upLeaf ) $ cPath commit + -- add user to global conv as a member as well + lusr <- qualifyLocal (qUnqualified qusr) + void $ createMember (convId <$> lconv) lusr addMLSClients (cnvmlsGroupId mlsMeta) qusr diff --git a/services/galley/src/Galley/API/MLS/Util.hs b/services/galley/src/Galley/API/MLS/Util.hs index 0fefb44af15..9ea00c340e6 100644 --- a/services/galley/src/Galley/API/MLS/Util.hs +++ b/services/galley/src/Galley/API/MLS/Util.hs @@ -26,7 +26,6 @@ import Galley.Effects import Galley.Effects.ConversationStore import Galley.Effects.MemberStore import Galley.Effects.ProposalStore -import Galley.Types.Conversations.Members import Imports import Polysemy import Polysemy.TinyLog (TinyLog) @@ -41,31 +40,6 @@ import Wire.API.MLS.KeyPackage import Wire.API.MLS.Proposal import Wire.API.MLS.Serialisation -globalTeamConvToConversation :: - GlobalTeamConversation -> - UserId -> - [LocalMember] -> - Conversation -globalTeamConvToConversation gtc creator lMembers = - Conversation - { convId = qUnqualified $ gtcId gtc, - convLocalMembers = lMembers, - convRemoteMembers = mempty, - convDeleted = False, - convMetadata = - ConversationMetadata - { cnvmType = GlobalTeamConv, - cnvmCreator = creator, - cnvmAccess = gtcAccess gtc, - cnvmAccessRoles = mempty, - cnvmName = Just (gtcName gtc), - cnvmTeam = Just (gtcTeam gtc), - cnvmMessageTimer = Nothing, - cnvmReceiptMode = Nothing - }, - convProtocol = ProtocolMLS (gtcMlsMetadata gtc) - } - getLocalConvForUser :: Members '[ ErrorS 'ConvNotFound, @@ -80,16 +54,8 @@ getLocalConvForUser qusr lcnv = do gtc <- getGlobalTeamConversationById lcnv conv <- case gtc of Just conv -> do - let creator = gtcCreator conv localMembers <- getLocalMembers (qUnqualified . gtcId $ conv) - - -- no creator means the conversation has been setup on backend but not on MLS. - case creator of - Nothing -> do - setGlobalTeamConversationCreator conv (qUnqualified qusr) - pure $ globalTeamConvToConversation conv (qUnqualified qusr) localMembers - Just creator' -> - pure $ globalTeamConvToConversation conv creator' localMembers + pure $ toConv conv (qUnqualified qusr) localMembers Nothing -> do getConversation (tUnqualified lcnv) >>= noteS @'ConvNotFound @@ -103,6 +69,28 @@ getLocalConvForUser qusr lcnv = do unless isMember' $ throwS @'ConvNotFound pure conv + where + toConv gtc usr lm = + let mlsData = gtcMlsMetadata gtc + in Data.Conversation + { convId = qUnqualified $ gtcId gtc, + convLocalMembers = lm, + convRemoteMembers = mempty, + convDeleted = False, + convMetadata = + ConversationMetadata + { cnvmType = GlobalTeamConv, + -- FUTUREWORK: Make this a qualified user ID. + cnvmCreator = usr, + cnvmAccess = [SelfInviteAccess], + cnvmAccessRoles = mempty, + cnvmName = Just $ gtcName gtc, + cnvmTeam = Just $ gtcTeam gtc, + cnvmMessageTimer = Nothing, + cnvmReceiptMode = Nothing + }, + convProtocol = ProtocolMLS mlsData + } getPendingBackendRemoveProposals :: Members '[ProposalStore, TinyLog] r => diff --git a/services/galley/src/Galley/API/Query.hs b/services/galley/src/Galley/API/Query.hs index 4643c462fdb..2529b0a30b0 100644 --- a/services/galley/src/Galley/API/Query.hs +++ b/services/galley/src/Galley/API/Query.hs @@ -127,7 +127,11 @@ getBotConversation :: Local ConvId -> Sem r Public.BotConvView getBotConversation zbot lcnv = do - (c, _) <- getConversationAndMemberWithError @'ConvNotFound (botUserId zbot) lcnv + (c, _) <- + getConversationAndMemberWithError + @'ConvNotFound + (qUntagged . qualifyAs lcnv . botUserId $ zbot) + lcnv let domain = tDomain lcnv cmems = mapMaybe (mkMember domain) (toList (Data.convLocalMembers c)) pure $ Public.botConvView (tUnqualified lcnv) (Data.convName c) cmems diff --git a/services/galley/src/Galley/API/Util.hs b/services/galley/src/Galley/API/Util.hs index 47e1a9182e9..a9b843913af 100644 --- a/services/galley/src/Galley/API/Util.hs +++ b/services/galley/src/Galley/API/Util.hs @@ -76,6 +76,7 @@ import Wire.API.Event.Conversation import Wire.API.Federation.API import Wire.API.Federation.API.Galley import Wire.API.Federation.Error +import Wire.API.MLS.GlobalTeamConversation import Wire.API.Routes.Public.Galley.Conversation import Wire.API.Routes.Public.Util import Wire.API.Team.Member @@ -508,7 +509,7 @@ getConversationAndCheckMembership uid lcnv = do (conv, _) <- getConversationAndMemberWithError @'ConvAccessDenied - uid + (qUntagged $ qualifyAs lcnv uid) lcnv pure conv @@ -517,18 +518,49 @@ getConversationWithError :: Member (ErrorS 'ConvNotFound) r ) => Local ConvId -> + UserId -> Sem r Data.Conversation -getConversationWithError lcnv = - getConversation (tUnqualified lcnv) >>= noteS @'ConvNotFound +getConversationWithError lcnv uid = + let cid = tUnqualified lcnv + in getConversation cid >>= \case + Just c -> pure c + Nothing -> do + gtc <- noteS @'ConvNotFound =<< getGlobalTeamConversationById lcnv + pure $ toConv uid mempty gtc + where + toConv usr lm gtc = + let mlsData = gtcMlsMetadata gtc + in Data.Conversation + { convId = qUnqualified $ gtcId gtc, + convLocalMembers = lm, + convRemoteMembers = mempty, + convDeleted = False, + convMetadata = + ConversationMetadata + { cnvmType = GlobalTeamConv, + -- FUTUREWORK: Make this a qualified user ID. + cnvmCreator = usr, + cnvmAccess = [SelfInviteAccess], + cnvmAccessRoles = mempty, + cnvmName = Just $ gtcName gtc, + cnvmTeam = Just $ gtcTeam gtc, + cnvmMessageTimer = Nothing, + cnvmReceiptMode = Nothing + }, + convProtocol = ProtocolMLS mlsData + } getConversationAndMemberWithError :: forall e uid mem r. - (Members '[ConversationStore, ErrorS 'ConvNotFound, ErrorS e] r, IsConvMemberId uid mem) => + ( Members '[ConversationStore, ErrorS 'ConvNotFound, ErrorS e] r, + IsConvMemberId uid mem, + uid ~ Qualified UserId + ) => uid -> Local ConvId -> Sem r (Data.Conversation, mem) getConversationAndMemberWithError usr lcnv = do - c <- getConversationWithError lcnv + c <- getConversationWithError lcnv (qUnqualified usr) member <- noteS @e $ getConvMember lcnv c usr pure (c, member) diff --git a/services/galley/src/Galley/Cassandra/Conversation.hs b/services/galley/src/Galley/Cassandra/Conversation.hs index d02ecbc473d..de72cc1264f 100644 --- a/services/galley/src/Galley/Cassandra/Conversation.hs +++ b/services/galley/src/Galley/Cassandra/Conversation.hs @@ -316,25 +316,6 @@ createGlobalTeamConversation tid = do "Global team conversation" (tUnqualified tid) -setGlobalTeamConversationCreator :: - GlobalTeamConversation -> - UserId -> - Client () -setGlobalTeamConversationCreator gtc uid = do - retry x5 . batch $ do - setType BatchLogged - setConsistency LocalQuorum - addPrepQuery - Cql.setGlobalTeamConvCreator - ( uid, - qUnqualified . gtcId $ gtc - ) - addPrepQuery - Cql.insertUserConv - ( uid, - qUnqualified . gtcId $ gtc - ) - -- | "Garbage collect" a 'Conversation', i.e. if the conversation is -- marked as deleted, actually remove it from the database and return -- 'Nothing'. @@ -470,7 +451,6 @@ interpretConversationStoreToCassandra = interpret $ \case GetGlobalTeamConversation tid -> embedClient $ getGlobalTeamConversation tid GetGlobalTeamConversationById lconv -> embedClient $ getGlobalTeamConversationById lconv CreateGlobalTeamConversation tid -> embedClient $ createGlobalTeamConversation tid - SetGlobalTeamConversationCreator gtc uid -> embedClient $ setGlobalTeamConversationCreator gtc uid GetConversationIdByGroupId gId -> embedClient $ lookupGroupId gId GetConversations cids -> localConversations cids GetConversationMetadata cid -> embedClient $ conversationMeta cid diff --git a/services/galley/src/Galley/Effects/ConversationStore.hs b/services/galley/src/Galley/Effects/ConversationStore.hs index 6c6ac31b08a..f1d9f374951 100644 --- a/services/galley/src/Galley/Effects/ConversationStore.hs +++ b/services/galley/src/Galley/Effects/ConversationStore.hs @@ -46,7 +46,6 @@ module Galley.Effects.ConversationStore setConversationReceiptMode, setConversationMessageTimer, setConversationEpoch, - setGlobalTeamConversationCreator, acceptConnectConversation, setGroupId, setPublicGroupState, @@ -86,7 +85,6 @@ data ConversationStore m a where GetGlobalTeamConversation :: Local TeamId -> ConversationStore m (Maybe GlobalTeamConversation) GetGlobalTeamConversationById :: Local ConvId -> ConversationStore m (Maybe GlobalTeamConversation) CreateGlobalTeamConversation :: Local TeamId -> ConversationStore m GlobalTeamConversation - SetGlobalTeamConversationCreator :: GlobalTeamConversation -> UserId -> ConversationStore m () GetConversationIdByGroupId :: GroupId -> ConversationStore m (Maybe (Qualified ConvId)) GetConversations :: [ConvId] -> ConversationStore m [Conversation] GetConversationMetadata :: ConvId -> ConversationStore m (Maybe ConversationMetadata) diff --git a/services/galley/test/integration/API/MLS.hs b/services/galley/test/integration/API/MLS.hs index 37d95a1866b..0606754ee19 100644 --- a/services/galley/test/integration/API/MLS.hs +++ b/services/galley/test/integration/API/MLS.hs @@ -201,7 +201,8 @@ tests s = test s "Global team conversation is created on get if not present" (testGetGlobalTeamConv s), test s "Can't leave global team conversation" testGlobalTeamConversationLeave, test s "Send message in global team conversation" testGlobalTeamConversationMessage, - test s "Listing convs includes global team conversation" testConvListIncludesGlobal + test s "Listing convs includes global team conversation" testConvListIncludesGlobal, + test s "Listing convs includes global team conversation for new users" testConvListIncludesGlobalForNewUsers ], testGroup "Self conversation" @@ -2246,6 +2247,30 @@ testConvListIncludesGlobal = do const 200 === statusCode const (Just [globalTeamConv tid]) =~= (rightToMaybe . (<$$>) qUnqualified . decodeQualifiedConvIdList) +testConvListIncludesGlobalForNewUsers :: TestM () +testConvListIncludesGlobalForNewUsers = do + localDomain <- viewFederationDomain + -- c <- view tsCannon + (tid, alice, [bob]) <- Util.createBindingTeamWithMembers 2 + let aliceQ = Qualified alice localDomain + bobQ = Qualified bob localDomain + + runMLSTest $ do + [alice1, bob1] <- traverse createMLSClient [aliceQ, bobQ] + void $ uploadNewKeyPackage bob1 + + void $ setupMLSGroup alice1 + void $ createAddCommit alice1 [bobQ] >>= sendAndConsumeCommitBundle + + let paginationOpts = GetPaginatedConversationIds Nothing (toRange (Proxy @5)) + listConvIds alice paginationOpts !!! do + const 200 === statusCode + const (Just [globalTeamConv tid]) =/~= (rightToMaybe . (<$$>) qUnqualified . decodeQualifiedConvIdList) + + listConvIds bob paginationOpts !!! do + const 200 === statusCode + const (Just [globalTeamConv tid]) =/~= (rightToMaybe . (<$$>) qUnqualified . decodeQualifiedConvIdList) + rightToMaybe :: Either a b -> Maybe b rightToMaybe = either (const Nothing) Just From efb0439d74be1668259891fcaf6fcca35685834a Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Wed, 23 Nov 2022 13:20:43 +0000 Subject: [PATCH 2/7] Re-use function instead of redefining. --- services/galley/test/integration/API/MLS.hs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/services/galley/test/integration/API/MLS.hs b/services/galley/test/integration/API/MLS.hs index 0606754ee19..85054fc9ad6 100644 --- a/services/galley/test/integration/API/MLS.hs +++ b/services/galley/test/integration/API/MLS.hs @@ -25,6 +25,7 @@ import API.Util as Util import Bilge hiding (head) import Bilge.Assert import Cassandra +import Control.Error.Util (hush) import Control.Lens (view, (^.)) import qualified Control.Monad.State as State import Crypto.Error @@ -2225,7 +2226,7 @@ testConvListIncludesGlobal = do let paginationOpts = GetPaginatedConversationIds Nothing (toRange (Proxy @5)) listConvIds alice paginationOpts !!! do const 200 === statusCode - const (Just [globalTeamConv tid]) =/~= (rightToMaybe . (<$$>) qUnqualified . decodeQualifiedConvIdList) + const (Just [globalTeamConv tid]) =/~= (hush . (<$$>) qUnqualified . decodeQualifiedConvIdList) -- add user to conv runMLSTest $ do @@ -2245,7 +2246,7 @@ testConvListIncludesGlobal = do -- Now we should have the user as part of that conversation also in the backend listConvIds alice paginationOpts !!! do const 200 === statusCode - const (Just [globalTeamConv tid]) =~= (rightToMaybe . (<$$>) qUnqualified . decodeQualifiedConvIdList) + const (Just [globalTeamConv tid]) =~= (hush . (<$$>) qUnqualified . decodeQualifiedConvIdList) testConvListIncludesGlobalForNewUsers :: TestM () testConvListIncludesGlobalForNewUsers = do @@ -2265,14 +2266,11 @@ testConvListIncludesGlobalForNewUsers = do let paginationOpts = GetPaginatedConversationIds Nothing (toRange (Proxy @5)) listConvIds alice paginationOpts !!! do const 200 === statusCode - const (Just [globalTeamConv tid]) =/~= (rightToMaybe . (<$$>) qUnqualified . decodeQualifiedConvIdList) + const (Just [globalTeamConv tid]) =/~= (hush . (<$$>) qUnqualified . decodeQualifiedConvIdList) listConvIds bob paginationOpts !!! do const 200 === statusCode - const (Just [globalTeamConv tid]) =/~= (rightToMaybe . (<$$>) qUnqualified . decodeQualifiedConvIdList) - -rightToMaybe :: Either a b -> Maybe b -rightToMaybe = either (const Nothing) Just + const (Just [globalTeamConv tid]) =/~= (hush . (<$$>) qUnqualified . decodeQualifiedConvIdList) testGlobalTeamConversationMessage :: TestM () testGlobalTeamConversationMessage = do From 77e60db365d732df581196d6a74f2e6601b0fe50 Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Thu, 24 Nov 2022 09:12:44 +0000 Subject: [PATCH 3/7] Re-extracted helper conversation function --- services/galley/src/Galley/API/MLS/Util.hs | 52 ++++++++++++---------- services/galley/src/Galley/API/Util.hs | 26 +---------- 2 files changed, 31 insertions(+), 47 deletions(-) diff --git a/services/galley/src/Galley/API/MLS/Util.hs b/services/galley/src/Galley/API/MLS/Util.hs index 9ea00c340e6..5b45bac8191 100644 --- a/services/galley/src/Galley/API/MLS/Util.hs +++ b/services/galley/src/Galley/API/MLS/Util.hs @@ -26,6 +26,7 @@ import Galley.Effects import Galley.Effects.ConversationStore import Galley.Effects.MemberStore import Galley.Effects.ProposalStore +import Galley.Types.Conversations.Members import Imports import Polysemy import Polysemy.TinyLog (TinyLog) @@ -55,7 +56,7 @@ getLocalConvForUser qusr lcnv = do conv <- case gtc of Just conv -> do localMembers <- getLocalMembers (qUnqualified . gtcId $ conv) - pure $ toConv conv (qUnqualified qusr) localMembers + pure $ gtcToConv conv (qUnqualified qusr) localMembers Nothing -> do getConversation (tUnqualified lcnv) >>= noteS @'ConvNotFound @@ -69,28 +70,6 @@ getLocalConvForUser qusr lcnv = do unless isMember' $ throwS @'ConvNotFound pure conv - where - toConv gtc usr lm = - let mlsData = gtcMlsMetadata gtc - in Data.Conversation - { convId = qUnqualified $ gtcId gtc, - convLocalMembers = lm, - convRemoteMembers = mempty, - convDeleted = False, - convMetadata = - ConversationMetadata - { cnvmType = GlobalTeamConv, - -- FUTUREWORK: Make this a qualified user ID. - cnvmCreator = usr, - cnvmAccess = [SelfInviteAccess], - cnvmAccessRoles = mempty, - cnvmName = Just $ gtcName gtc, - cnvmTeam = Just $ gtcTeam gtc, - cnvmMessageTimer = Nothing, - cnvmReceiptMode = Nothing - }, - convProtocol = ProtocolMLS mlsData - } getPendingBackendRemoveProposals :: Members '[ProposalStore, TinyLog] r => @@ -111,3 +90,30 @@ getPendingBackendRemoveProposals gid epoch = do TinyLog.warn $ Log.msg ("found pending proposal without origin, ignoring" :: ByteString) pure Nothing ) + +gtcToConv :: + GlobalTeamConversation -> + UserId -> + [LocalMember] -> + Conversation +gtcToConv gtc usr lm = + let mlsData = gtcMlsMetadata gtc + in Conversation + { convId = qUnqualified $ gtcId gtc, + convLocalMembers = lm, + convRemoteMembers = mempty, + convDeleted = False, + convMetadata = + ConversationMetadata + { cnvmType = GlobalTeamConv, + -- FUTUREWORK: Make this a qualified user ID. + cnvmCreator = usr, + cnvmAccess = [SelfInviteAccess], + cnvmAccessRoles = mempty, + cnvmName = Just $ gtcName gtc, + cnvmTeam = Just $ gtcTeam gtc, + cnvmMessageTimer = Nothing, + cnvmReceiptMode = Nothing + }, + convProtocol = ProtocolMLS mlsData + } diff --git a/services/galley/src/Galley/API/Util.hs b/services/galley/src/Galley/API/Util.hs index a9b843913af..4234d409e54 100644 --- a/services/galley/src/Galley/API/Util.hs +++ b/services/galley/src/Galley/API/Util.hs @@ -36,6 +36,7 @@ import Data.Singletons import qualified Data.Text as T import Data.Time import Galley.API.Error +import Galley.API.MLS.Util import Galley.API.Mapping import qualified Galley.Data.Conversation as Data import Galley.Data.Services (BotMember, newBotMember) @@ -76,7 +77,6 @@ import Wire.API.Event.Conversation import Wire.API.Federation.API import Wire.API.Federation.API.Galley import Wire.API.Federation.Error -import Wire.API.MLS.GlobalTeamConversation import Wire.API.Routes.Public.Galley.Conversation import Wire.API.Routes.Public.Util import Wire.API.Team.Member @@ -526,29 +526,7 @@ getConversationWithError lcnv uid = Just c -> pure c Nothing -> do gtc <- noteS @'ConvNotFound =<< getGlobalTeamConversationById lcnv - pure $ toConv uid mempty gtc - where - toConv usr lm gtc = - let mlsData = gtcMlsMetadata gtc - in Data.Conversation - { convId = qUnqualified $ gtcId gtc, - convLocalMembers = lm, - convRemoteMembers = mempty, - convDeleted = False, - convMetadata = - ConversationMetadata - { cnvmType = GlobalTeamConv, - -- FUTUREWORK: Make this a qualified user ID. - cnvmCreator = usr, - cnvmAccess = [SelfInviteAccess], - cnvmAccessRoles = mempty, - cnvmName = Just $ gtcName gtc, - cnvmTeam = Just $ gtcTeam gtc, - cnvmMessageTimer = Nothing, - cnvmReceiptMode = Nothing - }, - convProtocol = ProtocolMLS mlsData - } + pure $ gtcToConv gtc uid mempty getConversationAndMemberWithError :: forall e uid mem r. From c7747e90eaca931defddb0025f3510b6e2209a46 Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Thu, 24 Nov 2022 10:59:49 +0000 Subject: [PATCH 4/7] Preemptively create GTC on listing Ids --- services/galley/src/Galley/API/Query.hs | 15 ++++++++++++++- services/galley/test/integration/API/MLS.hs | 11 ++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/services/galley/src/Galley/API/Query.hs b/services/galley/src/Galley/API/Query.hs index 2529b0a30b0..231e75ae1ad 100644 --- a/services/galley/src/Galley/API/Query.hs +++ b/services/galley/src/Galley/API/Query.hs @@ -68,6 +68,7 @@ import Data.Proxy import Data.Qualified import Data.Range import qualified Data.Set as Set +import Data.Tagged import Galley.API.Error import Galley.API.MLS.Keys import Galley.API.Mapping @@ -318,12 +319,24 @@ getConversationRoles lusr cnv = do pure $ Public.ConversationRolesList wireConvRoles conversationIdsPageFromUnqualified :: - Member (ListItems LegacyPaging ConvId) r => + Members + [ ListItems LegacyPaging ConvId, + ConversationStore, + MemberStore, + TeamStore + ] + r => Local UserId -> Maybe ConvId -> Maybe (Range 1 1000 Int32) -> Sem r (Public.ConversationList ConvId) conversationIdsPageFromUnqualified lusr start msize = do + void $ + E.getUserTeams (tUnqualified lusr) >>= \tids -> + runError @InternalError $ + runError @(Tagged 'NotATeamMember ()) + (for_ tids $ \tid -> getGlobalTeamConversation lusr tid) + let size = fromMaybe (toRange (Proxy @1000)) msize ids <- E.listItems (tUnqualified lusr) start size pure $ diff --git a/services/galley/test/integration/API/MLS.hs b/services/galley/test/integration/API/MLS.hs index 85054fc9ad6..569d6cf1f5c 100644 --- a/services/galley/test/integration/API/MLS.hs +++ b/services/galley/test/integration/API/MLS.hs @@ -203,7 +203,8 @@ tests s = test s "Can't leave global team conversation" testGlobalTeamConversationLeave, test s "Send message in global team conversation" testGlobalTeamConversationMessage, test s "Listing convs includes global team conversation" testConvListIncludesGlobal, - test s "Listing convs includes global team conversation for new users" testConvListIncludesGlobalForNewUsers + test s "Listing convs includes global team conversation for new users" testConvListIncludesGlobalForNewUsers, + test s "Listing convs before calling GET on global team conversation still includes it" testConvListIncludesGlobalBeforeGet ], testGroup "Self conversation" @@ -2248,6 +2249,14 @@ testConvListIncludesGlobal = do const 200 === statusCode const (Just [globalTeamConv tid]) =~= (hush . (<$$>) qUnqualified . decodeQualifiedConvIdList) +testConvListIncludesGlobalBeforeGet :: TestM () +testConvListIncludesGlobalBeforeGet = do + (tid, alice, []) <- Util.createBindingTeamWithMembers 1 + let paginationOpts = GetPaginatedConversationIds Nothing (toRange (Proxy @5)) + listConvIds alice paginationOpts !!! do + const 200 === statusCode + const (Just [globalTeamConv tid]) =/~= (hush . (<$$>) qUnqualified . decodeQualifiedConvIdList) + testConvListIncludesGlobalForNewUsers :: TestM () testConvListIncludesGlobalForNewUsers = do localDomain <- viewFederationDomain From 15eb936ed48688a8f15717856d68fdaab034aa37 Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Thu, 24 Nov 2022 13:09:28 +0000 Subject: [PATCH 5/7] Fixed typo --- services/galley/src/Galley/API/MLS/Message.hs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index 24d6ea0c0b6..8bdb592a417 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -846,11 +846,10 @@ processInternalCommit qusr senderClient con lconv mlsMeta cm epoch action sender qusr (Set.singleton (creatorClient, creatorRef)) (Left _, SelfConv, _) -> + -- this is a newly created conversation, and it should contain exactly one + -- client (the creator) throw . InternalErrorWithDescription $ "Unexpected creator client set in a self-conversation" - -- this is a newly created conversation, and it should contain exactly one - -- client (the creator) - (Left _, GlobalTeamConv, []) -> do creatorClient <- noteS @'MLSMissingSenderClient senderClient creatorRef <- @@ -870,7 +869,7 @@ processInternalCommit qusr senderClient con lconv mlsMeta cm epoch action sender (Set.singleton (creatorClient, creatorRef)) (Left _, GlobalTeamConv, _) -> throw . InternalErrorWithDescription $ - "Unexpected creator client set in a global teamconversation" + "Unexpected creator client set in a global team conversation" (Left lm, _, [(qu, (creatorClient, _))]) | qu == qUntagged (qualifyAs lconv (lmId lm)) -> do -- use update path as sender reference and if not existing fall back to sender From 81fb3f8f8f52cb46e8332f36e278a4825fc71a16 Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Thu, 24 Nov 2022 15:23:06 +0000 Subject: [PATCH 6/7] Improved error type for unexpected sender client on creation for Self/GTC --- libs/wire-api/src/Wire/API/Error/Galley.hs | 3 + .../src/Wire/API/Routes/Public/Galley/MLS.hs | 3 + services/galley/src/Galley/API/MLS/Message.hs | 127 ++++++++++-------- 3 files changed, 77 insertions(+), 56 deletions(-) diff --git a/libs/wire-api/src/Wire/API/Error/Galley.hs b/libs/wire-api/src/Wire/API/Error/Galley.hs index c53fa996b60..d714737c03c 100644 --- a/libs/wire-api/src/Wire/API/Error/Galley.hs +++ b/libs/wire-api/src/Wire/API/Error/Galley.hs @@ -84,6 +84,7 @@ data GalleyError | MLSWelcomeMismatch | MLSMissingGroupInfo | MLSMissingSenderClient + | MLSUnexpectedSenderClient | -- NoBindingTeamMembers | NoBindingTeam @@ -203,6 +204,8 @@ type instance MapError 'MLSGroupConversationMismatch = 'StaticError 400 "mls-gro type instance MapError 'MLSClientSenderUserMismatch = 'StaticError 400 "mls-client-sender-user-mismatch" "User ID resolved from Client ID does not match message's sender user ID" +type instance MapError 'MLSUnexpectedSenderClient = 'StaticError 422 "mls-unexpected-sender-client-found" "Unexpected creator client set. This is a newly created conversation and it expected exactly one client." + type instance MapError 'MLSWelcomeMismatch = 'StaticError 400 "mls-welcome-mismatch" "The list of targets of a welcome message does not match the list of new clients in a group" type instance MapError 'MLSMissingGroupInfo = 'StaticError 404 "mls-missing-group-info" "The conversation has no group information" diff --git a/libs/wire-api/src/Wire/API/Routes/Public/Galley/MLS.hs b/libs/wire-api/src/Wire/API/Routes/Public/Galley/MLS.hs index 6b3666208eb..03421544b04 100644 --- a/libs/wire-api/src/Wire/API/Routes/Public/Galley/MLS.hs +++ b/libs/wire-api/src/Wire/API/Routes/Public/Galley/MLS.hs @@ -60,6 +60,7 @@ type MLSMessagingAPI = :> CanThrow 'MLSStaleMessage :> CanThrow 'MLSUnsupportedMessage :> CanThrow 'MLSUnsupportedProposal + :> CanThrow 'MLSUnexpectedSenderClient :> CanThrow 'MLSClientSenderUserMismatch :> CanThrow 'MLSGroupConversationMismatch :> CanThrow 'MLSMissingSenderClient @@ -88,6 +89,7 @@ type MLSMessagingAPI = :> CanThrow 'MLSStaleMessage :> CanThrow 'MLSUnsupportedMessage :> CanThrow 'MLSUnsupportedProposal + :> CanThrow 'MLSUnexpectedSenderClient :> CanThrow 'MLSClientSenderUserMismatch :> CanThrow 'MLSGroupConversationMismatch :> CanThrow 'MLSMissingSenderClient @@ -116,6 +118,7 @@ type MLSMessagingAPI = :> CanThrow 'MLSStaleMessage :> CanThrow 'MLSUnsupportedMessage :> CanThrow 'MLSUnsupportedProposal + :> CanThrow 'MLSUnexpectedSenderClient :> CanThrow 'MLSClientSenderUserMismatch :> CanThrow 'MLSGroupConversationMismatch :> CanThrow 'MLSMissingSenderClient diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index 8bdb592a417..7080b94941f 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -105,6 +105,7 @@ type MLSMessageStaticErrors = ErrorS 'MLSCommitMissingReferences, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSClientSenderUserMismatch, + ErrorS 'MLSUnexpectedSenderClient, ErrorS 'MLSGroupConversationMismatch, ErrorS 'MLSMissingSenderClient ] @@ -122,7 +123,6 @@ postMLSMessageFromLocalUserV1 :: ErrorS 'ConvAccessDenied, ErrorS 'ConvMemberNotFound, ErrorS 'ConvNotFound, - ErrorS 'MissingLegalholdConsent, ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, ErrorS 'MLSGroupConversationMismatch, @@ -130,7 +130,9 @@ postMLSMessageFromLocalUserV1 :: ErrorS 'MLSProposalNotFound, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, + ErrorS 'MLSUnexpectedSenderClient, ErrorS 'MLSUnsupportedMessage, + ErrorS 'MissingLegalholdConsent, Input (Local ()), ProposalStore, Resource, @@ -157,7 +159,6 @@ postMLSMessageFromLocalUser :: ErrorS 'ConvAccessDenied, ErrorS 'ConvMemberNotFound, ErrorS 'ConvNotFound, - ErrorS 'MissingLegalholdConsent, ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, ErrorS 'MLSGroupConversationMismatch, @@ -165,7 +166,9 @@ postMLSMessageFromLocalUser :: ErrorS 'MLSProposalNotFound, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, + ErrorS 'MLSUnexpectedSenderClient, ErrorS 'MLSUnsupportedMessage, + ErrorS 'MissingLegalholdConsent, Input (Local ()), ProposalStore, Resource, @@ -367,7 +370,6 @@ postMLSMessage :: ErrorS 'ConvAccessDenied, ErrorS 'ConvMemberNotFound, ErrorS 'ConvNotFound, - ErrorS 'MissingLegalholdConsent, ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, ErrorS 'MLSGroupConversationMismatch, @@ -375,7 +377,9 @@ postMLSMessage :: ErrorS 'MLSProposalNotFound, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, + ErrorS 'MLSUnexpectedSenderClient, ErrorS 'MLSUnsupportedMessage, + ErrorS 'MissingLegalholdConsent, Input (Local ()), ProposalStore, Resource, @@ -455,14 +459,15 @@ postMLSMessageToLocalConv :: '[ Error FederationError, Error InternalError, ErrorS 'ConvNotFound, - ErrorS 'MissingLegalholdConsent, ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, ErrorS 'MLSMissingSenderClient, ErrorS 'MLSProposalNotFound, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, + ErrorS 'MLSUnexpectedSenderClient, ErrorS 'MLSUnsupportedMessage, + ErrorS 'MissingLegalholdConsent, ProposalStore, Resource, TinyLog @@ -539,26 +544,29 @@ postMLSMessageToRemoteConv loc qusr _senderClient con smsg rcnv = do pure (LocalConversationUpdate e update) type HasProposalEffects r = - ( Member BrigAccess r, - Member ConversationStore r, - Member (Error InternalError) r, - Member (Error MLSProposalFailure) r, - Member (Error MLSProtocolError) r, - Member (ErrorS 'MLSClientMismatch) r, - Member (ErrorS 'MLSKeyPackageRefNotFound) r, - Member (ErrorS 'MLSUnsupportedProposal) r, - Member ExternalAccess r, - Member FederatorAccess r, - Member GundeckAccess r, - Member (Input Env) r, - Member (Input (Local ())) r, - Member (Input Opts) r, - Member (Input UTCTime) r, - Member LegalHoldStore r, - Member MemberStore r, - Member ProposalStore r, - Member TeamStore r, - Member TinyLog r + ( Members + '[ BrigAccess, + ConversationStore, + Error InternalError, + Error MLSProposalFailure, + Error MLSProtocolError, + ErrorS 'MLSClientMismatch, + ErrorS 'MLSKeyPackageRefNotFound, + ErrorS 'MLSUnsupportedProposal, + ExternalAccess, + FederatorAccess, + GundeckAccess, + Input Env, + Input (Local ()), + Input Opts, + Input UTCTime, + LegalHoldStore, + MemberStore, + ProposalStore, + TeamStore, + TinyLog + ] + r ) data ProposalAction = ProposalAction @@ -616,20 +624,24 @@ getCommitData lconv mlsMeta epoch commit = do processCommit :: ( HasProposalEffects r, - Member (Error FederationError) r, - Member (Error InternalError) r, - Member (ErrorS 'ConvNotFound) r, - Member (ErrorS 'MLSClientSenderUserMismatch) r, - Member (ErrorS 'MLSCommitMissingReferences) r, - Member (ErrorS 'MLSMissingSenderClient) r, - Member (ErrorS 'MLSProposalNotFound) r, - Member (ErrorS 'MLSSelfRemovalNotAllowed) r, - Member (ErrorS 'MLSStaleMessage) r, - Member (ErrorS 'MissingLegalholdConsent) r, - Member (Input (Local ())) r, - Member ProposalStore r, - Member BrigAccess r, - Member Resource r + Members + '[ Error FederationError, + Error InternalError, + ErrorS 'ConvNotFound, + ErrorS 'MLSClientSenderUserMismatch, + ErrorS 'MLSCommitMissingReferences, + ErrorS 'MLSMissingSenderClient, + ErrorS 'MLSProposalNotFound, + ErrorS 'MLSSelfRemovalNotAllowed, + ErrorS 'MLSStaleMessage, + ErrorS 'MLSUnexpectedSenderClient, + ErrorS 'MissingLegalholdConsent, + Input (Local ()), + ProposalStore, + BrigAccess, + Resource + ] + r ) => Qualified UserId -> Maybe ClientId -> @@ -758,20 +770,24 @@ processExternalCommit qusr mSenderClient lconv mlsMeta cm epoch action updatePat processCommitWithAction :: forall r. ( HasProposalEffects r, - Member (Error FederationError) r, - Member (Error InternalError) r, - Member (ErrorS 'ConvNotFound) r, - Member (ErrorS 'MLSClientSenderUserMismatch) r, - Member (ErrorS 'MLSCommitMissingReferences) r, - Member (ErrorS 'MLSMissingSenderClient) r, - Member (ErrorS 'MLSProposalNotFound) r, - Member (ErrorS 'MLSSelfRemovalNotAllowed) r, - Member (ErrorS 'MLSStaleMessage) r, - Member (ErrorS 'MissingLegalholdConsent) r, - Member (Input (Local ())) r, - Member ProposalStore r, - Member BrigAccess r, - Member Resource r + Members + '[ Error FederationError, + Error InternalError, + ErrorS 'ConvNotFound, + ErrorS 'MLSClientSenderUserMismatch, + ErrorS 'MLSCommitMissingReferences, + ErrorS 'MLSMissingSenderClient, + ErrorS 'MLSProposalNotFound, + ErrorS 'MLSSelfRemovalNotAllowed, + ErrorS 'MLSStaleMessage, + ErrorS 'MLSUnexpectedSenderClient, + ErrorS 'MissingLegalholdConsent, + Input (Local ()), + ProposalStore, + BrigAccess, + Resource + ] + r ) => Qualified UserId -> Maybe ClientId -> @@ -803,6 +819,7 @@ processInternalCommit :: ErrorS 'MLSProposalNotFound, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, + ErrorS 'MLSUnexpectedSenderClient, ErrorS 'MissingLegalholdConsent, Input (Local ()), ProposalStore, @@ -848,8 +865,7 @@ processInternalCommit qusr senderClient con lconv mlsMeta cm epoch action sender (Left _, SelfConv, _) -> -- this is a newly created conversation, and it should contain exactly one -- client (the creator) - throw . InternalErrorWithDescription $ - "Unexpected creator client set in a self-conversation" + throwS @'MLSUnexpectedSenderClient (Left _, GlobalTeamConv, []) -> do creatorClient <- noteS @'MLSMissingSenderClient senderClient creatorRef <- @@ -868,8 +884,7 @@ processInternalCommit qusr senderClient con lconv mlsMeta cm epoch action sender qusr (Set.singleton (creatorClient, creatorRef)) (Left _, GlobalTeamConv, _) -> - throw . InternalErrorWithDescription $ - "Unexpected creator client set in a global team conversation" + throwS @'MLSUnexpectedSenderClient (Left lm, _, [(qu, (creatorClient, _))]) | qu == qUntagged (qualifyAs lconv (lmId lm)) -> do -- use update path as sender reference and if not existing fall back to sender From f554038f955d46c6c0e9c6b255ecd8b3bc4fe82c Mon Sep 17 00:00:00 2001 From: Igor Ranieri Date: Mon, 28 Nov 2022 11:01:25 +0000 Subject: [PATCH 7/7] Apply feedback from review. --- services/galley/src/Galley/API/MLS/Util.hs | 1 - services/galley/src/Galley/API/Query.hs | 1 - 2 files changed, 2 deletions(-) diff --git a/services/galley/src/Galley/API/MLS/Util.hs b/services/galley/src/Galley/API/MLS/Util.hs index 5b45bac8191..9738b5b0464 100644 --- a/services/galley/src/Galley/API/MLS/Util.hs +++ b/services/galley/src/Galley/API/MLS/Util.hs @@ -106,7 +106,6 @@ gtcToConv gtc usr lm = convMetadata = ConversationMetadata { cnvmType = GlobalTeamConv, - -- FUTUREWORK: Make this a qualified user ID. cnvmCreator = usr, cnvmAccess = [SelfInviteAccess], cnvmAccessRoles = mempty, diff --git a/services/galley/src/Galley/API/Query.hs b/services/galley/src/Galley/API/Query.hs index 231e75ae1ad..c8c9d4377a3 100644 --- a/services/galley/src/Galley/API/Query.hs +++ b/services/galley/src/Galley/API/Query.hs @@ -164,7 +164,6 @@ getGlobalTeamConversation :: Members '[ ConversationStore, ErrorS 'NotATeamMember, - Error InternalError, MemberStore, TeamStore ]