From e2ab6a05ab85fb94b35f028874b72bad716a3df2 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 3 Dec 2024 16:40:03 +0900 Subject: [PATCH] Fix permissions and accompanying tests --- app/controllers/api/databases.php | 66 +++++++++++-------- .../Databases/DatabasesCustomClientTest.php | 50 ++++++++++---- 2 files changed, 74 insertions(+), 42 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 101a85a63a..114fd28404 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -2919,38 +2919,46 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') Database::PERMISSION_DELETE, ]; - // Map aggregate permissions to into the set of individual permissions they represent. - $permissions = Permission::aggregate($permissions, $allowedPermissions); - - // Add permissions for current the user if none were provided. - if (\is_null($permissions)) { - $permissions = []; - if (!empty($user->getId())) { - foreach ($allowedPermissions as $permission) { - $permissions[] = (new Permission($permission, 'user', $user->getId()))->toString(); - } + $setPermissions = function (Document $document, ?array $permissions) use ($user, $allowedPermissions, $isAPIKey, $isPrivilegedUser, $isBulk) { + // Map aggregate permissions to into the set of individual permissions they represent. + if ($isBulk) { + $permissions = $document['$permissions'] ?? null; } - } - // Users can only manage their own roles, API keys and Admin users can manage any - if (!$isAPIKey && !$isPrivilegedUser) { - foreach (Database::PERMISSIONS as $type) { - foreach ($permissions as $permission) { - $permission = Permission::parse($permission); - if ($permission->getPermission() != $type) { - continue; - } - $role = (new Role( - $permission->getRole(), - $permission->getIdentifier(), - $permission->getDimension() - ))->toString(); - if (!Authorization::isRole($role)) { - throw new Exception(Exception::USER_UNAUTHORIZED, 'Permissions must be one of: (' . \implode(', ', Authorization::getRoles()) . ')'); + $permissions = Permission::aggregate($permissions, $allowedPermissions); + + // Add permissions for current the user if none were provided. + if (\is_null($permissions)) { + $permissions = []; + if (!empty($user->getId())) { + foreach ($allowedPermissions as $permission) { + $permissions[] = (new Permission($permission, 'user', $user->getId()))->toString(); } } } - } + + // Users can only manage their own roles, API keys and Admin users can manage any + if (!$isAPIKey && !$isPrivilegedUser) { + foreach (Database::PERMISSIONS as $type) { + foreach ($permissions as $permission) { + $permission = Permission::parse($permission); + if ($permission->getPermission() != $type) { + continue; + } + $role = (new Role( + $permission->getRole(), + $permission->getIdentifier(), + $permission->getDimension() + ))->toString(); + if (!Authorization::isRole($role)) { + throw new Exception(Exception::USER_UNAUTHORIZED, 'Permissions must be one of: (' . \implode(', ', Authorization::getRoles()) . ')'); + } + } + } + } + + $document->setAttribute('$permissions', $permissions); + }; $checkPermissions = function (Document $collection, Document $document, string $permission) use (&$checkPermissions, $dbForProject, $database) { $documentSecurity = $collection->getAttribute('documentSecurity', false); @@ -3032,9 +3040,8 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') } }; - $documents = array_map(function ($document) use ($collection, $permissions, $checkPermissions, $isBulk, $documentId) { + $documents = array_map(function ($document) use ($collection, $permissions, $checkPermissions, $isBulk, $documentId, $setPermissions) { $document['$collection'] = $collection->getId(); - $document['$permissions'] = $permissions; if (!$isBulk) { $document['$id'] = $documentId == 'unique()' ? ID::unique() : $documentId; @@ -3048,6 +3055,7 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') $document = new Document($document); + $setPermissions($document, $permissions); $checkPermissions($collection, $document, Database::PERMISSION_CREATE); return $document; diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index 56922b602b..399a63eb2b 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -914,7 +914,7 @@ class DatabasesCustomClientTest extends Scope ]), [ 'collectionId' => ID::unique(), 'name' => 'Bulk Create', - 'documentSecurity' => true, + 'documentSecurity' => false, 'permissions' => [ Permission::read(Role::any()), Permission::delete(Role::any()), @@ -1029,22 +1029,34 @@ class DatabasesCustomClientTest extends Scope [ '$id' => ID::unique(), 'number' => 1, + '$permissions' => [ + Permission::create(Role::user('aaaaaa')), + Permission::read(Role::user('aaaaaa')), + Permission::update(Role::user('aaaaaa')), + Permission::delete(Role::user('aaaaaa')), + ] ], [ '$id' => ID::unique(), 'number' => 2, + '$permissions' => [ + Permission::create(Role::user('aaaaaa')), + Permission::read(Role::user('aaaaaa')), + Permission::update(Role::user('aaaaaa')), + Permission::delete(Role::user('aaaaaa')), + ] ], [ '$id' => ID::unique(), 'number' => 3, + '$permissions' => [ + Permission::create(Role::user('aaaaaa')), + Permission::read(Role::user('aaaaaa')), + Permission::update(Role::user('aaaaaa')), + Permission::delete(Role::user('aaaaaa')), + ] ], ], - 'permissions' => [ - Permission::write(Role::user('aaaaaa')), - Permission::read(Role::user('aaaaaa')), - Permission::update(Role::user('aaaaaa')), - Permission::delete(Role::user('aaaaaa')), - ] ]); $this->assertEquals(401, $response['headers']['status-code']); @@ -1058,22 +1070,34 @@ class DatabasesCustomClientTest extends Scope [ '$id' => ID::unique(), 'number' => 1, + '$permissions' => [ + Permission::create(Role::user($this->getUser()['$id'])), + Permission::read(Role::user($this->getUser()['$id'])), + Permission::update(Role::user($this->getUser()['$id'])), + Permission::delete(Role::user($this->getUser()['$id'])), + ] ], [ '$id' => ID::unique(), 'number' => 2, + '$permissions' => [ + Permission::create(Role::user($this->getUser()['$id'])), + Permission::read(Role::user($this->getUser()['$id'])), + Permission::update(Role::user($this->getUser()['$id'])), + Permission::delete(Role::user($this->getUser()['$id'])), + ] ], [ '$id' => ID::unique(), 'number' => 3, + '$permissions' => [ + Permission::create(Role::user($this->getUser()['$id'])), + Permission::read(Role::user($this->getUser()['$id'])), + Permission::update(Role::user($this->getUser()['$id'])), + Permission::delete(Role::user($this->getUser()['$id'])), + ] ], ], - 'permissions' => [ - Permission::write(Role::user($this->getUser()['$id'])), - Permission::read(Role::user($this->getUser()['$id'])), - Permission::update(Role::user($this->getUser()['$id'])), - Permission::delete(Role::user($this->getUser()['$id'])), - ] ]); $this->assertEquals(201, $response['headers']['status-code']);