Merge pull request #9592 from nextcloud/enh/backend-check-for-download-permission

enh: add backend check for download permission for cloud attachements

Author: st3iny

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());

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

tests/psalm-baseline.xml

@@ -41,6 +41,14 @@
       <code>IAPIWidgetV2</code>
     </UndefinedClass>
   </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>
   <file src="lib/Dashboard/UnreadMailWidgetV2.php">
     <MissingDependency>
       <code>MailWidgetV2</code>