diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 5563fc6a59..0d2e210ed9 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -4421,7 +4421,9 @@ App::post('/v1/account/mfa/recovery-codes') 'recoveryCodes' => $mfaRecoveryCodes ]); - $response->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); + $response + ->setStatusCode(Response::STATUS_CODE_CREATED) + ->dynamic($document, Response::MODEL_MFA_RECOVERY_CODES); }); App::patch('/v1/account/mfa/recovery-codes') @@ -4874,7 +4876,9 @@ App::post('/v1/account/mfa/challenge') ->setParam('userId', $user->getId()) ->setParam('challengeId', $challenge->getId()); - $response->dynamic($challenge, Response::MODEL_MFA_CHALLENGE); + $response + ->setStatusCode(Response::STATUS_CODE_CREATED) + ->dynamic($challenge, Response::MODEL_MFA_CHALLENGE); }); App::put('/v1/account/mfa/challenge') @@ -4942,7 +4946,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 +4968,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..4564d90e23 100644 --- a/tests/e2e/Services/Account/AccountCustomClientTest.php +++ b/tests/e2e/Services/Account/AccountCustomClientTest.php @@ -3072,4 +3072,83 @@ class AccountCustomClientTest extends Scope $this->assertEquals('test-identifier-updated', $response['body']['identifier']); $this->assertEquals(false, $response['body']['expired']); } + + public function testMFARecoveryCodeChallenge(): void + { + // Generate recovery codes using existing authenticated session + $response = $this->client->call(Client::METHOD_POST, '/account/mfa/recovery-codes', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), []); + + $this->assertEquals(201, $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([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + '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([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + '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([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'factor' => 'recoveryCode' + ]); + + $this->assertEquals(201, $challenge2['headers']['status-code']); + + $verification2 = $this->client->call(Client::METHOD_PUT, '/account/mfa/challenge', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + '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([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'factor' => 'recoveryCode' + ]); + + $this->assertEquals(201, $challenge3['headers']['status-code']); + + $verification3 = $this->client->call(Client::METHOD_PUT, '/account/mfa/challenge', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'challengeId' => $challenge3['body']['$id'], + 'otp' => 'invalid-code-123' + ]); + + $this->assertEquals(401, $verification3['headers']['status-code']); + } }