diff --git a/app/config/collections.php b/app/config/collections.php index 1ab9c42fa2..3b4a7972b2 100644 --- a/app/config/collections.php +++ b/app/config/collections.php @@ -278,50 +278,6 @@ $commonCollections = [ 'array' => false, 'filters' => [], ], - [ - '$id' => ID::custom('totp'), - 'type' => Database::VAR_BOOLEAN, - 'format' => '', - 'size' => 0, - 'signed' => true, - 'required' => false, - 'default' => null, - 'array' => false, - 'filters' => [], - ], - [ - '$id' => ID::custom('totpVerification'), - 'type' => Database::VAR_BOOLEAN, - 'format' => '', - 'size' => 0, - 'signed' => true, - 'required' => false, - 'default' => null, - 'array' => false, - 'filters' => [], - ], - [ - '$id' => ID::custom('totpSecret'), - 'type' => Database::VAR_STRING, - 'format' => '', - 'size' => 256, - 'signed' => true, - 'required' => false, - 'default' => null, - 'array' => false, - 'filters' => [], - ], - [ - '$id' => ID::custom('totpBackup'), - 'type' => Database::VAR_STRING, - 'format' => '', - 'size' => 6, - 'signed' => true, - 'required' => false, - 'default' => null, - 'array' => true, - 'filters' => [], - ], [ '$id' => ID::custom('sessions'), 'type' => Database::VAR_STRING, @@ -344,6 +300,17 @@ $commonCollections = [ 'array' => false, 'filters' => ['subQueryTokens'], ], + [ + '$id' => ID::custom('authenticators'), + 'type' => Database::VAR_STRING, + 'format' => '', + 'size' => 16384, + 'signed' => true, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => ['subQueryAuthenticators'], + ], [ '$id' => ID::custom('challenges'), 'type' => Database::VAR_STRING, @@ -568,6 +535,78 @@ $commonCollections = [ ], ], + 'authenticators' => [ + '$collection' => ID::custom(Database::METADATA), + '$id' => ID::custom('authenticators'), + 'name' => 'Authenticators', + 'attributes' => [ + [ + '$id' => ID::custom('userInternalId'), + 'type' => Database::VAR_STRING, + 'format' => '', + 'size' => Database::LENGTH_KEY, + 'signed' => true, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => [], + ], + [ + '$id' => ID::custom('userId'), + 'type' => Database::VAR_STRING, + 'format' => '', + 'size' => Database::LENGTH_KEY, + 'signed' => true, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => [], + ], + [ + '$id' => ID::custom('type'), + 'type' => Database::VAR_STRING, + 'format' => '', + 'size' => Database::LENGTH_KEY, + 'signed' => true, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => [], + ], + [ + '$id' => ID::custom('verified'), + 'type' => Database::VAR_BOOLEAN, + 'format' => '', + 'size' => 0, + 'signed' => true, + 'required' => false, + 'default' => false, + 'array' => false, + 'filters' => [], + ], + [ + '$id' => ID::custom('data'), + 'type' => Database::VAR_STRING, + 'format' => '', + 'size' => 65535, + 'signed' => true, + 'required' => false, + 'default' => [], + 'array' => false, + 'filters' => ['json', 'encrypt'], + ], + ], + 'indexes' => [ + [ + '$id' => ID::custom('_key_user'), + 'type' => Database::INDEX_KEY, + 'attributes' => ['userInternalId'], + 'lengths' => [Database::LENGTH_KEY], + 'orders' => [Database::ORDER_ASC], + ] + ], + ], + 'challenges' => [ '$collection' => ID::custom(Database::METADATA), '$id' => ID::custom('challenges'), @@ -596,7 +635,7 @@ $commonCollections = [ 'filters' => [], ], [ - '$id' => ID::custom('provider'), + '$id' => ID::custom('type'), 'type' => Database::VAR_STRING, 'format' => '', 'size' => Database::LENGTH_KEY, diff --git a/app/console b/app/console index f196bcfb48..4769c50189 160000 --- a/app/console +++ b/app/console @@ -1 +1 @@ -Subproject commit f196bcfb485adfb36324aabf32a3449471319bbd +Subproject commit 4769c5018979b3f00393ce3015ff8bf69d9c1657 diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 6c748bec5a..2ec5e55c91 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -151,11 +151,11 @@ App::post('/v1/account') 'reset' => false, 'name' => $name, 'mfa' => false, - 'totp' => false, 'prefs' => new \stdClass(), 'sessions' => null, 'tokens' => null, 'memberships' => null, + 'authenticators' => null, 'search' => implode(' ', [$userId, $email, $name]), 'accessedAt' => DateTime::now(), ]); @@ -766,11 +766,11 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') 'reset' => false, 'name' => $name, 'mfa' => false, - 'totp' => false, 'prefs' => new \stdClass(), 'sessions' => null, 'tokens' => null, 'memberships' => null, + 'authenticators' => null, 'search' => implode(' ', [$userId, $email, $name]), 'accessedAt' => DateTime::now(), ]); @@ -1152,11 +1152,11 @@ App::post('/v1/account/tokens/magic-url') 'registration' => DateTime::now(), 'reset' => false, 'mfa' => false, - 'totp' => false, 'prefs' => new \stdClass(), 'sessions' => null, 'tokens' => null, 'memberships' => null, + 'authenticators' => null, 'search' => implode(' ', [$userId, $email]), 'accessedAt' => DateTime::now(), ]); @@ -1971,11 +1971,11 @@ App::post('/v1/account/sessions/anonymous') 'reset' => false, 'name' => null, 'mfa' => false, - 'totp' => false, 'prefs' => new \stdClass(), 'sessions' => null, 'tokens' => null, 'memberships' => null, + 'authenticators' => null, 'search' => $userId, 'accessedAt' => DateTime::now(), ]); @@ -2630,7 +2630,7 @@ App::patch('/v1/account/status') App::delete('/v1/account/sessions/:sessionId') ->desc('Delete session') - ->groups(['api', 'account']) + ->groups(['api', 'account', 'mfa']) ->label('scope', 'account') ->label('event', 'users.[userId].sessions.[sessionId].delete') ->label('audits.event', 'session.delete') @@ -3554,13 +3554,15 @@ App::get('/v1/account/mfa/factors') ->inject('user') ->action(function (Response $response, Document $user) { - $providers = new Document([ - 'totp' => $user->getAttribute('totp', false) && $user->getAttribute('totpVerification', false), + $totp = TOTP::getAuthenticatorFromUser($user); + + $factors = new Document([ + 'totp' => $totp !== null && $totp->getAttribute('verified', false), 'email' => $user->getAttribute('email', false) && $user->getAttribute('emailVerification', false), 'phone' => $user->getAttribute('phone', false) && $user->getAttribute('phoneVerification', false) ]); - $response->dynamic($providers, Response::MODEL_MFA_FACTORS); + $response->dynamic($factors, Response::MODEL_MFA_FACTORS); }); App::post('/v1/account/mfa/:type') @@ -3599,23 +3601,37 @@ App::post('/v1/account/mfa/:type') $backups = Provider::generateBackupCodes(); - if ($user->getAttribute('totp') && $user->getAttribute('totpVerification')) { - throw new Exception(Exception::GENERAL_UNKNOWN, 'TOTP already exists on this account.'); + $authenticator = TOTP::getAuthenticatorFromUser($user); + + if ($authenticator && $authenticator->getAttribute('verified')) { + throw new Exception(Exception::GENERAL_UNKNOWN, 'Authenticator already exists on this account.'); } - $user - ->setAttribute('totp', true) - ->setAttribute('totpVerification', false) - ->setAttribute('totpBackup', $backups) - ->setAttribute('totpSecret', $otp->getSecret()); + $authenticator = new Document([ + '$id' => ID::unique(), + 'userId' => $user->getId(), + 'userInternalId' => $user->getInternalId(), + 'type' => 'totp', + 'verified' => false, + 'data' => [ + 'secret' => $otp->getSecret(), + 'backups' => $backups + ], + '$permissions' => [ + Permission::read(Role::user($user->getId())), + Permission::update(Role::user($user->getId())), + Permission::delete(Role::user($user->getId())), + ] + ]); - $model = new Document(); - $model - ->setAttribute('backups', $backups) - ->setAttribute('secret', $otp->getSecret()) - ->setAttribute('uri', $otp->getProvisioningUri()); + $model = new Document([ + 'backups' => $backups, + 'secret' => $otp->getSecret(), + 'uri' => $otp->getProvisioningUri() + ]); - $user = $dbForProject->withRequestTimestamp($requestTimestamp, fn () => $dbForProject->updateDocument('users', $user->getId(), $user)); + $authenticator = $dbForProject->createDocument('authenticators', $authenticator); + $dbForProject->purgeCachedDocument('users', $user->getId()); $queueForEvents->setParam('userId', $user->getId()); @@ -3641,32 +3657,39 @@ App::put('/v1/account/mfa/:type') ->label('sdk.offline.key', 'current') ->param('type', null, new WhiteList(['totp']), 'Type of authenticator.') ->param('otp', '', new Text(256), 'Valid verification token.') - ->inject('requestTimestamp') ->inject('response') ->inject('user') ->inject('project') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (string $type, string $otp, ?\DateTime $requestTimestamp, Response $response, Document $user, Document $project, Database $dbForProject, Event $queueForEvents) { + ->action(function (string $type, string $otp, Response $response, Document $user, Document $project, Database $dbForProject, Event $queueForEvents) { + + $authenticator = match ($type) { + 'totp' => TOTP::getAuthenticatorFromUser($user), + default => null + }; + + if ($authenticator === null) { + throw new Exception(Exception::GENERAL_UNKNOWN, 'Authenticator not found on this account.'); + } + + if ($authenticator->getAttribute('verified')) { + throw new Exception(Exception::GENERAL_UNKNOWN, 'Authenticator already verified on this account.'); + } $success = match ($type) { 'totp' => Challenge\TOTP::verify($user, $otp), - default => false + default => throw new Exception(Exception::USER_INVALID_TOKEN) }; if (!$success) { throw new Exception(Exception::USER_INVALID_TOKEN); } - if (!$user->getAttribute('totp')) { - throw new Exception(Exception::GENERAL_UNKNOWN, 'Authenticator needs to be added first.'); - } elseif ($user->getAttribute('totpVerification')) { - throw new Exception(Exception::GENERAL_UNKNOWN, 'Authenticator already verified on this account.'); - } + $authenticator->setAttribute('verified', true); - $user->setAttribute('totpVerification', true); - - $user = $dbForProject->withRequestTimestamp($requestTimestamp, fn () => $dbForProject->updateDocument('users', $user->getId(), $user)); + $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); @@ -3702,6 +3725,15 @@ App::delete('/v1/account/mfa/:type') ->inject('queueForEvents') ->action(function (string $type, string $otp, ?\DateTime $requestTimestamp, Response $response, Document $user, Database $dbForProject, Event $queueForEvents) { + $authenticator = match ($type) { + 'totp' => TOTP::getAuthenticatorFromUser($user), + default => null + }; + + if (!$authenticator) { + throw new Exception(Exception::GENERAL_UNKNOWN, 'Authenticator not found.'); + } + $success = match ($type) { 'totp' => Challenge\TOTP::verify($user, $otp), default => false @@ -3711,17 +3743,8 @@ App::delete('/v1/account/mfa/:type') throw new Exception(Exception::USER_INVALID_TOKEN); } - if (!$user->getAttribute('totp')) { - throw new Exception(Exception::GENERAL_UNKNOWN, 'TOTP not added.'); - } - - $user - ->setAttribute('totp', false) - ->setAttribute('totpVerification', false) - ->setAttribute('totpSecret', null) - ->setAttribute('totpBackup', null); - - $user = $dbForProject->withRequestTimestamp($requestTimestamp, fn () => $dbForProject->updateDocument('users', $user->getId(), $user)); + $dbForProject->deleteDocument('authenticators', $authenticator->getId()); + $dbForProject->purgeCachedDocument('users', $user->getId()); $queueForEvents->setParam('userId', $user->getId()); @@ -3762,7 +3785,7 @@ App::post('/v1/account/mfa/challenge') $challenge = new Document([ 'userId' => $user->getId(), 'userInternalId' => $user->getInternalId(), - 'provider' => $factor, + 'type' => $factor, 'token' => Auth::tokenGenerator(), 'code' => $code, 'expire' => $expire, @@ -3942,26 +3965,29 @@ App::put('/v1/account/mfa/challenge') ->action(function (string $challengeId, string $otp, Document $project, Response $response, Document $user, Database $dbForProject, Event $queueForEvents) { $challenge = $dbForProject->getDocument('challenges', $challengeId); + var_dump($challenge); if ($challenge->isEmpty()) { throw new Exception(Exception::USER_INVALID_TOKEN); } - $provider = $challenge->getAttribute('provider'); - $success = match ($provider) { + $type = $challenge->getAttribute('type'); + + $success = match ($type) { 'totp' => Challenge\TOTP::challenge($challenge, $user, $otp), 'phone' => Challenge\Phone::challenge($challenge, $user, $otp), 'email' => Challenge\Email::challenge($challenge, $user, $otp), default => false }; - if (!$success && $provider === 'totp') { - $backups = $user->getAttribute('totpBackup', []); - if (in_array($otp, $backups)) { + if (!$success && $type === 'totp') { + $authenticator = TOTP::getAuthenticatorFromUser($user); + $data = $authenticator->getAttribute('data', []); + if (in_array($otp, $data['backups'])) { $success = true; - $backups = array_diff($backups, [$otp]); - $user->setAttribute('totpBackup', $backups); - $dbForProject->updateDocument('users', $user->getId(), $user); + $backups = array_diff($data['backups'], [$otp]); + $authenticator->setAttribute('totpBackup', $backups); + $dbForProject->updateDocument('authenticators', $authenticator->getId(), $authenticator); } } @@ -3976,10 +4002,9 @@ App::put('/v1/account/mfa/challenge') $sessionId = Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret, $authDuration); $session = $dbForProject->getDocument('sessions', $sessionId); - $dbForProject->updateDocument('sessions', $sessionId, $session->setAttribute('factors', $provider, Document::SET_TYPE_APPEND)); + $dbForProject->updateDocument('sessions', $sessionId, $session->setAttribute('factors', $type, Document::SET_TYPE_APPEND)); - $queueForEvents - ->setParam('userId', $user->getId()); + $queueForEvents->setParam('userId', $user->getId()); $response->dynamic($session, Response::MODEL_SESSION); }); diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index 2a8b951a7d..a25f02848b 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -1,7 +1,7 @@ $user->getAttribute('totp', false) && $user->getAttribute('totpVerification', false), + $totp = TOTP::getAuthenticatorFromUser($user); + + $factors = new Document([ + 'totp' => $totp !== null && $totp->getAttribute('verified', false), 'email' => $user->getAttribute('email', false) && $user->getAttribute('emailVerification', false), 'phone' => $user->getAttribute('phone', false) && $user->getAttribute('phoneVerification', false) ]); - $response->dynamic($providers, Response::MODEL_MFA_FACTORS); + $response->dynamic($factors, Response::MODEL_MFA_FACTORS); }); App::delete('/v1/users/:userId/mfa/:type') @@ -1606,28 +1608,24 @@ App::delete('/v1/users/:userId/mfa/:type') ->label('sdk.response.model', Response::MODEL_USER) ->param('userId', '', new UID(), 'User ID.') ->param('type', null, new WhiteList(['totp']), 'Type of authenticator.') - ->inject('requestTimestamp') ->inject('response') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (string $userId, string $type, ?\DateTime $requestTimestamp, Response $response, Database $dbForProject, Event $queueForEvents) { + ->action(function (string $userId, string $type, Response $response, Database $dbForProject, Event $queueForEvents) { $user = $dbForProject->getDocument('users', $userId); if ($user->isEmpty()) { throw new Exception(Exception::USER_NOT_FOUND); } - if (!$user->getAttribute('totp')) { + $authenticator = TOTP::getAuthenticatorFromUser($user); + + if ($authenticator === null) { throw new Exception(Exception::GENERAL_UNKNOWN, 'TOTP not added.'); } - $user - ->setAttribute('totp', false) - ->setAttribute('totpVerification', false) - ->setAttribute('totpSecret', null) - ->setAttribute('totpBackup', null); - - $user = $dbForProject->withRequestTimestamp($requestTimestamp, fn () => $dbForProject->updateDocument('users', $user->getId(), $user)); + $dbForProject->deleteDocument('authenticators', $authenticator->getId()); + $dbForProject->purgeCachedDocument('users', $user->getId()); $queueForEvents->setParam('userId', $user->getId()); diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index 0101d72116..4b66b11afa 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -1,6 +1,7 @@ getAttribute('mfa', false); - $hasVerifiedAuthenticator = $user->getAttribute('totpVerification', false); $hasVerifiedEmail = $user->getAttribute('emailVerification', false); $hasVerifiedPhone = $user->getAttribute('phoneVerification', false); + $hasVerifiedAuthenticator = TOTP::getAuthenticatorFromUser($user)?->getAttribute('verified') ?? false; $hasMoreFactors = $hasVerifiedEmail || $hasVerifiedPhone || $hasVerifiedAuthenticator; $minimumFactors = ($mfaEnabled && $hasMoreFactors) ? 2 : 1; diff --git a/app/init.php b/app/init.php index 17b4a4321d..0ce44fa181 100644 --- a/app/init.php +++ b/app/init.php @@ -471,6 +471,20 @@ Database::addFilter( } ); +Database::addFilter( + 'subQueryAuthenticators', + function (mixed $value) { + return null; + }, + function (mixed $value, Document $document, Database $database) { + return Authorization::skip(fn() => $database + ->find('authenticators', [ + Query::equal('userInternalId', [$document->getInternalId()]), + Query::limit(APP_LIMIT_SUBQUERY), + ])); + } +); + Database::addFilter( 'subQueryMemberships', function (mixed $value) { diff --git a/src/Appwrite/Auth/MFA/Challenge/Email.php b/src/Appwrite/Auth/MFA/Challenge/Email.php index add4a98e2a..000e206eaa 100644 --- a/src/Appwrite/Auth/MFA/Challenge/Email.php +++ b/src/Appwrite/Auth/MFA/Challenge/Email.php @@ -15,8 +15,8 @@ class Email extends Challenge public static function challenge(Document $challenge, Document $user, string $otp): bool { if ( - $challenge->isSet('provider') && - $challenge->getAttribute('provider') === 'email' + $challenge->isSet('type') && + $challenge->getAttribute('type') === 'email' ) { return self::verify($challenge, $otp); } diff --git a/src/Appwrite/Auth/MFA/Challenge/Phone.php b/src/Appwrite/Auth/MFA/Challenge/Phone.php index bb0af7382c..379271946e 100644 --- a/src/Appwrite/Auth/MFA/Challenge/Phone.php +++ b/src/Appwrite/Auth/MFA/Challenge/Phone.php @@ -15,8 +15,8 @@ class Phone extends Challenge public static function challenge(Document $challenge, Document $user, string $otp): bool { if ( - $challenge->isSet('provider') && - $challenge->getAttribute('provider') === 'phone' + $challenge->isSet('type') && + $challenge->getAttribute('type') === 'phone' ) { return self::verify($challenge, $otp); } diff --git a/src/Appwrite/Auth/MFA/Challenge/TOTP.php b/src/Appwrite/Auth/MFA/Challenge/TOTP.php index dfb2fe3c03..d05eb15195 100644 --- a/src/Appwrite/Auth/MFA/Challenge/TOTP.php +++ b/src/Appwrite/Auth/MFA/Challenge/TOTP.php @@ -3,6 +3,7 @@ namespace Appwrite\Auth\MFA\Challenge; use Appwrite\Auth\MFA\Challenge; +use Appwrite\Auth\MFA\Provider; use OTPHP\TOTP as TOTPLibrary; use Utopia\Database\Document; @@ -10,7 +11,9 @@ class TOTP extends Challenge { public static function verify(Document $user, string $otp): bool { - $instance = TOTPLibrary::create($user->getAttribute('totpSecret')); + $authenticator = Provider\TOTP::getAuthenticatorFromUser($user); + $data = $authenticator->getAttribute('data'); + $instance = TOTPLibrary::create($data['secret']); return $instance->now() === $otp; } @@ -18,8 +21,8 @@ class TOTP extends Challenge public static function challenge(Document $challenge, Document $user, string $otp): bool { if ( - $challenge->isSet('provider') && - $challenge->getAttribute('provider') === 'totp' + $challenge->isSet('type') && + $challenge->getAttribute('type') === 'totp' ) { return self::verify($user, $otp); } diff --git a/src/Appwrite/Auth/MFA/Provider/TOTP.php b/src/Appwrite/Auth/MFA/Provider/TOTP.php index c1d85448cc..9945a16846 100644 --- a/src/Appwrite/Auth/MFA/Provider/TOTP.php +++ b/src/Appwrite/Auth/MFA/Provider/TOTP.php @@ -4,6 +4,7 @@ namespace Appwrite\Auth\MFA\Provider; use Appwrite\Auth\MFA\Provider; use OTPHP\TOTP as TOTPLibrary; +use Utopia\Database\Document; class TOTP extends Provider { @@ -11,4 +12,16 @@ class TOTP extends Provider { $this->instance = TOTPLibrary::create($secret); } + + public static function getAuthenticatorFromUser(Document $user): ?Document + { + foreach ($user->getAttribute('authenticators') as $authenticator) { + /** @var Document $authenticator */ + if ($authenticator->getAttribute('type') === 'totp') { + return $authenticator; + } + } + + return null; + } } diff --git a/src/Appwrite/Utopia/Response/Model/User.php b/src/Appwrite/Utopia/Response/Model/User.php index 98ff3bcf6e..34e7c31621 100644 --- a/src/Appwrite/Utopia/Response/Model/User.php +++ b/src/Appwrite/Utopia/Response/Model/User.php @@ -120,12 +120,6 @@ class User extends Model 'default' => false, 'example' => true, ]) - ->addRule('totp', [ - 'type' => self::TYPE_BOOLEAN, - 'description' => 'TOTP status.', - 'default' => false, - 'example' => true, - ]) ->addRule('prefs', [ 'type' => Response::MODEL_PREFERENCES, 'description' => 'User preferences as a key-value object',