From 7dec6c73211caebcbe2669d1095a9e68a24ad8db Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Thu, 25 May 2023 14:06:08 -0700 Subject: [PATCH 1/3] Fix 500 error when a passwordless user creates an email session --- app/controllers/api/account.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index dd5ac4a2da..9ee63634be 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -180,7 +180,7 @@ App::post('/v1/account/sessions/email') Query::equal('email', [$email]), ]); - if (!$profile || !Auth::passwordVerify($password, $profile->getAttribute('password'), $profile->getAttribute('hash'), $profile->getAttribute('hashOptions'))) { + if (!$profile || empty($profile->getAttribute('passwordUpdate')) || !Auth::passwordVerify($password, $profile->getAttribute('password'), $profile->getAttribute('hash'), $profile->getAttribute('hashOptions'))) { throw new Exception(Exception::USER_INVALID_CREDENTIALS); } From 5ef3162ceedcf61496b3f88378c0d44b19d88ef4 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Thu, 25 May 2023 14:16:19 -0700 Subject: [PATCH 2/3] 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); From 5afc49784e946a0d1693ba0e9d4d76b6cb590d79 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Wed, 31 May 2023 13:52:05 -0700 Subject: [PATCH 3/3] Update the Update Phone API to also set the password This is to ensure the behavior matches the Update Email endpoint. --- app/controllers/api/account.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index e2282fd988..85d59bd028 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1706,6 +1706,14 @@ App::patch('/v1/account/phone') ->setAttribute('phoneVerification', false) // After this user needs to confirm phone number again ->setAttribute('search', implode(' ', [$user->getId(), $user->getAttribute('name', ''), $user->getAttribute('email', ''), $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) {