From a75d5b8bc17f5d1352ae5db7804df10f26ff751b Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Thu, 22 Feb 2024 11:22:08 +0000
Subject: [PATCH] Sessions: Prevent image urls being part of session URL
history
To prevent them being considered for redirects.
Includes test to cover.
For #4863
---
app/Http/Kernel.php | 2 +-
app/Http/Middleware/StartSessionExtended.php | 34 ++++++++++++++++++++
tests/Uploads/ImageTest.php | 23 +++++++++++++
3 files changed, 58 insertions(+), 1 deletion(-)
create mode 100644 app/Http/Middleware/StartSessionExtended.php
diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php
index 1b96ff3db..d23f56a2c 100644
--- a/app/Http/Kernel.php
+++ b/app/Http/Kernel.php
@@ -28,7 +28,7 @@ class Kernel extends HttpKernel
\BookStack\Http\Middleware\ApplyCspRules::class,
\BookStack\Http\Middleware\EncryptCookies::class,
\Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
- \Illuminate\Session\Middleware\StartSession::class,
+ \BookStack\Http\Middleware\StartSessionExtended::class,
\Illuminate\View\Middleware\ShareErrorsFromSession::class,
\BookStack\Http\Middleware\VerifyCsrfToken::class,
\BookStack\Http\Middleware\CheckEmailConfirmed::class,
diff --git a/app/Http/Middleware/StartSessionExtended.php b/app/Http/Middleware/StartSessionExtended.php
new file mode 100644
index 000000000..26cd250ac
--- /dev/null
+++ b/app/Http/Middleware/StartSessionExtended.php
@@ -0,0 +1,34 @@
+<?php
+
+namespace BookStack\Http\Middleware;
+
+use Illuminate\Http\Request;
+use Illuminate\Session\Middleware\StartSession as Middleware;
+
+/**
+ * An extended version of the default Laravel "StartSession" middleware
+ * with customizations applied as required:
+ *
+ * - Adds filtering for the request URLs stored in session history.
+ */
+class StartSessionExtended extends Middleware
+{
+ protected static array $pathPrefixesExcludedFromHistory = [
+ 'uploads/images/'
+ ];
+
+ /**
+ * @inheritdoc
+ */
+ protected function storeCurrentUrl(Request $request, $session): void
+ {
+ $requestPath = strtolower($request->path());
+ foreach (static::$pathPrefixesExcludedFromHistory as $excludedPath) {
+ if (str_starts_with($requestPath, $excludedPath)) {
+ return;
+ }
+ }
+
+ parent::storeCurrentUrl($request, $session);
+ }
+}
diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php
index af249951f..d24b6202b 100644
--- a/tests/Uploads/ImageTest.php
+++ b/tests/Uploads/ImageTest.php
@@ -383,6 +383,29 @@ class ImageTest extends TestCase
}
}
+ public function test_secure_images_not_tracked_in_session_history()
+ {
+ config()->set('filesystems.images', 'local_secure');
+ $this->asEditor();
+ $page = $this->entities->page();
+ $result = $this->files->uploadGalleryImageToPage($this, $page);
+ $expectedPath = storage_path($result['path']);
+ $this->assertFileExists($expectedPath);
+
+ $this->get('/books');
+ $this->assertEquals(url('/books'), session()->previousUrl());
+
+ $resp = $this->get($result['path']);
+ $resp->assertOk();
+ $resp->assertHeader('Content-Type', 'image/png');
+
+ $this->assertEquals(url('/books'), session()->previousUrl());
+
+ if (file_exists($expectedPath)) {
+ unlink($expectedPath);
+ }
+ }
+
public function test_system_images_remain_public_with_local_secure_restricted()
{
config()->set('filesystems.images', 'local_secure_restricted');