From 1ad7fe5a6a6f1badc536dbf790f9219b2c9e20b9 Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Thu, 14 Oct 2021 12:31:10 +0200 Subject: [PATCH 01/11] Refactor: Use pushConversationEvent --- services/galley/src/Galley/API/Teams.hs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index d0221f3206c..2b893de1d2e 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -99,7 +99,7 @@ import Galley.Intra.User import Galley.Options import qualified Galley.Options as Opts import qualified Galley.Queue as Q -import Galley.Types (UserIdList (UserIdList)) +import Galley.Types (LocalMember (lmId), UserIdList (UserIdList)) import qualified Galley.Types as Conv import Galley.Types.Conversations.Roles as Roles import Galley.Types.Teams hiding (newTeam) @@ -771,10 +771,8 @@ deleteTeamConversation zusr zcon tid cid = do flip Data.deleteCode Data.ReusableCode =<< Data.mkKey cid now <- liftIO getCurrentTime let ce = Conv.Event Conv.ConvDelete qconvId qusr now Conv.EdConvDelete - let recps = fmap recipient cmems - let convPush = newPushLocal ListComplete zusr (ConvEvent ce) recps <&> pushConn .~ Just zcon - pushSome $ maybeToList convPush - void . forkIO $ void $ External.deliver (bots `zip` repeat ce) + + pushConversationEvent (Just zcon) ce (map lmId cmems) bots -- TODO: we don't delete bots here, but we should do that, since every -- bot user can only be in a single conversation Data.removeTeamConv tid cid From ae417bcec32ed0495050ea19c3ff577d1061504f Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Thu, 14 Oct 2021 13:34:36 +0200 Subject: [PATCH 02/11] add onConversationDeleted RPC --- .../src/Wire/API/Federation/API/Galley.hs | 19 +++++++ .../Wire/API/Federation/Golden/GoldenSpec.hs | 2 + .../wire-api-federation.cabal | 3 +- services/galley/src/Galley/API/Federation.hs | 13 ++++- services/galley/test/integration/API.hs | 1 + .../galley/test/integration/API/Federation.hs | 50 ++++++++++++++++++- 6 files changed, 85 insertions(+), 3 deletions(-) diff --git a/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs b/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs index bdb02efccb3..1f4dffd910c 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs @@ -78,6 +78,13 @@ data Api routes = Api :> OriginDomainHeader :> ReqBody '[JSON] ConversationUpdate :> Post '[JSON] (), + onConversationDeleted :: + routes + :- "federation" + :> "on-conversation-deleted" + :> OriginDomainHeader + :> ReqBody '[JSON] ConversationDelete + :> Post '[JSON] (), leaveConversation :: routes :- "federation" @@ -192,6 +199,18 @@ data ConversationUpdate = ConversationUpdate deriving (Arbitrary) via (GenericUniform ConversationUpdate) deriving (ToJSON, FromJSON) via (CustomEncoded ConversationUpdate) +data ConversationDelete = ConversationDelete + { cdTime :: UTCTime, + cdOriginUserId :: Qualified UserId, + -- | The unqualified converation which is owned by the requesting domain. + cdConvId :: ConvId, + -- | Local members of the conversation. + cdMembers :: [UserId] + } + deriving stock (Eq, Show, Generic) + deriving (Arbitrary) via (GenericUniform ConversationDelete) + deriving (ToJSON, FromJSON) via (CustomEncoded ConversationDelete) + data LeaveConversationRequest = LeaveConversationRequest { -- | The conversation is assumed to be owned by the target domain, which -- allows us to protect against relay attacks diff --git a/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/GoldenSpec.hs b/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/GoldenSpec.hs index 8142d17ae17..6842ff72f51 100644 --- a/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/GoldenSpec.hs +++ b/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/GoldenSpec.hs @@ -19,6 +19,7 @@ module Test.Wire.API.Federation.Golden.GoldenSpec where import Imports import Test.Hspec +import qualified Test.Wire.API.Federation.Golden.ConversationDelete as ConversationDelete import qualified Test.Wire.API.Federation.Golden.ConversationUpdate as ConversationUpdate import qualified Test.Wire.API.Federation.Golden.LeaveConversationRequest as LeaveConversationRequest import qualified Test.Wire.API.Federation.Golden.LeaveConversationResponse as LeaveConversationResponse @@ -62,3 +63,4 @@ spec = (NewConnectionResponse.testObject_NewConnectionResponse3, "testObject_NewConnectionResponse3.json"), (NewConnectionResponse.testObject_NewConnectionResponse4, "testObject_NewConnectionResponse4.json") ] + testObjects [(ConversationDelete.testObject_ConversationDelete1, "testObject_ConversationDelete1")] diff --git a/libs/wire-api-federation/wire-api-federation.cabal b/libs/wire-api-federation/wire-api-federation.cabal index e8729651c85..61bb2e5a98a 100644 --- a/libs/wire-api-federation/wire-api-federation.cabal +++ b/libs/wire-api-federation/wire-api-federation.cabal @@ -4,7 +4,7 @@ cabal-version: 1.12 -- -- see: https://github.com/sol/hpack -- --- hash: 03f7245b036ccc38819ed5f5654dae8d96b7ec5917b2f898be3305193bc3faf5 +-- hash: b6b3446eb43105b1212eecc23dcc752f233f7e5e933e6a141fdbe464747508f6 name: wire-api-federation version: 0.1.0 @@ -77,6 +77,7 @@ test-suite spec other-modules: Test.Wire.API.Federation.API.BrigSpec Test.Wire.API.Federation.ClientSpec + Test.Wire.API.Federation.Golden.ConversationDelete Test.Wire.API.Federation.Golden.ConversationUpdate Test.Wire.API.Federation.Golden.GoldenSpec Test.Wire.API.Federation.Golden.LeaveConversationRequest diff --git a/services/galley/src/Galley/API/Federation.hs b/services/galley/src/Galley/API/Federation.hs index 359f301ba11..efd3d5754b8 100644 --- a/services/galley/src/Galley/API/Federation.hs +++ b/services/galley/src/Galley/API/Federation.hs @@ -50,8 +50,10 @@ import Wire.API.Conversation.Action import Wire.API.Conversation.Member (OtherMember (..)) import qualified Wire.API.Conversation.Role as Public import Wire.API.Event.Conversation +import qualified Wire.API.Event.Conversation as Conv import Wire.API.Federation.API.Galley - ( ConversationUpdate (..), + ( ConversationDelete (..), + ConversationUpdate (..), GetConversationsRequest (..), GetConversationsResponse (..), LeaveConversationRequest (..), @@ -74,6 +76,7 @@ federationSitemap = { FederationAPIGalley.onConversationCreated = onConversationCreated, FederationAPIGalley.getConversations = getConversations, FederationAPIGalley.onConversationUpdated = onConversationUpdated, + FederationAPIGalley.onConversationDeleted = onConversationDeleted, FederationAPIGalley.leaveConversation = leaveConversation, FederationAPIGalley.onMessageSent = onMessageSent, FederationAPIGalley.sendMessage = sendMessage @@ -195,6 +198,14 @@ addLocalUsersToRemoteConv remoteConvId qAdder localUsers = do Data.addLocalMembersToRemoteConv (qUntagged remoteConvId) connectedList pure connected +onConversationDeleted :: Domain -> ConversationDelete -> Galley () +onConversationDeleted requestingDomain convDelete = do + let qconvId = Qualified (cdConvId convDelete) requestingDomain + let event = Conv.Event Conv.ConvDelete qconvId (cdOriginUserId convDelete) (cdTime convDelete) Conv.EdConvDelete + let bots = [] + pushConversationEvent Nothing event (cdMembers convDelete) bots + Data.removeLocalMembersFromRemoteConv qconvId (cdMembers convDelete) + -- FUTUREWORK: actually return errors as part of the response instead of throwing leaveConversation :: Domain -> diff --git a/services/galley/test/integration/API.hs b/services/galley/test/integration/API.hs index 4ea430c9b94..42d0afdbd22 100644 --- a/services/galley/test/integration/API.hs +++ b/services/galley/test/integration/API.hs @@ -247,6 +247,7 @@ emptyFederatedGalley = { FederatedGalley.onConversationCreated = \_ _ -> e "onConversationCreated", FederatedGalley.getConversations = \_ _ -> e "getConversations", FederatedGalley.onConversationUpdated = \_ _ -> e "onConversationUpdated", + FederatedGalley.onConversationDeleted = \_ _ -> e "onConversationDeleted", FederatedGalley.leaveConversation = \_ _ -> e "leaveConversation", FederatedGalley.onMessageSent = \_ _ -> e "onMessageSent", FederatedGalley.sendMessage = \_ _ -> e "sendMessage" diff --git a/services/galley/test/integration/API/Federation.hs b/services/galley/test/integration/API/Federation.hs index f114ee2ac83..f90faa376bc 100644 --- a/services/galley/test/integration/API/Federation.hs +++ b/services/galley/test/integration/API/Federation.hs @@ -51,7 +51,7 @@ import TestSetup import Wire.API.Conversation.Action (ConversationAction (..)) import Wire.API.Conversation.Member (Member (..)) import Wire.API.Conversation.Role -import Wire.API.Federation.API.Galley (GetConversationsRequest (..), GetConversationsResponse (..), RemoteConvMembers (..), RemoteConversation (..)) +import Wire.API.Federation.API.Galley (ConversationDelete (..), GetConversationsRequest (..), GetConversationsResponse (..), RemoteConvMembers (..), RemoteConversation (..)) import qualified Wire.API.Federation.API.Galley as FedGalley import qualified Wire.API.Federation.GRPC.Types as F import Wire.API.Message (ClientMismatchStrategy (..), MessageSendingStatus (mssDeletedClients, mssFailedToSend, mssRedundantClients), mkQualifiedOtrPayload, mssMissingClients) @@ -75,6 +75,7 @@ tests s = test s "POST /federation/on-conversation-updated : Notify local user about member update" notifyMemberUpdate, test s "POST /federation/on-conversation-updated : Notify local user about receipt mode update" notifyReceiptMode, test s "POST /federation/on-conversation-updated : Notify local user about access update" notifyAccess, + test s "POST /federation/on-conversation-deleted : Notify local users about a deleted conversation" notifyDeletedConversation, test s "POST /federation/leave-conversation : Success" leaveConversationSuccess, test s "POST /federation/on-message-sent : Receive a message from another backend" onMessageSent, test s "POST /federation/send-message : Post a message sent from another backend" sendMessage @@ -490,6 +491,53 @@ notifyMemberUpdate = do MemberStateUpdate (EdMemberUpdate d) +notifyDeletedConversation :: TestM () +notifyDeletedConversation = do + c <- view tsCannon + + qalice <- randomQualifiedUser + let alice = qUnqualified qalice + + bob <- randomId + conv <- randomId + let bobDomain = Domain "bob.example.com" + qbob = Qualified bob bobDomain + qconv = Qualified conv bobDomain + mkMember quid = OtherMember quid Nothing roleNameWireMember + + mapM_ (`connectWithRemoteUser` qbob) [alice] + registerRemoteConv + qconv + qbob + (Just "gossip") + (Set.fromList (map mkMember [qalice])) + + fedGalleyClient <- view tsFedGalleyClient + + do + aliceConvs <- listRemoteConvs bobDomain alice + liftIO $ aliceConvs @?= [qconv] + + WS.bracketR c alice $ \wsAlice -> do + now <- liftIO getCurrentTime + let convDelete = + ConversationDelete + { cdTime = now, + cdOriginUserId = qbob, + cdConvId = conv, + cdMembers = [alice] + } + FedGalley.onConversationDeleted fedGalleyClient bobDomain convDelete + + liftIO $ do + WS.assertMatch_ (5 # Second) wsAlice $ \n -> do + let e = List1.head (WS.unpackPayload n) + ConvDelete @=? evtType e + + do + aliceConvs <- listRemoteConvs bobDomain alice + liftIO $ aliceConvs @?= [] + -- TODO: test adding non-existing users -- TODO: test adding resulting in an empty notification From f907c5af4a9575cbdbf1e6f5ca94455bd36a04df Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Thu, 14 Oct 2021 13:34:47 +0200 Subject: [PATCH 03/11] deleteTeamConversation: rpc onConversationDeleted --- .../src/Wire/API/Federation/API/Galley.hs | 7 ++- .../Federation/Golden/ConversationDelete.hs | 41 ++++++++++++++ .../golden/testObject_ConversationDelete1 | 12 +++++ services/galley/src/Galley/API/Teams.hs | 30 +++++++++-- services/galley/test/integration/API.hs | 53 ++++++++++++++++++- services/galley/test/integration/API/Teams.hs | 23 +++----- services/galley/test/integration/API/Util.hs | 10 ++++ 7 files changed, 153 insertions(+), 23 deletions(-) create mode 100644 libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/ConversationDelete.hs create mode 100644 libs/wire-api-federation/test/golden/testObject_ConversationDelete1 diff --git a/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs b/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs index 1f4dffd910c..97b2ea50d89 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs @@ -199,12 +199,15 @@ data ConversationUpdate = ConversationUpdate deriving (Arbitrary) via (GenericUniform ConversationUpdate) deriving (ToJSON, FromJSON) via (CustomEncoded ConversationUpdate) +-- | Notifies the target domain that a conversation owned by the origin domain +-- has been deleted. data ConversationDelete = ConversationDelete { cdTime :: UTCTime, + -- | User that deleted the conversation. cdOriginUserId :: Qualified UserId, - -- | The unqualified converation which is owned by the requesting domain. + -- | Conversation that is being deleted. It is owned by the origin domain. cdConvId :: ConvId, - -- | Local members of the conversation. + -- | All members of the conversation that are owned by target domain. cdMembers :: [UserId] } deriving stock (Eq, Show, Generic) diff --git a/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/ConversationDelete.hs b/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/ConversationDelete.hs new file mode 100644 index 00000000000..236932daf12 --- /dev/null +++ b/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/ConversationDelete.hs @@ -0,0 +1,41 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2021 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it under +-- the terms of the GNU Affero General Public License as published by the Free +-- Software Foundation, either version 3 of the License, or (at your option) any +-- later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +-- details. +-- +-- You should have received a copy of the GNU Affero General Public License along +-- with this program. If not, see . + +module Test.Wire.API.Federation.Golden.ConversationDelete where + +import Data.Domain (Domain (Domain)) +import Data.Id (Id (Id)) +import Data.Qualified (Qualified (Qualified)) +import qualified Data.UUID as UUID +import Imports +import Wire.API.Federation.API.Galley (ConversationDelete (..)) + +testObject_ConversationDelete1 :: ConversationDelete +testObject_ConversationDelete1 = + ConversationDelete + { cdTime = read "1864-04-12 12:22:43.673 UTC", + cdOriginUserId = + Qualified + (Id (fromJust (UUID.fromString "00000000-0000-0000-0000-000100000007"))) + (Domain "golden.example.com"), + cdConvId = + Id (fromJust (UUID.fromString "00000000-0000-0000-0000-000100000006")), + cdMembers = + [ Id (fromJust (UUID.fromString "00000000-0000-0000-0000-000100000008")), + Id (fromJust (UUID.fromString "00000000-0000-0000-0000-000100000009")) + ] + } diff --git a/libs/wire-api-federation/test/golden/testObject_ConversationDelete1 b/libs/wire-api-federation/test/golden/testObject_ConversationDelete1 new file mode 100644 index 00000000000..8f7554cf0f6 --- /dev/null +++ b/libs/wire-api-federation/test/golden/testObject_ConversationDelete1 @@ -0,0 +1,12 @@ +{ + "time": "1864-04-12T12:22:43.673Z", + "members": [ + "00000000-0000-0000-0000-000100000008", + "00000000-0000-0000-0000-000100000009" + ], + "origin_user_id": { + "domain": "golden.example.com", + "id": "00000000-0000-0000-0000-000100000007" + }, + "conv_id": "00000000-0000-0000-0000-000100000006" +} \ No newline at end of file diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index 2b893de1d2e..6285cc3609e 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -99,7 +99,7 @@ import Galley.Intra.User import Galley.Options import qualified Galley.Options as Opts import qualified Galley.Queue as Q -import Galley.Types (LocalMember (lmId), UserIdList (UserIdList)) +import Galley.Types (LocalMember (lmId), RemoteMember (rmId), UserIdList (UserIdList)) import qualified Galley.Types as Conv import Galley.Types.Conversations.Roles as Roles import Galley.Types.Teams hiding (newTeam) @@ -115,6 +115,8 @@ import qualified System.Logger.Class as Log import UnliftIO (mapConcurrently) import qualified Wire.API.Conversation.Role as Public import Wire.API.ErrorDescription (ConvNotFound, NotATeamMember, operationDenied) +import Wire.API.Federation.API.Galley (ConversationDelete (..)) +import qualified Wire.API.Federation.API.Galley as FederatedGalley import qualified Wire.API.Notification as Public import qualified Wire.API.Team as Public import qualified Wire.API.Team.Conversation as Public @@ -769,12 +771,34 @@ deleteTeamConversation zusr zcon tid cid = do (bots, cmems) <- localBotsAndUsers <$> Data.members cid ensureActionAllowed Roles.DeleteConversation =<< getSelfMemberFromLocalsLegacy zusr cmems flip Data.deleteCode Data.ReusableCode =<< Data.mkKey cid + now <- liftIO getCurrentTime - let ce = Conv.Event Conv.ConvDelete qconvId qusr now Conv.EdConvDelete + do + let event = Conv.Event Conv.ConvDelete qconvId qusr now Conv.EdConvDelete + pushConversationEvent (Just zcon) event (map lmId cmems) bots + + do + remoteMembers <- Data.lookupRemoteMembers cid + for_ (bucketQualified (qUntagged . rmId <$> remoteMembers)) $ + \(Qualified remoteMembersDomain domain) -> do + let convDelete = + ConversationDelete + { cdTime = now, + cdOriginUserId = qusr, + cdConvId = cid, + cdMembers = remoteMembersDomain + } + let rpc = + FederatedGalley.onConversationDeleted + FederatedGalley.clientRoutes + localDomain + convDelete + runFederatedGalley domain rpc - pushConversationEvent (Just zcon) ce (map lmId cmems) bots -- TODO: we don't delete bots here, but we should do that, since every -- bot user can only be in a single conversation + + -- TODO: also delete remote members Data.removeTeamConv tid cid getSearchVisibilityH :: UserId ::: TeamId ::: JSON -> Galley Response diff --git a/services/galley/test/integration/API.hs b/services/galley/test/integration/API.hs index 42d0afdbd22..529cf5cb668 100644 --- a/services/galley/test/integration/API.hs +++ b/services/galley/test/integration/API.hs @@ -87,12 +87,15 @@ import Wire.API.Conversation import Wire.API.Conversation.Action import qualified Wire.API.Federation.API.Brig as FederatedBrig import Wire.API.Federation.API.Galley - ( GetConversationsResponse (..), + ( Api (onConversationDeleted, onConversationUpdated), + ConversationDelete (..), + GetConversationsResponse (..), RemoteConvMembers (..), RemoteConversation (..), ) import qualified Wire.API.Federation.API.Galley as FederatedGalley import qualified Wire.API.Federation.GRPC.Types as F +import qualified Wire.API.Federation.GRPC.Types as GRPC import qualified Wire.API.Message as Message import Wire.API.Routes.MultiTablePaging import Wire.API.User.Client @@ -163,6 +166,7 @@ tests s = test s "fail to add members when not connected" postMembersFail, test s "fail to add too many members" postTooManyMembersFail, test s "add remote members" testAddRemoteMember, + test s "delete conversation with remote members" testDeleteConversationWithRemoteMembers, test s "get conversations/:domain/:cnv - local" testGetQualifiedLocalConv, test s "get conversations/:domain/:cnv - local, not found" testGetQualifiedLocalConvNotFound, test s "get conversations/:domain/:cnv - local, not participating" testGetQualifiedLocalConvNotParticipating, @@ -1904,6 +1908,53 @@ testAddRemoteMember = do toJSON [mkProfile bob (Name "bob")] | otherwise = toJSON () +testDeleteConversationWithRemoteMembers :: TestM () +testDeleteConversationWithRemoteMembers = do + (alice, tid) <- createBindingTeam + localDomain <- viewFederationDomain + let qalice = Qualified alice localDomain + + bobId <- randomId + let remoteDomain = Domain "far-away.example.com" + remoteBob = Qualified bobId remoteDomain + + convId <- decodeConvId <$> postTeamConv tid alice [] (Just "remote gossip") [] Nothing Nothing + let _qconvId = Qualified convId localDomain + + connectWithRemoteUser alice remoteBob + + let brigApi = emptyFederatedBrig + galleyApi = + emptyFederatedGalley + { onConversationUpdated = \_domain _update -> pure (), + onConversationDeleted = \_ _ -> pure () + } + + opts <- view tsGConf + (_, received) <- withTempServantMockFederator opts brigApi galleyApi localDomain remoteDomain $ do + postQualifiedMembers alice (remoteBob :| []) convId + !!! const 200 === statusCode + + deleteTeamConv tid convId alice + !!! const 200 === statusCode + + liftIO $ do + let convDeletes = mapMaybe parseFedRequest received + convDelete <- case convDeletes of + [] -> assertFailure "No ConversationDelete requests received" + [convDelete] -> pure convDelete + _ -> assertFailure "Multiple ConversationDelete requests received" + cdMembers convDelete @?= [bobId] + cdConvId convDelete @?= convId + cdOriginUserId convDelete @?= qalice + where + parseFedRequest :: FromJSON a => GRPC.FederatedRequest -> Maybe a + parseFedRequest fr = + case GRPC.request fr of + Just r -> + (decode . cs) (GRPC.body r) + Nothing -> Nothing + testGetQualifiedLocalConv :: TestM () testGetQualifiedLocalConv = do alice <- randomUser diff --git a/services/galley/test/integration/API/Teams.hs b/services/galley/test/integration/API/Teams.hs index ebc335b9fca..4529b88c4ff 100644 --- a/services/galley/test/integration/API/Teams.hs +++ b/services/galley/test/integration/API/Teams.hs @@ -1161,7 +1161,6 @@ testDeleteBindingTeam ownerHasPassword = do testDeleteTeamConv :: TestM () testDeleteTeamConv = do localDomain <- viewFederationDomain - g <- view tsGalley c <- view tsCannon owner <- Util.randomUser let p = Util.symmPermissions [DoNotUseDeprecatedDeleteConversation] @@ -1179,14 +1178,9 @@ testDeleteTeamConv = do for_ [owner, member ^. userId, extern] $ \u -> Util.assertConvMember u cid1 for_ [owner, member ^. userId] $ \u -> Util.assertConvMember u cid2 WS.bracketR3 c owner extern (member ^. userId) $ \(wsOwner, wsExtern, wsMember) -> do - delete - ( g - . paths ["teams", toByteString' tid, "conversations", toByteString' cid2] - . zUser (member ^. userId) - . zConn "conn" - ) - !!! const 200 - === statusCode + deleteTeamConv tid cid2 (member ^. userId) + !!! const 200 === statusCode + -- We no longer send duplicate conv deletion events -- i.e., as both a regular "conversation.delete" to all -- conversation members and as "team.conversation-delete" @@ -1195,14 +1189,9 @@ testDeleteTeamConv = do checkConvDeleteEvent qcid2 wsOwner checkConvDeleteEvent qcid2 wsMember WS.assertNoEvent timeout [wsOwner, wsMember] - delete - ( g - . paths ["teams", toByteString' tid, "conversations", toByteString' cid1] - . zUser (member ^. userId) - . zConn "conn" - ) - !!! const 200 - === statusCode + + deleteTeamConv tid cid1 (member ^. userId) + !!! const 200 === statusCode -- We no longer send duplicate conv deletion events -- i.e., as both a regular "conversation.delete" to all -- conversation members and as "team.conversation-delete" diff --git a/services/galley/test/integration/API/Util.hs b/services/galley/test/integration/API/Util.hs index 92871f49cf1..b5a4a011dba 100644 --- a/services/galley/test/integration/API/Util.hs +++ b/services/galley/test/integration/API/Util.hs @@ -593,6 +593,16 @@ postTeamConv tid u us name a r mtimer = do let conv = NewConvUnmanaged $ NewConv us [] name (Set.fromList a) r (Just (ConvTeamInfo tid False)) mtimer Nothing roleNameWireAdmin post $ g . path "/conversations" . zUser u . zConn "conn" . zType "access" . json conv +deleteTeamConv :: (HasGalley m, MonadIO m, MonadHttp m) => TeamId -> ConvId -> UserId -> m ResponseLBS +deleteTeamConv tid convId zusr = do + g <- viewGalley + delete + ( g + . paths ["teams", toByteString' tid, "conversations", toByteString' convId] + . zUser zusr + . zConn "conn" + ) + postConvWithRole :: UserId -> [UserId] -> Maybe Text -> [Access] -> Maybe AccessRole -> Maybe Milliseconds -> RoleName -> TestM ResponseLBS postConvWithRole u members name access arole timer role = postConvQualified From d0bbc8e4d850a62029ff4e9d2003ec84cbd54d91 Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Fri, 15 Oct 2021 11:02:09 +0200 Subject: [PATCH 04/11] Data.deleteConversation: remove remotes --- services/galley/src/Galley/API/Teams.hs | 1 - services/galley/src/Galley/Data.hs | 13 ++++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index 6285cc3609e..50a3fc6db2d 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -798,7 +798,6 @@ deleteTeamConversation zusr zcon tid cid = do -- TODO: we don't delete bots here, but we should do that, since every -- bot user can only be in a single conversation - -- TODO: also delete remote members Data.removeTeamConv tid cid getSearchVisibilityH :: UserId ::: TeamId ::: JSON -> Galley Response diff --git a/services/galley/src/Galley/Data.hs b/services/galley/src/Galley/Data.hs index 0e3267a7f54..0406c6c7de5 100644 --- a/services/galley/src/Galley/Data.hs +++ b/services/galley/src/Galley/Data.hs @@ -137,7 +137,7 @@ import Data.Id as Id import Data.Json.Util (UTCTimeMillis (..)) import Data.LegalHold (UserLegalHoldStatus (..), defUserLegalHoldStatus) import qualified Data.List.Extra as List -import Data.List.NonEmpty (NonEmpty) +import Data.List.NonEmpty (NonEmpty ((:|))) import Data.List.Split (chunksOf) import qualified Data.Map.Strict as Map import Data.Misc (Milliseconds) @@ -762,8 +762,15 @@ updateConversationMessageTimer cid mtimer = retry x5 $ write Cql.updateConvMessa deleteConversation :: (MonadClient m, Log.MonadLogger m, MonadThrow m) => ConvId -> m () deleteConversation cid = do retry x5 $ write Cql.markConvDeleted (params Quorum (Identity cid)) - mm <- members cid - for_ mm $ \m -> removeMember (lmId m) cid + + members cid >>= \case + [] -> pure () + (m : ms) -> removeLocalMembersFromLocalConv cid (lmId <$> (m :| ms)) + + lookupRemoteMembers cid >>= \case + [] -> pure () + (m : ms) -> removeRemoteMembersFromLocalConv cid (rmId <$> (m :| ms)) + retry x5 $ write Cql.deleteConv (params Quorum (Identity cid)) acceptConnect :: MonadClient m => ConvId -> m () From b689c3b71852a73ea4c0de503cc1a379593a851e Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Fri, 15 Oct 2021 11:18:41 +0200 Subject: [PATCH 05/11] add changelog entry --- changelog.d/6-federation/delete-conversations | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6-federation/delete-conversations diff --git a/changelog.d/6-federation/delete-conversations b/changelog.d/6-federation/delete-conversations new file mode 100644 index 00000000000..6fcac46d765 --- /dev/null +++ b/changelog.d/6-federation/delete-conversations @@ -0,0 +1 @@ +Support deleting conversations with federated users From faece90a35a51ce470d3dc769ba3c2766f8630f1 Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Tue, 19 Oct 2021 11:37:43 +0200 Subject: [PATCH 06/11] wire-api: extend ConversationAction --- libs/wire-api/src/Wire/API/Conversation/Action.hs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libs/wire-api/src/Wire/API/Conversation/Action.hs b/libs/wire-api/src/Wire/API/Conversation/Action.hs index 855376d8528..a7bf22c23b2 100644 --- a/libs/wire-api/src/Wire/API/Conversation/Action.hs +++ b/libs/wire-api/src/Wire/API/Conversation/Action.hs @@ -44,6 +44,7 @@ data ConversationAction | ConversationActionReceiptModeUpdate ConversationReceiptModeUpdate | ConversationActionMemberUpdate (Qualified UserId) OtherMemberUpdate | ConversationActionAccessUpdate ConversationAccessData + | ConversationActionDelete deriving stock (Eq, Show, Generic) deriving (Arbitrary) via (GenericUniform ConversationAction) deriving (ToJSON, FromJSON) via (CustomEncoded ConversationAction) @@ -71,6 +72,8 @@ conversationActionToEvent now quid qcnv (ConversationActionMemberUpdate target ( in Event MemberStateUpdate qcnv quid now (EdMemberUpdate update) conversationActionToEvent now quid qcnv (ConversationActionAccessUpdate update) = Event ConvAccessUpdate qcnv quid now (EdConvAccessUpdate update) +conversationActionToEvent now quid qcnv ConversationActionDelete = + Event ConvDelete qcnv quid now EdConvDelete conversationActionTag :: Qualified UserId -> ConversationAction -> Action conversationActionTag _ (ConversationActionAddMembers _ _) = AddConversationMember @@ -82,3 +85,4 @@ conversationActionTag _ (ConversationActionMessageTimerUpdate _) = ModifyConvers conversationActionTag _ (ConversationActionReceiptModeUpdate _) = ModifyConversationReceiptMode conversationActionTag _ (ConversationActionMemberUpdate _ _) = ModifyOtherConversationMember conversationActionTag _ (ConversationActionAccessUpdate _) = ModifyConversationAccess +conversationActionTag _ ConversationActionDelete = DeleteConversation From 0105e5fac38d5c00af678e37b3c5f1bf86f6ef59 Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Tue, 19 Oct 2021 14:34:16 +0200 Subject: [PATCH 07/11] onConversationDeleted -> onConversationUpdated --- .../src/Wire/API/Federation/API/Galley.hs | 22 --------- .../Federation/Golden/ConversationDelete.hs | 41 ----------------- .../Wire/API/Federation/Golden/GoldenSpec.hs | 2 - .../golden/testObject_ConversationDelete1 | 12 ----- .../wire-api-federation.cabal | 3 +- services/galley/src/Galley/API/Federation.hs | 16 ++----- services/galley/src/Galley/API/Teams.hs | 45 +++---------------- services/galley/src/Galley/API/Update.hs | 17 +++++++ services/galley/src/Galley/API/Util.hs | 15 ++++++- services/galley/test/integration/API.hs | 27 +++++------ .../galley/test/integration/API/Federation.hs | 19 ++++---- 11 files changed, 64 insertions(+), 155 deletions(-) delete mode 100644 libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/ConversationDelete.hs delete mode 100644 libs/wire-api-federation/test/golden/testObject_ConversationDelete1 diff --git a/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs b/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs index 97b2ea50d89..bdb02efccb3 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/API/Galley.hs @@ -78,13 +78,6 @@ data Api routes = Api :> OriginDomainHeader :> ReqBody '[JSON] ConversationUpdate :> Post '[JSON] (), - onConversationDeleted :: - routes - :- "federation" - :> "on-conversation-deleted" - :> OriginDomainHeader - :> ReqBody '[JSON] ConversationDelete - :> Post '[JSON] (), leaveConversation :: routes :- "federation" @@ -199,21 +192,6 @@ data ConversationUpdate = ConversationUpdate deriving (Arbitrary) via (GenericUniform ConversationUpdate) deriving (ToJSON, FromJSON) via (CustomEncoded ConversationUpdate) --- | Notifies the target domain that a conversation owned by the origin domain --- has been deleted. -data ConversationDelete = ConversationDelete - { cdTime :: UTCTime, - -- | User that deleted the conversation. - cdOriginUserId :: Qualified UserId, - -- | Conversation that is being deleted. It is owned by the origin domain. - cdConvId :: ConvId, - -- | All members of the conversation that are owned by target domain. - cdMembers :: [UserId] - } - deriving stock (Eq, Show, Generic) - deriving (Arbitrary) via (GenericUniform ConversationDelete) - deriving (ToJSON, FromJSON) via (CustomEncoded ConversationDelete) - data LeaveConversationRequest = LeaveConversationRequest { -- | The conversation is assumed to be owned by the target domain, which -- allows us to protect against relay attacks diff --git a/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/ConversationDelete.hs b/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/ConversationDelete.hs deleted file mode 100644 index 236932daf12..00000000000 --- a/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/ConversationDelete.hs +++ /dev/null @@ -1,41 +0,0 @@ --- This file is part of the Wire Server implementation. --- --- Copyright (C) 2021 Wire Swiss GmbH --- --- This program is free software: you can redistribute it and/or modify it under --- the terms of the GNU Affero General Public License as published by the Free --- Software Foundation, either version 3 of the License, or (at your option) any --- later version. --- --- This program is distributed in the hope that it will be useful, but WITHOUT --- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS --- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more --- details. --- --- You should have received a copy of the GNU Affero General Public License along --- with this program. If not, see . - -module Test.Wire.API.Federation.Golden.ConversationDelete where - -import Data.Domain (Domain (Domain)) -import Data.Id (Id (Id)) -import Data.Qualified (Qualified (Qualified)) -import qualified Data.UUID as UUID -import Imports -import Wire.API.Federation.API.Galley (ConversationDelete (..)) - -testObject_ConversationDelete1 :: ConversationDelete -testObject_ConversationDelete1 = - ConversationDelete - { cdTime = read "1864-04-12 12:22:43.673 UTC", - cdOriginUserId = - Qualified - (Id (fromJust (UUID.fromString "00000000-0000-0000-0000-000100000007"))) - (Domain "golden.example.com"), - cdConvId = - Id (fromJust (UUID.fromString "00000000-0000-0000-0000-000100000006")), - cdMembers = - [ Id (fromJust (UUID.fromString "00000000-0000-0000-0000-000100000008")), - Id (fromJust (UUID.fromString "00000000-0000-0000-0000-000100000009")) - ] - } diff --git a/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/GoldenSpec.hs b/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/GoldenSpec.hs index 6842ff72f51..8142d17ae17 100644 --- a/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/GoldenSpec.hs +++ b/libs/wire-api-federation/test/Test/Wire/API/Federation/Golden/GoldenSpec.hs @@ -19,7 +19,6 @@ module Test.Wire.API.Federation.Golden.GoldenSpec where import Imports import Test.Hspec -import qualified Test.Wire.API.Federation.Golden.ConversationDelete as ConversationDelete import qualified Test.Wire.API.Federation.Golden.ConversationUpdate as ConversationUpdate import qualified Test.Wire.API.Federation.Golden.LeaveConversationRequest as LeaveConversationRequest import qualified Test.Wire.API.Federation.Golden.LeaveConversationResponse as LeaveConversationResponse @@ -63,4 +62,3 @@ spec = (NewConnectionResponse.testObject_NewConnectionResponse3, "testObject_NewConnectionResponse3.json"), (NewConnectionResponse.testObject_NewConnectionResponse4, "testObject_NewConnectionResponse4.json") ] - testObjects [(ConversationDelete.testObject_ConversationDelete1, "testObject_ConversationDelete1")] diff --git a/libs/wire-api-federation/test/golden/testObject_ConversationDelete1 b/libs/wire-api-federation/test/golden/testObject_ConversationDelete1 deleted file mode 100644 index 8f7554cf0f6..00000000000 --- a/libs/wire-api-federation/test/golden/testObject_ConversationDelete1 +++ /dev/null @@ -1,12 +0,0 @@ -{ - "time": "1864-04-12T12:22:43.673Z", - "members": [ - "00000000-0000-0000-0000-000100000008", - "00000000-0000-0000-0000-000100000009" - ], - "origin_user_id": { - "domain": "golden.example.com", - "id": "00000000-0000-0000-0000-000100000007" - }, - "conv_id": "00000000-0000-0000-0000-000100000006" -} \ No newline at end of file diff --git a/libs/wire-api-federation/wire-api-federation.cabal b/libs/wire-api-federation/wire-api-federation.cabal index 61bb2e5a98a..e8729651c85 100644 --- a/libs/wire-api-federation/wire-api-federation.cabal +++ b/libs/wire-api-federation/wire-api-federation.cabal @@ -4,7 +4,7 @@ cabal-version: 1.12 -- -- see: https://github.com/sol/hpack -- --- hash: b6b3446eb43105b1212eecc23dcc752f233f7e5e933e6a141fdbe464747508f6 +-- hash: 03f7245b036ccc38819ed5f5654dae8d96b7ec5917b2f898be3305193bc3faf5 name: wire-api-federation version: 0.1.0 @@ -77,7 +77,6 @@ test-suite spec other-modules: Test.Wire.API.Federation.API.BrigSpec Test.Wire.API.Federation.ClientSpec - Test.Wire.API.Federation.Golden.ConversationDelete Test.Wire.API.Federation.Golden.ConversationUpdate Test.Wire.API.Federation.Golden.GoldenSpec Test.Wire.API.Federation.Golden.LeaveConversationRequest diff --git a/services/galley/src/Galley/API/Federation.hs b/services/galley/src/Galley/API/Federation.hs index efd3d5754b8..b716b1b2e89 100644 --- a/services/galley/src/Galley/API/Federation.hs +++ b/services/galley/src/Galley/API/Federation.hs @@ -50,10 +50,8 @@ import Wire.API.Conversation.Action import Wire.API.Conversation.Member (OtherMember (..)) import qualified Wire.API.Conversation.Role as Public import Wire.API.Event.Conversation -import qualified Wire.API.Event.Conversation as Conv import Wire.API.Federation.API.Galley - ( ConversationDelete (..), - ConversationUpdate (..), + ( ConversationUpdate (..), GetConversationsRequest (..), GetConversationsResponse (..), LeaveConversationRequest (..), @@ -76,7 +74,6 @@ federationSitemap = { FederationAPIGalley.onConversationCreated = onConversationCreated, FederationAPIGalley.getConversations = getConversations, FederationAPIGalley.onConversationUpdated = onConversationUpdated, - FederationAPIGalley.onConversationDeleted = onConversationDeleted, FederationAPIGalley.leaveConversation = leaveConversation, FederationAPIGalley.onMessageSent = onMessageSent, FederationAPIGalley.sendMessage = sendMessage @@ -158,6 +155,9 @@ onConversationUpdated requestingDomain cu = do ConversationActionMemberUpdate _ _ -> pure (Just $ cuAction cu, []) ConversationActionReceiptModeUpdate _ -> pure (Just $ cuAction cu, []) ConversationActionAccessUpdate _ -> pure (Just $ cuAction cu, []) + ConversationActionDelete -> do + Data.removeLocalMembersFromRemoteConv qconvId presentUsers + pure (Just $ cuAction cu, []) unless allUsersArePresent $ Log.warn $ @@ -198,14 +198,6 @@ addLocalUsersToRemoteConv remoteConvId qAdder localUsers = do Data.addLocalMembersToRemoteConv (qUntagged remoteConvId) connectedList pure connected -onConversationDeleted :: Domain -> ConversationDelete -> Galley () -onConversationDeleted requestingDomain convDelete = do - let qconvId = Qualified (cdConvId convDelete) requestingDomain - let event = Conv.Event Conv.ConvDelete qconvId (cdOriginUserId convDelete) (cdTime convDelete) Conv.EdConvDelete - let bots = [] - pushConversationEvent Nothing event (cdMembers convDelete) bots - Data.removeLocalMembersFromRemoteConv qconvId (cdMembers convDelete) - -- FUTUREWORK: actually return errors as part of the response instead of throwing leaveConversation :: Domain -> diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index 50a3fc6db2d..dd1c93e023f 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -82,6 +82,7 @@ import qualified Data.UUID.Util as UUID import Galley.API.Error as Galley import Galley.API.LegalHold import qualified Galley.API.Teams.Notifications as APITeamQueue +import qualified Galley.API.Update as API import Galley.API.Util import Galley.App import qualified Galley.Data as Data @@ -89,7 +90,6 @@ import qualified Galley.Data.LegalHold as Data import qualified Galley.Data.SearchVisibility as SearchVisibilityData import Galley.Data.Services (BotMember) import qualified Galley.Data.TeamFeatures as TeamFeatures -import qualified Galley.Data.Types as Data import qualified Galley.External as External import qualified Galley.Intra.Journal as Journal import Galley.Intra.Push @@ -99,7 +99,7 @@ import Galley.Intra.User import Galley.Options import qualified Galley.Options as Opts import qualified Galley.Queue as Q -import Galley.Types (LocalMember (lmId), RemoteMember (rmId), UserIdList (UserIdList)) +import Galley.Types (UserIdList (UserIdList)) import qualified Galley.Types as Conv import Galley.Types.Conversations.Roles as Roles import Galley.Types.Teams hiding (newTeam) @@ -115,8 +115,6 @@ import qualified System.Logger.Class as Log import UnliftIO (mapConcurrently) import qualified Wire.API.Conversation.Role as Public import Wire.API.ErrorDescription (ConvNotFound, NotATeamMember, operationDenied) -import Wire.API.Federation.API.Galley (ConversationDelete (..)) -import qualified Wire.API.Federation.API.Galley as FederatedGalley import qualified Wire.API.Notification as Public import qualified Wire.API.Team as Public import qualified Wire.API.Team.Conversation as Public @@ -764,41 +762,10 @@ getTeamConversation zusr tid cid = do Data.teamConversation tid cid >>= maybe (throwErrorDescriptionType @ConvNotFound) pure deleteTeamConversation :: UserId -> ConnId -> TeamId -> ConvId -> Galley () -deleteTeamConversation zusr zcon tid cid = do - localDomain <- viewFederationDomain - let qconvId = Qualified cid localDomain - qusr = Qualified zusr localDomain - (bots, cmems) <- localBotsAndUsers <$> Data.members cid - ensureActionAllowed Roles.DeleteConversation =<< getSelfMemberFromLocalsLegacy zusr cmems - flip Data.deleteCode Data.ReusableCode =<< Data.mkKey cid - - now <- liftIO getCurrentTime - do - let event = Conv.Event Conv.ConvDelete qconvId qusr now Conv.EdConvDelete - pushConversationEvent (Just zcon) event (map lmId cmems) bots - - do - remoteMembers <- Data.lookupRemoteMembers cid - for_ (bucketQualified (qUntagged . rmId <$> remoteMembers)) $ - \(Qualified remoteMembersDomain domain) -> do - let convDelete = - ConversationDelete - { cdTime = now, - cdOriginUserId = qusr, - cdConvId = cid, - cdMembers = remoteMembersDomain - } - let rpc = - FederatedGalley.onConversationDeleted - FederatedGalley.clientRoutes - localDomain - convDelete - runFederatedGalley domain rpc - - -- TODO: we don't delete bots here, but we should do that, since every - -- bot user can only be in a single conversation - - Data.removeTeamConv tid cid +deleteTeamConversation zusr zcon _tid cid = do + lusr <- qualifyLocal zusr + lconv <- qualifyLocal cid + void $ API.deleteLocalConversation lusr zcon lconv getSearchVisibilityH :: UserId ::: TeamId ::: JSON -> Galley Response getSearchVisibilityH (uid ::: tid ::: _) = do diff --git a/services/galley/src/Galley/API/Update.hs b/services/galley/src/Galley/API/Update.hs index 4a4dce798e7..bd795ee8617 100644 --- a/services/galley/src/Galley/API/Update.hs +++ b/services/galley/src/Galley/API/Update.hs @@ -36,6 +36,7 @@ module Galley.API.Update updateLocalConversation, updateConversationAccessUnqualified, updateConversationAccess, + deleteLocalConversation, -- * Managing Members addMembersUnqualified, @@ -368,6 +369,15 @@ updateLocalConversationMessageTimer lusr con lcnv update = updateLocalConversation lcnv (qUntagged lusr) (Just con) $ ConversationActionMessageTimerUpdate update +deleteLocalConversation :: + Local UserId -> + ConnId -> + Local ConvId -> + Galley (UpdateResult Event) +deleteLocalConversation lusr con lcnv = + getUpdateResult $ + updateLocalConversation lcnv (qUntagged lusr) (Just con) ConversationActionDelete + -- | Update a local conversation, and notify all local and remote members. updateLocalConversation :: Local ConvId -> @@ -435,6 +445,13 @@ performAction qusr conv action = case action of ConversationActionAccessUpdate update -> do performAccessUpdateAction qusr conv update pure (mempty, action) + ConversationActionDelete -> lift $ do + let cid = Data.convId conv + (`Data.deleteCode` ReusableCode) =<< mkKey cid + case Data.convTeam conv of + Nothing -> Data.deleteConversation cid + Just tid -> Data.removeTeamConv tid cid + pure (mempty, action) addCodeH :: UserId ::: ConnId ::: ConvId -> Galley Response addCodeH (usr ::: zcon ::: cnv) = diff --git a/services/galley/src/Galley/API/Util.hs b/services/galley/src/Galley/API/Util.hs index fae4c5a54c6..12098dcdbc7 100644 --- a/services/galley/src/Galley/API/Util.hs +++ b/services/galley/src/Galley/API/Util.hs @@ -65,7 +65,7 @@ import Wire.API.ErrorDescription import qualified Wire.API.Federation.API.Brig as FederatedBrig import Wire.API.Federation.API.Galley as FederatedGalley import Wire.API.Federation.Client (FederationClientFailure, FederatorClient, executeFederated) -import Wire.API.Federation.Error (federationErrorToWai) +import Wire.API.Federation.Error (federationErrorToWai, federationNotImplemented) import Wire.API.Federation.GRPC.Types (Component (..)) import qualified Wire.API.User as User @@ -162,6 +162,19 @@ ensureConversationActionAllowed action conv self = do -- extra action-specific checks case action of ConversationActionAddMembers _ role -> ensureConvRoleNotElevated self role + ConversationActionDelete -> do + case Data.convTeam conv of + Just tid -> do + foldQualified + loc + ( \lusr -> do + void $ + Data.teamMember tid (tUnqualified lusr) + >>= ifNothing (errorDescriptionTypeToWai @NotATeamMember) + ) + (\_ -> throwM federationNotImplemented) + (convMemberId loc self) + Nothing -> pure () ConversationActionAccessUpdate target -> do -- 'PrivateAccessRole' is for self-conversations, 1:1 conversations and -- so on; users are not supposed to be able to make other conversations diff --git a/services/galley/test/integration/API.hs b/services/galley/test/integration/API.hs index 529cf5cb668..a610c618923 100644 --- a/services/galley/test/integration/API.hs +++ b/services/galley/test/integration/API.hs @@ -87,8 +87,8 @@ import Wire.API.Conversation import Wire.API.Conversation.Action import qualified Wire.API.Federation.API.Brig as FederatedBrig import Wire.API.Federation.API.Galley - ( Api (onConversationDeleted, onConversationUpdated), - ConversationDelete (..), + ( Api (onConversationUpdated), + ConversationUpdate (cuAction, cuAlreadyPresentUsers, cuOrigUserId), GetConversationsResponse (..), RemoteConvMembers (..), RemoteConversation (..), @@ -166,7 +166,7 @@ tests s = test s "fail to add members when not connected" postMembersFail, test s "fail to add too many members" postTooManyMembersFail, test s "add remote members" testAddRemoteMember, - test s "delete conversation with remote members" testDeleteConversationWithRemoteMembers, + test s "delete conversation with remote members" testDeleteTeamConversationWithRemoteMembers, test s "get conversations/:domain/:cnv - local" testGetQualifiedLocalConv, test s "get conversations/:domain/:cnv - local, not found" testGetQualifiedLocalConvNotFound, test s "get conversations/:domain/:cnv - local, not participating" testGetQualifiedLocalConvNotParticipating, @@ -251,7 +251,6 @@ emptyFederatedGalley = { FederatedGalley.onConversationCreated = \_ _ -> e "onConversationCreated", FederatedGalley.getConversations = \_ _ -> e "getConversations", FederatedGalley.onConversationUpdated = \_ _ -> e "onConversationUpdated", - FederatedGalley.onConversationDeleted = \_ _ -> e "onConversationDeleted", FederatedGalley.leaveConversation = \_ _ -> e "leaveConversation", FederatedGalley.onMessageSent = \_ _ -> e "onMessageSent", FederatedGalley.sendMessage = \_ _ -> e "sendMessage" @@ -1908,8 +1907,8 @@ testAddRemoteMember = do toJSON [mkProfile bob (Name "bob")] | otherwise = toJSON () -testDeleteConversationWithRemoteMembers :: TestM () -testDeleteConversationWithRemoteMembers = do +testDeleteTeamConversationWithRemoteMembers :: TestM () +testDeleteTeamConversationWithRemoteMembers = do (alice, tid) <- createBindingTeam localDomain <- viewFederationDomain let qalice = Qualified alice localDomain @@ -1926,8 +1925,7 @@ testDeleteConversationWithRemoteMembers = do let brigApi = emptyFederatedBrig galleyApi = emptyFederatedGalley - { onConversationUpdated = \_domain _update -> pure (), - onConversationDeleted = \_ _ -> pure () + { onConversationUpdated = \_domain _update -> pure () } opts <- view tsGConf @@ -1939,14 +1937,13 @@ testDeleteConversationWithRemoteMembers = do !!! const 200 === statusCode liftIO $ do - let convDeletes = mapMaybe parseFedRequest received - convDelete <- case convDeletes of - [] -> assertFailure "No ConversationDelete requests received" + let convUpdates = mapMaybe parseFedRequest received + convUpdate <- case (filter ((== ConversationActionDelete) . cuAction) convUpdates) of + [] -> assertFailure "No ConversationUpdate requests received" [convDelete] -> pure convDelete - _ -> assertFailure "Multiple ConversationDelete requests received" - cdMembers convDelete @?= [bobId] - cdConvId convDelete @?= convId - cdOriginUserId convDelete @?= qalice + _ -> assertFailure "Multiple ConversationUpdate requests received" + cuAlreadyPresentUsers convUpdate @?= [bobId] + cuOrigUserId convUpdate @?= qalice where parseFedRequest :: FromJSON a => GRPC.FederatedRequest -> Maybe a parseFedRequest fr = diff --git a/services/galley/test/integration/API/Federation.hs b/services/galley/test/integration/API/Federation.hs index f90faa376bc..6c7b5d6902d 100644 --- a/services/galley/test/integration/API/Federation.hs +++ b/services/galley/test/integration/API/Federation.hs @@ -51,7 +51,7 @@ import TestSetup import Wire.API.Conversation.Action (ConversationAction (..)) import Wire.API.Conversation.Member (Member (..)) import Wire.API.Conversation.Role -import Wire.API.Federation.API.Galley (ConversationDelete (..), GetConversationsRequest (..), GetConversationsResponse (..), RemoteConvMembers (..), RemoteConversation (..)) +import Wire.API.Federation.API.Galley (GetConversationsRequest (..), GetConversationsResponse (..), RemoteConvMembers (..), RemoteConversation (..)) import qualified Wire.API.Federation.API.Galley as FedGalley import qualified Wire.API.Federation.GRPC.Types as F import Wire.API.Message (ClientMismatchStrategy (..), MessageSendingStatus (mssDeletedClients, mssFailedToSend, mssRedundantClients), mkQualifiedOtrPayload, mssMissingClients) @@ -75,7 +75,7 @@ tests s = test s "POST /federation/on-conversation-updated : Notify local user about member update" notifyMemberUpdate, test s "POST /federation/on-conversation-updated : Notify local user about receipt mode update" notifyReceiptMode, test s "POST /federation/on-conversation-updated : Notify local user about access update" notifyAccess, - test s "POST /federation/on-conversation-deleted : Notify local users about a deleted conversation" notifyDeletedConversation, + test s "POST /federation/on-conversation-updated : Notify local users about a deleted conversation" notifyDeletedConversation, test s "POST /federation/leave-conversation : Success" leaveConversationSuccess, test s "POST /federation/on-message-sent : Receive a message from another backend" onMessageSent, test s "POST /federation/send-message : Post a message sent from another backend" sendMessage @@ -520,14 +520,15 @@ notifyDeletedConversation = do WS.bracketR c alice $ \wsAlice -> do now <- liftIO getCurrentTime - let convDelete = - ConversationDelete - { cdTime = now, - cdOriginUserId = qbob, - cdConvId = conv, - cdMembers = [alice] + let cu = + FedGalley.ConversationUpdate + { FedGalley.cuTime = now, + FedGalley.cuOrigUserId = qbob, + FedGalley.cuConvId = qUnqualified qconv, + FedGalley.cuAlreadyPresentUsers = [alice], + FedGalley.cuAction = ConversationActionDelete } - FedGalley.onConversationDeleted fedGalleyClient bobDomain convDelete + FedGalley.onConversationUpdated fedGalleyClient bobDomain cu liftIO $ do WS.assertMatch_ (5 # Second) wsAlice $ \n -> do From 71c75c38934fb8ae1c4a331cb5d7b73344a341ba Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Tue, 19 Oct 2021 16:51:57 +0200 Subject: [PATCH 08/11] fix compilation --- services/galley/src/Galley/API/Federation.hs | 2 +- services/galley/test/integration/API/Federation.hs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/galley/src/Galley/API/Federation.hs b/services/galley/src/Galley/API/Federation.hs index 81cb3f1411a..3870705d46f 100644 --- a/services/galley/src/Galley/API/Federation.hs +++ b/services/galley/src/Galley/API/Federation.hs @@ -164,7 +164,7 @@ onConversationUpdated requestingDomain cu = do ConversationActionReceiptModeUpdate _ -> pure (Just $ cuAction cu, []) ConversationActionAccessUpdate _ -> pure (Just $ cuAction cu, []) ConversationActionDelete -> do - Data.removeLocalMembersFromRemoteConv qconvId presentUsers + Data.removeLocalMembersFromRemoteConv rconvId presentUsers pure (Just $ cuAction cu, []) unless allUsersArePresent $ diff --git a/services/galley/test/integration/API/Federation.hs b/services/galley/test/integration/API/Federation.hs index aec833f00e1..0bac6092342 100644 --- a/services/galley/test/integration/API/Federation.hs +++ b/services/galley/test/integration/API/Federation.hs @@ -508,7 +508,7 @@ notifyDeletedConversation = do mapM_ (`connectWithRemoteUser` qbob) [alice] registerRemoteConv qconv - qbob + bob (Just "gossip") (Set.fromList (map mkMember [qalice])) From 7f3fe5349daac7b877972593a558e3c42d66268d Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Wed, 20 Oct 2021 12:42:32 +0200 Subject: [PATCH 09/11] remove duplicated import --- services/galley/test/integration/API.hs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/services/galley/test/integration/API.hs b/services/galley/test/integration/API.hs index eee14c6b966..201fba6d1b0 100644 --- a/services/galley/test/integration/API.hs +++ b/services/galley/test/integration/API.hs @@ -95,7 +95,6 @@ import Wire.API.Federation.API.Galley ) import qualified Wire.API.Federation.API.Galley as FederatedGalley import qualified Wire.API.Federation.GRPC.Types as F -import qualified Wire.API.Federation.GRPC.Types as GRPC import qualified Wire.API.Message as Message import Wire.API.Routes.MultiTablePaging import Wire.API.User.Client @@ -1944,11 +1943,11 @@ testDeleteTeamConversationWithRemoteMembers = do cuAlreadyPresentUsers convUpdate @?= [bobId] cuOrigUserId convUpdate @?= qalice where - parseFedRequest :: FromJSON a => GRPC.FederatedRequest -> Maybe a + parseFedRequest :: FromJSON a => F.FederatedRequest -> Maybe a parseFedRequest fr = - case GRPC.request fr of + case F.request fr of Just r -> - (decode . cs) (GRPC.body r) + (decode . cs) (F.body r) Nothing -> Nothing testGetQualifiedLocalConv :: TestM () From e712f8ae264adf54dd241d14400a9de4c5390f0d Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Wed, 20 Oct 2021 12:57:33 +0200 Subject: [PATCH 10/11] cosmetic change --- services/galley/src/Galley/Data.hs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/services/galley/src/Galley/Data.hs b/services/galley/src/Galley/Data.hs index c8f1d402dc3..04353e202e9 100644 --- a/services/galley/src/Galley/Data.hs +++ b/services/galley/src/Galley/Data.hs @@ -137,7 +137,7 @@ import Data.Id as Id import Data.Json.Util (UTCTimeMillis (..)) import Data.LegalHold (UserLegalHoldStatus (..), defUserLegalHoldStatus) import qualified Data.List.Extra as List -import Data.List.NonEmpty (NonEmpty ((:|))) +import Data.List.NonEmpty (NonEmpty, nonEmpty) import Data.List.Split (chunksOf) import qualified Data.Map.Strict as Map import Data.Misc (Milliseconds) @@ -763,13 +763,13 @@ deleteConversation :: (MonadClient m, Log.MonadLogger m, MonadThrow m) => ConvId deleteConversation cid = do retry x5 $ write Cql.markConvDeleted (params Quorum (Identity cid)) - members cid >>= \case - [] -> pure () - (m : ms) -> removeLocalMembersFromLocalConv cid (lmId <$> (m :| ms)) + localMembers <- members cid + for_ (nonEmpty localMembers) $ \ms -> + removeLocalMembersFromLocalConv cid (lmId <$> ms) - lookupRemoteMembers cid >>= \case - [] -> pure () - (m : ms) -> removeRemoteMembersFromLocalConv cid (rmId <$> (m :| ms)) + remoteMembers <- lookupRemoteMembers cid + for_ (nonEmpty remoteMembers) $ \ms -> + removeRemoteMembersFromLocalConv cid (rmId <$> ms) retry x5 $ write Cql.deleteConv (params Quorum (Identity cid)) From fb33ea411b1216ebf2047137985a25f6c42dba4b Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Wed, 20 Oct 2021 15:16:31 +0200 Subject: [PATCH 11/11] fix call to withTempServantMockFederator --- services/galley/test/integration/API.hs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/galley/test/integration/API.hs b/services/galley/test/integration/API.hs index dc96931800a..31e12d7970c 100644 --- a/services/galley/test/integration/API.hs +++ b/services/galley/test/integration/API.hs @@ -1933,8 +1933,7 @@ testDeleteTeamConversationWithRemoteMembers = do { onConversationUpdated = \_domain _update -> pure () } - opts <- view tsGConf - (_, received) <- withTempServantMockFederator opts brigApi galleyApi localDomain remoteDomain $ do + (_, received) <- withTempServantMockFederator brigApi galleyApi localDomain $ do postQualifiedMembers alice (remoteBob :| []) convId !!! const 200 === statusCode