Merge pull request #295 from utopia-php/fix-no-authorization-exceptio…

…n-no-change-update-document No Exception when no change in update document function
utopia-php · Jul 26, 2023 · a9f7060 · a9f7060
2 parents 0f7850c + 68206dc
commit a9f7060
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 6 deletions.
diff --git a/src/Database/Database.php b/src/Database/Database.php
@@ -2878,24 +2878,40 @@ public function updateDocument(string $collection, string $id, Document $documen
         }
 
         $time = DateTime::now();
-        $document->setAttribute('$updatedAt', $time);
-
         $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));
 
         $validator = new Authorization(self::PERMISSION_UPDATE);
+        $shouldUpdate = false;
 
         if ($collection->getId() !== self::METADATA) {
             $documentSecurity = $collection->getAttribute('documentSecurity', false);
 
-            if (!$validator->isValid([
+            // 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) {
+                    continue;
+                }
+                if ($oldAttributeValue !== $value) {
+                    $shouldUpdate = true;
+                    break;
+                }
+            }
+            if ($shouldUpdate && !$validator->isValid([
                 ...$collection->getUpdate(),
                 ...($documentSecurity ? $old->getUpdate() : [])
             ])) {
                 throw new AuthorizationException($validator->getDescription());
             }
         }
 
+        if ($shouldUpdate) {
+            $document->setAttribute('$updatedAt', $time);
+        }
+
         // Check if document was updated after the request timestamp
         $oldUpdatedAt = new \DateTime($old->getUpdatedAt());
         if (!is_null($this->timestamp) && $oldUpdatedAt > $this->timestamp) {

diff --git a/tests/Database/Base.php b/tests/Database/Base.php
@@ -207,9 +207,10 @@ public function testIndexValidation(): void
     public function testCreatedAtUpdatedAt(): void
     {
         $this->assertInstanceOf('Utopia\Database\Document', static::getDatabase()->createCollection('created_at'));
-
+        static::getDatabase()->createAttribute('created_at', 'title', Database::VAR_STRING, 100, false);
         $document = static::getDatabase()->createDocument('created_at', new Document([
             '$id' => ID::custom('uid123'),
+
             '$permissions' => [
                 Permission::read(Role::any()),
                 Permission::create(Role::any()),
@@ -2993,6 +2994,26 @@ public function testWritePermissionsUpdateFailure(Document $document): Document
                 Permission::update(Role::any()),
                 Permission::delete(Role::any()),
             ],
+            'string' => 'text📝',
+            'integer' => 6,
+            'bigint' => 8589934592, // 2^33
+            'float' => 5.55,
+            'boolean' => true,
+            'colors' => ['pink', 'green', 'blue'],
+        ]));
+
+        return $document;
+    }
+
+
+    /**
+     * @depends testCreateDocument
+     */
+    public function testNoChangeUpdateDocumentWithoutPermission(Document $document): Document
+    {
+        Authorization::setRole(Role::any()->toString());
+
+        $document = static::getDatabase()->createDocument('documents', new Document([
             'string' => 'text📝',
             'integer' => 5,
             'bigint' => 8589934592, // 2^33
@@ -3001,6 +3022,12 @@ public function testWritePermissionsUpdateFailure(Document $document): Document
             'colors' => ['pink', 'green', 'blue'],
         ]));
 
+        Authorization::cleanRoles();
+        $updatedDocument = static::getDatabase()->updateDocument('documents', $document->getId(), $document);
+
+        // Document should not be updated as there is no change. It should also not throw any authorization exception without any permission because of no change.
+        $this->assertEquals($updatedDocument->getUpdatedAt(), $document->getUpdatedAt());
+
         return $document;
     }
 
@@ -3750,6 +3777,7 @@ public function testCreatedAtUpdatedAtAssert(): void
         $document = static::getDatabase()->getDocument('created_at', 'uid123');
         $this->assertEquals(true, !$document->isEmpty());
         sleep(1);
+        $document->setAttribute('title', 'new title');
         static::getDatabase()->updateDocument('created_at', 'uid123', $document);
         $document = static::getDatabase()->getDocument('created_at', 'uid123');
 
@@ -10764,15 +10792,14 @@ public function testCollectionPermissionsUpdateWorks(array $data): array
     public function testCollectionPermissionsUpdateThrowsException(array $data): void
     {
         [$collection, $document] = $data;
-
         Authorization::cleanRoles();
         Authorization::setRole(Role::any()->toString());
 
         $this->expectException(AuthorizationException::class);
         $document = static::getDatabase()->updateDocument(
             $collection->getId(),
             $document->getId(),
-            $document->setAttribute('test', 'ipsum')
+            $document->setAttribute('test', 'lorem')
         );
     }