Skip to content

Commit 638201b

Browse files
authored
Merge pull request #9709 from nextcloud/backport/9592/stable3.7
[stable3.7] enh: add backend check for download permission for cloud attachements
2 parents c77a806 + e23dcc4 commit 638201b

File tree

3 files changed

+84
-1
lines changed

3 files changed

+84
-1
lines changed

lib/Service/Attachment/AttachmentService.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
use finfo;
2929
use InvalidArgumentException;
30+
use OCA\Files_Sharing\SharedStorage;
3031
use OCA\Mail\Account;
3132
use OCA\Mail\Contracts\IAttachmentService;
3233
use OCA\Mail\Contracts\IMailManager;
@@ -343,6 +344,23 @@ private function handleForwardedAttachment(Account $account, array $attachment,
343344
return $localAttachment->getId();
344345
}
345346

347+
private function hasDownloadPermissions(File $file, string $fileName): bool {
348+
$storage = $file->getStorage();
349+
if ($storage->instanceOfStorage(SharedStorage::class)) {
350+
351+
/** @var SharedStorage $storage */
352+
$share = $storage->getShare();
353+
$attributes = $share->getAttributes();
354+
355+
if($attributes->getAttribute('permissions', 'download') === false) {
356+
$this->logger->warning("Could not create attachment, no download permission for file: ".$fileName);
357+
return false;
358+
359+
}
360+
}
361+
return true;
362+
}
363+
346364
/**
347365
* @param Account $account
348366
* @param array $attachment
@@ -362,6 +380,9 @@ private function handleCloudAttachment(Account $account, array $attachment): ?in
362380
if (!$file instanceof File) {
363381
return null;
364382
}
383+
if(!$this->hasDownloadPermissions($file, $fileName)) {
384+
return null;
385+
}
365386

366387
try {
367388
$localAttachment = $this->addFileFromString($account->getUserId(), $file->getName(), $file->getMimeType(), $file->getContent());

tests/Unit/Service/Attachment/AttachmentServiceTest.php

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use ChristophWurst\Nextcloud\Testing\TestCase;
2626
use Horde_Imap_Client_Socket;
2727
use OC\Files\Node\File;
28+
use OCA\Files_Sharing\SharedStorage;
2829
use OCA\Mail\Account;
2930
use OCA\Mail\Contracts\IMailManager;
3031
use OCA\Mail\Db\LocalAttachment;
@@ -40,6 +41,8 @@
4041
use OCP\AppFramework\Db\DoesNotExistException;
4142
use OCP\Files\Folder;
4243
use OCP\Files\NotPermittedException;
44+
use OCP\Share\IAttributes;
45+
use OCP\Share\IShare;
4346
use PHPUnit\Framework\MockObject\MockObject;
4447
use Psr\Log\LoggerInterface;
4548

@@ -389,12 +392,63 @@ public function testHandleAttachmentsForwardedAttachment(): void {
389392
$this->service->handleAttachments($account, [$attachments], $client);
390393
}
391394

395+
public function testHandleAttachmentsCloudAttachmentNoDownloadPermission(): void {
396+
$userId = 'linus';
397+
$storage = $this->createMock(SharedStorage::class);
398+
$storage->expects(self::once())
399+
->method('instanceOfStorage')
400+
->with(SharedStorage::class)
401+
->willReturn(true);
402+
$share = $this->createMock(IShare::class);
403+
$attributes = $this->createMock(IAttributes::class);
404+
$attributes->expects(self::once())
405+
->method('getAttribute')
406+
->with('permissions', 'download')
407+
->willReturn(false);
408+
$share->expects(self::once())
409+
->method('getAttributes')
410+
->willReturn($attributes);
411+
$storage->expects(self::once())
412+
->method('getShare')
413+
->willReturn($share);
414+
415+
$file = $this->createConfiguredMock(File::class, [
416+
'getName' => 'cat.jpg',
417+
'getMimeType' => 'text/plain',
418+
'getContent' => 'sjdhfkjsdhfkjsdhfkjdshfjhdskfjhds',
419+
'getStorage' => $storage
420+
]);
421+
$account = $this->createConfiguredMock(Account::class, [
422+
'getUserId' => $userId
423+
]);
424+
$client = $this->createMock(Horde_Imap_Client_Socket::class);
425+
$attachments = [
426+
'type' => 'cloud',
427+
'messageId' => 999,
428+
'fileName' => 'cat.jpg',
429+
'mimeType' => 'text/plain',
430+
];
431+
$this->userFolder->expects(self::once())
432+
->method('nodeExists')
433+
->with('cat.jpg')
434+
->willReturn(true);
435+
$this->userFolder->expects(self::once())
436+
->method('get')
437+
->with('cat.jpg')
438+
->willReturn($file);
439+
440+
$result = $this->service->handleAttachments($account, [$attachments], $client);
441+
$this->assertEquals([], $result);
442+
443+
}
444+
392445
public function testHandleAttachmentsCloudAttachment(): void {
393446
$userId = 'linus';
394447
$file = $this->createConfiguredMock(File::class, [
395448
'getName' => 'cat.jpg',
396449
'getMimeType' => 'text/plain',
397-
'getContent' => 'sjdhfkjsdhfkjsdhfkjdshfjhdskfjhds'
450+
'getContent' => 'sjdhfkjsdhfkjsdhfkjdshfjhdskfjhds',
451+
'getStorage' => $this->createMock(SharedStorage::class)
398452
]);
399453
$account = $this->createConfiguredMock(Account::class, [
400454
'getUserId' => $userId

tests/psalm-baseline.xml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@
4141
<code>IAPIWidgetV2</code>
4242
</UndefinedClass>
4343
</file>
44+
<file src="lib/Service/Attachment/AttachmentService.php">
45+
<UndefinedDocblockClass occurrences="2">
46+
<code>OCA\Files_Sharing\SharedStorage</code>
47+
</UndefinedDocblockClass>
48+
<UndefinedClass occurrences="1">
49+
<code>SharedStorage</code>
50+
</UndefinedClass>
51+
</file>
4452
<file src="lib/Dashboard/UnreadMailWidgetV2.php">
4553
<MissingDependency>
4654
<code>MailWidgetV2</code>

0 commit comments

Comments
 (0)