fixup! enh: add backend check for download permission for cloud attac…

…hements Signed-off-by: Hamza Mahjoubi <hamzamahjoubi221@gmail.com>
nextcloud · Jun 3, 2024 · 0e71d88 · 0e71d88
1 parent 6648bce
commit 0e71d88
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 13 deletions.
diff --git a/lib/Service/Attachment/AttachmentService.php b/lib/Service/Attachment/AttachmentService.php
@@ -344,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
@@ -363,19 +380,8 @@ private function handleCloudAttachment(Account $account, array $attachment): ?in
 		if (!$file instanceof File) {
 			return null;
 		}
-
-		$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: ".$attachment['fileName']);
-				return null;
-
-			}
+		if(!$this->hasDownloadPermissions($file, $fileName)) {
+			return null;
 		}
 
 		try {

diff --git a/tests/Unit/Service/Attachment/AttachmentServiceTest.php b/tests/Unit/Service/Attachment/AttachmentServiceTest.php
@@ -41,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;
 
@@ -390,6 +392,56 @@ 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, [