diff --git a/app/controllers/api/database.php b/app/controllers/api/database.php index 9d5ac716b..ec30f1d2b 100644 --- a/app/controllers/api/database.php +++ b/app/controllers/api/database.php @@ -2009,14 +2009,18 @@ App::patch('/v1/database/collections/:collectionId/documents/:documentId') $roles = Authorization::getRoles(); if (!Auth::isAppUser($roles) && !Auth::isPrivilegedUser($roles)) { - foreach ($data['$read'] as $read) { - if (!Authorization::isRole($read)) { - throw new Exception('Read permissions must be one of: ('.\implode(', ', $roles).')', 400); + if(!is_null($read)) { + foreach ($data['$read'] as $read) { + if (!Authorization::isRole($read)) { + throw new Exception('Read permissions must be one of: ('.\implode(', ', $roles).')', 400); + } } } - foreach ($data['$write'] as $write) { - if (!Authorization::isRole($write)) { - throw new Exception('Write permissions must be one of: ('.\implode(', ', $roles).')', 400); + if(!is_null($write)) { + foreach ($data['$write'] as $write) { + if (!Authorization::isRole($write)) { + throw new Exception('Write permissions must be one of: (' . \implode(', ', $roles) . ')', 400); + } } } } diff --git a/tests/e2e/Services/Database/DatabaseCustomClientTest.php b/tests/e2e/Services/Database/DatabaseCustomClientTest.php index 5bb035ac1..9d5aae758 100644 --- a/tests/e2e/Services/Database/DatabaseCustomClientTest.php +++ b/tests/e2e/Services/Database/DatabaseCustomClientTest.php @@ -2,13 +2,118 @@ namespace Tests\E2E\Services\Database; +use Tests\E2E\Client; use Tests\E2E\Scopes\Scope; use Tests\E2E\Scopes\ProjectCustom; use Tests\E2E\Scopes\SideClient; +use function array_merge; +use function sleep; class DatabaseCustomClientTest extends Scope { use DatabaseBase; use ProjectCustom; use SideClient; + + public function testUpdateWithoutPermission(): array + { + // If document has been created by server and client tried to update it without adjusting permissions, permission validation should be skipped + + // As a part of preparation, we get ID of currently logged-in user + $response = $this->client->call(Client::METHOD_GET, '/account', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + $this->assertEquals(200, $response['headers']['status-code']); + + $userId = $response['body']['$id']; + + // Create collection + $response = $this->client->call(Client::METHOD_POST, '/database/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => 'permissionCheck', + 'name' => 'permissionCheck', + 'read' => [], + 'write' => [], + 'permission' => 'document' + ]); + $this->assertEquals(201, $response['headers']['status-code']); + + // Add attribute to collection + $response = $this->client->call(Client::METHOD_POST, '/database/collections/permissionCheck/attributes/string', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'key' => 'name', + 'size' => 255, + 'required' => true, + ]); + $this->assertEquals(201, $response['headers']['status-code']); + + // Wait for database worker to finish creating attributes + sleep(2); + + // Creating document by server, give read permission to our user + some other user + $response = $this->client->call(Client::METHOD_POST, '/database/collections/permissionCheck/documents', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'documentId' => 'permissionCheckDocument', + 'data' => [ + 'name' => 'AppwriteBeginner', + ], + 'read' => ['user:' . $userId, 'user:user2'], + 'write' => ['user:' . $userId], + ]); + $this->assertEquals(201, $response['headers']['status-code']); + + // Update document + // This is the point of this test. We should be allowed to do this action, and it should not fail on permission check + $response = $this->client->call(Client::METHOD_PATCH, '/database/collections/permissionCheck/documents/permissionCheckDocument', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'data' => [ + 'name' => 'AppwriteExpert', + ] + ]); + $this->assertEquals(200, $response['headers']['status-code']); + + // Get name of the document, should be the new one + $response = $this->client->call(Client::METHOD_GET, '/database/collections/permissionCheck/documents/permissionCheckDocument', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEquals("AppwriteExpert", $response['body']['name']); + + // Cleanup to prevent collision with other tests + // Delete collection + $response = $this->client->call(Client::METHOD_DELETE, '/database/collections/permissionCheck', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ])); + + $this->assertEquals(204, $response['headers']['status-code']); + + + // Wait for database worker to finish deleting collection + sleep(2); + + // Make sure collection has been deleted + $response = $this->client->call(Client::METHOD_GET, '/database/collections/permissionCheck', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ])); + $this->assertEquals(404, $response['headers']['status-code']); + + return []; + } } \ No newline at end of file