upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-02-22 17:31:10 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

fix(files): Harden thumbnail endpoint

Browse source 
- Catch all thrown exceptions and handle in such a way you do not get
  information about forbidden files.
- Resepect download permissions of shares.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
This commit is contained in:
Ferdinand Thiessen 2025-01-26 16:56:00 +01:00
parent 823782b44d
commit da32af0185
No known key found for this signature in database
GPG key ID: 45FAE7268762B400
2 changed files with 104 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
apps/files/lib/Controller/ApiController.php
View file

@ -28,8 +28,11 @@ use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Http\StreamResponse;
use OCP\Files\File;
use OCP\Files\Folder;
use OCP\Files\InvalidPathException;
use OCP\Files\IRootFolder;
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\Files\Storage\ISharedStorage;
use OCP\IConfig;
use OCP\IL10N;
use OCP\IPreview;
@ -87,20 +90,28 @@ class ApiController extends Controller {
}
try {
$file = $this->userFolder->get($file);
if ($file instanceof Folder) {
$file = $this->userFolder?->get($file);
if ($file === null
|| !($file instanceof File)
|| ($file->getId() <= 0)
) {
throw new NotFoundException();
}
if ($file->getId() <= 0) {
return new DataResponse(['message' => 'File not found.'], Http::STATUS_NOT_FOUND);
// Validate the user is allowed to download the file (preview is some kind of download)
$storage = $file->getStorage();
if ($storage->instanceOfStorage(ISharedStorage::class)) {
/** @var ISharedStorage $storage */
$attributes = $storage->getShare()->getAttributes();
if ($attributes !== null && $attributes->getAttribute('permissions', 'download') === false) {
throw new NotFoundException();
}
}
/** @var File $file */
$preview = $this->previewManager->getPreview($file, $x, $y, true);
return new FileDisplayResponse($preview, Http::STATUS_OK, ['Content-Type' => $preview->getMimeType()]);
} catch (NotFoundException $e) {
} catch (NotFoundException|NotPermittedException|InvalidPathException) {
return new DataResponse(['message' => 'File not found.'], Http::STATUS_NOT_FOUND);
} catch (\Exception $e) {
return new DataResponse([], Http::STATUS_BAD_REQUEST);

89
apps/files/tests/Controller/ApiControllerTest.php
View file

@ -17,6 +17,8 @@ use OCP\Files\Folder;
use OCP\Files\IRootFolder;
use OCP\Files\NotFoundException;
use OCP\Files\SimpleFS\ISimpleFile;
use OCP\Files\Storage\ISharedStorage;
use OCP\Files\Storage\IStorage;
use OCP\Files\StorageNotAvailableException;
use OCP\IConfig;
use OCP\IL10N;
@ -24,7 +26,9 @@ use OCP\IPreview;
use OCP\IRequest;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Share\IAttributes;
use OCP\Share\IManager;
use OCP\Share\IShare;
use Psr\Log\LoggerInterface;
use Test\TestCase;
@ -170,9 +174,13 @@ class ApiControllerTest extends TestCase {
$this->assertEquals($expected, $this->apiController->getThumbnail(0, 0, ''));
}
public function testGetThumbnailInvalidImage() {
public function testGetThumbnailInvalidImage(): void {
$storage = $this->createMock(IStorage::class);
$storage->method('instanceOfStorage')->with(ISharedStorage::class)->willReturn(false);
$file = $this->createMock(File::class);
$file->method('getId')->willReturn(123);
$file->method('getStorage')->willReturn($storage);
$this->userFolder->method('get')
->with($this->equalTo('unknown.jpg'))
->willReturn($file);
@ -194,9 +202,86 @@ class ApiControllerTest extends TestCase {
$this->assertEquals($expected, $this->apiController->getThumbnail(10, 10, 'unknown.jpg'));
}
public function testGetThumbnail() {
public function testGetThumbnailSharedNoDownload(): void {
$attributes = $this->createMock(IAttributes::class);
$attributes->expects(self::once())
->method('getAttribute')
->with('permissions', 'download')
->willReturn(false);
$share = $this->createMock(IShare::class);
$share->expects(self::once())
->method('getAttributes')
->willReturn($attributes);
$storage = $this->createMock(ISharedStorage::class);
$storage->expects(self::once())
->method('instanceOfStorage')
->with(ISharedStorage::class)
->willReturn(true);
$storage->expects(self::once())
->method('getShare')
->willReturn($share);
$file = $this->createMock(File::class);
$file->method('getId')->willReturn(123);
$file->method('getStorage')->willReturn($storage);
$this->userFolder->method('get')
->with('unknown.jpg')
->willReturn($file);
$this->preview->expects($this->never())
->method('getPreview');
$expected = new DataResponse(['message' => 'File not found.'], Http::STATUS_NOT_FOUND);
$this->assertEquals($expected, $this->apiController->getThumbnail(10, 10, 'unknown.jpg'));
}
public function testGetThumbnailShared(): void {
$share = $this->createMock(IShare::class);
$share->expects(self::once())
->method('getAttributes')
->willReturn(null);
$storage = $this->createMock(ISharedStorage::class);
$storage->expects(self::once())
->method('instanceOfStorage')
->with(ISharedStorage::class)
->willReturn(true);
$storage->expects(self::once())
->method('getShare')
->willReturn($share);
$file = $this->createMock(File::class);
$file->method('getId')->willReturn(123);
$file->method('getStorage')->willReturn($storage);
$this->userFolder->method('get')
->with($this->equalTo('known.jpg'))
->willReturn($file);
$preview = $this->createMock(ISimpleFile::class);
$preview->method('getName')->willReturn('my name');
$preview->method('getMTime')->willReturn(42);
$this->preview->expects($this->once())
->method('getPreview')
->with($this->equalTo($file), 10, 10, true)
->willReturn($preview);
$ret = $this->apiController->getThumbnail(10, 10, 'known.jpg');
$this->assertEquals(Http::STATUS_OK, $ret->getStatus());
$this->assertInstanceOf(Http\FileDisplayResponse::class, $ret);
}
public function testGetThumbnail(): void {
$storage = $this->createMock(IStorage::class);
$storage->method('instanceOfStorage')->with(ISharedStorage::class)->willReturn(false);
$file = $this->createMock(File::class);
$file->method('getId')->willReturn(123);
$file->method('getStorage')->willReturn($storage);
$this->userFolder->method('get')
->with($this->equalTo('known.jpg'))
->willReturn($file);