SQSERVICES-1628-dont-allow-deleting-id-ps-if-the-deleting-user-is-authenticated-by-it

changelog.d/3-bug-fixes/pr-2519

@@ -0,0 +1 @@
+A user now cannot delete an identity provider that they are authenticated with any more

libs/wire-api/src/Wire/API/User.hs

@@ -31,6 +31,7 @@ module Wire.API.User
     userEmail,
     userPhone,
     userSSOId,
+    userIssuer,
     userSCIMExternalId,
     scimExternalId,
     ssoIssuerAndNameId,
@@ -396,6 +397,13 @@ ssoIssuerAndNameId (UserSSOId (SAML.UserRef (SAML.Issuer uri) nameIdXML)) = Just
     fromNameId = CI.original . SAML.unsafeShowNameID
 ssoIssuerAndNameId (UserScimExternalId _) = Nothing
 
+userIssuer :: User -> Maybe SAML.Issuer
+userIssuer user = userSSOId user >>= fromSSOId
+  where
+    fromSSOId :: UserSSOId -> Maybe SAML.Issuer
+    fromSSOId (UserSSOId (SAML.UserRef issuer _)) = Just issuer
+    fromSSOId _ = Nothing
+
 connectedProfile :: User -> UserLegalHoldStatus -> UserProfile
 connectedProfile u legalHoldStatus =
   UserProfile

services/spar/src/Spar/API.hs

@@ -44,6 +44,7 @@ module Spar.API
   )
 where
 
+import Brig.Types.User (HavePendingInvitations (NoPendingInvitations))
 import Control.Lens
 import Control.Monad.Except
 import qualified Data.ByteString as SBS
@@ -94,6 +95,7 @@ import qualified Spar.Sem.VerdictFormatStore as VerdictFormatStore
 import System.Logger (Msg)
 import qualified URI.ByteString as URI
 import Wire.API.Routes.Public.Spar
+import Wire.API.User (userIssuer)
 import Wire.API.User.IdentityProvider
 import Wire.API.User.Saml
 import Wire.Sem.Logger (Logger)
@@ -412,14 +414,15 @@ idpDelete zusr idpid (fromMaybe False -> purge) = withDebugLog "idpDelete" (cons
   let issuer = idp ^. SAML.idpMetadata . SAML.edIssuer
       team = idp ^. SAML.idpExtraInfo . wiTeam
   -- if idp is not empty: fail or purge
-  idpIsEmpty <- isNothing <$> SAMLUserStore.getAnyByIssuer issuer
   let doPurge :: Sem r ()
       doPurge = do
         some <- SAMLUserStore.getSomeByIssuer issuer
         forM_ some $ \(uref, uid) -> do
           BrigAccess.delete uid
           SAMLUserStore.delete uid uref
         unless (null some) doPurge
+  idpIsEmpty <- isNothing <$> SAMLUserStore.getAnyByIssuer issuer
+  whenM (maybe (pure False) (idpDoesAuthSelf idp) zusr) $ throwSparSem SparIdPCannotDeleteOwnIdp
   unless idpIsEmpty $
     if purge
       then doPurge
@@ -444,7 +447,7 @@ idpDelete zusr idpid (fromMaybe False -> purge) = withDebugLog "idpDelete" (cons
     -- to be deleted in its old issuers list, but it's tricky to avoid race conditions, and
     -- there is little to be gained here: we only use old issuers to find users that have not
     -- been migrated yet, and if an old user points to a deleted idp, it just means that we
-    -- won't find any users to migrate.  still, doesn't hurt mucht to look either.  so we
+    -- won't find any users to migrate.  still, doesn't hurt much to look either.  so we
     -- leave old issuers dangling for now.
 
     updateReplacingIdP :: IdP -> Sem r ()
@@ -456,6 +459,12 @@ idpDelete zusr idpid (fromMaybe False -> purge) = withDebugLog "idpDelete" (cons
         GetIdPNonUnique _ -> pure ()
         GetIdPWrongTeam _ -> pure ()
 
+    idpDoesAuthSelf :: IdP -> UserId -> Sem r Bool
+    idpDoesAuthSelf idp uid = do
+      let idpIssuer = idp ^. SAML.idpMetadata . SAML.edIssuer
+      mUserIssuer <- (>>= userIssuer) <$> Brig.getBrigUser NoPendingInvitations uid
+      pure $ mUserIssuer == Just idpIssuer
+
 -- | This handler only does the json parsing, and leaves all authorization checks and
 -- application logic to 'idpCreateXML'.
 idpCreate ::

services/spar/src/Spar/Error.hs

@@ -94,6 +94,7 @@ data SparCustomError
   | SparNewIdPWantHttps LT
   | SparIdPHasBoundUsers
   | SparIdPIssuerInUse
+  | SparIdPCannotDeleteOwnIdp
   | SparProvisioningMoreThanOneIdP LT
   | SparProvisioningTokenLimitReached
   | -- | FUTUREWORK(fisx): This constructor is used in exactly one place (see
@@ -172,6 +173,7 @@ renderSparError (SAML.CustomError (SparNewIdPAlreadyInUse msg)) = Right $ Wai.mk
 renderSparError (SAML.CustomError (SparNewIdPWantHttps msg)) = Right $ Wai.mkError status400 "idp-must-be-https" ("an idp request uri must be https, not http or other: " <> msg)
 renderSparError (SAML.CustomError SparIdPHasBoundUsers) = Right $ Wai.mkError status412 "idp-has-bound-users" "an idp can only be deleted if it is empty"
 renderSparError (SAML.CustomError SparIdPIssuerInUse) = Right $ Wai.mkError status400 "idp-issuer-in-use" "The issuer of your IdP is already in use.  Remove the entry in the team that uses it, or construct a new IdP issuer."
+renderSparError (SAML.CustomError SparIdPCannotDeleteOwnIdp) = Right $ Wai.mkError status409 "cannot-delete-own-idp" "You cannot delete the IdP used to login with your own account."
 -- Errors related to provisioning
 renderSparError (SAML.CustomError (SparProvisioningMoreThanOneIdP msg)) = Right $ Wai.mkError status400 "more-than-one-idp" ("Team can have at most one IdP configured: " <> msg)
 renderSparError (SAML.CustomError SparProvisioningTokenLimitReached) = Right $ Wai.mkError status403 "token-limit-reached" "The limit of provisioning tokens per team has been reached"

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

@@ -675,17 +675,17 @@ specCRUDIdentityProvider = do
       it "responds with 412 and does not remove IdP" $ do
         env <- ask
         (firstOwner, tid, idp, (_, privcreds)) <- registerTestIdPWithMeta
-        ssoOwner <- mkSsoOwner firstOwner tid idp privcreds
-        callIdpDelete' (env ^. teSpar) (Just ssoOwner) (idp ^. idpId)
+        _ <- mkSsoOwner firstOwner tid idp privcreds
+        callIdpDelete' (env ^. teSpar) (Just firstOwner) (idp ^. idpId)
           `shouldRespondWith` checkErrHspec 412 "idp-has-bound-users"
-        callIdpGet' (env ^. teSpar) (Just ssoOwner) (idp ^. idpId)
+        callIdpGet' (env ^. teSpar) (Just firstOwner) (idp ^. idpId)
           `shouldRespondWith` \resp -> statusCode resp < 300
     context "with email, idp non-empty, purge=true" $ do
       it "responds with 2xx and removes IdP and users *synchronously*" $ do
         env <- ask
         (firstOwner, tid, idp, (_, privcreds)) <- registerTestIdPWithMeta
         ssoOwner <- mkSsoOwner firstOwner tid idp privcreds
-        callIdpDeletePurge' (env ^. teSpar) (Just ssoOwner) (idp ^. idpId)
+        callIdpDeletePurge' (env ^. teSpar) (Just firstOwner) (idp ^. idpId)
           `shouldRespondWith` \resp -> statusCode resp < 300
         _ <- aFewTimes (getUserBrig ssoOwner) isNothing
         ssoOwner' <- userId <$$> getUserBrig ssoOwner
@@ -695,6 +695,13 @@ specCRUDIdentityProvider = do
           firstOwner' `shouldBe` Just firstOwner
         callIdpGet' (env ^. teSpar) (Just firstOwner) (idp ^. idpId)
           `shouldRespondWith` checkErrHspec 404 "not-found"
+    context "with email, user who tries to delete is authenticated by the IdP, purge=true" $ do
+      it "responds with 409 'cannot-delete-own-idp'" $ do
+        env <- ask
+        (firstOwner, tid, idp, (_, privcreds)) <- registerTestIdPWithMeta
+        ssoOwner <- mkSsoOwner firstOwner tid idp privcreds
+        callIdpDeletePurge' (env ^. teSpar) (Just ssoOwner) (idp ^. idpId)
+          `shouldRespondWith` checkErrHspec 409 "cannot-delete-own-idp"
   describe "PUT /identity-providers/:idp" $ do
     testGetPutDelete callIdpUpdate'
     context "known IdP, client is team owner" $ do