From 98d18ecc473d4a112e4935120f90ec57dcb9b585 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Mon, 6 May 2024 17:16:56 -0700 Subject: [PATCH 1/3] refactor(auth): remove auth duration from Auth::sessionVerify() calls The paramter was removed from the method so we don't need to pass it in anymore. --- app/controllers/api/account.php | 6 ++---- app/init.php | 7 +++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 90a35ede78..500b0842ab 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -3665,8 +3665,7 @@ App::put('/v1/account/mfa/authenticators/:type') $dbForProject->updateDocument('authenticators', $authenticator->getId(), $authenticator); $dbForProject->purgeCachedDocument('users', $user->getId()); - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; - $sessionId = Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret, $authDuration); + $sessionId = Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret); $session = $dbForProject->getDocument('sessions', $sessionId); $dbForProject->updateDocument('sessions', $sessionId, $session->setAttribute('factors', $type, Document::SET_TYPE_APPEND)); @@ -4105,8 +4104,7 @@ App::put('/v1/account/mfa/challenge') $dbForProject->deleteDocument('challenges', $challengeId); $dbForProject->purgeCachedDocument('users', $user->getId()); - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; - $sessionId = Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret, $authDuration); + $sessionId = Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret); $session = $dbForProject->getDocument('sessions', $sessionId); $session = $session diff --git a/app/init.php b/app/init.php index 5165f64d7f..8bf5d682cd 100644 --- a/app/init.php +++ b/app/init.php @@ -1239,14 +1239,13 @@ App::setResource('project', function ($dbForConsole, $request, $console) { return $project; }, ['dbForConsole', 'request', 'console']); -App::setResource('session', function (Document $user, Document $project) { +App::setResource('session', function (Document $user) { if ($user->isEmpty()) { return; } $sessions = $user->getAttribute('sessions', []); - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; - $sessionId = Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret, $authDuration); + $sessionId = Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret); if (!$sessionId) { return; @@ -1259,7 +1258,7 @@ App::setResource('session', function (Document $user, Document $project) { } return; -}, ['user', 'project']); +}, ['user']); App::setResource('console', function () { return new Document([ From 7e07f6b95814f7a2b4356db541b71edafcaaf597 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Mon, 6 May 2024 17:43:34 -0700 Subject: [PATCH 2/3] feat(auth): ensure user isn't kicked out after enabling MFA User's were kicked out and forced to verify their session after enabling MFA if they already had factors enabled. This change ensures that they are not kicked out of their current session after MFA is enabled by adding all relevant factors to the session. --- app/controllers/api/account.php | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 500b0842ab..b57b7cb891 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -3495,14 +3495,33 @@ App::patch('/v1/account/mfa') ->inject('requestTimestamp') ->inject('response') ->inject('user') + ->inject('session') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (bool $mfa, ?\DateTime $requestTimestamp, Response $response, Document $user, Database $dbForProject, Event $queueForEvents) { + ->action(function (bool $mfa, ?\DateTime $requestTimestamp, Response $response, Document $user, Document $session, Database $dbForProject, Event $queueForEvents) { $user->setAttribute('mfa', $mfa); $user = $dbForProject->withRequestTimestamp($requestTimestamp, fn () => $dbForProject->updateDocument('users', $user->getId(), $user)); + if ($mfa) { + $factors = $session->getAttribute('factors', []); + $totp = TOTP::getAuthenticatorFromUser($user); + if ($totp !== null && $totp->getAttribute('verified', false)) { + $factors[] = Type::TOTP; + } + if ($user->getAttribute('email', false) && $user->getAttribute('emailVerification', false)) { + $factors[] = Type::EMAIL; + } + if ($user->getAttribute('phone', false) && $user->getAttribute('phoneVerification', false)) { + $factors[] = Type::PHONE; + } + $factors = \array_unique($factors); + + $session->setAttribute('factors', $factors); + $dbForProject->updateDocument('sessions', $session->getId(), $session); + } + $queueForEvents->setParam('userId', $user->getId()); $response->dynamic($user, Response::MODEL_ACCOUNT); From 5b5505cf97fe12cc35df38962724245d7e86068a Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Mon, 6 May 2024 17:48:44 -0700 Subject: [PATCH 3/3] fix(auth): ensure session factors don't contain duplicates --- app/controllers/api/account.php | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index b57b7cb891..8c0f25be09 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -3652,10 +3652,10 @@ App::put('/v1/account/mfa/authenticators/:type') ->param('otp', '', new Text(256), 'Valid verification token.') ->inject('response') ->inject('user') - ->inject('project') + ->inject('session') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (string $type, string $otp, Response $response, Document $user, Document $project, Database $dbForProject, Event $queueForEvents) { + ->action(function (string $type, string $otp, Response $response, Document $user, Document $session, Database $dbForProject, Event $queueForEvents) { $authenticator = (match ($type) { Type::TOTP => TOTP::getAuthenticatorFromUser($user), @@ -3684,9 +3684,12 @@ App::put('/v1/account/mfa/authenticators/:type') $dbForProject->updateDocument('authenticators', $authenticator->getId(), $authenticator); $dbForProject->purgeCachedDocument('users', $user->getId()); - $sessionId = Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret); - $session = $dbForProject->getDocument('sessions', $sessionId); - $dbForProject->updateDocument('sessions', $sessionId, $session->setAttribute('factors', $type, Document::SET_TYPE_APPEND)); + $factors = $session->getAttribute('factors', []); + $factors[] = $type; + $factors = \array_unique($factors); + + $session->setAttribute('factors', $factors); + $dbForProject->updateDocument('sessions', $session->getId(), $session); $queueForEvents->setParam('userId', $user->getId()); @@ -4075,9 +4078,10 @@ App::put('/v1/account/mfa/challenge') ->inject('project') ->inject('response') ->inject('user') + ->inject('session') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (string $challengeId, string $otp, Document $project, Response $response, Document $user, Database $dbForProject, Event $queueForEvents) { + ->action(function (string $challengeId, string $otp, Document $project, Response $response, Document $user, Document $session, Database $dbForProject, Event $queueForEvents) { $challenge = $dbForProject->getDocument('challenges', $challengeId); @@ -4123,14 +4127,15 @@ App::put('/v1/account/mfa/challenge') $dbForProject->deleteDocument('challenges', $challengeId); $dbForProject->purgeCachedDocument('users', $user->getId()); - $sessionId = Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret); - $session = $dbForProject->getDocument('sessions', $sessionId); + $factors = $session->getAttribute('factors', []); + $factors[] = $type; + $factors = \array_unique($factors); - $session = $session - ->setAttribute('factors', $type, Document::SET_TYPE_APPEND) + $session + ->setAttribute('factors', $factors) ->setAttribute('mfaUpdatedAt', DateTime::now()); - $dbForProject->updateDocument('sessions', $sessionId, $session); + $dbForProject->updateDocument('sessions', $session->getId(), $session); $queueForEvents ->setParam('userId', $user->getId())