diff --git a/docs/guide/form-fields/upload.md b/docs/guide/form-fields/upload.md index f61927a7a..04b450096 100644 --- a/docs/guide/form-fields/upload.md +++ b/docs/guide/form-fields/upload.md @@ -23,7 +23,23 @@ class SharpServiceProvider extends SharpAppServiceProvider } ``` -The fourth argument, `keepOriginalImageOnTransform`, is a boolean that defines if the original image should be kept when a transformation is applied on it (meaning that transformations are stored and applied on-the-fly: this is transparent when using Sharp’s [built-in way to handle uploads](../sharp-uploads.md). It can be overriden by each field (see below). +The fourth argument, `keepOriginalImageOnTransform`, is a boolean that defines if the original image should be kept when a transformation is applied on it (meaning that transformations are stored and applied on-the-fly: this is transparent when using Sharp’s [built-in way to handle uploads](../sharp-uploads.md). It can be overridden by each field (see below). + +Sharp allows admins to download all uploaded files directly from the Upload field UI. However, this capability may introduce security concerns, since Sharp can access any file on the server (although this is largely mitigated by Flysystem, which is used under the hood). You can control this behavior by specifying a list of allowed disks in the configuration: + +```php +class SharpServiceProvider extends SharpAppServiceProvider +{ + protected function configureSharp(SharpConfigBuilder $config): void + { + $config + ->configureDownloads( + allowedDisks: ['local', 'public'], + ) + // ... + } +} +``` ## Field Configuration diff --git a/docs/guide/show-fields/file.md b/docs/guide/show-fields/file.md index b4037f2c6..7af079530 100644 --- a/docs/guide/show-fields/file.md +++ b/docs/guide/show-fields/file.md @@ -25,10 +25,28 @@ Sharp expects an array formatted like this: ] ``` -If you are using Sharp solution for uploads, meaning the `SharpUploadModel` class [detailed here](../sharp-uploads.md), you can simply call the built-in transformer: +If you are using Sharp’s solution for uploads, meaning the `SharpUploadModel` class [detailed here](../sharp-uploads.md), you can call the built-in transformer: ```php $this->setCustomTransformer('file', new SharpUploadModelFormAttributeTransformer()); ``` -This transformer allows to act a bit on the thumbnail creation part, see its constructor for more details. \ No newline at end of file +This transformer allows acting a bit on the thumbnail creation part, see its constructor for more details. + +## A note on security + +Sharp allows admins to download all uploaded files directly from the File field UI. However, this capability may introduce security concerns, since Sharp can access any file on the server (although this is largely mitigated by Flysystem, which is used under the hood). You can control this behavior by specifying a list of allowed disks in the configuration: + +```php +class SharpServiceProvider extends SharpAppServiceProvider +{ + protected function configureSharp(SharpConfigBuilder $config): void + { + $config + ->configureDownloads( + allowedDisks: ['local', 'public'], + ) + // ... + } +} +``` diff --git a/src/Config/SharpConfigBuilder.php b/src/Config/SharpConfigBuilder.php index ce8fcf3df..cfeb8b216 100644 --- a/src/Config/SharpConfigBuilder.php +++ b/src/Config/SharpConfigBuilder.php @@ -93,6 +93,9 @@ class SharpConfigBuilder 'file_handling_queue' => 'default', 'file_handling_queue_connection' => 'sync', ], + 'downloads' => [ + 'allowed_disks' => '*', + ], 'search' => [ 'enabled' => false, ], @@ -320,6 +323,13 @@ public function configureUploadsThumbnailCreation( return $this; } + public function configureDownloads(array $allowedDisks): self + { + $this->config['downloads']['allowed_disks'] = $allowedDisks; + + return $this; + } + public function setThemeColor(string $hexColor): self { $this->config['theme']['primary_color'] = $hexColor; diff --git a/src/Http/Controllers/Api/DownloadController.php b/src/Http/Controllers/Api/DownloadController.php index 112b8592a..487f7ac3a 100644 --- a/src/Http/Controllers/Api/DownloadController.php +++ b/src/Http/Controllers/Api/DownloadController.php @@ -10,6 +10,13 @@ public function show(string $entityKey, ?string $instanceId = null) { $this->authorizationManager->check('view', $entityKey, $instanceId); + if ( + ($allowedDisks = sharp()->config()->get('downloads.allowed_disks')) !== null // Legacy config + && $allowedDisks != '*' + ) { + abort_if(! in_array(request()->get('disk'), $allowedDisks), 403); + } + abort_if( ! ($path = request()->get('path')) || ! ($disk = request()->get('disk')) diff --git a/tests/Http/Api/DownloadControllerTest.php b/tests/Http/Api/DownloadControllerTest.php index 910ae6841..0f0fe1167 100644 --- a/tests/Http/Api/DownloadControllerTest.php +++ b/tests/Http/Api/DownloadControllerTest.php @@ -77,3 +77,39 @@ ) ->assertForbidden(); }); + +it('allows to download a file of an allowed disk', function () { + sharp()->config()->configureDownloads(['local']); + + $file = UploadedFile::fake()->image('test.jpg', 600, 600); + $file->storeAs('/files', 'test.jpg', ['disk' => 'local']); + + $this + ->get( + route('code16.sharp.download.show', [ + 'entityKey' => 'person', + 'instanceId' => 1, + 'disk' => 'local', + 'path' => '/files/test.jpg', + ]), + ) + ->assertOk(); +}); + +it('does not allow to download a file of a not allowed disk', function () { + sharp()->config()->configureDownloads(['s3']); + + $file = UploadedFile::fake()->image('test.jpg', 600, 600); + $file->storeAs('/files', 'test.jpg', ['disk' => 'local']); + + $this + ->get( + route('code16.sharp.download.show', [ + 'entityKey' => 'person', + 'instanceId' => 1, + 'disk' => 'local', + 'path' => '/files/test.jpg', + ]), + ) + ->assertForbidden(); +});