WPB-5695 Enforce group conversation permission for external partner role (#3788)

Author: battermann

changelog.d/3-bug-fixes/WPB-5695

@@ -0,0 +1 @@
+Enforce external partner permissions on the backend

integration/integration.cabal

@@ -117,6 +117,7 @@ library
     Test.Conversation
     Test.Demo
     Test.Errors
+    Test.ExternalPartner
     Test.Federation
     Test.Federator
     Test.MessageTimer

integration/test/API/Galley.hs

@@ -416,6 +416,7 @@ getConversationCode user conv mbZHost = do
         & maybe id zHost mbZHost
     )
 
+-- https://staging-nginz-https.zinfra.io/v5/api/swagger-ui/#/default/put_conversations__cnv_domain___cnv__name
 changeConversationName ::
   (HasCallStack, MakesValue user, MakesValue conv, MakesValue name) =>
   user ->

integration/test/SetupHelpers.hs

@@ -29,7 +29,7 @@ deleteUser :: (HasCallStack, MakesValue user) => user -> App ()
 deleteUser user = bindResponse (API.Brig.deleteUser user) $ \resp -> do
   resp.status `shouldMatchInt` 200
 
--- | returns (user, team id)
+-- | returns (owner, team id, members)
 createTeam :: (HasCallStack, MakesValue domain) => domain -> Int -> App (Value, String, [Value])
 createTeam domain memberCount = do
   res <- createUser domain def {team = True}

integration/test/Test/ExternalPartner.hs

@@ -0,0 +1,82 @@
+{-# OPTIONS_GHC -Wno-ambiguous-fields #-}
+
+-- This file is part of the Wire Server implementation.
+--
+-- Copyright (C) 2023 Wire Swiss GmbH <opensource@wire.com>
+--
+-- 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 <https://www.gnu.org/licenses/>.
+
+module Test.ExternalPartner where
+
+import API.Galley
+import GHC.Stack
+import MLS.Util
+import SetupHelpers
+import Testlib.Prelude
+
+testExternalPartnerPermissions :: HasCallStack => App ()
+testExternalPartnerPermissions = do
+  (owner, tid, u1 : u2 : u3 : _) <- createTeam OwnDomain 4
+
+  partner <- createTeamMemberWithRole owner tid "partner"
+
+  -- a partner should not be able to create conversation with 2 additional users or more
+  void $ postConversation partner (defProteus {team = Just tid, qualifiedUsers = [u1, u2]}) >>= getJSON 403
+
+  do
+    -- a partner can create a one to one conversation with a user from the same team
+    conv <- postConversation partner (defProteus {team = Just tid, qualifiedUsers = [u1]}) >>= getJSON 201
+
+    -- they should not be able to add another team member to the one to one conversation
+    bindResponse (addMembers partner conv def {users = [u2]}) $ \resp -> do
+      resp.status `shouldMatchInt` 403
+
+    -- the other member in the conversation gets deleted
+    deleteUser u1
+
+    -- now they still should not be able to add another member
+    bindResponse (addMembers partner conv def {users = [u2]}) $ \resp -> do
+      resp.status `shouldMatchInt` 403
+
+  do
+    -- also an external partner cannot add someone to a conversation, even if it is empty
+    conv <- postConversation partner (defProteus {team = Just tid}) >>= getJSON 201
+    bindResponse (addMembers partner conv def {users = [u3]}) $ \resp -> do
+      resp.status `shouldMatchInt` 403
+
+testExternalPartnerPermissionsMls :: HasCallStack => App ()
+testExternalPartnerPermissionsMls = do
+  -- external partners should not be able to create (MLS) conversations
+  (owner, tid, _) <- createTeam OwnDomain 2
+  bobExt <- createTeamMemberWithRole owner tid "partner"
+  bobExtClient <- createMLSClient def bobExt
+  bindResponse (postConversation bobExtClient defMLS) $ \resp -> do
+    resp.status `shouldMatchInt` 403
+
+testExternalPartnerPermissionMlsOne2One :: HasCallStack => App ()
+testExternalPartnerPermissionMlsOne2One = do
+  (owner, tid, alice : _) <- createTeam OwnDomain 2
+  bobExternal <- createTeamMemberWithRole owner tid "partner"
+  void $ getMLSOne2OneConversation alice bobExternal >>= getJSON 200
+
+testExternalPartnerPermissionsConvName :: HasCallStack => App ()
+testExternalPartnerPermissionsConvName = do
+  (owner, tid, u1 : _) <- createTeam OwnDomain 2
+
+  partner <- createTeamMemberWithRole owner tid "partner"
+
+  conv <- postConversation partner (defProteus {team = Just tid, qualifiedUsers = [u1]}) >>= getJSON 201
+
+  bindResponse (changeConversationName partner conv "new name") $ \resp -> do
+    resp.status `shouldMatchInt` 403

libs/galley-types/src/Galley/Types/Teams.hs

@@ -134,9 +134,9 @@ rolePerms RoleAdmin =
 rolePerms RoleMember =
   rolePerms RoleExternalPartner
     <> Set.fromList
-      [ DoNotUseDeprecatedDeleteConversation,
-        DoNotUseDeprecatedAddRemoveConvMember,
-        DoNotUseDeprecatedModifyConvName,
+      [ DeleteConversation,
+        AddRemoveConvMember,
+        ModifyConvName,
         GetMemberPermissions
       ]
 rolePerms RoleExternalPartner =

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

@@ -117,7 +117,7 @@ serviceWhitelistPermissions =
   Set.fromList
     [ AddTeamMember,
       RemoveTeamMember,
-      DoNotUseDeprecatedAddRemoveConvMember,
+      AddRemoveConvMember,
       SetTeamData
     ]
 
@@ -127,11 +127,20 @@ serviceWhitelistPermissions =
 -- | Team-level permission.  Analog to conversation-level 'Action'.
 data Perm
   = CreateConversation
-  | DoNotUseDeprecatedDeleteConversation -- NOTE: This gets now overruled by conv level checks
+  | -- NOTE: This may get overruled by conv level checks in case those are more restrictive
+    -- We currently cannot get rid of this team-level permission in favor of the conv-level action
+    -- because it is used for e.g. for the team role 'RoleExternalPartner'
+    DeleteConversation
   | AddTeamMember
   | RemoveTeamMember
-  | DoNotUseDeprecatedAddRemoveConvMember -- NOTE: This gets now overruled by conv level checks
-  | DoNotUseDeprecatedModifyConvName -- NOTE: This gets now overruled by conv level checks
+  | -- NOTE: This may get overruled by conv level checks in case those are more restrictive
+    -- We currently cannot get rid of this team-level permission in favor of the conv-level action
+    -- because it is used for e.g. for the team role 'RoleExternalPartner'
+    AddRemoveConvMember
+  | -- NOTE: This may get overruled by conv level checks in case those are more restrictive
+    -- We currently cannot get rid of this team-level permission in favor of the conv-level action
+    -- because it is used for e.g. for the team role 'RoleExternalPartner'
+    ModifyConvName
   | GetBilling
   | SetBilling
   | SetTeamData
@@ -159,11 +168,11 @@ intToPerms n =
 
 permToInt :: Perm -> Word64
 permToInt CreateConversation = 0x0001
-permToInt DoNotUseDeprecatedDeleteConversation = 0x0002
+permToInt DeleteConversation = 0x0002
 permToInt AddTeamMember = 0x0004
 permToInt RemoveTeamMember = 0x0008
-permToInt DoNotUseDeprecatedAddRemoveConvMember = 0x0010
-permToInt DoNotUseDeprecatedModifyConvName = 0x0020
+permToInt AddRemoveConvMember = 0x0010
+permToInt ModifyConvName = 0x0020
 permToInt GetBilling = 0x0040
 permToInt SetBilling = 0x0080
 permToInt SetTeamData = 0x0100
@@ -174,11 +183,11 @@ permToInt SetMemberPermissions = 0x1000
 
 intToPerm :: Word64 -> Maybe Perm
 intToPerm 0x0001 = Just CreateConversation
-intToPerm 0x0002 = Just DoNotUseDeprecatedDeleteConversation
+intToPerm 0x0002 = Just DeleteConversation
 intToPerm 0x0004 = Just AddTeamMember
 intToPerm 0x0008 = Just RemoveTeamMember
-intToPerm 0x0010 = Just DoNotUseDeprecatedAddRemoveConvMember
-intToPerm 0x0020 = Just DoNotUseDeprecatedModifyConvName
+intToPerm 0x0010 = Just AddRemoveConvMember
+intToPerm 0x0020 = Just ModifyConvName
 intToPerm 0x0040 = Just GetBilling
 intToPerm 0x0080 = Just SetBilling
 intToPerm 0x0100 = Just SetTeamData

libs/wire-api/test/golden/Test/Wire/API/Golden/Generated/Event_team.hs

@@ -233,11 +233,11 @@ testObject_Event_team_18 =
                 { _self =
                     fromList
                       [ CreateConversation,
-                        DoNotUseDeprecatedDeleteConversation,
+                        DeleteConversation,
                         AddTeamMember,
                         RemoveTeamMember,
-                        DoNotUseDeprecatedAddRemoveConvMember,
-                        DoNotUseDeprecatedModifyConvName,
+                        AddRemoveConvMember,
+                        ModifyConvName,
                         GetBilling,
                         SetBilling,
                         SetTeamData,
@@ -249,11 +249,11 @@ testObject_Event_team_18 =
                   _copy =
                     fromList
                       [ CreateConversation,
-                        DoNotUseDeprecatedDeleteConversation,
+                        DeleteConversation,
                         AddTeamMember,
                         RemoveTeamMember,
-                        DoNotUseDeprecatedAddRemoveConvMember,
-                        DoNotUseDeprecatedModifyConvName,
+                        AddRemoveConvMember,
+                        ModifyConvName,
                         GetBilling,
                         GetMemberPermissions,
                         SetMemberPermissions,
@@ -275,20 +275,20 @@ testObject_Event_team_19 =
             ( Permissions
                 { _self =
                     fromList
-                      [ DoNotUseDeprecatedDeleteConversation,
+                      [ DeleteConversation,
                         RemoveTeamMember,
-                        DoNotUseDeprecatedAddRemoveConvMember,
-                        DoNotUseDeprecatedModifyConvName,
+                        AddRemoveConvMember,
+                        ModifyConvName,
                         GetBilling,
                         SetBilling,
                         GetMemberPermissions,
                         GetTeamConversations
                       ],
                   _copy =
                     fromList
-                      [ DoNotUseDeprecatedDeleteConversation,
+                      [ DeleteConversation,
                         RemoveTeamMember,
-                        DoNotUseDeprecatedAddRemoveConvMember,
+                        AddRemoveConvMember,
                         GetBilling,
                         SetBilling,
                         GetMemberPermissions,

libs/wire-api/test/golden/Test/Wire/API/Golden/Generated/NewTeamMember_team.hs

@@ -27,15 +27,15 @@ import Imports (Maybe (Just, Nothing), fromJust)
 import Wire.API.Team.Member (NewTeamMember, mkNewTeamMember)
 import Wire.API.Team.Permission
   ( Perm
-      ( AddTeamMember,
+      ( AddRemoveConvMember,
+        AddTeamMember,
         CreateConversation,
+        DeleteConversation,
         DeleteTeam,
-        DoNotUseDeprecatedAddRemoveConvMember,
-        DoNotUseDeprecatedDeleteConversation,
-        DoNotUseDeprecatedModifyConvName,
         GetBilling,
         GetMemberPermissions,
         GetTeamConversations,
+        ModifyConvName,
         RemoveTeamMember,
         SetBilling,
         SetMemberPermissions,
@@ -63,13 +63,13 @@ testObject_NewTeamMember_team_2 =
         { _self =
             fromList
               [ CreateConversation,
-                DoNotUseDeprecatedDeleteConversation,
+                DeleteConversation,
                 AddTeamMember,
                 RemoveTeamMember,
-                DoNotUseDeprecatedAddRemoveConvMember,
-                DoNotUseDeprecatedModifyConvName
+                AddRemoveConvMember,
+                ModifyConvName
               ],
-          _copy = fromList [DoNotUseDeprecatedDeleteConversation, DoNotUseDeprecatedAddRemoveConvMember]
+          _copy = fromList [DeleteConversation, AddRemoveConvMember]
         }
     )
     ( Just
@@ -85,8 +85,8 @@ testObject_NewTeamMember_team_3 =
     ( Permissions
         { _self =
             fromList
-              [CreateConversation, DoNotUseDeprecatedDeleteConversation, RemoveTeamMember, GetBilling, DeleteTeam],
-          _copy = fromList [CreateConversation, DoNotUseDeprecatedDeleteConversation, GetBilling]
+              [CreateConversation, DeleteConversation, RemoveTeamMember, GetBilling, DeleteTeam],
+          _copy = fromList [CreateConversation, DeleteConversation, GetBilling]
         }
     )
     ( Just
@@ -124,7 +124,7 @@ testObject_NewTeamMember_team_6 =
     ( Permissions
         { _self =
             fromList
-              [CreateConversation, DoNotUseDeprecatedDeleteConversation, GetBilling, SetTeamData, SetMemberPermissions],
+              [CreateConversation, DeleteConversation, GetBilling, SetTeamData, SetMemberPermissions],
           _copy = fromList [CreateConversation, GetBilling]
         }
     )
@@ -141,7 +141,7 @@ testObject_NewTeamMember_team_7 =
     ( Permissions
         { _self =
             fromList
-              [AddTeamMember, RemoveTeamMember, DoNotUseDeprecatedModifyConvName, GetTeamConversations, DeleteTeam],
+              [AddTeamMember, RemoveTeamMember, ModifyConvName, GetTeamConversations, DeleteTeam],
           _copy = fromList [AddTeamMember]
         }
     )
@@ -156,8 +156,8 @@ testObject_NewTeamMember_team_8 =
   mkNewTeamMember
     (Id (fromJust (UUID.fromString "00000008-0000-0003-0000-000200000003")))
     ( Permissions
-        { _self = fromList [DoNotUseDeprecatedModifyConvName],
-          _copy = fromList [DoNotUseDeprecatedModifyConvName]
+        { _self = fromList [ModifyConvName],
+          _copy = fromList [ModifyConvName]
         }
     )
     ( Just
@@ -193,7 +193,7 @@ testObject_NewTeamMember_team_11 =
   mkNewTeamMember
     (Id (fromJust (UUID.fromString "00000006-0000-0005-0000-000000000002")))
     ( Permissions
-        { _self = fromList [CreateConversation, DoNotUseDeprecatedModifyConvName, SetTeamData],
+        { _self = fromList [CreateConversation, ModifyConvName, SetTeamData],
           _copy = fromList []
         }
     )
@@ -215,8 +215,8 @@ testObject_NewTeamMember_team_13 =
   mkNewTeamMember
     (Id (fromJust (UUID.fromString "00000002-0000-0004-0000-000600000001")))
     ( Permissions
-        { _self = fromList [AddTeamMember, DoNotUseDeprecatedAddRemoveConvMember, SetTeamData, GetTeamConversations],
-          _copy = fromList [AddTeamMember, DoNotUseDeprecatedAddRemoveConvMember, GetTeamConversations]
+        { _self = fromList [AddTeamMember, AddRemoveConvMember, SetTeamData, GetTeamConversations],
+          _copy = fromList [AddTeamMember, AddRemoveConvMember, GetTeamConversations]
         }
     )
     Nothing
@@ -228,7 +228,7 @@ testObject_NewTeamMember_team_14 =
     ( Permissions
         { _self =
             fromList
-              [CreateConversation, DoNotUseDeprecatedDeleteConversation, DoNotUseDeprecatedModifyConvName, GetBilling],
+              [CreateConversation, DeleteConversation, ModifyConvName, GetBilling],
           _copy = fromList []
         }
     )
@@ -291,8 +291,8 @@ testObject_NewTeamMember_team_19 =
   mkNewTeamMember
     (Id (fromJust (UUID.fromString "00000004-0000-0005-0000-000100000008")))
     ( Permissions
-        { _self = fromList [DoNotUseDeprecatedDeleteConversation, RemoveTeamMember, SetBilling, SetMemberPermissions],
-          _copy = fromList [DoNotUseDeprecatedDeleteConversation, SetBilling]
+        { _self = fromList [DeleteConversation, RemoveTeamMember, SetBilling, SetMemberPermissions],
+          _copy = fromList [DeleteConversation, SetBilling]
         }
     )
     Nothing
@@ -305,8 +305,8 @@ testObject_NewTeamMember_team_20 =
         { _self =
             fromList
               [ AddTeamMember,
-                DoNotUseDeprecatedAddRemoveConvMember,
-                DoNotUseDeprecatedModifyConvName,
+                AddRemoveConvMember,
+                ModifyConvName,
                 SetBilling,
                 GetMemberPermissions,
                 GetTeamConversations