From f0e072edaadfd04ccd7a3dd28fedf1599ece3faa Mon Sep 17 00:00:00 2001 From: Hamza Mahjoubi Date: Mon, 22 Apr 2024 20:14:26 +0200 Subject: [PATCH] enh: add backend check for download permission for cloud attachements Signed-off-by: Hamza Mahjoubi --- lib/Service/Attachment/AttachmentService.php | 21 +++++++ .../Attachment/AttachmentServiceTest.php | 56 ++++++++++++++++++- tests/psalm-baseline.xml | 8 +++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/lib/Service/Attachment/AttachmentService.php b/lib/Service/Attachment/AttachmentService.php index 717312db8a..f073a2f8fb 100644 --- a/lib/Service/Attachment/AttachmentService.php +++ b/lib/Service/Attachment/AttachmentService.php @@ -27,6 +27,7 @@ use finfo; use InvalidArgumentException; +use OCA\Files_Sharing\SharedStorage; use OCA\Mail\Account; use OCA\Mail\Contracts\IAttachmentService; use OCA\Mail\Contracts\IMailManager; @@ -343,6 +344,23 @@ private function handleForwardedAttachment(Account $account, array $attachment, return $localAttachment->getId(); } + private function hasDownloadPermissions(File $file, string $fileName): bool { + $storage = $file->getStorage(); + if ($storage->instanceOfStorage(SharedStorage::class)) { + + /** @var SharedStorage $storage */ + $share = $storage->getShare(); + $attributes = $share->getAttributes(); + + if($attributes->getAttribute('permissions', 'download') === false) { + $this->logger->warning("Could not create attachment, no download permission for file: ".$fileName); + return false; + + } + } + return true; + } + /** * @param Account $account * @param array $attachment @@ -362,6 +380,9 @@ private function handleCloudAttachment(Account $account, array $attachment): ?in if (!$file instanceof File) { return null; } + if(!$this->hasDownloadPermissions($file, $fileName)) { + return null; + } try { $localAttachment = $this->addFileFromString($account->getUserId(), $file->getName(), $file->getMimeType(), $file->getContent()); diff --git a/tests/Unit/Service/Attachment/AttachmentServiceTest.php b/tests/Unit/Service/Attachment/AttachmentServiceTest.php index b186a63e09..01f63483eb 100644 --- a/tests/Unit/Service/Attachment/AttachmentServiceTest.php +++ b/tests/Unit/Service/Attachment/AttachmentServiceTest.php @@ -25,6 +25,7 @@ use ChristophWurst\Nextcloud\Testing\TestCase; use Horde_Imap_Client_Socket; use OC\Files\Node\File; +use OCA\Files_Sharing\SharedStorage; use OCA\Mail\Account; use OCA\Mail\Contracts\IMailManager; use OCA\Mail\Db\LocalAttachment; @@ -40,6 +41,8 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\Files\Folder; use OCP\Files\NotPermittedException; +use OCP\Share\IAttributes; +use OCP\Share\IShare; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -389,12 +392,63 @@ public function testHandleAttachmentsForwardedAttachment(): void { $this->service->handleAttachments($account, [$attachments], $client); } + public function testHandleAttachmentsCloudAttachmentNoDownloadPermission(): void { + $userId = 'linus'; + $storage = $this->createMock(SharedStorage::class); + $storage->expects(self::once()) + ->method('instanceOfStorage') + ->with(SharedStorage::class) + ->willReturn(true); + $share = $this->createMock(IShare::class); + $attributes = $this->createMock(IAttributes::class); + $attributes->expects(self::once()) + ->method('getAttribute') + ->with('permissions', 'download') + ->willReturn(false); + $share->expects(self::once()) + ->method('getAttributes') + ->willReturn($attributes); + $storage->expects(self::once()) + ->method('getShare') + ->willReturn($share); + + $file = $this->createConfiguredMock(File::class, [ + 'getName' => 'cat.jpg', + 'getMimeType' => 'text/plain', + 'getContent' => 'sjdhfkjsdhfkjsdhfkjdshfjhdskfjhds', + 'getStorage' => $storage + ]); + $account = $this->createConfiguredMock(Account::class, [ + 'getUserId' => $userId + ]); + $client = $this->createMock(Horde_Imap_Client_Socket::class); + $attachments = [ + 'type' => 'cloud', + 'messageId' => 999, + 'fileName' => 'cat.jpg', + 'mimeType' => 'text/plain', + ]; + $this->userFolder->expects(self::once()) + ->method('nodeExists') + ->with('cat.jpg') + ->willReturn(true); + $this->userFolder->expects(self::once()) + ->method('get') + ->with('cat.jpg') + ->willReturn($file); + + $result = $this->service->handleAttachments($account, [$attachments], $client); + $this->assertEquals([], $result); + + } + public function testHandleAttachmentsCloudAttachment(): void { $userId = 'linus'; $file = $this->createConfiguredMock(File::class, [ 'getName' => 'cat.jpg', 'getMimeType' => 'text/plain', - 'getContent' => 'sjdhfkjsdhfkjsdhfkjdshfjhdskfjhds' + 'getContent' => 'sjdhfkjsdhfkjsdhfkjdshfjhdskfjhds', + 'getStorage' => $this->createMock(SharedStorage::class) ]); $account = $this->createConfiguredMock(Account::class, [ 'getUserId' => $userId diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index c787ffca25..9c373d1676 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -41,6 +41,14 @@ IAPIWidgetV2 + + + OCA\Files_Sharing\SharedStorage + + + SharedStorage + + MailWidgetV2