Merge pull request #348 from utopia-php/fix-authorization-skip-list-c…

…ollections fix: remove authorization skip from `listCollections`
utopia-php · Oct 26, 2023 · b0c3fd8 · b0c3fd8
2 parents b2d403c + f107449
commit b0c3fd8
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 16 deletions.
diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php
@@ -1312,8 +1312,6 @@ public function sum(string $collection, string $attribute, array $queries = [],
         $where = [];
         $limit = \is_null($max) ? '' : 'LIMIT :max';
 
-        $permissions = (Authorization::$status) ? $this->getSQLPermissionsCondition($collection, $roles) : '1=1'; // Disable join when no authorization required
-
         foreach ($queries as $query) {
             $where[] = $this->getSQLCondition($query);
         }
@@ -1322,11 +1320,15 @@ public function sum(string $collection, string $attribute, array $queries = [],
             $where[] = $this->getSQLPermissionsCondition($name, $roles);
         }
 
+        $sqlWhere = !empty($where)
+            ? 'WHERE ' . \implode(' AND ', $where)
+            : '';
+
         $sql = "
 			SELECT SUM({$attribute}) as sum FROM (
 				SELECT {$attribute}
 				FROM {$this->getSQLTable($name)} table_main
-				WHERE {$permissions} AND " . implode(' AND ', $where) . "
+				{$sqlWhere}
 				{$limit}
 			) table_count
         ";

diff --git a/src/Database/Database.php b/src/Database/Database.php
@@ -876,15 +876,11 @@ public function getCollection(string $id): Document
      */
     public function listCollections(int $limit = 25, int $offset = 0): array
     {
-        Authorization::disable();
-
         $result = $this->silent(fn () => $this->find(self::METADATA, [
             Query::limit($limit),
             Query::offset($offset)
         ]));
 
-        Authorization::reset();
-
         $this->trigger(self::EVENT_COLLECTION_LIST, $result);
 
         return $result;
@@ -4156,8 +4152,8 @@ public function find(string $collection, array $queries = []): array
         $documentSecurity = $collection->getAttribute('documentSecurity', false);
         $skipAuth = $authorization->isValid($collection->getRead());
 
-        if (!$skipAuth && !$documentSecurity) {
-            throw new AuthorizationException($validator->getDescription());
+        if (!$skipAuth && !$documentSecurity && $collection->getId() !== self::METADATA) {
+            throw new AuthorizationException($authorization->getDescription());
         }
 
         $relationships = \array_filter(

diff --git a/tests/Database/Base.php b/tests/Database/Base.php
@@ -129,7 +129,10 @@ public function testIndexValidation(): void
         $this->assertEquals($errorMessage, $validator->getDescription());
 
         try {
-            static::getDatabase()->createCollection($collection->getId(), $attributes, $indexes);
+            static::getDatabase()->createCollection($collection->getId(), $attributes, $indexes, [
+                Permission::read(Role::any()),
+                Permission::create(Role::any()),
+            ]);
             $this->fail('Failed to throw exception');
         } catch (Exception $e) {
             $this->assertEquals($errorMessage, $e->getMessage());
@@ -262,19 +265,29 @@ public function testQueryTimeout(): void
      */
     public function testCreateListExistsDeleteCollection(): void
     {
-        $this->assertInstanceOf('Utopia\Database\Document', static::getDatabase()->createCollection('actors'));
-        $this->assertCount(2, static::getDatabase()->listCollections());
+        $this->assertInstanceOf('Utopia\Database\Document', static::getDatabase()->createCollection('actors', permissions: [
+            Permission::create(Role::any()),
+            Permission::read(Role::any()),
+        ]));
+        $this->assertCount(1, static::getDatabase()->listCollections());
         $this->assertEquals(true, static::getDatabase()->exists($this->testDatabase, 'actors'));
 
         // Collection names should not be unique
-        $this->assertInstanceOf('Utopia\Database\Document', static::getDatabase()->createCollection('actors2'));
-        $this->assertCount(3, static::getDatabase()->listCollections());
+        $this->assertInstanceOf('Utopia\Database\Document', static::getDatabase()->createCollection('actors2', permissions: [
+            Permission::create(Role::any()),
+            Permission::read(Role::any()),
+        ]));
+        $this->assertCount(2, static::getDatabase()->listCollections());
         $this->assertEquals(true, static::getDatabase()->exists($this->testDatabase, 'actors2'));
         $collection = static::getDatabase()->getCollection('actors2');
         $collection->setAttribute('name', 'actors'); // change name to one that exists
-        $this->assertInstanceOf('Utopia\Database\Document', static::getDatabase()->updateDocument($collection->getCollection(), $collection->getId(), $collection));
+        $this->assertInstanceOf('Utopia\Database\Document', static::getDatabase()->updateDocument(
+            $collection->getCollection(),
+            $collection->getId(),
+            $collection
+        ));
         $this->assertEquals(true, static::getDatabase()->deleteCollection('actors2')); // Delete collection when finished
-        $this->assertCount(2, static::getDatabase()->listCollections());
+        $this->assertCount(1, static::getDatabase()->listCollections());
 
         $this->assertEquals(false, static::getDatabase()->getCollection('actors')->isEmpty());
         $this->assertEquals(true, static::getDatabase()->deleteCollection('actors'));