From 83ed6e4990f1b22812619d7242e4b124f42a0d84 Mon Sep 17 00:00:00 2001
From: sunguangning <sun-guangning@126.com>
Date: Wed, 13 May 2026 21:47:26 +0800
Subject: [PATCH] feat: allow read-only users to assign labels to cards

Lower the permission check in assignLabel() from PERMISSION_EDIT to
PERMISSION_READ, allowing users with read-only access to add labels
to cards. This is useful when a board owner wants to share cards
with external contributors who should be able to self-assign labels
but not modify card content.

RemoveLabel() still requires PERMISSION_EDIT to prevent read-only
users from removing labels.

Fixes #590
---
 lib/Service/CardService.php            |  2 +-
 tests/unit/Service/CardServiceTest.php | 65 ++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php
index 3b12d2a89..589eb62dc 100644
--- a/lib/Service/CardService.php
+++ b/lib/Service/CardService.php
@@ -617,7 +617,7 @@ public function undone(int $id): Card {
 	public function assignLabel(int $cardId, int $labelId): Card {
 		$this->cardServiceValidator->check(compact('cardId', 'labelId'));
 
-		$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT);
+		$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ);
 		$this->permissionService->checkPermission($this->labelMapper, $labelId, Acl::PERMISSION_READ);
 
 		if ($this->boardService->isArchived($this->cardMapper, $cardId)) {
diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php
index 840cdd6fc..1d52a881e 100644
--- a/tests/unit/Service/CardServiceTest.php
+++ b/tests/unit/Service/CardServiceTest.php
@@ -25,8 +25,10 @@
 namespace OCA\Deck\Service;
 
 use OCA\Deck\Activity\ActivityManager;
+use OCA\Deck\Db\Acl;
 use OCA\Deck\Db\Assignment;
 use OCA\Deck\Db\AssignmentMapper;
+use OCA\Deck\NoPermissionException;
 use OCA\Deck\Db\Board;
 use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\Card;
@@ -549,6 +551,69 @@ public function testRemoveLabelArchived() {
 		$this->cardService->removeLabel(123, 999);
 	}
 
+	public function testAssignLabelWithReadPermission() {
+		// Users with READ permission (but not EDIT) should be able to assign labels
+		// This allows read-only users to label cards they are assigned to
+		$card = new Card();
+		$card->setArchived(false);
+		$card->setId(123);
+		$label = new Label();
+		$label->setBoardId(1);
+
+		$this->permissionService->expects($this->exactly(2))
+			->method('checkPermission')
+			->willReturn(true);
+		$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);
+	}
+
+	public function testAssignLabelWithoutReadPermission() {
+		// Users without READ permission should get NoPermissionException
+		$this->permissionService->expects($this->once())
+			->method('checkPermission')
+			->willThrowException(new NoPermissionException('Permission denied'));
+		$this->cardMapper->expects($this->never())->method('assignLabel');
+		$this->expectException(NoPermissionException::class);
+		$this->cardService->assignLabel(123, 999);
+	}
+
+	public function testRemoveLabelRequiresEditPermission() {
+		// Removing a label requires EDIT permission (higher than READ)
+		// This is intentional: read-only users can add labels but not remove them
+		$card = new Card();
+		$card->setArchived(false);
+		$card->setId(123);
+		$label = new Label();
+		$label->setBoardId(1);
+
+		$this->permissionService->expects($this->exactly(2))
+			->method('checkPermission')
+			->willReturn(true);
+		$this->cardMapper->expects($this->once())->method('find')->willReturn($card);
+		$this->cardMapper->expects($this->once())->method('removeLabel');
+		$this->labelMapper->expects($this->once())
+			->method('find')
+			->willReturn($label);
+		$this->cardService->removeLabel(123, 999);
+	}
+
+	public function testRemoveLabelWithoutEditPermission() {
+		// Without EDIT permission, removing a label should fail
+		$this->permissionService->expects($this->once())
+			->method('checkPermission')
+			->willThrowException(new NoPermissionException('Permission denied'));
+		$this->cardMapper->expects($this->never())->method('removeLabel');
+		$this->expectException(NoPermissionException::class);
+		$this->cardService->removeLabel(123, 999);
+	}
+
 	public function testDoneMarksCardAsDone(): void {
 		$card = new Card();
 		$card->setId(42);