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' + ] + ] + ] + ] ] ]);