Skip to content

Commit b17144f

Browse files
authored
IdP permissions for admins (#1274)
* Also change some error labels for status 403 responses under `/identity-providers`.
1 parent 96f4975 commit b17144f

File tree

5 files changed

+56
-19
lines changed

5 files changed

+56
-19
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ data HiddenPerm
321321
| ChangeTeamSearchVisibility
322322
| ViewTeamSearchVisibility
323323
| ViewSameTeamEmails
324+
| CreateUpdateDeleteIdp
324325
deriving (Eq, Ord, Show)
325326

326327
-- | See Note [hidden team roles]
@@ -344,7 +345,8 @@ roleHiddenPermissions role = HiddenPermissions p p
344345
[ ChangeLegalHoldTeamSettings,
345346
ChangeLegalHoldUserSettings,
346347
ChangeTeamSearchVisibility,
347-
ChangeTeamFeature TeamFeatureAppLock
348+
ChangeTeamFeature TeamFeatureAppLock {- the other features can only be changed in stern -},
349+
CreateUpdateDeleteIdp
348350
]
349351
roleHiddenPerms RoleMember =
350352
(roleHiddenPerms RoleExternalPartner <>) $

services/spar/src/Spar/API.hs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import Data.Id
5151
import Data.Proxy
5252
import Data.String.Conversions
5353
import Data.Time
54+
import Galley.Types.Teams (HiddenPerm (CreateUpdateDeleteIdp))
5455
import Imports
5556
import OpenSSL.Random (randBytes)
5657
import qualified SAML2.WebSSO as SAML
@@ -395,9 +396,10 @@ authorizeIdP ::
395396
Maybe UserId ->
396397
IdP ->
397398
m TeamId
398-
authorizeIdP zusr idp = do
399-
teamid <- Brig.getZUsrOwnedTeam zusr
400-
when (teamid /= idp ^. SAML.idpExtraInfo . wiTeam) $ throwSpar SparNotInTeam
399+
authorizeIdP Nothing _ = throwSpar (SparNoPermission (cs $ show CreateUpdateDeleteIdp))
400+
authorizeIdP (Just zusr) idp = do
401+
let teamid = idp ^. SAML.idpExtraInfo . wiTeam
402+
Galley.assertHasPermission teamid CreateUpdateDeleteIdp zusr
401403
pure teamid
402404

403405
enforceHttps :: URI.URI -> Spar ()

services/spar/src/Spar/Error.hs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ data SparCustomError
7272
| SparMissingZUsr
7373
| SparNotInTeam
7474
| SparNotTeamOwner
75+
| SparNoPermission LT
7576
| SparSSODisabled
7677
| SparInitLoginWithAuth
7778
| SparInitBindWithoutAuth
@@ -161,6 +162,7 @@ renderSparError (SAML.CustomError SparIdPNotFound) = Right $ Wai.Error status404
161162
renderSparError (SAML.CustomError SparMissingZUsr) = Right $ Wai.Error status400 "client-error" "[header] 'Z-User' required"
162163
renderSparError (SAML.CustomError SparNotInTeam) = Right $ Wai.Error status403 "no-team-member" "Requesting user is not a team member or not a member of this team."
163164
renderSparError (SAML.CustomError SparNotTeamOwner) = Right $ Wai.Error status403 "insufficient-permissions" "You need to be a team owner."
165+
renderSparError (SAML.CustomError (SparNoPermission perm)) = Right $ Wai.Error status403 "insufficient-permissions" ("You need permission " <> cs perm <> ".")
164166
renderSparError (SAML.CustomError SparSSODisabled) = Right $ Wai.Error status403 "sso-disabled" "Please ask customer support to enable this feature for your team."
165167
renderSparError (SAML.CustomError SparInitLoginWithAuth) = Right $ Wai.Error status403 "login-with-auth" "This end-point is only for login, not binding."
166168
renderSparError (SAML.CustomError SparInitBindWithoutAuth) = Right $ Wai.Error status403 "bind-without-auth" "This end-point is only for binding, not login."

services/spar/src/Spar/Intra/Galley.hs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import Control.Lens
2525
import Control.Monad.Except
2626
import Data.ByteString.Conversion
2727
import Data.Id (TeamId, UserId)
28+
import Data.String.Conversions (cs)
2829
import Galley.Types.Teams
2930
import Imports
3031
import Network.HTTP.Types (status403)
@@ -62,6 +63,22 @@ assertIsTeamOwner tid uid = do
6263
when (responseStatus r == status403) $ do
6364
throwSpar SparNotTeamOwner
6465

66+
-- | user is member of a given team and has a given permission there.
67+
assertHasPermission ::
68+
(HasCallStack, MonadSparToGalley m, MonadError SparError m, IsPerm perm, Show perm) =>
69+
TeamId ->
70+
perm ->
71+
UserId ->
72+
m ()
73+
assertHasPermission tid perm uid = do
74+
resp <-
75+
call $
76+
method GET
77+
. (paths ["i", "teams", toByteString' tid, "members", toByteString' uid])
78+
case (statusCode resp, parseResponse @TeamMember "galley" resp) of
79+
(200, Right member) | hasPermission member perm -> pure ()
80+
_ -> throwSpar (SparNoPermission (cs $ show perm))
81+
6582
assertSSOEnabled ::
6683
(HasCallStack, MonadError SparError m, MonadSparToGalley m) =>
6784
TeamId ->

services/spar/test-integration/Test/Spar/APISpec.hs

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ specBindingUsers = describe "binding existing users to sso identities" $ do
479479
(uid, teamid, idp, (_, privcreds)) <- registerTestIdPWithMeta
480480
(subj, _, _) <- initialBind uid idp privcreds
481481
uid' <-
482-
let Just perms = Galley.newPermissions mempty mempty
482+
let perms = Galley.noPermissions
483483
in call $ createTeamMember (env ^. teBrig) (env ^. teGalley) teamid perms
484484
(_, sparresp) <- reBindSame uid' idp privcreds subj
485485
checkDenyingAuthnResp sparresp "subject-id-taken"
@@ -530,24 +530,25 @@ testGetPutDelete whichone = do
530530
whichone (env ^. teSpar) Nothing (IdPId UUID.nil) idpmeta
531531
`shouldRespondWith` checkErrHspec 404 "not-found"
532532
context "no zuser" $ do
533-
it "responds with 'client error'" $ do
533+
it "responds with 'insufficient permissions'" $ do
534534
env <- ask
535535
(_, _, (^. idpId) -> idpid, (idpmeta, _)) <- registerTestIdPWithMeta
536536
whichone (env ^. teSpar) Nothing idpid idpmeta
537-
`shouldRespondWith` checkErrHspec 400 "client-error"
537+
`shouldRespondWith` checkErrHspec 403 "insufficient-permissions"
538+
538539
context "zuser has no team" $ do
539-
it "responds with 'no team member'" $ do
540+
it "responds with 'insufficient permissions'" $ do
540541
env <- ask
541542
(_, _, (^. idpId) -> idpid, (idpmeta, _)) <- registerTestIdPWithMeta
542543
(uid, _) <- call $ createRandomPhoneUser (env ^. teBrig)
543544
whichone (env ^. teSpar) (Just uid) idpid idpmeta
544-
`shouldRespondWith` checkErrHspec 403 "no-team-member"
545+
`shouldRespondWith` checkErrHspec 403 "insufficient-permissions"
545546
context "zuser is a team member, but not a team owner" $ do
546547
it "responds with 'insufficient-permissions' and a helpful message" $ do
547548
env <- ask
548549
(_, teamid, (^. idpId) -> idpid, (idpmeta, _)) <- registerTestIdPWithMeta
549550
newmember <-
550-
let Just perms = Galley.newPermissions mempty mempty
551+
let perms = Galley.noPermissions
551552
in call $ createTeamMember (env ^. teBrig) (env ^. teGalley) teamid perms
552553
whichone (env ^. teSpar) (Just newmember) idpid idpmeta
553554
`shouldRespondWith` checkErrHspec 403 "insufficient-permissions"
@@ -571,12 +572,12 @@ specCRUDIdentityProvider = do
571572
describe "GET /identity-providers/:idp" $ do
572573
testGetPutDelete (\o t i _ -> callIdpGet' o t i)
573574
context "zuser has wrong team" $ do
574-
it "responds with 'no team member'" $ do
575+
it "responds with 'insufficient permissions'" $ do
575576
env <- ask
576577
(_, _, (^. idpId) -> idpid) <- registerTestIdP
577578
(uid, _) <- call $ createUserWithTeam (env ^. teBrig) (env ^. teGalley)
578579
callIdpGet' (env ^. teSpar) (Just uid) idpid
579-
`shouldRespondWith` checkErrHspec 403 "no-team-member"
580+
`shouldRespondWith` checkErrHspec 403 "insufficient-permissions"
580581
context "known IdP, client is team owner" $ do
581582
it "responds with 2xx and IdP" $ do
582583
env <- ask
@@ -595,7 +596,7 @@ specCRUDIdentityProvider = do
595596
(_owner :: UserId, teamid :: TeamId) <-
596597
call $ createUserWithTeam (env ^. teBrig) (env ^. teGalley)
597598
member :: UserId <-
598-
let Just perms = Galley.newPermissions mempty mempty
599+
let perms = Galley.noPermissions
599600
in call $ createTeamMember (env ^. teBrig) (env ^. teGalley) teamid perms
600601
callIdpGetAll' (env ^. teSpar) (Just member)
601602
`shouldRespondWith` checkErrHspec 403 "insufficient-permissions"
@@ -631,7 +632,21 @@ specCRUDIdentityProvider = do
631632
(_, _, (^. idpId) -> idpid) <- registerTestIdP
632633
(uid, _) <- call $ createUserWithTeam (env ^. teBrig) (env ^. teGalley)
633634
callIdpDelete' (env ^. teSpar) (Just uid) idpid
634-
`shouldRespondWith` checkErrHspec 403 "no-team-member"
635+
`shouldRespondWith` checkErrHspec 403 "insufficient-permissions"
636+
context "zuser is admin resp. member" $ do
637+
it "responds 204 resp. 403" $ do
638+
env <- ask
639+
(_, tid, (^. idpId) -> idpid) <- registerTestIdP
640+
let mkUser :: Galley.Role -> TestSpar UserId
641+
mkUser role = do
642+
let perms = Galley.rolePermissions role
643+
call $ createTeamMember (env ^. teBrig) (env ^. teGalley) tid perms
644+
admin <- mkUser Galley.RoleAdmin
645+
member <- mkUser Galley.RoleMember
646+
callIdpDelete' (env ^. teSpar) (Just member) idpid
647+
`shouldRespondWith` checkErrHspec 403 "insufficient-permissions"
648+
callIdpDelete' (env ^. teSpar) (Just admin) idpid
649+
`shouldRespondWith` ((== 204) . statusCode)
635650
context "known IdP, IdP empty, client is team owner, without email" $ do
636651
it "responds with 2xx and removes IdP" $ do
637652
env <- ask
@@ -838,7 +853,7 @@ specCRUDIdentityProvider = do
838853
env <- ask
839854
(_owner, tid, idp) <- registerTestIdP
840855
newmember <-
841-
let Just perms = Galley.newPermissions mempty mempty
856+
let perms = Galley.noPermissions
842857
in call $ createTeamMember (env ^. teBrig) (env ^. teGalley) tid perms
843858
callIdpCreate' (env ^. teSpar) (Just newmember) (idp ^. idpMetadata)
844859
`shouldRespondWith` checkErrHspec 403 "insufficient-permissions"
@@ -1144,10 +1159,9 @@ specAux = do
11441159
liftIO $ userTeam parsedResp `shouldSatisfy` isJust
11451160
permses :: [Galley.Permissions]
11461161
permses =
1147-
fromJust
1148-
<$> [ Just Galley.fullPermissions,
1149-
Galley.newPermissions mempty mempty
1150-
]
1162+
[ Galley.fullPermissions,
1163+
Galley.noPermissions
1164+
]
11511165
sequence_ [check tryowner perms | tryowner <- [minBound ..], perms <- [0 .. (length permses - 1)]]
11521166

11531167
specSsoSettings :: SpecWith TestEnv

0 commit comments

Comments
 (0)