Merge pull request #6952 from nextcloud/backport/6933/stable29

[stable29] fix: Limit label actions to labels of the cards board

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]);

tests/integration/base-query-count.txt

@@ -1 +1 @@
-71831
+72025

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);
 	}