Fix temporary URL on docker setup

app/Exceptions/SecurePaths/SignatureExpiredException.php

@@ -0,0 +1,20 @@
+<?php
+
+/**
+ * SPDX-License-Identifier: MIT
+ * Copyright (c) 2017-2018 Tobias Reich
+ * Copyright (c) 2018-2025 LycheeOrg.
+ */
+
+namespace App\Exceptions\SecurePaths;
+
+use App\Exceptions\BaseLycheeException;
+use Symfony\Component\HttpFoundation\Response;
+
+class SignatureExpiredException extends BaseLycheeException
+{
+	public function __construct(?\Throwable $previous = null)
+	{
+		parent::__construct(Response::HTTP_FORBIDDEN, 'Link expired', $previous);
+	}
+}

app/Http/Controllers/SecurePathController.php

@@ -11,25 +11,47 @@
 use App\Enum\StorageDiskType;
 use App\Exceptions\SecurePaths\InvalidPayloadException;
 use App\Exceptions\SecurePaths\InvalidSignatureException;
+use App\Exceptions\SecurePaths\SignatureExpiredException;
 use App\Exceptions\SecurePaths\WrongPathException;
 use App\Models\Configs;
+use App\Models\Extensions\HasUrlGenerator;
 use Illuminate\Contracts\Encryption\DecryptException;
 use Illuminate\Http\Request;
 use Illuminate\Routing\Controller;
+use Illuminate\Support\Carbon;
+use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Crypt;
+use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Storage;
 
 /**
  * Controller responsible for serving files securely.
  */
 class SecurePathController extends Controller
 {
+	use HasUrlGenerator;
+
 	public function __invoke(Request $request, ?string $path)
 	{
-		if (!$request->hasValidSignature()) {
+		// In theory we should use the `$request->hasCorrectSignature()` method here.
+		// However, for some stupid unknown reason, the path value is added to the server Query String.
+		// This completely invalidates the signature check.
+		// For example the url http://localhost:8000/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png?expires=1748380289
+		// will be verified as :
+		// http://localhost:8000/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png?/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png&expires=1748380289
+		// which makes the signature check fail as the hmac does not match.
+		if (!self::shouldNotUseSignedUrl() && !$this->hasCorrectSignature($request)) {
+			Log::error('Invalid signature for secure path request. Verify that the url generated for the image match.', [
+				'candidate url' => $this->getUrl($request),
+			]);
 			throw new InvalidSignatureException();
 		}
 
+		// On the bright side, we can now differentiate between a missing/failed signature and an expired one.
+		if (!self::shouldNotUseSignedUrl() && !$this->signatureHasNotExpired($request)) {
+			throw new SignatureExpiredException();
+		}
+
 		if (is_null($path)) {
 			throw new WrongPathException();
 		}
@@ -49,4 +71,55 @@ public function __invoke(Request $request, ?string $path)
 
 		return response()->file($file);
 	}
+
+	private function getUrl(Request $request, bool $absolute = true): string
+	{
+		$ignore_query = ['signature'];
+
+		$url = $absolute ? $request->url() : ('/' . $request->path());
+
+		$query_string = '';
+		$query_string = (new Collection(explode('&', (string) $request->server->get('QUERY_STRING'))))
+			->reject(fn ($parameter) => in_array(\Str::before($parameter, '='), $ignore_query, true))
+			->reject(fn ($parameter) => count(explode('=', $parameter)) === 1) // Ignore parameters without value => avoid problem above mentionned.
+			->join('&');
+
+		return rtrim($url . '?' . $query_string, '?');
+	}
+
+	/**
+	 * Determine if the signature from the given request matches the URL.
+	 *
+	 * @param \Illuminate\Http\Request $request
+	 * @param bool                     $absolute
+	 *
+	 * @return bool
+	 */
+	private function hasCorrectSignature(Request $request, bool $absolute = true): bool
+	{
+		$original = $this->getUrl($request, $absolute);
+		$key = new \SensitiveParameterValue(config('app.key'));
+		if (hash_equals(
+			hash_hmac('sha256', $original, $key->getValue()),
+			$request->query('signature', '')
+		)) {
+			return true;
+		}
+
+		return false;
+	}
+
+	/**
+	 * Determine if the expires timestamp from the given request is not from the past.
+	 *
+	 * @param \Illuminate\Http\Request $request
+	 *
+	 * @return bool
+	 */
+	private function signatureHasNotExpired(Request $request)
+	{
+		$expires = $request->query('expires');
+
+		return !($expires !== null && $expires !== '' && Carbon::now()->getTimestamp() > $expires);
+	}
 }

app/Models/Extensions/HasUrlGenerator.php

@@ -44,18 +44,25 @@ public static function pathToUrl(string $short_path, string $storage_disk, SizeV
 			// @codeCoverageIgnoreEnd
 		}
 
-		if (self::shouldNotUseSignedUrl()) {
+		// If we do not sign the URL or we do not use secure image links, we return the URL directly.
+		if (self::shouldNotUseSignedUrl() && !Configs::getValueAsBool('secure_image_link_enabled')) {
 			return $image_disk->url($short_path);
 		}
 
+		// We we use the secure image link, we encrypt the path.
 		if (Configs::getValueAsBool('secure_image_link_enabled')) {
 			$short_path = Crypt::encryptString($short_path);
 		}
 
+		// Return the url directly if we do not use signed URLs.
+		if (self::shouldNotUseSignedUrl()) {
+			return Url::route('image', ['path' => $short_path]);
+		}
+
 		$temporary_image_link_life_in_seconds = Configs::getValueAsInt('temporary_image_link_life_in_seconds');
 
 		/** @disregard P1013 */
-		return URL::temporarySignedRoute('image', now()->addSeconds($temporary_image_link_life_in_seconds), ['path' => $short_path]);
+		return URL::temporarySignedRoute('image', now()->addSeconds($temporary_image_link_life_in_seconds), ['path' => $short_path], true);
 	}
 
 	/**

database/migrations/2025_05_26_221704_bump_version060605.php

@@ -0,0 +1,46 @@
+<?php
+
+/**
+ * SPDX-License-Identifier: MIT
+ * Copyright (c) 2017-2018 Tobias Reich
+ * Copyright (c) 2018-2025 LycheeOrg.
+ */
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Support\Facades\Artisan;
+use Illuminate\Support\Facades\DB;
+use Symfony\Component\Console\Output\ConsoleOutput;
+use Symfony\Component\Console\Output\ConsoleSectionOutput;
+
+return new class() extends Migration {
+	private ConsoleOutput $output;
+	private ConsoleSectionOutput $msg_section;
+
+	public function __construct()
+	{
+		$this->output = new ConsoleOutput();
+		$this->msg_section = $this->output->section();
+	}
+
+	/**
+	 * Run the migrations.
+	 *
+	 * @return void
+	 */
+	public function up(): void
+	{
+		DB::table('configs')->where('key', 'version')->update(['value' => '060605']);
+		Artisan::call('cache:clear');
+		$this->msg_section->writeln('<info>Info:</info> Cleared cache for version 6.6.5');
+	}
+
+	/**
+	 * Reverse the migrations.
+	 *
+	 * @return void
+	 */
+	public function down(): void
+	{
+		DB::table('configs')->where('key', 'version')->update(['value' => '060604']);
+	}
+};

tests/Feature_v2/SecureImageLinksTest.php

@@ -24,9 +24,14 @@
 
 class SecureImageLinksTest extends BaseApiWithDataTest
 {
-	public function setUp(): void
+	private function setSecureLink()
+	{
+		Configs::set('secure_image_link_enabled', '1');
+		Configs::invalidateCache();
+	}
+
+	private function setTemporaryLink()
 	{
-		parent::setUp();
 		Configs::set('temporary_image_link_enabled', '1');
 		Configs::invalidateCache();
 	}
@@ -39,8 +44,9 @@ public function tearDown(): void
 		parent::tearDown();
 	}
 
-	public function testTemporaryImage(): void
+	public function testSignedImage(): void
 	{
+		$this->setTemporaryLink();
 		$response = $this->getJsonWithData('Album', ['album_id' => $this->album4->id]);
 		$this->assertOk($response);
 		$url = $response->json('resource.photos.0.size_variants.medium.url');
@@ -49,6 +55,24 @@ public function testTemporaryImage(): void
 		$response = $this->get($url);
 		$this->assertNotFound($response);
 		$response->assertSeeText('File not found'); // We mocked the file !
+	}
+
+	public function testExpiredSignature(): void
+	{
+		$this->setTemporaryLink();
+		$expired_url = URL::temporarySignedRoute('image', now()->subMinutes(10), ['path' => 'c3/3d/c661c594a5a781cd44db06828783.png']);
+		$response = $this->get($expired_url);
+		$this->assertForbidden($response);
+		$response->assertSeeText('Link expired');
+	}
+
+	public function testBrokenSignature(): void
+	{
+		$this->setTemporaryLink();
+		$response = $this->getJsonWithData('Album', ['album_id' => $this->album4->id]);
+		$this->assertOk($response);
+		$url = $response->json('resource.photos.0.size_variants.medium.url');
+		$this->assertStringContainsString('/image/medium/', $url);
 
 		$unsigned_url = explode('?', $url)[0];
 		$response = $this->get($unsigned_url);
@@ -57,8 +81,7 @@ public function testTemporaryImage(): void
 
 	public function testEncryptedImages(): void
 	{
-		Configs::set('secure_image_link_enabled', '1');
-		Configs::invalidateCache();
+		$this->setSecureLink();
 
 		$response = $this->getJsonWithData('Album', ['album_id' => $this->album4->id]);
 		$this->assertOk($response);
@@ -68,18 +91,12 @@ public function testEncryptedImages(): void
 		$response = $this->get($url);
 		$this->assertNotFound($response);
 		$response->assertSeeText('File not found'); // We mocked the file !
-
-		$unsigned_url = explode('?', $url)[0];
-		$response = $this->get($unsigned_url);
-		$this->assertForbidden($response);
 	}
 
 	public function testBrokenEncryption(): void
 	{
-		Configs::set('secure_image_link_enabled', '1');
-		Configs::invalidateCache();
-
-		$broken_url = URL::temporarySignedRoute('image', now()->addSeconds(10), ['path' => 'broken_path']);
+		$this->setSecureLink();
+		$broken_url = URL::route('image', ['path' => 'broken_path']);
 		$response = $this->get($broken_url);
 		$this->assertForbidden($response);
 		$response->assertSeeText('Invalid payload');

version.md

@@ -1 +1 @@
-6.6.4
+6.6.5