From 520d19e3352fc75cb651fc5d846af3cc4711aecc Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 21 Nov 2023 12:35:36 +0530 Subject: [PATCH 1/4] changes TextMagic to Textmagic in all places and uses email validator --- CHANGES.md | 2 +- app/config/variables.php | 2 +- app/controllers/api/messaging.php | 12 ++++++------ src/Appwrite/Platform/Workers/Messaging.php | 4 ++-- tests/e2e/Services/GraphQL/Base.php | 8 ++++---- tests/e2e/Services/GraphQL/MessagingTest.php | 4 ++-- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 889f65e1e7..fbe1e548db 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -466,7 +466,7 @@ ## Features - Added Phone Authentication by @TorstenDittmann in https://github.com/appwrite/appwrite/pull/3357 - Added Twilio Support - - Added TextMagic Support + - Added Textmagic Support - Added Telesign Support - Added Endpoint to create Phone Session (`POST:/v1/account/sessions/phone`) - Added Endpoint to confirm Phone Session (`PUT:/v1/account/sessions/phone`) diff --git a/app/config/variables.php b/app/config/variables.php index 6f49d4a5da..c9329f6d55 100644 --- a/app/config/variables.php +++ b/app/config/variables.php @@ -440,7 +440,7 @@ return [ 'variables' => [ [ 'name' => '_APP_SMS_PROVIDER', - 'description' => "Provider used for delivering SMS for Phone authentication. Use the following format: 'sms://[USER]:[SECRET]@[PROVIDER]'.\n\nEnsure `[USER]` and `[SECRET]` are URL encoded if they contain any non-alphanumeric characters.\n\nAvailable providers are twilio, textmagic, telesign, msg91, and vonage.", + 'description' => "Provider used for delivering SMS for Phone authentication. Use the following format: 'sms://[USER]:[SECRET]@[PROVIDER]'.\n\nEnsure `[USER]` and `[SECRET]` are URL encoded if they contain any non-alphanumeric characters.\n\nAvailable providers are twilio, Textmagic, telesign, msg91, and vonage.", 'introduction' => '0.15.0', 'default' => '', 'required' => false, diff --git a/app/controllers/api/messaging.php b/app/controllers/api/messaging.php index 35ba97c1af..536ebaf408 100644 --- a/app/controllers/api/messaging.php +++ b/app/controllers/api/messaging.php @@ -256,7 +256,7 @@ App::post('/v1/messaging/providers/telesign') }); App::post('/v1/messaging/providers/textmagic') - ->desc('Create TextMagic provider') + ->desc('Create Textmagic provider') ->groups(['api', 'messaging']) ->label('audits.event', 'provider.create') ->label('audits.resource', 'provider/{response.$id}') @@ -264,7 +264,7 @@ App::post('/v1/messaging/providers/textmagic') ->label('scope', 'providers.write') ->label('sdk.auth', [APP_AUTH_TYPE_ADMIN, APP_AUTH_TYPE_KEY]) ->label('sdk.namespace', 'messaging') - ->label('sdk.method', 'createTextMagicProvider') + ->label('sdk.method', 'createTextmagicProvider') ->label('sdk.description', '/docs/references/messaging/create-textmagic-provider.md') ->label('sdk.response.code', Response::STATUS_CODE_CREATED) ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) @@ -689,7 +689,7 @@ App::patch('/v1/messaging/providers/mailgun/:providerId') ->param('name', '', new Text(128), 'Provider name.', true) ->param('enabled', null, new Boolean(), 'Set as enabled.', true) ->param('isEuRegion', null, new Boolean(), 'Set as EU region.', true) - ->param('from', '', new Text(256), 'Sender email address.', true) + ->param('from', '', new Email(), 'Sender email address.', true) ->param('apiKey', '', new Text(0), 'Mailgun API Key.', true) ->param('domain', '', new Text(0), 'Mailgun Domain.', true) ->inject('queueForEvents') @@ -764,7 +764,7 @@ App::patch('/v1/messaging/providers/sendgrid/:providerId') ->param('name', '', new Text(128), 'Provider name.', true) ->param('enabled', null, new Boolean(), 'Set as enabled.', true) ->param('apiKey', '', new Text(0), 'Sendgrid API key.', true) - ->param('from', '', new Text(256), 'Sender email address.', true) + ->param('from', '', new Email(), 'Sender email address.', true) ->inject('queueForEvents') ->inject('dbForProject') ->inject('response') @@ -950,7 +950,7 @@ App::patch('/v1/messaging/providers/telesign/:providerId') }); App::patch('/v1/messaging/providers/textmagic/:providerId') - ->desc('Update TextMagic provider') + ->desc('Update Textmagic provider') ->groups(['api', 'messaging']) ->label('audits.event', 'provider.update') ->label('audits.resource', 'provider/{response.$id}') @@ -958,7 +958,7 @@ App::patch('/v1/messaging/providers/textmagic/:providerId') ->label('scope', 'providers.write') ->label('sdk.auth', [APP_AUTH_TYPE_ADMIN, APP_AUTH_TYPE_KEY]) ->label('sdk.namespace', 'messaging') - ->label('sdk.method', 'updateTextMagicProvider') + ->label('sdk.method', 'updateTextmagicProvider') ->label('sdk.description', '/docs/references/messaging/update-textmagic-provider.md') ->label('sdk.response.code', Response::STATUS_CODE_OK) ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) diff --git a/src/Appwrite/Platform/Workers/Messaging.php b/src/Appwrite/Platform/Workers/Messaging.php index 299ef4526f..0862f0b8fa 100644 --- a/src/Appwrite/Platform/Workers/Messaging.php +++ b/src/Appwrite/Platform/Workers/Messaging.php @@ -17,7 +17,7 @@ use Utopia\Messaging\Adapters\SMS as SMSAdapter; use Utopia\Messaging\Adapters\SMS\Mock; use Utopia\Messaging\Adapters\SMS\Msg91; use Utopia\Messaging\Adapters\SMS\Telesign; -use Utopia\Messaging\Adapters\SMS\TextMagic; +use Utopia\Messaging\Adapters\SMS\Textmagic; use Utopia\Messaging\Adapters\SMS\Twilio; use Utopia\Messaging\Adapters\SMS\Vonage; use Utopia\Messaging\Adapters\Push as PushAdapter; @@ -303,7 +303,7 @@ class Messaging extends Action return match ($provider->getAttribute('provider')) { 'mock' => new Mock('username', 'password'), 'twilio' => new Twilio($credentials['accountSid'], $credentials['authToken']), - 'textmagic' => new TextMagic($credentials['username'], $credentials['apiKey']), + 'textmagic' => new Textmagic($credentials['username'], $credentials['apiKey']), 'telesign' => new Telesign($credentials['username'], $credentials['password']), 'msg91' => new Msg91($credentials['senderId'], $credentials['authKey']), 'vonage' => new Vonage($credentials['apiKey'], $credentials['apiSecret']), diff --git a/tests/e2e/Services/GraphQL/Base.php b/tests/e2e/Services/GraphQL/Base.php index 8655395dc2..3777d67b73 100644 --- a/tests/e2e/Services/GraphQL/Base.php +++ b/tests/e2e/Services/GraphQL/Base.php @@ -1830,8 +1830,8 @@ trait Base } }'; case self::$CREATE_TEXTMAGIC_PROVIDER: - return 'mutation createTextMagicProvider($providerId: String!, $name: String!, $from: String!, $username: String!, $apiKey: String!) { - messagingCreateTextMagicProvider(providerId: $providerId, name: $name, from: $from, username: $username, apiKey: $apiKey) { + return 'mutation createTextmagicProvider($providerId: String!, $name: String!, $from: String!, $username: String!, $apiKey: String!) { + messagingCreateTextmagicProvider(providerId: $providerId, name: $name, from: $from, username: $username, apiKey: $apiKey) { _id name provider @@ -1944,8 +1944,8 @@ trait Base } }'; case self::$UPDATE_TEXTMAGIC_PROVIDER: - return 'mutation updateTextMagicProvider($providerId: String!, $name: String!, $username: String!, $apiKey: String!) { - messagingUpdateTextMagicProvider(providerId: $providerId, name: $name, username: $username, apiKey: $apiKey) { + return 'mutation updateTextmagicProvider($providerId: String!, $name: String!, $username: String!, $apiKey: String!) { + messagingUpdateTextmagicProvider(providerId: $providerId, name: $name, username: $username, apiKey: $apiKey) { _id name provider diff --git a/tests/e2e/Services/GraphQL/MessagingTest.php b/tests/e2e/Services/GraphQL/MessagingTest.php index 27b2b52cd3..e5a4ca3a72 100644 --- a/tests/e2e/Services/GraphQL/MessagingTest.php +++ b/tests/e2e/Services/GraphQL/MessagingTest.php @@ -47,7 +47,7 @@ class MessagingTest extends Scope 'password' => 'my-password', 'from' => '+123456789', ], - 'TextMagic' => [ + 'Textmagic' => [ 'providerId' => ID::unique(), 'name' => 'Textmagic1', 'username' => 'my-username', @@ -134,7 +134,7 @@ class MessagingTest extends Scope 'username' => 'my-username', 'password' => 'my-password', ], - 'TextMagic' => [ + 'Textmagic' => [ 'providerId' => $providers[4]['_id'], 'name' => 'Textmagic2', 'username' => 'my-username', From 3394218cb0164585094f21d922f8ee1ebc084728 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 21 Nov 2023 15:00:02 +0530 Subject: [PATCH 2/4] adds validation for phone and email identifier --- app/config/errors.php | 10 ++++++++++ app/controllers/api/users.php | 25 +++++++++++++++++++++++++ src/Appwrite/Extend/Exception.php | 2 ++ 3 files changed, 37 insertions(+) diff --git a/app/config/errors.php b/app/config/errors.php index f7c9e8e55f..3c0b67c76a 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -103,6 +103,16 @@ return [ 'description' => 'This method was not fully implemented yet. If you believe this is a mistake, please upgrade your Appwrite server version.', 'code' => 405, ], + Exception::GENERAL_INVALID_EMAIL => [ + 'name' => Exception::GENERAL_INVALID_EMAIL, + 'description' => 'Value must be a valid email address.', + 'code' => 400, + ], + Exception::GENERAL_INVALID_PHONE => [ + 'name' => Exception::GENERAL_INVALID_PHONE, + 'description' => 'Value must be a valid phone number. Format this number with a leading \'+\' and a country code, e.g., +16175551212.', + 'code' => 400, + ], /** User Errors */ Exception::USER_COUNT_EXCEEDED => [ diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index d9968c83a7..992a28ba8e 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -415,6 +415,18 @@ App::post('/v1/users/:userId/targets') } } + if ($providerType === 'email') { + $validator = new Email(); + if (!$validator->isValid($identifier)) { + throw new Exception(Exception::GENERAL_INVALID_EMAIL); + } + } elseif ($providerType === 'sms') { + $validator = new Phone(); + if (!$validator->isValid($identifier)) { + throw new Exception(Exception::GENERAL_INVALID_PHONE); + } + } + $user = $dbForProject->getDocument('users', $userId); if ($user->isEmpty()) { @@ -1270,6 +1282,19 @@ App::patch('/v1/users/:userId/targets/:targetId') } if ($identifier) { + $providerType = $target->getAttribute('providerType'); + if ($providerType === 'email') { + $emailValidator = new Email(); + if (!$emailValidator->isValid($identifier)) { + throw new Exception(Exception::GENERAL_INVALID_EMAIL); + } + } elseif ($providerType === 'sms') { + $phoneValidator = new Phone(); + if (!$phoneValidator->isValid($identifier)) { + throw new Exception(Exception::GENERAL_INVALID_PHONE); + } + } + $target->setAttribute('identifier', $identifier); } diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index 3869444752..61ea597941 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -55,6 +55,8 @@ class Exception extends \Exception public const GENERAL_CODES_DISABLED = 'general_codes_disabled'; public const GENERAL_USAGE_DISABLED = 'general_usage_disabled'; public const GENERAL_NOT_IMPLEMENTED = 'general_not_implemented'; + public const GENERAL_INVALID_EMAIL = 'general_invalid_email'; + public const GENERAL_INVALID_PHONE = 'general_invalid_phone'; /** Users */ public const USER_COUNT_EXCEEDED = 'user_count_exceeded'; From 88f228c106889805aa91f7087edfdb7a3a69ed5d Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 21 Nov 2023 15:11:09 +0530 Subject: [PATCH 3/4] review changes --- app/controllers/api/users.php | 51 +++++++++++-------- tests/e2e/Services/GraphQL/MessagingTest.php | 4 +- .../e2e/Services/Messaging/MessagingBase.php | 2 +- tests/e2e/Services/Users/UsersBase.php | 8 +-- 4 files changed, 38 insertions(+), 27 deletions(-) diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index 992a28ba8e..c339b77fed 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -415,16 +415,21 @@ App::post('/v1/users/:userId/targets') } } - if ($providerType === 'email') { - $validator = new Email(); - if (!$validator->isValid($identifier)) { - throw new Exception(Exception::GENERAL_INVALID_EMAIL); - } - } elseif ($providerType === 'sms') { - $validator = new Phone(); - if (!$validator->isValid($identifier)) { - throw new Exception(Exception::GENERAL_INVALID_PHONE); - } + switch ($providerType) { + case 'email': + $validator = new Email(); + if (!$validator->isValid($identifier)) { + throw new Exception(Exception::GENERAL_INVALID_EMAIL); + } + break; + case 'sms': + $validator = new Phone(); + if (!$validator->isValid($identifier)) { + throw new Exception(Exception::GENERAL_INVALID_PHONE); + } + break; + default: + throw new Exception(Exception::PROVIDER_INCORRECT_TYPE); } $user = $dbForProject->getDocument('users', $userId); @@ -1283,16 +1288,22 @@ App::patch('/v1/users/:userId/targets/:targetId') if ($identifier) { $providerType = $target->getAttribute('providerType'); - if ($providerType === 'email') { - $emailValidator = new Email(); - if (!$emailValidator->isValid($identifier)) { - throw new Exception(Exception::GENERAL_INVALID_EMAIL); - } - } elseif ($providerType === 'sms') { - $phoneValidator = new Phone(); - if (!$phoneValidator->isValid($identifier)) { - throw new Exception(Exception::GENERAL_INVALID_PHONE); - } + + switch ($providerType) { + case 'email': + $validator = new Email(); + if (!$validator->isValid($identifier)) { + throw new Exception(Exception::GENERAL_INVALID_EMAIL); + } + break; + case 'sms': + $validator = new Phone(); + if (!$validator->isValid($identifier)) { + throw new Exception(Exception::GENERAL_INVALID_PHONE); + } + break; + default: + throw new Exception(Exception::PROVIDER_INCORRECT_TYPE); } $target->setAttribute('identifier', $identifier); diff --git a/tests/e2e/Services/GraphQL/MessagingTest.php b/tests/e2e/Services/GraphQL/MessagingTest.php index e5a4ca3a72..e3ca99f2ac 100644 --- a/tests/e2e/Services/GraphQL/MessagingTest.php +++ b/tests/e2e/Services/GraphQL/MessagingTest.php @@ -398,7 +398,7 @@ class MessagingTest extends Scope 'providerType' => 'email', 'userId' => $userId, 'providerId' => $providerId, - 'identifier' => 'token', + 'identifier' => 'random-email@mail.org', ], ]; $response = $this->client->call(Client::METHOD_POST, '/graphql', \array_merge([ @@ -409,7 +409,7 @@ class MessagingTest extends Scope $this->assertEquals(200, $response['headers']['status-code']); $this->assertEquals($userId, $response['body']['data']['usersCreateTarget']['userId']); - $this->assertEquals('token', $response['body']['data']['usersCreateTarget']['identifier']); + $this->assertEquals('random-email@mail.org', $response['body']['data']['usersCreateTarget']['identifier']); $targetId = $response['body']['data']['usersCreateTarget']['_id']; diff --git a/tests/e2e/Services/Messaging/MessagingBase.php b/tests/e2e/Services/Messaging/MessagingBase.php index 1f09d95522..37a581b088 100644 --- a/tests/e2e/Services/Messaging/MessagingBase.php +++ b/tests/e2e/Services/Messaging/MessagingBase.php @@ -319,7 +319,7 @@ trait MessagingBase 'targetId' => ID::unique(), 'providerType' => 'email', 'providerId' => $provider['body']['$id'], - 'identifier' => 'my-token', + 'identifier' => 'random-email@mail.org', ]); $this->assertEquals(201, $target['headers']['status-code']); diff --git a/tests/e2e/Services/Users/UsersBase.php b/tests/e2e/Services/Users/UsersBase.php index c9cb17d7b5..70230b8c8a 100644 --- a/tests/e2e/Services/Users/UsersBase.php +++ b/tests/e2e/Services/Users/UsersBase.php @@ -1245,11 +1245,11 @@ trait UsersBase 'targetId' => ID::unique(), 'providerId' => $provider['body']['$id'], 'providerType' => 'email', - 'identifier' => 'my-token', + 'identifier' => 'random-email@mail.org', ]); $this->assertEquals(201, $response['headers']['status-code']); $this->assertEquals($provider['body']['$id'], $response['body']['providerId']); - $this->assertEquals('my-token', $response['body']['identifier']); + $this->assertEquals('random-email@mail.org', $response['body']['identifier']); return $response['body']; } @@ -1262,10 +1262,10 @@ trait UsersBase 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ - 'identifier' => 'my-updated-token', + 'identifier' => 'random-email1@mail.org', ]); $this->assertEquals(200, $response['headers']['status-code']); - $this->assertEquals('my-updated-token', $response['body']['identifier']); + $this->assertEquals('random-email1@mail.org', $response['body']['identifier']); return $response['body']; } From 9c6cd8512faf52438b2186f494b03d83f1976ffc Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 21 Nov 2023 15:19:19 +0530 Subject: [PATCH 4/4] review changes --- app/controllers/api/users.php | 4 ++++ tests/e2e/Services/GraphQL/UsersTest.php | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index c339b77fed..0c1edb6a96 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -428,6 +428,8 @@ App::post('/v1/users/:userId/targets') throw new Exception(Exception::GENERAL_INVALID_PHONE); } break; + case 'push': + break; default: throw new Exception(Exception::PROVIDER_INCORRECT_TYPE); } @@ -1302,6 +1304,8 @@ App::patch('/v1/users/:userId/targets/:targetId') throw new Exception(Exception::GENERAL_INVALID_PHONE); } break; + case 'push': + break; default: throw new Exception(Exception::PROVIDER_INCORRECT_TYPE); } diff --git a/tests/e2e/Services/GraphQL/UsersTest.php b/tests/e2e/Services/GraphQL/UsersTest.php index 59c0c4a805..3b49337a63 100644 --- a/tests/e2e/Services/GraphQL/UsersTest.php +++ b/tests/e2e/Services/GraphQL/UsersTest.php @@ -80,7 +80,7 @@ class UsersTest extends Scope 'userId' => $user['_id'], 'providerType' => 'email', 'providerId' => $providerId, - 'identifier' => 'identifier', + 'identifier' => 'random-email@mail.org', ] ]; @@ -90,7 +90,7 @@ class UsersTest extends Scope ], $this->getHeaders()), $graphQLPayload); $this->assertEquals(200, $target['headers']['status-code']); - $this->assertEquals('identifier', $target['body']['data']['usersCreateTarget']['identifier']); + $this->assertEquals('random-email@mail.org', $target['body']['data']['usersCreateTarget']['identifier']); return $target['body']['data']['usersCreateTarget']; } @@ -271,7 +271,7 @@ class UsersTest extends Scope ], $this->getHeaders()), $graphQLPayload); $this->assertEquals(200, $target['headers']['status-code']); - $this->assertEquals('identifier', $target['body']['data']['usersGetTarget']['identifier']); + $this->assertEquals('random-email@mail.org', $target['body']['data']['usersGetTarget']['identifier']); } public function testUpdateUserStatus() @@ -470,7 +470,7 @@ class UsersTest extends Scope 'variables' => [ 'userId' => $target['userId'], 'targetId' => $target['_id'], - 'identifier' => 'newidentifier', + 'identifier' => 'random-email1@mail.org', ], ]; @@ -480,7 +480,7 @@ class UsersTest extends Scope ], $this->getHeaders()), $graphQLPayload); $this->assertEquals(200, $target['headers']['status-code']); - $this->assertEquals('newidentifier', $target['body']['data']['usersUpdateTarget']['identifier']); + $this->assertEquals('random-email1@mail.org', $target['body']['data']['usersUpdateTarget']['identifier']); } public function testDeleteUserSessions()