Skip to content

Commit cc0434d

Browse files
committed
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 Signed-off-by: Sun Guangning <sun-guangning@126.com> Signed-off-by: sunguangning <sun-guangning@126.com>
1 parent dff73f4 commit cc0434d

2 files changed

Lines changed: 66 additions & 1 deletion

File tree

lib/Service/CardService.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ public function undone(int $id): Card {
617617
public function assignLabel(int $cardId, int $labelId): Card {
618618
$this->cardServiceValidator->check(compact('cardId', 'labelId'));
619619

620-
$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT);
620+
$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ);
621621
$this->permissionService->checkPermission($this->labelMapper, $labelId, Acl::PERMISSION_READ);
622622

623623
if ($this->boardService->isArchived($this->cardMapper, $cardId)) {

tests/unit/Service/CardServiceTest.php

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@
2525
namespace OCA\Deck\Service;
2626

2727
use OCA\Deck\Activity\ActivityManager;
28+
use OCA\Deck\Db\Acl;
2829
use OCA\Deck\Db\Assignment;
2930
use OCA\Deck\Db\AssignmentMapper;
31+
use OCA\Deck\NoPermissionException;
3032
use OCA\Deck\Db\Board;
3133
use OCA\Deck\Db\BoardMapper;
3234
use OCA\Deck\Db\Card;
@@ -549,6 +551,69 @@ public function testRemoveLabelArchived() {
549551
$this->cardService->removeLabel(123, 999);
550552
}
551553

554+
public function testAssignLabelWithReadPermission() {
555+
// Users with READ permission (but not EDIT) should be able to assign labels
556+
// This allows read-only users to label cards they are assigned to
557+
$card = new Card();
558+
$card->setArchived(false);
559+
$card->setId(123);
560+
$label = new Label();
561+
$label->setBoardId(1);
562+
563+
$this->permissionService->expects($this->exactly(2))
564+
->method('checkPermission')
565+
->willReturn(true);
566+
$this->cardMapper->expects($this->once())->method('find')->willReturn($card);
567+
$this->cardMapper->expects($this->once())->method('assignLabel');
568+
$this->cardMapper->expects($this->once())
569+
->method('findBoardId')
570+
->willReturn(1);
571+
$this->labelMapper->expects($this->once())
572+
->method('find')
573+
->willReturn($label);
574+
$this->cardService->assignLabel(123, 999);
575+
}
576+
577+
public function testAssignLabelWithoutReadPermission() {
578+
// Users without READ permission should get NoPermissionException
579+
$this->permissionService->expects($this->once())
580+
->method('checkPermission')
581+
->willThrowException(new NoPermissionException('Permission denied'));
582+
$this->cardMapper->expects($this->never())->method('assignLabel');
583+
$this->expectException(NoPermissionException::class);
584+
$this->cardService->assignLabel(123, 999);
585+
}
586+
587+
public function testRemoveLabelRequiresEditPermission() {
588+
// Removing a label requires EDIT permission (higher than READ)
589+
// This is intentional: read-only users can add labels but not remove them
590+
$card = new Card();
591+
$card->setArchived(false);
592+
$card->setId(123);
593+
$label = new Label();
594+
$label->setBoardId(1);
595+
596+
$this->permissionService->expects($this->exactly(2))
597+
->method('checkPermission')
598+
->willReturn(true);
599+
$this->cardMapper->expects($this->once())->method('find')->willReturn($card);
600+
$this->cardMapper->expects($this->once())->method('removeLabel');
601+
$this->labelMapper->expects($this->once())
602+
->method('find')
603+
->willReturn($label);
604+
$this->cardService->removeLabel(123, 999);
605+
}
606+
607+
public function testRemoveLabelWithoutEditPermission() {
608+
// Without EDIT permission, removing a label should fail
609+
$this->permissionService->expects($this->once())
610+
->method('checkPermission')
611+
->willThrowException(new NoPermissionException('Permission denied'));
612+
$this->cardMapper->expects($this->never())->method('removeLabel');
613+
$this->expectException(NoPermissionException::class);
614+
$this->cardService->removeLabel(123, 999);
615+
}
616+
552617
public function testDoneMarksCardAsDone(): void {
553618
$card = new Card();
554619
$card->setId(42);

0 commit comments

Comments
 (0)