From 58e4e0bb25bebe8a71b4468c37e3ce092e007df8 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 14 Jan 2019 12:26:33 +0100 Subject: [PATCH 1/9] Update `Perm` data type. --- libs/galley-types/src/Galley/Types/Teams.hs | 12 ++++++------ services/galley/src/Galley/Intra/Journal.hs | 2 +- services/galley/test/integration/API/Teams.hs | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/libs/galley-types/src/Galley/Types/Teams.hs b/libs/galley-types/src/Galley/Types/Teams.hs index 20f05da2412..12e2c268736 100644 --- a/libs/galley-types/src/Galley/Types/Teams.hs +++ b/libs/galley-types/src/Galley/Types/Teams.hs @@ -223,8 +223,8 @@ data Perm = | RemoveTeamMember | AddConversationMember | RemoveConversationMember - | GetBilling - | SetBilling + | ModifyConversationMetadata + | CRUDBilling | SetTeamData | GetMemberPermissions | SetMemberPermissions @@ -400,8 +400,8 @@ permToInt AddTeamMember = 0x0004 permToInt RemoveTeamMember = 0x0008 permToInt AddConversationMember = 0x0010 permToInt RemoveConversationMember = 0x0020 -permToInt GetBilling = 0x0040 -permToInt SetBilling = 0x0080 +permToInt ModifyConversationMetadata = 0x0040 +permToInt CRUDBilling = 0x0080 permToInt SetTeamData = 0x0100 permToInt GetMemberPermissions = 0x0200 permToInt GetTeamConversations = 0x0400 @@ -415,8 +415,8 @@ intToPerm 0x0004 = Just AddTeamMember intToPerm 0x0008 = Just RemoveTeamMember intToPerm 0x0010 = Just AddConversationMember intToPerm 0x0020 = Just RemoveConversationMember -intToPerm 0x0040 = Just GetBilling -intToPerm 0x0080 = Just SetBilling +intToPerm 0x0040 = Just ModifyConversationMetadata +intToPerm 0x0080 = Just CRUDBilling intToPerm 0x0100 = Just SetTeamData intToPerm 0x0200 = Just GetMemberPermissions intToPerm 0x0400 = Just GetTeamConversations diff --git a/services/galley/src/Galley/Intra/Journal.hs b/services/galley/src/Galley/Intra/Journal.hs index 7c4a7097104..e38a3a255d9 100644 --- a/services/galley/src/Galley/Intra/Journal.hs +++ b/services/galley/src/Galley/Intra/Journal.hs @@ -52,5 +52,5 @@ journalEvent typ tid dat tim = view aEnv >>= \mEnv -> for_ mEnv $ \e -> do evData :: [TeamMember] -> Maybe Currency.Alpha -> TeamEvent'EventData evData mems cur = TeamEvent'EventData count (toBytes <$> uids) (pack . show <$> cur) [] where - uids = view userId <$> filter (`hasPermission` SetBilling) mems + uids = view userId <$> filter (`hasPermission` CRUDBilling) mems count = fromIntegral $ length mems diff --git a/services/galley/test/integration/API/Teams.hs b/services/galley/test/integration/API/Teams.hs index 56f49aef794..92dccd805ec 100644 --- a/services/galley/test/integration/API/Teams.hs +++ b/services/galley/test/integration/API/Teams.hs @@ -241,7 +241,7 @@ testAddTeamMemberInternal :: Galley -> Brig -> Cannon -> Maybe Aws.Env -> Http ( testAddTeamMemberInternal g b c a = do owner <- Util.randomUser b tid <- Util.createTeam g "foo" owner [] - let p1 = Util.symmPermissions [GetBilling] -- permissions are irrelevant on internal endpoint + let p1 = Util.symmPermissions [CRUDBilling] -- permissions are irrelevant on internal endpoint mem1 <- newTeamMember' p1 <$> Util.randomUser b WS.bracketRN c [owner, mem1^.userId] $ \[wsOwner, wsMem1] -> do From b2153e1fd910c194d624ffba304910ebfd47714a Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 14 Jan 2019 12:41:40 +0100 Subject: [PATCH 2/9] Cleanup (whitespace). --- libs/galley-types/src/Galley/Types/Teams.hs | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/libs/galley-types/src/Galley/Types/Teams.hs b/libs/galley-types/src/Galley/Types/Teams.hs index 12e2c268736..7d0f5aa1344 100644 --- a/libs/galley-types/src/Galley/Types/Teams.hs +++ b/libs/galley-types/src/Galley/Types/Teams.hs @@ -394,19 +394,19 @@ isTeamOwner :: TeamMember -> Bool isTeamOwner tm = fullPermissions == (tm^.permissions) permToInt :: Perm -> Word64 -permToInt CreateConversation = 0x0001 -permToInt DeleteConversation = 0x0002 -permToInt AddTeamMember = 0x0004 -permToInt RemoveTeamMember = 0x0008 -permToInt AddConversationMember = 0x0010 -permToInt RemoveConversationMember = 0x0020 +permToInt CreateConversation = 0x0001 +permToInt DeleteConversation = 0x0002 +permToInt AddTeamMember = 0x0004 +permToInt RemoveTeamMember = 0x0008 +permToInt AddConversationMember = 0x0010 +permToInt RemoveConversationMember = 0x0020 permToInt ModifyConversationMetadata = 0x0040 -permToInt CRUDBilling = 0x0080 -permToInt SetTeamData = 0x0100 -permToInt GetMemberPermissions = 0x0200 -permToInt GetTeamConversations = 0x0400 -permToInt DeleteTeam = 0x0800 -permToInt SetMemberPermissions = 0x1000 +permToInt CRUDBilling = 0x0080 +permToInt SetTeamData = 0x0100 +permToInt GetMemberPermissions = 0x0200 +permToInt GetTeamConversations = 0x0400 +permToInt DeleteTeam = 0x0800 +permToInt SetMemberPermissions = 0x1000 intToPerm :: Word64 -> Maybe Perm intToPerm 0x0001 = Just CreateConversation From 11442bb0687f51f862cf4b52d038b4f9f02d21fa Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 14 Jan 2019 13:30:53 +0100 Subject: [PATCH 3/9] Check for new permission where applicable. --- services/galley/src/Galley/API/Update.hs | 7 ++++++- services/galley/src/Galley/API/Util.hs | 13 ++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/services/galley/src/Galley/API/Update.hs b/services/galley/src/Galley/API/Update.hs index 32a60384148..1add7e41f43 100644 --- a/services/galley/src/Galley/API/Update.hs +++ b/services/galley/src/Galley/API/Update.hs @@ -1,11 +1,13 @@ {-# LANGUAGE ConstraintKinds #-} {-# LANGUAGE DataKinds #-} {-# LANGUAGE FlexibleContexts #-} +{-# LANGUAGE LambdaCase #-} {-# LANGUAGE MultiWayIf #-} {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE TupleSections #-} {-# LANGUAGE TypeFamilies #-} {-# LANGUAGE TypeOperators #-} +{-# LANGUAGE ViewPatterns #-} module Galley.API.Update ( -- * Managing Conversations @@ -122,7 +124,7 @@ updateConversationAccess (usr ::: zcon ::: cnv ::: req ::: _ ) = do ensureGroupConv conv -- Team conversations incur another round of checks case Data.convTeam conv of - Just tid -> checkTeamConv tid + Just tid -> checkTeamConv tid >> permissionCheckConv usr cnv ModifyConversationMetadata Nothing -> when (targetRole == TeamAccessRole) $ throwM invalidTargetAccess -- When there is no update to be done, we return 204; otherwise we go -- with 'uncheckedUpdateConversationAccess', which will potentially kick @@ -208,6 +210,7 @@ uncheckedUpdateConversationAccess body usr zcon conv (currentAccess, targetAcces updateConversationReceiptMode :: UserId ::: ConnId ::: ConvId ::: Request ::: JSON ::: JSON -> Galley Response updateConversationReceiptMode (usr ::: zcon ::: cnv ::: req ::: _ ::: _) = do ConversationReceiptModeUpdate target <- fromBody req invalidPayload + permissionCheckConv usr cnv ModifyConversationMetadata (bots, users) <- botsAndUsers <$> Data.members cnv current <- Data.lookupReceiptMode cnv if current == Just target @@ -232,6 +235,7 @@ updateConversationMessageTimer (usr ::: zcon ::: cnv ::: req ::: _ ) = do conv <- Data.conversation cnv >>= ifNothing convNotFound ensureGroupConv conv traverse_ ensureTeamMember $ Data.convTeam conv -- only team members can change the timer + permissionCheckConv usr cnv ModifyConversationMetadata let currentTimer = Data.convMessageTimer conv if currentTimer == messageTimer then return $ empty & setStatus status204 @@ -499,6 +503,7 @@ newMessage usr con cnv msg now (m, c, t) ~(toBots, toUsers) = updateConversation :: UserId ::: ConnId ::: ConvId ::: Request ::: JSON -> Galley Response updateConversation (zusr ::: zcon ::: cnv ::: req ::: _) = do body <- fromBody req invalidPayload + permissionCheckConv zusr cnv ModifyConversationMetadata alive <- Data.isConvAlive cnv unless alive $ do Data.deleteConversation cnv diff --git a/services/galley/src/Galley/API/Util.hs b/services/galley/src/Galley/API/Util.hs index 9bfc4fe2e5a..8db48039da3 100644 --- a/services/galley/src/Galley/API/Util.hs +++ b/services/galley/src/Galley/API/Util.hs @@ -1,7 +1,8 @@ {-# LANGUAGE DataKinds #-} +{-# LANGUAGE LambdaCase #-} {-# LANGUAGE MultiWayIf #-} -{-# LANGUAGE ViewPatterns #-} {-# LANGUAGE OverloadedStrings #-} +{-# LANGUAGE ViewPatterns #-} module Galley.API.Util where @@ -72,6 +73,9 @@ bindingTeamMembers tid = do Binding -> Data.teamMembers tid NonBinding -> throwM nonBindingTeam +-- | Pick a team member with a given user id from some team members. If the filter comes up empty, +-- throw 'noTeamMember'; if the user is found and does not have the given permission, throw +-- 'operationDenied'. Otherwise, return the found user. permissionCheck :: Foldable m => UserId -> Perm -> m TeamMember -> Galley TeamMember permissionCheck u p t = case find ((u ==) . view userId) t of @@ -81,6 +85,13 @@ permissionCheck u p t = pure m Nothing -> throwM noTeamMember +permissionCheckConv :: UserId -> ConvId -> Perm -> Galley () +permissionCheckConv zusr cnv perm = Data.conversation cnv >>= \case + Just cnv' -> case Data.convTeam cnv' of + Just tid -> void $ permissionCheck zusr perm =<< Data.teamMembers tid + Nothing -> pure () + Nothing -> throwM convNotFound + -- | Try to accept a 1-1 conversation, promoting connect conversations as appropriate. acceptOne2One :: UserId -> Data.Conversation -> Maybe ConnId -> Galley Data.Conversation acceptOne2One usr conv conn = case Data.convType conv of From 97c10e2612ae2b01e7fd285a072584c631f8f633 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 14 Jan 2019 16:34:43 +0100 Subject: [PATCH 4/9] no, they are not! --- services/galley/test/integration/API/Teams.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/galley/test/integration/API/Teams.hs b/services/galley/test/integration/API/Teams.hs index 92dccd805ec..f16d33b1c12 100644 --- a/services/galley/test/integration/API/Teams.hs +++ b/services/galley/test/integration/API/Teams.hs @@ -241,7 +241,7 @@ testAddTeamMemberInternal :: Galley -> Brig -> Cannon -> Maybe Aws.Env -> Http ( testAddTeamMemberInternal g b c a = do owner <- Util.randomUser b tid <- Util.createTeam g "foo" owner [] - let p1 = Util.symmPermissions [CRUDBilling] -- permissions are irrelevant on internal endpoint + let p1 = Util.symmPermissions [AddTeamMember] -- permissions are irrelevant on internal endpoint mem1 <- newTeamMember' p1 <$> Util.randomUser b WS.bracketRN c [owner, mem1^.userId] $ \[wsOwner, wsMem1] -> do From 09bac88a0923e8b43fb91e8977c2cfffa2dc69c1 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 14 Jan 2019 20:23:01 +0100 Subject: [PATCH 5/9] Add test case. --- services/galley/test/integration/API/Teams.hs | 26 +++++++++++++++++++ services/galley/test/integration/API/Util.hs | 8 ++++++ 2 files changed, 34 insertions(+) diff --git a/services/galley/test/integration/API/Teams.hs b/services/galley/test/integration/API/Teams.hs index f16d33b1c12..3f78ae66ce8 100644 --- a/services/galley/test/integration/API/Teams.hs +++ b/services/galley/test/integration/API/Teams.hs @@ -62,6 +62,8 @@ tests s = testGroup "Teams API" , test s "add managed conversation through public endpoint (fail)" testAddManagedConv , test s "add managed team conversation ignores given users" testAddTeamConvWithUsers , test s "add team member to conversation without connection" testAddTeamMemberToConv + , test s "update conversation as member" (testUpdateTeamConv True) + , test s "update conversation as collaborator" (testUpdateTeamConv False) , test s "delete non-binding team" testDeleteTeam , test s "delete binding team (owner has passwd)" (testDeleteBindingTeam True) , test s "delete binding team (owner has no passwd)" (testDeleteBindingTeam False) @@ -482,6 +484,30 @@ testAddTeamMemberToConv g b _ _ = do const 403 === statusCode const "operation-denied" === (Error.label . Util.decodeBody' "error label") +testUpdateTeamConv :: Bool -> Galley -> Brig -> Cannon -> Maybe Aws.Env -> Http () +testUpdateTeamConv roleIsMember g b _ _ = do + owner <- Util.randomUser b + member <- Util.randomUser b + let perms = Util.symmPermissions $ if roleIsMember then permsMember else permsCollaborator + permsMember = permsCollaborator <> + [ DeleteConversation + , AddConversationMember + , RemoveConversationMember + , ModifyConversationMetadata + , GetMemberPermissions + ] + permsCollaborator = + [ CreateConversation + , GetTeamConversations + ] + Util.connectUsers b owner (list1 member []) + tid <- Util.createTeam g "foo" owner [newTeamMember member perms Nothing] + cid <- Util.createManagedConv g owner tid [member] (Just "gossip") Nothing Nothing + resp <- updateTeamConv g member cid (ConversationRename "not gossip") + liftIO $ if roleIsMember + then assertEqual "status" (statusCode resp) 200 + else assertEqual "status" (statusCode resp) 401 + testDeleteTeam :: Galley -> Brig -> Cannon -> Maybe Aws.Env -> Http () testDeleteTeam g b c a = do owner <- Util.randomUser b diff --git a/services/galley/test/integration/API/Util.hs b/services/galley/test/integration/API/Util.hs index 8815feb58d7..b0c4e7f53f3 100644 --- a/services/galley/test/integration/API/Util.hs +++ b/services/galley/test/integration/API/Util.hs @@ -148,6 +148,14 @@ createTeamConvAccessRaw g u tid us name acc role mtimer = do . json conv ) +updateTeamConv :: Galley -> UserId -> ConvId -> ConversationRename -> Http ResponseLBS +updateTeamConv g zusr convid upd = do + put ( g + . paths ["/conversations", toByteString' convid] + . zUser zusr + . json upd + ) + createManagedConv :: HasCallStack => Galley -> UserId -> TeamId -> [UserId] -> Maybe Text -> Maybe (Set Access) -> Maybe Milliseconds -> Http ConvId createManagedConv g u tid us name acc mtimer = do let tinfo = ConvTeamInfo tid True From 5080d40675331fbf136c7a20c5c3d32c3f73a887 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 14 Jan 2019 23:37:56 +0100 Subject: [PATCH 6/9] Fixup --- services/galley/test/integration/API/Teams.hs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/services/galley/test/integration/API/Teams.hs b/services/galley/test/integration/API/Teams.hs index 3f78ae66ce8..3507bec3c6b 100644 --- a/services/galley/test/integration/API/Teams.hs +++ b/services/galley/test/integration/API/Teams.hs @@ -504,9 +504,7 @@ testUpdateTeamConv roleIsMember g b _ _ = do tid <- Util.createTeam g "foo" owner [newTeamMember member perms Nothing] cid <- Util.createManagedConv g owner tid [member] (Just "gossip") Nothing Nothing resp <- updateTeamConv g member cid (ConversationRename "not gossip") - liftIO $ if roleIsMember - then assertEqual "status" (statusCode resp) 200 - else assertEqual "status" (statusCode resp) 401 + liftIO $ assertEqual "status" (if roleIsMember then 200 else 403) (statusCode resp) testDeleteTeam :: Galley -> Brig -> Cannon -> Maybe Aws.Env -> Http () testDeleteTeam g b c a = do From cc7549db0fd1e47d2cce195a0e0618a0a97b6262 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 14 Jan 2019 23:46:49 +0100 Subject: [PATCH 7/9] Fixup --- services/galley/test/integration/API/Teams.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/galley/test/integration/API/Teams.hs b/services/galley/test/integration/API/Teams.hs index 3507bec3c6b..5fc14ff303e 100644 --- a/services/galley/test/integration/API/Teams.hs +++ b/services/galley/test/integration/API/Teams.hs @@ -502,7 +502,7 @@ testUpdateTeamConv roleIsMember g b _ _ = do ] Util.connectUsers b owner (list1 member []) tid <- Util.createTeam g "foo" owner [newTeamMember member perms Nothing] - cid <- Util.createManagedConv g owner tid [member] (Just "gossip") Nothing Nothing + cid <- Util.createTeamConv g owner tid [member] (Just "gossip") Nothing Nothing resp <- updateTeamConv g member cid (ConversationRename "not gossip") liftIO $ assertEqual "status" (if roleIsMember then 200 else 403) (statusCode resp) From adadccac52b8ec4832a4ea685894155adaf8fdff Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 15 Jan 2019 11:14:53 +0100 Subject: [PATCH 8/9] Fixup --- services/galley/test/integration/API/Util.hs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/galley/test/integration/API/Util.hs b/services/galley/test/integration/API/Util.hs index b0c4e7f53f3..36564ce9940 100644 --- a/services/galley/test/integration/API/Util.hs +++ b/services/galley/test/integration/API/Util.hs @@ -153,6 +153,8 @@ updateTeamConv g zusr convid upd = do put ( g . paths ["/conversations", toByteString' convid] . zUser zusr + . zConn "conn" + . zType "access" . json upd ) From e62a698f42e9ccc1abc9d26cfb012b0f4d469391 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 15 Jan 2019 11:15:05 +0100 Subject: [PATCH 9/9] Cleanup --- services/galley/src/Galley/API/Update.hs | 9 +++++---- services/galley/src/Galley/API/Util.hs | 6 ++++-- services/galley/test/integration/API/Util.hs | 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/services/galley/src/Galley/API/Update.hs b/services/galley/src/Galley/API/Update.hs index 1add7e41f43..5205f2c450d 100644 --- a/services/galley/src/Galley/API/Update.hs +++ b/services/galley/src/Galley/API/Update.hs @@ -124,7 +124,8 @@ updateConversationAccess (usr ::: zcon ::: cnv ::: req ::: _ ) = do ensureGroupConv conv -- Team conversations incur another round of checks case Data.convTeam conv of - Just tid -> checkTeamConv tid >> permissionCheckConv usr cnv ModifyConversationMetadata + Just tid -> checkTeamConv tid >> + permissionCheckTeamConv usr cnv ModifyConversationMetadata Nothing -> when (targetRole == TeamAccessRole) $ throwM invalidTargetAccess -- When there is no update to be done, we return 204; otherwise we go -- with 'uncheckedUpdateConversationAccess', which will potentially kick @@ -210,7 +211,7 @@ uncheckedUpdateConversationAccess body usr zcon conv (currentAccess, targetAcces updateConversationReceiptMode :: UserId ::: ConnId ::: ConvId ::: Request ::: JSON ::: JSON -> Galley Response updateConversationReceiptMode (usr ::: zcon ::: cnv ::: req ::: _ ::: _) = do ConversationReceiptModeUpdate target <- fromBody req invalidPayload - permissionCheckConv usr cnv ModifyConversationMetadata + permissionCheckTeamConv usr cnv ModifyConversationMetadata (bots, users) <- botsAndUsers <$> Data.members cnv current <- Data.lookupReceiptMode cnv if current == Just target @@ -235,7 +236,7 @@ updateConversationMessageTimer (usr ::: zcon ::: cnv ::: req ::: _ ) = do conv <- Data.conversation cnv >>= ifNothing convNotFound ensureGroupConv conv traverse_ ensureTeamMember $ Data.convTeam conv -- only team members can change the timer - permissionCheckConv usr cnv ModifyConversationMetadata + permissionCheckTeamConv usr cnv ModifyConversationMetadata let currentTimer = Data.convMessageTimer conv if currentTimer == messageTimer then return $ empty & setStatus status204 @@ -503,7 +504,7 @@ newMessage usr con cnv msg now (m, c, t) ~(toBots, toUsers) = updateConversation :: UserId ::: ConnId ::: ConvId ::: Request ::: JSON -> Galley Response updateConversation (zusr ::: zcon ::: cnv ::: req ::: _) = do body <- fromBody req invalidPayload - permissionCheckConv zusr cnv ModifyConversationMetadata + permissionCheckTeamConv zusr cnv ModifyConversationMetadata alive <- Data.isConvAlive cnv unless alive $ do Data.deleteConversation cnv diff --git a/services/galley/src/Galley/API/Util.hs b/services/galley/src/Galley/API/Util.hs index 8db48039da3..b3fcf1eb4b6 100644 --- a/services/galley/src/Galley/API/Util.hs +++ b/services/galley/src/Galley/API/Util.hs @@ -85,8 +85,10 @@ permissionCheck u p t = pure m Nothing -> throwM noTeamMember -permissionCheckConv :: UserId -> ConvId -> Perm -> Galley () -permissionCheckConv zusr cnv perm = Data.conversation cnv >>= \case +-- | If the conversation is in a team, throw iff zusr is a team member and does not have named +-- permission. If the conversation is not in a team, do nothing (no error). +permissionCheckTeamConv :: UserId -> ConvId -> Perm -> Galley () +permissionCheckTeamConv zusr cnv perm = Data.conversation cnv >>= \case Just cnv' -> case Data.convTeam cnv' of Just tid -> void $ permissionCheck zusr perm =<< Data.teamMembers tid Nothing -> pure () diff --git a/services/galley/test/integration/API/Util.hs b/services/galley/test/integration/API/Util.hs index 36564ce9940..f8bb8eed626 100644 --- a/services/galley/test/integration/API/Util.hs +++ b/services/galley/test/integration/API/Util.hs @@ -158,6 +158,7 @@ updateTeamConv g zusr convid upd = do . json upd ) +-- | See Note [managed conversations] createManagedConv :: HasCallStack => Galley -> UserId -> TeamId -> [UserId] -> Maybe Text -> Maybe (Set Access) -> Maybe Milliseconds -> Http ConvId createManagedConv g u tid us name acc mtimer = do let tinfo = ConvTeamInfo tid True