Ensured uploaded system images remain public

Also added tests to cover local_secure image storage. Fixes #725
BookStackApp · Mar 25, 2018 · 23f90ed · 23f90ed
1 parent f1586be
commit 23f90ed
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 33 deletions.
diff --git a/app/Repos/ImageRepo.php b/app/Repos/ImageRepo.php
@@ -225,7 +225,6 @@ public function getThumbnail(Image $image, $width = 220, $height = 220, $keepRat
         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
@@ -14,20 +14,14 @@ 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.
         if ($storageType === 'local') {
             $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
@@ -31,6 +31,23 @@ public function __construct(ImageManager $imageTool, FileSystem $fileSystem, Cac
         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 @@ private function saveNewFromUrl($url, $type, $imageName = false)
      */
     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 @@ public function getThumbnail(Image $image, $width = 220, $height = 220, $keepRat
             return $this->getPublicUrl($thumbFilePath);
         }
 
-        $storage = $this->getStorage();
+        $storage = $this->getStorage($image->type);
         if ($storage->exists($thumbFilePath)) {
             return $this->getPublicUrl($thumbFilePath);
         }
@@ -287,8 +304,9 @@ public function destroyImage(Image $image)
     /**
      * 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
@@ -11,11 +11,6 @@ class UploadService
      */
     protected $fileSystem;
 
-    /**
-     * @var FileSystemInstance
-     */
-    protected $storageInstance;
-
 
     /**
      * FileService constructor.
@@ -32,14 +27,8 @@ public function __construct(FileSystem $fileSystem)
      */
     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 @@ protected function isFolderEmpty($path)
         $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
@@ -21,7 +21,7 @@ protected function getTestImageFilePath()
      */
     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 @@ public function test_image_upload()
             '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()