From 116e124b2d74556ef24839f70821bd76a4b4f84d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 4 Jan 2024 00:30:41 +0100 Subject: [PATCH 1/8] tests: Add integration tests for deleted boards/cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- tests/integration/features/acl.feature | 50 ++++++++++ .../features/bootstrap/AttachmentContext.php | 10 ++ .../features/bootstrap/BoardContext.php | 35 ++++++- .../features/bootstrap/CommentContext.php | 31 ++++++ .../features/bootstrap/ServerContext.php | 8 +- tests/integration/features/decks.feature | 98 +++++++++++++++++++ 6 files changed, 226 insertions(+), 6 deletions(-) diff --git a/tests/integration/features/acl.feature b/tests/integration/features/acl.feature index 51cb1be3b..b7bf2743c 100644 --- a/tests/integration/features/acl.feature +++ b/tests/integration/features/acl.feature @@ -90,3 +90,53 @@ Feature: acl And the current user should not have "edit" permissions on the board And the current user should have "share" permissions on the board And the current user should not have "manage" permissions on the board + + Scenario: Share a board multiple times + Given Logging in using web as "user0" + And creates a board named "Double shared board" with color "ff0000" + And shares the board with user "user1" + And shares the board with group "group1" + And creates a board named "Single shared board" with color "00ff00" + And shares the board with user "user1" + When Logging in using web as "user1" + And fetching the board list + Then the response should have a status code "200" + And the response should be a list of objects + And the response should contain an element with the properties + | property | value | + | title | Double shared board | + + + Scenario: Deleted board is inaccessible to share recipients + Given acting as user "user0" + When creates a board with example content + And remember the last card as "user0-card" + When post a comment with content "hello comment" on the card + And uploads an attachment to the last used card + And remember the last attachment as "user0-attachment" + And shares the board with user "user1" + Then the HTTP status code should be "200" + And delete the board + + Given acting as user "user1" + When fetching the attachments for the card "user0-card" + Then the response should have a status code 403 + + When get the comments on the card + Then the response should have a status code 403 + + When update a comment with content "hello deleted" on the card + Then the response should have a status code 403 + + When delete the comment on the card + Then the response should have a status code 403 + # 644 + When post a comment with content "hello deleted" on the card + Then the response should have a status code 403 + + When get the card details + 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 403 + When deleting the attachment "user0-attachment" for the card "user0-card" + Then the response should have a status code 403 diff --git a/tests/integration/features/bootstrap/AttachmentContext.php b/tests/integration/features/bootstrap/AttachmentContext.php index 051789aec..84ef35048 100644 --- a/tests/integration/features/bootstrap/AttachmentContext.php +++ b/tests/integration/features/bootstrap/AttachmentContext.php @@ -87,4 +87,14 @@ public function fetchingTheAttachmentForTheCard($attachmentReference, $cardRefer $this->requestContext->sendPlainRequest('GET', '/index.php/apps/deck/cards/' . $cardId . '/attachment/file:' . $attachmentId); } + + /** + * @When fetching the attachments for the card :cardReference + */ + public function fetchingTheAttachmentsForTheCard($cardReference) { + $cardId = $this->boardContext->getRememberedCard($cardReference)['id'] ?? null; + Assert::assertNotNull($cardId, 'Card needs to be available'); + + $this->requestContext->sendPlainRequest('GET', '/index.php/apps/deck/cards/' . $cardId . '/attachments'); + } } diff --git a/tests/integration/features/bootstrap/BoardContext.php b/tests/integration/features/bootstrap/BoardContext.php index a423acab4..d29ed1524 100644 --- a/tests/integration/features/bootstrap/BoardContext.php +++ b/tests/integration/features/bootstrap/BoardContext.php @@ -204,7 +204,9 @@ public function setTheDescriptionTo($description) { ['description' => $description] )); $this->requestContext->getResponse()->getBody()->seek(0); - $this->card = json_decode((string)$this->getResponse()->getBody(), true); + if ($this->requestContext->getResponse()->getStatusCode() === 200) { + $this->card = json_decode((string)$this->getResponse()->getBody(), true); + } } /** @@ -216,7 +218,22 @@ public function setCardAttribute($attribute, $value) { [$attribute => $value] )); $this->requestContext->getResponse()->getBody()->seek(0); - $this->card = json_decode((string)$this->getResponse()->getBody(), true); + if ($this->requestContext->getResponse()->getStatusCode() === 200) { + $this->card = json_decode((string)$this->getResponse()->getBody(), true); + } + } + + /** + * @Given /^get the card details$/ + */ + public function getCard() { + $this->requestContext->sendJSONrequest('GET', '/index.php/apps/deck/cards/' . $this->card['id'], array_merge( + $this->card + )); + $this->requestContext->getResponse()->getBody()->seek(0); + if ($this->requestContext->getResponse()->getStatusCode() === 200) { + $this->card = json_decode((string)$this->getResponse()->getBody(), true); + } } /** @@ -271,4 +288,18 @@ public function rememberTheLastCardAs($arg1) { public function getRememberedCard($arg1) { return $this->storedCards[$arg1] ?? null; } + + /** + * @Given /^delete the card$/ + */ + public function deleteTheCard() { + $this->requestContext->sendJSONrequest('DELETE', '/index.php/apps/deck/cards/' . $this->card['id']); + } + + /** + * @Given /^delete the board/ + */ + public function deleteTheBoard() { + $this->requestContext->sendJSONrequest('DELETE', '/index.php/apps/deck/boards/' . $this->board['id']); + } } diff --git a/tests/integration/features/bootstrap/CommentContext.php b/tests/integration/features/bootstrap/CommentContext.php index 40b6f4e45..92bc9a347 100644 --- a/tests/integration/features/bootstrap/CommentContext.php +++ b/tests/integration/features/bootstrap/CommentContext.php @@ -11,6 +11,8 @@ class CommentContext implements Context { /** @var BoardContext */ protected $boardContext; + private $lastComment = null; + /** @BeforeScenario */ public function gatherContexts(BeforeScenarioScope $scope) { $environment = $scope->getEnvironment(); @@ -27,5 +29,34 @@ public function postACommentWithContentOnTheCard($content) { 'message' => $content, 'parentId' => null ]); + $this->lastComment = $this->requestContext->getResponseBodyFromJson()['ocs']['data'] ?? null; + } + + /** + * @Given /^get the comments on the card$/ + */ + public function getCommentsOnTheCard() { + $card = $this->boardContext->getLastUsedCard(); + $this->requestContext->sendOCSRequest('GET', '/apps/deck/api/v1.0/cards/' . $card['id'] . '/comments'); + } + + /** + * @When /^update a comment with content "([^"]*)" on the card$/ + */ + public function updateACommentWithContentOnTheCard($content) { + $card = $this->boardContext->getLastUsedCard(); + $this->requestContext->sendOCSRequest('PUT', '/apps/deck/api/v1.0/cards/' . $card['id'] . '/comments/'. $this->lastComment['id'], [ + 'message' => $content, + 'parentId' => null + ]); + } + + /** + * @When /^delete the comment on the card$/ + */ + public function deleteTheCommentOnTheCard() { + $card = $this->boardContext->getLastUsedCard(); + $this->requestContext->sendOCSRequest('DELETE', '/apps/deck/api/v1.0/cards/' . $card['id'] . '/comments/'. $this->lastComment['id']); } + } diff --git a/tests/integration/features/bootstrap/ServerContext.php b/tests/integration/features/bootstrap/ServerContext.php index f7c192384..cdc8f1984 100644 --- a/tests/integration/features/bootstrap/ServerContext.php +++ b/tests/integration/features/bootstrap/ServerContext.php @@ -10,15 +10,15 @@ class ServerContext implements Context { WebDav::__construct as private __tConstruct; } + private string $rawBaseUrl; + private string $mappedUserId; + private array $lastInsertIds = []; + public function __construct($baseUrl) { $this->rawBaseUrl = $baseUrl; $this->__tConstruct($baseUrl . '/index.php/ocs/', ['admin', 'admin'], '123456'); } - /** @var string */ - private $mappedUserId; - - private $lastInsertIds = []; /** * @BeforeSuite diff --git a/tests/integration/features/decks.feature b/tests/integration/features/decks.feature index 22a74baec..3582af430 100644 --- a/tests/integration/features/decks.feature +++ b/tests/integration/features/decks.feature @@ -32,3 +32,101 @@ Feature: decks And creates a board named "MyBoard" with color "000000" And create a stack named "ToDo" When create a card named "This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters" + + Scenario: Setting a duedate on a card + Given acting as user "user0" + And creates a board named "MyBoard" with color "000000" + And create a stack named "ToDo" + And create a card named "Overdue task" + When get the card details + And the response should be a JSON array with the following mandatory values + |key|value| + |title|Overdue task| + |duedate|| + |overdue|0| + And set the card attribute "duedate" to "2020-12-12 13:37:00" + When get the card details + And the response should be a JSON array with the following mandatory values + |key|value| + |title|Overdue task| + |duedate|2020-12-12T13:37:00+00:00| + |overdue|3| + And set the card attribute "duedate" to "" + When get the card details + And the response should be a JSON array with the following mandatory values + |key|value| + |title|Overdue task| + |duedate|| + |overdue|0| + + Scenario: Cannot access card on a deleted board + Given acting as user "user0" + And creates a board named "MyBoard" with color "000000" + And create a stack named "ToDo" + And create a card named "Overdue task" + And remember the last card as "deletedCard" + And uploads an attachment to the last used card + And remember the last attachment as "my-attachment" + And post a comment with content "My first comment" on the card + And delete the board + + When fetching the attachment "my-attachment" for the card "deletedCard" + Then the response should have a status code 403 + + When get the comments on the card + Then the response should have a status code 403 + + When post a comment with content "My second comment" on the card + Then the response should have a status code 403 + + When uploads an attachment to the last used card + Then the response should have a status code 403 + + When set the description to "Update some text" + Then the response should have a status code 403 + + When get the card details + Then the response should have a status code 403 + + When create a card named "Overdue task" + Then the response should have a status code 403 + + When create a stack named "ToDo" + Then the response should have a status code 403 + + Scenario: Cannot access card on a deleted card + Given acting as user "user0" + And creates a board named "MyBoard" with color "000000" + And create a stack named "ToDo" + And create a card named "Overdue task" + And remember the last card as "deletedCard" + And uploads an attachment to the last used card + And remember the last attachment as "my-attachment" + And post a comment with content "My first comment" on the card + And delete the card + + When fetching the attachment "my-attachment" for the card "deletedCard" + Then the response should have a status code 403 + + When get the comments on the card + Then the response should have a status code 403 + + When post a comment with content "My second comment" on the card + Then the response should have a status code 403 + + When deleting the attachment "my-attachment" for the card "deletedCard" + Then the response should have a status code 403 + + When uploads an attachment to the last used card + Then the response should have a status code 403 + + When get the card details + Then the response should have a status code 403 + + # We currently still expect to be able to update the card as this is used to undo deletion + When set the description to "Update some text" + Then the response should have a status code 403 + #When set the card attribute "deletedAt" to "0" + #Then the response should have a status code 200 + #When set the description to "Update some text" + #Then the response should have a status code 200 From 25875f1c051d9c52c34a8fb88c934ccb9d1481b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 3 Jan 2024 15:32:40 +0100 Subject: [PATCH 2/8] fix: Consider a deleted board inaccessible to share recipients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only the owner can delete/undo a board deletion so there is no reason other users should have any permission on a board marked as deleted Signed-off-by: Julius Härtl --- lib/Service/PermissionService.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 64c4305a8..556fb6e60 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -107,8 +107,9 @@ public function getPermissions($boardId, ?string $userId = null) { return $cached; } + $board = $this->getBoard($boardId); $owner = $this->userIsBoardOwner($boardId, $userId); - $acls = $this->aclMapper->findAll($boardId); + $acls = $board->getDeletedAt() === 0 ? $this->aclMapper->findAll($boardId) : []; $permissions = [ Acl::PERMISSION_READ => $owner || $this->userCan($acls, Acl::PERMISSION_READ, $userId), Acl::PERMISSION_EDIT => $owner || $this->userCan($acls, Acl::PERMISSION_EDIT, $userId), From a91e0eb1ac6f5b0220a99a6920dd376cfa4e6848 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 4 Jan 2024 10:53:11 +0100 Subject: [PATCH 3/8] fix: limit to non-deleted cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Db/Card.php | 2 ++ lib/Service/BoardService.php | 2 +- lib/Service/CommentService.php | 15 ++++----------- lib/Service/PermissionService.php | 16 +++++++++++----- lib/Sharing/ShareAPIHelper.php | 2 +- 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/lib/Db/Card.php b/lib/Db/Card.php index e36a9fb0c..3c45d910e 100644 --- a/lib/Db/Card.php +++ b/lib/Db/Card.php @@ -36,6 +36,8 @@ * @method int getLastModified() * @method int getCreatedAt() * @method bool getArchived() + * @method int getDeletedAt() + * @method void setDeletedAt(int $deletedAt) * @method bool getNotified() * * @method void setLabels(Label[] $labels) diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index e0c8aa7fc..4ab9e2c4a 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -446,7 +446,7 @@ public function addAcl($boardId, $type, $participant, $edit, $share, $manage) { $newAcl = $this->aclMapper->insert($acl); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $newAcl, ActivityManager::SUBJECT_BOARD_SHARE, [], $this->userId); - $this->notificationHelper->sendBoardShared((int)$boardId, $acl); + $this->notificationHelper->sendBoardShared($boardId, $acl); $this->boardMapper->mapAcl($newAcl); $this->changeHelper->boardChanged($boardId); diff --git a/lib/Service/CommentService.php b/lib/Service/CommentService.php index b33f84342..be2f4f14e 100644 --- a/lib/Service/CommentService.php +++ b/lib/Service/CommentService.php @@ -94,7 +94,7 @@ private function get(int $cardId, int $commentId): IComment { throw new NotFoundException('No comment found.'); } if ($comment->getParentId() !== '0') { - $this->permissionService->checkPermission($this->cardMapper, $comment->getParentId(), Acl::PERMISSION_READ); + $this->permissionService->checkPermission($this->cardMapper, (int)$comment->getParentId(), Acl::PERMISSION_READ); } return $comment; @@ -113,24 +113,17 @@ public function getFormatted(int $cardId, int $commentId): array { } /** - * @param string $cardId - * @param string $message - * @param string $replyTo - * @return DataResponse * @throws BadRequestException * @throws NotFoundException|NoPermissionException */ - public function create(string $cardId, string $message, string $replyTo = '0'): DataResponse { - if (!is_numeric($cardId)) { - throw new BadRequestException('A valid card id must be provided'); - } + public function create(int $cardId, string $message, string $replyTo = '0'): DataResponse { $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ); // Check if parent is a comment on the same card if ($replyTo !== '0') { try { $comment = $this->commentsManager->get($replyTo); - if ($comment->getObjectType() !== Application::COMMENT_ENTITY_TYPE || $comment->getObjectId() !== $cardId) { + if ($comment->getObjectType() !== Application::COMMENT_ENTITY_TYPE || (int)$comment->getObjectId() !== $cardId) { throw new CommentNotFoundException(); } } catch (CommentNotFoundException $e) { @@ -139,7 +132,7 @@ public function create(string $cardId, string $message, string $replyTo = '0'): } try { - $comment = $this->commentsManager->create('users', $this->userId, Application::COMMENT_ENTITY_TYPE, $cardId); + $comment = $this->commentsManager->create('users', $this->userId, Application::COMMENT_ENTITY_TYPE, (string)$cardId); $comment->setMessage($message); $comment->setVerb('comment'); $comment->setParentId($replyTo); diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 556fb6e60..bb66631e7 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -29,6 +29,7 @@ use OCA\Deck\Db\AclMapper; use OCA\Deck\Db\Board; use OCA\Deck\Db\BoardMapper; +use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\IPermissionMapper; use OCA\Deck\Db\User; use OCA\Deck\NoPermissionException; @@ -143,13 +144,10 @@ public function matchPermissions(Board $board) { /** * check permissions for replacing dark magic middleware * - * @param $mapper IPermissionMapper|null null if $id is a boardId - * @param $id int unique identifier of the Entity - * @param $permission int - * @return bool + * @param numeric $id * @throws NoPermissionException */ - public function checkPermission($mapper, $id, $permission, $userId = null): bool { + public function checkPermission(?IPermissionMapper $mapper, $id, int $permission, $userId = null, bool $allowDeletedCard = false): bool { $boardId = $id; if ($mapper instanceof IPermissionMapper && !($mapper instanceof BoardMapper)) { $boardId = $mapper->findBoardId($id); @@ -161,6 +159,14 @@ public function checkPermission($mapper, $id, $permission, $userId = null): bool $permissions = $this->getPermissions($boardId, $userId); if ($permissions[$permission] === true) { + + if (!$allowDeletedCard && $mapper instanceof CardMapper) { + $card = $mapper->find($id); + if ($card->getDeletedAt() > 0) { + throw new NoPermissionException('Card is deleted'); + } + } + return true; } diff --git a/lib/Sharing/ShareAPIHelper.php b/lib/Sharing/ShareAPIHelper.php index 5528b6a92..41a5dfc6d 100644 --- a/lib/Sharing/ShareAPIHelper.php +++ b/lib/Sharing/ShareAPIHelper.php @@ -115,7 +115,7 @@ private function parseDate(string $expireDate): \DateTime { */ public function canAccessShare(IShare $share, string $user): bool { try { - $this->permissionService->checkPermission($this->cardMapper, $share->getSharedWith(), Acl::PERMISSION_READ, $user); + $this->permissionService->checkPermission($this->cardMapper, (int)$share->getSharedWith(), Acl::PERMISSION_READ, $user); } catch (NoPermissionException $e) { return false; } From df2beaf5a53076788081cd843766172e7bb2828f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 4 Jan 2024 14:01:24 +0100 Subject: [PATCH 4/8] fix: Further limit updating cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Service/CardService.php | 6 +++--- tests/integration/features/bootstrap/BoardContext.php | 1 + tests/integration/features/decks.feature | 8 ++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index cc30b3f4a..cbee1853d 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -288,7 +288,7 @@ public function delete($id) { public function update($id, $title, $stackId, $type, $owner, $description = '', $order = 0, $duedate = null, $deletedAt = null, $archived = null) { $this->cardServiceValidator->check(compact('id', 'title', 'stackId', 'type', 'owner', 'order')); - $this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT); + $this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT, allowDeletedCard: true); $this->permissionService->checkPermission($this->stackMapper, $stackId, Acl::PERMISSION_EDIT); if ($this->boardService->isArchived($this->cardMapper, $id)) { @@ -300,9 +300,9 @@ public function update($id, $title, $stackId, $type, $owner, $description = '', } if ($card->getDeletedAt() !== 0) { - if ($deletedAt === null) { + if ($deletedAt === null || $deletedAt > 0) { // Only allow operations when restoring the card - throw new StatusException('Operation not allowed. This card was deleted.'); + throw new NoPermissionException('Operation not allowed. This card was deleted.'); } } diff --git a/tests/integration/features/bootstrap/BoardContext.php b/tests/integration/features/bootstrap/BoardContext.php index d29ed1524..ce076d985 100644 --- a/tests/integration/features/bootstrap/BoardContext.php +++ b/tests/integration/features/bootstrap/BoardContext.php @@ -294,6 +294,7 @@ public function getRememberedCard($arg1) { */ public function deleteTheCard() { $this->requestContext->sendJSONrequest('DELETE', '/index.php/apps/deck/cards/' . $this->card['id']); + $this->card['deletedAt'] = time(); } /** diff --git a/tests/integration/features/decks.feature b/tests/integration/features/decks.feature index 3582af430..9656c2a23 100644 --- a/tests/integration/features/decks.feature +++ b/tests/integration/features/decks.feature @@ -126,7 +126,7 @@ Feature: decks # We currently still expect to be able to update the card as this is used to undo deletion When set the description to "Update some text" Then the response should have a status code 403 - #When set the card attribute "deletedAt" to "0" - #Then the response should have a status code 200 - #When set the description to "Update some text" - #Then the response should have a status code 200 + When set the card attribute "deletedAt" to "0" + Then the response should have a status code 200 + When set the description to "Update some text" + Then the response should have a status code 200 From 6c1f4744cc4fe9b4973789e0da73775d00320143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 4 Jan 2024 14:01:49 +0100 Subject: [PATCH 5/8] fix: Limit card activities for deleted cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Activity/ActivityManager.php | 21 +++++++++++++++++ lib/Activity/DeckProvider.php | 6 +++++ .../features/bootstrap/BoardContext.php | 23 +++++++++++++++++-- tests/integration/features/decks.feature | 5 ++++ tests/unit/Activity/DeckProviderTest.php | 3 +++ 5 files changed, 56 insertions(+), 2 deletions(-) diff --git a/lib/Activity/ActivityManager.php b/lib/Activity/ActivityManager.php index 4e1d192e0..109fa5315 100644 --- a/lib/Activity/ActivityManager.php +++ b/lib/Activity/ActivityManager.php @@ -38,6 +38,7 @@ use OCA\Deck\Db\Label; use OCA\Deck\Db\Stack; use OCA\Deck\Db\StackMapper; +use OCA\Deck\NoPermissionException; use OCA\Deck\Service\PermissionService; use OCP\Activity\IEvent; use OCP\Activity\IManager; @@ -554,4 +555,24 @@ private function findDetailsForAcl($aclId) { 'board' => $board ]; } + + public function canSeeCardActivity(int $cardId): bool { + try { + $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ); + $card = $this->cardMapper->find($cardId); + return $card->getDeletedAt() === 0; + } catch (NoPermissionException $e) { + return false; + } + } + + public function canSeeBoardActivity(int $boardId): bool { + try { + $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); + $board = $this->boardMapper->find($boardId); + return $board->getDeletedAt() === 0; + } catch (NoPermissionException $e) { + return false; + } + } } diff --git a/lib/Activity/DeckProvider.php b/lib/Activity/DeckProvider.php index 5a79275cd..4d9d64a87 100644 --- a/lib/Activity/DeckProvider.php +++ b/lib/Activity/DeckProvider.php @@ -111,6 +111,9 @@ public function parse($language, IEvent $event, IEvent $previousEvent = null): I $event->setAuthor($author); } if ($event->getObjectType() === ActivityManager::DECK_OBJECT_BOARD) { + if (!$this->activityManager->canSeeBoardActivity($event->getObjectId())) { + throw new \InvalidArgumentException(); + } if (isset($subjectParams['board']) && $event->getObjectName() === '') { $event->setObject($event->getObjectType(), $event->getObjectId(), $subjectParams['board']['title']); } @@ -125,6 +128,9 @@ public function parse($language, IEvent $event, IEvent $previousEvent = null): I } if (isset($subjectParams['card']) && $event->getObjectType() === ActivityManager::DECK_OBJECT_CARD) { + if (!$this->activityManager->canSeeCardActivity($event->getObjectId())) { + throw new \InvalidArgumentException(); + } if ($event->getObjectName() === '') { $event->setObject($event->getObjectType(), $event->getObjectId(), $subjectParams['card']['title']); } diff --git a/tests/integration/features/bootstrap/BoardContext.php b/tests/integration/features/bootstrap/BoardContext.php index ce076d985..09d05fa30 100644 --- a/tests/integration/features/bootstrap/BoardContext.php +++ b/tests/integration/features/bootstrap/BoardContext.php @@ -17,9 +17,9 @@ class BoardContext implements Context { /** @var array last card response */ private $card = null; private array $storedCards = []; + private ?array $activities = null; - /** @var ServerContext */ - private $serverContext; + private ServerContext $serverContext; /** @BeforeScenario */ public function gatherContexts(BeforeScenarioScope $scope) { @@ -303,4 +303,23 @@ public function deleteTheCard() { public function deleteTheBoard() { $this->requestContext->sendJSONrequest('DELETE', '/index.php/apps/deck/boards/' . $this->board['id']); } + + + /** + * @Given /^get the activities for the last card$/ + */ + public function getActivitiesForTheLastCard() { + $card = $this->getLastUsedCard(); + $this->requestContext->sendOCSRequest('GET', '/apps/activity/api/v2/activity/filter?format=json&type=deck&since=0&object_type=deck_card&object_id=' . $card['id'] . '&limit=50'); + $this->activities = json_decode((string)$this->getResponse()->getBody(), true)['ocs']['data'] ?? null; + } + + /** + * @Then the fetched activities should have :count entries + */ + public function theFetchedActivitiesShouldHaveEntries($count) { + Assert::assertEquals($count, count($this->activities ?? [])); + } + + } diff --git a/tests/integration/features/decks.feature b/tests/integration/features/decks.feature index 9656c2a23..0351f6269 100644 --- a/tests/integration/features/decks.feature +++ b/tests/integration/features/decks.feature @@ -103,8 +103,13 @@ Feature: decks And uploads an attachment to the last used card And remember the last attachment as "my-attachment" And post a comment with content "My first comment" on the card + When get the activities for the last card + Then the fetched activities should have 3 entries And delete the card + When get the activities for the last card + Then the fetched activities should have 0 entries + When fetching the attachment "my-attachment" for the card "deletedCard" Then the response should have a status code 403 diff --git a/tests/unit/Activity/DeckProviderTest.php b/tests/unit/Activity/DeckProviderTest.php index 4302834cb..13cdf691e 100644 --- a/tests/unit/Activity/DeckProviderTest.php +++ b/tests/unit/Activity/DeckProviderTest.php @@ -74,6 +74,9 @@ public function setUp(): void { $this->config = $this->createMock(IConfig::class); $this->cardService = $this->createMock(CardService::class); $this->provider = new DeckProvider($this->urlGenerator, $this->activityManager, $this->userManager, $this->commentsManager, $this->l10nFactory, $this->config, $this->userId, $this->cardService); + + $this->activityManager->method('canSeeCardActivity')->willReturn(true); + $this->activityManager->method('canSeeBoardActivity')->willReturn(true); } private function mockEvent($objectType, $objectId, $objectName, $subject, $subjectParameters = []) { From 632871ad4d00c410c860c21743352dfb41a64290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 4 Jan 2024 16:01:01 +0100 Subject: [PATCH 6/8] chore: Fix ci setup for activity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .github/workflows/integration.yml | 7 +++++++ composer.json | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index bd3d09d9d..a4c32a09c 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -70,6 +70,13 @@ jobs: with: path: apps/${{ env.APP_NAME }} + - name: Checkout activity + uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + with: + repository: nextcloud/activity + ref: ${{ matrix.server-versions }} + path: apps/activity + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@2.25.5 with: diff --git a/composer.json b/composer.json index be04d1341..a916f270f 100644 --- a/composer.json +++ b/composer.json @@ -42,7 +42,8 @@ "@test:integration" ], "test:unit": "vendor/bin/phpunit -c tests/phpunit.xml", - "test:integration": "vendor/bin/phpunit -c tests/phpunit.integration.xml && cd tests/integration && ./run.sh" + "test:integration": "vendor/bin/phpunit -c tests/phpunit.integration.xml", + "test:api": "cd tests/integration && ./run.sh" }, "autoload-dev": { "psr-4": { From dda1702afb7c6e16f700aaf595a2a91b4eb4d162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 9 Jan 2024 17:52:41 +0100 Subject: [PATCH 7/8] tests: Fix missing behat context methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Service/PermissionService.php | 1 - .../features/bootstrap/BoardContext.php | 2 -- .../features/bootstrap/CommentContext.php | 1 - .../features/bootstrap/RequestContext.php | 25 ++++++++++++++++++ tests/integration/features/decks.feature | 26 ------------------- 5 files changed, 25 insertions(+), 30 deletions(-) diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index bb66631e7..e573c0257 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -159,7 +159,6 @@ public function checkPermission(?IPermissionMapper $mapper, $id, int $permission $permissions = $this->getPermissions($boardId, $userId); if ($permissions[$permission] === true) { - if (!$allowDeletedCard && $mapper instanceof CardMapper) { $card = $mapper->find($id); if ($card->getDeletedAt() > 0) { diff --git a/tests/integration/features/bootstrap/BoardContext.php b/tests/integration/features/bootstrap/BoardContext.php index 09d05fa30..45038de64 100644 --- a/tests/integration/features/bootstrap/BoardContext.php +++ b/tests/integration/features/bootstrap/BoardContext.php @@ -320,6 +320,4 @@ public function getActivitiesForTheLastCard() { public function theFetchedActivitiesShouldHaveEntries($count) { Assert::assertEquals($count, count($this->activities ?? [])); } - - } diff --git a/tests/integration/features/bootstrap/CommentContext.php b/tests/integration/features/bootstrap/CommentContext.php index 92bc9a347..aba5a6065 100644 --- a/tests/integration/features/bootstrap/CommentContext.php +++ b/tests/integration/features/bootstrap/CommentContext.php @@ -58,5 +58,4 @@ public function deleteTheCommentOnTheCard() { $card = $this->boardContext->getLastUsedCard(); $this->requestContext->sendOCSRequest('DELETE', '/apps/deck/api/v1.0/cards/' . $card['id'] . '/comments/'. $this->lastComment['id']); } - } diff --git a/tests/integration/features/bootstrap/RequestContext.php b/tests/integration/features/bootstrap/RequestContext.php index 9df6be205..ebf01e80a 100644 --- a/tests/integration/features/bootstrap/RequestContext.php +++ b/tests/integration/features/bootstrap/RequestContext.php @@ -166,4 +166,29 @@ public function getResponseBodyFromJson() { $this->getResponse()->getBody()->seek(0); return json_decode((string)$this->getResponse()->getBody(), true); } + + /** + * @Given /^the response should be a list of objects$/ + */ + public function theResponseShouldBeAListOfObjects() { + $jsonResponse = $this->getResponseBodyFromJson(); + Assert::assertEquals(array_keys($jsonResponse), range(0, count($jsonResponse) - 1)); + } + + /** + * @When /^the response should contain an element with the properties$/ + */ + public function responseContainsElement(TableNode $element) { + $json = $this->getResponseBodyFromJson(); + $found = array_filter($json, function ($board) use ($element) { + foreach ($element as $row) { + if ($row['value'] !== $board[$row['property']]) { + return false; + } + } + + return true; + }); + Assert::assertEquals(1, count($found)); + } } diff --git a/tests/integration/features/decks.feature b/tests/integration/features/decks.feature index 0351f6269..1913c8420 100644 --- a/tests/integration/features/decks.feature +++ b/tests/integration/features/decks.feature @@ -33,32 +33,6 @@ Feature: decks And create a stack named "ToDo" When create a card named "This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters" - Scenario: Setting a duedate on a card - Given acting as user "user0" - And creates a board named "MyBoard" with color "000000" - And create a stack named "ToDo" - And create a card named "Overdue task" - When get the card details - And the response should be a JSON array with the following mandatory values - |key|value| - |title|Overdue task| - |duedate|| - |overdue|0| - And set the card attribute "duedate" to "2020-12-12 13:37:00" - When get the card details - And the response should be a JSON array with the following mandatory values - |key|value| - |title|Overdue task| - |duedate|2020-12-12T13:37:00+00:00| - |overdue|3| - And set the card attribute "duedate" to "" - When get the card details - And the response should be a JSON array with the following mandatory values - |key|value| - |title|Overdue task| - |duedate|| - |overdue|0| - Scenario: Cannot access card on a deleted board Given acting as user "user0" And creates a board named "MyBoard" with color "000000" From 95c274c59b0d7b176dbb4f9e2748b1d1b5aac2a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 16 Oct 2023 22:54:30 +0200 Subject: [PATCH 8/8] ci(cypress): Fix file picker selector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- cypress/e2e/cardFeatures.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cypress/e2e/cardFeatures.js b/cypress/e2e/cardFeatures.js index cb4772e52..269566ed1 100644 --- a/cypress/e2e/cardFeatures.js +++ b/cypress/e2e/cardFeatures.js @@ -85,9 +85,10 @@ describe('Card', function() { cy.get('button.icon-upload').should('be.visible') cy.get('button.icon-folder').should('be.visible') .click() - cy.get('.oc-dialog #picker-filestable tr[data-entryname="welcome.txt"] td.filename').should('be.visible') + cy.get('.file-picker__main').should('be.visible') + cy.get('.file-picker__main [data-filename="welcome.txt"]').should('be.visible') .click() - cy.get('.oc-dialog button.primary').click() + cy.get('.dialog__actions button.button-vue--vue-primary').click() cy.get('.attachment-list .basename').contains('welcome.txt') })