From 9b5552663fd587ebed21c6665c01326e8d1c3207 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] 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 010820c43..eaa79c70e 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; @@ -564,4 +565,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 0b7a63e7e..9fcf6e1f0 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 02b6be1c2..198be24e0 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 = []) {