diff --git a/lib/Service/ApiService.php b/lib/Service/ApiService.php index e9db5a2d12..3bf3027160 100644 --- a/lib/Service/ApiService.php +++ b/lib/Service/ApiService.php @@ -93,7 +93,12 @@ public function create($fileId = null, $filePath = null, $token = null, $guestNa return new DataResponse($this->l10n->t('This file cannot be displayed as download is disabled by the share'), 404); } } elseif ($fileId) { - $file = $this->documentService->getFileById($fileId); + try { + $file = $this->documentService->getFileById($fileId); + } catch (NotFoundException|NotPermittedException $e) { + $this->logger->error('No permission to access this file', [ 'exception' => $e ]); + return new DataResponse($this->l10n->t('No permission to access this file.'), Http::STATUS_NOT_FOUND); + } } else { return new DataResponse('No valid file argument provided', 500); } diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index 657ad2312f..b1825b2943 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -395,6 +395,7 @@ public function getFileForSession(Session $session, $shareToken) { /** * @throws NotFoundException + * @throws NotPermittedException */ public function getFileById($fileId, $userId = null): Node { $userId = $userId ?? $this->userId; @@ -428,7 +429,17 @@ public function getFileById($fileId, $userId = null): Node { return ($b->getPermissions() & Constants::PERMISSION_UPDATE) <=> ($a->getPermissions() & Constants::PERMISSION_UPDATE); }); - return array_shift($files); + $file = array_shift($files); + + if (!$file instanceof File) { + throw new NotFoundException(); + } + + if (($file->getPermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ) { + throw new NotPermittedException(); + } + + return $file; } /** diff --git a/tests/unit/Service/DocumentServiceTest.php b/tests/unit/Service/DocumentServiceTest.php new file mode 100644 index 0000000000..00d740216e --- /dev/null +++ b/tests/unit/Service/DocumentServiceTest.php @@ -0,0 +1,108 @@ +documentMapper = $this->createMock(DocumentMapper::class); + $this->setpMapper = $this->createMock(StepMapper::class); + $this->sessionMapper = $this->createMock(SessionMapper::class); + $this->appData = $this->createMock(IAppData::class); + $this->userId = 'admin'; + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->loggerInterface = $this->createMock(LoggerInterface::class); + $this->shareManager = $this->createMock(IManager::class); + $this->request = $this->createMock(IRequest::class); + $this->directManager = $this->createMock(\OCP\DirectEditing\IManager::class); + $this->lockingProvider = $this->createMock(ILockingProvider::class); + $this->lockManager = $this->createMock(ILockManager::class); + $this->userMountCache = $this->createMock(IUserMountCache::class); + + $this->documentService = new DocumentService( + $this->documentMapper, + $this->setpMapper, + $this->sessionMapper, + $this->appData, + $this->userId, + $this->rootFolder, + $this->cacheFactory, + $this->loggerInterface, + $this->shareManager, + $this->request, + $this->directManager, + $this->lockingProvider, + $this->lockManager, + $this->userMountCache, + ); + } + + public function testGetFileById() { + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + + $file = $this->createMock(\OCP\Files\File::class); + $file->method('getPermissions')->willReturn(Constants::PERMISSION_READ); + $userFolder->method('getById')->willReturn([$file]); + $actual = $this->documentService->getFileById(1234); + self::assertEquals($file, $actual); + } + + public function testGetFileByIdSortUpdatableFirst() { + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + + $file1 = $this->createMock(\OCP\Files\File::class); + $file1->method('getPermissions')->willReturn(Constants::PERMISSION_READ); + $file2 = $this->createMock(\OCP\Files\File::class); + $file2->method('getPermissions')->willReturn(Constants::PERMISSION_READ & Constants::PERMISSION_UPDATE); + $userFolder->method('getById')->willReturn([$file1, $file2]); + $actual = $this->documentService->getFileById(1234); + self::assertEquals($file2, $actual); + } + + public function testGetFileByIdNoRead() { + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + + $file = $this->createMock(\OCP\Files\File::class); + $file->method('getPermissions')->willReturn(Constants::PERMISSION_UPDATE); + $userFolder->method('getById')->willReturn([$file]); + $this->expectException(NotPermittedException::class); + $actual = $this->documentService->getFileById(1234); + } +}