From 2ae01d367900346f529758406dff53cdb91c0924 Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Wed, 29 Apr 2026 19:14:23 +0200 Subject: [PATCH 01/20] feat: add owner_type field to boards for circle ownership support Add an owner_type column to deck_boards (SMALLINT, default 0) that mirrors the existing Acl::PERMISSION_TYPE_* constants. A value of 0 means the owner is a user (preserving all existing behaviour); 7 means the owner is a circle/team. - DB migration Version11002Date20260429000000 adds the column idempotently - Board entity gains $ownerType property, type registration, docblock accessors, and automatic serialisation into API responses as ownerType - BoardMapper: add owner_type to every explicit SELECT column list so the field is populated when entities are hydrated from those queries (SELECT * queries already include it automatically) - BoardTest: update all jsonSerialize assertions to expect ownerType: 0 No functional changes in this commit; subsequent steps will wire up permission checks, transfer logic, and the UI. AI-assistant: Claude Code 2.1.80 (Claude Sonnet 4.7) Signed-off-by: Jos Poortvliet --- CHANGELOG.md | 1 + lib/Db/Board.php | 4 ++ lib/Db/BoardMapper.php | 14 +++---- .../Version11002Date20260429000000.php | 41 +++++++++++++++++++ tests/unit/Db/BoardTest.php | 4 ++ 5 files changed, 57 insertions(+), 7 deletions(-) create mode 100644 lib/Migration/Version11002Date20260429000000.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4be848709e..3c8a661473 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file. ## 1.16.0-beta.1 ### Added +- feat: add owner_type to boards for circle ownership support @jospoortvliet - feat: update default content @luka-nextcloud [#6740](https://github.com/nextcloud/deck/pull/6740) - feat: add board import and export @luka-nextcloud [#6872](https://github.com/nextcloud/deck/pull/6872) - feat: use outline icons @luka-nextcloud [#7114](https://github.com/nextcloud/deck/pull/7114) diff --git a/lib/Db/Board.php b/lib/Db/Board.php index a0663dbd3d..80865dcb67 100644 --- a/lib/Db/Board.php +++ b/lib/Db/Board.php @@ -22,6 +22,8 @@ * @method void setLastModified(int $lastModified) * @method string getOwner() * @method void setOwner(string $owner) + * @method int getOwnerType() + * @method void setOwnerType(int $ownerType) * @method string getColor() * @method void setColor(string $color) * @method void setShareToken(string $shareToken) @@ -32,6 +34,7 @@ class Board extends RelationalEntity { protected $title; protected $owner; + protected $ownerType = 0; protected $color; protected $archived = false; /** @var Label[]|null */ @@ -53,6 +56,7 @@ class Board extends RelationalEntity { public function __construct() { $this->addType('id', 'integer'); $this->addType('shared', 'integer'); + $this->addType('ownerType', 'integer'); $this->addType('archived', 'boolean'); $this->addType('deletedAt', 'integer'); $this->addType('lastModified', 'integer'); diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 2583a117cd..723a9ca5de 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -192,7 +192,7 @@ public function findAllByUser(string $userId, ?int $limit = null, ?int $offset = // FIXME this used to be a UNION to get boards owned by $userId and the user shares in one single query // Is it possible with the query builder? $qb = $this->db->getQueryBuilder(); - $qb->select('id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') // this does not work in MySQL/PostgreSQL //->selectAlias('0', 'shared') ->from('deck_boards', 'b') @@ -232,7 +232,7 @@ public function findAllByUser(string $userId, ?int $limit = null, ?int $offset = // shared with user $qb = $this->db->getQueryBuilder(); - $qb->select('b.id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') //->selectAlias('1', 'shared') ->from('deck_boards', 'b') ->innerJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id')) @@ -300,7 +300,7 @@ public function findAllByGroups(string $userId, array $groups, ?int $limit = nul return []; } $qb = $this->db->getQueryBuilder(); - $qb->select('b.id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') //->selectAlias('2', 'shared') ->from('deck_boards', 'b') ->innerJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id')) @@ -356,7 +356,7 @@ public function findAllByCircles(string $userId, ?int $limit = null, ?int $offse } $qb = $this->db->getQueryBuilder(); - $qb->select('b.id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') //->selectAlias('2', 'shared') ->from('deck_boards', 'b') ->innerJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id')) @@ -406,7 +406,7 @@ public function findAllByCircles(string $userId, ?int $limit = null, ?int $offse public function findAllByTeam(string $teamId): array { $qb = $this->db->getQueryBuilder(); - $qb->select('b.id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') ->from('deck_boards', 'b') ->innerJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id')) ->where($qb->expr()->eq('acl.type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_CIRCLE, IQueryBuilder::PARAM_INT))) @@ -434,7 +434,7 @@ public function findTeamsForBoard(int $boardId): array { public function isSharedWithTeam(int $boardId, string $teamId): bool { $qb = $this->db->getQueryBuilder(); - $qb->select('b.id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') ->from('deck_boards', 'b') ->innerJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id')) ->where($qb->expr()->eq('b.id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))) @@ -460,7 +460,7 @@ public function findToDelete() { // add buffer of 5 min $timeLimit = time() - (60 * 5); $qb = $this->db->getQueryBuilder(); - $qb->select('id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') ->from('deck_boards') ->where($qb->expr()->gt('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->lt('deleted_at', $qb->createNamedParameter($timeLimit, IQueryBuilder::PARAM_INT))); diff --git a/lib/Migration/Version11002Date20260429000000.php b/lib/Migration/Version11002Date20260429000000.php new file mode 100644 index 0000000000..972687399a --- /dev/null +++ b/lib/Migration/Version11002Date20260429000000.php @@ -0,0 +1,41 @@ +hasTable('deck_boards')) { + $table = $schema->getTable('deck_boards'); + if (!$table->hasColumn('owner_type')) { + $table->addColumn('owner_type', 'smallint', [ + 'notnull' => true, + 'default' => 0, + 'unsigned' => false, + ]); + } + } + + return $schema; + } +} diff --git a/tests/unit/Db/BoardTest.php b/tests/unit/Db/BoardTest.php index 5d729b518e..0fff6620b0 100644 --- a/tests/unit/Db/BoardTest.php +++ b/tests/unit/Db/BoardTest.php @@ -22,6 +22,7 @@ public function testJsonSerialize() { 'id' => 1, 'title' => 'My Board', 'owner' => 'admin', + 'ownerType' => 0, 'color' => '000000', 'labels' => [], 'permissions' => [], @@ -48,6 +49,7 @@ public function testUnfetchedValues() { 'id' => 1, 'title' => 'My Board', 'owner' => 'admin', + 'ownerType' => 0, 'color' => '000000', 'labels' => [], 'permissions' => [], @@ -72,6 +74,7 @@ public function testSetLabels() { 'id' => 1, 'title' => 'My Board', 'owner' => 'admin', + 'ownerType' => 0, 'color' => '000000', 'labels' => ['foo', 'bar'], 'permissions' => [], @@ -103,6 +106,7 @@ public function testSetShared() { 'id' => 1, 'title' => 'My Board', 'owner' => 'admin', + 'ownerType' => 0, 'color' => '000000', 'labels' => [], 'permissions' => [], From 3629a1e5e4ceb29a70bb3b875722b425b65ccb5f Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Wed, 29 Apr 2026 19:29:00 +0200 Subject: [PATCH 02/20] feat: wire circle ownership into BoardMapper mapOwner(): when owner_type = PERMISSION_TYPE_CIRCLE (7), resolve the owner string as a circle ID via CirclesService and return a Circle object, matching the behaviour already used in mapAcl() for circle ACL entries. The federated-user and plain-user paths are unchanged. findAllByCircleOwner(): new method that finds boards where owner_type = 7 and owner is a circle the requesting user belongs to. Follows the same filter-parameter contract as the other findAllBy* methods; sets shared = 0 (user is effectively an owner, not just a collaborator). findAllForUser(): includes findAllByCircleOwner() results in the merged board list alongside the existing user, group, and circle-share sources. findBoardIds(): adds a third query segment for circle-owned boards, reusing the $circles list already fetched for the circle-share segment. transferOwnership(): adds an optional $newOwnerType parameter (default PERMISSION_TYPE_USER, placed after $boardId to preserve backward compatibility) and stores it as owner_type in the UPDATE, so a future transfer to a circle atomically sets both owner and owner_type. No functional change for existing user-owned boards; all new paths either return empty results (no circles app / user in no circles) or are blocked by the as-yet-unchanged PermissionService (step 4). AI-assistant: Claude Code 2.1.80 (Claude Sonnet 4.7) Signed-off-by: Jos Poortvliet --- CHANGELOG.md | 1 + lib/Db/BoardMapper.php | 99 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 95 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c8a661473..f38327ac5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file. ### Added - feat: add owner_type to boards for circle ownership support @jospoortvliet +- feat: resolve circle board owners in BoardMapper and include circle-owned boards in user board queries @jospoortvliet - feat: update default content @luka-nextcloud [#6740](https://github.com/nextcloud/deck/pull/6740) - feat: add board import and export @luka-nextcloud [#6872](https://github.com/nextcloud/deck/pull/6872) - feat: use outline icons @luka-nextcloud [#7114](https://github.com/nextcloud/deck/pull/7114) diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 723a9ca5de..dfdd5a84af 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -131,7 +131,23 @@ public function findBoardIds(string $userId): array { return (int)$id; }, $result->fetchAll(\PDO::FETCH_COLUMN)); $result->closeCursor(); - return array_unique(array_merge($ownerBoards, $sharedBoards)); + + // Owned by circles the user is in (reuse $circles already fetched above) + $circleOwnedBoards = []; + if (count($circles) !== 0) { + $qb = $this->db->getQueryBuilder(); + $qb->selectDistinct('b.id') + ->from($this->getTableName(), 'b') + ->where($qb->expr()->eq('owner_type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_CIRCLE, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->in('owner', $qb->createNamedParameter($circles, IQueryBuilder::PARAM_STR_ARRAY))); + $result = $qb->executeQuery(); + $circleOwnedBoards = array_map(function (string $id) { + return (int)$id; + }, $result->fetchAll(\PDO::FETCH_COLUMN)); + $result->closeCursor(); + } + + return array_unique(array_merge($ownerBoards, $sharedBoards, $circleOwnedBoards)); } /** * @param int $externalId @@ -156,7 +172,8 @@ public function findAllForUser(string $userId, ?int $since = null, bool $include $userBoards = $this->findAllByUser($userId, null, null, $since, $includeArchived, $before, $term); $groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived, $before, $term); $circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived, $before, $term); - $allBoards = array_values(array_unique(array_merge($userBoards, $groupBoards, $circleBoards))); + $circleOwnedBoards = $this->findAllByCircleOwner($userId, null, null, $since, $includeArchived, $before, $term); + $allBoards = array_values(array_unique(array_merge($userBoards, $groupBoards, $circleBoards, $circleOwnedBoards))); // Could be moved outside $acls = $this->aclMapper->findIn(array_map(function ($board) { @@ -404,6 +421,60 @@ public function findAllByCircles(string $userId, ?int $limit = null, ?int $offse return $entries; } + /** + * Find all boards that are owned by a circle the given user is a member of. + * + * These are boards where owner_type = PERMISSION_TYPE_CIRCLE and the owner field + * holds a circle ID that the user belongs to. They are distinct from boards that + * are merely *shared* with a circle via an ACL entry (handled by findAllByCircles). + */ + public function findAllByCircleOwner(string $userId, ?int $limit = null, ?int $offset = null, ?int $since = null, + bool $includeArchived = true, ?int $before = null, ?string $term = null): array { + $circles = $this->circlesService->getUserCircles($userId); + if (count($circles) === 0) { + return []; + } + + $qb = $this->db->getQueryBuilder(); + $qb->select('id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + ->from('deck_boards') + ->where($qb->expr()->eq('owner_type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_CIRCLE, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->in('owner', $qb->createNamedParameter($circles, IQueryBuilder::PARAM_STR_ARRAY))); + if (!$includeArchived) { + $qb->andWhere($qb->expr()->eq('archived', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))) + ->andWhere($qb->expr()->eq('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + } + if ($since !== null) { + $qb->andWhere($qb->expr()->gt('last_modified', $qb->createNamedParameter($since, IQueryBuilder::PARAM_INT))); + } + if ($before !== null) { + $qb->andWhere($qb->expr()->lt('last_modified', $qb->createNamedParameter($before, IQueryBuilder::PARAM_INT))); + } + if ($term !== null) { + $qb->andWhere( + $qb->expr()->iLike( + 'title', + $qb->createNamedParameter( + '%' . $this->db->escapeLikeParameter($term) . '%', + IQueryBuilder::PARAM_STR + ) + ) + ); + } + $qb->orderBy('id'); + if ($limit !== null) { + $qb->setMaxResults($limit); + } + if ($offset !== null) { + $qb->setFirstResult($offset); + } + $entries = $this->findEntities($qb); + foreach ($entries as $entry) { + $entry->setShared(0); + } + return $entries; + } + public function findAllByTeam(string $teamId): array { $qb = $this->db->getQueryBuilder(); $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') @@ -540,11 +611,28 @@ public function mapAcl(Acl &$acl): void { /** * @param Board $board */ - public function mapOwner(Board &$board) { + public function mapOwner(Board &$board): void { $userManager = $this->userManager; $cloudIdManager = $this->cloudIdManager; + $circlesService = $this->circlesService; + $logger = $this->logger; $externalId = $board->getExternalId(); - $board->resolveRelation('owner', function ($owner) use (&$userManager, &$cloudIdManager, $externalId) { + $ownerType = $board->getOwnerType(); + $board->resolveRelation('owner', function ($owner) use ($userManager, $cloudIdManager, $circlesService, $logger, $externalId, $ownerType) { + if ($ownerType === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$circlesService->isCirclesEnabled()) { + return null; + } + try { + $circle = $circlesService->getCircle($owner); + if ($circle !== null) { + return new Circle($circle); + } + } catch (\Throwable $e) { + $logger->error('Failed to get circle details when mapping board owner', ['exception' => $e]); + } + return null; + } if ($externalId !== null) { $cloudId = $cloudIdManager->resolveCloudId($owner); return new FederatedUser($cloudId); @@ -559,10 +647,11 @@ public function mapOwner(Board &$board) { /** * @throws \OCP\DB\Exception */ - public function transferOwnership(string $ownerId, string $newOwnerId, $boardId = null): void { + public function transferOwnership(string $ownerId, string $newOwnerId, ?int $boardId = null, int $newOwnerType = Acl::PERMISSION_TYPE_USER): void { $qb = $this->db->getQueryBuilder(); $qb->update('deck_boards') ->set('owner', $qb->createNamedParameter($newOwnerId, IQueryBuilder::PARAM_STR)) + ->set('owner_type', $qb->createNamedParameter($newOwnerType, IQueryBuilder::PARAM_INT)) ->where($qb->expr()->eq('owner', $qb->createNamedParameter($ownerId, IQueryBuilder::PARAM_STR))); if ($boardId !== null) { $qb->andWhere($qb->expr()->eq('id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))); From 67960bd351ff315a7618e876f2ea8d5cc8228e9d Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Wed, 29 Apr 2026 19:30:04 +0200 Subject: [PATCH 03/20] feat: grant owner-level permissions to circle members on circle-owned boards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit userIsBoardOwner(): when the board's owner_type is PERMISSION_TYPE_CIRCLE, delegate to CirclesService::isUserInCircle() instead of comparing the owner string directly to the user ID. Because getPermissions() and matchPermissions() both gate every permission on userIsBoardOwner(), this single change gives every circle member full read/edit/manage/share access to a circle-owned board with no further changes to the permission stack. findUsers(): for circle-owned boards the owner field holds a circle ID, not a user ID, so the existing "add board owner as a User" path would create a dangling entry. It is replaced by an expansion of the owning circle's inherited members, reusing the same Member::LEVEL_MEMBER + getUserType()===1 guard already present for circle ACL entries below. Tests: add testUserIsBoardOwnerCircleMember covering the member→true and non-member→false cases for a circle-owned board. AI-assistant: Claude Code 2.1.80 (Claude Sonnet 4.7) Signed-off-by: Jos Poortvliet --- CHANGELOG.md | 1 + lib/Service/PermissionService.php | 31 ++++++++++++++++++-- tests/unit/Service/PermissionServiceTest.php | 21 +++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f38327ac5f..80ec22c2c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ All notable changes to this project will be documented in this file. ### Added - feat: add owner_type to boards for circle ownership support @jospoortvliet - feat: resolve circle board owners in BoardMapper and include circle-owned boards in user board queries @jospoortvliet +- feat: grant full owner permissions to circle members on circle-owned boards @jospoortvliet - feat: update default content @luka-nextcloud [#6740](https://github.com/nextcloud/deck/pull/6740) - feat: add board import and export @luka-nextcloud [#6872](https://github.com/nextcloud/deck/pull/6872) - feat: use outline icons @luka-nextcloud [#7114](https://github.com/nextcloud/deck/pull/7114) diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 4de5327367..3ca174d6b9 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -180,12 +180,15 @@ public function checkPermission(?IPermissionMapper $mapper, $id, int $permission * @param $boardId * @return bool */ - public function userIsBoardOwner($boardId, $userId = null, bool $allowDeleted = false) { + public function userIsBoardOwner($boardId, $userId = null, bool $allowDeleted = false): bool { if ($userId === null) { $userId = $this->userId; } try { $board = $this->getBoard($boardId, $allowDeleted); + if ($board->getOwnerType() === Acl::PERMISSION_TYPE_CIRCLE) { + return $this->circlesService->isUserInCircle($board->getOwner(), $userId); + } return $userId === $board->getOwner(); } catch (DoesNotExistException|MultipleObjectsReturnedException $e) { } @@ -283,7 +286,31 @@ public function findUsers($boardId, $refresh = false) { } /** @var array */ $users = []; - if (!$this->userManager->userExists($board->getOwner())) { + if ($board->getOwnerType() === Acl::PERMISSION_TYPE_CIRCLE) { + // Board is owned by a circle: expand its members just like a circle ACL entry below. + if ($this->circlesService->isCirclesEnabled()) { + try { + $owningCircle = $this->circlesService->getCircle($board->getOwner()); + if ($owningCircle !== null) { + foreach ($owningCircle->getInheritedMembers() as $member) { + if ($member->getUserType() !== 1 || $member->getLevel() < Member::LEVEL_MEMBER) { + continue; + } + $user = $this->userManager->get($member->getUserId()); + if ($user === null) { + $this->logger->info('No user found for circle owner member ' . $member->getUserId()); + } else { + $users[(string)$member->getUserId()] = new User($member->getUserId(), $this->userManager); + } + } + } else { + $this->logger->info('No circle found for board owner ' . $board->getOwner()); + } + } catch (\Exception $e) { + $this->logger->error('Failed to expand circle owner members for board ' . $board->getId(), ['exception' => $e]); + } + } + } elseif (!$this->userManager->userExists($board->getOwner())) { $this->logger->info('No owner found for board ' . $board->getId()); } else { $users[(string)$board->getOwner()] = new User($board->getOwner(), $this->userManager); diff --git a/tests/unit/Service/PermissionServiceTest.php b/tests/unit/Service/PermissionServiceTest.php index 3099b9edd2..cc9257386b 100644 --- a/tests/unit/Service/PermissionServiceTest.php +++ b/tests/unit/Service/PermissionServiceTest.php @@ -156,6 +156,27 @@ public function testUserIsBoardOwnerNull() { $this->assertEquals(false, $this->service->userIsBoardOwner(123)); } + public function testUserIsBoardOwnerCircleMember() { + $board = new Board(); + $board->setOwner('circle-id-abc'); + $board->setOwnerType(Acl::PERMISSION_TYPE_CIRCLE); + + $this->boardMapper->expects($this->exactly(2)) + ->method('find') + ->willReturn($board); + + $this->circlesService->expects($this->exactly(2)) + ->method('isUserInCircle') + ->with('circle-id-abc', $this->anything()) + ->willReturnMap([ + ['circle-id-abc', 'admin', true], + ['circle-id-abc', 'user1', false], + ]); + + $this->assertEquals(true, $this->service->userIsBoardOwner(123, 'admin')); + $this->assertEquals(false, $this->service->userIsBoardOwner(123, 'user1')); + } + public static function dataTestUserCan() { return [ // participant permissions type From 003221fafa3b1b610fe0b3490e26e1e285da50ef Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Wed, 29 Apr 2026 23:34:44 +0200 Subject: [PATCH 04/20] feat: support transferring board ownership to a circle in BoardService transferBoardOwnership() gains a newOwnerType parameter (default PERMISSION_TYPE_USER, backward-compatible). Validates new owner before any DB change: userExists() for user targets, CirclesService::getCircle() for circle targets, throwing BadRequestException on failure (also fixes the silent corruption bug when transferring to a non-existent user). For circle transfers: correct ACL type used in deleteParticipantFromBoard, content remap is skipped (card owners cannot map to a circle), previous user owner receives a back-fill ACL entry unless changeContent=true. transferOwnership() (bulk OCC path) gains the same newOwnerType parameter and switches to findAllByOwner so it works for both user-owned and circle-owned boards. CirclesService added to the constructor for circle validation. Tests: transfer to circle, to missing user, to missing circle. AI-assistant: Claude Code 2.1.80 (Claude Sonnet 4.7) Signed-off-by: Jos Poortvliet --- CHANGELOG.md | 1 + lib/Service/BoardService.php | 42 +++++++--- tests/unit/Service/BoardServiceTest.php | 102 ++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80ec22c2c6..2aea2e9aa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file. - feat: add owner_type to boards for circle ownership support @jospoortvliet - feat: resolve circle board owners in BoardMapper and include circle-owned boards in user board queries @jospoortvliet - feat: grant full owner permissions to circle members on circle-owned boards @jospoortvliet +- feat: support transferring board ownership to a circle in BoardService @jospoortvliet - feat: update default content @luka-nextcloud [#6740](https://github.com/nextcloud/deck/pull/6740) - feat: add board import and export @luka-nextcloud [#6872](https://github.com/nextcloud/deck/pull/6872) - feat: use outline icons @luka-nextcloud [#7114](https://github.com/nextcloud/deck/pull/7114) diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index e08b34b8c9..49f5c84ef6 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -82,6 +82,7 @@ public function __construct( private IUserManager $userManager, private ISecureRandom $random, private ConfigService $configService, + private CirclesService $circlesService, private ?string $userId, ) { } @@ -615,14 +616,34 @@ public function clone( return $this->find($newBoard->getId()); } - public function transferBoardOwnership(int $boardId, string $newOwner, bool $changeContent = false): Board { + public function transferBoardOwnership(int $boardId, string $newOwner, bool $changeContent = false, int $newOwnerType = Acl::PERMISSION_TYPE_USER): Board { $this->connection->beginTransaction(); try { $board = $this->boardMapper->find($boardId); $previousOwner = $board->getOwner(); + + // Validate the new owner before touching anything + if ($newOwnerType === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$this->circlesService->isCirclesEnabled()) { + throw new BadRequestException('The Circles app is not enabled'); + } + if ($this->circlesService->getCircle($newOwner) === null) { + throw new BadRequestException('Circle not found: ' . $newOwner); + } + } else { + if (!$this->userManager->userExists($newOwner)) { + throw new BadRequestException('User not found: ' . $newOwner); + } + } + $this->clearBoardFromCache($board); - $this->aclMapper->deleteParticipantFromBoard($boardId, Acl::PERMISSION_TYPE_USER, $newOwner); - if (!$changeContent) { + // Remove new owner from ACL (avoids a duplicate entry once they become the owner) + $this->aclMapper->deleteParticipantFromBoard($boardId, $newOwnerType, $newOwner); + + // Preserve the previous owner's access via an explicit ACL entry unless the + // caller opted into a full content transfer. Skip for circle previous-owners + // because a circle ID is not a valid user participant. + if (!$changeContent && $board->getOwnerType() === Acl::PERMISSION_TYPE_USER) { try { $this->addAcl($boardId, Acl::PERMISSION_TYPE_USER, $previousOwner, true, true, true); } catch (DbException $e) { @@ -631,13 +652,15 @@ public function transferBoardOwnership(int $boardId, string $newOwner, bool $cha } } } - $this->boardMapper->transferOwnership($previousOwner, $newOwner, $boardId); - // Optionally also change user assignments and card owner information - if ($changeContent) { + $this->boardMapper->transferOwnership($previousOwner, $newOwner, $boardId, $newOwnerType); + + // Card-content remap is only meaningful when transferring to a user, not a circle + if ($changeContent && $newOwnerType === Acl::PERMISSION_TYPE_USER) { $this->assignedUsersMapper->remapAssignedUser($boardId, $previousOwner, $newOwner); $this->cardMapper->remapCardOwner($boardId, $previousOwner, $newOwner); } + $this->connection->commit(); return $this->boardMapper->find($boardId); } catch (\Throwable $e) { @@ -646,11 +669,12 @@ public function transferBoardOwnership(int $boardId, string $newOwner, bool $cha } } - public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false): \Generator { - $boards = $this->boardMapper->findAllByUser($owner); + public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false, int $newOwnerType = Acl::PERMISSION_TYPE_USER): \Generator { + // findAllByOwner uses SELECT * so it works for both user-owned and circle-owned boards + $boards = $this->boardMapper->findAllByOwner($owner); foreach ($boards as $board) { if ($board->getOwner() === $owner) { - yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent); + yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent, $newOwnerType); } } } diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index 549021bf0c..faea13853d 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -32,6 +32,7 @@ use OC\L10N\L10N; use OC\Security\SecureRandom; use OCA\Deck\Activity\ActivityManager; +use OCA\Deck\BadRequestException; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; use OCA\Deck\Db\Assignment; @@ -103,6 +104,8 @@ class BoardServiceTest extends TestCase { /** @var IUserManager */ private $userManager; + /** @var CirclesService|MockObject */ + private $circlesService; public function setUp(): void { parent::setUp(); @@ -125,6 +128,7 @@ public function setUp(): void { $this->boardServiceValidator = $this->createMock(BoardServiceValidator::class); $this->sessionMapper = $this->createMock(SessionMapper::class); $this->userManager = $this->createMock(IUserManager::class); + $this->circlesService = $this->createMock(CirclesService::class); $this->service = new BoardService( $this->boardMapper, @@ -151,6 +155,7 @@ public function setUp(): void { $this->userManager, $this->createMock(SecureRandom::class), $this->createMock(ConfigService::class), + $this->circlesService, $this->userId ); @@ -474,4 +479,101 @@ public function testDeleteAcl() { ->with(new AclDeletedEvent($acl)); $this->assertEquals($acl, $this->service->deleteAcl(123)); } + + public function testTransferBoardOwnershipToCircle(): void { + $board = new Board(); + $board->setId(10); + $board->setOwner('alice'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $updatedBoard = new Board(); + $updatedBoard->setId(10); + $updatedBoard->setOwner('circle-id-xyz'); + $updatedBoard->setOwnerType(Acl::PERMISSION_TYPE_CIRCLE); + + $this->connection->expects($this->once())->method('beginTransaction'); + $this->connection->expects($this->once())->method('commit'); + + $this->boardMapper->expects($this->exactly(2)) + ->method('find') + ->willReturnOnConsecutiveCalls($board, $updatedBoard); + + $this->circlesService->expects($this->once()) + ->method('isCirclesEnabled') + ->willReturn(true); + $this->circlesService->expects($this->once()) + ->method('getCircle') + ->with('circle-id-xyz') + ->willReturn($this->createMock(\OCA\Circles\Model\Circle::class)); + + $this->aclMapper->expects($this->once()) + ->method('deleteParticipantFromBoard') + ->with(10, Acl::PERMISSION_TYPE_CIRCLE, 'circle-id-xyz'); + + // Previous user owner gets an ACL entry when changeContent = false + $this->aclMapper->expects($this->once()) + ->method('findAll') + ->willReturn([]); + $this->aclMapper->expects($this->once()) + ->method('insert') + ->willReturnCallback(fn ($acl) => $acl); + + $this->boardMapper->expects($this->once()) + ->method('transferOwnership') + ->with('alice', 'circle-id-xyz', 10, Acl::PERMISSION_TYPE_CIRCLE); + + // No content remap when target is a circle + $this->assignedUsersMapper->expects($this->never())->method('remapAssignedUser'); + $this->cardMapper->expects($this->never())->method('remapCardOwner'); + + $result = $this->service->transferBoardOwnership(10, 'circle-id-xyz', false, Acl::PERMISSION_TYPE_CIRCLE); + $this->assertSame($updatedBoard, $result); + } + + public function testTransferBoardOwnershipToNonExistentUserThrows(): void { + $board = new Board(); + $board->setId(10); + $board->setOwner('alice'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $this->connection->expects($this->once())->method('beginTransaction'); + $this->connection->expects($this->once())->method('rollBack'); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->willReturn($board); + + $this->userManager->expects($this->once()) + ->method('userExists') + ->with('ghost') + ->willReturn(false); + + $this->expectException(BadRequestException::class); + $this->service->transferBoardOwnership(10, 'ghost', false, Acl::PERMISSION_TYPE_USER); + } + + public function testTransferBoardOwnershipToNonExistentCircleThrows(): void { + $board = new Board(); + $board->setId(10); + $board->setOwner('alice'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $this->connection->expects($this->once())->method('beginTransaction'); + $this->connection->expects($this->once())->method('rollBack'); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->willReturn($board); + + $this->circlesService->expects($this->once()) + ->method('isCirclesEnabled') + ->willReturn(true); + $this->circlesService->expects($this->once()) + ->method('getCircle') + ->with('bad-circle') + ->willReturn(null); + + $this->expectException(BadRequestException::class); + $this->service->transferBoardOwnership(10, 'bad-circle', false, Acl::PERMISSION_TYPE_CIRCLE); + } } From 1cb1761f3850f30661831320d836b5b3c0976309 Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Wed, 29 Apr 2026 23:35:02 +0200 Subject: [PATCH 05/20] feat: accept newOwnerType in the transfer-ownership REST endpoint BoardController::transferOwner() now accepts an optional newOwnerType parameter (0=user, 7=circle). Unknown values return HTTP 400. The validated type is forwarded to BoardService::transferBoardOwnership(). AI-assistant: Claude Code 2.1.80 (Claude Sonnet 4.7) Signed-off-by: Jos Poortvliet --- CHANGELOG.md | 1 + lib/Controller/BoardController.php | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aea2e9aa7..b396db3e04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file. - feat: resolve circle board owners in BoardMapper and include circle-owned boards in user board queries @jospoortvliet - feat: grant full owner permissions to circle members on circle-owned boards @jospoortvliet - feat: support transferring board ownership to a circle in BoardService @jospoortvliet +- feat: accept newOwnerType in the transfer-ownership REST endpoint @jospoortvliet - feat: update default content @luka-nextcloud [#6740](https://github.com/nextcloud/deck/pull/6740) - feat: add board import and export @luka-nextcloud [#6872](https://github.com/nextcloud/deck/pull/6872) - feat: use outline icons @luka-nextcloud [#7114](https://github.com/nextcloud/deck/pull/7114) diff --git a/lib/Controller/BoardController.php b/lib/Controller/BoardController.php index 5c43ab7b12..3a3ae7e7c1 100644 --- a/lib/Controller/BoardController.php +++ b/lib/Controller/BoardController.php @@ -122,11 +122,13 @@ public function clone(int $boardId, bool $withCards = false, bool $withAssignmen /** * @NoAdminRequired */ - public function transferOwner(int $boardId, string $newOwner): DataResponse { + public function transferOwner(int $boardId, string $newOwner, int $newOwnerType = Acl::PERMISSION_TYPE_USER): DataResponse { + if ($newOwnerType !== Acl::PERMISSION_TYPE_USER && $newOwnerType !== Acl::PERMISSION_TYPE_CIRCLE) { + return new DataResponse(['message' => 'Invalid owner type'], HTTP::STATUS_BAD_REQUEST); + } if ($this->permissionService->userIsBoardOwner($boardId, $this->userId)) { - return new DataResponse($this->boardService->transferBoardOwnership($boardId, $newOwner), HTTP::STATUS_OK); + return new DataResponse($this->boardService->transferBoardOwnership($boardId, $newOwner, false, $newOwnerType), HTTP::STATUS_OK); } - return new DataResponse([], HTTP::STATUS_UNAUTHORIZED); } From 92e5075dccf57c6bfb00b1e67aba599e9ba16137 Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Wed, 29 Apr 2026 23:35:37 +0200 Subject: [PATCH 06/20] feat: add --to-circle flag to deck:transfer-ownership OCC command New --to-circle option treats the newOwner argument as a circle ID. The command labels output accordingly, wraps the transfer in an error handler so invalid circle IDs print a clean message, and forwards PERMISSION_TYPE_CIRCLE to the service layer. Error messages are now surfaced for both single-board and bulk transfers. AI-assistant: Claude Code 2.1.80 (Claude Sonnet 4.7) Signed-off-by: Jos Poortvliet --- CHANGELOG.md | 1 + lib/Command/TransferOwnership.php | 38 +++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b396db3e04..f1923a4f54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ All notable changes to this project will be documented in this file. - feat: grant full owner permissions to circle members on circle-owned boards @jospoortvliet - feat: support transferring board ownership to a circle in BoardService @jospoortvliet - feat: accept newOwnerType in the transfer-ownership REST endpoint @jospoortvliet +- feat: add --to-circle flag to deck:transfer-ownership OCC command @jospoortvliet - feat: update default content @luka-nextcloud [#6740](https://github.com/nextcloud/deck/pull/6740) - feat: add board import and export @luka-nextcloud [#6872](https://github.com/nextcloud/deck/pull/6872) - feat: use outline icons @luka-nextcloud [#7114](https://github.com/nextcloud/deck/pull/7114) diff --git a/lib/Command/TransferOwnership.php b/lib/Command/TransferOwnership.php index dfa9a01c4b..fd09e65e86 100644 --- a/lib/Command/TransferOwnership.php +++ b/lib/Command/TransferOwnership.php @@ -6,6 +6,7 @@ */ namespace OCA\Deck\Command; +use OCA\Deck\Db\Acl; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Service\BoardService; use OCA\Deck\Service\PermissionService; @@ -57,6 +58,12 @@ protected function configure() { InputOption::VALUE_NONE, 'Reassign card details of the old owner to the new one' ) + ->addOption( + 'to-circle', + null, + InputOption::VALUE_NONE, + 'Treat as a circle ID instead of a user UID' + ) ; } @@ -64,8 +71,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int $owner = $input->getArgument('owner'); $newOwner = $input->getArgument('newOwner'); $boardId = $input->getArgument('boardId'); - $remapAssignment = $input->getOption('remap'); + $toCircle = $input->getOption('to-circle'); + $newOwnerType = $toCircle ? Acl::PERMISSION_TYPE_CIRCLE : Acl::PERMISSION_TYPE_USER; + $newOwnerLabel = $toCircle ? "circle $newOwner" : $newOwner; $this->boardService->setUserId($owner); $this->permissionService->setUserId($owner); @@ -83,9 +92,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int } if ($boardId) { - $output->writeln('Transfer board ' . $board->getTitle() . ' from ' . $board->getOwner() . " to $newOwner"); + $output->writeln('Transfer board ' . $board->getTitle() . ' from ' . $board->getOwner() . " to $newOwnerLabel"); } else { - $output->writeln("Transfer all boards from $owner to $newOwner"); + $output->writeln("Transfer all boards from $owner to $newOwnerLabel"); } $question = new ConfirmationQuestion('Do you really want to continue? (y/n) ', false); @@ -93,16 +102,21 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - if ($boardId) { - $this->boardService->transferBoardOwnership($boardId, $newOwner, $remapAssignment); - $output->writeln('Board ' . $board->getTitle() . ' from ' . $board->getOwner() . " transferred to $newOwner completed"); - return 0; - } - - foreach ($this->boardService->transferOwnership($owner, $newOwner, $remapAssignment) as $board) { - $output->writeln(' - ' . $board->getTitle() . ' transferred'); + try { + if ($boardId) { + $this->boardService->transferBoardOwnership($boardId, $newOwner, $remapAssignment, $newOwnerType); + $output->writeln('Board ' . $board->getTitle() . " transferred to $newOwnerLabel"); + return 0; + } + + foreach ($this->boardService->transferOwnership($owner, $newOwner, $remapAssignment, $newOwnerType) as $board) { + $output->writeln(' - ' . $board->getTitle() . ' transferred'); + } + $output->writeln("All boards from $owner transferred to $newOwnerLabel"); + } catch (\Exception $e) { + $output->writeln('' . $e->getMessage() . ''); + return 1; } - $output->writeln("All boards from $owner to $newOwner transferred"); return 0; } From d2b7924c9756c8148e91835c0bf6436660b80252 Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Wed, 29 Apr 2026 23:37:13 +0200 Subject: [PATCH 07/20] feat: show team icon for circle-owned boards and add Transfer ownership button SharingTabSidebar: when board.ownerType === 7, render a team icon instead of NcAvatar for the owner row and label it Team. The hidden Owner NcActionCheckbox is replaced by a NcActionButton labelled Transfer ownership. For user-owned boards it appears only for user ACL entries when the current user is the owner (unchanged). For circle-owned boards it appears for any ACL entry when canManage is true. Confirmation dialog and success/error messages include the target label (team name or user ID). newOwnerType is forwarded through the Vuex transferOwnership action to the PUT payload. BoardItem: guard NcAvatar with v-if board.ownerType !== 7 and show a team icon div for circle-owned boards, preventing a lookup of a circle ID as a user avatar. AI-assistant: Claude Code 2.1.80 (Claude Sonnet 4.7) Signed-off-by: Jos Poortvliet --- CHANGELOG.md | 1 + src/components/board/SharingTabSidebar.vue | 42 +++++++++++++++------- src/components/boards/BoardItem.vue | 6 +++- src/store/main.js | 3 +- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1923a4f54..5e3c0e912a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file. - feat: support transferring board ownership to a circle in BoardService @jospoortvliet - feat: accept newOwnerType in the transfer-ownership REST endpoint @jospoortvliet - feat: add --to-circle flag to deck:transfer-ownership OCC command @jospoortvliet +- feat: show team icon for circle-owned boards and add Transfer ownership button in sharing sidebar @jospoortvliet - feat: update default content @luka-nextcloud [#6740](https://github.com/nextcloud/deck/pull/6740) - feat: add board import and export @luka-nextcloud [#6872](https://github.com/nextcloud/deck/pull/6872) - feat: use outline icons @luka-nextcloud [#7114](https://github.com/nextcloud/deck/pull/7114) diff --git a/src/components/board/SharingTabSidebar.vue b/src/components/board/SharingTabSidebar.vue index 035b1f363b..013ad4c5a7 100644 --- a/src/components/board/SharingTabSidebar.vue +++ b/src/components/board/SharingTabSidebar.vue @@ -12,12 +12,16 @@
  • - + +
    {{ board.owner.displayname }} - + {{ t('deck', 'Board owner') }} + + {{ t('deck', 'Team') }} +
  • @@ -51,12 +55,12 @@ @change="clickManageAcl(acl)"> {{ t('deck', 'Can manage') }} - - {{ t('deck', 'Owner') }} - + @click="clickTransferOwner(acl.participant.uid || acl.participant.id, acl.type)"> + {{ t('deck', 'Transfer ownership') }} + uid === getCurrentUser().uid }, + canTransferTo() { + return (acl) => { + // For user-owned boards: only the current owner can transfer, and only to users + if (this.board.ownerType !== 7) { + return acl.type === 0 && this.isCurrentUser(this.board.owner.uid) + } + // For circle-owned boards: any circle member with manage rights can transfer to any participant + return this.canManage + } + }, formatedSharees() { const result = this.unallocatedSharees.map(item => { const res = { @@ -224,10 +238,13 @@ export default { clickDeleteAcl(acl) { this.$store.dispatch('deleteAclFromCurrentBoard', acl) }, - clickTransferOwner(newOwner) { + clickTransferOwner(newOwner, newOwnerType) { + const targetLabel = newOwnerType === 7 + ? t('deck', 'team {name}', { name: newOwner }) + : newOwner OC.dialogs.confirmDestructive( - t('deck', 'Are you sure you want to transfer the board {title} to {user}?', { title: this.board.title, user: newOwner }), - t('deck', 'Transfer the board.'), + t('deck', 'Are you sure you want to transfer the board {title} to {target}?', { title: this.board.title, target: targetLabel }), + t('deck', 'Transfer the board'), { type: OC.dialogs.YES_NO_BUTTONS, confirm: t('deck', 'Transfer'), @@ -241,12 +258,13 @@ export default { await this.$store.dispatch('transferOwnership', { boardId: this.board.id, newOwner, + newOwnerType, }) - const successMessage = t('deck', 'The board has been transferred to {user}', { user: newOwner }) + const successMessage = t('deck', 'The board has been transferred to {target}', { target: targetLabel }) showSuccess(successMessage) this.$router.push({ name: 'main' }) } catch (e) { - const errorMessage = t('deck', 'Failed to transfer the board to {user}', { user: newOwner.user }) + const errorMessage = t('deck', 'Failed to transfer the board to {target}', { target: targetLabel }) showError(errorMessage) } finally { this.isLoading = false diff --git a/src/components/boards/BoardItem.vue b/src/components/boards/BoardItem.vue index 86c174eb4c..66fb2a6ae8 100644 --- a/src/components/boards/BoardItem.vue +++ b/src/components/boards/BoardItem.vue @@ -16,10 +16,14 @@ {{ board.title }}
    - +
    Date: Thu, 30 Apr 2026 00:15:43 +0200 Subject: [PATCH 08/20] fix: use Member::TYPE_USER constant instead of magic integer 1 The circle member type check `getUserType() !== 1` uses a raw integer where a named constant is available and already imported. The existing ACL-circle expansion path already uses the named constant `Member::LEVEL_MEMBER` right next to the same condition, making the inconsistency obvious. Replace both occurrences (circle-owned board owner expansion added in this feature, and the pre-existing ACL-share expansion) with `Member::TYPE_USER`. --- lib/Service/PermissionService.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 3ca174d6b9..81779a54df 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -293,7 +293,7 @@ public function findUsers($boardId, $refresh = false) { $owningCircle = $this->circlesService->getCircle($board->getOwner()); if ($owningCircle !== null) { foreach ($owningCircle->getInheritedMembers() as $member) { - if ($member->getUserType() !== 1 || $member->getLevel() < Member::LEVEL_MEMBER) { + if ($member->getUserType() !== Member::TYPE_USER || $member->getLevel() < Member::LEVEL_MEMBER) { continue; } $user = $this->userManager->get($member->getUserId()); @@ -349,7 +349,7 @@ public function findUsers($boardId, $refresh = false) { } foreach ($circle->getInheritedMembers() as $member) { - if ($member->getUserType() !== 1 || $member->getLevel() < Member::LEVEL_MEMBER) { + if ($member->getUserType() !== Member::TYPE_USER || $member->getLevel() < Member::LEVEL_MEMBER) { // deck currently only supports user members in circles continue; } From 8806ca455151f74b51450ae983e3bd3898b63e8e Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Thu, 30 Apr 2026 00:16:19 +0200 Subject: [PATCH 09/20] fix: scope user-owner queries to owner_type = PERMISSION_TYPE_USER Now that owner_type distinguishes user owners (0) from circle owners (7), queries that look up boards by user ID should explicitly exclude circle-owned boards from the user-owner path. Without this guard, findBoardIds and findAllByUser would accidentally return a circle-owned board if the circle's single ID happened to match the user ID string - impossible today (circle IDs are UUIDs, user IDs are logins) but semantically wrong and a latent bug. Being explicit also makes the intent clear to future readers. --- lib/Db/BoardMapper.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index dfdd5a84af..41ade67f2a 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -86,6 +86,7 @@ public function findBoardIds(string $userId): array { ->from($this->getTableName(), 'b') ->where($qb->expr()->andX( $qb->expr()->eq('owner', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)), + $qb->expr()->eq('b.owner_type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_USER, IQueryBuilder::PARAM_INT)), )); $result = $qb->executeQuery(); $ownerBoards = array_map(function (string $id) { @@ -213,7 +214,8 @@ public function findAllByUser(string $userId, ?int $limit = null, ?int $offset = // this does not work in MySQL/PostgreSQL //->selectAlias('0', 'shared') ->from('deck_boards', 'b') - ->where($qb->expr()->eq('owner', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))); + ->where($qb->expr()->eq('owner', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('b.owner_type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_USER, IQueryBuilder::PARAM_INT))); if (!$includeArchived) { $qb->andWhere($qb->expr()->eq('archived', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))) ->andWhere($qb->expr()->eq('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); From 534be4598c8aa1c1e91428dbbff063a4f14d93db Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Thu, 30 Apr 2026 00:16:52 +0200 Subject: [PATCH 10/20] perf: cache getUserCircles result per user within a request findAllForUser now calls both findAllByCircles() and findAllByCircleOwner(), each of which independently calls getUserCircles(). Without caching, every board-list request launches two Circles API sessions (getFederatedUser, startSession, getCircles) for the same user in the same PHP process. Add a $userCirclesCache keyed by userId, mirroring the existing $userCircleCache already used by isUserInCircle. The cache is per-object (per-request in a normal Nextcloud HTTP context), so stale data is not a concern. --- lib/Service/CirclesService.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/Service/CirclesService.php b/lib/Service/CirclesService.php index e604a78698..bafd9949ea 100644 --- a/lib/Service/CirclesService.php +++ b/lib/Service/CirclesService.php @@ -26,6 +26,8 @@ class CirclesService { private bool $circlesEnabled; private $userCircleCache = []; + /** @var array */ + private array $userCirclesCache = []; public function __construct(IAppManager $appManager) { $this->circlesEnabled = $appManager->isEnabledForUser('circles'); @@ -88,15 +90,21 @@ public function getUserCircles(string $userId): array { return []; } + if (isset($this->userCirclesCache[$userId])) { + return $this->userCirclesCache[$userId]; + } + try { $circlesManager = Server::get(CirclesManager::class); $federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER); $circlesManager->startSession($federatedUser); $probe = new CircleProbe(); $probe->mustBeMember(); - return array_map(function (Circle $circle) { + $circles = array_map(function (Circle $circle) { return $circle->getSingleId(); }, $circlesManager->getCircles($probe)); + $this->userCirclesCache[$userId] = $circles; + return $circles; } catch (Throwable $e) { } return []; From 5d4bd01e8153db26f00def52a7b0426f86ac53e8 Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Thu, 30 Apr 2026 00:18:51 +0200 Subject: [PATCH 11/20] refactor: centralise ACL permission type constants in src/helpers/constants.js The feature introduced comparisons like `ownerType !== 7`, `ownerType === 7`, and `newOwnerType === 7` in three different files (SharingTabSidebar.vue, BoardItem.vue, main.js), spreading the magic number 7 (PERMISSION_TYPE_CIRCLE) through the frontend. The same file already defined SOURCE_TO_SHARE_TYPE with `circles: 7` locally, duplicating the constant yet again. Introduce src/helpers/constants.js that exports named constants mirroring the PHP Acl::PERMISSION_TYPE_* values, and move SOURCE_TO_SHARE_TYPE there as well. All three files now import and use the named constants; the local SOURCE_TO_SHARE_TYPE definition in SharingTabSidebar.vue is removed. --- src/components/board/SharingTabSidebar.vue | 24 +++++++++------------ src/components/boards/BoardItem.vue | 8 ++++++- src/helpers/constants.js | 25 ++++++++++++++++++++++ src/store/main.js | 3 ++- 4 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 src/helpers/constants.js diff --git a/src/components/board/SharingTabSidebar.vue b/src/components/board/SharingTabSidebar.vue index 013ad4c5a7..d01c82bedf 100644 --- a/src/components/board/SharingTabSidebar.vue +++ b/src/components/board/SharingTabSidebar.vue @@ -12,14 +12,14 @@
    • - +
      {{ board.owner.displayname }} - + {{ t('deck', 'Board owner') }} - + {{ t('deck', 'Team') }} @@ -89,14 +89,7 @@ import { getCurrentUser } from '@nextcloud/auth' import { showError, showSuccess } from '@nextcloud/dialogs' import { loadState } from '@nextcloud/initial-state' import debounce from 'lodash/debounce.js' -const SOURCE_TO_SHARE_TYPE = { - users: 0, - groups: 1, - emails: 4, - remotes: 6, - circles: 7, - teams: 7, -} +import { PERMISSION_TYPE_CIRCLE, PERMISSION_TYPE_USER, SOURCE_TO_SHARE_TYPE } from '../../helpers/constants.js' export default { name: 'SharingTabSidebar', @@ -123,6 +116,9 @@ export default { addAclForAPI: null, newOwner: null, projectsEnabled: loadState('core', 'projects_enabled', false), + // Expose ACL type constants to the template + PERMISSION_TYPE_CIRCLE, + PERMISSION_TYPE_USER, } }, computed: { @@ -140,8 +136,8 @@ export default { canTransferTo() { return (acl) => { // For user-owned boards: only the current owner can transfer, and only to users - if (this.board.ownerType !== 7) { - return acl.type === 0 && this.isCurrentUser(this.board.owner.uid) + if (this.board.ownerType !== PERMISSION_TYPE_CIRCLE) { + return acl.type === PERMISSION_TYPE_USER && this.isCurrentUser(this.board.owner.uid) } // For circle-owned boards: any circle member with manage rights can transfer to any participant return this.canManage @@ -239,7 +235,7 @@ export default { this.$store.dispatch('deleteAclFromCurrentBoard', acl) }, clickTransferOwner(newOwner, newOwnerType) { - const targetLabel = newOwnerType === 7 + const targetLabel = newOwnerType === PERMISSION_TYPE_CIRCLE ? t('deck', 'team {name}', { name: newOwner }) : newOwner OC.dialogs.confirmDestructive( diff --git a/src/components/boards/BoardItem.vue b/src/components/boards/BoardItem.vue index 66fb2a6ae8..f355a28ef3 100644 --- a/src/components/boards/BoardItem.vue +++ b/src/components/boards/BoardItem.vue @@ -16,7 +16,7 @@ {{ board.title }}
      - import { NcAvatar } from '@nextcloud/vue' +import { PERMISSION_TYPE_CIRCLE } from '../../helpers/constants.js' export default { name: 'BoardItem', @@ -50,6 +51,11 @@ export default { default: () => { return {} }, }, }, + data() { + return { + PERMISSION_TYPE_CIRCLE, + } + }, computed: { routeTo() { return { diff --git a/src/helpers/constants.js b/src/helpers/constants.js new file mode 100644 index 0000000000..620b51cf8d --- /dev/null +++ b/src/helpers/constants.js @@ -0,0 +1,25 @@ +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +/** + * ACL / owner permission types – mirror Acl::PERMISSION_TYPE_* PHP constants. + * Keep in sync with lib/Db/Acl.php. + */ +export const PERMISSION_TYPE_USER = 0 +export const PERMISSION_TYPE_GROUP = 1 +export const PERMISSION_TYPE_REMOTE = 6 +export const PERMISSION_TYPE_CIRCLE = 7 + +/** + * Map from the Nextcloud sharee-picker source identifier to the board ACL type. + */ +export const SOURCE_TO_SHARE_TYPE = { + users: PERMISSION_TYPE_USER, + groups: PERMISSION_TYPE_GROUP, + emails: 4, + remotes: PERMISSION_TYPE_REMOTE, + circles: PERMISSION_TYPE_CIRCLE, + teams: PERMISSION_TYPE_CIRCLE, +} diff --git a/src/store/main.js b/src/store/main.js index 4bdecf4503..b8963d24bf 100644 --- a/src/store/main.js +++ b/src/store/main.js @@ -11,6 +11,7 @@ import Vuex from 'vuex' import axios from '@nextcloud/axios' import { generateOcsUrl, generateUrl } from '@nextcloud/router' import { BoardApi } from '../services/BoardApi.js' +import { PERMISSION_TYPE_USER } from '../helpers/constants.js' import stackModuleFactory from './stack.js' import cardModuleFactory from './card.js' import comment from './comment.js' @@ -524,7 +525,7 @@ export default function storeFactory() { dispatch('loadBoardById', acl.boardId) }) }, - async transferOwnership({ commit }, { boardId, newOwner, newOwnerType = 0 }) { + async transferOwnership({ commit }, { boardId, newOwner, newOwnerType = PERMISSION_TYPE_USER }) { await axios.put(generateUrl(`apps/deck/boards/${boardId}/transferOwner`), { newOwner, newOwnerType, From aaa00287d0bce484daabc096b07bd0bc8ef407a2 Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Thu, 30 Apr 2026 00:19:12 +0200 Subject: [PATCH 12/20] refactor: remove redundant owner equality check in transferOwnership findAllByOwner() already queries WHERE owner = $owner, so every board in the returned collection is guaranteed to have getOwner() === $owner. The inner guard is always true and adds noise without benefit. --- lib/Service/BoardService.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 49f5c84ef6..700d8e1aa2 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -670,12 +670,9 @@ public function transferBoardOwnership(int $boardId, string $newOwner, bool $cha } public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false, int $newOwnerType = Acl::PERMISSION_TYPE_USER): \Generator { - // findAllByOwner uses SELECT * so it works for both user-owned and circle-owned boards $boards = $this->boardMapper->findAllByOwner($owner); foreach ($boards as $board) { - if ($board->getOwner() === $owner) { - yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent, $newOwnerType); - } + yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent, $newOwnerType); } } From 15078655643c8d2e41c5682182ca950be3e899fb Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Thu, 30 Apr 2026 00:19:33 +0200 Subject: [PATCH 13/20] chore: drop explicit unsigned=false from migration column definition Doctrine DBAL defaults to unsigned=false for integer columns; spelling it out adds noise without conveying intent and may imply the choice was deliberate rather than incidental. --- lib/Migration/Version11002Date20260429000000.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Migration/Version11002Date20260429000000.php b/lib/Migration/Version11002Date20260429000000.php index 972687399a..a1b2d4ab73 100644 --- a/lib/Migration/Version11002Date20260429000000.php +++ b/lib/Migration/Version11002Date20260429000000.php @@ -31,7 +31,6 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $table->addColumn('owner_type', 'smallint', [ 'notnull' => true, 'default' => 0, - 'unsigned' => false, ]); } } From 073846c050dbea4ed02e8c8f95ed86914e3c971d Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Thu, 30 Apr 2026 08:39:48 +0200 Subject: [PATCH 14/20] Fix issue in SharingTabSidebar & add migration back Signed-off-by: Jos Poortvliet --- lib/Migration/Version11002Date20260429000000.php | 1 + src/components/board/SharingTabSidebar.vue | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Migration/Version11002Date20260429000000.php b/lib/Migration/Version11002Date20260429000000.php index a1b2d4ab73..972687399a 100644 --- a/lib/Migration/Version11002Date20260429000000.php +++ b/lib/Migration/Version11002Date20260429000000.php @@ -31,6 +31,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $table->addColumn('owner_type', 'smallint', [ 'notnull' => true, 'default' => 0, + 'unsigned' => false, ]); } } diff --git a/src/components/board/SharingTabSidebar.vue b/src/components/board/SharingTabSidebar.vue index d01c82bedf..8969ec656c 100644 --- a/src/components/board/SharingTabSidebar.vue +++ b/src/components/board/SharingTabSidebar.vue @@ -150,7 +150,7 @@ export default { displayName: item.displayname || item.name || item.label || item.id, user: item.id, subname: item.shareWithDisplayNameUnique || item.subline || item.id, // NcSelectUser does its own pattern matching to filter things out - type: SOURCE_TO_SHARE_TYPE[item.source] + type: SOURCE_TO_SHARE_TYPE[item.source], } return res }).slice(0, 10) From 7dae6b0310cdab36ce1a2251b883c49636f9648e Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Thu, 30 Apr 2026 09:15:00 +0200 Subject: [PATCH 15/20] fix: show Transfer ownership button for circle/team ACL entries canTransferTo was guarded with acl.type === PERMISSION_TYPE_USER, which silently excluded circle entries (type=7) from ever seeing the Transfer ownership button. The board backend already accepts PERMISSION_TYPE_CIRCLE as a valid newOwnerType, so the UI restriction had no purpose. Split the eligibility check into two separate concerns: 1. canBeOwnershipTarget: only user and circle participants are valid new owners (groups, remotes, etc. are not). 2. Permission to perform the transfer: current user must be the board owner (user-owned board) or have manage rights (circle-owned board). This means the Transfer ownership button now appears in the ... menu for both user and team/circle participants, as long as the current user has the right to initiate the transfer. --- src/components/board/SharingTabSidebar.vue | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/components/board/SharingTabSidebar.vue b/src/components/board/SharingTabSidebar.vue index 8969ec656c..ce06fee0a1 100644 --- a/src/components/board/SharingTabSidebar.vue +++ b/src/components/board/SharingTabSidebar.vue @@ -135,11 +135,16 @@ export default { }, canTransferTo() { return (acl) => { - // For user-owned boards: only the current owner can transfer, and only to users + const canBeOwnershipTarget = acl.type === PERMISSION_TYPE_USER || acl.type === PERMISSION_TYPE_CIRCLE + if (!canBeOwnershipTarget) { + return false + } + + // For user-owned boards: only the current owner can transfer if (this.board.ownerType !== PERMISSION_TYPE_CIRCLE) { - return acl.type === PERMISSION_TYPE_USER && this.isCurrentUser(this.board.owner.uid) + return this.isCurrentUser(this.board.owner.uid) } - // For circle-owned boards: any circle member with manage rights can transfer to any participant + // For circle-owned boards: any circle member with manage rights can transfer return this.canManage } }, From 68474e281e76243fb6044944980a51673da3cb2d Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Fri, 1 May 2026 11:46:41 +0200 Subject: [PATCH 16/20] add the package lot that was missed earlier, and update .gitignore to not include the php-cs-fixer cache file --- .gitignore | 1 + package-lock.json | 134 ---------------------------------------------- 2 files changed, 1 insertion(+), 134 deletions(-) diff --git a/.gitignore b/.gitignore index e8034c743f..087b37fd2b 100644 --- a/.gitignore +++ b/.gitignore @@ -9,5 +9,6 @@ tests/integration/composer.lock tests/.phpunit.result.cache vendor/ .php_cs.cache +.php-cs-fixer.cache \.idea/ settings.json diff --git a/package-lock.json b/package-lock.json index 271b222139..a9421bae5b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4750,34 +4750,6 @@ "@vue/shared": "3.5.30" } }, - "node_modules/@nextcloud/password-confirmation/node_modules/@vue/devtools-kit": { - "version": "7.7.9", - "resolved": "https://registry.npmjs.org/@vue/devtools-kit/-/devtools-kit-7.7.9.tgz", - "integrity": "sha512-PyQ6odHSgiDVd4hnTP+aDk2X4gl2HmLDfiyEnn3/oV+ckFDuswRs4IbBT7vacMuGdwY/XemxBoh302ctbsptuA==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "@vue/devtools-shared": "^7.7.9", - "birpc": "^2.3.0", - "hookable": "^5.5.3", - "mitt": "^3.0.1", - "perfect-debounce": "^1.0.0", - "speakingurl": "^14.0.1", - "superjson": "^2.2.2" - } - }, - "node_modules/@nextcloud/password-confirmation/node_modules/@vue/devtools-shared": { - "version": "7.7.9", - "resolved": "https://registry.npmjs.org/@vue/devtools-shared/-/devtools-shared-7.7.9.tgz", - "integrity": "sha512-iWAb0v2WYf0QWmxCGy0seZNDPdO3Sp5+u78ORnyeonS6MT4PC7VPrryX2BpMJrwlDeaZ6BD4vP4XKjK0SZqaeA==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "rfdc": "^1.4.1" - } - }, "node_modules/@nextcloud/password-confirmation/node_modules/@vue/server-renderer": { "version": "3.5.30", "resolved": "https://registry.npmjs.org/@vue/server-renderer/-/server-renderer-3.5.30.tgz", @@ -4973,14 +4945,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/@nextcloud/password-confirmation/node_modules/perfect-debounce": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/perfect-debounce/-/perfect-debounce-1.0.0.tgz", - "integrity": "sha512-xCy9V055GLEqoFaHoC1SoLIaLmWctgCUaBaWxDZ7/Zx4CTyX7cJQLJOok/orfjZAh9kEYpjJa4d0KcJmCbctZA==", - "license": "MIT", - "optional": true, - "peer": true - }, "node_modules/@nextcloud/password-confirmation/node_modules/picomatch": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.4.tgz", @@ -4993,40 +4957,6 @@ "url": "https://github.com/sponsors/jonschlinkert" } }, - "node_modules/@nextcloud/password-confirmation/node_modules/pinia": { - "version": "3.0.4", - "resolved": "https://registry.npmjs.org/pinia/-/pinia-3.0.4.tgz", - "integrity": "sha512-l7pqLUFTI/+ESXn6k3nu30ZIzW5E2WZF/LaHJEpoq6ElcLD+wduZoB2kBN19du6K/4FDpPMazY2wJr+IndBtQw==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "@vue/devtools-api": "^7.7.7" - }, - "funding": { - "url": "https://github.com/sponsors/posva" - }, - "peerDependencies": { - "typescript": ">=4.5.0", - "vue": "^3.5.11" - }, - "peerDependenciesMeta": { - "typescript": { - "optional": true - } - } - }, - "node_modules/@nextcloud/password-confirmation/node_modules/pinia/node_modules/@vue/devtools-api": { - "version": "7.7.9", - "resolved": "https://registry.npmjs.org/@vue/devtools-api/-/devtools-api-7.7.9.tgz", - "integrity": "sha512-kIE8wvwlcZ6TJTbNeU2HQNtaxLx3a84aotTITUuL/4bzfPxzajGBOoqjMhwZJ8L9qFYDU/lAYMEEm11dnZOD6g==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "@vue/devtools-kit": "^7.7.9" - } - }, "node_modules/@nextcloud/password-confirmation/node_modules/readdirp": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/readdirp/-/readdirp-5.0.0.tgz", @@ -9845,23 +9775,6 @@ "dev": true, "peer": true }, - "node_modules/copy-anything": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/copy-anything/-/copy-anything-4.0.5.tgz", - "integrity": "sha512-7Vv6asjS4gMOuILabD3l739tsaxFQmC+a7pLZm02zyvs8p977bL3zEgq3yDk5rn9B0PbYgIv++jmHcuUab4RhA==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "is-what": "^5.2.0" - }, - "engines": { - "node": ">=18" - }, - "funding": { - "url": "https://github.com/sponsors/mesqueeb" - } - }, "node_modules/core-js": { "version": "2.6.9", "hasInstallScript": true, @@ -15470,20 +15383,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/is-what": { - "version": "5.5.0", - "resolved": "https://registry.npmjs.org/is-what/-/is-what-5.5.0.tgz", - "integrity": "sha512-oG7cgbmg5kLYae2N5IVd3jm2s+vldjxJzK1pcu9LfpGuQ93MQSzo0okvRna+7y5ifrD+20FE8FvjusyGaz14fw==", - "license": "MIT", - "optional": true, - "peer": true, - "engines": { - "node": ">=18" - }, - "funding": { - "url": "https://github.com/sponsors/mesqueeb" - } - }, "node_modules/is-whitespace": { "version": "0.3.0", "dev": true, @@ -19114,14 +19013,6 @@ "node": ">=16 || 14 >=14.17" } }, - "node_modules/mitt": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/mitt/-/mitt-3.0.1.tgz", - "integrity": "sha512-vKivATfr97l2/QBCYAkXYDbrIWPM2IIKEl7YPhjCvKlG3kE2gm+uBo6nEXK3M5/Ffh/FLpKExzOQ3JJoJGFKBw==", - "license": "MIT", - "optional": true, - "peer": true - }, "node_modules/mkdirp-classic": { "version": "0.5.3", "resolved": "https://registry.npmjs.org/mkdirp-classic/-/mkdirp-classic-0.5.3.tgz", @@ -22694,17 +22585,6 @@ "wbuf": "^1.7.3" } }, - "node_modules/speakingurl": { - "version": "14.0.1", - "resolved": "https://registry.npmjs.org/speakingurl/-/speakingurl-14.0.1.tgz", - "integrity": "sha512-1POYv7uv2gXoyGFpBCmpDVSNV74IfsWlDW216UPjbWufNf+bSU6GdbDsxdcxtfwb4xlI3yxzOTKClUosxARYrQ==", - "license": "BSD-3-Clause", - "optional": true, - "peer": true, - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/spec-change": { "version": "1.11.21", "resolved": "https://registry.npmjs.org/spec-change/-/spec-change-1.11.21.tgz", @@ -23865,20 +23745,6 @@ "node": ">=18" } }, - "node_modules/superjson": { - "version": "2.2.6", - "resolved": "https://registry.npmjs.org/superjson/-/superjson-2.2.6.tgz", - "integrity": "sha512-H+ue8Zo4vJmV2nRjpx86P35lzwDT3nItnIsocgumgr0hHMQ+ZGq5vrERg9kJBo5AWGmxZDhzDo+WVIJqkB0cGA==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "copy-anything": "^4" - }, - "engines": { - "node": ">=16" - } - }, "node_modules/superstruct": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/superstruct/-/superstruct-2.0.2.tgz", From ebe4d7d9f1369e5f71a4229214dbf3bc11a04a27 Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Fri, 1 May 2026 12:35:57 +0200 Subject: [PATCH 17/20] feat: support team-aware ownership transfer and notifications - allow transfer ownership to team targets from UI and OCC with team-first wording - rename OCC flag to --to-team (drop --to-circle) and auto-detect team IDs safely - validate transfer target up front in transferOwnership to fail fast on invalid users/teams - show team display names (not circle IDs) in transfer confirmations and OCC output - extend board share notifications to team members - extend card assignment notifications to team members and add team-specific notification text - mark team assignment notifications as processed on unassign - keep internal backend semantics based on circle IDs, with clarifying comments --- lib/Command/TransferOwnership.php | 57 ++++++++- lib/Notification/NotificationHelper.php | 139 ++++++++++++++++++++- lib/Notification/Notifier.php | 86 +++++++++---- lib/Service/AssignmentService.php | 8 +- lib/Service/BoardService.php | 32 +++-- src/components/board/SharingTabSidebar.vue | 6 +- 6 files changed, 270 insertions(+), 58 deletions(-) diff --git a/lib/Command/TransferOwnership.php b/lib/Command/TransferOwnership.php index fd09e65e86..a7da2c09d6 100644 --- a/lib/Command/TransferOwnership.php +++ b/lib/Command/TransferOwnership.php @@ -9,7 +9,9 @@ use OCA\Deck\Db\Acl; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Service\BoardService; +use OCA\Deck\Service\CirclesService; use OCA\Deck\Service\PermissionService; +use OCP\IUserManager; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputArgument; @@ -23,14 +25,18 @@ final class TransferOwnership extends Command { protected $boardMapper; protected $permissionService; protected $questionHelper; + protected $userManager; + protected $circlesService; - public function __construct(BoardService $boardService, BoardMapper $boardMapper, PermissionService $permissionService, QuestionHelper $questionHelper) { + public function __construct(BoardService $boardService, BoardMapper $boardMapper, PermissionService $permissionService, QuestionHelper $questionHelper, IUserManager $userManager, CirclesService $circlesService) { parent::__construct(); $this->boardService = $boardService; $this->boardMapper = $boardMapper; $this->permissionService = $permissionService; $this->questionHelper = $questionHelper; + $this->userManager = $userManager; + $this->circlesService = $circlesService; } protected function configure() { @@ -59,10 +65,10 @@ protected function configure() { 'Reassign card details of the old owner to the new one' ) ->addOption( - 'to-circle', + 'to-team', null, InputOption::VALUE_NONE, - 'Treat as a circle ID instead of a user UID' + 'Treat as a team ID (internally stored as a circle ID) instead of a user UID' ) ; } @@ -72,9 +78,48 @@ protected function execute(InputInterface $input, OutputInterface $output): int $newOwner = $input->getArgument('newOwner'); $boardId = $input->getArgument('boardId'); $remapAssignment = $input->getOption('remap'); - $toCircle = $input->getOption('to-circle'); - $newOwnerType = $toCircle ? Acl::PERMISSION_TYPE_CIRCLE : Acl::PERMISSION_TYPE_USER; - $newOwnerLabel = $toCircle ? "circle $newOwner" : $newOwner; + $toTeam = $input->getOption('to-team'); + $newOwnerType = Acl::PERMISSION_TYPE_USER; + $teamDisplayName = null; + if ($toTeam) { + $newOwnerType = Acl::PERMISSION_TYPE_CIRCLE; + if ($this->circlesService->isCirclesEnabled()) { + try { + $circle = $this->circlesService->getCircle($newOwner); + if ($circle !== null) { + $teamDisplayName = $circle->getDisplayName(); + } + } catch (\Throwable $e) { + $teamDisplayName = null; + } + } + } else { + $userExists = $this->userManager->userExists($newOwner); + $circleExists = false; + $circle = null; + if ($this->circlesService->isCirclesEnabled()) { + try { + $circle = $this->circlesService->getCircle($newOwner); + $circleExists = $circle !== null; + if ($circle !== null) { + $teamDisplayName = $circle->getDisplayName(); + } + } catch (\Throwable $e) { + $circleExists = false; + } + } + + if ($userExists && $circleExists) { + $output->writeln('Ambiguous target: ' . $newOwner . ' matches both a user and a team (circle ID). Use --to-team to transfer to the team.'); + return 1; + } + + if ($circleExists && !$userExists) { + $newOwnerType = Acl::PERMISSION_TYPE_CIRCLE; + $output->writeln('Detected team target: treating ' . $newOwner . ' as team ' . ($teamDisplayName ?: $newOwner) . '.'); + } + } + $newOwnerLabel = $newOwnerType === Acl::PERMISSION_TYPE_CIRCLE ? 'team ' . ($teamDisplayName ?: $newOwner) : $newOwner; $this->boardService->setUserId($owner); $this->permissionService->setUserId($owner); diff --git a/lib/Notification/NotificationHelper.php b/lib/Notification/NotificationHelper.php index 0813111334..b20e053b32 100644 --- a/lib/Notification/NotificationHelper.php +++ b/lib/Notification/NotificationHelper.php @@ -11,6 +11,7 @@ use DateTime; use Exception; +use OCA\Circles\Model\Member; use OCA\Deck\AppInfo\Application; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AssignmentMapper; @@ -20,6 +21,7 @@ use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\User; use OCA\Deck\Service\ConfigService; +use OCA\Deck\Service\CirclesService; use OCA\Deck\Service\PermissionService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; @@ -39,6 +41,7 @@ public function __construct( protected readonly BoardMapper $boardMapper, protected readonly AssignmentMapper $assignmentMapper, protected readonly PermissionService $permissionService, + protected readonly CirclesService $circlesService, protected readonly IConfig $config, protected readonly IManager $notificationManager, protected readonly IGroupManager $groupManager, @@ -105,6 +108,45 @@ public function markDuedateAsRead(Card $card): void { } public function sendCardAssigned(Card $card, string $userId): void { + $this->sendCardAssignedByType($card, $userId, Acl::PERMISSION_TYPE_USER); + } + + public function sendCardAssignedByType(Card $card, string $participantId, int $type): void { + if ($type === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$this->circlesService->isCirclesEnabled()) { + return; + } + + $circle = $this->circlesService->getCircle($participantId); + if ($circle === null) { + return; + } + + $teamName = $circle->getDisplayName() ?: $participantId; + $members = array_filter($circle->getInheritedMembers(), static function (Member $member) { + return $member->getUserType() === Member::TYPE_USER; + }); + foreach ($members as $member) { + $userId = $member->getUserId(); + if ($userId === $this->userId) { + continue; + } + + $this->sendCardAssignedToUser($card, $userId, $teamName); + } + return; + } + + foreach ($this->resolveParticipantUserIds($participantId, $type) as $userId) { + if ($userId === $this->userId) { + continue; + } + + $this->sendCardAssignedToUser($card, $userId); + } + } + + private function sendCardAssignedToUser(Card $card, string $userId, ?string $teamName = null): void { $boardId = $this->cardMapper->findBoardId($card->getId()); try { $board = $this->getBoard($boardId); @@ -112,21 +154,41 @@ public function sendCardAssigned(Card $card, string $userId): void { return; } + $subjectParams = [ + $card->getTitle(), + $board->getTitle(), + $this->userId, + ]; + if ($teamName !== null) { + $subjectParams[] = 'team'; + $subjectParams[] = $teamName; + } + $notification = $this->notificationManager->createNotification(); $notification ->setApp('deck') ->setUser($userId) ->setDateTime(new DateTime()) ->setObject('card', (string)$card->getId()) - ->setSubject('card-assigned', [ - $card->getTitle(), - $board->getTitle(), - $this->userId - ]); + ->setSubject('card-assigned', $subjectParams); $this->notificationManager->notify($notification); } public function markCardAssignedAsRead(Card $card, string $userId): void { + $this->markCardAssignedAsReadByType($card, $userId, Acl::PERMISSION_TYPE_USER); + } + + public function markCardAssignedAsReadByType(Card $card, string $participantId, int $type): void { + foreach ($this->resolveParticipantUserIds($participantId, $type) as $userId) { + if ($userId === $this->userId) { + continue; + } + + $this->markCardAssignedAsReadForUser($card, $userId); + } + } + + private function markCardAssignedAsReadForUser(Card $card, string $userId): void { $notification = $this->notificationManager->createNotification(); $notification ->setApp('deck') @@ -137,7 +199,45 @@ public function markCardAssignedAsRead(Card $card, string $userId): void { } /** - * Send notifications that a board was shared with a user/group + * Resolve assignment participant ids to concrete user ids for notifications. + * Teams are represented by circles internally. + * + * @return string[] + */ + private function resolveParticipantUserIds(string $participantId, int $type): array { + if ($type === Acl::PERMISSION_TYPE_USER) { + return [$participantId]; + } + + if ($type === Acl::PERMISSION_TYPE_GROUP) { + $group = $this->groupManager->get($participantId); + if ($group === null) { + return []; + } + return array_map(static fn ($user) => $user->getUID(), $group->getUsers()); + } + + if ($type === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$this->circlesService->isCirclesEnabled()) { + return []; + } + + $circle = $this->circlesService->getCircle($participantId); + if ($circle === null) { + return []; + } + + $members = array_filter($circle->getInheritedMembers(), static function (Member $member) { + return $member->getUserType() === Member::TYPE_USER; + }); + return array_values(array_unique(array_map(static fn (Member $member) => $member->getUserId(), $members))); + } + + return []; + } + + /** + * Send notifications that a board was shared with a user/group/team. */ public function sendBoardShared(int $boardId, Acl $acl, bool $markAsRead = false): void { try { @@ -173,6 +273,33 @@ public function sendBoardShared(int $boardId, Acl $acl, bool $markAsRead = false } } } + if ($acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$this->circlesService->isCirclesEnabled()) { + return; + } + + $circle = $this->circlesService->getCircle($acl->getParticipant()); + if ($circle === null) { + return; + } + + $members = array_filter($circle->getInheritedMembers(), static function (Member $member) { + return $member->getUserType() === Member::TYPE_USER; + }); + foreach ($members as $member) { + $userId = $member->getUserId(); + if ($userId === $this->userId) { + continue; + } + $notification = $this->generateBoardShared($board, $userId); + if ($markAsRead) { + $this->notificationManager->markProcessed($notification); + } else { + $notification->setDateTime(new DateTime()); + $this->notificationManager->notify($notification); + } + } + } } public function sendMention(IComment $comment): void { diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index 87ee25941e..5dc6a05898 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -79,33 +79,65 @@ public function prepare(INotification $notification, string $languageCode): INot } else { $dn = $params[2]; } - $notification->setParsedSubject( - $l->t('The card "%s" on "%s" has been assigned to you by %s.', [$params[0], $params[1], $dn]) - ); - $notification->setRichSubject( - $l->t('{user} has assigned the card {deck-card} on {deck-board} to you.'), - [ - 'deck-card' => [ - 'type' => 'deck-card', - 'id' => (string)$cardId, - 'name' => $params[0], - 'boardname' => (string)$params[1], - 'stackname' => $stack->getTitle(), - 'link' => $this->getCardUrl($boardId, $cardId), - ], - 'deck-board' => [ - 'type' => 'deck-board', - 'id' => (string)$boardId, - 'name' => (string)$params[1], - 'link' => $this->getBoardUrl($boardId), - ], - 'user' => [ - 'type' => 'user', - 'id' => (string)$params[2], - 'name' => $dn, - ] - ] - ); + $isTeamAssignment = isset($params[3]) && $params[3] === 'team'; + if ($isTeamAssignment) { + $teamName = $params[4] ?? ''; + $notification->setParsedSubject( + $l->t('The card "%1$s" on "%2$s" has been assigned to team "%3$s" by %4$s. You are receiving this because you are a member of that team.', [$params[0], $params[1], $teamName, $dn]) + ); + $notification->setRichSubject( + $l->t('{user} has assigned the card {deck-card} on {deck-board} to a team you are a member of.'), + [ + 'deck-card' => [ + 'type' => 'deck-card', + 'id' => (string)$cardId, + 'name' => $params[0], + 'boardname' => (string)$params[1], + 'stackname' => $stack->getTitle(), + 'link' => $this->getCardUrl($boardId, $cardId), + ], + 'deck-board' => [ + 'type' => 'deck-board', + 'id' => (string)$boardId, + 'name' => (string)$params[1], + 'link' => $this->getBoardUrl($boardId), + ], + 'user' => [ + 'type' => 'user', + 'id' => (string)$params[2], + 'name' => $dn, + ] + ] + ); + } else { + $notification->setParsedSubject( + $l->t('The card "%s" on "%s" has been assigned to you by %s.', [$params[0], $params[1], $dn]) + ); + $notification->setRichSubject( + $l->t('{user} has assigned the card {deck-card} on {deck-board} to you.'), + [ + 'deck-card' => [ + 'type' => 'deck-card', + 'id' => (string)$cardId, + 'name' => $params[0], + 'boardname' => (string)$params[1], + 'stackname' => $stack->getTitle(), + 'link' => $this->getCardUrl($boardId, $cardId), + ], + 'deck-board' => [ + 'type' => 'deck-board', + 'id' => (string)$boardId, + 'name' => (string)$params[1], + 'link' => $this->getBoardUrl($boardId), + ], + 'user' => [ + 'type' => 'user', + 'id' => (string)$params[2], + 'name' => $dn, + ] + ] + ); + } $notification->setLink($this->getCardUrl($boardId, $cardId)); break; case 'card-overdue': diff --git a/lib/Service/AssignmentService.php b/lib/Service/AssignmentService.php index dd8d8f17a0..81be618ef8 100644 --- a/lib/Service/AssignmentService.php +++ b/lib/Service/AssignmentService.php @@ -75,8 +75,8 @@ public function assignUser(int $cardId, string $userId, int $type = Assignment:: } - if ($type === Assignment::TYPE_USER && $userId !== $this->userId) { - $this->notificationHelper->sendCardAssigned($card, $userId); + if ($userId !== $this->userId) { + $this->notificationHelper->sendCardAssignedByType($card, $userId, $type); } $assignment = new Assignment(); @@ -109,8 +109,8 @@ public function unassignUser(int $cardId, string $userId, int $type = 0): Assign $assignment = $this->assignedUsersMapper->delete($assignment); $card = $this->cardMapper->find($cardId); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_CARD_USER_UNASSIGN, ['assigneduser' => $userId]); - if ($type === Assignment::TYPE_USER && $userId !== $this->userId) { - $this->notificationHelper->markCardAssignedAsRead($card, $userId); + if ($userId !== $this->userId) { + $this->notificationHelper->markCardAssignedAsReadByType($card, $userId, $type); } $this->changeHelper->cardChanged($cardId); diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 700d8e1aa2..d46abfb9d6 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -623,18 +623,7 @@ public function transferBoardOwnership(int $boardId, string $newOwner, bool $cha $previousOwner = $board->getOwner(); // Validate the new owner before touching anything - if ($newOwnerType === Acl::PERMISSION_TYPE_CIRCLE) { - if (!$this->circlesService->isCirclesEnabled()) { - throw new BadRequestException('The Circles app is not enabled'); - } - if ($this->circlesService->getCircle($newOwner) === null) { - throw new BadRequestException('Circle not found: ' . $newOwner); - } - } else { - if (!$this->userManager->userExists($newOwner)) { - throw new BadRequestException('User not found: ' . $newOwner); - } - } + $this->validateTransferTarget($newOwner, $newOwnerType); $this->clearBoardFromCache($board); // Remove new owner from ACL (avoids a duplicate entry once they become the owner) @@ -670,12 +659,31 @@ public function transferBoardOwnership(int $boardId, string $newOwner, bool $cha } public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false, int $newOwnerType = Acl::PERMISSION_TYPE_USER): \Generator { + // Validate once up front so invalid targets fail even if no boards match. + $this->validateTransferTarget($newOwner, $newOwnerType); $boards = $this->boardMapper->findAllByOwner($owner); foreach ($boards as $board) { yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent, $newOwnerType); } } + private function validateTransferTarget(string $newOwner, int $newOwnerType): void { + // Teams are represented by Circles internally (PERMISSION_TYPE_CIRCLE + circle ID). + if ($newOwnerType === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$this->circlesService->isCirclesEnabled()) { + throw new BadRequestException('The Circles app is not enabled'); + } + if ($this->circlesService->getCircle($newOwner) === null) { + throw new BadRequestException('Circle not found: ' . $newOwner); + } + return; + } + + if (!$this->userManager->userExists($newOwner)) { + throw new BadRequestException('User not found: ' . $newOwner); + } + } + /** * @throws DoesNotExistException * @throws NoPermissionException diff --git a/src/components/board/SharingTabSidebar.vue b/src/components/board/SharingTabSidebar.vue index ce06fee0a1..2649b2b67d 100644 --- a/src/components/board/SharingTabSidebar.vue +++ b/src/components/board/SharingTabSidebar.vue @@ -58,7 +58,7 @@ + @click="clickTransferOwner(acl.participant.uid || acl.participant.id, acl.type, acl.participant.displayname || acl.participant.id || acl.participant.uid)"> {{ t('deck', 'Transfer ownership') }} Date: Sat, 2 May 2026 14:02:44 +0200 Subject: [PATCH 18/20] * check via canTransferTo if the user is in a circle that has ownership * Show that removing a user who has access rights through a group or team membership only removes these ACL's, not their entire access. Comes with tests. * Add transfer-ownership-to-self button for user who is in a circle that owns a board Signed-off-by: Jos Poortvliet --- lib/Command/TransferOwnership.php | 4 +- lib/Db/Acl.php | 6 + lib/Service/BoardService.php | 46 +++++- lib/Service/PermissionService.php | 3 +- src/components/board/SharingTabSidebar.vue | 23 ++- src/store/main.js | 3 + tests/unit/Service/BoardServiceTest.php | 173 +++++++++++++++++++++ 7 files changed, 248 insertions(+), 10 deletions(-) diff --git a/lib/Command/TransferOwnership.php b/lib/Command/TransferOwnership.php index a7da2c09d6..38b20e78ed 100644 --- a/lib/Command/TransferOwnership.php +++ b/lib/Command/TransferOwnership.php @@ -46,12 +46,12 @@ protected function configure() { ->addArgument( 'owner', InputArgument::REQUIRED, - 'Owner uid' + 'Owner uid or Team (circle) ID to transfer from' ) ->addArgument( 'newOwner', InputArgument::REQUIRED, - 'New owner uid' + 'New owner uid or Team (circle) ID to transfer to' ) ->addArgument( 'boardId', diff --git a/lib/Db/Acl.php b/lib/Db/Acl.php index 735f9a19fc..8f7fe660a9 100644 --- a/lib/Db/Acl.php +++ b/lib/Db/Acl.php @@ -21,6 +21,8 @@ * @method void setOwner(int $owner) * @method void setToken(string $token) * @method string getToken() + * @method bool isRetainsAccessViaMembership() + * @method void setRetainsAccessViaMembership(bool $retainsAccessViaMembership) * */ class Acl extends RelationalEntity { @@ -28,6 +30,7 @@ class Acl extends RelationalEntity { public const PERMISSION_EDIT = 1; public const PERMISSION_SHARE = 2; public const PERMISSION_MANAGE = 3; + public const PERMISSION_OWNER = 4; public const PERMISSION_TYPE_USER = 0; public const PERMISSION_TYPE_GROUP = 1; @@ -41,6 +44,7 @@ class Acl extends RelationalEntity { protected $permissionShare = false; protected $permissionManage = false; protected $owner = false; + protected $retainsAccessViaMembership = false; protected $token = null; public function __construct() { @@ -51,8 +55,10 @@ public function __construct() { $this->addType('permissionManage', 'boolean'); $this->addType('type', 'integer'); $this->addType('owner', 'boolean'); + $this->addType('retainsAccessViaMembership', 'boolean'); $this->addType('token', 'string'); $this->addRelation('owner'); + $this->addRelation('retainsAccessViaMembership'); $this->addResolvable('participant'); } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index d46abfb9d6..8f8aabe24f 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -222,7 +222,8 @@ public function create(string $title, string $userId, string $color): Board { 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false, 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, - 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false + 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false, + 'PERMISSION_OWNER' => $permissions[Acl::PERMISSION_OWNER] ?? false, ]); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $board, ActivityManager::SUBJECT_BOARD_CREATE, [], $userId); $this->changeHelper->boardChanged($board->getId()); @@ -573,8 +574,9 @@ public function clone( 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false, 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, - 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false - ]); + 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false, + 'PERMISSION_OWNER' => $permissions[Acl::PERMISSION_OWNER] ?? false, + ]); $this->boardMapper->insert($newBoard); foreach ($this->aclMapper->findAll($board->getId()) as $acl) { @@ -711,6 +713,7 @@ private function enrichBoards(array $boards, bool $fullDetails = true): array { foreach ($board->getAcl() as &$acl) { $this->boardMapper->mapAcl($acl); } + $this->annotateAclRetainedAccess($board); } $permissions = $this->permissionService->matchPermissions($board); @@ -718,7 +721,8 @@ private function enrichBoards(array $boards, bool $fullDetails = true): array { 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false, 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, - 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false + 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false, + 'PERMISSION_OWNER' => $permissions[Acl::PERMISSION_OWNER] ?? false, ]); if ($fullDetails) { @@ -740,6 +744,40 @@ private function enrichBoards(array $boards, bool $fullDetails = true): array { return $boards; } + private function annotateAclRetainedAccess(Board $board): void { + $acls = $board->getAcl() ?? []; + foreach ($acls as $acl) { + if ($acl->getType() !== Acl::PERMISSION_TYPE_USER) { + $acl->setRetainsAccessViaMembership(false); + continue; + } + + $participant = (string)$acl->getParticipant(); + if ($participant === '') { + $acl->setRetainsAccessViaMembership(false); + continue; + } + + if ($board->getOwnerType() === Acl::PERMISSION_TYPE_USER && $board->getOwner() === $participant) { + $acl->setRetainsAccessViaMembership(true); + continue; + } + + if ($board->getOwnerType() === Acl::PERMISSION_TYPE_CIRCLE) { + try { + if ($this->circlesService->isUserInCircle($board->getOwner(), $participant)) { + $acl->setRetainsAccessViaMembership(true); + continue; + } + } catch (\Throwable) { + } + } + + $otherAcls = array_filter($acls, static fn (Acl $candidate): bool => $candidate->getId() !== $acl->getId()); + $acl->setRetainsAccessViaMembership($this->permissionService->userCan($otherAcls, Acl::PERMISSION_READ, $participant)); + } + } + private function cloneCards(Board $board, Board $newBoard, bool $withAssignments = false, bool $withLabels = false, bool $withDueDate = false, bool $moveCardsToLeftStack = false, bool $restoreArchivedCards = false): void { $stacks = $this->stackMapper->findAll($board->getId()); $newStacks = $this->stackMapper->findAll($newBoard->getId()); diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 81779a54df..82d7f5ad29 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -135,7 +135,8 @@ public function matchPermissions(Board $board) { Acl::PERMISSION_EDIT => $owner || $this->userCan($acls, Acl::PERMISSION_EDIT), Acl::PERMISSION_MANAGE => $owner || $this->userCan($acls, Acl::PERMISSION_MANAGE), Acl::PERMISSION_SHARE => ($owner || $this->userCan($acls, Acl::PERMISSION_SHARE)) - && (!$this->shareManager->sharingDisabledForUser($this->userId)) + && (!$this->shareManager->sharingDisabledForUser($this->userId)), + Acl::PERMISSION_OWNER => $owner, ]; } diff --git a/src/components/board/SharingTabSidebar.vue b/src/components/board/SharingTabSidebar.vue index 2649b2b67d..93dcdeaf81 100644 --- a/src/components/board/SharingTabSidebar.vue +++ b/src/components/board/SharingTabSidebar.vue @@ -23,6 +23,14 @@ {{ t('deck', 'Team') }} + + + {{ t('deck', 'Transfer ownership to myself') }} + +
    • @@ -64,8 +72,13 @@ - {{ t('deck', 'Delete') }} + {{ acl.type === PERMISSION_TYPE_USER && acl.retainsAccessViaMembership + ? t('deck', 'Remove extra permissions') + : t('deck', 'Delete') }}
      • @@ -129,10 +142,14 @@ export default { 'canEdit', 'canManage', 'canShare', + 'isBoardOwner', ]), isCurrentUser() { return (uid) => uid === getCurrentUser().uid }, + currentUserUid() { + return getCurrentUser().uid + }, canTransferTo() { return (acl) => { const canBeOwnershipTarget = acl.type === PERMISSION_TYPE_USER || acl.type === PERMISSION_TYPE_CIRCLE @@ -144,8 +161,8 @@ export default { if (this.board.ownerType !== PERMISSION_TYPE_CIRCLE) { return this.isCurrentUser(this.board.owner.uid) } - // For circle-owned boards: any circle member with manage rights can transfer - return this.canManage + // For circle-owned boards: only actual board owners (team members) can transfer + return this.isBoardOwner } }, formatedSharees() { diff --git a/src/store/main.js b/src/store/main.js index b8963d24bf..9d20ce79ce 100644 --- a/src/store/main.js +++ b/src/store/main.js @@ -127,6 +127,9 @@ export default function storeFactory() { canShare: state => { return state.currentBoard ? state.currentBoard.permissions.PERMISSION_SHARE : false }, + isBoardOwner: state => { + return state.currentBoard ? state.currentBoard.permissions.PERMISSION_OWNER : false + }, isArchived: state => { return state.currentBoard && state.currentBoard.archived }, diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index faea13853d..ae26e13cb3 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -480,6 +480,179 @@ public function testDeleteAcl() { $this->assertEquals($acl, $this->service->deleteAcl(123)); } + public function testFindMarksUserAclAsRetainedViaOwnerCircleMembership(): void { + $board = new Board(); + $board->setId(10); + $board->setOwner('circle-1'); + $board->setOwnerType(Acl::PERMISSION_TYPE_CIRCLE); + + $userAcl = new Acl(); + $userAcl->setId(501); + $userAcl->setBoardId(10); + $userAcl->setType(Acl::PERMISSION_TYPE_USER); + $userAcl->setParticipant('alice'); + $board->setAcl([$userAcl]); + + $this->permissionService->expects($this->once()) + ->method('checkPermission') + ->with($this->boardMapper, 10, Acl::PERMISSION_READ, null, false); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(10, true, true, false) + ->willReturn($board); + + $this->boardMapper->expects($this->once()) + ->method('mapOwner') + ->with($board); + + $this->boardMapper->expects($this->once()) + ->method('mapAcl') + ->with($userAcl); + + $this->circlesService->expects($this->once()) + ->method('isUserInCircle') + ->with('circle-1', 'alice') + ->willReturn(true); + + $this->permissionService->expects($this->never()) + ->method('userCan'); + + $this->permissionService->expects($this->once()) + ->method('matchPermissions') + ->with($board) + ->willReturn([ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + Acl::PERMISSION_OWNER => true, + ]); + + $result = $this->service->find(10, false); + $this->assertTrue($result->getAcl()[0]->isRetainsAccessViaMembership()); + } + + public function testFindMarksUserAclAsRetainedViaOtherAclMembership(): void { + $board = new Board(); + $board->setId(11); + $board->setOwner('bob'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $userAcl = new Acl(); + $userAcl->setId(601); + $userAcl->setBoardId(11); + $userAcl->setType(Acl::PERMISSION_TYPE_USER); + $userAcl->setParticipant('alice'); + + $groupAcl = new Acl(); + $groupAcl->setId(602); + $groupAcl->setBoardId(11); + $groupAcl->setType(Acl::PERMISSION_TYPE_GROUP); + $groupAcl->setParticipant('devs'); + + $board->setAcl([$userAcl, $groupAcl]); + + $this->permissionService->expects($this->once()) + ->method('checkPermission') + ->with($this->boardMapper, 11, Acl::PERMISSION_READ, null, false); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(11, true, true, false) + ->willReturn($board); + + $this->boardMapper->expects($this->once()) + ->method('mapOwner') + ->with($board); + + $this->boardMapper->expects($this->exactly(2)) + ->method('mapAcl') + ->withConsecutive([$userAcl], [$groupAcl]); + + $this->circlesService->expects($this->never()) + ->method('isUserInCircle'); + + $this->permissionService->expects($this->once()) + ->method('userCan') + ->with( + $this->callback(static function (array $otherAcls): bool { + $otherAcls = array_values($otherAcls); + return count($otherAcls) === 1 && $otherAcls[0]->getId() === 602; + }), + Acl::PERMISSION_READ, + 'alice' + ) + ->willReturn(true); + + $this->permissionService->expects($this->once()) + ->method('matchPermissions') + ->with($board) + ->willReturn([ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + Acl::PERMISSION_OWNER => false, + ]); + + $result = $this->service->find(11, false); + $this->assertTrue($result->getAcl()[0]->isRetainsAccessViaMembership()); + } + + public function testFindMarksUserAclAsNotRetainedWithoutInheritedAccess(): void { + $board = new Board(); + $board->setId(12); + $board->setOwner('bob'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $userAcl = new Acl(); + $userAcl->setId(701); + $userAcl->setBoardId(12); + $userAcl->setType(Acl::PERMISSION_TYPE_USER); + $userAcl->setParticipant('alice'); + $board->setAcl([$userAcl]); + + $this->permissionService->expects($this->once()) + ->method('checkPermission') + ->with($this->boardMapper, 12, Acl::PERMISSION_READ, null, false); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(12, true, true, false) + ->willReturn($board); + + $this->boardMapper->expects($this->once()) + ->method('mapOwner') + ->with($board); + + $this->boardMapper->expects($this->once()) + ->method('mapAcl') + ->with($userAcl); + + $this->circlesService->expects($this->never()) + ->method('isUserInCircle'); + + $this->permissionService->expects($this->once()) + ->method('userCan') + ->with([], Acl::PERMISSION_READ, 'alice') + ->willReturn(false); + + $this->permissionService->expects($this->once()) + ->method('matchPermissions') + ->with($board) + ->willReturn([ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + Acl::PERMISSION_OWNER => false, + ]); + + $result = $this->service->find(12, false); + $this->assertFalse($result->getAcl()[0]->isRetainsAccessViaMembership()); + } + public function testTransferBoardOwnershipToCircle(): void { $board = new Board(); $board->setId(10); From cf244957a920b3af9b14f2004646091779ec73c3 Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Sat, 2 May 2026 14:43:10 +0200 Subject: [PATCH 19/20] ensure a team stays in sharelist when made owner, just like with users when they transfer ownership. This means that when removing ownership, the team is still in the list of sharees. Signed-off-by: Jos Poortvliet --- lib/Service/BoardService.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 8f8aabe24f..12c1cb91ad 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -646,6 +646,19 @@ public function transferBoardOwnership(int $boardId, string $newOwner, bool $cha $this->boardMapper->transferOwnership($previousOwner, $newOwner, $boardId, $newOwnerType); + // When transferring ownership to a circle, re-add the circle as an explicit ACL + // sharee with full rights so it remains visible in the sharing sidebar. + // (It was removed above to prevent a duplicate DB entry.) + if ($newOwnerType === Acl::PERMISSION_TYPE_CIRCLE) { + try { + $this->addAcl($boardId, Acl::PERMISSION_TYPE_CIRCLE, $newOwner, true, true, true); + } catch (DbException $e) { + if ($e->getReason() !== DbException::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } + } + } + // Card-content remap is only meaningful when transferring to a user, not a circle if ($changeContent && $newOwnerType === Acl::PERMISSION_TYPE_USER) { $this->assignedUsersMapper->remapAssignedUser($boardId, $previousOwner, $newOwner); From 03bf7db5b23b46a765a5ba245561a47718e63d04 Mon Sep 17 00:00:00 2001 From: Jos Poortvliet Date: Sat, 2 May 2026 18:20:01 +0200 Subject: [PATCH 20/20] * Ensure we delete boards owned by a circle when the circle gets deleted following the same behavior for users * make sure findAllByOwner is explicit about the type of ownership * Move 'PERMISSION_OWNER' to a sane spot * Fix the annotateAclRetainedAccess missing the owning circle's ACL entry Signed-off-by: Jos Poortvliet --- lib/Command/TransferOwnership.php | 32 ++++- lib/Db/Acl.php | 1 - lib/Db/Board.php | 2 + lib/Db/BoardMapper.php | 5 +- lib/Listeners/ParticipantCleanupListener.php | 7 +- lib/Service/BoardService.php | 26 +++- lib/Service/PermissionService.php | 4 +- src/components/board/SharingTabSidebar.vue | 8 +- tests/integration/import/ImportExportTest.php | 2 +- .../ParticipantCleanupListenerTest.php | 78 +++++++++++ tests/unit/Service/BoardServiceTest.php | 128 +++++++++++++++++- 11 files changed, 271 insertions(+), 22 deletions(-) create mode 100644 tests/unit/Listeners/ParticipantCleanupListenerTest.php diff --git a/lib/Command/TransferOwnership.php b/lib/Command/TransferOwnership.php index 38b20e78ed..9e42acdf43 100644 --- a/lib/Command/TransferOwnership.php +++ b/lib/Command/TransferOwnership.php @@ -70,6 +70,12 @@ protected function configure() { InputOption::VALUE_NONE, 'Treat as a team ID (internally stored as a circle ID) instead of a user UID' ) + ->addOption( + 'from-team', + null, + InputOption::VALUE_NONE, + 'Treat as a team ID (internally stored as a circle ID) instead of a user UID' + ) ; } @@ -79,8 +85,32 @@ protected function execute(InputInterface $input, OutputInterface $output): int $boardId = $input->getArgument('boardId'); $remapAssignment = $input->getOption('remap'); $toTeam = $input->getOption('to-team'); + $fromTeam = $input->getOption('from-team'); + $ownerType = Acl::PERMISSION_TYPE_USER; $newOwnerType = Acl::PERMISSION_TYPE_USER; $teamDisplayName = null; + if ($fromTeam) { + $ownerType = Acl::PERMISSION_TYPE_CIRCLE; + } else { + $ownerUserExists = $this->userManager->userExists($owner); + $ownerCircleExists = false; + if ($this->circlesService->isCirclesEnabled()) { + try { + $ownerCircleExists = $this->circlesService->getCircle($owner) !== null; + } catch (\Throwable $e) { + $ownerCircleExists = false; + } + } + + if ($ownerUserExists && $ownerCircleExists) { + $output->writeln('Ambiguous source owner: ' . $owner . ' matches both a user and a team (circle ID). Use --from-team if you mean the team.'); + return 1; + } + + if ($ownerCircleExists && !$ownerUserExists) { + $ownerType = Acl::PERMISSION_TYPE_CIRCLE; + } + } if ($toTeam) { $newOwnerType = Acl::PERMISSION_TYPE_CIRCLE; if ($this->circlesService->isCirclesEnabled()) { @@ -154,7 +184,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 0; } - foreach ($this->boardService->transferOwnership($owner, $newOwner, $remapAssignment, $newOwnerType) as $board) { + foreach ($this->boardService->transferOwnership($owner, $newOwner, $remapAssignment, $newOwnerType, $ownerType) as $board) { $output->writeln(' - ' . $board->getTitle() . ' transferred'); } $output->writeln("All boards from $owner transferred to $newOwnerLabel"); diff --git a/lib/Db/Acl.php b/lib/Db/Acl.php index 8f7fe660a9..7fb1d7a986 100644 --- a/lib/Db/Acl.php +++ b/lib/Db/Acl.php @@ -30,7 +30,6 @@ class Acl extends RelationalEntity { public const PERMISSION_EDIT = 1; public const PERMISSION_SHARE = 2; public const PERMISSION_MANAGE = 3; - public const PERMISSION_OWNER = 4; public const PERMISSION_TYPE_USER = 0; public const PERMISSION_TYPE_GROUP = 1; diff --git a/lib/Db/Board.php b/lib/Db/Board.php index 80865dcb67..ee84880fae 100644 --- a/lib/Db/Board.php +++ b/lib/Db/Board.php @@ -32,6 +32,8 @@ * @method int | null getExternalId() */ class Board extends RelationalEntity { + public const PERMISSION_OWNER = 4; + protected $title; protected $owner; protected $ownerType = 0; diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 41ade67f2a..d912cb2b51 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -295,11 +295,12 @@ public function findAllByUser(string $userId, ?int $limit = null, ?int $offset = return $entries; } - public function findAllByOwner(string $userId, ?int $limit = null, ?int $offset = null) { + public function findAllByOwner(string $ownerId, int $ownerType = Acl::PERMISSION_TYPE_USER, ?int $limit = null, ?int $offset = null): array { $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from('deck_boards') - ->where($qb->expr()->eq('owner', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->where($qb->expr()->eq('owner', $qb->createNamedParameter($ownerId, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('owner_type', $qb->createNamedParameter($ownerType, IQueryBuilder::PARAM_INT))) ->orderBy('id'); if ($limit !== null) { $qb->setMaxResults($limit); diff --git a/lib/Listeners/ParticipantCleanupListener.php b/lib/Listeners/ParticipantCleanupListener.php index 3ba4e27c78..1cc837040e 100644 --- a/lib/Listeners/ParticipantCleanupListener.php +++ b/lib/Listeners/ParticipantCleanupListener.php @@ -30,7 +30,7 @@ public function __construct(AclMapper $aclMapper, AssignmentMapper $assignmentMa public function handle(Event $event): void { if ($event instanceof UserDeletedEvent) { - $boards = $this->boardMapper->findAllByOwner($event->getUser()->getUID()); + $boards = $this->boardMapper->findAllByOwner($event->getUser()->getUID(), Acl::PERMISSION_TYPE_USER); foreach ($boards as $board) { $this->boardMapper->delete($board); } @@ -44,6 +44,11 @@ public function handle(Event $event): void { if ($event instanceof CircleDestroyedEvent) { $circleId = $event->getCircle()->getSingleId(); + $boards = $this->boardMapper->findAllByOwner($circleId, Acl::PERMISSION_TYPE_CIRCLE); + foreach ($boards as $board) { + $this->boardMapper->delete($board); + } + $this->cleanupByParticipant(Acl::PERMISSION_TYPE_CIRCLE, $circleId); } } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 12c1cb91ad..8f76c9c179 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -223,7 +223,7 @@ public function create(string $title, string $userId, string $color): Board { 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false, - 'PERMISSION_OWNER' => $permissions[Acl::PERMISSION_OWNER] ?? false, + 'PERMISSION_OWNER' => $permissions[Board::PERMISSION_OWNER] ?? false, ]); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $board, ActivityManager::SUBJECT_BOARD_CREATE, [], $userId); $this->changeHelper->boardChanged($board->getId()); @@ -575,7 +575,7 @@ public function clone( 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false, - 'PERMISSION_OWNER' => $permissions[Acl::PERMISSION_OWNER] ?? false, + 'PERMISSION_OWNER' => $permissions[Board::PERMISSION_OWNER] ?? false, ]); $this->boardMapper->insert($newBoard); @@ -673,10 +673,10 @@ public function transferBoardOwnership(int $boardId, string $newOwner, bool $cha } } - public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false, int $newOwnerType = Acl::PERMISSION_TYPE_USER): \Generator { + public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false, int $newOwnerType = Acl::PERMISSION_TYPE_USER, int $ownerType = Acl::PERMISSION_TYPE_USER): \Generator { // Validate once up front so invalid targets fail even if no boards match. $this->validateTransferTarget($newOwner, $newOwnerType); - $boards = $this->boardMapper->findAllByOwner($owner); + $boards = $this->boardMapper->findAllByOwner($owner, $ownerType); foreach ($boards as $board) { yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent, $newOwnerType); } @@ -735,7 +735,7 @@ private function enrichBoards(array $boards, bool $fullDetails = true): array { 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false, - 'PERMISSION_OWNER' => $permissions[Acl::PERMISSION_OWNER] ?? false, + 'PERMISSION_OWNER' => $permissions[Board::PERMISSION_OWNER] ?? false, ]); if ($fullDetails) { @@ -760,6 +760,22 @@ private function enrichBoards(array $boards, bool $fullDetails = true): array { private function annotateAclRetainedAccess(Board $board): void { $acls = $board->getAcl() ?? []; foreach ($acls as $acl) { + if ($acl->getType() === Acl::PERMISSION_TYPE_GROUP) { + $acl->setRetainsAccessViaMembership(true); + continue; + } + + if ($acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { + if ($board->getOwnerType() === Acl::PERMISSION_TYPE_CIRCLE + && (string)$acl->getParticipant() === $board->getOwner()) { + $acl->setRetainsAccessViaMembership(true); + continue; + } + + $acl->setRetainsAccessViaMembership(true); + continue; + } + if ($acl->getType() !== Acl::PERMISSION_TYPE_USER) { $acl->setRetainsAccessViaMembership(false); continue; diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 82d7f5ad29..fd8d36c43d 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -116,7 +116,7 @@ public function getPermissions(int $boardId, ?string $userId = null, bool $allow * Get current user permissions for a board * * @param Board $board - * @return array|bool + * @return array * @internal param $boardId */ public function matchPermissions(Board $board) { @@ -136,7 +136,7 @@ public function matchPermissions(Board $board) { Acl::PERMISSION_MANAGE => $owner || $this->userCan($acls, Acl::PERMISSION_MANAGE), Acl::PERMISSION_SHARE => ($owner || $this->userCan($acls, Acl::PERMISSION_SHARE)) && (!$this->shareManager->sharingDisabledForUser($this->userId)), - Acl::PERMISSION_OWNER => $owner, + Board::PERMISSION_OWNER => $owner, ]; } diff --git a/src/components/board/SharingTabSidebar.vue b/src/components/board/SharingTabSidebar.vue index 93dcdeaf81..defdf8d34f 100644 --- a/src/components/board/SharingTabSidebar.vue +++ b/src/components/board/SharingTabSidebar.vue @@ -25,7 +25,7 @@ {{ t('deck', 'Transfer ownership to myself') }} @@ -64,7 +64,7 @@ {{ t('deck', 'Can manage') }} {{ t('deck', 'Transfer ownership') }} @@ -72,11 +72,11 @@ - {{ acl.type === PERMISSION_TYPE_USER && acl.retainsAccessViaMembership + {{ acl.retainsAccessViaMembership ? t('deck', 'Remove extra permissions') : t('deck', 'Delete') }} diff --git a/tests/integration/import/ImportExportTest.php b/tests/integration/import/ImportExportTest.php index 261a01756c..247830e06a 100644 --- a/tests/integration/import/ImportExportTest.php +++ b/tests/integration/import/ImportExportTest.php @@ -279,7 +279,7 @@ public function assertDatabase(string $owner = 'admin') { $stackMapper = self::getFreshService(StackMapper::class); $cardMapper = self::getFreshService(CardMapper::class); - $boards = $boardMapper->findAllByOwner($owner); + $boards = $boardMapper->findAllByOwner($owner, Acl::PERMISSION_TYPE_USER); $boardNames = array_map(fn ($board) => $board->getTitle(), $boards); self::assertEquals(2, count($boards)); diff --git a/tests/unit/Listeners/ParticipantCleanupListenerTest.php b/tests/unit/Listeners/ParticipantCleanupListenerTest.php new file mode 100644 index 0000000000..1b6fd70d55 --- /dev/null +++ b/tests/unit/Listeners/ParticipantCleanupListenerTest.php @@ -0,0 +1,78 @@ +aclMapper = $this->createMock(AclMapper::class); + $this->assignmentMapper = $this->createMock(AssignmentMapper::class); + $this->boardMapper = $this->createMock(BoardMapper::class); + $this->listener = new ParticipantCleanupListener($this->aclMapper, $this->assignmentMapper, $this->boardMapper); + } + + public function testCircleDestroyedDeletesCircleOwnedBoardsAndCleansParticipantData(): void { + $circleId = 'circle-123'; + + $circle = $this->createMock(Circle::class); + $circle->expects($this->once()) + ->method('getSingleId') + ->willReturn($circleId); + + $event = $this->createMock(CircleDestroyedEvent::class); + $event->expects($this->once()) + ->method('getCircle') + ->willReturn($circle); + + $boardOne = new Board(); + $boardTwo = new Board(); + $this->boardMapper->expects($this->once()) + ->method('findAllByOwner') + ->with($circleId, Acl::PERMISSION_TYPE_CIRCLE) + ->willReturn([$boardOne, $boardTwo]); + $this->boardMapper->expects($this->exactly(2)) + ->method('delete'); + + $acl = new Acl(); + $this->aclMapper->expects($this->once()) + ->method('findByParticipant') + ->with(Acl::PERMISSION_TYPE_CIRCLE, $circleId) + ->willReturn([$acl]); + $this->aclMapper->expects($this->once()) + ->method('delete') + ->with($acl); + + $assignment = new Assignment(); + $this->assignmentMapper->expects($this->once()) + ->method('findByParticipant') + ->with($circleId, Acl::PERMISSION_TYPE_CIRCLE) + ->willReturn([$assignment]); + $this->assignmentMapper->expects($this->once()) + ->method('delete') + ->with($assignment); + + $this->listener->handle($event); + } +} diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index ae26e13cb3..83ed22768e 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -526,7 +526,7 @@ public function testFindMarksUserAclAsRetainedViaOwnerCircleMembership(): void { Acl::PERMISSION_EDIT => true, Acl::PERMISSION_MANAGE => true, Acl::PERMISSION_SHARE => true, - Acl::PERMISSION_OWNER => true, + Board::PERMISSION_OWNER => true, ]); $result = $this->service->find(10, false); @@ -593,11 +593,12 @@ public function testFindMarksUserAclAsRetainedViaOtherAclMembership(): void { Acl::PERMISSION_EDIT => true, Acl::PERMISSION_MANAGE => true, Acl::PERMISSION_SHARE => true, - Acl::PERMISSION_OWNER => false, + Board::PERMISSION_OWNER => false, ]); $result = $this->service->find(11, false); $this->assertTrue($result->getAcl()[0]->isRetainsAccessViaMembership()); + $this->assertTrue($result->getAcl()[1]->isRetainsAccessViaMembership()); } public function testFindMarksUserAclAsNotRetainedWithoutInheritedAccess(): void { @@ -646,7 +647,7 @@ public function testFindMarksUserAclAsNotRetainedWithoutInheritedAccess(): void Acl::PERMISSION_EDIT => true, Acl::PERMISSION_MANAGE => true, Acl::PERMISSION_SHARE => true, - Acl::PERMISSION_OWNER => false, + Board::PERMISSION_OWNER => false, ]); $result = $this->service->find(12, false); @@ -684,10 +685,10 @@ public function testTransferBoardOwnershipToCircle(): void { ->with(10, Acl::PERMISSION_TYPE_CIRCLE, 'circle-id-xyz'); // Previous user owner gets an ACL entry when changeContent = false - $this->aclMapper->expects($this->once()) + $this->aclMapper->expects($this->exactly(2)) ->method('findAll') ->willReturn([]); - $this->aclMapper->expects($this->once()) + $this->aclMapper->expects($this->exactly(2)) ->method('insert') ->willReturnCallback(fn ($acl) => $acl); @@ -703,6 +704,108 @@ public function testTransferBoardOwnershipToCircle(): void { $this->assertSame($updatedBoard, $result); } + public function testFindMarksCircleAclAsRetainedViaMembership(): void { + $board = new Board(); + $board->setId(13); + $board->setOwner('bob'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $circleAcl = new Acl(); + $circleAcl->setId(801); + $circleAcl->setBoardId(13); + $circleAcl->setType(Acl::PERMISSION_TYPE_CIRCLE); + $circleAcl->setParticipant('circle-2'); + $board->setAcl([$circleAcl]); + + $this->permissionService->expects($this->once()) + ->method('checkPermission') + ->with($this->boardMapper, 13, Acl::PERMISSION_READ, null, false); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(13, true, true, false) + ->willReturn($board); + + $this->boardMapper->expects($this->once()) + ->method('mapOwner') + ->with($board); + + $this->boardMapper->expects($this->once()) + ->method('mapAcl') + ->with($circleAcl); + + $this->circlesService->expects($this->never()) + ->method('isUserInCircle'); + + $this->permissionService->expects($this->never()) + ->method('userCan'); + + $this->permissionService->expects($this->once()) + ->method('matchPermissions') + ->with($board) + ->willReturn([ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + Board::PERMISSION_OWNER => false, + ]); + + $result = $this->service->find(13, false); + $this->assertTrue($result->getAcl()[0]->isRetainsAccessViaMembership()); + } + + public function testFindMarksRemoteAclAsNotRetainedViaMembership(): void { + $board = new Board(); + $board->setId(14); + $board->setOwner('bob'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $remoteAcl = new Acl(); + $remoteAcl->setId(901); + $remoteAcl->setBoardId(14); + $remoteAcl->setType(Acl::PERMISSION_TYPE_REMOTE); + $remoteAcl->setParticipant('https://remote.example'); + $board->setAcl([$remoteAcl]); + + $this->permissionService->expects($this->once()) + ->method('checkPermission') + ->with($this->boardMapper, 14, Acl::PERMISSION_READ, null, false); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(14, true, true, false) + ->willReturn($board); + + $this->boardMapper->expects($this->once()) + ->method('mapOwner') + ->with($board); + + $this->boardMapper->expects($this->once()) + ->method('mapAcl') + ->with($remoteAcl); + + $this->circlesService->expects($this->never()) + ->method('isUserInCircle'); + + $this->permissionService->expects($this->never()) + ->method('userCan'); + + $this->permissionService->expects($this->once()) + ->method('matchPermissions') + ->with($board) + ->willReturn([ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + Board::PERMISSION_OWNER => false, + ]); + + $result = $this->service->find(14, false); + $this->assertFalse($result->getAcl()[0]->isRetainsAccessViaMembership()); + } + public function testTransferBoardOwnershipToNonExistentUserThrows(): void { $board = new Board(); $board->setId(10); @@ -725,6 +828,21 @@ public function testTransferBoardOwnershipToNonExistentUserThrows(): void { $this->service->transferBoardOwnership(10, 'ghost', false, Acl::PERMISSION_TYPE_USER); } + public function testTransferOwnershipUsesSourceOwnerTypeWhenFetchingBoards(): void { + $this->userManager->expects($this->once()) + ->method('userExists') + ->with('bob') + ->willReturn(true); + + $this->boardMapper->expects($this->once()) + ->method('findAllByOwner') + ->with('circle-1', Acl::PERMISSION_TYPE_CIRCLE) + ->willReturn([]); + + $result = iterator_to_array($this->service->transferOwnership('circle-1', 'bob', false, Acl::PERMISSION_TYPE_USER, Acl::PERMISSION_TYPE_CIRCLE)); + $this->assertSame([], $result); + } + public function testTransferBoardOwnershipToNonExistentCircleThrows(): void { $board = new Board(); $board->setId(10);