From 0fc0255a38d8dd20011a80b8e77c2a1b8bd7520f Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Thu, 13 Jul 2023 22:28:08 +0530 Subject: [PATCH 01/30] Skipping checking permission of relations that are not being updated --- app/controllers/api/databases.php | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index f3726c39e7..acfd1d9f8f 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3293,8 +3293,9 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $data['$id'] = $document->getId(); // Make sure user doesn't switch document unique ID $data['$permissions'] = $permissions; $newDocument = new Document($data); + $oldDocumentToBeUpdated = $document; - $checkPermissions = function (Document $collection, Document $document, Document $old, string $permission) use (&$checkPermissions, $dbForProject, $database) { + $checkPermissions = function (Document $collection, Document $document, Document $old, string $permission) use (&$checkPermissions, $dbForProject, $database, $newDocument, $oldDocumentToBeUpdated) { $documentSecurity = $collection->getAttribute('documentSecurity', false); $validator = new Authorization($permission); @@ -3310,9 +3311,31 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } } - $relationships = \array_filter( + $relationships = array_filter( $collection->getAttribute('attributes', []), - fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP + function (Document $attribute) use ($oldDocumentToBeUpdated, $newDocument) { + if ($attribute->getAttribute('type') === Database::VAR_RELATIONSHIP) { + $relationKey = $attribute->getAttribute('key'); + + $oldRelationDocument = $oldDocumentToBeUpdated[$relationKey] ?? []; + $newRelationDocuemntFromRequestData = $newDocument[$relationKey] ?? []; + + if (count($oldRelationDocument) !== count($newRelationDocuemntFromRequestData)) { + // Return true if a difference is found in the relationships + return true; + } + + foreach ($oldRelationDocument as $key => $obj1) { + $obj2 = $newRelationDocuemntFromRequestData[$key] ?? null; + if (($obj1 instanceof Document && $obj2 instanceof Document && $obj1->getArrayCopy() !== $obj2->getArrayCopy()) || !($obj2 instanceof Document)) { + // Return true if a difference is found in the relationships + return true; + } + } + } + + return false; + } ); foreach ($relationships as $relationship) { From b8b3d13f221759e197ac998f1eff57f5f55125f4 Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Thu, 13 Jul 2023 23:15:10 +0530 Subject: [PATCH 02/30] added test case for verifying permission issue with relations --- .../Databases/DatabasesCustomClientTest.php | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index b0ab884a07..c754bafe58 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -6,6 +6,7 @@ use Tests\E2E\Client; use Tests\E2E\Scopes\Scope; use Tests\E2E\Scopes\ProjectCustom; use Tests\E2E\Scopes\SideClient; +use Tests\E2E\Services\Storage\StoragePermissionsScope; use Utopia\Database\Helpers\ID; use Utopia\Database\Helpers\Permission; use Utopia\Database\Helpers\Role; @@ -316,4 +317,147 @@ class DatabasesCustomClientTest extends Scope $this->assertEquals($relation['body']['relatedCollection'], $collection1RelationAttribute['relatedCollection']); $this->assertEquals('restrict', $collection1RelationAttribute['onDelete']); } + + public function testUpdateWithoutRelationPermission(): void + { + + $response = $this->client->call(Client::METHOD_GET, '/account', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ], $this->getHeaders())); + + $userId = $response['body']['$id']; + $database = $this->client->call(Client::METHOD_POST, '/databases', [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ], [ + 'databaseId' => ID::unique(), + 'name' => ID::unique(), + ]); + + $databaseId = $database['body']['$id']; + + + // Creating collection 1 + $collection1 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::unique(), + 'name' => ID::unique(), + 'documentSecurity' => false, + 'permissions' => [ + Permission::create(Role::user($userId)), + Permission::read(Role::user($userId)), + Permission::update(Role::user($userId)), + Permission::delete(Role::user($userId)), + ] + ]); + + // Creating collection 2 + $collection2 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::unique(), + 'name' => ID::unique(), + 'documentSecurity' => false, + 'permissions' => [ + Permission::read(Role::user($userId)), + ] + ]); + + \sleep(2); + + // Creating one to many relationship from collection 1 to colletion 2 + $relation = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/attributes/relationship', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'relatedCollectionId' => $collection2['body']['$id'], + 'type' => 'oneToMany', + 'twoWay' => false, + 'onDelete' => 'setNull', + 'key' => $collection2['body']['$id'] + ]); + + \sleep(2); + + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/attributes/string', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'key' => "Title", + 'size' => 100, + 'required' => false, + 'array' => false, + 'default' => null, + ]); + + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection2['body']['$id'] . '/attributes/string', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'key' => "Rating", + 'size' => 100, + 'required' => false, + 'array' => false, + 'default' => null, + ]); + + \sleep(3); + + + $childDocument = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection2['body']['$id'] . '/documents', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'documentId' => ID::unique(), + 'data' => [ + 'Rating' => '10', + ], + 'permissions' => [], + ]); + + \sleep(3); + + // Creating parent document with a child reference to test the permissions + $parentDocument = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'documentId' => ID::unique(), + 'data' => [ + 'Title' => 'Captain America', + $collection2['body']['$id'] => [$childDocument['body']['$id']] + ], + 'permissions' => [], + ]); + $this->assertEquals(201, $parentDocument['headers']['status-code']); + + \sleep(3); + + // Update document + // This is the point of this test. We should be allowed to do this action, and it should not fail on permission check + $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' .$collection1['body']['$id']. '/documents/' .$parentDocument['body']['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'data' => [ + 'Title' => 'Updated Title', + ] + ]); + + $this->assertEquals(200, $response['headers']['status-code']); + + } } From 21bc2ae56025210f87036aeea7ba72bb75be75ed Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Thu, 13 Jul 2023 23:17:31 +0530 Subject: [PATCH 03/30] lint issues --- tests/e2e/Services/Databases/DatabasesCustomClientTest.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index c754bafe58..89529998a5 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -404,13 +404,13 @@ class DatabasesCustomClientTest extends Scope 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ + ]), [ 'key' => "Rating", 'size' => 100, 'required' => false, 'array' => false, 'default' => null, - ]); + ]); \sleep(3); @@ -448,7 +448,7 @@ class DatabasesCustomClientTest extends Scope // Update document // This is the point of this test. We should be allowed to do this action, and it should not fail on permission check - $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' .$collection1['body']['$id']. '/documents/' .$parentDocument['body']['$id'], array_merge([ + $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents/' . $parentDocument['body']['$id'], array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ @@ -458,6 +458,5 @@ class DatabasesCustomClientTest extends Scope ]); $this->assertEquals(200, $response['headers']['status-code']); - } } From 54ea98afb5b95a18f94c80f40a50a1e5ed78dc8c Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Tue, 18 Jul 2023 16:46:33 +0530 Subject: [PATCH 04/30] updated logic to skip checking permission if no change in new document --- app/controllers/api/databases.php | 116 +++++++++++++++++------------- 1 file changed, 66 insertions(+), 50 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index acfd1d9f8f..1458e4db5e 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -74,7 +74,7 @@ function createAttribute(string $databaseId, string $collectionId, Document $att $default = $attribute->getAttribute('default'); $options = $attribute->getAttribute('options', []); - $db = Authorization::skip(fn () => $dbForProject->getDocument('databases', $databaseId)); + $db = Authorization::skip(fn() => $dbForProject->getDocument('databases', $databaseId)); if ($db->isEmpty()) { throw new Exception(Exception::DATABASE_NOT_FOUND); @@ -194,16 +194,14 @@ function createAttribute(string $databaseId, string $collectionId, Document $att ->setType(DATABASE_TYPE_CREATE_ATTRIBUTE) ->setDatabase($db) ->setCollection($collection) - ->setDocument($attribute) - ; + ->setDocument($attribute); $events ->setContext('collection', $collection) ->setContext('database', $db) ->setParam('databaseId', $databaseId) ->setParam('collectionId', $collection->getId()) - ->setParam('attributeId', $attribute->getId()) - ; + ->setParam('attributeId', $attribute->getId()); $response->setStatusCode(Response::STATUS_CODE_CREATED); @@ -225,7 +223,7 @@ function updateAttribute( array $elements = null, array $options = [] ): Document { - $db = Authorization::skip(fn () => $dbForProject->getDocument('databases', $databaseId)); + $db = Authorization::skip(fn() => $dbForProject->getDocument('databases', $databaseId)); if ($db->isEmpty()) { throw new Exception(Exception::DATABASE_NOT_FOUND); @@ -685,13 +683,11 @@ App::delete('/v1/databases/:databaseId') $deletes ->setType(DELETE_TYPE_DOCUMENT) - ->setDocument($database) - ; + ->setDocument($database); $events ->setParam('databaseId', $database->getId()) - ->setPayload($response->output($database, Response::MODEL_DATABASE)) - ; + ->setPayload($response->output($database, Response::MODEL_DATABASE)); $response->noContent(); }); @@ -2365,7 +2361,7 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/indexes') } // Convert Document[] to array of attribute metadata - $oldAttributes = \array_map(fn($a) => $a->getArrayCopy(), $collection->getAttribute('attributes')); + $oldAttributes = \array_map(fn ($a) => $a->getArrayCopy(), $collection->getAttribute('attributes')); $oldAttributes[] = [ 'key' => '$id', @@ -2546,7 +2542,7 @@ App::get('/v1/databases/:databaseId/collections/:collectionId/indexes/:key') $indexes = $collection->getAttribute('indexes'); // Search for index - $indexIndex = array_search($key, array_map(fn($idx) => $idx['key'], $indexes)); + $indexIndex = array_search($key, array_map(fn ($idx) => $idx['key'], $indexes)); if ($indexIndex === false) { throw new Exception(Exception::INDEX_NOT_FOUND); @@ -2745,7 +2741,7 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') $relationships = \array_filter( $collection->getAttribute('attributes', []), - fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP + fn ($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP ); foreach ($relationships as $relationship) { @@ -2824,7 +2820,7 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') $relationships = \array_filter( $collection->getAttribute('attributes', []), - fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP + fn ($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP ); foreach ($relationships as $relationship) { @@ -2949,7 +2945,7 @@ App::get('/v1/databases/:databaseId/collections/:collectionId/documents') $relationships = \array_filter( $collection->getAttribute('attributes', []), - fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP + fn ($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP ); foreach ($relationships as $relationship) { @@ -3061,7 +3057,7 @@ App::get('/v1/databases/:databaseId/collections/:collectionId/documents/:documen $relationships = \array_filter( $collection->getAttribute('attributes', []), - fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP + fn ($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP ); foreach ($relationships as $relationship) { @@ -3288,14 +3284,12 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } $data = \array_merge($document->getArrayCopy(), $data); // Merge existing data with new data - $data['$collection'] = $collection->getId(); // Make sure user doesn't switch collectionID $data['$createdAt'] = $document->getCreatedAt(); // Make sure user doesn't switch createdAt $data['$id'] = $document->getId(); // Make sure user doesn't switch document unique ID $data['$permissions'] = $permissions; $newDocument = new Document($data); - $oldDocumentToBeUpdated = $document; - $checkPermissions = function (Document $collection, Document $document, Document $old, string $permission) use (&$checkPermissions, $dbForProject, $database, $newDocument, $oldDocumentToBeUpdated) { + $checkPermissions = function (Document $collection, Document $document, Document $old, string $permission) use (&$checkPermissions, $dbForProject, $database) { $documentSecurity = $collection->getAttribute('documentSecurity', false); $validator = new Authorization($permission); @@ -3311,30 +3305,10 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } } - $relationships = array_filter( + $relationships = \array_filter( $collection->getAttribute('attributes', []), - function (Document $attribute) use ($oldDocumentToBeUpdated, $newDocument) { - if ($attribute->getAttribute('type') === Database::VAR_RELATIONSHIP) { - $relationKey = $attribute->getAttribute('key'); - - $oldRelationDocument = $oldDocumentToBeUpdated[$relationKey] ?? []; - $newRelationDocuemntFromRequestData = $newDocument[$relationKey] ?? []; - - if (count($oldRelationDocument) !== count($newRelationDocuemntFromRequestData)) { - // Return true if a difference is found in the relationships - return true; - } - - foreach ($oldRelationDocument as $key => $obj1) { - $obj2 = $newRelationDocuemntFromRequestData[$key] ?? null; - if (($obj1 instanceof Document && $obj2 instanceof Document && $obj1->getArrayCopy() !== $obj2->getArrayCopy()) || !($obj2 instanceof Document)) { - // Return true if a difference is found in the relationships - return true; - } - } - } - - return false; + function (Document $attribute) { + return $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP; } ); @@ -3359,6 +3333,8 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum ); foreach ($relations as &$relation) { + $skipCheckingPermission = false; + // If the relation is an array it can be either update or create a child document. if ( \is_array($relation) && \array_values($relation) !== $relation @@ -3368,25 +3344,44 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $relation = new Document($relation); } if ($relation instanceof Document) { - $oldDocument = Authorization::skip(fn() => $dbForProject->getDocument( + $relatedDocumentOldVersion = Authorization::skip(fn() => $dbForProject->getDocument( 'database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId(), $relation->getId() )); - if ($oldDocument->isEmpty()) { + // If the child document has to be created it will need checking permissions. + if ($relatedDocumentOldVersion->isEmpty()) { $type = Database::PERMISSION_CREATE; if (isset($relation['$id']) && $relation['$id'] === 'unique()') { $relation['$id'] = ID::unique(); } } else { - $relation->removeAttribute('$collectionId'); - $relation->removeAttribute('$databaseId'); - $relation->setAttribute('$collection', $relatedCollection->getId()); $type = Database::PERMISSION_UPDATE; - } + $skipCheckingPermission = true; - $checkPermissions($relatedCollection, $relation, $oldDocument, $type); + foreach ($relation as $key => $value) { + //No need to compare values of relations as for each relation we are recursively checking permission. + if ($relatedDocumentOldVersion->getAttribute($key) instanceof Document) { + continue; + } + //If any of the values are different, we need to check permission. + if ($relatedDocumentOldVersion->getAttribute($key) !== $value) { + $skipCheckingPermission = false; + $relation->removeAttribute('$collectionId'); + $relation->removeAttribute('$databaseId'); + $relation->setAttribute('$collection', $relatedCollection->getId()); + break; + } + } + } + if ($skipCheckingPermission) { + Authorization::skip( + fn() => $checkPermissions($relatedCollection, $relation, $relatedDocumentOldVersion, $type) + ); + } else { + $checkPermissions($relatedCollection, $relation, $relatedDocumentOldVersion, $type); + } } } @@ -3397,8 +3392,29 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } } }; - + $skipCheckingPermission = true; + foreach ($newDocument as $key => $value) { + if ($document->getAttribute($key) instanceof Document) { + continue; + } + //If any of the values are different, we need to check permission. + if ($newDocument->getAttribute($key) !== $value) { + $skipCheckingPermission = false; + $newDocument->removeAttribute('$collectionId'); + $newDocument->removeAttribute('$databaseId'); + $newDocument->setAttribute('$collection', $collection->getId()); + break; + } + } + if ($skipCheckingPermission) { + Authorization::skip( + fn() => $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE) + ); + } else { $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE); + ; + } + try { $document = $dbForProject->withRequestTimestamp( From 5c5a8a91153c483a6c09c259d2e2db75246257b4 Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Tue, 18 Jul 2023 16:52:00 +0530 Subject: [PATCH 05/30] format issues --- app/controllers/api/databases.php | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 1458e4db5e..700d082aad 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -74,7 +74,7 @@ function createAttribute(string $databaseId, string $collectionId, Document $att $default = $attribute->getAttribute('default'); $options = $attribute->getAttribute('options', []); - $db = Authorization::skip(fn() => $dbForProject->getDocument('databases', $databaseId)); + $db = Authorization::skip(fn () => $dbForProject->getDocument('databases', $databaseId)); if ($db->isEmpty()) { throw new Exception(Exception::DATABASE_NOT_FOUND); @@ -223,7 +223,7 @@ function updateAttribute( array $elements = null, array $options = [] ): Document { - $db = Authorization::skip(fn() => $dbForProject->getDocument('databases', $databaseId)); + $db = Authorization::skip(fn () => $dbForProject->getDocument('databases', $databaseId)); if ($db->isEmpty()) { throw new Exception(Exception::DATABASE_NOT_FOUND); @@ -2361,7 +2361,7 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/indexes') } // Convert Document[] to array of attribute metadata - $oldAttributes = \array_map(fn ($a) => $a->getArrayCopy(), $collection->getAttribute('attributes')); + $oldAttributes = \array_map(fn($a) => $a->getArrayCopy(), $collection->getAttribute('attributes')); $oldAttributes[] = [ 'key' => '$id', @@ -2542,7 +2542,7 @@ App::get('/v1/databases/:databaseId/collections/:collectionId/indexes/:key') $indexes = $collection->getAttribute('indexes'); // Search for index - $indexIndex = array_search($key, array_map(fn ($idx) => $idx['key'], $indexes)); + $indexIndex = array_search($key, array_map(fn($idx) => $idx['key'], $indexes)); if ($indexIndex === false) { throw new Exception(Exception::INDEX_NOT_FOUND); @@ -2741,7 +2741,7 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') $relationships = \array_filter( $collection->getAttribute('attributes', []), - fn ($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP + fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP ); foreach ($relationships as $relationship) { @@ -2820,7 +2820,7 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') $relationships = \array_filter( $collection->getAttribute('attributes', []), - fn ($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP + fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP ); foreach ($relationships as $relationship) { @@ -2945,7 +2945,7 @@ App::get('/v1/databases/:databaseId/collections/:collectionId/documents') $relationships = \array_filter( $collection->getAttribute('attributes', []), - fn ($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP + fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP ); foreach ($relationships as $relationship) { @@ -3057,7 +3057,7 @@ App::get('/v1/databases/:databaseId/collections/:collectionId/documents/:documen $relationships = \array_filter( $collection->getAttribute('attributes', []), - fn ($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP + fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP ); foreach ($relationships as $relationship) { @@ -3307,9 +3307,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $relationships = \array_filter( $collection->getAttribute('attributes', []), - function (Document $attribute) { - return $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP; - } + fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP ); foreach ($relationships as $relationship) { From e631f393fce16385b3aea509b5bdad826e53081a Mon Sep 17 00:00:00 2001 From: prateek banga Date: Thu, 27 Jul 2023 02:23:20 +0530 Subject: [PATCH 06/30] fix lint issues and remove sleep from test --- app/controllers/api/databases.php | 44 +++++++++---------- composer.json | 2 +- .../Databases/DatabasesCustomClientTest.php | 13 +----- 3 files changed, 24 insertions(+), 35 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 700d082aad..5cf95ab161 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3390,29 +3390,29 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } } }; - $skipCheckingPermission = true; - foreach ($newDocument as $key => $value) { - if ($document->getAttribute($key) instanceof Document) { - continue; - } - //If any of the values are different, we need to check permission. - if ($newDocument->getAttribute($key) !== $value) { - $skipCheckingPermission = false; - $newDocument->removeAttribute('$collectionId'); - $newDocument->removeAttribute('$databaseId'); - $newDocument->setAttribute('$collection', $collection->getId()); - break; - } - } - if ($skipCheckingPermission) { - Authorization::skip( - fn() => $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE) - ); - } else { - $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE); - ; - } + $skipCheckingPermission = true; + foreach ($newDocument as $key => $value) { + if ($document->getAttribute($key) instanceof Document) { + continue; + } + //If any of the values are different, we need to check permission. + if ($newDocument->getAttribute($key) !== $value) { + $skipCheckingPermission = false; + $newDocument->removeAttribute('$collectionId'); + $newDocument->removeAttribute('$databaseId'); + $newDocument->setAttribute('$collection', $collection->getId()); + break; + } + } + + if ($skipCheckingPermission) { + Authorization::skip( + fn() => $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE) + ); + } else { + $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE); + } try { $document = $dbForProject->withRequestTimestamp( diff --git a/composer.json b/composer.json index 8070096898..afd1f2aca9 100644 --- a/composer.json +++ b/composer.json @@ -49,7 +49,7 @@ "utopia-php/cache": "0.8.*", "utopia-php/cli": "0.13.*", "utopia-php/config": "0.2.*", - "utopia-php/database": "0.38.*", + "utopia-php/database": "0.39.*", "utopia-php/domains": "1.1.*", "utopia-php/framework": "0.28.*", "utopia-php/image": "0.5.*", diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index 89529998a5..58b5b133d3 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -320,7 +320,6 @@ class DatabasesCustomClientTest extends Scope public function testUpdateWithoutRelationPermission(): void { - $response = $this->client->call(Client::METHOD_GET, '/account', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], @@ -339,7 +338,6 @@ class DatabasesCustomClientTest extends Scope $databaseId = $database['body']['$id']; - // Creating collection 1 $collection1 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections', array_merge([ 'content-type' => 'application/json', @@ -371,8 +369,6 @@ class DatabasesCustomClientTest extends Scope ] ]); - \sleep(2); - // Creating one to many relationship from collection 1 to colletion 2 $relation = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/attributes/relationship', array_merge([ 'content-type' => 'application/json', @@ -386,8 +382,6 @@ class DatabasesCustomClientTest extends Scope 'key' => $collection2['body']['$id'] ]); - \sleep(2); - $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/attributes/string', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], @@ -412,8 +406,7 @@ class DatabasesCustomClientTest extends Scope 'default' => null, ]); - \sleep(3); - + \sleep(2); $childDocument = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection2['body']['$id'] . '/documents', array_merge([ 'content-type' => 'application/json', @@ -427,8 +420,6 @@ class DatabasesCustomClientTest extends Scope 'permissions' => [], ]); - \sleep(3); - // Creating parent document with a child reference to test the permissions $parentDocument = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents', array_merge([ 'content-type' => 'application/json', @@ -444,8 +435,6 @@ class DatabasesCustomClientTest extends Scope ]); $this->assertEquals(201, $parentDocument['headers']['status-code']); - \sleep(3); - // Update document // This is the point of this test. We should be allowed to do this action, and it should not fail on permission check $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents/' . $parentDocument['body']['$id'], array_merge([ From 6247e524b602ad0c22e13d5f1f3aa7665382724a Mon Sep 17 00:00:00 2001 From: prateek banga Date: Thu, 27 Jul 2023 02:27:31 +0530 Subject: [PATCH 07/30] fix lint issues --- app/controllers/api/databases.php | 38 +++++++++++++++---------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 5cf95ab161..44b6a6f205 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3392,27 +3392,27 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum }; $skipCheckingPermission = true; - foreach ($newDocument as $key => $value) { - if ($document->getAttribute($key) instanceof Document) { - continue; - } - //If any of the values are different, we need to check permission. - if ($newDocument->getAttribute($key) !== $value) { - $skipCheckingPermission = false; - $newDocument->removeAttribute('$collectionId'); - $newDocument->removeAttribute('$databaseId'); - $newDocument->setAttribute('$collection', $collection->getId()); - break; - } + foreach ($newDocument as $key => $value) { + if ($document->getAttribute($key) instanceof Document) { + continue; } + //If any of the values are different, we need to check permission. + if ($newDocument->getAttribute($key) !== $value) { + $skipCheckingPermission = false; + $newDocument->removeAttribute('$collectionId'); + $newDocument->removeAttribute('$databaseId'); + $newDocument->setAttribute('$collection', $collection->getId()); + break; + } + } - if ($skipCheckingPermission) { - Authorization::skip( - fn() => $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE) - ); - } else { - $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE); - } + if ($skipCheckingPermission) { + Authorization::skip( + fn() => $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE) + ); + } else { + $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE); + } try { $document = $dbForProject->withRequestTimestamp( From a639c2737a8508a0fd09a989568456d4379cef2b Mon Sep 17 00:00:00 2001 From: prateek banga Date: Thu, 27 Jul 2023 14:13:15 +0530 Subject: [PATCH 08/30] updates abuse and audit deps --- composer.json | 4 +-- composer.lock | 79 ++++++++++++++++++++++++++------------------------- 2 files changed, 42 insertions(+), 41 deletions(-) diff --git a/composer.json b/composer.json index afd1f2aca9..5794d208dd 100644 --- a/composer.json +++ b/composer.json @@ -43,9 +43,9 @@ "ext-sockets": "*", "appwrite/php-clamav": "2.0.*", "appwrite/php-runtimes": "0.11.*", - "utopia-php/abuse": "0.27.*", + "utopia-php/abuse": "0.28.*", "utopia-php/analytics": "0.2.*", - "utopia-php/audit": "0.29.*", + "utopia-php/audit": "0.30.*", "utopia-php/cache": "0.8.*", "utopia-php/cli": "0.13.*", "utopia-php/config": "0.2.*", diff --git a/composer.lock b/composer.lock index 5a25c90dc4..e292fc1a0c 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "c41be9b81d4ee69cf915be4c7ba37975", + "content-hash": "b8aa5b23fbd8e5e1bda485f5ef5798d8", "packages": [ { "name": "adhocore/jwt", @@ -1804,23 +1804,23 @@ }, { "name": "utopia-php/abuse", - "version": "0.27.0", + "version": "0.28.0", "source": { "type": "git", "url": "https://github.com/utopia-php/abuse.git", - "reference": "d1115f5843e903ffaba9c23e450b33c0fe265ae0" + "reference": "256584e2baa1fb8a7747619b1ef19f7917b2fa07" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/abuse/zipball/d1115f5843e903ffaba9c23e450b33c0fe265ae0", - "reference": "d1115f5843e903ffaba9c23e450b33c0fe265ae0", + "url": "https://api.github.com/repos/utopia-php/abuse/zipball/256584e2baa1fb8a7747619b1ef19f7917b2fa07", + "reference": "256584e2baa1fb8a7747619b1ef19f7917b2fa07", "shasum": "" }, "require": { "ext-curl": "*", "ext-pdo": "*", "php": ">=8.0", - "utopia-php/database": "0.38.*" + "utopia-php/database": "0.39.*" }, "require-dev": { "laravel/pint": "1.5.*", @@ -1847,9 +1847,9 @@ ], "support": { "issues": "https://github.com/utopia-php/abuse/issues", - "source": "https://github.com/utopia-php/abuse/tree/0.27.0" + "source": "https://github.com/utopia-php/abuse/tree/0.28.0" }, - "time": "2023-07-15T00:53:50+00:00" + "time": "2023-07-27T08:06:39+00:00" }, { "name": "utopia-php/analytics", @@ -1908,21 +1908,21 @@ }, { "name": "utopia-php/audit", - "version": "0.29.0", + "version": "0.30.0", "source": { "type": "git", "url": "https://github.com/utopia-php/audit.git", - "reference": "5318538f457bf73623629345c98ea06371ca5dd4" + "reference": "e171836a2b03129f28db9beb616f8975183f445a" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/audit/zipball/5318538f457bf73623629345c98ea06371ca5dd4", - "reference": "5318538f457bf73623629345c98ea06371ca5dd4", + "url": "https://api.github.com/repos/utopia-php/audit/zipball/e171836a2b03129f28db9beb616f8975183f445a", + "reference": "e171836a2b03129f28db9beb616f8975183f445a", "shasum": "" }, "require": { "php": ">=8.0", - "utopia-php/database": "0.38.*" + "utopia-php/database": "0.39.*" }, "require-dev": { "laravel/pint": "1.5.*", @@ -1949,9 +1949,9 @@ ], "support": { "issues": "https://github.com/utopia-php/audit/issues", - "source": "https://github.com/utopia-php/audit/tree/0.29.0" + "source": "https://github.com/utopia-php/audit/tree/0.30.0" }, - "time": "2023-07-15T00:51:10+00:00" + "time": "2023-07-27T08:04:56+00:00" }, { "name": "utopia-php/cache", @@ -2108,16 +2108,16 @@ }, { "name": "utopia-php/database", - "version": "0.38.0", + "version": "0.39.0", "source": { "type": "git", "url": "https://github.com/utopia-php/database.git", - "reference": "59e4684cf87e03c12dab9240158c1dfc6888e534" + "reference": "a9f706034a12c7c87e6fd38eca69587fd3def0e0" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/database/zipball/59e4684cf87e03c12dab9240158c1dfc6888e534", - "reference": "59e4684cf87e03c12dab9240158c1dfc6888e534", + "url": "https://api.github.com/repos/utopia-php/database/zipball/a9f706034a12c7c87e6fd38eca69587fd3def0e0", + "reference": "a9f706034a12c7c87e6fd38eca69587fd3def0e0", "shasum": "" }, "require": { @@ -2158,9 +2158,9 @@ ], "support": { "issues": "https://github.com/utopia-php/database/issues", - "source": "https://github.com/utopia-php/database/tree/0.38.0" + "source": "https://github.com/utopia-php/database/tree/0.39.0" }, - "time": "2023-07-14T07:49:38+00:00" + "time": "2023-07-26T18:03:25+00:00" }, { "name": "utopia-php/domains", @@ -3785,16 +3785,16 @@ }, { "name": "phpstan/phpdoc-parser", - "version": "1.22.1", + "version": "1.23.0", "source": { "type": "git", "url": "https://github.com/phpstan/phpdoc-parser.git", - "reference": "65c39594fbd8c67abfc68bb323f86447bab79cc0" + "reference": "a2b24135c35852b348894320d47b3902a94bc494" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpdoc-parser/zipball/65c39594fbd8c67abfc68bb323f86447bab79cc0", - "reference": "65c39594fbd8c67abfc68bb323f86447bab79cc0", + "url": "https://api.github.com/repos/phpstan/phpdoc-parser/zipball/a2b24135c35852b348894320d47b3902a94bc494", + "reference": "a2b24135c35852b348894320d47b3902a94bc494", "shasum": "" }, "require": { @@ -3826,22 +3826,22 @@ "description": "PHPDoc parser with support for nullable, intersection and generic types", "support": { "issues": "https://github.com/phpstan/phpdoc-parser/issues", - "source": "https://github.com/phpstan/phpdoc-parser/tree/1.22.1" + "source": "https://github.com/phpstan/phpdoc-parser/tree/1.23.0" }, - "time": "2023-06-29T20:46:06+00:00" + "time": "2023-07-23T22:17:56+00:00" }, { "name": "phpunit/php-code-coverage", - "version": "9.2.26", + "version": "9.2.27", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/php-code-coverage.git", - "reference": "443bc6912c9bd5b409254a40f4b0f4ced7c80ea1" + "reference": "b0a88255cb70d52653d80c890bd7f38740ea50d1" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/php-code-coverage/zipball/443bc6912c9bd5b409254a40f4b0f4ced7c80ea1", - "reference": "443bc6912c9bd5b409254a40f4b0f4ced7c80ea1", + "url": "https://api.github.com/repos/sebastianbergmann/php-code-coverage/zipball/b0a88255cb70d52653d80c890bd7f38740ea50d1", + "reference": "b0a88255cb70d52653d80c890bd7f38740ea50d1", "shasum": "" }, "require": { @@ -3897,7 +3897,8 @@ ], "support": { "issues": "https://github.com/sebastianbergmann/php-code-coverage/issues", - "source": "https://github.com/sebastianbergmann/php-code-coverage/tree/9.2.26" + "security": "https://github.com/sebastianbergmann/php-code-coverage/security/policy", + "source": "https://github.com/sebastianbergmann/php-code-coverage/tree/9.2.27" }, "funding": [ { @@ -3905,7 +3906,7 @@ "type": "github" } ], - "time": "2023-03-06T12:58:08+00:00" + "time": "2023-07-26T13:44:30+00:00" }, { "name": "phpunit/php-file-iterator", @@ -5580,16 +5581,16 @@ }, { "name": "twig/twig", - "version": "v3.6.1", + "version": "v3.7.0", "source": { "type": "git", "url": "https://github.com/twigphp/Twig.git", - "reference": "7e7d5839d4bec168dfeef0ac66d5c5a2edbabffd" + "reference": "5cf942bbab3df42afa918caeba947f1b690af64b" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/twigphp/Twig/zipball/7e7d5839d4bec168dfeef0ac66d5c5a2edbabffd", - "reference": "7e7d5839d4bec168dfeef0ac66d5c5a2edbabffd", + "url": "https://api.github.com/repos/twigphp/Twig/zipball/5cf942bbab3df42afa918caeba947f1b690af64b", + "reference": "5cf942bbab3df42afa918caeba947f1b690af64b", "shasum": "" }, "require": { @@ -5635,7 +5636,7 @@ ], "support": { "issues": "https://github.com/twigphp/Twig/issues", - "source": "https://github.com/twigphp/Twig/tree/v3.6.1" + "source": "https://github.com/twigphp/Twig/tree/v3.7.0" }, "funding": [ { @@ -5647,7 +5648,7 @@ "type": "tidelift" } ], - "time": "2023-06-08T12:52:13+00:00" + "time": "2023-07-26T07:16:09+00:00" } ], "aliases": [], From 0d8eeb5807e703d9bd52a98e325bd0a19b847583 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Thu, 27 Jul 2023 15:48:19 +0530 Subject: [PATCH 09/30] adds name for database events in on function --- app/controllers/shared/api.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index c89bf0252e..47c177f1be 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -207,9 +207,9 @@ App::init() $deletes->setProject($project); $database->setProject($project); - $dbForProject->on(Database::EVENT_DOCUMENT_CREATE, fn ($event, Document $document) => $databaseListener($event, $document, $usage)); + $dbForProject->on(Database::EVENT_DOCUMENT_CREATE, 'document-create', fn ($event, Document $document) => $databaseListener($event, $document, $usage)); - $dbForProject->on(Database::EVENT_DOCUMENT_DELETE, fn ($event, Document $document) => $databaseListener($event, $document, $usage)); + $dbForProject->on(Database::EVENT_DOCUMENT_DELETE, 'document-delete', fn ($event, Document $document) => $databaseListener($event, $document, $usage)); $useCache = $route->getLabel('cache', false); From d743eb23365f41857775cceab2671b23e3c054a1 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Thu, 27 Jul 2023 16:13:10 +0530 Subject: [PATCH 10/30] updates deps --- composer.json | 6 ++-- composer.lock | 79 ++++++++++++++++++++++++++------------------------- 2 files changed, 43 insertions(+), 42 deletions(-) diff --git a/composer.json b/composer.json index 8070096898..5794d208dd 100644 --- a/composer.json +++ b/composer.json @@ -43,13 +43,13 @@ "ext-sockets": "*", "appwrite/php-clamav": "2.0.*", "appwrite/php-runtimes": "0.11.*", - "utopia-php/abuse": "0.27.*", + "utopia-php/abuse": "0.28.*", "utopia-php/analytics": "0.2.*", - "utopia-php/audit": "0.29.*", + "utopia-php/audit": "0.30.*", "utopia-php/cache": "0.8.*", "utopia-php/cli": "0.13.*", "utopia-php/config": "0.2.*", - "utopia-php/database": "0.38.*", + "utopia-php/database": "0.39.*", "utopia-php/domains": "1.1.*", "utopia-php/framework": "0.28.*", "utopia-php/image": "0.5.*", diff --git a/composer.lock b/composer.lock index 5a25c90dc4..e292fc1a0c 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "c41be9b81d4ee69cf915be4c7ba37975", + "content-hash": "b8aa5b23fbd8e5e1bda485f5ef5798d8", "packages": [ { "name": "adhocore/jwt", @@ -1804,23 +1804,23 @@ }, { "name": "utopia-php/abuse", - "version": "0.27.0", + "version": "0.28.0", "source": { "type": "git", "url": "https://github.com/utopia-php/abuse.git", - "reference": "d1115f5843e903ffaba9c23e450b33c0fe265ae0" + "reference": "256584e2baa1fb8a7747619b1ef19f7917b2fa07" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/abuse/zipball/d1115f5843e903ffaba9c23e450b33c0fe265ae0", - "reference": "d1115f5843e903ffaba9c23e450b33c0fe265ae0", + "url": "https://api.github.com/repos/utopia-php/abuse/zipball/256584e2baa1fb8a7747619b1ef19f7917b2fa07", + "reference": "256584e2baa1fb8a7747619b1ef19f7917b2fa07", "shasum": "" }, "require": { "ext-curl": "*", "ext-pdo": "*", "php": ">=8.0", - "utopia-php/database": "0.38.*" + "utopia-php/database": "0.39.*" }, "require-dev": { "laravel/pint": "1.5.*", @@ -1847,9 +1847,9 @@ ], "support": { "issues": "https://github.com/utopia-php/abuse/issues", - "source": "https://github.com/utopia-php/abuse/tree/0.27.0" + "source": "https://github.com/utopia-php/abuse/tree/0.28.0" }, - "time": "2023-07-15T00:53:50+00:00" + "time": "2023-07-27T08:06:39+00:00" }, { "name": "utopia-php/analytics", @@ -1908,21 +1908,21 @@ }, { "name": "utopia-php/audit", - "version": "0.29.0", + "version": "0.30.0", "source": { "type": "git", "url": "https://github.com/utopia-php/audit.git", - "reference": "5318538f457bf73623629345c98ea06371ca5dd4" + "reference": "e171836a2b03129f28db9beb616f8975183f445a" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/audit/zipball/5318538f457bf73623629345c98ea06371ca5dd4", - "reference": "5318538f457bf73623629345c98ea06371ca5dd4", + "url": "https://api.github.com/repos/utopia-php/audit/zipball/e171836a2b03129f28db9beb616f8975183f445a", + "reference": "e171836a2b03129f28db9beb616f8975183f445a", "shasum": "" }, "require": { "php": ">=8.0", - "utopia-php/database": "0.38.*" + "utopia-php/database": "0.39.*" }, "require-dev": { "laravel/pint": "1.5.*", @@ -1949,9 +1949,9 @@ ], "support": { "issues": "https://github.com/utopia-php/audit/issues", - "source": "https://github.com/utopia-php/audit/tree/0.29.0" + "source": "https://github.com/utopia-php/audit/tree/0.30.0" }, - "time": "2023-07-15T00:51:10+00:00" + "time": "2023-07-27T08:04:56+00:00" }, { "name": "utopia-php/cache", @@ -2108,16 +2108,16 @@ }, { "name": "utopia-php/database", - "version": "0.38.0", + "version": "0.39.0", "source": { "type": "git", "url": "https://github.com/utopia-php/database.git", - "reference": "59e4684cf87e03c12dab9240158c1dfc6888e534" + "reference": "a9f706034a12c7c87e6fd38eca69587fd3def0e0" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/database/zipball/59e4684cf87e03c12dab9240158c1dfc6888e534", - "reference": "59e4684cf87e03c12dab9240158c1dfc6888e534", + "url": "https://api.github.com/repos/utopia-php/database/zipball/a9f706034a12c7c87e6fd38eca69587fd3def0e0", + "reference": "a9f706034a12c7c87e6fd38eca69587fd3def0e0", "shasum": "" }, "require": { @@ -2158,9 +2158,9 @@ ], "support": { "issues": "https://github.com/utopia-php/database/issues", - "source": "https://github.com/utopia-php/database/tree/0.38.0" + "source": "https://github.com/utopia-php/database/tree/0.39.0" }, - "time": "2023-07-14T07:49:38+00:00" + "time": "2023-07-26T18:03:25+00:00" }, { "name": "utopia-php/domains", @@ -3785,16 +3785,16 @@ }, { "name": "phpstan/phpdoc-parser", - "version": "1.22.1", + "version": "1.23.0", "source": { "type": "git", "url": "https://github.com/phpstan/phpdoc-parser.git", - "reference": "65c39594fbd8c67abfc68bb323f86447bab79cc0" + "reference": "a2b24135c35852b348894320d47b3902a94bc494" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpdoc-parser/zipball/65c39594fbd8c67abfc68bb323f86447bab79cc0", - "reference": "65c39594fbd8c67abfc68bb323f86447bab79cc0", + "url": "https://api.github.com/repos/phpstan/phpdoc-parser/zipball/a2b24135c35852b348894320d47b3902a94bc494", + "reference": "a2b24135c35852b348894320d47b3902a94bc494", "shasum": "" }, "require": { @@ -3826,22 +3826,22 @@ "description": "PHPDoc parser with support for nullable, intersection and generic types", "support": { "issues": "https://github.com/phpstan/phpdoc-parser/issues", - "source": "https://github.com/phpstan/phpdoc-parser/tree/1.22.1" + "source": "https://github.com/phpstan/phpdoc-parser/tree/1.23.0" }, - "time": "2023-06-29T20:46:06+00:00" + "time": "2023-07-23T22:17:56+00:00" }, { "name": "phpunit/php-code-coverage", - "version": "9.2.26", + "version": "9.2.27", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/php-code-coverage.git", - "reference": "443bc6912c9bd5b409254a40f4b0f4ced7c80ea1" + "reference": "b0a88255cb70d52653d80c890bd7f38740ea50d1" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/php-code-coverage/zipball/443bc6912c9bd5b409254a40f4b0f4ced7c80ea1", - "reference": "443bc6912c9bd5b409254a40f4b0f4ced7c80ea1", + "url": "https://api.github.com/repos/sebastianbergmann/php-code-coverage/zipball/b0a88255cb70d52653d80c890bd7f38740ea50d1", + "reference": "b0a88255cb70d52653d80c890bd7f38740ea50d1", "shasum": "" }, "require": { @@ -3897,7 +3897,8 @@ ], "support": { "issues": "https://github.com/sebastianbergmann/php-code-coverage/issues", - "source": "https://github.com/sebastianbergmann/php-code-coverage/tree/9.2.26" + "security": "https://github.com/sebastianbergmann/php-code-coverage/security/policy", + "source": "https://github.com/sebastianbergmann/php-code-coverage/tree/9.2.27" }, "funding": [ { @@ -3905,7 +3906,7 @@ "type": "github" } ], - "time": "2023-03-06T12:58:08+00:00" + "time": "2023-07-26T13:44:30+00:00" }, { "name": "phpunit/php-file-iterator", @@ -5580,16 +5581,16 @@ }, { "name": "twig/twig", - "version": "v3.6.1", + "version": "v3.7.0", "source": { "type": "git", "url": "https://github.com/twigphp/Twig.git", - "reference": "7e7d5839d4bec168dfeef0ac66d5c5a2edbabffd" + "reference": "5cf942bbab3df42afa918caeba947f1b690af64b" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/twigphp/Twig/zipball/7e7d5839d4bec168dfeef0ac66d5c5a2edbabffd", - "reference": "7e7d5839d4bec168dfeef0ac66d5c5a2edbabffd", + "url": "https://api.github.com/repos/twigphp/Twig/zipball/5cf942bbab3df42afa918caeba947f1b690af64b", + "reference": "5cf942bbab3df42afa918caeba947f1b690af64b", "shasum": "" }, "require": { @@ -5635,7 +5636,7 @@ ], "support": { "issues": "https://github.com/twigphp/Twig/issues", - "source": "https://github.com/twigphp/Twig/tree/v3.6.1" + "source": "https://github.com/twigphp/Twig/tree/v3.7.0" }, "funding": [ { @@ -5647,7 +5648,7 @@ "type": "tidelift" } ], - "time": "2023-06-08T12:52:13+00:00" + "time": "2023-07-26T07:16:09+00:00" } ], "aliases": [], From 6b2d246fba3a32afec4c446f30c4e870e493bbb1 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Fri, 28 Jul 2023 17:49:34 +0530 Subject: [PATCH 11/30] better naming for on callback function --- app/controllers/shared/api.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index 47c177f1be..566a800752 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -207,9 +207,11 @@ App::init() $deletes->setProject($project); $database->setProject($project); - $dbForProject->on(Database::EVENT_DOCUMENT_CREATE, 'document-create', fn ($event, Document $document) => $databaseListener($event, $document, $usage)); + $calculateUsage = fn ($event, Document $document) => $databaseListener($event, $document, $usage); - $dbForProject->on(Database::EVENT_DOCUMENT_DELETE, 'document-delete', fn ($event, Document $document) => $databaseListener($event, $document, $usage)); + $dbForProject->on(Database::EVENT_DOCUMENT_CREATE, 'calculate-usage', $calculateUsage); + + $dbForProject->on(Database::EVENT_DOCUMENT_DELETE, 'calculate-usage', $calculateUsage); $useCache = $route->getLabel('cache', false); From f7e96282db9318495e89223850286122134f66f9 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Sat, 29 Jul 2023 12:01:10 +0530 Subject: [PATCH 12/30] adds attribute for update method and makes test case to check more complex scenario --- app/controllers/api/databases.php | 94 ++++++++++--------- .../Databases/DatabasesCustomClientTest.php | 82 ++++++++++++---- 2 files changed, 113 insertions(+), 63 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 44b6a6f205..3c244d612e 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3289,7 +3289,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $data['$permissions'] = $permissions; $newDocument = new Document($data); - $checkPermissions = function (Document $collection, Document $document, Document $old, string $permission) use (&$checkPermissions, $dbForProject, $database) { + $checkPermissions = (function (Document $collection, Document $document, Document $old, string $permission) use (&$checkPermissions, $dbForProject, $database) { $documentSecurity = $collection->getAttribute('documentSecurity', false); $validator = new Authorization($permission); @@ -3331,7 +3331,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum ); foreach ($relations as &$relation) { - $skipCheckingPermission = false; + $shouldUpdate = false; // If the relation is an array it can be either update or create a child document. if ( \is_array($relation) @@ -3347,6 +3347,9 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $relation->getId() )); + // Attribute $collection is required for update document. Copying it from old version of document. + $relation->setAttribute('$collection', $relatedDocumentOldVersion->getAttribute('$collection')); + // If the child document has to be created it will need checking permissions. if ($relatedDocumentOldVersion->isEmpty()) { $type = Database::PERMISSION_CREATE; @@ -3356,16 +3359,15 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } } else { $type = Database::PERMISSION_UPDATE; - $skipCheckingPermission = true; foreach ($relation as $key => $value) { - //No need to compare values of relations as for each relation we are recursively checking permission. + // No need to compare values of relations as for each relation we are recursively checking permission. if ($relatedDocumentOldVersion->getAttribute($key) instanceof Document) { continue; } - //If any of the values are different, we need to check permission. + // If any of the values are different, we need to check permission. if ($relatedDocumentOldVersion->getAttribute($key) !== $value) { - $skipCheckingPermission = false; + $shouldUpdate = true; $relation->removeAttribute('$collectionId'); $relation->removeAttribute('$databaseId'); $relation->setAttribute('$collection', $relatedCollection->getId()); @@ -3373,12 +3375,12 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } } } - if ($skipCheckingPermission) { + if ($shouldUpdate) { + $checkPermissions($relatedCollection, $relation, $relatedDocumentOldVersion, $type); + } else { Authorization::skip( fn() => $checkPermissions($relatedCollection, $relation, $relatedDocumentOldVersion, $type) ); - } else { - $checkPermissions($relatedCollection, $relation, $relatedDocumentOldVersion, $type); } } } @@ -3389,47 +3391,47 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $document->setAttribute($relationship->getAttribute('key'), \reset($relations)); } } - }; + }); - $skipCheckingPermission = true; - foreach ($newDocument as $key => $value) { - if ($document->getAttribute($key) instanceof Document) { - continue; + $shouldUpdate = false; + foreach ($newDocument as $key => $value) { + if ($document->getAttribute($key) instanceof Document) { + continue; + } + //If any of the values are different, we need to check permission. + if ($newDocument->getAttribute($key) !== $value) { + $shouldUpdate = true; + $newDocument->removeAttribute('$collectionId'); + $newDocument->removeAttribute('$databaseId'); + $newDocument->setAttribute('$collection', $collection->getId()); + break; + } } - //If any of the values are different, we need to check permission. - if ($newDocument->getAttribute($key) !== $value) { - $skipCheckingPermission = false; - $newDocument->removeAttribute('$collectionId'); - $newDocument->removeAttribute('$databaseId'); - $newDocument->setAttribute('$collection', $collection->getId()); - break; + + if ($shouldUpdate) { + $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE); + } else { + Authorization::skip( + fn() => $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE) + ); } - } - if ($skipCheckingPermission) { - Authorization::skip( - fn() => $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE) - ); - } else { - $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE); - } - - try { - $document = $dbForProject->withRequestTimestamp( - $requestTimestamp, - fn() => $dbForProject->updateDocument( - 'database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), - $document->getId(), - $newDocument - ) - ); - } catch (AuthorizationException) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } catch (DuplicateException) { - throw new Exception(Exception::DOCUMENT_ALREADY_EXISTS); - } catch (StructureException $exception) { - throw new Exception(Exception::DOCUMENT_INVALID_STRUCTURE, $exception->getMessage()); - } + try { + $document = $dbForProject->withRequestTimestamp( + $requestTimestamp, + fn() => $dbForProject->updateDocument( + 'database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), + $document->getId(), + $newDocument + ) + ); + } catch (AuthorizationException) { + throw new Exception(Exception::USER_UNAUTHORIZED); + } catch (DuplicateException) { + throw new Exception(Exception::DOCUMENT_ALREADY_EXISTS); + } catch (StructureException $exception) { + throw new Exception(Exception::DOCUMENT_INVALID_STRUCTURE, $exception->getMessage()); + } // Add $collectionId and $databaseId for all documents $processDocument = function (Document $collection, Document $document) use (&$processDocument, $dbForProject, $database) { diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index 58b5b133d3..f0b26c77b6 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -369,19 +369,47 @@ class DatabasesCustomClientTest extends Scope ] ]); - // Creating one to many relationship from collection 1 to colletion 2 - $relation = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/attributes/relationship', array_merge([ + $collection3 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::unique(), + 'name' => ID::unique(), + 'documentSecurity' => false, + 'permissions' => [ + Permission::create(Role::user($userId)), + Permission::read(Role::user($userId)), + Permission::update(Role::user($userId)), + Permission::delete(Role::user($userId)), ] + ]); + + // Creating one to one relationship from collection 1 to colletion 2 + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/attributes/relationship', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] ]), [ 'relatedCollectionId' => $collection2['body']['$id'], - 'type' => 'oneToMany', + 'type' => 'oneToOne', 'twoWay' => false, 'onDelete' => 'setNull', 'key' => $collection2['body']['$id'] ]); + // Creating one to one relationship from collection 2 to colletion 3 + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection2['body']['$id'] . '/attributes/relationship', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'relatedCollectionId' => $collection3['body']['$id'], + 'type' => 'oneToOne', + 'twoWay' => false, + 'onDelete' => 'setNull', + 'key' => $collection3['body']['$id'] + ]); + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/attributes/string', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], @@ -406,20 +434,20 @@ class DatabasesCustomClientTest extends Scope 'default' => null, ]); - \sleep(2); - - $childDocument = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection2['body']['$id'] . '/documents', array_merge([ + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection3['body']['$id'] . '/attributes/string', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ - 'documentId' => ID::unique(), - 'data' => [ - 'Rating' => '10', - ], - 'permissions' => [], - ]); + ]), [ + 'key' => "Rating", + 'size' => 100, + 'required' => false, + 'array' => false, + 'default' => null, + ]); + + \sleep(2); // Creating parent document with a child reference to test the permissions $parentDocument = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents', array_merge([ 'content-type' => 'application/json', @@ -429,23 +457,43 @@ class DatabasesCustomClientTest extends Scope 'documentId' => ID::unique(), 'data' => [ 'Title' => 'Captain America', - $collection2['body']['$id'] => [$childDocument['body']['$id']] + $collection2['body']['$id'] => [ + '$id' => ID::custom($collection2['body']['$id']), + 'Rating' => '10', + $collection3['body']['$id'] => [ + '$id' => ID::custom($collection3['body']['$id']), + 'Rating' => '10' + ] + ] ], 'permissions' => [], ]); $this->assertEquals(201, $parentDocument['headers']['status-code']); - // Update document + // Update collection 3 document // This is the point of this test. We should be allowed to do this action, and it should not fail on permission check - $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents/' . $parentDocument['body']['$id'], array_merge([ + $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection3['body']['$id'] . '/documents/' . $collection3['body']['$id'], array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'data' => [ - 'Title' => 'Updated Title', + 'Rating' => '11', ] ]); $this->assertEquals(200, $response['headers']['status-code']); + + // Update collection 2 document + // We should not be allowed to update the document as we do not have permission for collection 2. + $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection2['body']['$id'] . '/documents/' . $collection2['body']['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'data' => [ + 'Rating' => '11', + ] + ]); + + $this->assertEquals(401, $response['headers']['status-code']); } } From 409376ef16a410e296106f13196e07002a25c237 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Mon, 31 Jul 2023 14:03:11 +0530 Subject: [PATCH 13/30] refactors checkPermission to throw exception when a change is found in updateDocument --- app/controllers/api/databases.php | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 3c244d612e..a61b453e0f 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3289,12 +3289,12 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $data['$permissions'] = $permissions; $newDocument = new Document($data); - $checkPermissions = (function (Document $collection, Document $document, Document $old, string $permission) use (&$checkPermissions, $dbForProject, $database) { + $checkPermissions = (function (Document $collection, Document $document, Document $old, string $permission, bool $shouldUpdate = false) use (&$checkPermissions, $dbForProject, $database) { $documentSecurity = $collection->getAttribute('documentSecurity', false); $validator = new Authorization($permission); $valid = $validator->isValid($collection->getPermissionsByType($permission)); - if (!$documentSecurity && !$valid) { + if (!$documentSecurity && !$valid && $shouldUpdate) { throw new Exception(Exception::USER_UNAUTHORIZED); } @@ -3375,13 +3375,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } } } - if ($shouldUpdate) { - $checkPermissions($relatedCollection, $relation, $relatedDocumentOldVersion, $type); - } else { - Authorization::skip( - fn() => $checkPermissions($relatedCollection, $relation, $relatedDocumentOldVersion, $type) - ); - } + $checkPermissions($relatedCollection, $relation, $relatedDocumentOldVersion, $type, $shouldUpdate); } } @@ -3408,13 +3402,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } } - if ($shouldUpdate) { - $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE); - } else { - Authorization::skip( - fn() => $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE) - ); - } + $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE, $shouldUpdate); try { $document = $dbForProject->withRequestTimestamp( From 40b0c081f72e9c1aa1f2afdc0bb0351dfcea261c Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 1 Aug 2023 23:29:15 +0530 Subject: [PATCH 14/30] remove checkPermission from Appwrite and more complex test case This commit removes check pemission from update document in appwrite as permission is being checked by Utopia already. This commits also improves the test case to have 3 levels of depth with relationships --- app/controllers/api/databases.php | 59 +------- .../Databases/DatabasesCustomClientTest.php | 129 ++++++++++++++++-- 2 files changed, 121 insertions(+), 67 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index a61b453e0f..b57ab1eff8 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3287,24 +3287,10 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $data['$createdAt'] = $document->getCreatedAt(); // Make sure user doesn't switch createdAt $data['$id'] = $document->getId(); // Make sure user doesn't switch document unique ID $data['$permissions'] = $permissions; + $data['$collection'] = $document->getAttribute('$collection'); // Attribute $collection is required for Utopia. Copying it from old version of document $newDocument = new Document($data); - $checkPermissions = (function (Document $collection, Document $document, Document $old, string $permission, bool $shouldUpdate = false) use (&$checkPermissions, $dbForProject, $database) { - $documentSecurity = $collection->getAttribute('documentSecurity', false); - $validator = new Authorization($permission); - - $valid = $validator->isValid($collection->getPermissionsByType($permission)); - if (!$documentSecurity && !$valid && $shouldUpdate) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } - - if ($permission === Database::PERMISSION_UPDATE) { - $valid = $valid || $validator->isValid($old->getPermissionsByType($permission)); - if ($documentSecurity && !$valid) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } - } - + $addCollectionAttributeToRelations = (function (Document $collection, Document $document) use (&$addCollectionAttributeToRelations, $dbForProject, $database) { $relationships = \array_filter( $collection->getAttribute('attributes', []), fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP @@ -3331,7 +3317,6 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum ); foreach ($relations as &$relation) { - $shouldUpdate = false; // If the relation is an array it can be either update or create a child document. if ( \is_array($relation) @@ -3347,35 +3332,18 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $relation->getId() )); - // Attribute $collection is required for update document. Copying it from old version of document. + // Attribute $collection is required for Utopia. Copying it from old version of document. $relation->setAttribute('$collection', $relatedDocumentOldVersion->getAttribute('$collection')); // If the child document has to be created it will need checking permissions. if ($relatedDocumentOldVersion->isEmpty()) { - $type = Database::PERMISSION_CREATE; - if (isset($relation['$id']) && $relation['$id'] === 'unique()') { $relation['$id'] = ID::unique(); } } else { - $type = Database::PERMISSION_UPDATE; - - foreach ($relation as $key => $value) { - // No need to compare values of relations as for each relation we are recursively checking permission. - if ($relatedDocumentOldVersion->getAttribute($key) instanceof Document) { - continue; - } - // If any of the values are different, we need to check permission. - if ($relatedDocumentOldVersion->getAttribute($key) !== $value) { - $shouldUpdate = true; - $relation->removeAttribute('$collectionId'); - $relation->removeAttribute('$databaseId'); - $relation->setAttribute('$collection', $relatedCollection->getId()); - break; - } - } + $relation->setAttribute('$collection', 'database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId()); } - $checkPermissions($relatedCollection, $relation, $relatedDocumentOldVersion, $type, $shouldUpdate); + $addCollectionAttributeToRelations($relatedCollection, $relation); } } @@ -3387,22 +3355,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } }); - $shouldUpdate = false; - foreach ($newDocument as $key => $value) { - if ($document->getAttribute($key) instanceof Document) { - continue; - } - //If any of the values are different, we need to check permission. - if ($newDocument->getAttribute($key) !== $value) { - $shouldUpdate = true; - $newDocument->removeAttribute('$collectionId'); - $newDocument->removeAttribute('$databaseId'); - $newDocument->setAttribute('$collection', $collection->getId()); - break; - } - } - - $checkPermissions($collection, $newDocument, $document, Database::PERMISSION_UPDATE, $shouldUpdate); + $addCollectionAttributeToRelations($collection, $newDocument); try { $document = $dbForProject->withRequestTimestamp( diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index f0b26c77b6..406d785aee 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -344,8 +344,8 @@ class DatabasesCustomClientTest extends Scope 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] ]), [ - 'collectionId' => ID::unique(), - 'name' => ID::unique(), + 'collectionId' => ID::custom('collection1'), + 'name' => ID::custom('collection1'), 'documentSecurity' => false, 'permissions' => [ Permission::create(Role::user($userId)), @@ -361,8 +361,8 @@ class DatabasesCustomClientTest extends Scope 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] ]), [ - 'collectionId' => ID::unique(), - 'name' => ID::unique(), + 'collectionId' => ID::custom('collection2'), + 'name' => ID::custom('collection2'), 'documentSecurity' => false, 'permissions' => [ Permission::read(Role::user($userId)), @@ -374,14 +374,44 @@ class DatabasesCustomClientTest extends Scope 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] ]), [ - 'collectionId' => ID::unique(), - 'name' => ID::unique(), + 'collectionId' => ID::custom('collection3'), + 'name' => ID::custom('collection3'), 'documentSecurity' => false, 'permissions' => [ Permission::create(Role::user($userId)), Permission::read(Role::user($userId)), Permission::update(Role::user($userId)), - Permission::delete(Role::user($userId)), ] + Permission::delete(Role::user($userId)), + ] + ]); + + $collection4 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::custom('collection4'), + 'name' => ID::custom('collection4'), + 'documentSecurity' => false, + 'permissions' => [ + Permission::read(Role::user($userId)), + ] + ]); + + $collection5 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::custom('collection5'), + 'name' => ID::custom('collection5'), + 'documentSecurity' => false, + 'permissions' => [ + Permission::create(Role::user($userId)), + Permission::read(Role::user($userId)), + Permission::update(Role::user($userId)), + Permission::delete(Role::user($userId)), + ] ]); // Creating one to one relationship from collection 1 to colletion 2 @@ -410,6 +440,32 @@ class DatabasesCustomClientTest extends Scope 'key' => $collection3['body']['$id'] ]); + // Creating one to one relationship from collection 3 to colletion 4 + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection3['body']['$id'] . '/attributes/relationship', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'relatedCollectionId' => $collection4['body']['$id'], + 'type' => 'oneToOne', + 'twoWay' => false, + 'onDelete' => 'setNull', + 'key' => $collection4['body']['$id'] + ]); + + // Creating one to one relationship from collection 4 to colletion 5 + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection4['body']['$id'] . '/attributes/relationship', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'relatedCollectionId' => $collection5['body']['$id'], + 'type' => 'oneToOne', + 'twoWay' => false, + 'onDelete' => 'setNull', + 'key' => $collection5['body']['$id'] + ]); + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/attributes/string', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], @@ -446,6 +502,29 @@ class DatabasesCustomClientTest extends Scope 'default' => null, ]); + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection4['body']['$id'] . '/attributes/string', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'key' => "Rating", + 'size' => 100, + 'required' => false, + 'array' => false, + 'default' => null, + ]); + + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection5['body']['$id'] . '/attributes/string', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'key' => "Rating", + 'size' => 100, + 'required' => false, + 'array' => false, + 'default' => null, + ]); \sleep(2); // Creating parent document with a child reference to test the permissions @@ -454,7 +533,7 @@ class DatabasesCustomClientTest extends Scope 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] ]), [ - 'documentId' => ID::unique(), + 'documentId' => ID::custom($collection1['body']['$id']), 'data' => [ 'Title' => 'Captain America', $collection2['body']['$id'] => [ @@ -462,22 +541,44 @@ class DatabasesCustomClientTest extends Scope 'Rating' => '10', $collection3['body']['$id'] => [ '$id' => ID::custom($collection3['body']['$id']), - 'Rating' => '10' + 'Rating' => '10', + $collection4['body']['$id'] => [ + '$id' => ID::custom($collection4['body']['$id']), + 'Rating' => '10', + $collection5['body']['$id'] => [ + '$id' => ID::custom($collection5['body']['$id']), + 'Rating' => '10' + ] + ] ] ] - ], - 'permissions' => [], + ] ]); $this->assertEquals(201, $parentDocument['headers']['status-code']); - // Update collection 3 document // This is the point of this test. We should be allowed to do this action, and it should not fail on permission check - $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection3['body']['$id'] . '/documents/' . $collection3['body']['$id'], array_merge([ + $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents/' . $collection1['body']['$id'], array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'data' => [ - 'Rating' => '11', + 'Title' => 'Captain America', + $collection2['body']['$id'] => [ + '$id' => ID::custom($collection2['body']['$id']), + 'Rating' => '10', + $collection3['body']['$id'] => [ + '$id' => ID::custom($collection3['body']['$id']), + 'Rating' => '11', + $collection4['body']['$id'] => [ + '$id' => ID::custom($collection4['body']['$id']), + 'Rating' => '10', + $collection5['body']['$id'] => [ + '$id' => ID::custom($collection5['body']['$id']), + 'Rating' => '11' + ] + ] + ] + ] ] ]); From fb3bab7e2e2cee94635c28018861e83f95259542 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 1 Aug 2023 23:32:15 +0530 Subject: [PATCH 15/30] fix lint issues --- tests/e2e/Services/Databases/DatabasesCustomClientTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index 406d785aee..f50dada5b8 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -381,7 +381,7 @@ class DatabasesCustomClientTest extends Scope Permission::create(Role::user($userId)), Permission::read(Role::user($userId)), Permission::update(Role::user($userId)), - Permission::delete(Role::user($userId)), + Permission::delete(Role::user($userId)), ] ]); @@ -410,7 +410,7 @@ class DatabasesCustomClientTest extends Scope Permission::create(Role::user($userId)), Permission::read(Role::user($userId)), Permission::update(Role::user($userId)), - Permission::delete(Role::user($userId)), + Permission::delete(Role::user($userId)), ] ]); From 838991d68b9d2eaca9845627ff7e4750bb225bfd Mon Sep 17 00:00:00 2001 From: prateek banga Date: Thu, 3 Aug 2023 15:24:08 +0530 Subject: [PATCH 16/30] update db, audit and abuse version --- composer.json | 6 ++--- composer.lock | 64 +++++++++++++++++++++++++-------------------------- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/composer.json b/composer.json index 5794d208dd..6c31fa8873 100644 --- a/composer.json +++ b/composer.json @@ -43,13 +43,13 @@ "ext-sockets": "*", "appwrite/php-clamav": "2.0.*", "appwrite/php-runtimes": "0.11.*", - "utopia-php/abuse": "0.28.*", + "utopia-php/abuse": "0.29.*", "utopia-php/analytics": "0.2.*", - "utopia-php/audit": "0.30.*", + "utopia-php/audit": "0.31.*", "utopia-php/cache": "0.8.*", "utopia-php/cli": "0.13.*", "utopia-php/config": "0.2.*", - "utopia-php/database": "0.39.*", + "utopia-php/database": "0.40.*", "utopia-php/domains": "1.1.*", "utopia-php/framework": "0.28.*", "utopia-php/image": "0.5.*", diff --git a/composer.lock b/composer.lock index e292fc1a0c..c9d6d1295f 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "b8aa5b23fbd8e5e1bda485f5ef5798d8", + "content-hash": "1dde9b87dc894a8ab765cad3567bfe7c", "packages": [ { "name": "adhocore/jwt", @@ -994,16 +994,16 @@ }, { "name": "matomo/device-detector", - "version": "6.1.3", + "version": "6.1.4", "source": { "type": "git", "url": "https://github.com/matomo-org/device-detector.git", - "reference": "3e0fac7e77f3faadc3858fea9f5fa7efeb9cf239" + "reference": "74f6c4f6732b3ad6cdf25560746841d522969112" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/matomo-org/device-detector/zipball/3e0fac7e77f3faadc3858fea9f5fa7efeb9cf239", - "reference": "3e0fac7e77f3faadc3858fea9f5fa7efeb9cf239", + "url": "https://api.github.com/repos/matomo-org/device-detector/zipball/74f6c4f6732b3ad6cdf25560746841d522969112", + "reference": "74f6c4f6732b3ad6cdf25560746841d522969112", "shasum": "" }, "require": { @@ -1059,7 +1059,7 @@ "source": "https://github.com/matomo-org/matomo", "wiki": "https://dev.matomo.org/" }, - "time": "2023-06-06T11:58:07+00:00" + "time": "2023-08-02T08:48:53+00:00" }, { "name": "mongodb/mongodb", @@ -1804,23 +1804,23 @@ }, { "name": "utopia-php/abuse", - "version": "0.28.0", + "version": "0.29.0", "source": { "type": "git", "url": "https://github.com/utopia-php/abuse.git", - "reference": "256584e2baa1fb8a7747619b1ef19f7917b2fa07" + "reference": "7589d0c7a6f685fcbb02d57875f034bd233ec262" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/abuse/zipball/256584e2baa1fb8a7747619b1ef19f7917b2fa07", - "reference": "256584e2baa1fb8a7747619b1ef19f7917b2fa07", + "url": "https://api.github.com/repos/utopia-php/abuse/zipball/7589d0c7a6f685fcbb02d57875f034bd233ec262", + "reference": "7589d0c7a6f685fcbb02d57875f034bd233ec262", "shasum": "" }, "require": { "ext-curl": "*", "ext-pdo": "*", "php": ">=8.0", - "utopia-php/database": "0.39.*" + "utopia-php/database": "0.40.*" }, "require-dev": { "laravel/pint": "1.5.*", @@ -1847,9 +1847,9 @@ ], "support": { "issues": "https://github.com/utopia-php/abuse/issues", - "source": "https://github.com/utopia-php/abuse/tree/0.28.0" + "source": "https://github.com/utopia-php/abuse/tree/0.29.0" }, - "time": "2023-07-27T08:06:39+00:00" + "time": "2023-08-03T08:22:12+00:00" }, { "name": "utopia-php/analytics", @@ -1908,21 +1908,21 @@ }, { "name": "utopia-php/audit", - "version": "0.30.0", + "version": "0.31.0", "source": { "type": "git", "url": "https://github.com/utopia-php/audit.git", - "reference": "e171836a2b03129f28db9beb616f8975183f445a" + "reference": "f1b0165fccb6ec665f7ae947bfed56a8f6b877cb" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/audit/zipball/e171836a2b03129f28db9beb616f8975183f445a", - "reference": "e171836a2b03129f28db9beb616f8975183f445a", + "url": "https://api.github.com/repos/utopia-php/audit/zipball/f1b0165fccb6ec665f7ae947bfed56a8f6b877cb", + "reference": "f1b0165fccb6ec665f7ae947bfed56a8f6b877cb", "shasum": "" }, "require": { "php": ">=8.0", - "utopia-php/database": "0.39.*" + "utopia-php/database": "0.40.*" }, "require-dev": { "laravel/pint": "1.5.*", @@ -1949,9 +1949,9 @@ ], "support": { "issues": "https://github.com/utopia-php/audit/issues", - "source": "https://github.com/utopia-php/audit/tree/0.30.0" + "source": "https://github.com/utopia-php/audit/tree/0.31.0" }, - "time": "2023-07-27T08:04:56+00:00" + "time": "2023-08-03T08:22:32+00:00" }, { "name": "utopia-php/cache", @@ -2108,16 +2108,16 @@ }, { "name": "utopia-php/database", - "version": "0.39.0", + "version": "0.40.0", "source": { "type": "git", "url": "https://github.com/utopia-php/database.git", - "reference": "a9f706034a12c7c87e6fd38eca69587fd3def0e0" + "reference": "ec586ba1c5fc83856df4feaa05464da72b72ee0d" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/database/zipball/a9f706034a12c7c87e6fd38eca69587fd3def0e0", - "reference": "a9f706034a12c7c87e6fd38eca69587fd3def0e0", + "url": "https://api.github.com/repos/utopia-php/database/zipball/ec586ba1c5fc83856df4feaa05464da72b72ee0d", + "reference": "ec586ba1c5fc83856df4feaa05464da72b72ee0d", "shasum": "" }, "require": { @@ -2158,9 +2158,9 @@ ], "support": { "issues": "https://github.com/utopia-php/database/issues", - "source": "https://github.com/utopia-php/database/tree/0.39.0" + "source": "https://github.com/utopia-php/database/tree/0.40.0" }, - "time": "2023-07-26T18:03:25+00:00" + "time": "2023-08-03T08:01:37+00:00" }, { "name": "utopia-php/domains", @@ -4758,16 +4758,16 @@ }, { "name": "sebastian/global-state", - "version": "5.0.5", + "version": "5.0.6", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/global-state.git", - "reference": "0ca8db5a5fc9c8646244e629625ac486fa286bf2" + "reference": "bde739e7565280bda77be70044ac1047bc007e34" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/global-state/zipball/0ca8db5a5fc9c8646244e629625ac486fa286bf2", - "reference": "0ca8db5a5fc9c8646244e629625ac486fa286bf2", + "url": "https://api.github.com/repos/sebastianbergmann/global-state/zipball/bde739e7565280bda77be70044ac1047bc007e34", + "reference": "bde739e7565280bda77be70044ac1047bc007e34", "shasum": "" }, "require": { @@ -4810,7 +4810,7 @@ ], "support": { "issues": "https://github.com/sebastianbergmann/global-state/issues", - "source": "https://github.com/sebastianbergmann/global-state/tree/5.0.5" + "source": "https://github.com/sebastianbergmann/global-state/tree/5.0.6" }, "funding": [ { @@ -4818,7 +4818,7 @@ "type": "github" } ], - "time": "2022-02-14T08:28:10+00:00" + "time": "2023-08-02T09:26:13+00:00" }, { "name": "sebastian/lines-of-code", From 60236b3af67492338bbc2a98a8c5147f329ae2c5 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 8 Aug 2023 04:40:07 +0530 Subject: [PATCH 17/30] refactor according to review comments --- app/controllers/api/databases.php | 22 +++++++++---------- composer.lock | 36 +++++++++++++++---------------- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index ca1148354d..607711d95d 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3284,7 +3284,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $data['$collection'] = $document->getAttribute('$collection'); // Attribute $collection is required for Utopia. Copying it from old version of document $newDocument = new Document($data); - $addCollectionAttributeToRelations = (function (Document $collection, Document $document) use (&$addCollectionAttributeToRelations, $dbForProject, $database) { + $setCollection = (function (Document $collection, Document $document) use (&$setCollection, $dbForProject, $database) { $relationships = \array_filter( $collection->getAttribute('attributes', []), fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP @@ -3321,23 +3321,22 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $relation = new Document($relation); } if ($relation instanceof Document) { - $relatedDocumentOldVersion = Authorization::skip(fn() => $dbForProject->getDocument( + $oldDocument = Authorization::skip(fn() => $dbForProject->getDocument( 'database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId(), $relation->getId() )); + $relation->removeAttribute('$collectionId'); + $relation->removeAttribute('$databaseId'); + // Attribute $collection is required for Utopia. + $relation->setAttribute('$collection', + 'database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId()); - // Attribute $collection is required for Utopia. Copying it from old version of document. - $relation->setAttribute('$collection', $relatedDocumentOldVersion->getAttribute('$collection')); - - // If the child document has to be created it will need checking permissions. - if ($relatedDocumentOldVersion->isEmpty()) { + if ($oldDocument->isEmpty()) { if (isset($relation['$id']) && $relation['$id'] === 'unique()') { $relation['$id'] = ID::unique(); } - } else { - $relation->setAttribute('$collection', 'database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId()); } - $addCollectionAttributeToRelations($relatedCollection, $relation); + $setCollection($relatedCollection, $relation); } } @@ -3349,8 +3348,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } }); - $addCollectionAttributeToRelations($collection, $newDocument); - + $setCollection($collection, $newDocument); try { $document = $dbForProject->withRequestTimestamp( $requestTimestamp, diff --git a/composer.lock b/composer.lock index 528fdb0f59..dcfe117923 100644 --- a/composer.lock +++ b/composer.lock @@ -607,16 +607,16 @@ }, { "name": "guzzlehttp/promises", - "version": "2.0.0", + "version": "2.0.1", "source": { "type": "git", "url": "https://github.com/guzzle/promises.git", - "reference": "3a494dc7dc1d7d12e511890177ae2d0e6c107da6" + "reference": "111166291a0f8130081195ac4556a5587d7f1b5d" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/guzzle/promises/zipball/3a494dc7dc1d7d12e511890177ae2d0e6c107da6", - "reference": "3a494dc7dc1d7d12e511890177ae2d0e6c107da6", + "url": "https://api.github.com/repos/guzzle/promises/zipball/111166291a0f8130081195ac4556a5587d7f1b5d", + "reference": "111166291a0f8130081195ac4556a5587d7f1b5d", "shasum": "" }, "require": { @@ -670,7 +670,7 @@ ], "support": { "issues": "https://github.com/guzzle/promises/issues", - "source": "https://github.com/guzzle/promises/tree/2.0.0" + "source": "https://github.com/guzzle/promises/tree/2.0.1" }, "funding": [ { @@ -686,20 +686,20 @@ "type": "tidelift" } ], - "time": "2023-05-21T13:50:22+00:00" + "time": "2023-08-03T15:11:55+00:00" }, { "name": "guzzlehttp/psr7", - "version": "2.5.0", + "version": "2.6.0", "source": { "type": "git", "url": "https://github.com/guzzle/psr7.git", - "reference": "b635f279edd83fc275f822a1188157ffea568ff6" + "reference": "8bd7c33a0734ae1c5d074360512beb716bef3f77" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/guzzle/psr7/zipball/b635f279edd83fc275f822a1188157ffea568ff6", - "reference": "b635f279edd83fc275f822a1188157ffea568ff6", + "url": "https://api.github.com/repos/guzzle/psr7/zipball/8bd7c33a0734ae1c5d074360512beb716bef3f77", + "reference": "8bd7c33a0734ae1c5d074360512beb716bef3f77", "shasum": "" }, "require": { @@ -786,7 +786,7 @@ ], "support": { "issues": "https://github.com/guzzle/psr7/issues", - "source": "https://github.com/guzzle/psr7/tree/2.5.0" + "source": "https://github.com/guzzle/psr7/tree/2.6.0" }, "funding": [ { @@ -802,7 +802,7 @@ "type": "tidelift" } ], - "time": "2023-04-17T16:11:26+00:00" + "time": "2023-08-03T15:06:02+00:00" }, { "name": "influxdb/influxdb-php", @@ -3785,16 +3785,16 @@ }, { "name": "phpstan/phpdoc-parser", - "version": "1.23.0", + "version": "1.23.1", "source": { "type": "git", "url": "https://github.com/phpstan/phpdoc-parser.git", - "reference": "a2b24135c35852b348894320d47b3902a94bc494" + "reference": "846ae76eef31c6d7790fac9bc399ecee45160b26" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpdoc-parser/zipball/a2b24135c35852b348894320d47b3902a94bc494", - "reference": "a2b24135c35852b348894320d47b3902a94bc494", + "url": "https://api.github.com/repos/phpstan/phpdoc-parser/zipball/846ae76eef31c6d7790fac9bc399ecee45160b26", + "reference": "846ae76eef31c6d7790fac9bc399ecee45160b26", "shasum": "" }, "require": { @@ -3826,9 +3826,9 @@ "description": "PHPDoc parser with support for nullable, intersection and generic types", "support": { "issues": "https://github.com/phpstan/phpdoc-parser/issues", - "source": "https://github.com/phpstan/phpdoc-parser/tree/1.23.0" + "source": "https://github.com/phpstan/phpdoc-parser/tree/1.23.1" }, - "time": "2023-07-23T22:17:56+00:00" + "time": "2023-08-03T16:32:59+00:00" }, { "name": "phpunit/php-code-coverage", From 385f76d7f29fa8c4d10bb6a8e961545517e4218f Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 8 Aug 2023 04:42:00 +0530 Subject: [PATCH 18/30] lint fix --- app/controllers/api/databases.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 607711d95d..35e4a623fb 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3328,8 +3328,10 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $relation->removeAttribute('$collectionId'); $relation->removeAttribute('$databaseId'); // Attribute $collection is required for Utopia. - $relation->setAttribute('$collection', - 'database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId()); + $relation->setAttribute( + '$collection', + 'database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId() + ); if ($oldDocument->isEmpty()) { if (isset($relation['$id']) && $relation['$id'] === 'unique()') { From ebcb43bbadacfe27754fb169fe90b483c3f6c586 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 8 Aug 2023 21:20:19 +0530 Subject: [PATCH 19/30] use getUser to fetch userId in test --- .../e2e/Services/Databases/DatabasesCustomClientTest.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index f50dada5b8..0b86dba9cb 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -320,13 +320,7 @@ class DatabasesCustomClientTest extends Scope public function testUpdateWithoutRelationPermission(): void { - $response = $this->client->call(Client::METHOD_GET, '/account', array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - 'x-appwrite-key' => $this->getProject()['apiKey'] - ], $this->getHeaders())); - - $userId = $response['body']['$id']; + $userId = $this->getUser()['$id']; $database = $this->client->call(Client::METHOD_POST, '/databases', [ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], From 983c810a45bc50c285e8a7fda43ab7ac6c744387 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Thu, 10 Aug 2023 17:18:45 +0530 Subject: [PATCH 20/30] adds assertion in test case --- tests/e2e/Services/Databases/DatabasesCustomClientTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index 0b86dba9cb..a1af48a0ea 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -577,7 +577,7 @@ class DatabasesCustomClientTest extends Scope ]); $this->assertEquals(200, $response['headers']['status-code']); - + $this->assertEquals(11, $response['body'][$collection2['body']['$id']]['collection3']['Rating']); // Update collection 2 document // We should not be allowed to update the document as we do not have permission for collection 2. $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection2['body']['$id'] . '/documents/' . $collection2['body']['$id'], array_merge([ From 40047a2ed3a99720fad62473b0dcd900d0ecd808 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Thu, 10 Aug 2023 20:03:02 +0530 Subject: [PATCH 21/30] fix test cases --- tests/e2e/Services/Databases/DatabasesBase.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/e2e/Services/Databases/DatabasesBase.php b/tests/e2e/Services/Databases/DatabasesBase.php index bd9443adec..16ff57ab76 100644 --- a/tests/e2e/Services/Databases/DatabasesBase.php +++ b/tests/e2e/Services/Databases/DatabasesBase.php @@ -1325,7 +1325,7 @@ trait DatabasesBase 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'queries' => [ - 'select(["title", "releaseYear"])', + 'select(["title", "releaseYear", "$id"])', ], ]); @@ -4045,7 +4045,7 @@ trait DatabasesBase 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'queries' => [ - 'select(["libraries.*"])', + 'select(["libraries.*", "$id"])', ], ]); @@ -4058,7 +4058,7 @@ trait DatabasesBase 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'queries' => [ - 'select(["fullName"])' + 'select(["fullName", "$id"])' ], ]); From 6cd0e328d93236829ba5af34963cbe7107dbff4b Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 10 Aug 2023 16:44:38 -0400 Subject: [PATCH 22/30] Review fixes --- app/controllers/api/databases.php | 7 ++++--- docker-compose.yml | 2 +- .../Databases/DatabasesCustomClientTest.php | 20 ++++++++++++++++++- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 66ac7ccf60..6e4273362c 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3197,13 +3197,13 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } $data = \array_merge($document->getArrayCopy(), $data); // Merge existing data with new data + $data['$collection'] = $collection->getId(); // Make sure user doesn't switch collectionID $data['$createdAt'] = $document->getCreatedAt(); // Make sure user doesn't switch createdAt $data['$id'] = $document->getId(); // Make sure user doesn't switch document unique ID $data['$permissions'] = $permissions; - $data['$collection'] = $document->getAttribute('$collection'); // Attribute $collection is required for Utopia. Copying it from old version of document $newDocument = new Document($data); - $setCollection = (function (Document $collection, Document $document) use (&$setCollection, $dbForProject, $database) { + $setCollection = function (Document $collection, Document $document) use (&$setCollection, $dbForProject, $database) { $relationships = \array_filter( $collection->getAttribute('attributes', []), fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP @@ -3267,9 +3267,10 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $document->setAttribute($relationship->getAttribute('key'), \reset($relations)); } } - }); + }; $setCollection($collection, $newDocument); + try { $document = $dbForProject->withRequestTimestamp( $requestTimestamp, diff --git a/docker-compose.yml b/docker-compose.yml index 25781932f1..7b37f9fed7 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -84,7 +84,7 @@ services: - ./docs:/usr/src/code/docs - ./public:/usr/src/code/public - ./src:/usr/src/code/src - - ./dev:/usr/local/dev + - ./dev:/usr/src/code/dev depends_on: - mariadb - redis diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index a1af48a0ea..046312fec2 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -548,6 +548,7 @@ class DatabasesCustomClientTest extends Scope ] ] ]); + $this->assertEquals(201, $parentDocument['headers']['status-code']); // This is the point of this test. We should be allowed to do this action, and it should not fail on permission check @@ -578,7 +579,24 @@ class DatabasesCustomClientTest extends Scope $this->assertEquals(200, $response['headers']['status-code']); $this->assertEquals(11, $response['body'][$collection2['body']['$id']]['collection3']['Rating']); - // Update collection 2 document + + // We should not be allowed to update the document as we do not have permission for collection 2. + $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents/' . $collection1['body']['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'data' => [ + 'Title' => 'Captain America', + $collection2['body']['$id'] => [ + '$id' => ID::custom($collection2['body']['$id']), + 'Rating' => '11', + $collection3['body']['$id'] => null, + ] + ] + ]); + + $this->assertEquals(401, $response['headers']['status-code']); + // We should not be allowed to update the document as we do not have permission for collection 2. $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection2['body']['$id'] . '/documents/' . $collection2['body']['$id'], array_merge([ 'content-type' => 'application/json', From b0b6f937949656ef1258f72d9367c8fa04823d5b Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 10 Aug 2023 21:28:04 -0400 Subject: [PATCH 23/30] Update libs --- composer.json | 6 +++--- composer.lock | 42 +++++++++++++++++++++--------------------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/composer.json b/composer.json index b6ff28d89e..1edb6d0e1c 100644 --- a/composer.json +++ b/composer.json @@ -43,13 +43,13 @@ "ext-sockets": "*", "appwrite/php-clamav": "2.0.*", "appwrite/php-runtimes": "0.11.*", - "utopia-php/abuse": "0.30.*", + "utopia-php/abuse": "0.31.*", "utopia-php/analytics": "0.10.*", - "utopia-php/audit": "0.32.*", + "utopia-php/audit": "0.33.*", "utopia-php/cache": "0.8.*", "utopia-php/cli": "0.15.*", "utopia-php/config": "0.2.*", - "utopia-php/database": "0.41.*", + "utopia-php/database": "0.42.*", "utopia-php/domains": "1.1.*", "utopia-php/dsn": "0.1.*", "utopia-php/framework": "0.28.*", diff --git a/composer.lock b/composer.lock index dfb3d9e859..4a0862679d 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "8068408e6b0e275fb7887c956b918d03", + "content-hash": "2098172fc4b71eb0d41dcdbfea2f5061", "packages": [ { "name": "adhocore/jwt", @@ -1225,23 +1225,23 @@ }, { "name": "utopia-php/abuse", - "version": "0.30.0", + "version": "0.31.0", "source": { "type": "git", "url": "https://github.com/utopia-php/abuse.git", - "reference": "9ba1b9ca96386aedfb5be23d739433652124e00f" + "reference": "d771c2c8d7d1237b1d04b5bf57b07a9b4736e627" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/abuse/zipball/9ba1b9ca96386aedfb5be23d739433652124e00f", - "reference": "9ba1b9ca96386aedfb5be23d739433652124e00f", + "url": "https://api.github.com/repos/utopia-php/abuse/zipball/d771c2c8d7d1237b1d04b5bf57b07a9b4736e627", + "reference": "d771c2c8d7d1237b1d04b5bf57b07a9b4736e627", "shasum": "" }, "require": { "ext-curl": "*", "ext-pdo": "*", "php": ">=8.0", - "utopia-php/database": "0.41.*" + "utopia-php/database": "0.42.*" }, "require-dev": { "laravel/pint": "1.5.*", @@ -1268,9 +1268,9 @@ ], "support": { "issues": "https://github.com/utopia-php/abuse/issues", - "source": "https://github.com/utopia-php/abuse/tree/0.30.0" + "source": "https://github.com/utopia-php/abuse/tree/0.31.0" }, - "time": "2023-08-09T17:34:55+00:00" + "time": "2023-08-11T01:17:15+00:00" }, { "name": "utopia-php/analytics", @@ -1320,21 +1320,21 @@ }, { "name": "utopia-php/audit", - "version": "0.32.0", + "version": "0.33.0", "source": { "type": "git", "url": "https://github.com/utopia-php/audit.git", - "reference": "db769bcbc3bc4a187e3be97f5e532311ad8fae27" + "reference": "6fb82331c58c66cbdb8a419314a687fcb18a1d22" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/audit/zipball/db769bcbc3bc4a187e3be97f5e532311ad8fae27", - "reference": "db769bcbc3bc4a187e3be97f5e532311ad8fae27", + "url": "https://api.github.com/repos/utopia-php/audit/zipball/6fb82331c58c66cbdb8a419314a687fcb18a1d22", + "reference": "6fb82331c58c66cbdb8a419314a687fcb18a1d22", "shasum": "" }, "require": { "php": ">=8.0", - "utopia-php/database": "0.41.*" + "utopia-php/database": "0.42.*" }, "require-dev": { "laravel/pint": "1.5.*", @@ -1361,9 +1361,9 @@ ], "support": { "issues": "https://github.com/utopia-php/audit/issues", - "source": "https://github.com/utopia-php/audit/tree/0.32.0" + "source": "https://github.com/utopia-php/audit/tree/0.33.0" }, - "time": "2023-08-09T17:34:57+00:00" + "time": "2023-08-11T01:17:28+00:00" }, { "name": "utopia-php/cache", @@ -1516,16 +1516,16 @@ }, { "name": "utopia-php/database", - "version": "0.41.0", + "version": "0.42.0", "source": { "type": "git", "url": "https://github.com/utopia-php/database.git", - "reference": "98dc6a94e636fe8f955c20fa4833db8986b5c27a" + "reference": "5631f151ce2b454517f11c1914e50a1fa70c7bd1" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/database/zipball/98dc6a94e636fe8f955c20fa4833db8986b5c27a", - "reference": "98dc6a94e636fe8f955c20fa4833db8986b5c27a", + "url": "https://api.github.com/repos/utopia-php/database/zipball/5631f151ce2b454517f11c1914e50a1fa70c7bd1", + "reference": "5631f151ce2b454517f11c1914e50a1fa70c7bd1", "shasum": "" }, "require": { @@ -1566,9 +1566,9 @@ ], "support": { "issues": "https://github.com/utopia-php/database/issues", - "source": "https://github.com/utopia-php/database/tree/0.41.0" + "source": "https://github.com/utopia-php/database/tree/0.42.0" }, - "time": "2023-08-09T02:52:31+00:00" + "time": "2023-08-10T23:18:38+00:00" }, { "name": "utopia-php/domains", From 16cb495a41a67566c37e150fc2c96a4f28e5ca93 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 10 Aug 2023 21:31:31 -0400 Subject: [PATCH 24/30] Lint --- app/controllers/api/databases.php | 32 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 6e4273362c..efc93a5f4e 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3271,22 +3271,22 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $setCollection($collection, $newDocument); - try { - $document = $dbForProject->withRequestTimestamp( - $requestTimestamp, - fn() => $dbForProject->updateDocument( - 'database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), - $document->getId(), - $newDocument - ) - ); - } catch (AuthorizationException) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } catch (DuplicateException) { - throw new Exception(Exception::DOCUMENT_ALREADY_EXISTS); - } catch (StructureException $exception) { - throw new Exception(Exception::DOCUMENT_INVALID_STRUCTURE, $exception->getMessage()); - } + try { + $document = $dbForProject->withRequestTimestamp( + $requestTimestamp, + fn() => $dbForProject->updateDocument( + 'database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), + $document->getId(), + $newDocument + ) + ); + } catch (AuthorizationException) { + throw new Exception(Exception::USER_UNAUTHORIZED); + } catch (DuplicateException) { + throw new Exception(Exception::DOCUMENT_ALREADY_EXISTS); + } catch (StructureException $exception) { + throw new Exception(Exception::DOCUMENT_INVALID_STRUCTURE, $exception->getMessage()); + } // Add $collectionId and $databaseId for all documents $processDocument = function (Document $collection, Document $document) use (&$processDocument, $dbForProject, $database) { From 971ebbc686a813772db6b9af729ffa7c40461622 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Fri, 11 Aug 2023 16:17:31 +0530 Subject: [PATCH 25/30] fix test cases --- app/controllers/api/databases.php | 2 +- app/init.php | 8 ++++---- .../e2e/Services/Databases/DatabasesCustomClientTest.php | 1 - 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index efc93a5f4e..13e9a7bdb8 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3197,7 +3197,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } $data = \array_merge($document->getArrayCopy(), $data); // Merge existing data with new data - $data['$collection'] = $collection->getId(); // Make sure user doesn't switch collectionID + $data['$collection'] = $document->getAttribute('$collection'); // Make sure user doesn't switch collectionID $data['$createdAt'] = $document->getCreatedAt(); // Make sure user doesn't switch createdAt $data['$id'] = $document->getId(); // Make sure user doesn't switch document unique ID $data['$permissions'] = $permissions; diff --git a/app/init.php b/app/init.php index 2e59869993..0c8baa5273 100644 --- a/app/init.php +++ b/app/init.php @@ -947,7 +947,7 @@ App::setResource('user', function ($mode, $project, $console, $request, $respons if (APP_MODE_ADMIN !== $mode) { if ($project->isEmpty()) { - $user = new Document(['$id' => ID::custom(''), '$collection' => 'users']); + $user = new Document(); } else { $user = $dbForProject->getDocument('users', Auth::$unique); } @@ -959,14 +959,14 @@ App::setResource('user', function ($mode, $project, $console, $request, $respons $user->isEmpty() // Check a document has been found in the DB || !Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret, $authDuration) ) { // Validate user has valid login token - $user = new Document(['$id' => ID::custom(''), '$collection' => 'users']); + $user = new Document(); } if (APP_MODE_ADMIN === $mode) { if ($user->find('teamId', $project->getAttribute('teamId'), 'memberships')) { Authorization::setDefaultStatus(false); // Cancel security segmentation for admin users. } else { - $user = new Document(['$id' => ID::custom(''), '$collection' => 'users']); + $user = new Document(); } } @@ -989,7 +989,7 @@ App::setResource('user', function ($mode, $project, $console, $request, $respons } if (empty($user->find('$id', $jwtSessionId, 'sessions'))) { // Match JWT to active token - $user = new Document(['$id' => ID::custom(''), '$collection' => 'users']); + $user = new Document(); } } diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index 046312fec2..2e278b8437 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -344,7 +344,6 @@ class DatabasesCustomClientTest extends Scope 'permissions' => [ Permission::create(Role::user($userId)), Permission::read(Role::user($userId)), - Permission::update(Role::user($userId)), Permission::delete(Role::user($userId)), ] ]); From 68736a53f9d3011bb01ddf9f51e85d6401db3003 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Fri, 11 Aug 2023 17:25:34 +0530 Subject: [PATCH 26/30] Adds more complex test scenario --- .../Databases/DatabasesCustomClientTest.php | 47 ++++++++++++++++++- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index 2e278b8437..ef00041af3 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -373,7 +373,6 @@ class DatabasesCustomClientTest extends Scope 'permissions' => [ Permission::create(Role::user($userId)), Permission::read(Role::user($userId)), - Permission::update(Role::user($userId)), Permission::delete(Role::user($userId)), ] ]); @@ -402,7 +401,6 @@ class DatabasesCustomClientTest extends Scope 'permissions' => [ Permission::create(Role::user($userId)), Permission::read(Role::user($userId)), - Permission::update(Role::user($userId)), Permission::delete(Role::user($userId)), ] ]); @@ -549,6 +547,51 @@ class DatabasesCustomClientTest extends Scope ]); $this->assertEquals(201, $parentDocument['headers']['status-code']); + // This is the point of the test. We should not need any authorization permission to update the document with same data. + $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents/' . $collection1['body']['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'documentId' => ID::custom($collection1['body']['$id']), + 'data' => [ + 'Title' => 'Captain America', + $collection2['body']['$id'] => [ + '$id' => $collection2['body']['$id'], + 'Rating' => '10', + $collection3['body']['$id'] => [ + '$id' => $collection3['body']['$id'], + 'Rating' => '10', + $collection4['body']['$id'] => [ + '$id' => $collection4['body']['$id'], + 'Rating' => '10', + $collection5['body']['$id'] => [ + '$id' => $collection5['body']['$id'], + 'Rating' => '10' + ] + ] + ] + ] + ] + ]); + $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEquals($parentDocument['body'], $response['body']); + + // Giving update permission of collection 3 to user. + $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/collection3', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::custom('collection3'), + 'name' => ID::custom('collection3'), + 'documentSecurity' => false, + 'permissions' => [ + Permission::create(Role::user($userId)), + Permission::read(Role::user($userId)), + Permission::update(Role::user($userId)), + Permission::delete(Role::user($userId)), + ] + ]); // This is the point of this test. We should be allowed to do this action, and it should not fail on permission check $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents/' . $collection1['body']['$id'], array_merge([ From 29e1757364d57ce0ba64ceec57ef7372399fc378 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Fri, 11 Aug 2023 17:28:32 +0530 Subject: [PATCH 27/30] lint fix --- tests/e2e/Services/Databases/DatabasesCustomClientTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index ef00041af3..32552e4de7 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -576,7 +576,7 @@ class DatabasesCustomClientTest extends Scope $this->assertEquals(200, $response['headers']['status-code']); $this->assertEquals($parentDocument['body'], $response['body']); - // Giving update permission of collection 3 to user. + // Giving update permission of collection 3 to user. $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/collection3', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], From cb6760a957f94ff9780412b3543ce30ab78c8c12 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Mon, 14 Aug 2023 22:02:54 +0530 Subject: [PATCH 28/30] removes unnecessary use statement --- tests/e2e/Services/Databases/DatabasesCustomClientTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index 32552e4de7..87bc24762e 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -6,7 +6,6 @@ use Tests\E2E\Client; use Tests\E2E\Scopes\Scope; use Tests\E2E\Scopes\ProjectCustom; use Tests\E2E\Scopes\SideClient; -use Tests\E2E\Services\Storage\StoragePermissionsScope; use Utopia\Database\Helpers\ID; use Utopia\Database\Helpers\Permission; use Utopia\Database\Helpers\Role; From d6f4d76807febeea2063b2ba7e01cfef7f6eb950 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Wed, 16 Aug 2023 14:57:50 +0530 Subject: [PATCH 29/30] adds another scenario in test case --- .../Databases/DatabasesCustomClientTest.php | 106 ++++++++++++++++-- 1 file changed, 94 insertions(+), 12 deletions(-) diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index 87bc24762e..743df9e53a 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -468,53 +468,53 @@ class DatabasesCustomClientTest extends Scope 'default' => null, ]); - $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection2['body']['$id'] . '/attributes/string', array_merge([ + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection2['body']['$id'] . '/attributes/string', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ + ]), [ 'key' => "Rating", 'size' => 100, 'required' => false, 'array' => false, 'default' => null, - ]); + ]); - $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection3['body']['$id'] . '/attributes/string', array_merge([ + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection3['body']['$id'] . '/attributes/string', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ + ]), [ 'key' => "Rating", 'size' => 100, 'required' => false, 'array' => false, 'default' => null, - ]); + ]); - $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection4['body']['$id'] . '/attributes/string', array_merge([ + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection4['body']['$id'] . '/attributes/string', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ + ]), [ 'key' => "Rating", 'size' => 100, 'required' => false, 'array' => false, 'default' => null, - ]); + ]); - $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection5['body']['$id'] . '/attributes/string', array_merge([ + $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection5['body']['$id'] . '/attributes/string', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ + ]), [ 'key' => "Rating", 'size' => 100, 'required' => false, 'array' => false, 'default' => null, - ]); + ]); \sleep(2); // Creating parent document with a child reference to test the permissions @@ -649,5 +649,87 @@ class DatabasesCustomClientTest extends Scope ]); $this->assertEquals(401, $response['headers']['status-code']); + + // Removing update permission from collection 3. + $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/collection3', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::custom('collection3'), + 'name' => ID::custom('collection3'), + 'documentSecurity' => false, + 'permissions' => [ + Permission::create(Role::user($userId)), + Permission::read(Role::user($userId)), + Permission::delete(Role::user($userId)), + ] + ]); + + // Giving update permission to collection 2. + $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/collection2', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::custom('collection2'), + 'name' => ID::custom('collection2'), + 'documentSecurity' => false, + 'permissions' => [ + Permission::create(Role::user($userId)), + Permission::update(Role::user($userId)), + Permission::read(Role::user($userId)), + Permission::delete(Role::user($userId)), + ] + ]); + + // Creating collection 3 new document + $response = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $collection3['body']['$id'] . '/documents', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'documentId' => ID::custom('collection3Doc1'), + 'data' => [ + 'Rating' => '20' + ] + ]); + + $this->assertEquals(201, $response['headers']['status-code']); + + // We should be allowed to link a new document from collection 3 to collection 2. + $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents/' . $collection1['body']['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'data' => [ + 'Title' => 'Captain America', + $collection2['body']['$id'] => [ + '$id' => ID::custom($collection2['body']['$id']), + $collection3['body']['$id'] => 'collection3Doc1', + ] + ] + ]); + + $this->assertEquals(200, $response['headers']['status-code']); + + + // We should be allowed to link and create a new document from collection 3 to collection 2. + $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents/' . $collection1['body']['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'data' => [ + 'Title' => 'Captain America', + $collection2['body']['$id'] => [ + '$id' => ID::custom($collection2['body']['$id']), + $collection3['body']['$id'] => [ + '$id' => ID::custom('collection3Doc2') + ], + ] + ] + ]); + + $this->assertEquals(200, $response['headers']['status-code']); } } From 860e9958c72a63fd55c28e92631ed690db39798c Mon Sep 17 00:00:00 2001 From: prateek banga Date: Thu, 17 Aug 2023 19:04:41 +0530 Subject: [PATCH 30/30] lint fix --- app/controllers/api/databases.php | 36 +++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index fa4bcc7291..4f9abbbe65 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3252,7 +3252,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $data['$permissions'] = $permissions; $newDocument = new Document($data); - $setCollection = function (Document $collection, Document $document) use (&$setCollection, $dbForProject, $database) { + $setCollection = (function (Document $collection, Document $document) use (&$setCollection, $dbForProject, $database) { $relationships = \array_filter( $collection->getAttribute('attributes', []), fn($attribute) => $attribute->getAttribute('type') === Database::VAR_RELATIONSHIP @@ -3316,26 +3316,26 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $document->setAttribute($relationship->getAttribute('key'), \reset($relations)); } } - }; + }); $setCollection($collection, $newDocument); - try { - $document = $dbForProject->withRequestTimestamp( - $requestTimestamp, - fn() => $dbForProject->updateDocument( - 'database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), - $document->getId(), - $newDocument - ) - ); - } catch (AuthorizationException) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } catch (DuplicateException) { - throw new Exception(Exception::DOCUMENT_ALREADY_EXISTS); - } catch (StructureException $exception) { - throw new Exception(Exception::DOCUMENT_INVALID_STRUCTURE, $exception->getMessage()); - } + try { + $document = $dbForProject->withRequestTimestamp( + $requestTimestamp, + fn() => $dbForProject->updateDocument( + 'database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), + $document->getId(), + $newDocument + ) + ); + } catch (AuthorizationException) { + throw new Exception(Exception::USER_UNAUTHORIZED); + } catch (DuplicateException) { + throw new Exception(Exception::DOCUMENT_ALREADY_EXISTS); + } catch (StructureException $exception) { + throw new Exception(Exception::DOCUMENT_INVALID_STRUCTURE, $exception->getMessage()); + } // Add $collectionId and $databaseId for all documents $processDocument = function (Document $collection, Document $document) use (&$processDocument, $dbForProject, $database) {