Skip to content

Commit cca066a

Browse files
committed
Merge branch 'development' into release
2 parents bbda5fd + 22a7772 commit cca066a

File tree

5 files changed

+102
-30
lines changed

5 files changed

+102
-30
lines changed

.env.example

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ DB_DATABASE=database_database
2626
DB_USERNAME=database_username
2727
DB_PASSWORD=database_user_password
2828

29+
# Storage system to use
30+
# By default files are stored on the local filesystem, with images being placed in
31+
# public web space so they can be efficiently served directly by the web-server.
32+
# For other options with different security levels & considerations, refer to:
33+
# https://www.bookstackapp.com/docs/admin/upload-config/
34+
STORAGE_TYPE=local
35+
2936
# Mail system to use
3037
# Can be 'smtp' or 'sendmail'
3138
MAIL_DRIVER=smtp

app/Uploads/ImageService.php

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ public function pathAccessible(string $imagePath): bool
264264
return false;
265265
}
266266

267-
if ($this->storage->usingSecureImages() && user()->isGuest()) {
267+
if ($this->blockedBySecureImages()) {
268268
return false;
269269
}
270270

@@ -280,13 +280,24 @@ public function imageAccessible(Image $image): bool
280280
return false;
281281
}
282282

283-
if ($this->storage->usingSecureImages() && user()->isGuest()) {
283+
if ($this->blockedBySecureImages()) {
284284
return false;
285285
}
286286

287287
return $this->imageFileExists($image->path, $image->type);
288288
}
289289

290+
/**
291+
* Check if the current user should be blocked from accessing images based on if secure images are enabled
292+
* and if public access is enabled for the application.
293+
*/
294+
protected function blockedBySecureImages(): bool
295+
{
296+
$enforced = $this->storage->usingSecureImages() && !setting('app-public');
297+
298+
return $enforced && user()->isGuest();
299+
}
300+
290301
/**
291302
* Check if the given image path exists for the given image type and that it is likely an image file.
292303
*/

app/Uploads/ImageStorage.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ protected function getDiskName(string $imageType): string
7474
return 'local';
7575
}
7676

77-
// Rename local_secure options to get our image specific storage driver which
77+
// Rename local_secure options to get our image-specific storage driver, which
7878
// is scoped to the relevant image directories.
7979
if ($localSecureInUse) {
8080
return 'local_secure_images';

composer.lock

Lines changed: 27 additions & 27 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/Uploads/ImageTest.php

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use BookStack\Entities\Repos\PageRepo;
66
use BookStack\Uploads\Image;
77
use BookStack\Uploads\ImageService;
8+
use BookStack\Uploads\UserAvatars;
9+
use BookStack\Users\Models\Role;
810
use Illuminate\Support\Str;
911
use Tests\TestCase;
1012

@@ -467,6 +469,26 @@ public function test_system_images_remain_public_with_local_secure_restricted()
467469
}
468470
}
469471

472+
public function test_avatar_images_visible_only_when_public_access_enabled_with_local_secure_restricted()
473+
{
474+
config()->set('filesystems.images', 'local_secure_restricted');
475+
$user = $this->users->admin();
476+
$avatars = $this->app->make(UserAvatars::class);
477+
$avatars->assignToUserFromExistingData($user, $this->files->pngImageData(), 'png');
478+
479+
$avatarUrl = $user->getAvatar();
480+
481+
$resp = $this->get($avatarUrl);
482+
$resp->assertRedirect('/login');
483+
484+
$this->permissions->makeAppPublic();
485+
486+
$resp = $this->get($avatarUrl);
487+
$resp->assertOk();
488+
489+
$this->files->deleteAtRelativePath($user->avatar->path);
490+
}
491+
470492
public function test_secure_restricted_images_inaccessible_without_relation_permission()
471493
{
472494
config()->set('filesystems.images', 'local_secure_restricted');
@@ -491,6 +513,38 @@ public function test_secure_restricted_images_inaccessible_without_relation_perm
491513
}
492514
}
493515

516+
public function test_secure_restricted_images_accessible_with_public_guest_access()
517+
{
518+
config()->set('filesystems.images', 'local_secure_restricted');
519+
$this->permissions->makeAppPublic();
520+
521+
$this->asEditor();
522+
$page = $this->entities->page();
523+
$this->files->uploadGalleryImageToPage($this, $page);
524+
$image = Image::query()->where('type', '=', 'gallery')
525+
->where('uploaded_to', '=', $page->id)
526+
->first();
527+
528+
$expectedUrl = url($image->path);
529+
$expectedPath = storage_path($image->path);
530+
auth()->logout();
531+
532+
$this->get($expectedUrl)->assertOk();
533+
534+
$this->permissions->setEntityPermissions($page, [], []);
535+
536+
$resp = $this->get($expectedUrl);
537+
$resp->assertNotFound();
538+
539+
$this->permissions->setEntityPermissions($page, ['view'], [Role::getSystemRole('public')]);
540+
541+
$this->get($expectedUrl)->assertOk();
542+
543+
if (file_exists($expectedPath)) {
544+
unlink($expectedPath);
545+
}
546+
}
547+
494548
public function test_thumbnail_path_handled_by_secure_restricted_images()
495549
{
496550
config()->set('filesystems.images', 'local_secure_restricted');

0 commit comments

Comments
 (0)