diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 639fa078e..3c6742e1e 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -609,8 +609,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.'); } @@ -619,6 +620,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]); @@ -640,6 +644,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.'); } @@ -648,6 +654,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]); diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php index f7e8b3a99..73e29093f 100644 --- a/tests/unit/Service/CardServiceTest.php +++ b/tests/unit/Service/CardServiceTest.php @@ -258,6 +258,17 @@ class CardServiceTest extends TestCase { ->method('find') ->willReturn($card, $clonedCard); + $this->cardMapper->expects($this->any()) + ->method('findBoardId') + ->willReturn(1234); + + $this->labelMapper->expects($this->any()) + ->method('find') + ->willReturn(Label::fromRow([ + 'id' => 1, + 'boardId' => 1234, + ])); + // check if users are assigned $this->assignmentService->expects($this->once()) ->method('assignUser') @@ -433,8 +444,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); } @@ -450,8 +470,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); }