From 4244bdedd318c60360dbafd2f195c5933503d999 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Tue, 15 Mar 2022 10:51:51 +0100 Subject: [PATCH] fix: storage bucket permissions --- app/controllers/api/storage.php | 69 ++++++-------- tests/e2e/Services/Storage/StorageBase.php | 10 +-- .../Storage/StorageCustomClientTest.php | 90 +++++++++++++++++++ 3 files changed, 120 insertions(+), 49 deletions(-) diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index b3b175de52..e9ab683aa3 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -715,7 +715,7 @@ App::get('/v1/storage/buckets/:bucketId/files') /** @var Appwrite\Stats\Stats $usage */ /** @var string $mode */ - $bucket = $dbForProject->getDocument('buckets', $bucketId); + $bucket = Authorization::skip(fn () => $dbForProject->getDocument('buckets', $bucketId)); if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN)) { @@ -738,9 +738,7 @@ App::get('/v1/storage/buckets/:bucketId/files') if (!empty($cursor)) { if ($bucket->getAttribute('permission') === 'bucket') { - $cursorFile = Authorization::skip(function () use ($dbForProject, $bucket, $cursor) { - return $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $cursor); - }); + $cursorFile = Authorization::skip(fn () => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $cursor)); } else { $cursorFile = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $cursor); } @@ -757,9 +755,7 @@ App::get('/v1/storage/buckets/:bucketId/files') } if ($bucket->getAttribute('permission') === 'bucket') { - $files = Authorization::skip(function () use ($dbForProject, $bucket, $queries, $limit, $offset, $cursor, $cursorDirection, $orderType) { - return $dbForProject->find('bucket_' . $bucket->getInternalId(), $queries, $limit, $offset, [], [$orderType], $cursorFile ?? null, $cursorDirection); - }); + $files = Authorization::skip(fn () => $dbForProject->find('bucket_' . $bucket->getInternalId(), $queries, $limit, $offset, [], [$orderType], $cursorFile ?? null, $cursorDirection)); } else { $files = $dbForProject->find('bucket_' . $bucket->getInternalId(), $queries, $limit, $offset, [], [$orderType], $cursorFile ?? null, $cursorDirection); } @@ -799,7 +795,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId') /** @var Appwrite\Stats\Stats $usage */ /** @var string $mode */ - $bucket = $dbForProject->getDocument('buckets', $bucketId); + $bucket = Authorization::skip(fn () => $dbForProject->getDocument('buckets', $bucketId)); if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN)) { @@ -815,9 +811,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId') } if ($bucket->getAttribute('permission') === 'bucket') { - $file = Authorization::skip(function () use ($dbForProject, $bucket, $fileId) { - return $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); - }); + $file = Authorization::skip(fn () => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); } else { $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); } @@ -825,10 +819,12 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId') if ($file->isEmpty() || $file->getAttribute('bucketId') !== $bucketId) { throw new Exception('File not found', 404, Exception::STORAGE_FILE_NOT_FOUND); } + $usage ->setParam('storage.files.read', 1) ->setParam('bucketId', $bucketId) ; + $response->dynamic($file, Response::MODEL_FILE); }); @@ -879,7 +875,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/preview') throw new Exception('Imagick extension is missing', 500, Exception::GENERAL_SERVER_ERROR); } - $bucket = $dbForProject->getDocument('buckets', $bucketId); + $bucket = Authorization::skip(fn () => $dbForProject->getDocument('buckets', $bucketId)); if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN)) { @@ -907,9 +903,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/preview') if ($bucket->getAttribute('permission') === 'bucket') { // skip authorization - $file = Authorization::skip(function () use ($dbForProject, $bucket, $fileId) { - return $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); - }); + $file = Authorization::skip(fn () => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); } else { $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); } @@ -1051,7 +1045,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/download') /** @var Utopia\Storage\Device $deviceFiles */ /** @var string $mode */ - $bucket = $dbForProject->getDocument('buckets', $bucketId); + $bucket = Authorization::skip(fn () => $dbForProject->getDocument('buckets', $bucketId)); if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN)) { @@ -1067,9 +1061,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/download') } if ($bucket->getAttribute('permission') === 'bucket') { - $file = Authorization::skip(function () use ($dbForProject, $fileId, $bucket) { - return $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); - }); + $file = Authorization::skip(fn () => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); } else { $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); } @@ -1196,7 +1188,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/view') /** @var Utopia\Storage\Device $deviceFiles */ /** @var string $mode */ - $bucket = $dbForProject->getDocument('buckets', $bucketId); + $bucket = Authorization::skip(fn () => $dbForProject->getDocument('buckets', $bucketId)); if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN)) { @@ -1212,9 +1204,7 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/view') } if ($bucket->getAttribute('permission') === 'bucket') { - $file = Authorization::skip(function () use ($dbForProject, $fileId, $bucket) { - return $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); - }); + $file = Authorization::skip(fn () => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); } else { $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); } @@ -1359,7 +1349,7 @@ App::put('/v1/storage/buckets/:bucketId/files/:fileId') /** @var Appwirte\Event\Event $events */ /** @var string $mode */ - $bucket = $dbForProject->getDocument('buckets', $bucketId); + $bucket = Authorization::skip(fn () => $dbForProject->getDocument('buckets', $bucketId)); $read = (is_null($read) && !$user->isEmpty()) ? ['user:' . $user->getId()] : $read ?? []; // By default set read permissions for user $write = (is_null($write) && !$user->isEmpty()) ? ['user:' . $user->getId()] : $write ?? []; @@ -1393,9 +1383,7 @@ App::put('/v1/storage/buckets/:bucketId/files/:fileId') } if ($bucket->getAttribute('permission') === 'bucket') { - $file = Authorization::skip(function () use ($dbForProject, $fileId, $bucket) { - return $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); - }); + $file = Authorization::skip(fn () => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); } else { $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); } @@ -1404,18 +1392,15 @@ App::put('/v1/storage/buckets/:bucketId/files/:fileId') throw new Exception('File not found', 404, Exception::STORAGE_FILE_NOT_FOUND); } + $file + ->setAttribute('$read', $read) + ->setAttribute('$write', $write) + ; + if ($bucket->getAttribute('permission') === 'bucket') { - $file = Authorization::skip(function () use ($dbForProject, $fileId, $bucket, $file, $read, $write) { - return $dbForProject->updateDocument('bucket_' . $bucket->getInternalId(), $fileId, $file - ->setAttribute('$read', $read) - ->setAttribute('$write', $write) - ); - }); + $file = Authorization::skip(fn () => $dbForProject->updateDocument('bucket_' . $bucket->getInternalId(), $fileId, $file)); } else { - $file = $dbForProject->updateDocument('bucket_' . $bucket->getInternalId(), $fileId, $file - ->setAttribute('$read', $read) - ->setAttribute('$write', $write) - ); + $file = $dbForProject->updateDocument('bucket_' . $bucket->getInternalId(), $fileId, $file); } $events->setParam('bucket', $bucket->getArrayCopy()); @@ -1465,7 +1450,7 @@ App::delete('/v1/storage/buckets/:bucketId/files/:fileId') /** @var Utopia\Storage\Device $deviceFiles */ /** @var string $mode */ - $bucket = $dbForProject->getDocument('buckets', $bucketId); + $bucket = Authorization::skip(fn () => $dbForProject->getDocument('buckets', $bucketId)); if ($bucket->isEmpty() || (!$bucket->getAttribute('enabled') && $mode !== APP_MODE_ADMIN)) { @@ -1481,9 +1466,7 @@ App::delete('/v1/storage/buckets/:bucketId/files/:fileId') } if ($bucket->getAttribute('permission') === 'bucket') { - $file = Authorization::skip(function () use ($dbForProject, $fileId, $bucket) { - return $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); - }); + $file = Authorization::skip(fn () => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); } else { $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); } @@ -1509,9 +1492,7 @@ App::delete('/v1/storage/buckets/:bucketId/files/:fileId') $deviceLocal->delete($cacheDir, true); if ($bucket->getAttribute('permission') === 'bucket') { - $deleted = Authorization::skip(function () use ($dbForProject, $fileId, $bucket) { - return $dbForProject->deleteDocument('bucket_' . $bucket->getInternalId(), $fileId); - }); + $deleted = Authorization::skip(fn () => $dbForProject->deleteDocument('bucket_' . $bucket->getInternalId(), $fileId)); } else { $deleted = $dbForProject->deleteDocument('bucket_' . $bucket->getInternalId(), $fileId); } diff --git a/tests/e2e/Services/Storage/StorageBase.php b/tests/e2e/Services/Storage/StorageBase.php index d55a2eca75..4796270d68 100644 --- a/tests/e2e/Services/Storage/StorageBase.php +++ b/tests/e2e/Services/Storage/StorageBase.php @@ -65,7 +65,7 @@ trait StorageBase ]); $this->assertEquals(201, $bucket2['headers']['status-code']); $this->assertNotEmpty($bucket2['body']['$id']); - + /** * Chunked Upload */ @@ -99,7 +99,7 @@ trait StorageBase $id = $largeFile['body']['$id']; } @fclose($handle); - + $this->assertEquals(201, $largeFile['headers']['status-code']); $this->assertNotEmpty($largeFile['body']['$id']); $this->assertIsInt($largeFile['body']['dateCreated']); @@ -107,7 +107,7 @@ trait StorageBase $this->assertEquals('video/mp4', $largeFile['body']['mimeType']); $this->assertEquals($totalSize, $largeFile['body']['sizeOriginal']); $this->assertEquals(md5_file(realpath(__DIR__ . '/../../../resources/disk-a/large-file.mp4')), $largeFile['body']['signature']); // should validate that the file is not encrypted - + /** * Failure * Test for Chunk above 5MB @@ -134,9 +134,9 @@ trait StorageBase 'write' => ['role:all'], ]); @fclose($handle); - + $this->assertEquals(413, $res['headers']['status-code']); - + /** * Test for FAILURE unknown Bucket diff --git a/tests/e2e/Services/Storage/StorageCustomClientTest.php b/tests/e2e/Services/Storage/StorageCustomClientTest.php index 5de505e658..ddc320fb24 100644 --- a/tests/e2e/Services/Storage/StorageCustomClientTest.php +++ b/tests/e2e/Services/Storage/StorageCustomClientTest.php @@ -17,6 +17,96 @@ class StorageCustomClientTest extends Scope use ProjectCustom; use SideClient; + public function testBucketPermissions(): void + { + /** + * Test for SUCCESS + */ + $bucket = $this->client->call(Client::METHOD_POST, '/storage/buckets', [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ], [ + 'bucketId' => 'unique()', + 'name' => 'Test Bucket', + 'permission' => 'bucket', + 'read' => ['role:all'], + 'write' => ['role:member'], + ]); + + $bucketId = $bucket['body']['$id']; + $this->assertEquals(201, $bucket['headers']['status-code']); + $this->assertNotEmpty($bucketId); + + $file = $this->client->call(Client::METHOD_POST, '/storage/buckets/'. $bucketId . '/files', array_merge([ + 'content-type' => 'multipart/form-data', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'fileId' => 'unique()', + 'file' => new CURLFile(realpath(__DIR__ . '/../../../resources/logo.png'), 'image/png', 'permissions.png'), + ]); + + $fileId = $file['body']['$id']; + $this->assertEquals($file['headers']['status-code'], 201); + $this->assertNotEmpty($fileId); + $this->assertIsInt($file['body']['dateCreated']); + $this->assertEquals('permissions.png', $file['body']['name']); + $this->assertEquals('image/png', $file['body']['mimeType']); + $this->assertEquals(47218, $file['body']['sizeOriginal']); + + $file = $this->client->call(Client::METHOD_GET, '/storage/buckets/' . $bucketId . '/files/' . $fileId, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + + $this->assertEquals(200, $file['headers']['status-code']); + + $file = $this->client->call(Client::METHOD_GET, '/storage/buckets/' . $bucketId . '/files/' . $fileId . '/preview', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + + $this->assertEquals(200, $file['headers']['status-code']); + + $file = $this->client->call(Client::METHOD_GET, '/storage/buckets/' . $bucketId . '/files/' . $fileId . '/download', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + + $this->assertEquals(200, $file['headers']['status-code']); + + $file = $this->client->call(Client::METHOD_GET, '/storage/buckets/' . $bucketId . '/files/' . $fileId . '/view', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + + $this->assertEquals(200, $file['headers']['status-code']); + + /** + * Test for FAILURE + */ + $file = $this->client->call(Client::METHOD_POST, '/storage/buckets/'. $bucketId . '/files', [ + 'content-type' => 'multipart/form-data', + 'x-appwrite-project' => $this->getProject()['$id'], + ], [ + 'fileId' => 'unique()', + 'file' => new CURLFile(realpath(__DIR__ . '/../../../resources/logo.png'), 'image/png', 'permissions.png'), + ]); + + $this->assertEquals($file['headers']['status-code'], 401); + + /** + * Test for SUCCESS + */ + $file = $this->client->call(Client::METHOD_DELETE, '/storage/buckets/' . $bucketId . '/files/' . $fileId, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + + $this->assertEquals(204, $file['headers']['status-code']); + $this->assertEmpty($file['body']); + } + public function testCreateFileDefaultPermissions(): array { /**