Merge pull request #301 from utopia-php/fix-5404-permission-check-upd…

…ate-document
utopia-php · Aug 3, 2023 · f4ccbc2 · f4ccbc2
2 parents 5ae3035 + 0d9a648
commit f4ccbc2
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 3 deletions.
diff --git a/src/Database/Database.php b/src/Database/Database.php
@@ -2881,20 +2881,30 @@ public function updateDocument(string $collection, string $id, Document $documen
         $old = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this
 
         $collection = $this->silent(fn () => $this->getCollection($collection));
+        $relationships = \array_filter($collection->getAttribute('attributes', []), function ($attribute) {
+            return $attribute['type'] === Database::VAR_RELATIONSHIP;
+        });
 
         $validator = new Authorization(self::PERMISSION_UPDATE);
         $shouldUpdate = false;
 
         if ($collection->getId() !== self::METADATA) {
             $documentSecurity = $collection->getAttribute('documentSecurity', false);
 
+            $relationshipKeys = [];
+            foreach ($relationships as $relationship) {
+                $relationshipKey = $relationship->getAttribute('key');
+                $relationshipKeys[$relationshipKey] = $relationshipKey;
+            }
             // Compare if the document has any changes
             foreach ($document as $key=>$value) {
                 // Skip the nested documents as they will be checked later in recursions.
-                $oldAttributeValue = $old->getAttribute($key);
-                if ($oldAttributeValue instanceof Document) {
+                if (array_key_exists($key, $relationshipKeys)) {
                     continue;
                 }
+
+                $oldAttributeValue = $old->getAttribute($key);
+                // If values are not equal we need to update document.
                 if ($oldAttributeValue !== $value) {
                     $shouldUpdate = true;
                     break;
@@ -2910,6 +2920,8 @@ public function updateDocument(string $collection, string $id, Document $documen
 
         if ($shouldUpdate) {
             $document->setAttribute('$updatedAt', $time);
+        } else {
+            $document->setAttribute('$updatedAt', $old->getUpdatedAt());
         }
 
         // Check if document was updated after the request timestamp

diff --git a/tests/Database/Base.php b/tests/Database/Base.php
@@ -3064,6 +3064,65 @@ public function testNoChangeUpdateDocumentWithoutPermission(Document $document):
         return $document;
     }
 
+    public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void
+    {
+        if (!static::getDatabase()->getAdapter()->getSupportForRelationships()) {
+            $this->expectNotToPerformAssertions();
+            return;
+        }
+
+        Authorization::cleanRoles();
+        Authorization::setRole(Role::user('a')->toString());
+
+        static::getDatabase()->createCollection('parentRelationTest', [], [], [
+            Permission::read(Role::user('a')),
+            Permission::create(Role::user('a')),
+        ]);
+        static::getDatabase()->createCollection('childRelationTest', [], [], [
+            Permission::read(Role::user('a')),
+            Permission::create(Role::user('a')),
+        ]);
+        static::getDatabase()->createAttribute('parentRelationTest', 'name', Database::VAR_STRING, 255, false);
+        static::getDatabase()->createAttribute('childRelationTest', 'name', Database::VAR_STRING, 255, false);
+
+        static::getDatabase()->createRelationship(
+            collection: 'parentRelationTest',
+            relatedCollection: 'childRelationTest',
+            type: Database::RELATION_ONE_TO_MANY,
+            id: 'childs'
+        );
+
+        // Create document with relationship with nested data
+        $parent = static::getDatabase()->createDocument('parentRelationTest', new Document([
+            '$id' => 'parent1',
+            'name' => 'Parent 1',
+            'childs' => [
+                [
+                    '$id' => 'child1',
+                    'name' => 'Child 1',
+                ],
+            ],
+        ]));
+        $this->assertEquals(1, \count($parent['childs']));
+        $updatedParent = static::getDatabase()->updateDocument('parentRelationTest', 'parent1', new Document([
+            '$id' => 'parent1',
+            'name'=>'Parent 1',
+            '$collection' => 'parentRelationTest',
+            'childs' => [
+                new Document([
+                    '$id' => 'child1',
+                    '$collection' => 'childRelationTest'
+                ]),
+            ]
+        ]));
+
+        $this->assertEquals($updatedParent->getUpdatedAt(), $parent->getUpdatedAt());
+        $this->assertEquals($updatedParent->getAttribute('childs')[0]->getUpdatedAt(), $parent->getAttribute('childs')[0]->getUpdatedAt());
+
+        static::getDatabase()->deleteCollection('parentRelationTest');
+        static::getDatabase()->deleteCollection('childRelationTest');
+    }
+
     public function testExceptionAttributeLimit(): void
     {
         if ($this->getDatabase()->getLimitForAttributes() > 0) {
@@ -11216,7 +11275,7 @@ public function testCollectionPermissionsRelationshipsUpdateThrowsException(arra
         $document = static::getDatabase()->updateDocument(
             $collection->getId(),
             $document->getId(),
-            $document->setAttribute('test', 'ipsum')
+            $document->setAttribute('test', $document->getAttribute('test').'new_value')
         );
     }