Merge pull request #665 from BookStackApp/authed_images

Adds ability to secure images behind auth

app/Http/Controllers/ImageController.php

@@ -1,6 +1,7 @@
 <?php namespace BookStack\Http\Controllers;
 
 use BookStack\Exceptions\ImageUploadException;
+use BookStack\Exceptions\NotFoundException;
 use BookStack\Repos\EntityRepo;
 use BookStack\Repos\ImageRepo;
 use Illuminate\Filesystem\Filesystem as File;
@@ -28,6 +29,21 @@ class ImageController extends Controller
         parent::__construct();
     }
 
+    /**
+     * Provide an image file from storage.
+     * @param string $path
+     * @return mixed
+     */
+    public function showImage(string $path)
+    {
+        $path = storage_path('uploads/images/' . $path);
+        if (!file_exists($path)) {
+            abort(404);
+        }
+
+        return response()->file($path);
+    }
+
     /**
      * Get all images for a specific type, Paginated
      * @param string $type

app/Repos/ImageRepo.php

@@ -183,7 +183,6 @@ class ImageRepo
      * Get the thumbnail for an image.
      * If $keepRatio is true only the width will be used.
      * Checks the cache then storage to avoid creating / accessing the filesystem on every check.
-     *
      * @param Image $image
      * @param int $width
      * @param int $height
@@ -194,9 +193,9 @@ class ImageRepo
     {
         try {
             return $this->imageService->getThumbnail($image, $width, $height, $keepRatio);
-        } catch (FileNotFoundException $exception) {
-            $image->delete();
-            return [];
+        } catch (\Exception $exception) {
+            dd($exception);
+            return null;
         }
     }
 

app/Services/AttachmentService.php

@@ -8,15 +8,35 @@ use Symfony\Component\HttpFoundation\File\UploadedFile;
 class AttachmentService extends UploadService
 {
 
+    /**
+     * Get the storage that will be used for storing files.
+     * @return \Illuminate\Contracts\Filesystem\Filesystem
+     */
+    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;
+    }
+
     /**
      * Get an attachment from storage.
      * @param Attachment $attachment
      * @return string
+     * @throws \Illuminate\Contracts\Filesystem\FileNotFoundException
      */
     public function getAttachmentFromStorage(Attachment $attachment)
     {
-        $attachmentPath = $this->getStorageBasePath() . $attachment->path;
-        return $this->getStorage()->get($attachmentPath);
+        return $this->getStorage()->get($attachment->path);
     }
 
     /**
@@ -92,16 +112,6 @@ class AttachmentService extends UploadService
         ]);
     }
 
-    /**
-     * Get the file storage base path, amended for storage type.
-     * This allows us to keep a generic path in the database.
-     * @return string
-     */
-    private function getStorageBasePath()
-    {
-        return $this->isLocal() ? 'storage/' : '';
-    }
-
     /**
      * Updates the file ordering for a listing of attached files.
      * @param array $attachmentList
@@ -138,6 +148,7 @@ class AttachmentService extends UploadService
     /**
      * Delete a File from the database and storage.
      * @param Attachment $attachment
+     * @throws Exception
      */
     public function deleteFile(Attachment $attachment)
     {
@@ -157,11 +168,10 @@ class AttachmentService extends UploadService
      */
     protected function deleteFileInStorage(Attachment $attachment)
     {
-        $storedFilePath = $this->getStorageBasePath() . $attachment->path;
         $storage = $this->getStorage();
-        $dirPath = dirname($storedFilePath);
+        $dirPath = dirname($attachment->path);
 
-        $storage->delete($storedFilePath);
+        $storage->delete($attachment->path);
         if (count($storage->allFiles($dirPath)) === 0) {
             $storage->deleteDirectory($dirPath);
         }
@@ -179,22 +189,20 @@ class AttachmentService extends UploadService
         $attachmentData = file_get_contents($uploadedFile->getRealPath());
 
         $storage = $this->getStorage();
-        $attachmentBasePath = 'uploads/files/' . Date('Y-m-M') . '/';
-        $storageBasePath = $this->getStorageBasePath() . $attachmentBasePath;
+        $basePath = 'uploads/files/' . Date('Y-m-M') . '/';
 
         $uploadFileName = $attachmentName;
-        while ($storage->exists($storageBasePath . $uploadFileName)) {
+        while ($storage->exists($basePath . $uploadFileName)) {
             $uploadFileName = str_random(3) . $uploadFileName;
         }
 
-        $attachmentPath = $attachmentBasePath . $uploadFileName;
-        $attachmentStoragePath = $this->getStorageBasePath() . $attachmentPath;
-
+        $attachmentPath = $basePath . $uploadFileName;
         try {
-            $storage->put($attachmentStoragePath, $attachmentData);
+            $storage->put($attachmentPath, $attachmentData);
         } catch (Exception $e) {
-            throw new FileUploadException(trans('errors.path_not_writable', ['filePath' => $attachmentStoragePath]));
+            throw new FileUploadException(trans('errors.path_not_writable', ['filePath' => $attachmentPath]));
         }
+
         return $attachmentPath;
     }
 

app/Services/ImageService.php

@@ -46,7 +46,6 @@ class ImageService extends UploadService
         return $this->saveNew($imageName, $imageData, $type, $uploadedTo);
     }
 
-
     /**
      * Gets an image from url and saves it to the database.
      * @param             $url
@@ -82,8 +81,6 @@ class ImageService extends UploadService
 
         $imagePath = '/uploads/images/' . $type . '/' . Date('Y-m-M') . '/';
 
-        if ($this->isLocal()) $imagePath = '/public' . $imagePath;
-
         while ($storage->exists($imagePath . $imageName)) {
             $imageName = str_random(3) . $imageName;
         }
@@ -96,8 +93,6 @@ class ImageService extends UploadService
             throw new ImageUploadException(trans('errors.path_not_writable', ['filePath' => $fullPath]));
         }
 
-        if ($this->isLocal()) $fullPath = str_replace_first('/public', '', $fullPath);
-
         $imageDetails = [
             'name'       => $imageName,
             'path'       => $fullPath,
@@ -112,8 +107,8 @@ class ImageService extends UploadService
             $imageDetails['updated_by'] = $userId;
         }
 
-        $image = Image::forceCreate($imageDetails);
-
+        $image = (new Image());
+        $image->forceFill($imageDetails)->save();
         return $image;
     }
 
@@ -124,14 +119,13 @@ class ImageService extends UploadService
      */
     protected function getPath(Image $image)
     {
-        return ($this->isLocal()) ? ('public/' . $image->path) : $image->path;
+        return $image->path;
     }
 
     /**
      * Get the thumbnail for an image.
      * If $keepRatio is true only the width will be used.
      * Checks the cache then storage to avoid creating / accessing the filesystem on every check.
-     *
      * @param Image $image
      * @param int $width
      * @param int $height
@@ -151,7 +145,6 @@ class ImageService extends UploadService
         }
 
         $storage = $this->getStorage();
-
         if ($storage->exists($thumbFilePath)) {
             return $this->getPublicUrl($thumbFilePath);
         }
@@ -161,9 +154,8 @@ class ImageService extends UploadService
         } catch (Exception $e) {
             if ($e instanceof \ErrorException || $e instanceof NotSupportedException) {
                 throw new ImageUploadException(trans('errors.cannot_create_thumbs'));
-            } else {
-                throw $e;
             }
+            throw $e;
         }
 
         if ($keepRatio) {
@@ -252,13 +244,11 @@ class ImageService extends UploadService
                     $storageUrl = 'https://s3-' . $storageDetails['region'] . '.amazonaws.com/' . $storageDetails['bucket'];
                 }
             }
-
             $this->storageUrl = $storageUrl;
         }
 
-        if ($this->isLocal()) $filePath = str_replace_first('public/', '', $filePath);
-
-        return ($this->storageUrl == false ? rtrim(baseUrl(''), '/') : rtrim($this->storageUrl, '/')) . $filePath;
+        $basePath = ($this->storageUrl == false) ? baseUrl('/') : $this->storageUrl;
+        return rtrim($basePath, '/') . $filePath;
     }
 
 

app/Services/UploadService.php

@@ -40,7 +40,6 @@ class UploadService
         return $this->storageInstance;
     }
 
-
     /**
      * Check whether or not a folder is empty.
      * @param $path

config/filesystems.php

@@ -56,7 +56,12 @@ return [
 
         'local' => [
             'driver' => 'local',
-            'root'   => base_path(),
+            'root' => public_path(),
+        ],
+
+        'local_secure' => [
+            'driver' => 'local',
+            'root'   => storage_path(),
         ],
 
         'ftp' => [

routes/web.php

@@ -5,6 +5,9 @@ Route::get('/translations', 'HomeController@getTranslations');
 // Authenticated routes...
 Route::group(['middleware' => 'auth'], function () {
 
+    Route::get('/uploads/images/{path}', 'ImageController@showImage')
+        ->where('path', '.*$');
+
     Route::group(['prefix' => 'pages'], function() {
         Route::get('/recently-created', 'PageController@showRecentlyCreated');
         Route::get('/recently-updated', 'PageController@showRecentlyUpdated');

storage/uploads/images/.gitignore

@@ -0,0 +1,2 @@
+*
+!.gitignore