Merge pull request #4179 from nextcloud/backport/4173/stable25

lib/Service/AttachmentService.php

@@ -194,7 +194,7 @@ class AttachmentService {
 	 * @throws BadRequestException
 	 */
 	public function create($cardId, $type, $data) {
-		$this->attachmentServiceValidator->check(compact('cardId', 'type', 'data'));
+		$this->attachmentServiceValidator->check(compact('cardId', 'type'));
 
 		$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT);
 

lib/Service/FilesAppService.php

@@ -138,7 +138,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 
 	public function extendData(Attachment $attachment) {
 		$userFolder = $this->rootFolder->getUserFolder($this->userId);
-		$share = $this->shareProvider->getShareById($attachment->getId());
+		$share = $this->getShareForAttachment($attachment);
 		$files = $userFolder->getById($share->getNode()->getId());
 		if (count($files) === 0) {
 			return $attachment;
@@ -161,7 +161,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 		// Problem: Folders
 		/** @psalm-suppress InvalidCatch */
 		try {
-			$share = $this->shareProvider->getShareById($attachment->getId());
+			$share = $this->getShareForAttachment($attachment);
 		} catch (ShareNotFound $e) {
 			throw new NotFoundException('File not found');
 		}
@@ -241,7 +241,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 	}
 
 	public function update(Attachment $attachment) {
-		$share = $this->shareProvider->getShareById($attachment->getId());
+		$share = $this->getShareForAttachment($attachment);
 		$target = $share->getNode();
 		$file = $this->getUploadedFile();
 		$fileName = $file['name'];
@@ -258,8 +258,13 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 		return $attachment;
 	}
 
+	/**
+	 * @throws NoPermissionException
+	 * @throws NotFoundException
+	 * @throws ShareNotFound
+	 */
 	public function delete(Attachment $attachment) {
-		$share = $this->shareProvider->getShareById($attachment->getId());
+		$share = $this->getShareForAttachment($attachment);
 		$file = $share->getNode();
 		$attachment->setData($file->getName());
 
@@ -282,4 +287,21 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 	public function markAsDeleted(Attachment $attachment) {
 		throw new \Exception('Not implemented');
 	}
+
+	/**
+	 * @throws NoPermissionException
+	 */
+	private function getShareForAttachment(Attachment $attachment): IShare {
+		try {
+			$share = $this->shareProvider->getShareById($attachment->getId());
+		} catch (ShareNotFound $e) {
+			throw new NoPermissionException('No permission to access the attachment from the card');
+		}
+
+		if ((int)$share->getSharedWith() !== (int)$attachment->getCardId()) {
+			throw new NoPermissionException('No permission to access the attachment from the card');
+		}
+
+		return $share;
+	}
 }

tests/integration/config/behat.yml

@@ -9,4 +9,5 @@ default:
         - RequestContext
         - BoardContext
         - CommentContext
+        - AttachmentContext
         - SearchContext

tests/integration/features/bootstrap/AttachmentContext.php

@@ -0,0 +1,90 @@
+<?php
+
+use Behat\Behat\Context\Context;
+use Behat\Behat\Hook\Scope\BeforeScenarioScope;
+use PHPUnit\Framework\Assert;
+
+require_once __DIR__ . '/../../vendor/autoload.php';
+
+class AttachmentContext implements Context {
+	use RequestTrait;
+
+	/** @var BoardContext */
+	protected $boardContext;
+	/** @var ServerContext */
+	private $serverContext;
+
+	protected $lastAttachment = null;
+	protected array $rememberedAttachments = [];
+
+	/** @BeforeScenario */
+	public function gatherContexts(BeforeScenarioScope $scope) {
+		$environment = $scope->getEnvironment();
+
+		$this->boardContext = $environment->getContext('BoardContext');
+		$this->serverContext = $environment->getContext('ServerContext');
+	}
+
+	public function delete(int $cardId, int $attachmentId) {
+		$this->requestContext->sendPlainRequest('DELETE', '/index.php/apps/deck/cards/' . $cardId . '/attachment/file:' . $attachmentId);
+		$response = $this->requestContext->getResponseBodyFromJson();
+	}
+
+	/**
+	 * @When deleting the attachment :attachmentReference for the card :cardReference
+	 */
+	public function deletingTheAttachmentForTheCard($attachmentReference, $cardReference) {
+		$cardId = $this->boardContext->getRememberedCard($cardReference)['id'] ?? null;
+		$attachmentId = $this->getRememberedAttachment($attachmentReference)['id'] ?? null;
+		Assert::assertNotNull($cardId, 'Card needs to be available');
+		Assert::assertNotNull($attachmentId, 'Attachment needs to be available');
+		$this->delete($cardId, $attachmentId);
+	}
+
+	/**
+	 * @Given /^uploads an attachment to the last used card$/
+	 */
+	public function uploadsAnAttachmentToTheLastUsedCard() {
+		$cardId = $this->boardContext->getLastUsedCard()['id'] ?? null;
+		Assert::assertNotNull($cardId, 'Card data is not set');
+
+		$this->requestContext->sendPlainRequest('POST', '/index.php/apps/deck/cards/' . $cardId . '/attachment', [
+			'multipart' => [
+				[
+					'name' => 'file',
+					'contents' => 'Example content',
+					'filename' => 'test.txt',
+				],
+				[
+					'name' => 'type',
+					'contents' => 'file'
+				]
+			]
+		]);
+	}
+
+	/**
+	 * @Given remember the last attachment as :arg1
+	 */
+	public function rememberTheLastAttachmentAs($arg1) {
+		$this->requestContext->theResponseShouldHaveStatusCode(200);
+		$this->lastAttachment = $this->requestContext->getResponseBodyFromJson();
+		$this->rememberedAttachments[$arg1] = $this->lastAttachment;
+	}
+
+	public function getRememberedAttachment($name) {
+		return $this->rememberedAttachments[$name] ?? null;
+	}
+
+	/**
+	 * @When fetching the attachment :attachmentReference for the card :cardReference
+	 */
+	public function fetchingTheAttachmentForTheCard($attachmentReference, $cardReference) {
+		$cardId = $this->boardContext->getRememberedCard($cardReference)['id'] ?? null;
+		$attachmentId = $this->getRememberedAttachment($attachmentReference)['id'] ?? null;
+		Assert::assertNotNull($cardId, 'Card needs to be available');
+		Assert::assertNotNull($attachmentId, 'Attachment needs to be available');
+
+		$this->requestContext->sendPlainRequest('GET', '/index.php/apps/deck/cards/' . $cardId . '/attachment/file:' . $attachmentId);
+	}
+}

tests/integration/features/bootstrap/BoardContext.php

@@ -16,6 +16,7 @@ class BoardContext implements Context {
 	private $stack = null;
 	/** @var array last card response */
 	private $card = null;
+	private array $storedCards = [];
 
 	/** @var ServerContext */
 	private $serverContext;
@@ -31,6 +32,15 @@ class BoardContext implements Context {
 		return $this->card;
 	}
 
+	/**
+	 * @Given /^creates a board with example content$/
+	 */
+	public function createExampleContent() {
+		$this->createsABoardNamedWithColor('Example board', 'ff0000');
+		$this->createAStackNamed('ToDo');
+		$this->createACardNamed('My example card');
+	}
+
 	/**
 	 * @Given /^creates a board named "([^"]*)" with color "([^"]*)"$/
 	 */
@@ -232,4 +242,15 @@ class BoardContext implements Context {
 		$this->requestContext->sendJSONrequest('POST', '/index.php/apps/deck/cards/' . $this->card['id'] .'/label/' . $label['id']);
 		$this->requestContext->getResponse()->getBody()->seek(0);
 	}
+
+	/**
+	 * @When remember the last card as :arg1
+	 */
+	public function rememberTheLastCardAs($arg1) {
+		$this->storedCards[$arg1] = $this->getLastUsedCard();
+	}
+
+	public function getRememberedCard($arg1) {
+		return $this->storedCards[$arg1] ?? null;
+	}
 }

tests/integration/features/bootstrap/RequestContext.php

@@ -134,7 +134,36 @@ class RequestContext implements Context {
 		}
 	}
 
+	public function sendPlainRequest(string $method, $uri = '', array $options = []) {
+		$client = new Client;
+		try {
+			$this->response = $client->request(
+				$method,
+				rtrim($this->serverContext->getBaseUrl(), '/') . '/' . ltrim($uri, '/'),
+				array_merge(
+					[
+						'cookies' => $this->serverContext->getCookieJar(),
+						'headers' => [
+							'requesttoken' => $this->serverContext->getReqestToken(),
+							'OCS-APIREQUEST' => 'true',
+							'Accept' => 'application/json'
+						]
+					],
+					$options,
+				)
+			);
+		} catch (ClientException $e) {
+			$this->response = $e->getResponse();
+		}
+	}
+
+
 	public function getResponse(): ResponseInterface {
 		return $this->response;
 	}
+
+	public function getResponseBodyFromJson() {
+		$this->getResponse()->getBody()->seek(0);
+		return json_decode((string)$this->getResponse()->getBody(), true);
+	}
 }

tests/integration/features/bootstrap/ServerContext.php

@@ -40,6 +40,7 @@ class ServerContext implements Context {
 	}
 
 	public function getCookieJar(): CookieJar {
+		echo $this->currentUser;
 		return $this->cookieJar;
 	}
 

tests/integration/features/sharing.feature

@@ -135,3 +135,31 @@ Feature: File sharing
     When Deleting last share
     And as "user2" the file "/Deck/user0-file2.txt" does not exist
     And as "user3" the file "/Deck/user0-file2.txt" does not exist
+
+  Scenario: Remove a share through the deck API
+	  Given acting as user "user0"
+	  When creates a board with example content
+	  And remember the last card as "user0-card"
+	  And uploads an attachment to the last used card
+	  And remember the last attachment as "user0-attachment"
+
+	  Given acting as user "user1"
+	  When creates a board with example content
+	  And remember the last card as "user1-card"
+	  And uploads an attachment to the last used card
+	  And remember the last attachment as "user1-attachment"
+
+	  Given acting as user "user0"
+	  When fetching the attachment "user1-attachment" for the card "user0-card"
+	  Then the response should have a status code 403
+	  When deleting the attachment "user1-attachment" for the card "user0-card"
+	  Then the response should have a status code 403
+
+	  When fetching the attachment "user0-attachment" for the card "user0-card"
+	  Then the response should have a status code 200
+	  When deleting the attachment "user0-attachment" for the card "user0-card"
+	  Then the response should have a status code 200
+
+	  Given acting as user "user1"
+	  When deleting the attachment "user1-attachment" for the card "user1-card"
+	  Then the response should have a status code 200