diff --git a/lib/Service/ApiService.php b/lib/Service/ApiService.php index 5e74c876ff4..72cdeda3b66 100644 --- a/lib/Service/ApiService.php +++ b/lib/Service/ApiService.php @@ -91,8 +91,9 @@ public function create($fileId = null, $filePath = null, $token = null, $guestNa } elseif ($fileId) { try { $file = $this->documentService->getFileById($fileId); - } catch (NotFoundException $e) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + } 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', Http::STATUS_PRECONDITION_FAILED); diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index b2b2f52f865..372943913cd 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -461,6 +461,7 @@ public function getFileForSession(Session $session, ?string $shareToken = null): /** * @throws NotFoundException + * @throws NotPermittedException */ public function getFileById($fileId, $userId = null): File { $userId = $userId ?? $this->userId; @@ -494,7 +495,17 @@ public function getFileById($fileId, $userId = null): File { 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 00000000000..00d740216e3 --- /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); + } +}