From 611dd9b86c13c87f6c5f73f990d7c09830dcbeef Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Tue, 18 Apr 2023 17:34:39 -0700 Subject: [PATCH] Return 404 if a user doesn't have access to a collection It is better to return 404 so that an end user doesn't know if the collection actually exists but they don't have access or they really don't have access. --- app/controllers/api/databases.php | 13 ++++++++++--- tests/e2e/Services/Databases/DatabasesBase.php | 4 +--- .../Databases/DatabasesPermissionsTeamTest.php | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 4af75839e0..ba3f0fc378 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -2886,12 +2886,19 @@ App::get('/v1/databases/:databaseId/collections/:collectionId/documents') $collection = Authorization::skip(fn() => $dbForProject->getDocument('database_' . $database->getInternalId(), $collectionId)); - if ($collection->isEmpty() || !$collection->getAttribute('enabled')) { - if (!($mode === APP_MODE_ADMIN && Auth::isPrivilegedUser(Authorization::getRoles()))) { - throw new Exception(Exception::COLLECTION_NOT_FOUND); + if (!($mode === APP_MODE_ADMIN && Auth::isPrivilegedUser(Authorization::getRoles()))) { + if (!$collection->getAttribute('documentSecurity', false)) { + $validator = new Authorization(Database::PERMISSION_READ); + if (!$validator->isValid($collection->getRead())) { + $collection = new Document(); + } } } + if ($collection->isEmpty() || !$collection->getAttribute('enabled')) { + throw new Exception(Exception::COLLECTION_NOT_FOUND); + } + // Validate queries $queriesValidator = new Documents($collection->getAttribute('attributes'), $collection->getAttribute('indexes')); $validQueries = $queriesValidator->isValid($queries); diff --git a/tests/e2e/Services/Databases/DatabasesBase.php b/tests/e2e/Services/Databases/DatabasesBase.php index a2cb48cf93..abff3437b8 100644 --- a/tests/e2e/Services/Databases/DatabasesBase.php +++ b/tests/e2e/Services/Databases/DatabasesBase.php @@ -2853,9 +2853,7 @@ trait DatabasesBase ]); // Current user has no collection permissions and document permissions are disabled - $this->assertEquals(200, $documentsUser2['headers']['status-code']); - $this->assertEquals(0, $documentsUser2['body']['total']); - $this->assertEquals(true, empty($documentsUser2['body']['documents'])); + $this->assertEquals(404, $documentsUser2['headers']['status-code']); // Enable document permissions $collection = $this->client->call(CLient::METHOD_PUT, '/databases/' . $databaseId . '/collections/' . $collectionId, [ diff --git a/tests/e2e/Services/Databases/DatabasesPermissionsTeamTest.php b/tests/e2e/Services/Databases/DatabasesPermissionsTeamTest.php index 1bb42d8cb6..dcbf3e4bff 100644 --- a/tests/e2e/Services/Databases/DatabasesPermissionsTeamTest.php +++ b/tests/e2e/Services/Databases/DatabasesPermissionsTeamTest.php @@ -176,7 +176,7 @@ class DatabasesPermissionsTeamTest extends Scope if ($success) { $this->assertCount(1, $documents['body']['documents']); } else { - $this->assertCount(0, $documents['body']['documents']); + $this->assertEquals(404, $documents['headers']['status-code']); } }