Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions lib/Controller/PublicFileHandlingController.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use OCP\Files\ForbiddenException;
use OCP\Files\GenericFileException;
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\IL10N;
use OCP\IRequest;
use OCP\ISession;
Expand Down Expand Up @@ -113,15 +114,15 @@ public function load($token) {

$fileNode = $node;
if ($node instanceof Folder) {
$dir = $this->request->getParam('dir');
$filename = $this->request->getParam('filename');
$fullpath = trim($dir.$filename);
if (!$fullpath) {
$dir = (string)$this->request->getParam('dir');
$filename = (string)$this->request->getParam('filename');
if ($filename === '') {
return new DataResponse(['message' => $this->l->t('File not found')], Http::STATUS_NOT_FOUND);
}
$fullpath = trim(rtrim($dir, '/') . '/' . $filename);
try {
$fileNode = $node->get($fullpath);
} catch (NotFoundException $e) {
} catch (NotFoundException|NotPermittedException $e) {
return new DataResponse(['message' => $this->l->t('File not found')], Http::STATUS_NOT_FOUND);
}
}
Expand Down Expand Up @@ -200,7 +201,7 @@ public function save($token, $filecontents, $path, $mtime) {
}
try {
$file = $node->get($fullpath);
} catch (NotFoundException $e) {
} catch (NotFoundException|NotPermittedException $e) {
return new DataResponse(['message' => $this->l->t('File not found')], Http::STATUS_NOT_FOUND);
}
}
Expand Down
43 changes: 39 additions & 4 deletions tests/Unit/Controller/PublicFileHandlingControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use OCP\Constants;
use OCP\Files\File;
use OCP\Files\Folder;
use OCP\Files\NotPermittedException;
use OCP\IL10N;
use OCP\IRequest;
use OCP\ISession;
Expand All @@ -31,13 +32,17 @@ class PublicFileHandlingControllerTest extends TestCase {
protected function setUp(): void {
parent::setUp();

$this->shareManager = $this->createMock(IManager::class);
$this->session = $this->createMock(ISession::class);
$this->createController('/', 'secret.km');
}

private function createController(string $dir, string $filename): void {
$request = $this->createMock(IRequest::class);
$request->method('getParam')->willReturnMap([
['dir', null, '/'],
['filename', null, 'secret.km'],
['dir', null, $dir],
['filename', null, $filename],
]);
$this->shareManager = $this->createMock(IManager::class);
$this->session = $this->createMock(ISession::class);
$l10n = $this->createMock(IL10N::class);
$l10n->method('t')->willReturnArgument(0);

Expand Down Expand Up @@ -72,6 +77,26 @@ public function testLoadFromReadableShare(): void {
$this->assertSame(base64_encode('{"root":{}}'), $response->getData()['filecontents']);
}

public function testLoadFromSubfolderOfShare(): void {
$this->createController('/Sub folder', 'notes.km');
$file = $this->createMock(File::class);
$file->method('getContent')->willReturn('{"root":{}}');
$folder = $this->createMock(Folder::class);
$folder->method('get')->with('/Sub folder/notes.km')->willReturn($file);
$this->mockShare(Constants::PERMISSION_READ, $folder);

$this->assertSame(Http::STATUS_OK, $this->controller->load('token')->getStatus());
}

public function testLoadOutsideOfShareIsNotFound(): void {
$this->createController('/../', 'notes.km');
$folder = $this->createMock(Folder::class);
$folder->method('get')->willThrowException(new NotPermittedException());
$this->mockShare(Constants::PERMISSION_READ, $folder);

$this->assertSame(Http::STATUS_NOT_FOUND, $this->controller->load('token')->getStatus());
}

public function testLoadFromPasswordProtectedShareWithoutSessionAuthIsRefused(): void {
$this->mockShare(password: 'secret', id: '42');
$this->session->method('get')->with('public_link_authenticated')->willReturn(['1', '2']);
Expand Down Expand Up @@ -99,6 +124,16 @@ public function testSaveFromPasswordProtectedShareWithoutSessionAuthIsRefused():
$this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus());
}

public function testSaveOutsideOfShareIsNotFound(): void {
$folder = $this->createMock(Folder::class);
$folder->method('get')->willThrowException(new NotPermittedException());
$this->mockShare(Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE, $folder);

$response = $this->controller->save('token', 'data', '/../notes.km', 123);

$this->assertSame(Http::STATUS_NOT_FOUND, $response->getStatus());
}

private function mockShare(int $permissions = 0, ?Folder $node = null, ?string $password = null, string $id = '1'): IShare&MockObject {
$share = $this->createMock(IShare::class);
$share->method('getPassword')->willReturn($password);
Expand Down