From 81bf5565630c31a3a2f466469a482a6aecbb2e83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 2 Sep 2024 14:32:13 +0200 Subject: [PATCH 1/3] fix: Limit label actions to labels of the cards board 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 | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index b0a91132c..3965f9eeb 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -620,8 +620,9 @@ class CardService { public function assignLabel($cardId, $labelId) { $this->cardServiceValidator->check(compact('cardId', 'labelId')); - $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT); + $this->permissionService->checkPermission($this->labelMapper, $labelId, Acl::PERMISSION_READ); + if ($this->boardService->isArchived($this->cardMapper, $cardId)) { throw new StatusException('Operation not allowed. This board is archived.'); } @@ -630,6 +631,9 @@ class CardService { throw new StatusException('Operation not allowed. This card is archived.'); } $label = $this->labelMapper->find($labelId); + if ($label->getBoardId() !== $this->cardMapper->findBoardId($card->getId())) { + throw new StatusException('Operation not allowed. Label does not exist.'); + } $this->cardMapper->assignLabel($cardId, $labelId); $this->changeHelper->cardChanged($cardId); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_LABEL_ASSIGN, ['label' => $label]); @@ -651,6 +655,8 @@ class CardService { $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT); + $this->permissionService->checkPermission($this->labelMapper, $labelId, Acl::PERMISSION_READ); + if ($this->boardService->isArchived($this->cardMapper, $cardId)) { throw new StatusException('Operation not allowed. This board is archived.'); } @@ -659,6 +665,9 @@ class CardService { throw new StatusException('Operation not allowed. This card is archived.'); } $label = $this->labelMapper->find($labelId); + if ($label->getBoardId() !== $this->cardMapper->findBoardId($card->getId())) { + throw new StatusException('Operation not allowed. Label does not exist.'); + } $this->cardMapper->removeLabel($cardId, $labelId); $this->changeHelper->cardChanged($cardId); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_LABEL_UNASSING, ['label' => $label]); From 00e513c299c4454ee9329ea041d716dde3336300 Mon Sep 17 00:00:00 2001 From: Julius Knorr Date: Tue, 22 Apr 2025 16:26:04 +0200 Subject: [PATCH 2/3] tests: Fix unit test mocking around label checks Signed-off-by: Julius Knorr --- tests/unit/Service/CardServiceTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php index 8e685376c..96a4330bd 100644 --- a/tests/unit/Service/CardServiceTest.php +++ b/tests/unit/Service/CardServiceTest.php @@ -31,6 +31,7 @@ use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\Card; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\ChangeHelper; +use OCA\Deck\Db\Label; use OCA\Deck\Db\LabelMapper; use OCA\Deck\Db\Stack; use OCA\Deck\Db\StackMapper; @@ -347,8 +348,17 @@ class CardServiceTest extends TestCase { public function testAssignLabel() { $card = new Card(); $card->setArchived(false); + $card->setId(123); + $label = new Label(); + $label->setBoardId(1); $this->cardMapper->expects($this->once())->method('find')->willReturn($card); $this->cardMapper->expects($this->once())->method('assignLabel'); + $this->cardMapper->expects($this->once()) + ->method('findBoardId') + ->willReturn(1); + $this->labelMapper->expects($this->once()) + ->method('find') + ->willReturn($label); $this->cardService->assignLabel(123, 999); } @@ -364,8 +374,17 @@ class CardServiceTest extends TestCase { public function testRemoveLabel() { $card = new Card(); $card->setArchived(false); + $card->setId(123); + $label = new Label(); + $label->setBoardId(1); $this->cardMapper->expects($this->once())->method('find')->willReturn($card); $this->cardMapper->expects($this->once())->method('removeLabel'); + $this->cardMapper->expects($this->once()) + ->method('findBoardId') + ->willReturn(1); + $this->labelMapper->expects($this->once()) + ->method('find') + ->willReturn($label); $this->cardService->removeLabel(123, 999); } From f00ffc57ef667920c8be325faaddde3884c16fb5 Mon Sep 17 00:00:00 2001 From: Julius Knorr Date: Mon, 28 Apr 2025 13:38:33 +0200 Subject: [PATCH 3/3] chore: Update base query count Signed-off-by: Julius Knorr --- tests/integration/base-query-count.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/base-query-count.txt b/tests/integration/base-query-count.txt index 05a480bb3..0223d108f 100644 --- a/tests/integration/base-query-count.txt +++ b/tests/integration/base-query-count.txt @@ -1 +1 @@ -71831 +72025