SQSERVICES-1557-be-unexpected-logic-in-password-verification-for-accounts-with-saml-credentials

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

@@ -0,0 +1 @@
+On actions that require re-authentication a password is not required if the user has SAML credentials

services/brig/src/Brig/API/Public.hs

@@ -196,7 +196,7 @@ servantSitemap = userAPI :<|> selfAPI :<|> accountAPI :<|> clientAPI :<|> prekey
     selfAPI :: ServerT SelfAPI (Handler r)
     selfAPI =
       Named @"get-self" getSelf
-        :<|> Named @"delete-self" deleteUser
+        :<|> Named @"delete-self" deleteSelfUser
         :<|> Named @"put-self" updateUser
         :<|> Named @"change-phone" changePhone
         :<|> Named @"remove-phone" removePhone
@@ -951,12 +951,12 @@ getConnection self other = do
   lself <- qualifyLocal self
   lift . wrapClient $ Data.lookupConnection lself other
 
-deleteUser ::
+deleteSelfUser ::
   UserId ->
   Public.DeleteUser ->
   (Handler r) (Maybe Code.Timeout)
-deleteUser u body =
-  API.deleteUser u (Public.deleteUserPassword body) !>> deleteUserError
+deleteSelfUser u body =
+  API.deleteSelfUser u (Public.deleteUserPassword body) !>> deleteUserError
 
 verifyDeleteUserH :: JsonRequest Public.VerifyDeleteUser ::: JSON -> (Handler r) Response
 verifyDeleteUserH (r ::: _) = do

services/brig/src/Brig/API/User.hs

@@ -51,7 +51,7 @@ module Brig.API.User
     revokeIdentity,
     deleteUserNoVerify,
     deleteUsersNoVerify,
-    Brig.API.User.deleteUser,
+    deleteSelfUser,
     verifyDeleteUser,
     deleteAccount,
     checkHandles,
@@ -1041,8 +1041,8 @@ mkPasswordResetKey ident = case ident of
 -- delete them in the team settings.  This protects teams against orphanhood.
 --
 -- TODO: communicate deletions of SSO users to SSO service.
-deleteUser :: UserId -> Maybe PlainTextPassword -> ExceptT DeleteUserError (AppT r) (Maybe Timeout)
-deleteUser uid pwd = do
+deleteSelfUser :: UserId -> Maybe PlainTextPassword -> ExceptT DeleteUserError (AppT r) (Maybe Timeout)
+deleteSelfUser uid pwd = do
   account <- lift . wrapClient $ Data.lookupAccount uid
   case account of
     Nothing -> throwE DeleteUserInvalid

services/brig/src/Brig/Data/User.hs

@@ -31,6 +31,7 @@ module Brig.Data.User
     reauthenticate,
     filterActive,
     isActivated,
+    isSSOUser,
 
     -- * Lookups
     lookupAccount,
@@ -196,9 +197,9 @@ authenticate u pw =
         throwE AuthInvalidCredentials
 
 -- | Password reauthentication. If the account has a password, reauthentication
--- is mandatory. If the account has no password and no password is given,
+-- is mandatory. If the account has no password, or is an SSO user, and no password is given,
 -- reauthentication is a no-op.
-reauthenticate :: MonadClient m => UserId -> Maybe PlainTextPassword -> ExceptT ReAuthError m ()
+reauthenticate :: (MonadClient m, MonadReader Env m) => UserId -> Maybe PlainTextPassword -> ExceptT ReAuthError m ()
 reauthenticate u pw =
   lift (lookupAuth u) >>= \case
     Nothing -> throwE (ReAuthError AuthInvalidUser)
@@ -210,11 +211,18 @@ reauthenticate u pw =
     Just (Just pw', Ephemeral) -> maybeReAuth pw'
   where
     maybeReAuth pw' = case pw of
-      Nothing -> throwE ReAuthMissingPassword
+      Nothing -> unlessM (isSSOUser u) $ throwE ReAuthMissingPassword
       Just p ->
         unless (verifyPassword p pw') $
           throwE (ReAuthError AuthInvalidCredentials)
 
+isSSOUser :: (MonadClient m, MonadReader Env m) => UserId -> m Bool
+isSSOUser uid = do
+  account <- lookupAccount uid
+  case userIdentity . accountUser =<< account of
+    Just SSOIdentity {} -> pure True
+    _ -> pure False
+
 insertAccount ::
   MonadClient m =>
   UserAccount ->

services/brig/src/Brig/User/Auth.hs

@@ -280,15 +280,15 @@ renewAccess uts at = do
   pure $ Access at' ck'
 
 revokeAccess ::
-  (MonadClient m, Log.MonadLogger m) =>
+  (MonadClient m, Log.MonadLogger m, MonadReader Env m) =>
   UserId ->
   PlainTextPassword ->
   [CookieId] ->
   [CookieLabel] ->
   ExceptT AuthError m ()
 revokeAccess u pw cc ll = do
   lift $ Log.debug $ field "user" (toByteString u) . field "action" (Log.val "User.revokeAccess")
-  Data.authenticate u pw
+  unlessM (Data.isSSOUser u) $ Data.authenticate u pw
   lift $ revokeCookies u cc ll
 
 --------------------------------------------------------------------------------