WPB-18190: Add route to delete collaborator from team (#4694)

Author: blackheaven

changelog.d/2-features/WPB-18190

@@ -0,0 +1 @@
+Allow collaborator to be removed from a team.

integration/test/API/Brig.hs

@@ -1183,3 +1183,9 @@ refreshAppCookie :: (MakesValue u) => u -> String -> String -> App Response
 refreshAppCookie u tid appId = do
   req <- baseRequest u Brig Versioned $ joinHttpPath ["teams", tid, "apps", appId, "cookies"]
   submit "POST" req
+
+removeTeamCollaborator :: (MakesValue owner, MakesValue collaborator, HasCallStack) => owner -> String -> collaborator -> App Response
+removeTeamCollaborator owner tid collaborator = do
+  (_, collabId) <- objQid collaborator
+  req <- baseRequest owner Galley Versioned $ joinHttpPath ["teams", tid, "collaborators", collabId]
+  submit "DELETE" req

integration/test/Notifications.hs

@@ -106,47 +106,47 @@ awaitNotification user lastNotifId selector = do
   since0 <- mapM objId lastNotifId
   head <$> awaitNotifications user (Nothing :: Maybe ()) since0 1 selector
 
-isDeleteUserNotif :: (MakesValue a) => a -> App Bool
+isDeleteUserNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isDeleteUserNotif n =
   nPayload n %. "type" `isEqual` "user.delete"
 
-isFeatureConfigUpdateNotif :: (MakesValue a) => a -> App Bool
+isFeatureConfigUpdateNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isFeatureConfigUpdateNotif n =
   nPayload n %. "type" `isEqual` "feature-config.update"
 
-isNewMessageNotif :: (MakesValue a) => a -> App Bool
+isNewMessageNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isNewMessageNotif n = fieldEquals n "payload.0.type" "conversation.otr-message-add"
 
-isNewMLSMessageNotif :: (MakesValue a) => a -> App Bool
+isNewMLSMessageNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isNewMLSMessageNotif n = fieldEquals n "payload.0.type" "conversation.mls-message-add"
 
-isWelcomeNotif :: (MakesValue a) => a -> App Bool
+isWelcomeNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isWelcomeNotif n = fieldEquals n "payload.0.type" "conversation.mls-welcome"
 
-isMemberJoinNotif :: (MakesValue a) => a -> App Bool
+isMemberJoinNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isMemberJoinNotif n = fieldEquals n "payload.0.type" "conversation.member-join"
 
-isConvLeaveNotif :: (MakesValue a) => a -> App Bool
+isConvLeaveNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isConvLeaveNotif n = fieldEquals n "payload.0.type" "conversation.member-leave"
 
-isConvLeaveNotifWithLeaver :: (MakesValue user, MakesValue a) => user -> a -> App Bool
+isConvLeaveNotifWithLeaver :: (HasCallStack, MakesValue user, MakesValue a) => user -> a -> App Bool
 isConvLeaveNotifWithLeaver user n =
   fieldEquals n "payload.0.type" "conversation.member-leave"
     &&~ (n %. "payload.0.data.user_ids.0") `isEqual` (user %. "id")
 
-isNotifConv :: (MakesValue conv, MakesValue a, HasCallStack) => conv -> a -> App Bool
+isNotifConv :: (HasCallStack, MakesValue conv, MakesValue a, HasCallStack) => conv -> a -> App Bool
 isNotifConv conv n = fieldEquals n "payload.0.qualified_conversation" (objQidObject conv)
 
-isNotifConvId :: (MakesValue a, HasCallStack) => ConvId -> a -> App Bool
+isNotifConvId :: (HasCallStack, MakesValue a, HasCallStack) => ConvId -> a -> App Bool
 isNotifConvId conv n = do
   let subconvField = "payload.0.subconv"
   fieldEquals n "payload.0.qualified_conversation" (convIdToQidObject conv)
     &&~ maybe (isNothing <$> lookupField n subconvField) (fieldEquals n subconvField) conv.subconvId
 
-isNotifForUser :: (MakesValue user, MakesValue a, HasCallStack) => user -> a -> App Bool
+isNotifForUser :: (HasCallStack, MakesValue user, MakesValue a, HasCallStack) => user -> a -> App Bool
 isNotifForUser user n = fieldEquals n "payload.0.data.qualified_user_ids.0" (objQidObject user)
 
-isNotifFromUser :: (MakesValue user, MakesValue a, HasCallStack) => user -> a -> App Bool
+isNotifFromUser :: (HasCallStack, MakesValue user, MakesValue a, HasCallStack) => user -> a -> App Bool
 isNotifFromUser user n = fieldEquals n "payload.0.qualified_from" (objQidObject user)
 
 isConvNameChangeNotif :: (HasCallStack, MakesValue a) => a -> App Bool
@@ -171,66 +171,69 @@ isConvAccessUpdateNotif :: (HasCallStack, MakesValue n) => n -> App Bool
 isConvAccessUpdateNotif n =
   fieldEquals n "payload.0.type" "conversation.access-update"
 
-isConvCreateNotif :: (MakesValue a) => a -> App Bool
+isConvCreateNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isConvCreateNotif n = fieldEquals n "payload.0.type" "conversation.create"
 
 -- | like 'isConvCreateNotif' but excludes self conversations
-isConvCreateNotifNotSelf :: (MakesValue a) => a -> App Bool
+isConvCreateNotifNotSelf :: (HasCallStack, MakesValue a) => a -> App Bool
 isConvCreateNotifNotSelf n =
   fieldEquals n "payload.0.type" "conversation.create"
     &&~ do not <$> fieldEquals n "payload.0.data.access" ["private"]
 
-isConvDeleteNotif :: (MakesValue a) => a -> App Bool
+isConvDeleteNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isConvDeleteNotif n = fieldEquals n "payload.0.type" "conversation.delete"
 
-notifTypeIsEqual :: (MakesValue a) => String -> a -> App Bool
+notifTypeIsEqual :: (HasCallStack, MakesValue a) => String -> a -> App Bool
 notifTypeIsEqual typ n = nPayload n %. "type" `isEqual` typ
 
-isTeamMemberJoinNotif :: (MakesValue a) => a -> App Bool
+isTeamMemberJoinNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isTeamMemberJoinNotif = notifTypeIsEqual "team.member-join"
 
-isTeamMemberLeaveNotif :: (MakesValue a) => a -> App Bool
+isTeamMemberLeaveNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isTeamMemberLeaveNotif = notifTypeIsEqual "team.member-leave"
 
-isTeamCollaboratorAddedNotif :: (MakesValue a) => a -> App Bool
+isTeamCollaboratorAddedNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isTeamCollaboratorAddedNotif = notifTypeIsEqual "team.collaborator-add"
 
-isUserActivateNotif :: (MakesValue a) => a -> App Bool
+isTeamCollaboratorRemovedNotif :: (HasCallStack, MakesValue a) => a -> App Bool
+isTeamCollaboratorRemovedNotif = notifTypeIsEqual "team.collaborator-remove"
+
+isUserActivateNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isUserActivateNotif = notifTypeIsEqual "user.activate"
 
-isUserClientAddNotif :: (MakesValue a) => a -> App Bool
+isUserClientAddNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isUserClientAddNotif = notifTypeIsEqual "user.client-add"
 
-isUserUpdatedNotif :: (MakesValue a) => a -> App Bool
+isUserUpdatedNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isUserUpdatedNotif = notifTypeIsEqual "user.update"
 
-isUserClientRemoveNotif :: (MakesValue a) => a -> App Bool
+isUserClientRemoveNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isUserClientRemoveNotif = notifTypeIsEqual "user.client-remove"
 
-isUserLegalholdRequestNotif :: (MakesValue a) => a -> App Bool
+isUserLegalholdRequestNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isUserLegalholdRequestNotif = notifTypeIsEqual "user.legalhold-request"
 
-isUserLegalholdEnabledNotif :: (MakesValue a) => a -> App Bool
+isUserLegalholdEnabledNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isUserLegalholdEnabledNotif = notifTypeIsEqual "user.legalhold-enable"
 
-isUserLegalholdDisabledNotif :: (MakesValue a) => a -> App Bool
+isUserLegalholdDisabledNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isUserLegalholdDisabledNotif = notifTypeIsEqual "user.legalhold-disable"
 
-isUserConnectionNotif :: (MakesValue a) => a -> App Bool
+isUserConnectionNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isUserConnectionNotif = notifTypeIsEqual "user.connection"
 
-isConnectionNotif :: (MakesValue a) => String -> a -> App Bool
+isConnectionNotif :: (HasCallStack, MakesValue a) => String -> a -> App Bool
 isConnectionNotif status n =
   -- NB:
   -- (&&) <$> (print "hello" *> pure False) <*> fail "bla" === _|_
   -- runMaybeT $  (lift (print "hello") *> MaybeT (pure Nothing)) *> lift (fail "bla") === pure Nothing
   nPayload n %. "type" `isEqual` "user.connection"
     &&~ nPayload n %. "connection.status" `isEqual` status
 
-isUserGroupCreatedNotif :: (MakesValue a) => a -> App Bool
+isUserGroupCreatedNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isUserGroupCreatedNotif = notifTypeIsEqual "user-group.created"
 
-isUserGroupUpdatedNotif :: (MakesValue a) => a -> App Bool
+isUserGroupUpdatedNotif :: (HasCallStack, MakesValue a) => a -> App Bool
 isUserGroupUpdatedNotif = notifTypeIsEqual "user-group.updated"
 
 isConvResetNotif :: (HasCallStack, MakesValue n) => n -> App Bool
@@ -264,7 +267,7 @@ assertLeaveNotification fromUser conv user client leaver =
           ]
       )
 
-assertConvUserDeletedNotif :: (MakesValue leaverId) => WebSocket -> leaverId -> App ()
+assertConvUserDeletedNotif :: (HasCallStack, MakesValue leaverId) => WebSocket -> leaverId -> App ()
 assertConvUserDeletedNotif ws leaverId = do
   n <- awaitMatch isConvLeaveNotif ws
   nPayload n %. "data.qualified_user_ids.0" `shouldMatch` leaverId

integration/test/Test/TeamCollaborators.hs

@@ -2,8 +2,9 @@ module Test.TeamCollaborators where
 
 import API.Brig
 import API.Galley
+import qualified API.GalleyInternal as Internal
 import Data.Tuple.Extra
-import Notifications (isTeamCollaboratorAddedNotif)
+import Notifications (isConvLeaveNotif, isTeamCollaboratorAddedNotif, isTeamCollaboratorRemovedNotif, isTeamMemberLeaveNotif)
 import SetupHelpers
 import Testlib.Prelude
 
@@ -156,4 +157,117 @@ testImplicitConnectionNoCollaborator = do
   -- Alice and Bob aren't connected at all.
   postOne2OneConversation bob alice team0 "chit-chat" >>= assertLabel 403 "no-team-member"
 
-  postOne2OneConversation alice bob team0 "chat-chit" >>= assertLabel 403 "non-binding-team-members"
+testRemoveCollaboratorInTeamsO2O :: (HasCallStack) => App ()
+testRemoveCollaboratorInTeamsO2O = do
+  (owner0, team0, [alice]) <- createTeam OwnDomain 2
+  (owner1, team1, [bob]) <- createTeam OwnDomain 2
+
+  -- At the time of writing, it wasn't clear if this should be a bot instead.
+  charlie <- randomUser OwnDomain def
+  addTeamCollaborator owner0 team0 charlie ["implicit_connection"] >>= assertSuccess
+  addTeamCollaborator owner1 team1 charlie ["implicit_connection"] >>= assertSuccess
+
+  convId <-
+    postOne2OneConversation charlie alice team0 "chit-chat" `bindResponse` \resp -> do
+      resp.status `shouldMatchInt` 201
+      resp.json %. "qualified_id"
+  postOne2OneConversation charlie bob team1 "chit-chat" >>= assertSuccess
+  Internal.getConversation convId >>= assertSuccess
+
+  removeTeamCollaborator owner0 team0 charlie >>= assertSuccess
+
+  getMLSOne2OneConversation charlie alice >>= assertLabel 403 "not-connected"
+  postOne2OneConversation charlie alice team0 "chit-chat" >>= assertLabel 403 "no-team-member"
+  Internal.getConversation convId >>= assertLabel 404 "no-conversation"
+  getMLSOne2OneConversation charlie bob >>= assertSuccess
+
+testRemoveCollaboratorInO2OConnected :: (HasCallStack) => App ()
+testRemoveCollaboratorInO2OConnected = do
+  (owner0, team0, [alice]) <- createTeam OwnDomain 2
+
+  -- At the time of writing, it wasn't clear if this should be a bot instead.
+  bob <- randomUser OwnDomain def
+  connectTwoUsers alice bob
+
+  addTeamCollaborator owner0 team0 bob ["implicit_connection"] >>= assertSuccess
+
+  postOne2OneConversation bob alice team0 "chit-chat" >>= assertSuccess
+
+  removeTeamCollaborator owner0 team0 bob >>= assertSuccess
+
+  getMLSOne2OneConversation bob alice >>= assertSuccess
+
+testRemoveCollaboratorInO2O :: (HasCallStack) => App ()
+testRemoveCollaboratorInO2O = do
+  (owner0, team0, [alice]) <- createTeam OwnDomain 2
+
+  -- At the time of writing, it wasn't clear if this should be a bot instead.
+  bob <- randomUser OwnDomain def
+  addTeamCollaborator owner0 team0 bob ["implicit_connection"] >>= assertSuccess
+
+  teamConvId <-
+    postOne2OneConversation bob alice team0 "chit-chat" `bindResponse` \resp -> do
+      resp.status `shouldMatchInt` 201
+      resp.json %. "qualified_id"
+  Internal.getConversation teamConvId >>= assertSuccess
+
+  connectTwoUsers alice bob
+  personalConvId <- postConversation alice defProteus {qualifiedUsers = [bob]} >>= getJSON 201
+  Internal.getConversation personalConvId >>= assertSuccess
+
+  removeTeamCollaborator owner0 team0 bob >>= assertSuccess
+
+  postOne2OneConversation bob alice team0 "chit-chat" >>= assertLabel 403 "no-team-member"
+  Internal.getConversation teamConvId >>= assertLabel 404 "no-conversation"
+
+  getMLSOne2OneConversation bob alice >>= assertSuccess
+  Internal.getConversation personalConvId >>= assertSuccess
+
+testRemoveCollaboratorInTeamConversation :: (HasCallStack) => App ()
+testRemoveCollaboratorInTeamConversation = do
+  (owner, team, [alice, bob]) <- createTeam OwnDomain 3
+
+  conv <-
+    postConversation
+      owner
+      defProteus {team = Just team, qualifiedUsers = [alice, bob]}
+      >>= getJSON 201
+
+  withWebSockets [owner, alice, bob] $ \[wsOwner, wsAlice, wsBob] -> do
+    removeTeamCollaborator owner team bob >>= assertSuccess
+
+    bobId <- bob %. "qualified_id"
+    bobUnqualifiedId <- bobId %. "id"
+    let checkLeaveEvent :: (MakesValue a, HasCallStack) => a -> App ()
+        checkLeaveEvent evt = do
+          evt %. "payload.0.data.user" `shouldMatch` bobUnqualifiedId
+          evt %. "payload.0.team" `shouldMatch` team
+        checkRemoveEvent :: (MakesValue a, HasCallStack) => a -> App ()
+        checkRemoveEvent evt = do
+          evt %. "payload.0.data.user" `shouldMatch` bobUnqualifiedId
+          evt %. "payload.0.team" `shouldMatch` team
+        checkConvLeaveEvent :: (MakesValue a, HasCallStack) => a -> App ()
+        checkConvLeaveEvent evt = do
+          evt %. "payload.0.data.qualified_user_ids" `shouldMatch` [bobId]
+          evt %. "payload.0.team" `shouldMatch` team
+
+    awaitMatch isTeamMemberLeaveNotif wsOwner >>= checkLeaveEvent
+    awaitMatch isTeamMemberLeaveNotif wsAlice >>= checkRemoveEvent
+    awaitMatch isTeamMemberLeaveNotif wsBob >>= checkLeaveEvent
+    awaitMatch isTeamCollaboratorRemovedNotif wsOwner >>= checkRemoveEvent
+    awaitMatch isConvLeaveNotif wsAlice >>= checkConvLeaveEvent
+
+  getConversation alice conv `bindResponse` \resp -> do
+    resp.status `shouldMatchInt` 200
+    otherMember <- assertOne =<< asList (resp.json %. "members.others")
+    otherMember %. "qualified_id" `shouldNotMatch` (bob %. "qualified_id")
+
+  getConversation bob conv `bindResponse` \resp -> do
+    -- should be 404
+    resp.status `shouldMatchInt` 403
+    resp.json %. "label" `shouldMatch` "access-denied"
+
+  Internal.getConversation conv `bindResponse` \resp -> do
+    resp.status `shouldMatchInt` 200
+    otherMembers <- asList (resp.json %. "members.others")
+    traverse (%. "qualified_id") otherMembers `shouldMatchSet` traverse (%. "qualified_id") [owner, alice]

libs/wire-api/src/Wire/API/Event/Team.hs

@@ -125,6 +125,7 @@ data EventType
   | ConvDelete
   | CollaboratorAdd
   | AppCreate
+  | CollaboratorRemove
   deriving stock (Eq, Show, Generic)
   deriving (Arbitrary) via (GenericUniform EventType)
   deriving (FromJSON, ToJSON, S.ToSchema) via Schema EventType
@@ -142,7 +143,8 @@ instance ToSchema EventType where
           element "team.conversation-create" ConvCreate,
           element "team.conversation-delete" ConvDelete,
           element "team.collaborator-add" CollaboratorAdd,
-          element "team.app-create" AppCreate
+          element "team.app-create" AppCreate,
+          element "team.collaborator-remove" CollaboratorRemove
         ]
 
 --------------------------------------------------------------------------------
@@ -159,6 +161,7 @@ data EventData
   | EdConvDelete ConvId
   | EdCollaboratorAdd UserId [CollaboratorPermission]
   | EdAppCreate UserId
+  | EdCollaboratorRemove UserId
   deriving stock (Eq, Show, Generic)
 
 -- FUTUREWORK: this is outright wrong; see "Wire.API.Event.Conversation" on how to do this properly.
@@ -189,6 +192,7 @@ instance ToJSON EventData where
         "permissions" A..= perms
       ]
   toJSON (EdAppCreate usr) = A.object ["user" A..= usr]
+  toJSON (EdCollaboratorRemove usr) = A.object ["user" A..= usr]
 
 eventDataType :: EventData -> EventType
 eventDataType (EdTeamCreate _) = TeamCreate
@@ -201,6 +205,7 @@ eventDataType (EdConvCreate _) = ConvCreate
 eventDataType (EdConvDelete _) = ConvDelete
 eventDataType (EdCollaboratorAdd _ _) = CollaboratorAdd
 eventDataType (EdAppCreate _) = AppCreate
+eventDataType (EdCollaboratorRemove _) = CollaboratorRemove
 
 parseEventData :: EventType -> Maybe Value -> Parser EventData
 parseEventData MemberJoin Nothing = fail "missing event data for type 'team.member-join'"
@@ -215,11 +220,11 @@ parseEventData MemberLeave Nothing = fail "missing event data for type 'team.mem
 parseEventData MemberLeave (Just j) = do
   let f o = EdMemberLeave <$> o .: "user"
   withObject "member leave data" f j
-parseEventData ConvCreate Nothing = fail "missing event data for type 'team.conversation-create"
+parseEventData ConvCreate Nothing = fail "missing event data for type 'team.conversation-create'"
 parseEventData ConvCreate (Just j) = do
   let f o = EdConvCreate <$> o .: "conv"
   withObject "conversation create data" f j
-parseEventData ConvDelete Nothing = fail "missing event data for type 'team.conversation-delete"
+parseEventData ConvDelete Nothing = fail "missing event data for type 'team.conversation-delete'"
 parseEventData ConvDelete (Just j) = do
   let f o = EdConvDelete <$> o .: "conv"
   withObject "conversation delete data" f j
@@ -235,6 +240,10 @@ parseEventData AppCreate Nothing = fail "missing event data for type 'team.app-c
 parseEventData AppCreate (Just j) = do
   let f o = EdAppCreate <$> o .: "user"
   withObject "app create data" f j
+parseEventData CollaboratorRemove Nothing = fail "missing event data for type 'team.collaborator-remove"
+parseEventData CollaboratorRemove (Just j) = do
+  let f o = EdCollaboratorRemove <$> o .: "user"
+  withObject "collaborator remove data" f j
 parseEventData _ Nothing = pure EdTeamDelete
 parseEventData t (Just _) = fail $ "unexpected event data for type " <> show t
 
@@ -250,5 +259,6 @@ genEventData = \case
   ConvDelete -> EdConvDelete <$> arbitrary
   CollaboratorAdd -> EdCollaboratorAdd <$> arbitrary <*> arbitrary
   AppCreate -> EdAppCreate <$> arbitrary
+  CollaboratorRemove -> EdCollaboratorRemove <$> arbitrary
 
 makeLenses ''Event

libs/wire-api/src/Wire/API/Routes/Public/Galley/TeamMember.hs

@@ -207,6 +207,19 @@ type TeamMemberAPI =
                     "CSV of team members"
                     CSV
            )
+    :<|> Named
+           "remove-team-collaborator"
+           ( Summary "Remove a collaborator from the team."
+               :> CanThrow OperationDenied
+               :> CanThrow 'NotATeamMember
+               :> From 'V12
+               :> ZLocalUser
+               :> "teams"
+               :> Capture "tid" TeamId
+               :> "collaborators"
+               :> Capture "uid" UserId
+               :> MultiVerb1 'DELETE '[JSON] (RespondEmpty 200 "")
+           )
 
 type TeamMemberDeleteResultResponseType =
   '[ RespondEmpty 202 "Team member scheduled for deletion",