From ee4c2d0e0dfcb9115f595458ab381dce49478572 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 28 Nov 2023 18:42:34 +0530 Subject: [PATCH 1/2] adds target when creating user via server endpoint --- app/controllers/api/account.php | 89 +++++++++++++-------- app/controllers/api/users.php | 89 ++++++++++++++++++++- src/Appwrite/Platform/Workers/Messaging.php | 24 +++--- tests/e2e/Services/GraphQL/UsersTest.php | 2 +- tests/e2e/Services/Users/UsersBase.php | 5 +- 5 files changed, 156 insertions(+), 53 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index eda29656f..7034c9391 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -149,18 +149,20 @@ App::post('/v1/account') ]); $user->removeAttribute('$internalId'); $user = Authorization::skip(fn() => $dbForProject->createDocument('users', $user)); - $target = Authorization::skip(fn() => $dbForProject->createDocument('targets', new Document([ - '$permissions' => [ - Permission::read(Role::any()), - Permission::update(Role::user($userId)), - Permission::delete(Role::user($userId)), - ], - 'userId' => $user->getId(), - 'userInternalId' => $user->getInternalId(), - 'providerType' => 'email', - 'identifier' => $email, - ]))); - $user->setAttribute('targets', [$target]); + try { + $target = Authorization::skip(fn() => $dbForProject->createDocument('targets', new Document([ + 'userId' => $user->getId(), + 'userInternalId' => $user->getInternalId(), + 'providerType' => 'email', + 'identifier' => $email, + ]))); + $user->setAttribute('targets', [...$user->getAttribute('targets', []), $target]); + } catch (Duplicate) { + $existingTarget = $dbForProject->findOne('targets', [ + Query::equal('identifier', [$email]), + ]); + $user->setAttribute('targets', [...$user->getAttribute('targets', []), $existingTarget]); + } $dbForProject->deleteCachedDocument('users', $user->getId()); } catch (Duplicate) { throw new Exception(Exception::USER_ALREADY_EXISTS); @@ -1309,6 +1311,21 @@ App::post('/v1/account/sessions/phone') $user->removeAttribute('$internalId'); Authorization::skip(fn () => $dbForProject->createDocument('users', $user)); + try { + $target = Authorization::skip(fn() => $dbForProject->createDocument('targets', new Document([ + 'userId' => $user->getId(), + 'userInternalId' => $user->getInternalId(), + 'providerType' => 'sms', + 'identifier' => $phone, + ]))); + $user->setAttribute('targets', [...$user->getAttribute('targets', []), $target]); + } catch (Duplicate) { + $existingTarget = $dbForProject->findOne('targets', [ + Query::equal('identifier', [$phone]), + ]); + $user->setAttribute('targets', [...$user->getAttribute('targets', []), $existingTarget]); + } + $dbForProject->deleteCachedDocument('users', $user->getId()); } $secret = Auth::codeGenerator(); @@ -2102,25 +2119,25 @@ App::patch('/v1/account/email') ->setAttribute('passwordUpdate', DateTime::now()); } - $target = $dbForProject->findOne('targets', [ + $target = Authorization::skip(fn () => $dbForProject->findOne('targets', [ Query::equal('identifier', [$email]), - ]); + ])); - if ($target && !$target->isEmpty()) { + if ($target instanceof Document && !$target->isEmpty()) { throw new Exception(Exception::USER_TARGET_ALREADY_EXISTS); } - /** - * @var Document $oldTarget - */ - $oldTarget = $user->find('identifier', $oldEmail, 'targets'); - - if ($oldTarget !== false && !$oldTarget->isEmpty()) { - $dbForProject->updateDocument('targets', $oldTarget->getId(), $oldTarget->setAttribute('identifier', $email)); - } - try { $user = $dbForProject->withRequestTimestamp($requestTimestamp, fn () => $dbForProject->updateDocument('users', $user->getId(), $user)); + /** + * @var Document $oldTarget + */ + $oldTarget = $user->find('identifier', $oldEmail, 'targets'); + + if ($oldTarget instanceof Document && !$oldTarget->isEmpty()) { + Authorization::skip(fn () => $dbForProject->updateDocument('targets', $oldTarget->getId(), $oldTarget->setAttribute('identifier', $email))); + } + $dbForProject->deleteCachedDocument('users', $user->getId()); } catch (Duplicate) { throw new Exception(Exception::USER_EMAIL_ALREADY_EXISTS); } @@ -2165,18 +2182,15 @@ App::patch('/v1/account/phone') throw new Exception(Exception::USER_INVALID_CREDENTIALS); } - $target = $dbForProject->findOne('targets', [ + $target = Authorization::skip(fn () => $dbForProject->findOne('targets', [ Query::equal('identifier', [$phone]), - ]); + ])); - if ($target && !$target->isEmpty()) { + if ($target instanceof Document && !$target->isEmpty()) { throw new Exception(Exception::USER_TARGET_ALREADY_EXISTS); } - /** - * @var Document $oldTarget - */ - $oldTarget = $user->find('identifier', $user->getAttribute('phone'), 'targets'); + $oldPhone = $user->getAttribute('phone'); $user ->setAttribute('phone', $phone) @@ -2191,12 +2205,17 @@ App::patch('/v1/account/phone') ->setAttribute('passwordUpdate', DateTime::now()); } - if ($oldTarget !== false && !$oldTarget->isEmpty()) { - $dbForProject->updateDocument('targets', $oldTarget->getId(), $oldTarget->setAttribute('identifier', $phone)); - } - try { $user = $dbForProject->withRequestTimestamp($requestTimestamp, fn () => $dbForProject->updateDocument('users', $user->getId(), $user)); + /** + * @var Document $oldTarget + */ + $oldTarget = $user->find('identifier', $oldPhone, 'targets'); + + if ($oldTarget instanceof Document && !$oldTarget->isEmpty()) { + Authorization::skip(fn () => $dbForProject->updateDocument('targets', $oldTarget->getId(), $oldTarget->setAttribute('identifier', $phone))); + } + $dbForProject->deleteCachedDocument('users', $user->getId()); } catch (Duplicate $th) { throw new Exception(Exception::USER_PHONE_ALREADY_EXISTS); } diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index 0c1edb6a9..0b099cb8d 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -99,6 +99,42 @@ function createUser(string $hash, mixed $hashOptions, string $userId, ?string $e 'memberships' => null, 'search' => implode(' ', [$userId, $email, $phone, $name]), ])); + + if ($email) { + try { + $target = $dbForProject->createDocument('targets', new Document([ + 'userId' => $user->getId(), + 'userInternalId' => $user->getInternalId(), + 'providerType' => 'email', + 'identifier' => $email, + ])); + $user->setAttribute('targets', [...$user->getAttribute('targets', []), $target]); + } catch (Duplicate) { + $existingTarget = $dbForProject->findOne('targets', [ + Query::equal('identifier', [$email]), + ]); + $user->setAttribute('targets', [...$user->getAttribute('targets', []), $existingTarget]); + } + } + + if ($phone) { + try { + $target = $dbForProject->createDocument('targets', new Document([ + 'userId' => $user->getId(), + 'userInternalId' => $user->getInternalId(), + 'providerType' => 'sms', + 'identifier' => $phone, + ])); + $user->setAttribute('targets', [...$user->getAttribute('targets', []), $target]); + } catch (Duplicate) { + $existingTarget = $dbForProject->findOne('targets', [ + Query::equal('identifier', [$phone]), + ]); + $user->setAttribute('targets', [...$user->getAttribute('targets', []), $existingTarget]); + } + } + + $dbForProject->deleteCachedDocument('users', $user->getId()); } catch (Duplicate $th) { throw new Exception(Exception::USER_ALREADY_EXISTS); } @@ -399,10 +435,11 @@ App::post('/v1/users/:userId/targets') ->param('providerType', '', new WhiteList(['email', 'sms', 'push']), 'The target provider type. Can be one of the following: `email`, `sms` or `push`.') ->param('identifier', '', new Text(Database::LENGTH_KEY), 'The target identifier (token, email, phone etc.)') ->param('providerId', '', new UID(), 'Provider ID. Message will be sent to this target from the specified provider ID. If no provider ID is set the first setup provider will be used.', true) + ->param('name', '', new Text(128), 'Target name. Max length: 128 chars. For example: My Awesome App Galaxy S23.', true) ->inject('queueForEvents') ->inject('response') ->inject('dbForProject') - ->action(function (string $targetId, string $userId, string $providerType, string $identifier, string $providerId, Event $queueForEvents, Response $response, Database $dbForProject) { + ->action(function (string $targetId, string $userId, string $providerType, string $identifier, string $providerId, string $name, Event $queueForEvents, Response $response, Database $dbForProject) { $targetId = $targetId == 'unique()' ? ID::unique() : $targetId; $provider = new Document(); @@ -455,6 +492,7 @@ App::post('/v1/users/:userId/targets') 'userId' => $userId, 'userInternalId' => $user->getInternalId(), 'identifier' => $identifier, + 'name' => ($name !== '') ? $name : null, ])); } catch (Duplicate) { throw new Exception(Exception::USER_TARGET_ALREADY_EXISTS); @@ -801,7 +839,7 @@ App::get('/v1/users/:userId/targets') if ($cursor) { $targetId = $cursor->getValue(); - $cursorDocument = Authorization::skip(fn () => $dbForProject->getDocument('targets', $targetId)); + $cursorDocument = $dbForProject->getDocument('targets', $targetId); if ($cursorDocument->isEmpty()) { throw new Exception(Exception::GENERAL_CURSOR_NOT_FOUND, "Target '{$targetId}' for the 'cursor' value not found."); @@ -1119,6 +1157,16 @@ App::patch('/v1/users/:userId/email') throw new Exception(Exception::USER_EMAIL_ALREADY_EXISTS); } + $target = $dbForProject->findOne('targets', [ + Query::equal('identifier', [$email]), + ]); + + if ($target && !$target->isEmpty()) { + throw new Exception(Exception::USER_TARGET_ALREADY_EXISTS); + } + + $oldEmail = $user->getAttribute('email'); + $user ->setAttribute('email', $email) ->setAttribute('emailVerification', false) @@ -1127,6 +1175,15 @@ App::patch('/v1/users/:userId/email') try { $user = $dbForProject->updateDocument('users', $user->getId(), $user); + /** + * @var Document $oldTarget + */ + $oldTarget = $user->find('identifier', $oldEmail, 'targets'); + + if ($oldTarget instanceof Document && !$oldTarget->isEmpty()) { + $dbForProject->updateDocument('targets', $oldTarget->getId(), $oldTarget->setAttribute('identifier', $email)); + } + $dbForProject->deleteCachedDocument('users', $user->getId()); } catch (Duplicate $th) { throw new Exception(Exception::USER_EMAIL_ALREADY_EXISTS); } @@ -1164,13 +1221,32 @@ App::patch('/v1/users/:userId/phone') throw new Exception(Exception::USER_NOT_FOUND); } + $oldPhone = $user->getAttribute('phone'); + $user ->setAttribute('phone', $number) ->setAttribute('phoneVerification', false) ; + $target = $dbForProject->findOne('targets', [ + Query::equal('identifier', [$number]), + ]); + + if ($target && !$target->isEmpty()) { + throw new Exception(Exception::USER_TARGET_ALREADY_EXISTS); + } + try { $user = $dbForProject->updateDocument('users', $user->getId(), $user); + /** + * @var Document $oldTarget + */ + $oldTarget = $user->find('identifier', $oldPhone, 'targets'); + + if ($oldTarget instanceof Document && !$oldTarget->isEmpty()) { + $dbForProject->updateDocument('targets', $oldTarget->getId(), $oldTarget->setAttribute('identifier', $number)); + } + $dbForProject->deleteCachedDocument('users', $user->getId()); } catch (Duplicate $th) { throw new Exception(Exception::USER_PHONE_ALREADY_EXISTS); } @@ -1266,12 +1342,13 @@ App::patch('/v1/users/:userId/targets/:targetId') ->label('sdk.response.model', Response::MODEL_TARGET) ->param('userId', '', new UID(), 'User ID.') ->param('targetId', '', new UID(), 'Target ID.') - ->param('providerId', '', new UID(), 'Provider ID. Message will be sent to this target from the specified provider ID. If no provider ID is set the first setup provider will be used.', true) ->param('identifier', '', new Text(Database::LENGTH_KEY), 'The target identifier (token, email, phone etc.)', true) + ->param('providerId', '', new UID(), 'Provider ID. Message will be sent to this target from the specified provider ID. If no provider ID is set the first setup provider will be used.', true) + ->param('name', '', new Text(128), 'Target name. Max length: 128 chars. For example: My Awesome App Galaxy S23.', true) ->inject('queueForEvents') ->inject('response') ->inject('dbForProject') - ->action(function (string $userId, string $targetId, string $providerId, string $identifier, Event $queueForEvents, Response $response, Database $dbForProject) { + ->action(function (string $userId, string $targetId, string $identifier, string $providerId, string $name, Event $queueForEvents, Response $response, Database $dbForProject) { $user = $dbForProject->getDocument('users', $userId); if ($user->isEmpty()) { @@ -1324,6 +1401,10 @@ App::patch('/v1/users/:userId/targets/:targetId') $target->setAttribute('providerInternalId', $provider->getInternalId()); } + if ($name) { + $target->setAttribute('name', $name); + } + $target = $dbForProject->updateDocument('targets', $target->getId(), $target); $dbForProject->deleteCachedDocument('users', $user->getId()); diff --git a/src/Appwrite/Platform/Workers/Messaging.php b/src/Appwrite/Platform/Workers/Messaging.php index 8488d8a9e..0414bbaf5 100644 --- a/src/Appwrite/Platform/Workers/Messaging.php +++ b/src/Appwrite/Platform/Workers/Messaging.php @@ -107,7 +107,7 @@ class Messaging extends Action $recipients = \array_merge($recipients, $targets); } - $internalProvider = $dbForProject->findOne('providers', [ + $primaryProvider = $dbForProject->findOne('providers', [ Query::equal('enabled', [true]), Query::equal('type', [$recipients[0]->getAttribute('providerType')]), ]); @@ -124,30 +124,32 @@ class Messaging extends Action foreach ($recipients as $recipient) { $providerId = $recipient->getAttribute('providerId'); - if (!$providerId) { - $providerId = $internalProvider->getId(); + if (!$providerId && $primaryProvider instanceof Document && !$primaryProvider->isEmpty()) { + $providerId = $primaryProvider->getId(); } - if (!isset($identifiersByProviderId[$providerId])) { - $identifiersByProviderId[$providerId] = []; + if ($providerId) { + if (!isset($identifiersByProviderId[$providerId])) { + $identifiersByProviderId[$providerId] = []; + } + $identifiersByProviderId[$providerId][] = $recipient->getAttribute('identifier'); } - $identifiersByProviderId[$providerId][] = $recipient->getAttribute('identifier'); } /** * @var array[] $results */ - $results = batch(\array_map(function ($providerId) use ($identifiersByProviderId, $providers, $internalProvider, $message, $dbForProject) { - return function () use ($providerId, $identifiersByProviderId, $providers, $internalProvider, $message, $dbForProject) { + $results = batch(\array_map(function ($providerId) use ($identifiersByProviderId, $providers, $primaryProvider, $message, $dbForProject) { + return function () use ($providerId, $identifiersByProviderId, $providers, $primaryProvider, $message, $dbForProject) { $provider = new Document(); - if ($internalProvider->getId() === $providerId) { - $provider = $internalProvider; + if ($primaryProvider->getId() === $providerId) { + $provider = $primaryProvider; } else { $provider = $dbForProject->getDocument('providers', $providerId, [Query::equal('enabled', [true])]); if ($provider->isEmpty()) { - $provider = $internalProvider; + $provider = $primaryProvider; } } diff --git a/tests/e2e/Services/GraphQL/UsersTest.php b/tests/e2e/Services/GraphQL/UsersTest.php index 3b49337a6..1dac9123a 100644 --- a/tests/e2e/Services/GraphQL/UsersTest.php +++ b/tests/e2e/Services/GraphQL/UsersTest.php @@ -247,7 +247,7 @@ class UsersTest extends Scope $this->assertEquals(200, $targets['headers']['status-code']); $this->assertIsArray($targets['body']['data']['usersListTargets']); - $this->assertCount(1, $targets['body']['data']['usersListTargets']['targets']); + $this->assertCount(2, $targets['body']['data']['usersListTargets']['targets']); } /** diff --git a/tests/e2e/Services/Users/UsersBase.php b/tests/e2e/Services/Users/UsersBase.php index 70230b8c8..7d22f2325 100644 --- a/tests/e2e/Services/Users/UsersBase.php +++ b/tests/e2e/Services/Users/UsersBase.php @@ -23,6 +23,7 @@ trait UsersBase 'password' => 'password', 'name' => 'Cristiano Ronaldo', ], false); + $this->assertEquals($user['headers']['status-code'], 201); // Test empty prefs is object not array $bodyString = $user['body']; @@ -1279,7 +1280,7 @@ trait UsersBase 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders())); $this->assertEquals(200, $response['headers']['status-code']); - $this->assertEquals(1, \count($response['body']['targets'])); + $this->assertEquals(2, \count($response['body']['targets'])); } /** @@ -1313,7 +1314,7 @@ trait UsersBase ], $this->getHeaders())); $this->assertEquals(200, $response['headers']['status-code']); - $this->assertEquals(0, $response['body']['total']); + $this->assertEquals(1, $response['body']['total']); } /** From 43b8fc2c31ebe4645ec954a8d574637708988e87 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Wed, 29 Nov 2023 13:47:44 +0530 Subject: [PATCH 2/2] review changes --- app/controllers/api/users.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index 0b099cb8d..1cbb3e963 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -1161,7 +1161,7 @@ App::patch('/v1/users/:userId/email') Query::equal('identifier', [$email]), ]); - if ($target && !$target->isEmpty()) { + if ($target instanceof Document && !$target->isEmpty()) { throw new Exception(Exception::USER_TARGET_ALREADY_EXISTS); } @@ -1232,7 +1232,7 @@ App::patch('/v1/users/:userId/phone') Query::equal('identifier', [$number]), ]); - if ($target && !$target->isEmpty()) { + if ($target instanceof Document && !$target->isEmpty()) { throw new Exception(Exception::USER_TARGET_ALREADY_EXISTS); }