From 23f90ed6b4600b7ea791b94ae4b7c34fe72fc727 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 25 Mar 2018 12:41:52 +0100
Subject: [PATCH] Ensured uploaded system images remain public

Also added tests to cover local_secure image storage.

Fixes #725
---
 app/Repos/ImageRepo.php            |  1 -
 app/Services/AttachmentService.php |  8 +------
 app/Services/ImageService.php      | 24 ++++++++++++++++---
 app/Services/UploadService.php     | 22 +-----------------
 tests/ImageTest.php                | 37 +++++++++++++++++++++++++++++-
 5 files changed, 59 insertions(+), 33 deletions(-)

diff --git a/app/Repos/ImageRepo.php b/app/Repos/ImageRepo.php
index ec2fda1d3..245c0f27b 100644
--- a/app/Repos/ImageRepo.php
+++ b/app/Repos/ImageRepo.php
@@ -225,7 +225,6 @@ class ImageRepo
         try {
             return $this->imageService->getThumbnail($image, $width, $height, $keepRatio);
         } catch (\Exception $exception) {
-            dd($exception);
             return null;
         }
     }
diff --git a/app/Services/AttachmentService.php b/app/Services/AttachmentService.php
index 381e44749..3a6f95dcc 100644
--- a/app/Services/AttachmentService.php
+++ b/app/Services/AttachmentService.php
@@ -14,10 +14,6 @@ class AttachmentService extends UploadService
      */
     protected function getStorage()
     {
-        if ($this->storageInstance !== null) {
-            return $this->storageInstance;
-        }
-
         $storageType = config('filesystems.default');
 
         // Override default location if set to local public to ensure not visible.
@@ -25,9 +21,7 @@ class AttachmentService extends UploadService
             $storageType = 'local_secure';
         }
 
-        $this->storageInstance = $this->fileSystem->disk($storageType);
-
-        return $this->storageInstance;
+        return $this->fileSystem->disk($storageType);
     }
 
     /**
diff --git a/app/Services/ImageService.php b/app/Services/ImageService.php
index 6686082ee..ebbcd9351 100644
--- a/app/Services/ImageService.php
+++ b/app/Services/ImageService.php
@@ -31,6 +31,23 @@ class ImageService extends UploadService
         parent::__construct($fileSystem);
     }
 
+    /**
+     * Get the storage that will be used for storing images.
+     * @param string $type
+     * @return \Illuminate\Contracts\Filesystem\Filesystem
+     */
+    protected function getStorage($type = '')
+    {
+        $storageType = config('filesystems.default');
+
+        // Override default location if set to local public to ensure not visible.
+        if ($type === 'system' && $storageType === 'local_secure') {
+            $storageType = 'local';
+        }
+
+        return $this->fileSystem->disk($storageType);
+    }
+
     /**
      * Saves a new image from an upload.
      * @param UploadedFile $uploadedFile
@@ -119,7 +136,7 @@ class ImageService extends UploadService
      */
     private function saveNew($imageName, $imageData, $type, $uploadedTo = 0)
     {
-        $storage = $this->getStorage();
+        $storage = $this->getStorage($type);
         $secureUploads = setting('app-secure-images');
         $imageName = str_replace(' ', '-', $imageName);
 
@@ -205,7 +222,7 @@ class ImageService extends UploadService
             return $this->getPublicUrl($thumbFilePath);
         }
 
-        $storage = $this->getStorage();
+        $storage = $this->getStorage($image->type);
         if ($storage->exists($thumbFilePath)) {
             return $this->getPublicUrl($thumbFilePath);
         }
@@ -287,8 +304,9 @@ class ImageService extends UploadService
     /**
      * Save a gravatar image and set a the profile image for a user.
      * @param User $user
-     * @param int  $size
+     * @param int $size
      * @return mixed
+     * @throws Exception
      */
     public function saveUserGravatar(User $user, $size = 500)
     {
diff --git a/app/Services/UploadService.php b/app/Services/UploadService.php
index cd0567559..df597a40f 100644
--- a/app/Services/UploadService.php
+++ b/app/Services/UploadService.php
@@ -11,11 +11,6 @@ class UploadService
      */
     protected $fileSystem;
 
-    /**
-     * @var FileSystemInstance
-     */
-    protected $storageInstance;
-
 
     /**
      * FileService constructor.
@@ -32,14 +27,8 @@ class UploadService
      */
     protected function getStorage()
     {
-        if ($this->storageInstance !== null) {
-            return $this->storageInstance;
-        }
-
         $storageType = config('filesystems.default');
-        $this->storageInstance = $this->fileSystem->disk($storageType);
-
-        return $this->storageInstance;
+        return $this->fileSystem->disk($storageType);
     }
 
     /**
@@ -53,13 +42,4 @@ class UploadService
         $folders = $this->getStorage()->directories($path);
         return (count($files) === 0 && count($folders) === 0);
     }
-
-    /**
-     * Check if using a local filesystem.
-     * @return bool
-     */
-    protected function isLocal()
-    {
-        return strtolower(config('filesystems.default')) === 'local';
-    }
 }
diff --git a/tests/ImageTest.php b/tests/ImageTest.php
index 881f73b55..8c96ae925 100644
--- a/tests/ImageTest.php
+++ b/tests/ImageTest.php
@@ -21,7 +21,7 @@ class ImageTest extends TestCase
      */
     protected function getTestImage($fileName)
     {
-        return new \Illuminate\Http\UploadedFile($this->getTestImageFilePath(), $fileName, 'image/jpeg', 5238);
+        return new \Illuminate\Http\UploadedFile($this->getTestImageFilePath(), $fileName, 'image/png', 5238);
     }
 
     /**
@@ -86,7 +86,42 @@ class ImageTest extends TestCase
             'updated_by' => $admin->id,
             'name' => $imageName
         ]);
+    }
 
+    public function test_secure_images_uploads_to_correct_place()
+    {
+        config()->set('filesystems.default', 'local_secure');
+        $this->asEditor();
+        $galleryFile = $this->getTestImage('my-secure-test-upload');
+        $page = Page::first();
+        $expectedPath = storage_path('uploads/images/gallery/' . Date('Y-m-M') . '/my-secure-test-upload');
+
+        $upload = $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $page->id], [], ['file' => $galleryFile], []);
+        $upload->assertStatus(200);
+
+        $this->assertTrue(file_exists($expectedPath), 'Uploaded image not found at path: '. $expectedPath);
+
+        if (file_exists($expectedPath)) {
+            unlink($expectedPath);
+        }
+    }
+
+    public function test_system_images_remain_public()
+    {
+        config()->set('filesystems.default', 'local_secure');
+        $this->asEditor();
+        $galleryFile = $this->getTestImage('my-system-test-upload');
+        $page = Page::first();
+        $expectedPath = public_path('uploads/images/system/' . Date('Y-m-M') . '/my-system-test-upload');
+
+        $upload = $this->call('POST', '/images/system/upload', ['uploaded_to' => $page->id], [], ['file' => $galleryFile], []);
+        $upload->assertStatus(200);
+
+        $this->assertTrue(file_exists($expectedPath), 'Uploaded image not found at path: '. $expectedPath);
+
+        if (file_exists($expectedPath)) {
+            unlink($expectedPath);
+        }
     }
 
     public function test_image_delete()