From 5ef3162ceedcf61496b3f88378c0d44b19d88ef4 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Thu, 25 May 2023 14:16:19 -0700 Subject: [PATCH] Don't rely on isAnonymousUser to check whether to verify password It's possible for a user to not be anonymous and not have a password. Using passwordUpdate is a better indicator as to whether the user has set a password or not. --- app/controllers/api/account.php | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 9ee63634be..e2282fd988 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1629,10 +1629,11 @@ App::patch('/v1/account/email') ->inject('dbForProject') ->inject('events') ->action(function (string $email, string $password, ?\DateTime $requestTimestamp, Response $response, Document $user, Database $dbForProject, Event $events) { - $isAnonymousUser = Auth::isAnonymousUser($user); // Check if request is from an anonymous account for converting + // passwordUpdate will be empty if the user has never set a password + $passwordUpdate = $user->getAttribute('passwordUpdate'); if ( - !$isAnonymousUser && + !empty($passwordUpdate) && !Auth::passwordVerify($password, $user->getAttribute('password'), $user->getAttribute('hash'), $user->getAttribute('hashOptions')) ) { // Double check user password throw new Exception(Exception::USER_INVALID_CREDENTIALS); @@ -1641,13 +1642,18 @@ App::patch('/v1/account/email') $email = \strtolower($email); $user - ->setAttribute('password', $isAnonymousUser ? Auth::passwordHash($password, Auth::DEFAULT_ALGO, Auth::DEFAULT_ALGO_OPTIONS) : $user->getAttribute('password', '')) - ->setAttribute('hash', $isAnonymousUser ? Auth::DEFAULT_ALGO : $user->getAttribute('hash', '')) - ->setAttribute('hashOptions', $isAnonymousUser ? Auth::DEFAULT_ALGO_OPTIONS : $user->getAttribute('hashOptions', '')) ->setAttribute('email', $email) ->setAttribute('emailVerification', false) // After this user needs to confirm mail again ->setAttribute('search', implode(' ', [$user->getId(), $user->getAttribute('name', ''), $email, $user->getAttribute('phone', '')])); + if (empty($passwordUpdate)) { + $user + ->setAttribute('password', Auth::passwordHash($password, Auth::DEFAULT_ALGO, Auth::DEFAULT_ALGO_OPTIONS)) + ->setAttribute('hash', Auth::DEFAULT_ALGO) + ->setAttribute('hashOptions', Auth::DEFAULT_ALGO_OPTIONS) + ->setAttribute('passwordUpdate', DateTime::now()); + } + try { $user = $dbForProject->withRequestTimestamp($requestTimestamp, fn () => $dbForProject->updateDocument('users', $user->getId(), $user)); } catch (Duplicate $th) { @@ -1685,11 +1691,11 @@ App::patch('/v1/account/phone') ->inject('dbForProject') ->inject('events') ->action(function (string $phone, string $password, ?\DateTime $requestTimestamp, Response $response, Document $user, Database $dbForProject, Event $events) { - - $isAnonymousUser = Auth::isAnonymousUser($user); // Check if request is from an anonymous account for converting + // passwordUpdate will be empty if the user has never set a password + $passwordUpdate = $user->getAttribute('passwordUpdate'); if ( - !$isAnonymousUser && + !empty($passwordUpdate) && !Auth::passwordVerify($password, $user->getAttribute('password'), $user->getAttribute('hash'), $user->getAttribute('hashOptions')) ) { // Double check user password throw new Exception(Exception::USER_INVALID_CREDENTIALS);