From 2c3523f6a1237be491f400340f2f887e2ea1a5a9 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Mon, 24 May 2021 12:09:28 +0100
Subject: [PATCH] Updated image permission setting logic

To ensure thhat the visibility is still set on local storage options
since the previous recent changes could cause problems where in
scenarios where the server user could not read images uploaded by the
php process user.

Closes #2758
---
 app/Uploads/ImageService.php | 11 ++++++-----
 tests/Uploads/ImageTest.php  | 22 +++++++++++-----------
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php
index 7e8eedada..293049f4f 100644
--- a/app/Uploads/ImageService.php
+++ b/app/Uploads/ImageService.php
@@ -140,12 +140,13 @@ class ImageService
     {
         $storage->put($path, $data);
 
-        // Set visibility if using s3 without an endpoint set.
-        // Done since this call can break s3-like services but desired for actual
-        // AWS s3 usage. Attempting to set ACL during above put request requires
-        // different permissions hence would technically be a breaking change.
+        // Set visibility when a non-AWS-s3, s3-like storage option is in use.
+        // Done since this call can break s3-like services but desired for other image stores.
+        // Attempting to set ACL during above put request requires different permissions
+        // hence would technically be a breaking change for actual s3 usage.
         $usingS3 = strtolower(config('filesystems.images')) === 's3';
-        if ($usingS3 && is_null(config('filesystems.disks.s3.endpoint'))) {
+        $usingS3Like = $usingS3 && !is_null(config('filesystems.disks.s3.endpoint'));
+        if (!$usingS3Like) {
             $storage->setVisibility($path, 'public');
         }
     }
diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php
index c03d15dd7..95332565e 100644
--- a/tests/Uploads/ImageTest.php
+++ b/tests/Uploads/ImageTest.php
@@ -14,7 +14,7 @@ class ImageTest extends TestCase
 
     public function test_image_upload()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $admin = $this->getAdmin();
         $this->actingAs($admin);
 
@@ -38,7 +38,7 @@ class ImageTest extends TestCase
 
     public function test_image_display_thumbnail_generation_does_not_increase_image_size()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $admin = $this->getAdmin();
         $this->actingAs($admin);
 
@@ -108,7 +108,7 @@ class ImageTest extends TestCase
 
     public function test_image_usage()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $editor = $this->getEditor();
         $this->actingAs($editor);
 
@@ -128,7 +128,7 @@ class ImageTest extends TestCase
 
     public function test_php_files_cannot_be_uploaded()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $admin = $this->getAdmin();
         $this->actingAs($admin);
 
@@ -150,7 +150,7 @@ class ImageTest extends TestCase
 
     public function test_php_like_files_cannot_be_uploaded()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $admin = $this->getAdmin();
         $this->actingAs($admin);
 
@@ -202,7 +202,7 @@ class ImageTest extends TestCase
         ];
         foreach ($badNames as $name) {
             $galleryFile = $this->getTestImage($name);
-            $page = Page::first();
+            $page = Page::query()->first();
             $badPath = $this->getTestImagePath('gallery', $name);
             $this->deleteImage($badPath);
 
@@ -227,7 +227,7 @@ class ImageTest extends TestCase
         config()->set('filesystems.images', 'local_secure');
         $this->asEditor();
         $galleryFile = $this->getTestImage('my-secure-test-upload.png');
-        $page = Page::first();
+        $page = Page::query()->first();
         $expectedPath = storage_path('uploads/images/gallery/' . Date('Y-m') . '/my-secure-test-upload.png');
 
         $upload = $this->call('POST', '/images/gallery', ['uploaded_to' => $page->id], [], ['file' => $galleryFile], []);
@@ -245,7 +245,7 @@ class ImageTest extends TestCase
         config()->set('filesystems.images', 'local_secure');
         $this->asEditor();
         $galleryFile = $this->getTestImage('my-secure-test-upload.png');
-        $page = Page::first();
+        $page = Page::query()->first();
         $expectedPath = storage_path('uploads/images/gallery/' . Date('Y-m') . '/my-secure-test-upload.png');
 
         $upload = $this->call('POST', '/images/gallery', ['uploaded_to' => $page->id], [], ['file' => $galleryFile], []);
@@ -282,7 +282,7 @@ class ImageTest extends TestCase
 
     public function test_image_delete()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $this->asAdmin();
         $imageName = 'first-image.png';
         $relPath = $this->getTestImagePath('gallery', $imageName);
@@ -304,7 +304,7 @@ class ImageTest extends TestCase
 
     public function test_image_delete_does_not_delete_similar_images()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $this->asAdmin();
         $imageName = 'first-image.png';
 
@@ -383,7 +383,7 @@ class ImageTest extends TestCase
 
     public function test_deleted_unused_images()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $admin = $this->getAdmin();
         $this->actingAs($admin);