From 9edcf420d95f2262fe85309dd9c5bd9b9ca3581f Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 21 Aug 2023 21:11:31 -0400 Subject: [PATCH 1/2] Make sure document permissions are not checked if not enabled --- src/Database/Database.php | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index e5f48e33a..2a553b169 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4062,8 +4062,13 @@ public function find(string $collection, array $queries = [], ?int $timeout = nu } $authorization = new Authorization(self::PERMISSION_READ); + $documentSecurity = $collection->getAttribute('documentSecurity', false); $skipAuth = $authorization->isValid($collection->getRead()); + if (!$skipAuth && !$documentSecurity) { + throw new AuthorizationException($validator->getDescription()); + } + $relationships = \array_filter( $collection->getAttribute('attributes', []), fn (Document $attribute) => $attribute->getAttribute('type') === self::VAR_RELATIONSHIP @@ -4092,7 +4097,6 @@ public function find(string $collection, array $queries = [], ?int $timeout = nu $selections = $this->validateSelections($collection, $selects); $nestedSelections = []; - $nestedQueries = []; foreach ($queries as $index => &$query) { switch ($query->getMethod()) { @@ -4129,7 +4133,6 @@ public function find(string $collection, array $queries = [], ?int $timeout = nu break; default: if (\str_contains($query->getAttribute(), '.')) { - $nestedQueries[] = $query; unset($queries[$index]); } break; @@ -4137,6 +4140,7 @@ public function find(string $collection, array $queries = [], ?int $timeout = nu } $queries = \array_values($queries); + $getResults = fn () => $this->adapter->find( $collection->getId(), $queries, @@ -4151,13 +4155,7 @@ public function find(string $collection, array $queries = [], ?int $timeout = nu $results = $skipAuth ? Authorization::skip($getResults) : $getResults(); - $attributes = $collection->getAttribute('attributes', []); - - $relationships = $this->resolveRelationships - ? \array_filter($attributes, fn (Document $attribute) => $attribute->getAttribute('type') === self::VAR_RELATIONSHIP) - : []; - - foreach ($results as $index => &$node) { + foreach ($results as &$node) { if ($this->resolveRelationships && (empty($selects) || !empty($nestedSelections))) { $node = $this->silent(fn () => $this->populateDocumentRelationships($collection, $node, $nestedSelections)); } From 2615bfd15b4eadca5e5eb975705de04b09b30c9b Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 21 Aug 2023 21:48:57 -0400 Subject: [PATCH 2/2] Fix false positive tests --- src/Database/Database.php | 4 ++-- tests/Database/Base.php | 19 ++++++++----------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 2a553b169..3c4d8d932 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2934,8 +2934,8 @@ public function updateDocument(string $collection, string $id, Document $documen $time = DateTime::now(); $old = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this $document = \array_merge($old->getArrayCopy(), $document->getArrayCopy()); - $document['$collection'] = $old->getAttribute('$collection'); // Make sure user doesn't switch collectionID - $document['$createdAt'] = $old->getCreatedAt(); // Make sure user doesn't switch createdAt + $document['$collection'] = $old->getAttribute('$collection'); // Make sure user doesn't switch collectionID + $document['$createdAt'] = $old->getCreatedAt(); // Make sure user doesn't switch createdAt $document = new Document($document); $collection = $this->silent(fn () => $this->getCollection($collection)); diff --git a/tests/Database/Base.php b/tests/Database/Base.php index 56d3757e5..84288b110 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -11315,18 +11315,17 @@ public function testCollectionPermissionsFindWorks(array $data): array Authorization::cleanRoles(); Authorization::setRole(Role::users()->toString()); - $documents = static::getDatabase()->find( - $collection->getId() - ); + $documents = static::getDatabase()->find($collection->getId()); $this->assertNotEmpty($documents); Authorization::cleanRoles(); Authorization::setRole(Role::user('random')->toString()); - $documents = static::getDatabase()->find( - $collection->getId() - ); - $this->assertNotEmpty($documents); + try { + static::getDatabase()->find($collection->getId()); + $this->fail('Failed to throw exception'); + } catch (AuthorizationException) { + } return $data; } @@ -11342,10 +11341,8 @@ public function testCollectionPermissionsFindThrowsException(array $data): void Authorization::cleanRoles(); Authorization::setRole(Role::any()->toString()); - $documents = static::getDatabase()->find( - $collection->getId() - ); - $this->assertEmpty($documents); + $this->expectException(AuthorizationException::class); + static::getDatabase()->find($collection->getId()); } /**