Merge pull request #669 from code16/download-allowed-disks

Implement config downloads.allowed_disks

Author: dvlpp

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
 

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.
+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'],
+            )
+            // ...
+    }
+}
+```

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;

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'))

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();
+});