enh: add backend check for download permission for cloud attachements

Signed-off-by: Hamza Mahjoubi <[email protected]>
nextcloud · Jun 25, 2024 · e1b7f81 · e1b7f81
1 parent e9606a1
commit e1b7f81
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 1 deletion.
diff --git a/lib/Service/Attachment/AttachmentService.php b/lib/Service/Attachment/AttachmentService.php
@@ -26,6 +26,7 @@
 
 use finfo;
 use InvalidArgumentException;
+use OCA\Files_Sharing\SharedStorage;
 use OCA\Mail\Account;
 use OCA\Mail\Contracts\IAttachmentService;
 use OCA\Mail\Contracts\IMailManager;
@@ -345,6 +346,21 @@ 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
@@ -364,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
@@ -24,6 +24,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;
@@ -39,6 +40,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;
 
@@ -388,12 +391,62 @@ 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
@@ -33,6 +33,7 @@
       <code>Command</code>
     </UndefinedClass>
   </file>
+
   <file src="lib/Command/CleanUp.php">
     <UndefinedClass occurrences="1">
       <code>Command</code>
@@ -377,4 +378,12 @@
 	  <code>OutboxMessageCreatedEvent</code>
 	</MissingDependency>
   </file>
+  <file src="lib/Service/Attachment/AttachmentService.php">
+    <UndefinedDocblockClass occurrences="2">
+      <code>OCA\Files_Sharing\SharedStorage</code>
+    </UndefinedDocblockClass>
+    <UndefinedClass occurrences="1">
+      <code>SharedStorage</code>
+    </UndefinedClass>
+  </file>
 </files>