Skip to content

Commit 70cb308

Browse files
authored
Fix secure link missing route (#3348)
1 parent 1818d01 commit 70cb308

4 files changed

Lines changed: 118 additions & 1 deletion

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
/**
4+
* SPDX-License-Identifier: MIT
5+
* Copyright (c) 2017-2018 Tobias Reich
6+
* Copyright (c) 2018-2025 LycheeOrg.
7+
*/
8+
9+
namespace App\Exceptions\SecurePaths;
10+
11+
use App\Exceptions\BaseLycheeException;
12+
use Symfony\Component\HttpFoundation\Response;
13+
14+
class InvalidPayloadException extends BaseLycheeException
15+
{
16+
public function __construct(?\Throwable $previous = null)
17+
{
18+
parent::__construct(Response::HTTP_FORBIDDEN, 'Invalid payload', $previous);
19+
}
20+
}

app/Http/Controllers/SecurePathController.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
namespace App\Http\Controllers;
1010

1111
use App\Enum\StorageDiskType;
12+
use App\Exceptions\SecurePaths\InvalidPayloadException;
1213
use App\Exceptions\SecurePaths\InvalidSignatureException;
1314
use App\Exceptions\SecurePaths\WrongPathException;
1415
use App\Models\Configs;
16+
use Illuminate\Contracts\Encryption\DecryptException;
1517
use Illuminate\Http\Request;
1618
use Illuminate\Routing\Controller;
1719
use Illuminate\Support\Facades\Crypt;
@@ -33,7 +35,11 @@ public function __invoke(Request $request, ?string $path)
3335
}
3436

3537
if (Configs::getValueAsBool('secure_image_link_enabled')) {
36-
$path = Crypt::decryptString($path);
38+
try {
39+
$path = Crypt::decryptString($path);
40+
} catch (DecryptException) {
41+
throw new InvalidPayloadException();
42+
}
3743
}
3844

3945
$file = Storage::disk(StorageDiskType::LOCAL->value)->path($path);

routes/web_v2.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,5 +68,9 @@
6868
->where('path', '.*')
6969
->middleware(['migration:complete']);
7070

71+
Route::get('image/{path}', SecurePathController::class)
72+
->name('image')
73+
->where('path', '.*');
74+
7175
// This route must be defined last because it is a catch all.
7276
Route::match(['get', 'post'], '{path}', HoneyPotController::class)->where('path', '.*');
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php
2+
3+
/**
4+
* SPDX-License-Identifier: MIT
5+
* Copyright (c) 2017-2018 Tobias Reich
6+
* Copyright (c) 2018-2025 LycheeOrg.
7+
*/
8+
9+
/**
10+
* We don't care for unhandled exceptions in tests.
11+
* It is the nature of a test to throw an exception.
12+
* Without this suppression we had 100+ Linter warning in this file which
13+
* don't help anything.
14+
*
15+
* @noinspection PhpDocMissingThrowsInspection
16+
* @noinspection PhpUnhandledExceptionInspection
17+
*/
18+
19+
namespace Tests\Feature_v2;
20+
21+
use App\Models\Configs;
22+
use Illuminate\Support\Facades\URL;
23+
use Tests\Feature_v2\Base\BaseApiWithDataTest;
24+
25+
class SecureImageLinksTest extends BaseApiWithDataTest
26+
{
27+
public function setUp(): void
28+
{
29+
parent::setUp();
30+
Configs::set('temporary_image_link_enabled', '1');
31+
Configs::invalidateCache();
32+
}
33+
34+
public function tearDown(): void
35+
{
36+
Configs::set('temporary_image_link_enabled', '0');
37+
Configs::set('secure_image_link_enabled', '0');
38+
Configs::invalidateCache();
39+
parent::tearDown();
40+
}
41+
42+
public function testTemporaryImage(): void
43+
{
44+
$response = $this->getJsonWithData('Album', ['album_id' => $this->album4->id]);
45+
$this->assertOk($response);
46+
$url = $response->json('resource.photos.0.size_variants.medium.url');
47+
$this->assertStringContainsString('/image/medium/', $url);
48+
49+
$response = $this->get($url);
50+
$this->assertNotFound($response);
51+
$response->assertSeeText('File not found'); // We mocked the file !
52+
53+
$unsigned_url = explode('?', $url)[0];
54+
$response = $this->get($unsigned_url);
55+
$this->assertForbidden($response);
56+
}
57+
58+
public function testEncryptedImages(): void
59+
{
60+
Configs::set('secure_image_link_enabled', '1');
61+
Configs::invalidateCache();
62+
63+
$response = $this->getJsonWithData('Album', ['album_id' => $this->album4->id]);
64+
$this->assertOk($response);
65+
$url = $response->json('resource.photos.0.size_variants.medium.url');
66+
$this->assertStringContainsString('/image/', $url);
67+
68+
$response = $this->get($url);
69+
$this->assertNotFound($response);
70+
$response->assertSeeText('File not found'); // We mocked the file !
71+
72+
$unsigned_url = explode('?', $url)[0];
73+
$response = $this->get($unsigned_url);
74+
$this->assertForbidden($response);
75+
}
76+
77+
public function testBrokenEncryption(): void
78+
{
79+
Configs::set('secure_image_link_enabled', '1');
80+
Configs::invalidateCache();
81+
82+
$broken_url = URL::temporarySignedRoute('image', now()->addSeconds(10), ['path' => 'broken_path']);
83+
$response = $this->get($broken_url);
84+
$this->assertForbidden($response);
85+
$response->assertSeeText('Invalid payload');
86+
}
87+
}

0 commit comments

Comments
 (0)