From e6ad1c6830613058911a2b39f4cd96e6838f7405 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Sat, 2 Mar 2024 13:05:22 +0000 Subject: [PATCH 1/7] Add more recovery code endpoints --- .gitpod.yml | 1 - app/config/collections.php | 13 ++- app/config/errors.php | 10 ++ app/controllers/api/account.php | 107 +++++++++++++++++- .../account/get-mfa-recovery-codes.md | 1 + .../account/update-mfa-recovery-codes.md | 1 + src/Appwrite/Auth/Auth.php | 5 + src/Appwrite/Extend/Exception.php | 2 + .../Utopia/Response/Model/Session.php | 6 + 9 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 docs/references/account/get-mfa-recovery-codes.md create mode 100644 docs/references/account/update-mfa-recovery-codes.md diff --git a/.gitpod.yml b/.gitpod.yml index d740c467cb..478b62fc8d 100644 --- a/.gitpod.yml +++ b/.gitpod.yml @@ -17,7 +17,6 @@ tasks: ports: - port: 8080 - onOpen: open-preview visibility: public vscode: diff --git a/app/config/collections.php b/app/config/collections.php index e27105974b..3c61dd4e9b 100644 --- a/app/config/collections.php +++ b/app/config/collections.php @@ -287,7 +287,7 @@ $commonCollections = [ 'required' => false, 'default' => [], 'array' => true, - 'filters' => [], + 'filters' => ['encrypt'], ], [ '$id' => ID::custom('authenticators'), @@ -979,6 +979,17 @@ $commonCollections = [ 'array' => false, 'filters' => ['datetime'], ], + [ + '$id' => ID::custom('mfaUpdatedAt'), + 'type' => Database::VAR_DATETIME, + 'format' => '', + 'size' => 0, + 'signed' => false, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => ['datetime'], + ], ], 'indexes' => [ [ diff --git a/app/config/errors.php b/app/config/errors.php index 68611e40de..1202dabddb 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -242,6 +242,11 @@ return [ 'description' => 'Authenticator could not be found on the current user.', 'code' => 404, ], + Exception::USER_RECOVERY_CODES_NOT_FOUND => [ + 'name' => Exception::USER_RECOVERY_CODES_NOT_FOUND, + 'description' => 'Recovery codes could not be found on the current user.', + 'code' => 404, + ], Exception::USER_AUTHENTICATOR_ALREADY_VERIFIED => [ 'name' => Exception::USER_AUTHENTICATOR_ALREADY_VERIFIED, 'description' => 'This authenticator is already verified on the current user.', @@ -262,6 +267,11 @@ return [ 'description' => 'More factors are required to complete the sign in process.', 'code' => 401, ], + Exception::USER_CHALLENGE_REQUIRED => [ + 'name' => Exception::USER_CHALLENGE_REQUIRED, + 'description' => 'A recently succeessful challenge is required to complete this action. A challenge is considered recent for 5 minutes.', + 'code' => 401, + ], Exception::USER_OAUTH2_BAD_REQUEST => [ 'name' => Exception::USER_OAUTH2_BAD_REQUEST, 'description' => 'OAuth2 provider rejected the bad request.', diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 845a16c716..ba0662f959 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -3714,15 +3714,14 @@ App::post('/v1/account/mfa/recovery-codes') ->label('sdk.description', '/docs/references/account/create-mfa-recovery-codes.md') ->label('sdk.response.code', Response::STATUS_CODE_OK) ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) - ->label('sdk.response.model', Response::MODEL_USER) + ->label('sdk.response.model', Response::MODEL_MFA_RECOVERY_CODES) ->label('sdk.offline.model', '/account') ->label('sdk.offline.key', 'current') ->inject('response') ->inject('user') - ->inject('project') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (Response $response, Document $user, Document $project, Database $dbForProject, Event $queueForEvents) { + ->action(function (Response $response, Document $user, Database $dbForProject, Event $queueForEvents) { $mfaRecoveryCodes = $user->getAttribute('mfaRecoveryCodes', []); @@ -3743,6 +3742,102 @@ App::post('/v1/account/mfa/recovery-codes') $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); }); +App::put('/v1/account/mfa/recovery-codes') + ->desc('Regenerate MFA Recovery Codes') + ->groups(['api', 'account']) + ->label('event', 'users.[userId].update.mfa') + ->label('scope', 'account') + ->label('audits.event', 'user.update') + ->label('audits.resource', 'user/{response.$id}') + ->label('audits.userId', '{response.$id}') + ->label('sdk.auth', [APP_AUTH_TYPE_SESSION, APP_AUTH_TYPE_JWT]) + ->label('sdk.namespace', 'account') + ->label('sdk.method', 'updateMfaRecoveryCodes') + ->label('sdk.description', '/docs/references/account/update-mfa-recovery-codes.md') + ->label('sdk.response.code', Response::STATUS_CODE_OK) + ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) + ->label('sdk.response.model', Response::MODEL_MFA_RECOVERY_CODES) + ->label('sdk.offline.model', '/account') + ->label('sdk.offline.key', 'current') + ->inject('session') + ->inject('dbForProject') + ->inject('response') + ->inject('user') + ->inject('queueForEvents') + ->action(function (Document $session, Database $dbForProject, Response $response, Document $user, Event $queueForEvents) { + + $isSessionSafe = false; + + $lastUpdate = $session->getAttribute('mfaUpdatedAt'); + if (!empty($lastUpdate)) { + $now = DateTime::now(); + $maxAllowedDate = DateTime::addSeconds($lastUpdate, Auth::MFA_RECENT_DURATION); // Maximum date until session is considered safe before asking for another challenge + + $isSessionSafe = DateTime::formatTz($maxAllowedDate) >= DateTime::formatTz($now); + } + + if (!$isSessionSafe) { + throw new Exception(Exception::USER_CHALLENGE_REQUIRED); + } + + $mfaRecoveryCodes = Type::generateBackupCodes(); + $user->setAttribute('mfaRecoveryCodes', $mfaRecoveryCodes); + $dbForProject->updateDocument('users', $user->getId(), $user); + + $queueForEvents->setParam('userId', $user->getId()); + + $document = new Document([ + 'recoveryCodes' => $mfaRecoveryCodes + ]); + + $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); + }); + +App::get('/v1/account/mfa/recovery-codes') + ->desc('Get MFA Recovery Codes') + ->groups(['api', 'account']) + ->label('scope', 'account') + ->label('sdk.auth', [APP_AUTH_TYPE_SESSION, APP_AUTH_TYPE_JWT]) + ->label('sdk.namespace', 'account') + ->label('sdk.method', 'getMfaRecoveryCodes') + ->label('sdk.description', '/docs/references/account/get-mfa-recovery-codes.md') + ->label('sdk.response.code', Response::STATUS_CODE_OK) + ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) + ->label('sdk.response.model', Response::MODEL_MFA_RECOVERY_CODES) + ->label('sdk.offline.model', '/account') + ->label('sdk.offline.key', 'current') + ->inject('session') + ->inject('response') + ->inject('user') + ->action(function (Document $session, Response $response, Document $user) { + + $isSessionSafe = false; + + $lastUpdate = $session->getAttribute('mfaUpdatedAt'); + if (!empty($lastUpdate)) { + $now = DateTime::now(); + $maxAllowedDate = DateTime::addSeconds($lastUpdate, Auth::MFA_RECENT_DURATION); // Maximum date until session is considered safe before asking for another challenge + + $isSessionSafe = DateTime::formatTz($maxAllowedDate) >= DateTime::formatTz($now); + } + + if (!$isSessionSafe) { + throw new Exception(Exception::USER_CHALLENGE_REQUIRED); + } + + $mfaRecoveryCodes = $user->getAttribute('mfaRecoveryCodes', []); + + if (empty($mfaRecoveryCodes)) { + throw new Exception(Exception::USER_RECOVERY_CODES_NOT_FOUND); + } + + $document = new Document([ + 'recoveryCodes' => $mfaRecoveryCodes + ]); + + $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); + }); + App::delete('/v1/account/mfa/authenticators/:type') ->desc('Delete Authenticator') ->groups(['api', 'account']) @@ -4063,7 +4158,11 @@ 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', $type, Document::SET_TYPE_APPEND)); + $session = $session + ->setAttribute('factors', $type, Document::SET_TYPE_APPEND) + ->setAttribute('mfaUpdatedAt', DateTime::now()); + + $dbForProject->updateDocument('sessions', $sessionId, $session); $queueForEvents ->setParam('userId', $user->getId()) diff --git a/docs/references/account/get-mfa-recovery-codes.md b/docs/references/account/get-mfa-recovery-codes.md new file mode 100644 index 0000000000..f3265cae85 --- /dev/null +++ b/docs/references/account/get-mfa-recovery-codes.md @@ -0,0 +1 @@ +Get recovery codes that can be used as backup for MFA flow. Before getting codes, they must be generated using [createMfaRecoveryCodes](/docs/references/cloud/client-web/account#createMfaRecoveryCodes) method. An OTP challenge is required to read recovery codes. \ No newline at end of file diff --git a/docs/references/account/update-mfa-recovery-codes.md b/docs/references/account/update-mfa-recovery-codes.md new file mode 100644 index 0000000000..2d3b266205 --- /dev/null +++ b/docs/references/account/update-mfa-recovery-codes.md @@ -0,0 +1 @@ +Regenerate recovery codes that can be used as backup for MFA flow. Before regenerating codes, they must be first generated using [createMfaRecoveryCodes](/docs/references/cloud/client-web/account#createMfaRecoveryCodes) method. An OTP challenge is required to regenreate recovery codes. \ No newline at end of file diff --git a/src/Appwrite/Auth/Auth.php b/src/Appwrite/Auth/Auth.php index 42caaefc0f..4c5ebb5f0e 100644 --- a/src/Appwrite/Auth/Auth.php +++ b/src/Appwrite/Auth/Auth.php @@ -86,6 +86,11 @@ class Auth public const TOKEN_LENGTH_OAUTH2 = 64; public const TOKEN_LENGTH_SESSION = 256; + /** + * MFA + */ + public const MFA_RECENT_DURATION = 600; // 5 minutes + /** * @var string */ diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index 70ab831797..c5bd53fe73 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -92,6 +92,8 @@ class Exception extends \Exception public const USER_AUTHENTICATOR_NOT_FOUND = 'user_authenticator_not_found'; public const USER_AUTHENTICATOR_ALREADY_VERIFIED = 'user_authenticator_already_verified'; public const USER_RECOVERY_CODES_ALREADY_EXISTS = 'user_recovery_codes_already_exists'; + public const USER_RECOVERY_CODES_NOT_FOUND = 'user_recovery_codes_not_found'; + public const USER_CHALLENGE_REQUIRED = 'user_challenge_required'; public const USER_OAUTH2_BAD_REQUEST = 'user_oauth2_bad_request'; public const USER_OAUTH2_UNAUTHORIZED = 'user_oauth2_unauthorized'; public const USER_OAUTH2_PROVIDER_ERROR = 'user_oauth2_provider_error'; diff --git a/src/Appwrite/Utopia/Response/Model/Session.php b/src/Appwrite/Utopia/Response/Model/Session.php index cb01157bae..8b219ef1eb 100644 --- a/src/Appwrite/Utopia/Response/Model/Session.php +++ b/src/Appwrite/Utopia/Response/Model/Session.php @@ -173,6 +173,12 @@ class Session extends Model 'default' => '', 'example' => '5e5bb8c16897e', ]) + ->addRule('mfaUpdatedAt', [ + 'type' => self::TYPE_DATETIME, + 'description' => 'Most recent date in ISO 8601 format when the session successfully passed MFA challenge.', + 'default' => '', + 'example' => self::TYPE_DATETIME_EXAMPLE, + ]) ; } From 9137dc82f8ce317dd5210c453978b075b816879f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Sun, 3 Mar 2024 14:18:09 +0000 Subject: [PATCH 2/7] Re-implement mfa protection as hook --- app/controllers/api/account.php | 38 +++-------------------------- app/controllers/shared/api/auth.php | 20 +++++++++++++++ 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index ba0662f959..a26eb50791 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -3744,7 +3744,7 @@ App::post('/v1/account/mfa/recovery-codes') App::put('/v1/account/mfa/recovery-codes') ->desc('Regenerate MFA Recovery Codes') - ->groups(['api', 'account']) + ->groups(['api', 'account', 'mfaProtected']) ->label('event', 'users.[userId].update.mfa') ->label('scope', 'account') ->label('audits.event', 'user.update') @@ -3759,26 +3759,11 @@ App::put('/v1/account/mfa/recovery-codes') ->label('sdk.response.model', Response::MODEL_MFA_RECOVERY_CODES) ->label('sdk.offline.model', '/account') ->label('sdk.offline.key', 'current') - ->inject('session') ->inject('dbForProject') ->inject('response') ->inject('user') ->inject('queueForEvents') - ->action(function (Document $session, Database $dbForProject, Response $response, Document $user, Event $queueForEvents) { - - $isSessionSafe = false; - - $lastUpdate = $session->getAttribute('mfaUpdatedAt'); - if (!empty($lastUpdate)) { - $now = DateTime::now(); - $maxAllowedDate = DateTime::addSeconds($lastUpdate, Auth::MFA_RECENT_DURATION); // Maximum date until session is considered safe before asking for another challenge - - $isSessionSafe = DateTime::formatTz($maxAllowedDate) >= DateTime::formatTz($now); - } - - if (!$isSessionSafe) { - throw new Exception(Exception::USER_CHALLENGE_REQUIRED); - } + ->action(function (Database $dbForProject, Response $response, Document $user, Event $queueForEvents) { $mfaRecoveryCodes = Type::generateBackupCodes(); $user->setAttribute('mfaRecoveryCodes', $mfaRecoveryCodes); @@ -3795,7 +3780,7 @@ App::put('/v1/account/mfa/recovery-codes') App::get('/v1/account/mfa/recovery-codes') ->desc('Get MFA Recovery Codes') - ->groups(['api', 'account']) + ->groups(['api', 'account', 'mfaProtected']) ->label('scope', 'account') ->label('sdk.auth', [APP_AUTH_TYPE_SESSION, APP_AUTH_TYPE_JWT]) ->label('sdk.namespace', 'account') @@ -3806,24 +3791,9 @@ App::get('/v1/account/mfa/recovery-codes') ->label('sdk.response.model', Response::MODEL_MFA_RECOVERY_CODES) ->label('sdk.offline.model', '/account') ->label('sdk.offline.key', 'current') - ->inject('session') ->inject('response') ->inject('user') - ->action(function (Document $session, Response $response, Document $user) { - - $isSessionSafe = false; - - $lastUpdate = $session->getAttribute('mfaUpdatedAt'); - if (!empty($lastUpdate)) { - $now = DateTime::now(); - $maxAllowedDate = DateTime::addSeconds($lastUpdate, Auth::MFA_RECENT_DURATION); // Maximum date until session is considered safe before asking for another challenge - - $isSessionSafe = DateTime::formatTz($maxAllowedDate) >= DateTime::formatTz($now); - } - - if (!$isSessionSafe) { - throw new Exception(Exception::USER_CHALLENGE_REQUIRED); - } + ->action(function (Response $response, Document $user) { $mfaRecoveryCodes = $user->getAttribute('mfaRecoveryCodes', []); diff --git a/app/controllers/shared/api/auth.php b/app/controllers/shared/api/auth.php index 0ae538372e..5e9499600d 100644 --- a/app/controllers/shared/api/auth.php +++ b/app/controllers/shared/api/auth.php @@ -7,6 +7,26 @@ use Appwrite\Extend\Exception; use Utopia\Database\Document; use Utopia\Database\Validator\Authorization; use MaxMind\Db\Reader; +use Utopia\Database\DateTime; + +App::init() + ->groups(['mfaProtected']) + ->inject('session') + ->action(function (Document $session) { + $isSessionSafe = false; + + $lastUpdate = $session->getAttribute('mfaUpdatedAt'); + if (!empty($lastUpdate)) { + $now = DateTime::now(); + $maxAllowedDate = DateTime::addSeconds($lastUpdate, Auth::MFA_RECENT_DURATION); // Maximum date until session is considered safe before asking for another challenge + + $isSessionSafe = DateTime::formatTz($maxAllowedDate) >= DateTime::formatTz($now); + } + + if (!$isSessionSafe) { + throw new Exception(Exception::USER_CHALLENGE_REQUIRED); + } + }); App::init() ->groups(['auth']) From 9bcad451f21d75578398c23a0a5cbfa70ce7c746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Sun, 3 Mar 2024 15:17:38 +0000 Subject: [PATCH 3/7] Add recovery codes to users API --- app/controllers/api/account.php | 7 +- app/controllers/api/users.php | 126 ++++++++++++++++++ .../users/create-mfa-recovery-codes.md | 1 + .../users/get-mfa-recovery-codes.md | 1 + .../users/update-mfa-recovery-codes.md | 1 + 5 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 docs/references/users/create-mfa-recovery-codes.md create mode 100644 docs/references/users/get-mfa-recovery-codes.md create mode 100644 docs/references/users/update-mfa-recovery-codes.md diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index a26eb50791..50b9380b70 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -3712,7 +3712,7 @@ App::post('/v1/account/mfa/recovery-codes') ->label('sdk.namespace', 'account') ->label('sdk.method', 'createMfaRecoveryCodes') ->label('sdk.description', '/docs/references/account/create-mfa-recovery-codes.md') - ->label('sdk.response.code', Response::STATUS_CODE_OK) + ->label('sdk.response.code', Response::STATUS_CODE_CREATED) ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_MFA_RECOVERY_CODES) ->label('sdk.offline.model', '/account') @@ -3765,6 +3765,11 @@ App::put('/v1/account/mfa/recovery-codes') ->inject('queueForEvents') ->action(function (Database $dbForProject, Response $response, Document $user, Event $queueForEvents) { + $mfaRecoveryCodes = $user->getAttribute('mfaRecoveryCodes', []); + if (empty($mfaRecoveryCodes)) { + throw new Exception(Exception::USER_RECOVERY_CODES_NOT_FOUND); + } + $mfaRecoveryCodes = Type::generateBackupCodes(); $user->setAttribute('mfaRecoveryCodes', $mfaRecoveryCodes); $dbForProject->updateDocument('users', $user->getId(), $user); diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index 67dd5de735..15bdc3d165 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -1591,6 +1591,132 @@ App::get('/v1/users/:userId/mfa/factors') $response->dynamic($factors, Response::MODEL_MFA_FACTORS); }); +App::get('/v1/users/:userId/mfa/recovery-codes') + ->desc('Get MFA Recovery Codes') + ->groups(['api', 'users']) + ->label('scope', 'users.read') + ->label('usage.metric', 'users.{scope}.requests.read') + ->label('sdk.auth', [APP_AUTH_TYPE_KEY]) + ->label('sdk.namespace', 'users') + ->label('sdk.method', 'getMfaRecoveryCodes') + ->label('sdk.description', '/docs/references/users/get-mfa-recovery-codes.md') + ->label('sdk.response.code', Response::STATUS_CODE_OK) + ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) + ->label('sdk.response.model', Response::MODEL_MFA_RECOVERY_CODES) + ->param('userId', '', new UID(), 'User ID.') + ->inject('response') + ->inject('dbForProject') + ->action(function (string $userId, Response $response, Database $dbForProject) { + $user = $dbForProject->getDocument('users', $userId); + + if ($user->isEmpty()) { + throw new Exception(Exception::USER_NOT_FOUND); + } + + $mfaRecoveryCodes = $user->getAttribute('mfaRecoveryCodes', []); + + if (empty($mfaRecoveryCodes)) { + throw new Exception(Exception::USER_RECOVERY_CODES_NOT_FOUND); + } + + $document = new Document([ + 'recoveryCodes' => $mfaRecoveryCodes + ]); + + $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); + }); + +App::post('/v1/users/:userId/mfa/recovery-codes') + ->desc('Create MFA Recovery Codes') + ->groups(['api', 'users']) + ->label('event', 'users.[userId].create.mfa.recovery-codes') + ->label('scope', 'users.write') + ->label('audits.event', 'user.update') + ->label('audits.resource', 'user/{response.$id}') + ->label('audits.userId', '{response.$id}') + ->label('usage.metric', 'users.{scope}.requests.update') + ->label('sdk.auth', [APP_AUTH_TYPE_KEY]) + ->label('sdk.namespace', 'users') + ->label('sdk.method', 'createMfaRecoveryCodes') + ->label('sdk.description', '/docs/references/users/create-mfa-recovery-codes.md') + ->label('sdk.response.code', Response::STATUS_CODE_CREATED) + ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) + ->label('sdk.response.model', Response::MODEL_MFA_RECOVERY_CODES) + ->param('userId', '', new UID(), 'User ID.') + ->inject('response') + ->inject('dbForProject') + ->inject('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); + } + + $mfaRecoveryCodes = $user->getAttribute('mfaRecoveryCodes', []); + + if (!empty($mfaRecoveryCodes)) { + throw new Exception(Exception::USER_RECOVERY_CODES_ALREADY_EXISTS); + } + + $mfaRecoveryCodes = Type::generateBackupCodes(); + $user->setAttribute('mfaRecoveryCodes', $mfaRecoveryCodes); + $dbForProject->updateDocument('users', $user->getId(), $user); + + $queueForEvents->setParam('userId', $user->getId()); + + $document = new Document([ + 'recoveryCodes' => $mfaRecoveryCodes + ]); + + $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); + }); + +App::put('/v1/users/:userId/mfa/recovery-codes') + ->desc('Regenerate MFA Recovery Codes') + ->groups(['api', 'users']) + ->label('event', 'users.[userId].update.mfa.recovery-codes') + ->label('scope', 'users.write') + ->label('audits.event', 'user.update') + ->label('audits.resource', 'user/{response.$id}') + ->label('audits.userId', '{response.$id}') + ->label('usage.metric', 'users.{scope}.requests.update') + ->label('sdk.auth', [APP_AUTH_TYPE_KEY]) + ->label('sdk.namespace', 'users') + ->label('sdk.method', 'updateMfaRecoveryCodes') + ->label('sdk.description', '/docs/references/users/update-mfa-recovery-codes.md') + ->label('sdk.response.code', Response::STATUS_CODE_OK) + ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) + ->label('sdk.response.model', Response::MODEL_MFA_RECOVERY_CODES) + ->param('userId', '', new UID(), 'User ID.') + ->inject('response') + ->inject('dbForProject') + ->inject('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); + } + + $mfaRecoveryCodes = $user->getAttribute('mfaRecoveryCodes', []); + if (empty($mfaRecoveryCodes)) { + throw new Exception(Exception::USER_RECOVERY_CODES_NOT_FOUND); + } + + $mfaRecoveryCodes = Type::generateBackupCodes(); + $user->setAttribute('mfaRecoveryCodes', $mfaRecoveryCodes); + $dbForProject->updateDocument('users', $user->getId(), $user); + + $queueForEvents->setParam('userId', $user->getId()); + + $document = new Document([ + 'recoveryCodes' => $mfaRecoveryCodes + ]); + + $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); + }); + App::delete('/v1/users/:userId/mfa/authenticators/:type') ->desc('Delete Authenticator') ->groups(['api', 'users']) diff --git a/docs/references/users/create-mfa-recovery-codes.md b/docs/references/users/create-mfa-recovery-codes.md new file mode 100644 index 0000000000..dc9a587afb --- /dev/null +++ b/docs/references/users/create-mfa-recovery-codes.md @@ -0,0 +1 @@ +Generate recovery codes used as backup for MFA flow for User ID. Recovery codes can be used as a MFA verification type in [createMfaChallenge](/docs/references/cloud/client-web/account#createMfaChallenge) method by client SDK. \ No newline at end of file diff --git a/docs/references/users/get-mfa-recovery-codes.md b/docs/references/users/get-mfa-recovery-codes.md new file mode 100644 index 0000000000..59c5c9e99e --- /dev/null +++ b/docs/references/users/get-mfa-recovery-codes.md @@ -0,0 +1 @@ +Get recovery codes that can be used as backup for MFA flow by User ID. Before getting codes, they must be generated using [createMfaRecoveryCodes](/docs/references/cloud/client-web/account#createMfaRecoveryCodes) method. \ No newline at end of file diff --git a/docs/references/users/update-mfa-recovery-codes.md b/docs/references/users/update-mfa-recovery-codes.md new file mode 100644 index 0000000000..9a2e8af525 --- /dev/null +++ b/docs/references/users/update-mfa-recovery-codes.md @@ -0,0 +1 @@ +Regenerate recovery codes that can be used as backup for MFA flow by User ID. Before regenerating codes, they must be first generated using [createMfaRecoveryCodes](/docs/references/cloud/client-web/account#createMfaRecoveryCodes) method. \ No newline at end of file From c4bd61cdc8bda640b2b238e4c8ce03a9c753f5c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Sun, 3 Mar 2024 18:11:55 +0000 Subject: [PATCH 4/7] QA changes --- app/controllers/api/account.php | 2 +- app/controllers/api/users.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 50b9380b70..3a0f7354f7 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -3742,7 +3742,7 @@ App::post('/v1/account/mfa/recovery-codes') $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); }); -App::put('/v1/account/mfa/recovery-codes') +App::patch('/v1/account/mfa/recovery-codes') ->desc('Regenerate MFA Recovery Codes') ->groups(['api', 'account', 'mfaProtected']) ->label('event', 'users.[userId].update.mfa') diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index 15bdc3d165..245cf5e98b 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -1626,7 +1626,7 @@ App::get('/v1/users/:userId/mfa/recovery-codes') $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); }); -App::post('/v1/users/:userId/mfa/recovery-codes') +App::patch('/v1/users/:userId/mfa/recovery-codes') ->desc('Create MFA Recovery Codes') ->groups(['api', 'users']) ->label('event', 'users.[userId].create.mfa.recovery-codes') @@ -1646,7 +1646,7 @@ App::post('/v1/users/:userId/mfa/recovery-codes') ->inject('response') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (string $userId, string $type, Response $response, Database $dbForProject, Event $queueForEvents) { + ->action(function (string $userId, Response $response, Database $dbForProject, Event $queueForEvents) { $user = $dbForProject->getDocument('users', $userId); if ($user->isEmpty()) { @@ -1692,7 +1692,7 @@ App::put('/v1/users/:userId/mfa/recovery-codes') ->inject('response') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (string $userId, string $type, Response $response, Database $dbForProject, Event $queueForEvents) { + ->action(function (string $userId, Response $response, Database $dbForProject, Event $queueForEvents) { $user = $dbForProject->getDocument('users', $userId); if ($user->isEmpty()) { From ccb5636bed43d2150d332eb4a1cb85bea4fb2a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 4 Mar 2024 07:27:26 +0000 Subject: [PATCH 5/7] fix session security --- app/controllers/api/account.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 3a0f7354f7..cb8cba30e2 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -2154,6 +2154,10 @@ App::get('/v1/account/sessions') ->inject('project') ->action(function (Response $response, Document $user, Locale $locale, Document $project) { + $roles = Authorization::getRoles(); + $isPrivilegedUser = Auth::isPrivilegedUser($roles); + $isAppUser = Auth::isAppUser($roles); + $sessions = $user->getAttribute('sessions', []); $current = Auth::sessionVerify($sessions, Auth::$secret); @@ -2162,6 +2166,8 @@ App::get('/v1/account/sessions') $session->setAttribute('countryName', $countryName); $session->setAttribute('current', ($current == $session->getId()) ? true : false); + $session->setAttribute('secret', ($isPrivilegedUser || $isAppUser) ? $session->getAttribute('secret', '') : ''); + $sessions[$key] = $session; } @@ -2256,6 +2262,10 @@ App::get('/v1/account/sessions/:sessionId') ->inject('project') ->action(function (?string $sessionId, Response $response, Document $user, Locale $locale, Document $project) { + $roles = Authorization::getRoles(); + $isPrivilegedUser = Auth::isPrivilegedUser($roles); + $isAppUser = Auth::isAppUser($roles); + $sessions = $user->getAttribute('sessions', []); $sessionId = ($sessionId === 'current') ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret) @@ -2268,6 +2278,7 @@ App::get('/v1/account/sessions/:sessionId') $session ->setAttribute('current', ($session->getAttribute('secret') == Auth::hash(Auth::$secret))) ->setAttribute('countryName', $countryName) + ->setAttribute('secret', ($isPrivilegedUser || $isAppUser) ? $session->getAttribute('secret', '') : '') ; return $response->dynamic($session, Response::MODEL_SESSION); From ae77a2466b8ede19880e5fa02f1277d33fd5d996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 4 Mar 2024 08:23:39 +0000 Subject: [PATCH 6/7] Improve tests --- tests/e2e/Services/Account/AccountCustomClientTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/e2e/Services/Account/AccountCustomClientTest.php b/tests/e2e/Services/Account/AccountCustomClientTest.php index 9b0d2ee0e9..3f7609aca1 100644 --- a/tests/e2e/Services/Account/AccountCustomClientTest.php +++ b/tests/e2e/Services/Account/AccountCustomClientTest.php @@ -234,6 +234,7 @@ class AccountCustomClientTest extends Scope $this->assertEquals(200, $response['headers']['status-code']); $this->assertEquals(2, $response['body']['total']); $this->assertEquals($sessionId, $response['body']['sessions'][0]['$id']); + $this->assertEmpty($response['body']['sessions'][0]['secret']); $this->assertEquals('Windows', $response['body']['sessions'][0]['osName']); $this->assertEquals('WIN', $response['body']['sessions'][0]['osCode']); @@ -1770,6 +1771,7 @@ class AccountCustomClientTest extends Scope ])); $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEmpty($response['body']['secret']); $this->assertEquals('123456', $response['body']['providerAccessToken']); $this->assertEquals('tuvwxyz', $response['body']['providerRefreshToken']); $this->assertGreaterThan(DateTime::addSeconds(new \DateTime(), 14400 - 5), $response['body']['providerAccessTokenExpiry']); // 5 seconds allowed networking delay @@ -1805,6 +1807,7 @@ class AccountCustomClientTest extends Scope ])); $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEmpty($response['body']['secret']); $this->assertEquals($response['body']['provider'], 'anonymous'); $sessionID = $response['body']['$id']; @@ -1817,6 +1820,7 @@ class AccountCustomClientTest extends Scope ])); $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEmpty($response['body']['secret']); $this->assertEquals($response['body']['provider'], 'anonymous'); $response = $this->client->call(Client::METHOD_GET, '/account/sessions/97823askjdkasd80921371980', array_merge([ From d4e4337c57b720ac32085652aa22248a5140bd24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 4 Mar 2024 08:50:50 +0000 Subject: [PATCH 7/7] PR review changes --- app/controllers/api/account.php | 2 +- app/controllers/api/users.php | 2 +- app/controllers/shared/api/auth.php | 6 +++--- src/Appwrite/Auth/Auth.php | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index cb8cba30e2..7d20ede720 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -3788,7 +3788,7 @@ App::patch('/v1/account/mfa/recovery-codes') $queueForEvents->setParam('userId', $user->getId()); $document = new Document([ - 'recoveryCodes' => $mfaRecoveryCodes + 'recoveryCodes' => $mfaRecoveryCodes ]); $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index 245cf5e98b..e91837c3ec 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -1711,7 +1711,7 @@ App::put('/v1/users/:userId/mfa/recovery-codes') $queueForEvents->setParam('userId', $user->getId()); $document = new Document([ - 'recoveryCodes' => $mfaRecoveryCodes + 'recoveryCodes' => $mfaRecoveryCodes ]); $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); diff --git a/app/controllers/shared/api/auth.php b/app/controllers/shared/api/auth.php index 5e9499600d..2f436a1436 100644 --- a/app/controllers/shared/api/auth.php +++ b/app/controllers/shared/api/auth.php @@ -13,17 +13,17 @@ App::init() ->groups(['mfaProtected']) ->inject('session') ->action(function (Document $session) { - $isSessionSafe = false; + $isSessionFresh = false; $lastUpdate = $session->getAttribute('mfaUpdatedAt'); if (!empty($lastUpdate)) { $now = DateTime::now(); $maxAllowedDate = DateTime::addSeconds($lastUpdate, Auth::MFA_RECENT_DURATION); // Maximum date until session is considered safe before asking for another challenge - $isSessionSafe = DateTime::formatTz($maxAllowedDate) >= DateTime::formatTz($now); + $isSessionFresh = DateTime::formatTz($maxAllowedDate) >= DateTime::formatTz($now); } - if (!$isSessionSafe) { + if (!$isSessionFresh) { throw new Exception(Exception::USER_CHALLENGE_REQUIRED); } }); diff --git a/src/Appwrite/Auth/Auth.php b/src/Appwrite/Auth/Auth.php index 4c5ebb5f0e..a25c6123ea 100644 --- a/src/Appwrite/Auth/Auth.php +++ b/src/Appwrite/Auth/Auth.php @@ -89,7 +89,7 @@ class Auth /** * MFA */ - public const MFA_RECENT_DURATION = 600; // 5 minutes + public const MFA_RECENT_DURATION = 1800; // 30 mins /** * @var string