From 26ac88c32e3548c6b14add906e75ed512f4a8662 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Mon, 25 Dec 2023 05:54:59 +0000 Subject: [PATCH 1/5] validate create permission while updating chunk uploaded file --- app/controllers/api/storage.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index 1fae48dae0..d390f9ef1c 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -621,8 +621,13 @@ App::post('/v1/storage/buckets/:bucketId/files') ->setAttribute('openSSLIV', $openSSLIV) ->setAttribute('metadata', $metadata) ->setAttribute('chunksUploaded', $chunksUploaded); - - $file = $dbForProject->updateDocument('bucket_' . $bucket->getInternalId(), $fileId, $file); + + // Validate create permission + $validator = new Authorization(Database::PERMISSION_CREATE); + if (!$validator->isValid($bucket->getCreate())) { + throw new Exception(Exception::USER_UNAUTHORIZED); + } + $file = Authorization::skip(fn() => $dbForProject->updateDocument('bucket_' . $bucket->getInternalId(), $fileId, $file)); } } catch (AuthorizationException) { throw new Exception(Exception::USER_UNAUTHORIZED); @@ -659,7 +664,12 @@ App::post('/v1/storage/buckets/:bucketId/files') ->setAttribute('chunksUploaded', $chunksUploaded) ->setAttribute('metadata', $metadata); - $file = $dbForProject->updateDocument('bucket_' . $bucket->getInternalId(), $fileId, $file); + // Validate create permission + $validator = new Authorization(Database::PERMISSION_CREATE); + if (!$validator->isValid($bucket->getCreate())) { + throw new Exception(Exception::USER_UNAUTHORIZED); + } + $file = Authorization::skip(fn() => $dbForProject->updateDocument('bucket_' . $bucket->getInternalId(), $fileId, $file)); } } catch (AuthorizationException) { throw new Exception(Exception::USER_UNAUTHORIZED); From b6b1b396b384c4d4b6cc520586c0468753a7d14e Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Mon, 25 Dec 2023 06:00:43 +0000 Subject: [PATCH 2/5] update chunk upload test --- tests/e2e/Services/Storage/StorageBase.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/e2e/Services/Storage/StorageBase.php b/tests/e2e/Services/Storage/StorageBase.php index 5463740761..be01d95368 100644 --- a/tests/e2e/Services/Storage/StorageBase.php +++ b/tests/e2e/Services/Storage/StorageBase.php @@ -74,10 +74,7 @@ trait StorageBase 'name' => 'Test Bucket 2', 'fileSecurity' => true, 'permissions' => [ - Permission::read(Role::any()), Permission::create(Role::any()), - Permission::update(Role::any()), - Permission::delete(Role::any()), ], ]); $this->assertEquals(201, $bucket2['headers']['status-code']); From a6b4ade39b72bb595bd5c7c03063bf6d475437d8 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Mon, 25 Dec 2023 06:06:18 +0000 Subject: [PATCH 3/5] update large file upload test to not include update permission --- tests/e2e/Services/Storage/StorageBase.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/e2e/Services/Storage/StorageBase.php b/tests/e2e/Services/Storage/StorageBase.php index be01d95368..eb6a52db6b 100644 --- a/tests/e2e/Services/Storage/StorageBase.php +++ b/tests/e2e/Services/Storage/StorageBase.php @@ -107,9 +107,7 @@ trait StorageBase 'fileId' => $fileId, 'file' => $curlFile, 'permissions' => [ - Permission::read(Role::any()), - Permission::update(Role::any()), - Permission::delete(Role::any()), + Permission::read(Role::any()) ], ]); $counter++; From cbd3e85b389400740973a9e8948b457062adcb9e Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Mon, 25 Dec 2023 06:07:28 +0000 Subject: [PATCH 4/5] fix formatting --- app/controllers/api/storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index d390f9ef1c..687971c2d0 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -621,7 +621,7 @@ App::post('/v1/storage/buckets/:bucketId/files') ->setAttribute('openSSLIV', $openSSLIV) ->setAttribute('metadata', $metadata) ->setAttribute('chunksUploaded', $chunksUploaded); - + // Validate create permission $validator = new Authorization(Database::PERMISSION_CREATE); if (!$validator->isValid($bucket->getCreate())) { From 879320e23ec198cd9049856301fbfb5342b9af40 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Tue, 2 Jan 2024 11:53:48 +0545 Subject: [PATCH 5/5] update comment regarding validation --- app/controllers/api/storage.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index e668ec2552..011f83f4b4 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -628,7 +628,12 @@ App::post('/v1/storage/buckets/:bucketId/files') ->setAttribute('metadata', $metadata) ->setAttribute('chunksUploaded', $chunksUploaded); - // Validate create permission + /** + * Validate create permission and skip authorization in updateDocument + * Without this, the file creation will fail when user doesn't have update permission + * However as with chunk upload even if we are updating, we are essentially creating a file + * adding it's new chunk so we validate create permission instead of update + */ $validator = new Authorization(Database::PERMISSION_CREATE); if (!$validator->isValid($bucket->getCreate())) { throw new Exception(Exception::USER_UNAUTHORIZED); @@ -670,7 +675,12 @@ App::post('/v1/storage/buckets/:bucketId/files') ->setAttribute('chunksUploaded', $chunksUploaded) ->setAttribute('metadata', $metadata); - // Validate create permission + /** + * Validate create permission and skip authorization in updateDocument + * Without this, the file creation will fail when user doesn't have update permission + * However as with chunk upload even if we are updating, we are essentially creating a file + * adding it's new chunk so we validate create permission instead of update + */ $validator = new Authorization(Database::PERMISSION_CREATE); if (!$validator->isValid($bucket->getCreate())) { throw new Exception(Exception::USER_UNAUTHORIZED);