From 762d1d759590aa25466c3702279baff92c09e854 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 23 Jun 2019 16:01:15 +0100
Subject: [PATCH] Allowed different storage types for images and attachments
- Added new env and config vars to allow this.
- Also added tests for awkward config logic including fallback for new
env vars.
Closes #1302
---
.env.example.complete | 10 ++++++
app/Uploads/AttachmentService.php | 2 +-
app/Uploads/ImageService.php | 6 ++--
config/filesystems.php | 6 ++++
phpunit.xml | 2 ++
tests/Unit/ConfigTest.php | 54 +++++++++++++++++++++++++++++++
tests/Uploads/ImageTest.php | 6 ++--
7 files changed, 79 insertions(+), 7 deletions(-)
create mode 100644 tests/Unit/ConfigTest.php
diff --git a/.env.example.complete b/.env.example.complete
index 37421a419..829a7509b 100644
--- a/.env.example.complete
+++ b/.env.example.complete
@@ -95,6 +95,16 @@ QUEUE_DRIVER=sync
# Can be 'local', 'local_secure' or 's3'
STORAGE_TYPE=local
+# Image storage system to use
+# Defaults to the value of STORAGE_TYPE if unset.
+# Accepts the same values as STORAGE_TYPE.
+STORAGE_IMAGE_TYPE=local
+
+# Attachment storage system to use
+# Defaults to the value of STORAGE_TYPE if unset.
+# Accepts the same values as STORAGE_TYPE although 'local' will be forced to 'local_secure'.
+STORAGE_ATTACHMENT_TYPE=local_secure
+
# Amazon S3 storage configuration
STORAGE_S3_KEY=your-s3-key
STORAGE_S3_SECRET=your-s3-secret
diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php
index e613642c4..6e875a1e7 100644
--- a/app/Uploads/AttachmentService.php
+++ b/app/Uploads/AttachmentService.php
@@ -13,7 +13,7 @@ class AttachmentService extends UploadService
*/
protected function getStorage()
{
- $storageType = config('filesystems.default');
+ $storageType = config('filesystems.attachments');
// Override default location if set to local public to ensure not visible.
if ($storageType === 'local') {
diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php
index 8eefbaf9d..71fd2cd4e 100644
--- a/app/Uploads/ImageService.php
+++ b/app/Uploads/ImageService.php
@@ -45,9 +45,9 @@ class ImageService extends UploadService
*/
protected function getStorage($type = '')
{
- $storageType = config('filesystems.default');
+ $storageType = config('filesystems.images');
- // Override default location if set to local public to ensure not visible.
+ // Ensure system images (App logo) are uploaded to a public space
if ($type === 'system' && $storageType === 'local_secure') {
$storageType = 'local';
}
@@ -458,7 +458,7 @@ class ImageService extends UploadService
// Get the standard public s3 url if s3 is set as storage type
// Uses the nice, short URL if bucket name has no periods in otherwise the longer
// region-based url will be used to prevent http issues.
- if ($storageUrl == false && config('filesystems.default') === 's3') {
+ if ($storageUrl == false && config('filesystems.images') === 's3') {
$storageDetails = config('filesystems.disks.s3');
if (strpos($storageDetails['bucket'], '.') === false) {
$storageUrl = 'https://' . $storageDetails['bucket'] . '.s3.amazonaws.com';
diff --git a/config/filesystems.php b/config/filesystems.php
index 13198a505..bd7d28300 100644
--- a/config/filesystems.php
+++ b/config/filesystems.php
@@ -14,6 +14,12 @@ return [
// Options: local, local_secure, s3
'default' => env('STORAGE_TYPE', 'local'),
+ // Filesystem to use specifically for image uploads.
+ 'images' => env('STORAGE_IMAGE_TYPE', env('STORAGE_TYPE', 'local')),
+
+ // Filesystem to use specifically for file attachments.
+ 'attachments' => env('STORAGE_ATTACHMENT_TYPE', env('STORAGE_TYPE', 'local')),
+
// Storage URL
// This is the url to where the storage is located for when using an external
// file storage service, such as s3, to store publicly accessible assets.
diff --git a/phpunit.xml b/phpunit.xml
index 804afcf5d..0e51f6af1 100644
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -34,6 +34,8 @@
<env name="AVATAR_URL" value=""/>
<env name="LDAP_VERSION" value="3"/>
<env name="STORAGE_TYPE" value="local"/>
+ <env name="ATTACHMENT_STORAGE_TYPE" value="local"/>
+ <env name="IMAGE_STORAGE_TYPE" value="local"/>
<env name="GITHUB_APP_ID" value="aaaaaaaaaaaaaa"/>
<env name="GITHUB_APP_SECRET" value="aaaaaaaaaaaaaa"/>
<env name="GITHUB_AUTO_REGISTER" value=""/>
diff --git a/tests/Unit/ConfigTest.php b/tests/Unit/ConfigTest.php
new file mode 100644
index 000000000..45f7b079d
--- /dev/null
+++ b/tests/Unit/ConfigTest.php
@@ -0,0 +1,54 @@
+<?php namespace Tests;
+
+/**
+ * Class ConfigTest
+ * Many of the tests here are to check on tweaks made
+ * to maintain backwards compatibility.
+ *
+ * @package Tests
+ */
+class ConfigTest extends TestCase
+{
+
+ public function test_filesystem_images_falls_back_to_storage_type_var()
+ {
+ putenv('STORAGE_TYPE=local_secure');
+
+ $this->checkEnvConfigResult('STORAGE_IMAGE_TYPE', 's3', 'filesystems.images', 's3');
+ $this->checkEnvConfigResult('STORAGE_IMAGE_TYPE', null, 'filesystems.images', 'local_secure');
+ }
+
+ public function test_filesystem_attachments_falls_back_to_storage_type_var()
+ {
+ putenv('STORAGE_TYPE=local_secure');
+
+ $this->checkEnvConfigResult('STORAGE_ATTACHMENT_TYPE', 's3', 'filesystems.attachments', 's3');
+ $this->checkEnvConfigResult('STORAGE_ATTACHMENT_TYPE', null, 'filesystems.attachments', 'local_secure');
+ }
+
+ public function test_app_url_blank_if_old_default_value()
+ {
+ $initUrl = 'https://example.com/docs';
+ $oldDefault = 'http://bookstack.dev';
+ $this->checkEnvConfigResult('APP_URL', $initUrl, 'app.url', $initUrl);
+ $this->checkEnvConfigResult('APP_URL', $oldDefault, 'app.url', '');
+ }
+
+ /**
+ * Set an environment variable of the given name and value
+ * then check the given config key to see if it matches the given result.
+ * Providing a null $envVal clears the variable.
+ * @param string $envName
+ * @param string|null $envVal
+ * @param string $configKey
+ * @param string $expectedResult
+ */
+ protected function checkEnvConfigResult(string $envName, $envVal, string $configKey, string $expectedResult)
+ {
+ $envString = $envName . (is_null($envVal) ? '' : '=') . ($envVal ?? '');
+ putenv($envString);
+ $this->refreshApplication();
+ $this->assertEquals($expectedResult, config($configKey));
+ }
+
+}
\ No newline at end of file
diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php
index 01bf23d5b..f92653378 100644
--- a/tests/Uploads/ImageTest.php
+++ b/tests/Uploads/ImageTest.php
@@ -176,7 +176,7 @@ class ImageTest extends TestCase
public function test_secure_images_uploads_to_correct_place()
{
- config()->set('filesystems.default', 'local_secure');
+ config()->set('filesystems.images', 'local_secure');
$this->asEditor();
$galleryFile = $this->getTestImage('my-secure-test-upload.png');
$page = Page::first();
@@ -194,7 +194,7 @@ class ImageTest extends TestCase
public function test_secure_images_included_in_exports()
{
- config()->set('filesystems.default', 'local_secure');
+ config()->set('filesystems.images', 'local_secure');
$this->asEditor();
$galleryFile = $this->getTestImage('my-secure-test-upload.png');
$page = Page::first();
@@ -217,7 +217,7 @@ class ImageTest extends TestCase
public function test_system_images_remain_public()
{
- config()->set('filesystems.default', 'local_secure');
+ config()->set('filesystems.images', 'local_secure');
$this->asAdmin();
$galleryFile = $this->getTestImage('my-system-test-upload.png');
$expectedPath = public_path('uploads/images/system/' . Date('Y-m') . '/my-system-test-upload.png');