From 711ca8801a3c0a21429ebc354462b725e5ee85a8 Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Sun, 21 Jul 2024 23:30:06 +0400 Subject: [PATCH] feat: address review comments --- app/controllers/api/projects.php | 20 ++++++++++ src/Appwrite/Auth/Validator/MockNumber.php | 4 +- .../Projects/ProjectsConsoleClientTest.php | 38 ++++++++++++++++++- 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index af588fffc4..1569b4685e 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -882,6 +882,26 @@ App::patch('/v1/projects/:projectId/auth/mock-numbers') throw new Exception(Exception::PROJECT_NOT_FOUND); } + /* Ensure there are no duplicate numbers . Numbers have a structure. Throw an exception if there are duplicate numbers + [ + { + "number": "+1234567890", + "code": "123456" + }, + { + "number": "+1234567891", + "code": "123456" + } + ] + */ + $uniqueNumbers = []; + foreach ($numbers as $number) { + if (isset($uniqueNumbers[$number['number']])) { + throw new Exception(Exception::GENERAL_BAD_REQUEST, 'Duplicate numbers are not allowed.'); + } + $uniqueNumbers[$number['number']] = $number['code']; + } + $auths = $project->getAttribute('auths', []); $auths['mockNumbers'] = $numbers; diff --git a/src/Appwrite/Auth/Validator/MockNumber.php b/src/Appwrite/Auth/Validator/MockNumber.php index 2c1c81f863..6f1cf81cae 100644 --- a/src/Appwrite/Auth/Validator/MockNumber.php +++ b/src/Appwrite/Auth/Validator/MockNumber.php @@ -47,8 +47,8 @@ class MockNumber extends Validator } $otp = new Text(6, 6); - if (!$otp->isValid($value['otp'])) { - $this->message = 'OTP must be a valid string and exactly 6 characters.'; + if (!$otp->isValid($value['otp']) || !\ctype_digit($value['otp'])) { + $this->message = 'Invalid OTP. Please make sure the OTP is a 6 digit number'; return false; } diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 753322f168..7be97e2136 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -1592,7 +1592,7 @@ class ProjectsConsoleClientTest extends Scope ] ]); $this->assertEquals(400, $response['headers']['status-code']); - $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and OTP must be a valid string and exactly 6 characters.', $response['body']['message']); + $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Invalid OTP. Please make sure the OTP is a 6 digit number', $response['body']['message']); /** Trying to pass an OTP shorter than 6 characters*/ $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/mock-numbers', array_merge([ @@ -1607,7 +1607,22 @@ class ProjectsConsoleClientTest extends Scope ] ]); $this->assertEquals(400, $response['headers']['status-code']); - $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and OTP must be a valid string and exactly 6 characters.', $response['body']['message']); + $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Invalid OTP. Please make sure the OTP is a 6 digit number', $response['body']['message']); + + /** Trying to pass an OTP with non numeric characters */ + $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/mock-numbers', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'numbers' => [ + [ + 'phone' => '+1655513432', + 'otp' => '123re2' + ] + ] + ]); + $this->assertEquals(400, $response['headers']['status-code']); + $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Invalid OTP. Please make sure the OTP is a 6 digit number', $response['body']['message']); /** Trying to pass an invalid phone number */ $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/mock-numbers', array_merge([ @@ -1639,6 +1654,25 @@ class ProjectsConsoleClientTest extends Scope $this->assertEquals(400, $response['headers']['status-code']); $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Phone number must start with a \'+\' can have a maximum of fifteen digits.', $response['body']['message']); + /** Trying to pass duplicate numbers */ + $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/mock-numbers', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'numbers' => [ + [ + 'phone' => '+1655513432', + 'otp' => '123456' + ], + [ + 'phone' => '+1655513432', + 'otp' => '123456' + ] + ] + ]); + $this->assertEquals(400, $response['headers']['status-code']); + $this->assertEquals('Invalid `numbers` param: Value must a valid array no longer than 10 items and Phone number must start with a \'+\' can have a maximum of fifteen digits.', $response['body']['message']); + $numbers = []; for ($i = 0; $i < 11; $i++) { $numbers[] = [