Skip to content

Commit

Permalink
enh: add backend check for download permission for cloud attachements
Browse files Browse the repository at this point in the history
Signed-off-by: Hamza Mahjoubi <hamzamahjoubi221@gmail.com>
  • Loading branch information
hamza221 committed Jun 5, 2024
1 parent 30a8b30 commit f0e072e
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 1 deletion.
21 changes: 21 additions & 0 deletions lib/Service/Attachment/AttachmentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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());
Expand Down
56 changes: 55 additions & 1 deletion tests/Unit/Service/Attachment/AttachmentServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions tests/psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit f0e072e

Please sign in to comment.