Exports: Updated perm checking for images in ZIP exports

For #5885
Adds to, uses and cleans-up central permission checking in ImageService
to mirror that which would be experienced by users in the UI to result
in the same image access conditions.

Adds testing to cover.

Author: ssddanbrown

app/Exports/ZipExports/ZipExportReferences.php

@@ -15,6 +15,7 @@
 use BookStack\Permissions\Permission;
 use BookStack\Uploads\Attachment;
 use BookStack\Uploads\Image;
+use BookStack\Uploads\ImageService;
 
 class ZipExportReferences
 {
@@ -33,6 +34,7 @@ class ZipExportReferences
 
     public function __construct(
         protected ZipReferenceParser $parser,
+        protected ImageService $imageService,
     ) {
     }
 
@@ -133,17 +135,25 @@ protected function handleModelReference(Model $model, ZipExportModel $exportMode
                 return "[[bsexport:image:{$model->id}]]";
             }
 
-            // Find and include images if in visibility
+            // Get the page which we'll reference this image upon
             $page = $model->getPage();
-            $pageExportModel = $this->pages[$page->id] ?? ($exportModel instanceof ZipExportPage ? $exportModel : null);
-            if (isset($this->images[$model->id]) || ($page && $pageExportModel && userCan(Permission::PageView, $page))) {
+            $pageExportModel = null;
+            if ($page && isset($this->pages[$page->id])) {
+                $pageExportModel = $this->pages[$page->id];
+            } elseif ($exportModel instanceof ZipExportPage) {
+                $pageExportModel = $exportModel;
+            }
+
+            // Add the image to the export if it's accessible or just return the existing reference if already added
+            if (isset($this->images[$model->id]) || ($pageExportModel && $this->imageService->imageAccessible($model))) {
                 if (!isset($this->images[$model->id])) {
                     $exportImage = ZipExportImage::fromModel($model, $files);
                     $this->images[$model->id] = $exportImage;
                     $pageExportModel->images[] = $exportImage;
                 }
                 return "[[bsexport:image:{$model->id}]]";
             }
+
             return null;
         }
 

app/Uploads/Image.php

@@ -13,14 +13,14 @@
 use Illuminate\Database\Eloquent\Relations\HasMany;
 
 /**
- * @property int    $id
- * @property string $name
- * @property string $url
- * @property string $path
- * @property string $type
- * @property int    $uploaded_to
- * @property int    $created_by
- * @property int    $updated_by
+ * @property int      $id
+ * @property string   $name
+ * @property string   $url
+ * @property string   $path
+ * @property string   $type
+ * @property int|null $uploaded_to
+ * @property int      $created_by
+ * @property int      $updated_by
  */
 class Image extends Model implements OwnableInterface
 {

app/Uploads/ImageService.php

@@ -148,7 +148,7 @@ public function getImageStream(Image $image): mixed
     }
 
     /**
-     * Destroy an image along with its revisions, thumbnails and remaining folders.
+     * Destroy an image along with its revisions, thumbnails, and remaining folders.
      *
      * @throws Exception
      */
@@ -252,33 +252,48 @@ public function pathAccessibleInLocalSecure(string $imagePath): bool
     {
         $disk = $this->storage->getDisk('gallery');
 
+        return $disk->usingSecureImages() && $this->pathAccessible($imagePath);
+    }
+
+    /**
+     * Check if the given path exists and is accessible depending on the current settings.
+     */
+    public function pathAccessible(string $imagePath): bool
+    {
         if ($this->storage->usingSecureRestrictedImages() && !$this->checkUserHasAccessToRelationOfImageAtPath($imagePath)) {
             return false;
         }
 
-        // Check local_secure is active
-        return $disk->usingSecureImages()
-            // Check the image file exists
-            && $disk->exists($imagePath)
-            // Check the file is likely an image file
-            && str_starts_with($disk->mimeType($imagePath), 'image/');
+        if ($this->storage->usingSecureImages() && user()->isGuest()) {
+            return false;
+        }
+
+        return $this->imageFileExists($imagePath, 'gallery');
     }
 
     /**
-     * Check if the given path exists and is accessible depending on the current settings.
+     * Check if the given image should be accessible to the current user.
      */
-    public function pathAccessible(string $imagePath): bool
+    public function imageAccessible(Image $image): bool
     {
-        $disk = $this->storage->getDisk('gallery');
+        if ($this->storage->usingSecureRestrictedImages() && !$this->checkUserHasAccessToRelationOfImage($image)) {
+            return false;
+        }
 
-        if ($this->storage->usingSecureRestrictedImages() && !$this->checkUserHasAccessToRelationOfImageAtPath($imagePath)) {
+        if ($this->storage->usingSecureImages() && user()->isGuest()) {
             return false;
         }
 
-        // Check local_secure is active
-        return $disk->exists($imagePath)
-            // Check the file is likely an image file
-            && str_starts_with($disk->mimeType($imagePath), 'image/');
+        return $this->imageFileExists($image->path, $image->type);
+    }
+
+    /**
+     * Check if the given image path exists for the given image type and that it is likely an image file.
+     */
+    protected function imageFileExists(string $imagePath, string $imageType): bool
+    {
+        $disk = $this->storage->getDisk($imageType);
+        return $disk->exists($imagePath) && str_starts_with($disk->mimeType($imagePath), 'image/');
     }
 
     /**
@@ -307,6 +322,11 @@ protected function checkUserHasAccessToRelationOfImageAtPath(string $path): bool
             return false;
         }
 
+        return $this->checkUserHasAccessToRelationOfImage($image);
+    }
+
+    protected function checkUserHasAccessToRelationOfImage(Image $image): bool
+    {
         $imageType = $image->type;
 
         // Allow user or system (logo) images

app/Uploads/ImageStorage.php

@@ -34,6 +34,15 @@ public function usingSecureRestrictedImages(): bool
         return config('filesystems.images') === 'local_secure_restricted';
     }
 
+    /**
+     * Check if "local secure" (Fetched behind auth, either with or without permissions enforced)
+     * is currently active in the instance.
+     */
+    public function usingSecureImages(): bool
+    {
+        return config('filesystems.images') === 'local_secure' || $this->usingSecureRestrictedImages();
+    }
+
     /**
      * Clean up an image file name to be both URL and storage safe.
      */

tests/Exports/ZipExportTest.php

@@ -374,6 +374,54 @@ public function test_image_links_are_handled_when_using_external_storage_url()
         $this->assertStringContainsString("<a href=\"{$ref}\">Original URL</a><a href=\"{$ref}\">Storage URL</a>", $pageData['html']);
     }
 
+    public function test_orphaned_images_can_be_used_on_default_local_storage()
+    {
+        $this->asEditor();
+        $page = $this->entities->page();
+        $result = $this->files->uploadGalleryImageToPage($this, $page);
+        $displayThumb = $result['response']->thumbs->gallery ?? '';
+        $page->html = '<p><img src="' . $displayThumb . '" alt="My image"></p>';
+        $page->save();
+
+        $image = Image::findOrFail($result['response']->id);
+        $image->uploaded_to = null;
+        $image->save();
+
+        $zipResp = $this->asEditor()->get($page->getUrl("/export/zip"));
+        $zipResp->assertOk();
+        $zip = ZipTestHelper::extractFromZipResponse($zipResp);
+        $pageData = $zip->data['page'];
+
+        $this->assertCount(1, $pageData['images']);
+        $imageData = $pageData['images'][0];
+        $this->assertEquals($image->id, $imageData['id']);
+
+        $this->assertEquals('<p><img src="[[bsexport:image:' . $imageData['id'] . ']]" alt="My image"></p>', $pageData['html']);
+    }
+
+    public function test_orphaned_images_cannot_be_used_on_local_secure_restricted()
+    {
+        config()->set('filesystems.images', 'local_secure_restricted');
+
+        $this->asEditor();
+        $page = $this->entities->page();
+        $result = $this->files->uploadGalleryImageToPage($this, $page);
+        $displayThumb = $result['response']->thumbs->gallery ?? '';
+        $page->html = '<p><img src="' . $displayThumb . '" alt="My image"></p>';
+        $page->save();
+
+        $image = Image::findOrFail($result['response']->id);
+        $image->uploaded_to = null;
+        $image->save();
+
+        $zipResp = $this->asEditor()->get($page->getUrl("/export/zip"));
+        $zipResp->assertOk();
+        $zip = ZipTestHelper::extractFromZipResponse($zipResp);
+        $pageData = $zip->data['page'];
+
+        $this->assertCount(0, $pageData['images']);
+    }
+
     public function test_cross_reference_links_external_to_export_are_not_converted()
     {
         $page = $this->entities->page();