From ce46b9c66f5c9941ce58220660a852d503b083f3 Mon Sep 17 00:00:00 2001
From: Leif Battermann <leif.battermann@wire.com>
Date: Thu, 9 Jun 2022 14:54:26 +0000
Subject: [PATCH] Revert "Fix: saml-auto-provisioned users don't get to change
 email address. (#2464)"

This reverts commit df1385545191691f73e5fbaef8918230aa20be7f.
---
 changelog.d/3-bug-fixes/fix-saml-corner-case |  1 -
 services/brig/src/Brig/API/User.hs           | 54 ++++++++------------
 services/brig/src/Brig/Data/User.hs          | 20 +++-----
 services/brig/src/Brig/User/Auth.hs          |  2 +-
 4 files changed, 31 insertions(+), 46 deletions(-)
 delete mode 100644 changelog.d/3-bug-fixes/fix-saml-corner-case

diff --git a/changelog.d/3-bug-fixes/fix-saml-corner-case b/changelog.d/3-bug-fixes/fix-saml-corner-case
deleted file mode 100644
index 00f26d06fd..0000000000
--- a/changelog.d/3-bug-fixes/fix-saml-corner-case
+++ /dev/null
@@ -1 +0,0 @@
-saml-auto-provisioned users don't get to change email address
diff --git a/services/brig/src/Brig/API/User.hs b/services/brig/src/Brig/API/User.hs
index 0fc21c80b3..4272fb3cb8 100644
--- a/services/brig/src/Brig/API/User.hs
+++ b/services/brig/src/Brig/API/User.hs
@@ -607,42 +607,32 @@ changeSelfEmail u email allowScim = do
 -- | Prepare changing the email (checking a number of invariants).
 changeEmail :: UserId -> Email -> AllowSCIMUpdates -> ExceptT ChangeEmailError (AppT r) ChangeEmailResult
 changeEmail u email allowScim = do
-  eml <- either (throwE . InvalidNewEmail email) pure (validateEmail email)
-  let eky = userEmailKey eml
-
-  blacklisted <- lift . wrapClient $ Blacklist.exists eky
+  em <-
+    either
+      (throwE . InvalidNewEmail email)
+      pure
+      (validateEmail email)
+  let ek = userEmailKey em
+  blacklisted <- lift . wrapClient $ Blacklist.exists ek
   when blacklisted $
     throwE (ChangeBlacklistedEmail email)
-
-  available <- lift . wrapClient $ Data.keyAvailable eky (Just u)
+  available <- lift . wrapClient $ Data.keyAvailable ek (Just u)
   unless available $
-    throwE (EmailExists email)
-
-  usr <-
-    maybe (throwM $ UserProfileNotFound u) pure
-      =<< lift (wrapClient $ Data.lookupUser WithPendingInvitations u)
-
-  if (emailIdentity =<< userIdentity usr) == Just eml
-    then do
-      -- The user already has an email address and the new one is exactly the same
-      pure ChangeEmailIdempotent
-    else do
-      -- case 2: No or different old email address
-      let changeAllowed =
-            ( userManagedBy usr /= ManagedByScim
-                || allowScim == AllowSCIMUpdates -- spar is always allowed to call this function (from the scim handlers)
-            )
-              && not
-                ( -- user is auto-provisioned by saml (deprecated use case)
-                  userManagedBy usr /= ManagedByScim && userHasSAML usr
-                )
-
-      unless changeAllowed $
-        throwE EmailManagedByScim
-
+    throwE $
+      EmailExists email
+  usr <- maybe (throwM $ UserProfileNotFound u) pure =<< lift (wrapClient $ Data.lookupUser WithPendingInvitations u)
+  case emailIdentity =<< userIdentity usr of
+    -- The user already has an email address and the new one is exactly the same
+    Just current | current == em -> pure ChangeEmailIdempotent
+    _ -> do
+      unless
+        ( userManagedBy usr /= ManagedByScim
+            || allowScim == AllowSCIMUpdates
+        )
+        $ throwE EmailManagedByScim
       timeout <- setActivationTimeout <$> view settings
-      act <- lift . wrapClient $ Data.newActivation eky timeout (Just u)
-      pure $ ChangeEmailNeedsActivation (usr, act, eml)
+      act <- lift . wrapClient $ Data.newActivation ek timeout (Just u)
+      pure $ ChangeEmailNeedsActivation (usr, act, em)
 
 -------------------------------------------------------------------------------
 -- Change Phone
diff --git a/services/brig/src/Brig/Data/User.hs b/services/brig/src/Brig/Data/User.hs
index c89811da92..10fe01409d 100644
--- a/services/brig/src/Brig/Data/User.hs
+++ b/services/brig/src/Brig/Data/User.hs
@@ -31,8 +31,7 @@ module Brig.Data.User
     reauthenticate,
     filterActive,
     isActivated,
-    userIdHasSAML,
-    userHasSAML,
+    isSamlUser,
 
     -- * Lookups
     lookupAccount,
@@ -213,20 +212,17 @@ reauthenticate u pw =
     Just (Just pw', Ephemeral) -> maybeReAuth pw'
   where
     maybeReAuth pw' = case pw of
-      Nothing -> unlessM (userIdHasSAML u) $ throwE ReAuthMissingPassword
+      Nothing -> unlessM (isSamlUser u) $ throwE ReAuthMissingPassword
       Just p ->
         unless (verifyPassword p pw') $
           throwE (ReAuthError AuthInvalidCredentials)
 
-userIdHasSAML :: (MonadClient m, MonadReader Env m) => UserId -> m Bool
-userIdHasSAML uid = do
-  maybe False (userHasSAML . accountUser) <$> lookupAccount uid
-
-userHasSAML :: User -> Bool
-userHasSAML user = do
-  case userIdentity user of
-    Just (SSOIdentity (UserSSOId _) _ _) -> True
-    _ -> False
+isSamlUser :: (MonadClient m, MonadReader Env m) => UserId -> m Bool
+isSamlUser uid = do
+  account <- lookupAccount uid
+  case userIdentity . accountUser =<< account of
+    Just (SSOIdentity (UserSSOId _) _ _) -> pure True
+    _ -> pure False
 
 insertAccount ::
   MonadClient m =>
diff --git a/services/brig/src/Brig/User/Auth.hs b/services/brig/src/Brig/User/Auth.hs
index b09ec22b83..3088a6eb61 100644
--- a/services/brig/src/Brig/User/Auth.hs
+++ b/services/brig/src/Brig/User/Auth.hs
@@ -288,7 +288,7 @@ revokeAccess ::
   ExceptT AuthError m ()
 revokeAccess u pw cc ll = do
   lift $ Log.debug $ field "user" (toByteString u) . field "action" (Log.val "User.revokeAccess")
-  unlessM (Data.userIdHasSAML u) $ Data.authenticate u pw
+  unlessM (Data.isSamlUser u) $ Data.authenticate u pw
   lift $ revokeCookies u cc ll
 
 --------------------------------------------------------------------------------