Merge pull request #429 from utopia-php/fix-update-permissions

Fix disallowed read on update
utopia-php · Jun 20, 2024 · 415588c · 415588c
2 parents fff42e0 + 02a4e3a
commit 415588c
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 9 deletions.
diff --git a/src/Database/Database.php b/src/Database/Database.php
@@ -3531,7 +3531,8 @@ public function updateDocument(string $collection, string $id, Document $documen
  return $attribute['type'] === Database::VAR_RELATIONSHIP;
  });
 
- $validator = new Authorization(self::PERMISSION_UPDATE);
+ $updateValidator = new Authorization(self::PERMISSION_UPDATE);
+ $readValidator = new Authorization(self::PERMISSION_READ);
  $shouldUpdate = false;
 
  if ($collection->getId() !== self::METADATA) {
@@ -3625,11 +3626,20 @@ public function updateDocument(string $collection, string $id, Document $documen
  }
  }
 
- if ($shouldUpdate && !$validator->isValid([
+ $updatePermissions = [
  ...$collection->getUpdate(),
  ...($documentSecurity ? $old->getUpdate() : [])
- ])) {
- throw new AuthorizationException($validator->getDescription());
+ ];
+
+ $readPermissions = [
+ ...$collection->getRead(),
+ ...($documentSecurity ? $old->getRead() : [])
+ ];
+
+ if ($shouldUpdate && !$updateValidator->isValid($updatePermissions)) {
+ throw new AuthorizationException($updateValidator->getDescription());
+ } elseif (!$shouldUpdate && !$readValidator->isValid($readPermissions)) {
+ throw new AuthorizationException($readValidator->getDescription());
  }
  }
 
@@ -3650,10 +3660,10 @@ public function updateDocument(string $collection, string $id, Document $documen
 
  $document = $this->encode($collection, $document);
 
- $validator = new Structure($collection);
+ $structureValidator = new Structure($collection);
 
- if (!$validator->isValid($document)) { // Make sure updated structure still apply collection rules (if any)
- throw new StructureException($validator->getDescription());
+ if (!$structureValidator->isValid($document)) { // Make sure updated structure still apply collection rules (if any)
+ throw new StructureException($structureValidator->getDescription());
  }
 
  if ($this->resolveRelationships) {

diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php
@@ -1045,7 +1045,7 @@ public function testSizeCollection(): void
  $byteDifference = 5000;
  $this->assertLessThan($byteDifference, $sizeDifference);
 
- static::getDatabase()->createAttribute('sizeTest2', 'string1', Database::VAR_STRING, 128, true);
+ static::getDatabase()->createAttribute('sizeTest2', 'string1', Database::VAR_STRING, 20000, true);
  static::getDatabase()->createAttribute('sizeTest2', 'string2', Database::VAR_STRING, 254 + 1, true);
  static::getDatabase()->createAttribute('sizeTest2', 'string3', Database::VAR_STRING, 254 + 1, true);
  static::getDatabase()->createIndex('sizeTest2', 'index', Database::INDEX_KEY, ['string1', 'string2', 'string3'], [128, 128, 128]);
@@ -4806,7 +4806,9 @@ public function testNoChangeUpdateDocumentWithoutPermission(): Document
  {
  $document = static::getDatabase()->createDocument('documents', new Document([
  '$id' => ID::unique(),
- '$permissions' => [],
+ '$permissions' => [
+ Permission::read(Role::any())
+ ],
  'string' => 'text📝',
  'integer_signed' => -Database::INT_MAX,
  'integer_unsigned' => Database::INT_MAX,
@@ -4828,6 +4830,31 @@ public function testNoChangeUpdateDocumentWithoutPermission(): Document
  // It should also not throw any authorization exception without any permission because of no change.
  $this->assertEquals($updatedDocument->getUpdatedAt(), $document->getUpdatedAt());
 
+ $document = static::getDatabase()->createDocument('documents', new Document([
+ '$id' => ID::unique(),
+ '$permissions' => [],
+ 'string' => 'text📝',
+ 'integer_signed' => -Database::INT_MAX,
+ 'integer_unsigned' => Database::INT_MAX,
+ 'bigint_signed' => -Database::BIG_INT_MAX,
+ 'bigint_unsigned' => Database::BIG_INT_MAX,
+ 'float_signed' => -123456789.12346,
+ 'float_unsigned' => 123456789.12346,
+ 'boolean' => true,
+ 'colors' => ['pink', 'green', 'blue'],
+ ]));
+
+ // Should throw exception, because nothing was updated, but there was no read permission
+ try {
+ static::getDatabase()->updateDocument(
+ 'documents',
+ $document->getId(),
+ $document
+ );
+ } catch (Exception $e) {
+ $this->assertInstanceOf(AuthorizationException::class, $e);
+ }
+
  return $document;
  }