diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index a26a04ac5..ed640913e 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -264,7 +264,7 @@ class ImageService return false; } - if ($this->storage->usingSecureImages() && user()->isGuest()) { + if ($this->blockedBySecureImages()) { return false; } @@ -280,13 +280,24 @@ class ImageService return false; } - if ($this->storage->usingSecureImages() && user()->isGuest()) { + if ($this->blockedBySecureImages()) { return false; } return $this->imageFileExists($image->path, $image->type); } + /** + * Check if the current user should be blocked from accessing images based on if secure images are enabled + * and if public access is enabled for the application. + */ + protected function blockedBySecureImages(): bool + { + $enforced = $this->storage->usingSecureImages() && !setting('app-public'); + + return $enforced && user()->isGuest(); + } + /** * Check if the given image path exists for the given image type and that it is likely an image file. */ diff --git a/app/Uploads/ImageStorage.php b/app/Uploads/ImageStorage.php index 38a22e3b4..abf2b429b 100644 --- a/app/Uploads/ImageStorage.php +++ b/app/Uploads/ImageStorage.php @@ -74,7 +74,7 @@ class ImageStorage return 'local'; } - // Rename local_secure options to get our image specific storage driver which + // Rename local_secure options to get our image-specific storage driver, which // is scoped to the relevant image directories. if ($localSecureInUse) { return 'local_secure_images'; diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php index c5a5eb2ba..f36be8702 100644 --- a/tests/Uploads/ImageTest.php +++ b/tests/Uploads/ImageTest.php @@ -5,6 +5,8 @@ namespace Tests\Uploads; use BookStack\Entities\Repos\PageRepo; use BookStack\Uploads\Image; use BookStack\Uploads\ImageService; +use BookStack\Uploads\UserAvatars; +use BookStack\Users\Models\Role; use Illuminate\Support\Str; use Tests\TestCase; @@ -467,6 +469,26 @@ class ImageTest extends TestCase } } + public function test_avatar_images_visible_only_when_public_access_enabled_with_local_secure_restricted() + { + config()->set('filesystems.images', 'local_secure_restricted'); + $user = $this->users->admin(); + $avatars = $this->app->make(UserAvatars::class); + $avatars->assignToUserFromExistingData($user, $this->files->pngImageData(), 'png'); + + $avatarUrl = $user->getAvatar(); + + $resp = $this->get($avatarUrl); + $resp->assertRedirect('/login'); + + $this->permissions->makeAppPublic(); + + $resp = $this->get($avatarUrl); + $resp->assertOk(); + + $this->files->deleteAtRelativePath($user->avatar->path); + } + public function test_secure_restricted_images_inaccessible_without_relation_permission() { config()->set('filesystems.images', 'local_secure_restricted'); @@ -491,6 +513,38 @@ class ImageTest extends TestCase } } + public function test_secure_restricted_images_accessible_with_public_guest_access() + { + config()->set('filesystems.images', 'local_secure_restricted'); + $this->permissions->makeAppPublic(); + + $this->asEditor(); + $page = $this->entities->page(); + $this->files->uploadGalleryImageToPage($this, $page); + $image = Image::query()->where('type', '=', 'gallery') + ->where('uploaded_to', '=', $page->id) + ->first(); + + $expectedUrl = url($image->path); + $expectedPath = storage_path($image->path); + auth()->logout(); + + $this->get($expectedUrl)->assertOk(); + + $this->permissions->setEntityPermissions($page, [], []); + + $resp = $this->get($expectedUrl); + $resp->assertNotFound(); + + $this->permissions->setEntityPermissions($page, ['view'], [Role::getSystemRole('public')]); + + $this->get($expectedUrl)->assertOk(); + + if (file_exists($expectedPath)) { + unlink($expectedPath); + } + } + public function test_thumbnail_path_handled_by_secure_restricted_images() { config()->set('filesystems.images', 'local_secure_restricted');