From 9eaf5af4b1b3de81ebb5987dcda68811d22aebf0 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Wed, 25 Oct 2023 19:00:47 +0200 Subject: [PATCH 1/3] fix: remove authorization skip from `listCollections` --- src/Database/Database.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 1dd6d9265..79d77c2ce 100644 --- 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; From e6dacf0e75bbe54fc9c253bd19bfc1e2d3947c02 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 26 Oct 2023 18:51:50 +1300 Subject: [PATCH 2/3] Don't throw auth exception for find queries against the metadata table --- src/Database/Database.php | 4 ++-- tests/Database/Base.php | 27 ++++++++++++++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 79d77c2ce..78fb9cfbd 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4152,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 index 815f42e78..b8357c31e 100644 --- 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')); From f107449160111acd2d35b5ef5612f6c18ed0b465 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 26 Oct 2023 18:52:41 +1300 Subject: [PATCH 3/3] Fix postgres sum query --- src/Database/Adapter/Postgres.php | 8 +++++--- tests/Database/Base.php | 26 +++++++++++++------------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index b2e27bda5..3e19f975b 100644 --- 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/tests/Database/Base.php b/tests/Database/Base.php index b8357c31e..438d5bd53 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -130,9 +130,9 @@ public function testIndexValidation(): void try { static::getDatabase()->createCollection($collection->getId(), $attributes, $indexes, [ - Permission::read(Role::any()), - Permission::create(Role::any()), - ]); + Permission::read(Role::any()), + Permission::create(Role::any()), + ]); $this->fail('Failed to throw exception'); } catch (Exception $e) { $this->assertEquals($errorMessage, $e->getMessage()); @@ -266,26 +266,26 @@ public function testQueryTimeout(): void public function testCreateListExistsDeleteCollection(): void { $this->assertInstanceOf('Utopia\Database\Document', static::getDatabase()->createCollection('actors', permissions: [ - Permission::create(Role::any()), - Permission::read(Role::any()), - ])); + 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', permissions: [ - Permission::create(Role::any()), - Permission::read(Role::any()), - ])); + 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 - )); + $collection->getCollection(), + $collection->getId(), + $collection + )); $this->assertEquals(true, static::getDatabase()->deleteCollection('actors2')); // Delete collection when finished $this->assertCount(1, static::getDatabase()->listCollections());