From 676d53b18e2057bf5fa75397544aca264260aa34 Mon Sep 17 00:00:00 2001 From: Ujjwaljain16 Date: Tue, 9 Dec 2025 02:18:40 +0530 Subject: [PATCH] fix: resolve MFA recovery code validation in 1.8.0 Remove strtolower() from recovery code type comparison (line 4945) Remove strtolower() from match statement (line 4967) Add comprehensive test for recovery code challenge validation Fixes issue where recovery codes fail with 'Invalid token' error Fixes #10740 --- app/controllers/api/account.php | 4 +- .../Account/AccountCustomClientTest.php | 112 ++++++++++++++++++ 2 files changed, 114 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 5563fc6a59..c697fe9ec6 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -4942,7 +4942,7 @@ App::put('/v1/account/mfa/challenge') $recoveryCodeChallenge = function (Document $challenge, Document $user, string $otp) use ($dbForProject) { if ( $challenge->isSet('type') && - $challenge->getAttribute('type') === \strtolower(Type::RECOVERY_CODE) + $challenge->getAttribute('type') === Type::RECOVERY_CODE ) { $mfaRecoveryCodes = $user->getAttribute('mfaRecoveryCodes', []); if (in_array($otp, $mfaRecoveryCodes)) { @@ -4964,7 +4964,7 @@ App::put('/v1/account/mfa/challenge') Type::TOTP => Challenge\TOTP::challenge($challenge, $user, $otp), Type::PHONE => Challenge\Phone::challenge($challenge, $user, $otp), Type::EMAIL => Challenge\Email::challenge($challenge, $user, $otp), - \strtolower(Type::RECOVERY_CODE) => $recoveryCodeChallenge($challenge, $user, $otp), + Type::RECOVERY_CODE => $recoveryCodeChallenge($challenge, $user, $otp), default => false }); diff --git a/tests/e2e/Services/Account/AccountCustomClientTest.php b/tests/e2e/Services/Account/AccountCustomClientTest.php index 0993f68a58..c93f8ae034 100644 --- a/tests/e2e/Services/Account/AccountCustomClientTest.php +++ b/tests/e2e/Services/Account/AccountCustomClientTest.php @@ -3072,4 +3072,116 @@ class AccountCustomClientTest extends Scope $this->assertEquals('test-identifier-updated', $response['body']['identifier']); $this->assertEquals(false, $response['body']['expired']); } + + /** + * @depends testCreateAccount + */ + public function testMFARecoveryCodeChallenge($data): void + { + $email = $data['email'] ?? ''; + $password = $data['password'] ?? ''; + + // Create session first + $session = $this->client->call(Client::METHOD_POST, '/account/sessions/email', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ]), [ + 'email' => $email, + 'password' => $password, + ]); + + $this->assertEquals(201, $session['headers']['status-code']); + $sessionCookie = $session['cookies']['a_session_' . $this->getProject()['$id']]; + + // Generate recovery codes + $response = $this->client->call(Client::METHOD_POST, '/account/mfa/recovery-codes', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'cookie' => 'a_session_' . $this->getProject()['$id'] . '=' . $sessionCookie, + 'x-appwrite-project' => $this->getProject()['$id'], + ])); + + $this->assertEquals(200, $response['headers']['status-code']); + $this->assertNotEmpty($response['body']['recoveryCodes']); + $recoveryCodes = $response['body']['recoveryCodes']; + $this->assertGreaterThan(0, count($recoveryCodes)); + + // Create recovery code challenge + $challenge = $this->client->call(Client::METHOD_POST, '/account/mfa/challenge', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'cookie' => 'a_session_' . $this->getProject()['$id'] . '=' . $sessionCookie, + 'x-appwrite-project' => $this->getProject()['$id'], + ]), [ + 'factor' => 'recoveryCode' + ]); + + $this->assertEquals(201, $challenge['headers']['status-code']); + $this->assertNotEmpty($challenge['body']['$id']); + $challengeId = $challenge['body']['$id']; + + // Test SUCCESS: Verify with valid recovery code (this tests the bug fix) + $verification = $this->client->call(Client::METHOD_PUT, '/account/mfa/challenge', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'cookie' => 'a_session_' . $this->getProject()['$id'] . '=' . $sessionCookie, + 'x-appwrite-project' => $this->getProject()['$id'], + ]), [ + 'challengeId' => $challengeId, + 'otp' => $recoveryCodes[0] + ]); + + $this->assertEquals(200, $verification['headers']['status-code']); + $this->assertArrayHasKey('factors', $verification['body']); + $this->assertContains('recoveryCode', $verification['body']['factors']); + + // Test that the code was consumed (can't use again) + $challenge2 = $this->client->call(Client::METHOD_POST, '/account/mfa/challenge', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'cookie' => 'a_session_' . $this->getProject()['$id'] . '=' . $sessionCookie, + 'x-appwrite-project' => $this->getProject()['$id'], + ]), [ + 'factor' => 'recoveryCode' + ]); + + $this->assertEquals(201, $challenge2['headers']['status-code']); + + $verification2 = $this->client->call(Client::METHOD_PUT, '/account/mfa/challenge', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'cookie' => 'a_session_' . $this->getProject()['$id'] . '=' . $sessionCookie, + 'x-appwrite-project' => $this->getProject()['$id'], + ]), [ + 'challengeId' => $challenge2['body']['$id'], + 'otp' => $recoveryCodes[0] // Same code should fail + ]); + + $this->assertEquals(401, $verification2['headers']['status-code']); + + // Test FAILURE: Invalid recovery code + $challenge3 = $this->client->call(Client::METHOD_POST, '/account/mfa/challenge', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'cookie' => 'a_session_' . $this->getProject()['$id'] . '=' . $sessionCookie, + 'x-appwrite-project' => $this->getProject()['$id'], + ]), [ + 'factor' => 'recoveryCode' + ]); + + $this->assertEquals(201, $challenge3['headers']['status-code']); + + $verification3 = $this->client->call(Client::METHOD_PUT, '/account/mfa/challenge', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'cookie' => 'a_session_' . $this->getProject()['$id'] . '=' . $sessionCookie, + 'x-appwrite-project' => $this->getProject()['$id'], + ]), [ + 'challengeId' => $challenge3['body']['$id'], + 'otp' => 'invalid-code-123' + ]); + + $this->assertEquals(401, $verification3['headers']['status-code']); + } }