From 834b678ddc9ab080c46df7aff1dcc9bf122d66c7 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Fri, 27 May 2022 09:21:57 +0000 Subject: [PATCH 1/3] pass re-auth if user is sso user and no password provided --- services/brig/src/Brig/API/Public.hs | 8 ++++---- services/brig/src/Brig/API/User.hs | 6 +++--- services/brig/src/Brig/Data/User.hs | 14 +++++++++++--- services/brig/src/Brig/User/Auth.hs | 4 ++-- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/services/brig/src/Brig/API/Public.hs b/services/brig/src/Brig/API/Public.hs index ed52f76273..05342c39d4 100644 --- a/services/brig/src/Brig/API/Public.hs +++ b/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 diff --git a/services/brig/src/Brig/API/User.hs b/services/brig/src/Brig/API/User.hs index 592f4d62d0..4272fb3cb8 100644 --- a/services/brig/src/Brig/API/User.hs +++ b/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 diff --git a/services/brig/src/Brig/Data/User.hs b/services/brig/src/Brig/Data/User.hs index 1cc046c4e1..1181439bc1 100644 --- a/services/brig/src/Brig/Data/User.hs +++ b/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 -> diff --git a/services/brig/src/Brig/User/Auth.hs b/services/brig/src/Brig/User/Auth.hs index a649fc9fcd..b27d4008b8 100644 --- a/services/brig/src/Brig/User/Auth.hs +++ b/services/brig/src/Brig/User/Auth.hs @@ -280,7 +280,7 @@ 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] -> @@ -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") - Data.authenticate u pw + unlessM (Data.isSSOUser u) $ Data.authenticate u pw lift $ revokeCookies u cc ll -------------------------------------------------------------------------------- From cfae90ddb63d7e7c0f2da2238baeda0439c26252 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Fri, 27 May 2022 09:31:29 +0000 Subject: [PATCH 2/3] changelog --- changelog.d/3-bug-fixes/pr-2430 | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3-bug-fixes/pr-2430 diff --git a/changelog.d/3-bug-fixes/pr-2430 b/changelog.d/3-bug-fixes/pr-2430 new file mode 100644 index 0000000000..b24c792e81 --- /dev/null +++ b/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 From f56e79d9361bf6bd02a9f6a42d81127641d40190 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 27 May 2022 13:09:09 +0200 Subject: [PATCH 3/3] hi ci