diff --git a/.github/workflows/phpunit-mariadb.yml b/.github/workflows/phpunit-mariadb.yml index 00e73db136..146ef9a531 100644 --- a/.github/workflows/phpunit-mariadb.yml +++ b/.github/workflows/phpunit-mariadb.yml @@ -103,6 +103,14 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Checkout circles + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + repository: nextcloud/circles + ref: ${{ matrix.server-versions }} + path: apps/circles + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: @@ -145,6 +153,7 @@ jobs: run: | mkdir data ./occ maintenance:install --verbose --database=mysql --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass admin + ./occ app:enable --force circles ./occ app:enable --force ${{ env.APP_NAME }} - name: Check PHPUnit script is defined diff --git a/.github/workflows/phpunit-mysql.yml b/.github/workflows/phpunit-mysql.yml index 731714ba66..f244472d58 100644 --- a/.github/workflows/phpunit-mysql.yml +++ b/.github/workflows/phpunit-mysql.yml @@ -101,6 +101,14 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Checkout circles + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + repository: nextcloud/circles + ref: ${{ matrix.server-versions }} + path: apps/circles + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: @@ -143,6 +151,7 @@ jobs: run: | mkdir data ./occ maintenance:install --verbose --database=mysql --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass admin + ./occ app:enable --force circles ./occ app:enable --force ${{ env.APP_NAME }} - name: Check PHPUnit script is defined diff --git a/.github/workflows/phpunit-pgsql.yml b/.github/workflows/phpunit-pgsql.yml index 0d63f7dad1..1d22254604 100644 --- a/.github/workflows/phpunit-pgsql.yml +++ b/.github/workflows/phpunit-pgsql.yml @@ -104,6 +104,14 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Checkout circles + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + repository: nextcloud/circles + ref: ${{ matrix.server-versions }} + path: apps/circles + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: @@ -141,6 +149,7 @@ jobs: run: | mkdir data ./occ maintenance:install --verbose --database=pgsql --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass admin + ./occ app:enable --force circles ./occ app:enable --force ${{ env.APP_NAME }} - name: Check PHPUnit script is defined diff --git a/.github/workflows/phpunit-sqlite.yml b/.github/workflows/phpunit-sqlite.yml index d6bcf8e7bb..cfacc6dbd0 100644 --- a/.github/workflows/phpunit-sqlite.yml +++ b/.github/workflows/phpunit-sqlite.yml @@ -93,6 +93,14 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Checkout circles + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + repository: nextcloud/circles + ref: ${{ matrix.server-versions }} + path: apps/circles + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: @@ -130,6 +138,7 @@ jobs: run: | mkdir data ./occ maintenance:install --verbose --database=sqlite --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass admin + ./occ app:enable --force circles ./occ app:enable --force ${{ env.APP_NAME }} - name: Check PHPUnit script is defined diff --git a/.gitignore b/.gitignore index 5754b58af2..ea00195671 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,5 @@ _build node_modules vendor /.php_cs.cache +venv +.idea diff --git a/appinfo/info.xml b/appinfo/info.xml index 8236a08bc6..69da548660 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -61,6 +61,7 @@ Requirements: OCA\Bookmarks\Migration\OrphanedSharesRepairStep OCA\Bookmarks\Migration\OrphanedTreeItemsRepairStep OCA\Bookmarks\Migration\GroupSharesUpdateRepairStep + OCA\Bookmarks\Migration\CircleSharesUpdateRepairStep diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 833522ee11..28daee9155 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -86,9 +86,9 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(UserAddedEvent::class, UsersGroupsCirclesListener::class); $context->registerEventListener(UserRemovedEvent::class, UsersGroupsCirclesListener::class); $context->registerEventListener(BeforeGroupDeletedEvent::class, UsersGroupsCirclesListener::class); - $context->registerEventListener('\OCA\Circles\Events\CircleMemberAddedEvent', UsersGroupsCirclesListener::class); - $context->registerEventListener('\OCA\Circles\Events\CircleMemberRemovedEvent', UsersGroupsCirclesListener::class); - $context->registerEventListener('\OCA\Circles\Events\CircleDestroyedEvent', UsersGroupsCirclesListener::class); + $context->registerEventListener('OCA\Circles\Events\MembershipsCreatedEvent', UsersGroupsCirclesListener::class); + $context->registerEventListener('OCA\Circles\Events\MembershipsRemovedEvent', UsersGroupsCirclesListener::class); + $context->registerEventListener('OCA\Circles\Events\DestroyingCircleEvent', UsersGroupsCirclesListener::class); $context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class); diff --git a/lib/Controller/FoldersController.php b/lib/Controller/FoldersController.php index 50e58ad798..c6fcc2556b 100644 --- a/lib/Controller/FoldersController.php +++ b/lib/Controller/FoldersController.php @@ -125,10 +125,9 @@ private function _returnFolderAsArray($folder): array { return $returnFolder; } if ($folder instanceof SharedFolder) { - $share = $this->shareMapper->findByFolderAndUser($folder->getFolderId(), $folder->getUserId()); $returnFolder = $folder->toArray(); $returnFolder['id'] = $folder->getFolderId(); - $returnFolder['userId'] = $share->getOwner(); + $returnFolder['userId'] = $this->folderMapper->find($folder->getFolderId())->getUserId(); $parent = $this->treeMapper->findParentOf(TreeMapper::TYPE_SHARE, $folder->getId()); $returnFolder['parent_folder'] = $this->toExternalFolderId($parent->getId()); $returnFolder['userDisplayName'] = $this->userManager->get($returnFolder['userId'])->getDisplayName(); @@ -735,11 +734,13 @@ public function findSharedFolders(): DataResponse { } catch (Exception $e) { return new Http\DataResponse(['status' => 'error', 'data' => ['Internal error']], Http::STATUS_INTERNAL_SERVER_ERROR); } - return new Http\DataResponse(['status' => 'success', 'data' => array_map(function (Share $share) { + // The user may have the same folder through several shares + $folderIds = array_values(array_unique(array_map(static fn (Share $share) => $share->getFolderId(), $shares))); + return new Http\DataResponse(['status' => 'success', 'data' => array_map(static function (int $folderId) { return [ - 'id' => $share->getFolderId(), + 'id' => $folderId, ]; - }, $shares)]); + }, $folderIds)]); } /** @@ -798,7 +799,7 @@ public function getShares(int $folderId): DataResponse { if (Authorizer::hasPermission(Authorizer::PERM_READ, $permissions) && $this->authorizer->getUserId() !== null) { try { $this->folderMapper->find($folderId); - $share = $this->shareMapper->findByFolderAndUser($folderId, $this->authorizer->getUserId()); + $shares = $this->shareMapper->findByFolderAndUser($folderId, $this->authorizer->getUserId()); } catch (MultipleObjectsReturnedException|Exception) { return new DataResponse(['status' => 'error', 'data' => ['Internal error']], Http::STATUS_INTERNAL_SERVER_ERROR); } catch (DoesNotExistException $e) { @@ -806,7 +807,12 @@ public function getShares(int $folderId): DataResponse { $res->throttle(['action' => 'getShares']); return $res; } - return new Http\DataResponse(['status' => 'success', 'data' => [$share->toArray()]]); + if (count($shares) === 0) { + $res = new DataResponse(['status' => 'error', 'data' => ['Could not find folder']], Http::STATUS_NOT_FOUND); + $res->throttle(['action' => 'getShares']); + return $res; + } + return new Http\DataResponse(['status' => 'success', 'data' => array_map(static fn (Share $share) => $share->toArray(), $shares)]); } $res = new DataResponse(['status' => 'error', 'data' => ['Could not find folder']], Http::STATUS_NOT_FOUND); $res->throttle(['action' => 'getShares']); diff --git a/lib/Db/Share.php b/lib/Db/Share.php index cea599df59..6db9aa7c3d 100644 --- a/lib/Db/Share.php +++ b/lib/Db/Share.php @@ -79,18 +79,18 @@ public function toArray(): array { } private static function getParticipantDisplayName($type, $participant) { + // The participant may not exist anymore, e.g. if the user, group or circle was deleted, + // or the circles app was disabled try { - if ($type === IShare::TYPE_USER) { - return \OCP\Server::get(IUserManager::class)->get($participant)->getDisplayName(); - } - if ($type === IShare::TYPE_GROUP) { - return \OCP\Server::get(IGroupManager::class)->get($participant)->getDisplayName(); - } - if ($type === IShare::TYPE_CIRCLE) { - return \OCP\Server::get(CirclesService::class)->getCircle($participant)->getName(); - } + $displayName = match ($type) { + IShare::TYPE_USER => \OCP\Server::get(IUserManager::class)->get($participant)?->getDisplayName(), + IShare::TYPE_GROUP => \OCP\Server::get(IGroupManager::class)->get($participant)?->getDisplayName(), + IShare::TYPE_CIRCLE => \OCP\Server::get(CirclesService::class)->getCircle($participant)?->getName(), + default => null, + }; } catch (NotFoundExceptionInterface|ContainerExceptionInterface $e) { - return $participant; + $displayName = null; } + return $displayName ?? $participant; } } diff --git a/lib/Db/ShareMapper.php b/lib/Db/ShareMapper.php index a4ec050b1e..56135b0236 100644 --- a/lib/Db/ShareMapper.php +++ b/lib/Db/ShareMapper.php @@ -118,13 +118,14 @@ public function findByFolderAndParticipant(int $folderId, int $type, string $par } /** + * Finds all shares of the folder that the user's shared folder is part of + * * @param int $folderId * @param string $userId - * @return Share - * @throws DoesNotExistException - * @throws MultipleObjectsReturnedException + * @return Share[] + * @throws Exception */ - public function findByFolderAndUser(int $folderId, string $userId): Share { + public function findByFolderAndUser(int $folderId, string $userId): array { $qb = $this->db->getQueryBuilder(); $qb->select(array_map(static function ($c) { return 's.' . $c; @@ -134,7 +135,7 @@ public function findByFolderAndUser(int $folderId, string $userId): Share { ->leftJoin('t', 'bookmarks_shared_folders', 'sf', 'sf.id = t.shared_folder_id') ->where($qb->expr()->eq('s.folder_id', $qb->createPositionalParameter($folderId, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('sf.user_id', $qb->createPositionalParameter($userId))); - return $this->findEntity($qb); + return $this->findEntities($qb); } /** @@ -178,11 +179,12 @@ public function insertOrUpdate(Entity $entity): Share { } /** - * @throws DoesNotExistException - * @throws MultipleObjectsReturnedException + * Finds all shares that the shared folder is part of + * + * @return Share[] * @throws Exception */ - public function findBySharedFolder(int $id): Share { + public function findBySharedFolder(int $id): array { $qb = $this->db->getQueryBuilder(); $qb->select(array_map(static function ($c) { return 's.' . $c; @@ -190,7 +192,7 @@ public function findBySharedFolder(int $id): Share { ->from('bookmarks_shares', 's') ->innerJoin('s', 'bookmarks_shared_to_shares', 't', 's.id = t.share_id') ->where($qb->expr()->eq('t.shared_folder_id', $qb->createPositionalParameter($id, IQueryBuilder::PARAM_INT))); - return $this->findEntity($qb); + return $this->findEntities($qb); } /** diff --git a/lib/Db/SharedFolderMapper.php b/lib/Db/SharedFolderMapper.php index 2cc976faa3..65e5648eea 100644 --- a/lib/Db/SharedFolderMapper.php +++ b/lib/Db/SharedFolderMapper.php @@ -9,6 +9,7 @@ namespace OCA\Bookmarks\Db; use OCA\Bookmarks\Events\CreateEvent; +use OCA\Bookmarks\Events\UpdateEvent; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\MultipleObjectsReturnedException; @@ -96,7 +97,8 @@ public function findByFolder(int $folderId): array { */ public function findByOwner(string $userId): array { $qb = $this->db->getQueryBuilder(); - $qb->select(array_map(static function ($c) { + // A shared folder can be part of several shares of the same owner + $qb->selectDistinct(array_map(static function ($c) { return 'p.' . $c; }, SharedFolder::$columns)) ->from('bookmarks_shared_folders', 'p') @@ -174,7 +176,8 @@ public function findByFolderAndUser(int $folderId, string $userId): SharedFolder */ public function findByOwnerAndUser(string $owner, string $userId): array { $qb = $this->db->getQueryBuilder(); - $qb->select(array_map(static function ($c) { + // A shared folder can be part of several shares of the same owner + $qb->selectDistinct(array_map(static function ($c) { return 'p.' . $c; }, SharedFolder::$columns)) ->from('bookmarks_shared_folders', 'p') @@ -188,12 +191,12 @@ public function findByOwnerAndUser(string $owner, string $userId): array { /** * @param int $shareId * @param string $userId - * @return SharedFolder - * @throws DoesNotExistException - * @throws MultipleObjectsReturnedException + * @return SharedFolder[] * @throws Exception + * + * @psalm-return array */ - public function findByShareAndUser(int $shareId, string $userId): SharedFolder { + public function findByShareAndUser(int $shareId, string $userId): array { $qb = $this->db->getQueryBuilder(); $qb->select(array_map(static function ($c) { return 'p.' . $c; @@ -202,7 +205,7 @@ public function findByShareAndUser(int $shareId, string $userId): SharedFolder { ->leftJoin('p', 'bookmarks_shared_to_shares', 't', 't.shared_folder_id = p.id') ->where($qb->expr()->eq('t.share_id', $qb->createPositionalParameter($shareId, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('p.user_id', $qb->createPositionalParameter($userId))); - return $this->findEntity($qb); + return $this->findEntities($qb); } /** @@ -244,6 +247,23 @@ public function mount(int $id, int $share_id): void { )); } + /** + * Removes the shared folder from a share. It may still be part of other shares of the same folder. + * + * @throws Exception + */ + public function unmount(int $id, int $shareId): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_shared_to_shares') + ->where($qb->expr()->eq('shared_folder_id', $qb->createPositionalParameter($id, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('share_id', $qb->createPositionalParameter($shareId, IQueryBuilder::PARAM_INT))) + ->executeStatement(); + $this->eventDispatcher->dispatch(UpdateEvent::class, new UpdateEvent( + TreeMapper::TYPE_SHARE, + $id + )); + } + /** * @return SharedFolder[] * diff --git a/lib/Db/TreeMapper.php b/lib/Db/TreeMapper.php index 756cda5250..3d261771a0 100644 --- a/lib/Db/TreeMapper.php +++ b/lib/Db/TreeMapper.php @@ -650,12 +650,28 @@ public function deleteShare(int $shareId): void { $share = $this->shareMapper->find($shareId); $sharedFolders = $this->sharedFolderMapper->findByShare($shareId); foreach ($sharedFolders as $sharedFolder) { - $this->sharedFolderMapper->delete($sharedFolder); - $this->deleteEntry(TreeMapper::TYPE_SHARE, $sharedFolder->getId()); + $this->removeSharedFolderFromShare($sharedFolder, $shareId); } $this->shareMapper->delete($share); } + /** + * Removes the shared folder from the share. The shared folder is only deleted + * if it isn't part of another share of the same folder. + * + * @throws DoesNotExistException + * @throws MultipleObjectsReturnedException + * @throws UnsupportedOperation|Exception + */ + public function removeSharedFolderFromShare(SharedFolder $sharedFolder, int $shareId): void { + $this->sharedFolderMapper->unmount($sharedFolder->getId(), $shareId); + if (count($this->shareMapper->findBySharedFolder($sharedFolder->getId())) > 0) { + return; + } + $this->sharedFolderMapper->delete($sharedFolder); + $this->deleteEntry(TreeMapper::TYPE_SHARE, $sharedFolder->getId()); + } + /** * @param string $type * @psalm-param TreeMapper::TYPE_* $type @@ -683,12 +699,12 @@ public function move(string $type, int $itemId, int $newParentFolderId, ?int $in } else { $sharedFolder = $this->sharedFolderMapper->find($itemId); $folderId = $sharedFolder->getFolderId(); - $share = $this->shareMapper->findBySharedFolder($sharedFolder->getId()); + $owner = $this->folderMapper->find($folderId)->getUserId(); // Make sure that the sharer of this share doesn't have a share of the target folder or one of its parents // would make a share loop very probable, which would be very bad. Breaks the whole app. - if ($this->isFolderSharedWithUser($newParentFolderId, $share->getOwner())) { + if ($this->isFolderSharedWithUser($newParentFolderId, $owner)) { throw new UnsupportedOperation('Cannot nest a folder shared from user A inside a folder shared with user A'); } } @@ -996,18 +1012,18 @@ public function getSubFolders(int $folderId, $layers = 0, ?bool $isSoftDeleted = }, $this->findChildren(TreeMapper::TYPE_FOLDER, $folderId, $isSoftDeleted)); $shares = array_map(function (SharedFolder $sharedFolder) use ($layers, $folderId, $isSoftDeleted) { try { - $share = $this->shareMapper->findBySharedFolder($sharedFolder->getId()); + $folder = $this->folderMapper->find($sharedFolder->getFolderId()); } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { $this->logger->error('Failed to load a shared folder', ['exception' => $e]); return null; } $array = $sharedFolder->toArray(); - $array['id'] = $share->getFolderId(); - $array['userId'] = $share->getOwner(); + $array['id'] = $folder->getId(); + $array['userId'] = $folder->getUserId(); $array['userDisplayName'] = $this->userManager->get($array['userId'])->getDisplayName(); $array['parent_folder'] = $folderId; if ($layers !== 0) { - $array['children'] = $this->getSubFolders($share->getFolderId(), $layers - 1, $isSoftDeleted); + $array['children'] = $this->getSubFolders($folder->getId(), $layers - 1, $isSoftDeleted); } return $array; }, $this->findChildren(TreeMapper::TYPE_SHARE, $folderId, $isSoftDeleted)); diff --git a/lib/Hooks/UsersGroupsCirclesListener.php b/lib/Hooks/UsersGroupsCirclesListener.php index 3d0926ab6a..d3b7807a18 100644 --- a/lib/Hooks/UsersGroupsCirclesListener.php +++ b/lib/Hooks/UsersGroupsCirclesListener.php @@ -18,11 +18,9 @@ use OCA\Bookmarks\Service\BookmarkService; use OCA\Bookmarks\Service\CirclesService; use OCA\Bookmarks\Service\FolderService; -use OCA\Circles\Events\CircleDestroyedEvent; -use OCA\Circles\Events\CircleMemberAddedEvent; -use OCA\Circles\Events\CircleMemberGenericEvent; -use OCA\Circles\Events\CircleMemberRemovedEvent; -use OCA\Circles\Model\Federated\FederatedEvent; +use OCA\Circles\Events\DestroyingCircleEvent; +use OCA\Circles\Events\MembershipsCreatedEvent; +use OCA\Circles\Events\MembershipsRemovedEvent; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\Exception; @@ -31,11 +29,10 @@ use OCP\Group\Events\BeforeGroupDeletedEvent; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; -use OCP\IGroupManager; use OCP\Share\IShare; use OCP\User\Events\BeforeUserDeletedEvent; -/** @template-implements IEventListener */ +/** @template-implements IEventListener */ class UsersGroupsCirclesListener implements IEventListener { public function __construct( private ShareMapper $shareMapper, @@ -43,48 +40,38 @@ public function __construct( private SharedFolderMapper $sharedFolderMapper, private TreeMapper $treeMapper, private CirclesService $circlesService, - private IGroupManager $groupManager, private BookmarkService $bookmarksService, ) { } public function handle(Event $event): void { - if ($event instanceof CircleDestroyedEvent) { + if ($event instanceof DestroyingCircleEvent) { $shares = $this->shareMapper->findByParticipant(IShare::TYPE_CIRCLE, $event->getCircle()->getSingleId()); foreach ($shares as $share) { try { $this->folderService->deleteShare($share->getId()); - } catch (UnsupportedOperation|DoesNotExistException|MultipleObjectsReturnedException $e) { + } catch (UnsupportedOperation|DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { } } } - if ($event instanceof CircleMemberGenericEvent) { - if ($event instanceof CircleMemberAddedEvent) { - $shares = $this->shareMapper->findByParticipant(IShare::TYPE_CIRCLE, $event->getCircle()->getSingleId()); - foreach ($shares as $share) { - $this->addParticipantToShare($share, $event->getMember()->getUserType(), $event->getMember()->getUserId()); + if ($event instanceof MembershipsCreatedEvent || $event instanceof MembershipsRemovedEvent) { + // Memberships are flattened: they include memberships through groups and nested circles, + // and a membership is only removed once the user isn't part of the circle in any way anymore + foreach ($event->getMemberships() as $membership) { + $shares = $this->shareMapper->findByParticipant(IShare::TYPE_CIRCLE, $membership->getCircleId()); + if (count($shares) === 0) { + continue; } - // propagate upward - foreach ($event->getCircle()->getMemberships() as $membership) { - $circle = $this->circlesService->getCircle($membership->getSingleId()); - $federatedEvent = new FederatedEvent(); - $federatedEvent->setCircle($circle); - $federatedEvent->setMember($event->getMember()); - $this->handle(new CircleMemberAddedEvent($federatedEvent)); + $userId = $this->circlesService->getLocalUserIdOfSingleId($membership->getSingleId()); + if ($userId === null) { + continue; } - } - if ($event instanceof CircleMemberRemovedEvent) { - $shares = $this->shareMapper->findByParticipant(IShare::TYPE_CIRCLE, $event->getCircle()->getSingleId()); foreach ($shares as $share) { - $this->removeParticipantFromShare($share, $event->getMember()->getUserType(), $event->getMember()->getUserId()); - } - // propagate upward - foreach ($event->getCircle()->getMemberships() as $membership) { - $circle = $this->circlesService->getCircle($membership->getSingleId()); - $federatedEvent = new FederatedEvent(); - $federatedEvent->setCircle($circle); - $federatedEvent->setMember($event->getMember()); - $this->handle(new CircleMemberRemovedEvent($federatedEvent)); + if ($event instanceof MembershipsCreatedEvent) { + $this->addParticipantToShare($share, $userId); + } else { + $this->folderService->removeSharedFolderOfUser($share, $userId); + } } } } @@ -98,8 +85,22 @@ public function handle(Event $event): void { $sharesToDelete = $this->shareMapper->findByParticipant(IShare::TYPE_USER, $event->getUser()->getUID()); foreach ($sharesToDelete as $share) { try { - $this->shareMapper->delete($share); - } catch (Exception $e) { + $this->folderService->deleteShare($share->getId()); + } catch (UnsupportedOperation|DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { + // noop + } + } + // delete the shared folders the user received through groups and circles + try { + $sharedFoldersToDelete = $this->sharedFolderMapper->findByUser($event->getUser()->getUID()); + } catch (Exception $e) { + $sharedFoldersToDelete = []; + } + foreach ($sharedFoldersToDelete as $sharedFolder) { + try { + $this->treeMapper->deleteEntry(TreeMapper::TYPE_SHARE, $sharedFolder->getId()); + $this->sharedFolderMapper->delete($sharedFolder); + } catch (UnsupportedOperation|DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { // noop } } @@ -107,88 +108,31 @@ public function handle(Event $event): void { if ($event instanceof UserAddedEvent) { $shares = $this->shareMapper->findByParticipant(IShare::TYPE_GROUP, $event->getGroup()->getGID()); foreach ($shares as $share) { - $this->addParticipantToShare($share, IShare::TYPE_USER, $event->getUser()->getUID()); + $this->addParticipantToShare($share, $event->getUser()->getUID()); } } if ($event instanceof UserRemovedEvent) { $shares = $this->shareMapper->findByParticipant(IShare::TYPE_GROUP, $event->getGroup()->getGID()); foreach ($shares as $share) { - $this->removeParticipantFromShare($share, IShare::TYPE_USER, $event->getUser()->getUID()); + $this->folderService->removeSharedFolderOfUser($share, $event->getUser()->getUID()); } } if ($event instanceof BeforeGroupDeletedEvent) { $sharesToDelete = $this->shareMapper->findByParticipant(IShare::TYPE_GROUP, $event->getGroup()->getGID()); foreach ($sharesToDelete as $share) { - $this->shareMapper->delete($share); - } - } - } - - private function removeParticipantFromShare(Share $share, int $type, string $participant): void { - if ($type === IShare::TYPE_CIRCLE) { - $circle = $this->circlesService->getCircle($participant); - if ($circle === null) { - return; - } - foreach ($circle->getMembers() as $member) { - $this->removeParticipantFromShare($share, $member->getUserType(), $member->getUserId()); - } - } elseif ($type === IShare::TYPE_GROUP) { - $group = $this->groupManager->get($participant); - if ($group === null) { - return; - } - foreach ($group->getUsers() as $user) { - $this->removeParticipantFromShare($share, IShare::TYPE_USER, $user->getUID()); - } - } elseif ($type === IShare::TYPE_USER) { - try { - $sharedFoldersToDelete = $this->sharedFolderMapper->findByShareAndUser($share->getId(), $participant); - } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { - return; - } - foreach ($sharedFoldersToDelete as $sharedFolder) { try { - $this->treeMapper->deleteEntry(TreeMapper::TYPE_SHARE, $sharedFolder->getId()); - } catch (UnsupportedOperation|DoesNotExistException|MultipleObjectsReturnedException $e) { + $this->folderService->deleteShare($share->getId()); + } catch (UnsupportedOperation|DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { + // noop } } } } - private function addParticipantToShare(Share $share, int $type, string $participant): void { - if ($type === IShare::TYPE_CIRCLE) { - $circle = $this->circlesService->getCircle($participant); - if ($circle === null) { - return; - } - foreach ($circle->getMembers() as $member) { - $this->addParticipantToShare($share, $member->getUserType(), $member->getUserId()); - } - } elseif ($type === IShare::TYPE_GROUP) { - $group = $this->groupManager->get($participant); - if ($group === null) { - return; - } - foreach ($group->getUsers() as $user) { - $this->addParticipantToShare($share, IShare::TYPE_USER, $user->getUID()); - } - } elseif ($type === IShare::TYPE_USER) { - if ($share->getOwner() === $participant) { - return; - } - try { - $this->sharedFolderMapper->findByShareAndUser($share->getId(), $participant); - // if this does not throw, the user already has this folder - return; - } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { - // noop - } - try { - $folder = $this->folderService->findById($share->getFolderId()); - $this->folderService->addSharedFolder($share, $folder, $participant); - } catch (DoesNotExistException|MultipleObjectsReturnedException|UnsupportedOperation $e) { - } + private function addParticipantToShare(Share $share, string $userId): void { + try { + $this->folderService->addParticipantToShare($share, $userId); + } catch (DoesNotExistException|MultipleObjectsReturnedException|UnsupportedOperation|Exception $e) { } } } diff --git a/lib/Migration/CircleSharesUpdateRepairStep.php b/lib/Migration/CircleSharesUpdateRepairStep.php new file mode 100644 index 0000000000..3aab07e9ee --- /dev/null +++ b/lib/Migration/CircleSharesUpdateRepairStep.php @@ -0,0 +1,67 @@ +circlesService->isCirclesEnabled()) { + $output->info('Circles app is not enabled, skipping'); + return; + } + + $qb = $this->db->getQueryBuilder(); + $shareIds = $qb->select('id') + ->from('bookmarks_shares') + ->where($qb->expr()->eq('type', $qb->createPositionalParameter(IShare::TYPE_CIRCLE))) + ->executeQuery() + ->fetchAll(PDO::FETCH_COLUMN); + + $added = 0; + $removed = 0; + $deleted = 0; + foreach ($shareIds as $shareId) { + try { + $result = $this->folders->syncShareParticipants($this->shareMapper->find((int)$shareId)); + } catch (Throwable $e) { + $output->warning('Could not update circle share ' . $shareId . ': ' . $e->getMessage()); + continue; + } + $added += $result['added']; + $removed += $result['removed']; + $deleted += $result['deleted'] ? 1 : 0; + } + $output->info("Added $added and removed $removed users in " . count($shareIds) . " circle shares, deleted $deleted shares of circles that don't exist anymore"); + } +} diff --git a/lib/Migration/DeduplicateSharedFoldersRepairStep.php b/lib/Migration/DeduplicateSharedFoldersRepairStep.php index 781ef9988e..1ca64418dd 100644 --- a/lib/Migration/DeduplicateSharedFoldersRepairStep.php +++ b/lib/Migration/DeduplicateSharedFoldersRepairStep.php @@ -8,9 +8,11 @@ namespace OCA\Bookmarks\Migration; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; +use PDO; class DeduplicateSharedFoldersRepairStep implements IRepairStep { /** @@ -39,12 +41,41 @@ public function getName() { public function run(IOutput $output) { $qb = $this->db->getQueryBuilder(); $qb->select('p1.id') + ->selectAlias($qb->func()->min('p2.id'), 'kept_id') ->from('bookmarks_shared_folders', 'p1') ->leftJoin('p1', 'bookmarks_shared_folders', 'p2', 'p1.folder_id = p2.folder_id AND p1.user_id = p2.user_id') - ->where($qb->expr()->lt('p2.id', 'p1.id')); - $duplicateSharedFolders = $qb->executeQuery(); + ->where($qb->expr()->lt('p2.id', 'p1.id')) + ->groupBy('p1.id'); + $duplicateSharedFolders = $qb->executeQuery()->fetchAll(); $i = 0; - while ($sharedFolder = $duplicateSharedFolders->fetch(\PDO::FETCH_COLUMN)) { + foreach ($duplicateSharedFolders as $row) { + $sharedFolder = (int)$row['id']; + $keptSharedFolder = (int)$row['kept_id']; + // The kept shared folder becomes part of the shares of the duplicate + $qb = $this->db->getQueryBuilder(); + $shareIds = $qb->select('share_id') + ->from('bookmarks_shared_to_shares') + ->where($qb->expr()->eq('shared_folder_id', $qb->createPositionalParameter($sharedFolder, IQueryBuilder::PARAM_INT))) + ->executeQuery() + ->fetchAll(PDO::FETCH_COLUMN); + $qb = $this->db->getQueryBuilder(); + $keptShareIds = $qb->select('share_id') + ->from('bookmarks_shared_to_shares') + ->where($qb->expr()->eq('shared_folder_id', $qb->createPositionalParameter($keptSharedFolder, IQueryBuilder::PARAM_INT))) + ->executeQuery() + ->fetchAll(PDO::FETCH_COLUMN); + foreach (array_diff($shareIds, $keptShareIds) as $shareId) { + $qb = $this->db->getQueryBuilder(); + $qb->insert('bookmarks_shared_to_shares')->values([ + 'shared_folder_id' => $qb->createPositionalParameter($keptSharedFolder, IQueryBuilder::PARAM_INT), + 'share_id' => $qb->createPositionalParameter((int)$shareId, IQueryBuilder::PARAM_INT), + ])->executeStatement(); + } + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_shared_to_shares') + ->where($qb->expr()->eq('shared_folder_id', $qb->createPositionalParameter($sharedFolder, IQueryBuilder::PARAM_INT))) + ->executeStatement(); + $qb = $this->db->getQueryBuilder(); $qb->delete('bookmarks_tree') ->where($qb->expr()->eq('id', $qb->createPositionalParameter($sharedFolder))) diff --git a/lib/Migration/GroupSharesUpdateRepairStep.php b/lib/Migration/GroupSharesUpdateRepairStep.php index c9c5ee08a0..77f6f0e9a9 100644 --- a/lib/Migration/GroupSharesUpdateRepairStep.php +++ b/lib/Migration/GroupSharesUpdateRepairStep.php @@ -8,59 +8,25 @@ namespace OCA\Bookmarks\Migration; -use OCA\Bookmarks\Db\Folder; -use OCA\Bookmarks\Db\FolderMapper; -use OCA\Bookmarks\Db\Share; -use OCA\Bookmarks\Db\SharedFolderMapper; use OCA\Bookmarks\Db\ShareMapper; -use OCA\Bookmarks\Exception\UnsupportedOperation; use OCA\Bookmarks\Service\FolderService; -use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\IDBConnection; -use OCP\IGroupManager; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; use OCP\Share\IShare; use PDO; +use Throwable; class GroupSharesUpdateRepairStep implements IRepairStep { - /** - * @var IDBConnection - */ - private $db; - /** - * @var IGroupManager - */ - private $groupManager; - /** - * @var FolderService - */ - private $folders; - /** - * @var ShareMapper - */ - private $shareMapper; - /** - * @var FolderMapper - */ - private $folderMapper; - /** - * @var SharedFolderMapper - */ - private $sharedFolderMapper; - - public function __construct(IDBConnection $db, IGroupManager $groupManager, FolderService $folders, ShareMapper $shareMapper, FolderMapper $folderMapper, SharedFolderMapper $sharedFolderMapper) { - $this->db = $db; - $this->groupManager = $groupManager; - $this->folders = $folders; - $this->shareMapper = $shareMapper; - $this->folderMapper = $folderMapper; - $this->sharedFolderMapper = $sharedFolderMapper; + public function __construct( + private IDBConnection $db, + private FolderService $folders, + private ShareMapper $shareMapper, + ) { } /** - * * Returns the step's name + * Returns the step's name * * @return string */ @@ -71,72 +37,31 @@ public function getName() { /** * @param IOutput $output * - * @throws UnsupportedOperation - * @throws DoesNotExistException - * @throws MultipleObjectsReturnedException - * * @return void */ public function run(IOutput $output) { - $deleted = 0; - $added = 0; - $groups = 0; - $deletedShares = 0; - - // find group shares $qb = $this->db->getQueryBuilder(); - $qb->select('s.id', 's.participant', 's.folder_id', 's.owner') - ->from('bookmarks_shares', 's') - ->where($qb->expr()->eq('s.type', $qb->createPositionalParameter(IShare::TYPE_GROUP))); - $groupShares = $qb->executeQuery(); + $shareIds = $qb->select('id') + ->from('bookmarks_shares') + ->where($qb->expr()->eq('type', $qb->createPositionalParameter(IShare::TYPE_GROUP))) + ->executeQuery() + ->fetchAll(PDO::FETCH_COLUMN); - while ($groupShare = $groupShares->fetch()) { - // find users in share - $qb = $this->db->getQueryBuilder(); - $qb->select('sf.user_id') - ->from('bookmarks_shared_folders', 'sf') - ->join('sf', 'bookmarks_shared_to_shares', 't', $qb->expr()->eq('t.shared_folder_id', 'sf.id')) - ->where($qb->expr()->eq('t.share_id', $qb->createPositionalParameter($groupShare['id']))); - $usersInShare = $qb->executeQuery()->fetchAll(PDO::FETCH_COLUMN); - - $group = $this->groupManager->get($groupShare['participant']); - if ($group === null) { - $this->folders->deleteShare($groupShare['id']); - $deletedShares++; + $added = 0; + $removed = 0; + $deleted = 0; + foreach ($shareIds as $shareId) { + try { + $result = $this->folders->syncShareParticipants($this->shareMapper->find((int)$shareId)); + } catch (Throwable $e) { + $output->warning('Could not update group share ' . $shareId . ': ' . $e->getMessage()); continue; } - $usersInGroup = array_filter(array_map(static function ($user) { - return $user->getUID(); - }, $group->getUsers()), static function ($userId) use ($groupShare) { - return $userId !== $groupShare['owner']; - }); - - $notInShareUsers = array_diff($usersInGroup, $usersInShare); - $notInGroupUsers = array_diff($usersInShare, $usersInGroup); - - foreach ($notInGroupUsers as $userId) { - $this->folders->deleteSharedFolderOrFolder($userId, $groupShare['folder_id'], true); - $sharedFolder = $this->sharedFolderMapper->findByFolderAndUser($groupShare['folder_id'], $userId); - $this->sharedFolderMapper->delete($sharedFolder); - $deleted++; - } - - foreach ($notInShareUsers as $userId) { - /** - * @var Share $share - */ - $share = $this->shareMapper->find($groupShare['id']); - /** - * @var Folder $folder - */ - $folder = $this->folderMapper->find($groupShare['folder_id']); - $this->folders->addSharedFolder($share, $folder, $userId); - - $added++; - } - $groups++; + $added += $result['added']; + $removed += $result['removed']; + $deleted += $result['deleted'] ? 1 : 0; } - $output->info("Removed $deleted users and added $added users to $groups groups"); - $output->info("Removed $deletedShares shares"); + $output->info("Removed $removed shared folders and added $added shared folders"); + $output->info("Removed $deleted shares"); } } diff --git a/lib/Migration/OrphanedSharesRepairStep.php b/lib/Migration/OrphanedSharesRepairStep.php index 889665a39b..a2e90c6463 100644 --- a/lib/Migration/OrphanedSharesRepairStep.php +++ b/lib/Migration/OrphanedSharesRepairStep.php @@ -60,8 +60,15 @@ public function run(IOutput $output) { ->where($qb->expr()->eq('type', $qb->createPositionalParameter('share'))) ->andWhere($qb->expr()->eq('id', $qb->createPositionalParameter($folderId, IQueryBuilder::PARAM_INT))) ->executeStatement(); + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_shared_folders') + ->where($qb->expr()->eq('id', $qb->createPositionalParameter($folderId, IQueryBuilder::PARAM_INT))) + ->executeStatement(); } - $this->db->executeQuery('DELETE sf FROM *PREFIX*bookmarks_shared_folders sf JOIN *PREFIX*bookmarks_shared_to_shares t ON sf.id = t.shared_folder_id WHERE t.share_id = ?', [$share]); + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_shared_to_shares') + ->where($qb->expr()->eq('share_id', $qb->createPositionalParameter($share, IQueryBuilder::PARAM_INT))) + ->executeStatement(); $qb = $this->db->getQueryBuilder(); $qb->delete('bookmarks_shares') ->where($qb->expr()->eq('id', $qb->createPositionalParameter($share))) @@ -86,5 +93,76 @@ public function run(IOutput $output) { $i++; } $output->info("Removed $i orphaned public links"); + + // Links between shares and shared folders whose share doesn't exist anymore, left behind by earlier versions when a group or user was deleted + $qb = $this->db->getQueryBuilder(); + $shareIds = $qb->selectDistinct('t.share_id') + ->from('bookmarks_shared_to_shares', 't') + ->leftJoin('t', 'bookmarks_shares', 's', $qb->expr()->eq('s.id', 't.share_id')) + ->where($qb->expr()->isNull('s.id')) + ->executeQuery() + ->fetchAll(PDO::FETCH_COLUMN); + foreach ($shareIds as $shareId) { + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_shared_to_shares') + ->where($qb->expr()->eq('share_id', $qb->createPositionalParameter($shareId, IQueryBuilder::PARAM_INT))) + ->executeStatement(); + } + $output->info('Removed links to ' . count($shareIds) . ' shares that don\'t exist anymore'); + + // Shared folders that aren't part of any share anymore + $qb = $this->db->getQueryBuilder(); + $sharedFolderIds = $qb->select('sf.id') + ->from('bookmarks_shared_folders', 'sf') + ->leftJoin('sf', 'bookmarks_shared_to_shares', 't', $qb->expr()->eq('t.shared_folder_id', 'sf.id')) + ->where($qb->expr()->isNull('t.share_id')) + ->executeQuery() + ->fetchAll(PDO::FETCH_COLUMN); + foreach ($sharedFolderIds as $sharedFolderId) { + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_tree') + ->where($qb->expr()->eq('type', $qb->createPositionalParameter('share'))) + ->andWhere($qb->expr()->eq('id', $qb->createPositionalParameter($sharedFolderId, IQueryBuilder::PARAM_INT))) + ->executeStatement(); + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_shared_folders') + ->where($qb->expr()->eq('id', $qb->createPositionalParameter($sharedFolderId, IQueryBuilder::PARAM_INT))) + ->executeStatement(); + } + $output->info('Removed ' . count($sharedFolderIds) . ' shared folders without a share'); + + // Links between shares and shared folders whose shared folder doesn't exist anymore + $qb = $this->db->getQueryBuilder(); + $linkIds = $qb->selectDistinct('t.shared_folder_id') + ->from('bookmarks_shared_to_shares', 't') + ->leftJoin('t', 'bookmarks_shared_folders', 'sf', $qb->expr()->eq('sf.id', 't.shared_folder_id')) + ->where($qb->expr()->isNull('sf.id')) + ->executeQuery() + ->fetchAll(PDO::FETCH_COLUMN); + foreach ($linkIds as $sharedFolderId) { + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_shared_to_shares') + ->where($qb->expr()->eq('shared_folder_id', $qb->createPositionalParameter($sharedFolderId, IQueryBuilder::PARAM_INT))) + ->executeStatement(); + } + $output->info('Removed ' . count($linkIds) . ' orphaned links between shares and shared folders'); + + // Tree entries of shared folders that don't exist anymore + $qb = $this->db->getQueryBuilder(); + $treeIds = $qb->select('t.id') + ->from('bookmarks_tree', 't') + ->leftJoin('t', 'bookmarks_shared_folders', 'sf', $qb->expr()->eq('sf.id', 't.id')) + ->where($qb->expr()->eq('t.type', $qb->createPositionalParameter('share'))) + ->andWhere($qb->expr()->isNull('sf.id')) + ->executeQuery() + ->fetchAll(PDO::FETCH_COLUMN); + foreach ($treeIds as $sharedFolderId) { + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_tree') + ->where($qb->expr()->eq('type', $qb->createPositionalParameter('share'))) + ->andWhere($qb->expr()->eq('id', $qb->createPositionalParameter($sharedFolderId, IQueryBuilder::PARAM_INT))) + ->executeStatement(); + } + $output->info('Removed ' . count($treeIds) . ' orphaned shared folder entries'); } } diff --git a/lib/Migration/Version017000000Date20261008124723.php b/lib/Migration/Version017000000Date20261008124723.php new file mode 100644 index 0000000000..57506f9dbd --- /dev/null +++ b/lib/Migration/Version017000000Date20261008124723.php @@ -0,0 +1,81 @@ +hasTable('bookmarks_shared_to_shares')) { + $table = $schema->getTable('bookmarks_shared_to_shares'); + $primaryKey = $table->getPrimaryKey(); + if ($primaryKey === null || $primaryKey->getColumns() !== ['shared_folder_id', 'share_id']) { + if ($primaryKey !== null) { + $table->dropPrimaryKey(); + } + // Keep the default name: Doctrine drops a changed primary key by the new key's name, + // which only resolves to the existing key on MySQL and Postgres if it is "primary" + $table->setPrimaryKey(['shared_folder_id', 'share_id']); + } + } + return $schema; + } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + // Users who were given a folder directly that they already had through a group or circle + // didn't get their shared folder linked to the user share + $qb = $this->db->getQueryBuilder(); + $qb->select('s.id') + ->selectAlias('sf.id', 'shared_folder_id') + ->from('bookmarks_shares', 's') + ->innerJoin('s', 'bookmarks_shared_folders', 'sf', $qb->expr()->andX( + $qb->expr()->eq('sf.folder_id', 's.folder_id'), + $qb->expr()->eq('sf.user_id', 's.participant'), + )) + ->leftJoin('sf', 'bookmarks_shared_to_shares', 't', $qb->expr()->andX( + $qb->expr()->eq('t.shared_folder_id', 'sf.id'), + $qb->expr()->eq('t.share_id', 's.id'), + )) + ->where($qb->expr()->eq('s.type', $qb->createPositionalParameter(IShare::TYPE_USER, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->isNull('t.share_id')); + $result = $qb->executeQuery(); + $i = 0; + while ($row = $result->fetch()) { + $qb = $this->db->getQueryBuilder(); + $qb->insert('bookmarks_shared_to_shares')->values([ + 'shared_folder_id' => $qb->createPositionalParameter((int)$row['shared_folder_id'], IQueryBuilder::PARAM_INT), + 'share_id' => $qb->createPositionalParameter((int)$row['id'], IQueryBuilder::PARAM_INT), + ])->executeStatement(); + $i++; + } + $result->closeCursor(); + $output->info("Linked $i shared folders to user shares"); + } +} diff --git a/lib/Service/Authorizer.php b/lib/Service/Authorizer.php index 3f3a76f6f1..f869621761 100644 --- a/lib/Service/Authorizer.php +++ b/lib/Service/Authorizer.php @@ -301,6 +301,9 @@ private function findPermissionsByUserAndItem(string $userId, string $type, int return self::PERM_ALL; } + // A user may be covered by several shares of the same folder (e.g. directly, through a group, + // or through someone who re-shared a folder they received), so they get the combined permissions. + $accessPerms = self::PERM_NONE; $shares = $this->shareMapper->findByOwner($item->getUserId()); foreach ($shares as $share) { if ($share->getFolderId() === $itemId && $type === TreeMapper::TYPE_FOLDER) { @@ -316,16 +319,17 @@ private function findPermissionsByUserAndItem(string $userId, string $type, int $sharedFolders = $this->sharedFolderMapper->findByShare($share->getId()); foreach ($sharedFolders as $sharedFolder) { if ($sharedFolder->getUserId() === $userId) { - return $perms; + $accessPerms |= $perms; + } else { + $accessPerms |= $perms & $this->findPermissionsByUserAndItem($userId, TreeMapper::TYPE_SHARE, $sharedFolder->getId()); } - $secondLevelPerms = $this->findPermissionsByUserAndItem($userId, TreeMapper::TYPE_SHARE, $sharedFolder->getId()); - if ($secondLevelPerms !== self::PERM_NONE) { - return $perms & $secondLevelPerms; + if ($accessPerms === self::PERM_ALL) { + return $accessPerms; } } } - return self::PERM_NONE; + return $accessPerms; } /** diff --git a/lib/Service/BookmarkPreviewer.php b/lib/Service/BookmarkPreviewer.php index 326677a05b..788858f55f 100644 --- a/lib/Service/BookmarkPreviewer.php +++ b/lib/Service/BookmarkPreviewer.php @@ -24,7 +24,6 @@ class BookmarkPreviewer implements IBookmarkPreviewer { // Cache for one month public const CACHE_TTL = 4 * 4 * 7 * 24 * 60 * 60; - private string $enabled; private FileCache $cache; private DefaultBookmarkPreviewer $defaultPreviewer; private ScreeenlyBookmarkPreviewer $screeenlyPreviewer; @@ -33,7 +32,7 @@ class BookmarkPreviewer implements IBookmarkPreviewer { private Previewers\GenericUrlBookmarkPreviewer $genericUrlPreviewer; public function __construct( - IConfig $config, + private IConfig $config, ScreeenlyBookmarkPreviewer $screeenlyPreviewer, DefaultBookmarkPreviewer $defaultPreviewer, FileCache $cache, @@ -47,7 +46,6 @@ public function __construct( $this->pageresPreviewer = $pageresPreviewer; $this->genericUrlPreviewer = $genericUrlPreviewer; - $this->enabled = $config->getAppValue('bookmarks', 'privacy.enableScraping', 'false'); $this->cache = $cache; } @@ -56,7 +54,7 @@ public function __construct( * @return IImage */ public function getImage($bookmark, $cacheOnly = false): ?IImage { - if ($this->enabled === 'false') { + if ($this->config->getAppValue('bookmarks', 'privacy.enableScraping', 'false') === 'false') { return null; } diff --git a/lib/Service/BookmarkService.php b/lib/Service/BookmarkService.php index a704f52151..b576d8ab60 100644 --- a/lib/Service/BookmarkService.php +++ b/lib/Service/BookmarkService.php @@ -180,8 +180,21 @@ private function _addBookmark($userId, $url, ?string $title = null, ?string $des isset($description) ? [] : ['description'], ); - $title = $title ?? trim($data['basic']['title']) ?? trim($url); - $description = $description ?? $data['basic']['description'] ?? ''; + if ($title === null) { + if (isset($data['basic']['title']) && $data['basic']['title'] !== '') { + $title = trim($data['basic']['title']); + } else { + $title = trim($url); + } + } + + if ($description === null) { + if (isset($data['basic']['description'])) { + $description = trim($data['basic']['description']); + } else { + $description = ''; + } + } $bookmark->setUrl($url); } diff --git a/lib/Service/CirclesService.php b/lib/Service/CirclesService.php index 0a51ca2c17..0a800a7e7d 100644 --- a/lib/Service/CirclesService.php +++ b/lib/Service/CirclesService.php @@ -20,6 +20,7 @@ */ class CirclesService { public const TYPE = 1; + public const TYPE_CIRCLE = 16; public const LEVEL_MEMBER = 1; private bool $circlesEnabled; @@ -39,11 +40,86 @@ public function getCircle(string $circleId) { } try { + return $this->fetchCircle($circleId); + } catch (Throwable $e) { + } + return null; + } + + /** + * @return bool|null whether the circle exists, or null if that can't be determined right now + */ + public function circleExists(string $circleId): ?bool { + if (!$this->circlesEnabled) { + return null; + } + + try { + $this->fetchCircle($circleId); + return true; + } catch (Throwable $e) { + if (is_a($e, 'OCA\Circles\Exceptions\FederatedUserNotFoundException')) { + return false; + } + } + return null; + } + + /** + * Looks up a circle without the visibility checks of the current circles session. + * CirclesManager::getCircle() filters by the session's initiator, and a super session + * doesn't help once circles has set a current app, which it does whenever it syncs a + * group (e.g. on group creation or membership changes) and never resets. + * + * @throws Throwable + */ + private function fetchCircle(string $circleId) { + $circlesManager = Server::get('OCA\Circles\CirclesManager'); + return $circlesManager->getFederatedUser($circleId, self::TYPE_CIRCLE)->getBasedOn(); + } - // Enforce current user condition since we always want the full list of members + /** + * Resolves a circle to the local users it contains, including users that are + * members through groups or nested circles + * + * @param string $circleId circle single id + * @return string[] user ids + */ + public function getUserIdsOfCircle(string $circleId): array { + $circle = $this->getCircle($circleId); + if ($circle === null) { + return []; + } + + try { + $userIds = []; + foreach ($circle->getInheritedMembers() as $member) { + if ($member->getUserType() === self::TYPE) { + $userIds[] = $member->getUserId(); + } + } + return array_values(array_unique($userIds)); + } catch (Throwable $e) { + } + return []; + } + + /** + * @param string $singleId single id of a federated user as found in a circles Membership + * @return string|null the user id if the single id belongs to a local user + */ + public function getLocalUserIdOfSingleId(string $singleId): ?string { + if (!$this->circlesEnabled) { + return null; + } + + try { $circlesManager = Server::get('OCA\Circles\CirclesManager'); - $circlesManager->startSuperSession(); - return $circlesManager->getCircle($circleId); + $federatedUser = $circlesManager->getFederatedUser($singleId); + if ($federatedUser->getUserType() !== self::TYPE || !$federatedUser->isLocal()) { + return null; + } + return $federatedUser->getUserId(); } catch (Throwable $e) { } return null; diff --git a/lib/Service/FaviconPreviewer.php b/lib/Service/FaviconPreviewer.php index 50dd9681ed..fc8ab48de9 100644 --- a/lib/Service/FaviconPreviewer.php +++ b/lib/Service/FaviconPreviewer.php @@ -47,18 +47,13 @@ class FaviconPreviewer implements IBookmarkPreviewer { * @var \OCP\IConfig */ private $config; - /** - * @var string - */ - private $enabled; public function __construct(FileCache $cache, LinkExplorer $linkExplorer, LoggerInterface $logger, IClientService $clientService, \OCP\IConfig $config) { $this->cache = $cache; $this->linkExplorer = $linkExplorer; $this->logger = $logger; $this->client = $clientService->newClient(); - - $this->enabled = $config->getAppValue('bookmarks', 'privacy.enableScraping', 'false'); + $this->config = $config; } /** @@ -67,7 +62,7 @@ public function __construct(FileCache $cache, LinkExplorer $linkExplorer, Logger * @return Image|null */ public function getImage($bookmark, $cacheOnly = false): ?IImage { - if ($this->enabled === 'false') { + if ($this->config->getAppValue('bookmarks', 'privacy.enableScraping', 'false') === 'false') { return null; } if (!isset($bookmark)) { diff --git a/lib/Service/FolderService.php b/lib/Service/FolderService.php index 0e4778b7eb..d51da20255 100644 --- a/lib/Service/FolderService.php +++ b/lib/Service/FolderService.php @@ -184,6 +184,157 @@ public function deleteShare($shareId): void { $this->treeMapper->deleteShare($shareId); } + /** + * Removes the user's shared folder from the given share. If the user still has + * the folder through another share, they keep it. + */ + public function removeSharedFolderOfUser(Share $share, string $userId): void { + try { + $sharedFolders = $this->sharedFolderMapper->findByShareAndUser($share->getId(), $userId); + } catch (Exception $e) { + return; + } + foreach ($sharedFolders as $sharedFolder) { + try { + $this->treeMapper->removeSharedFolderFromShare($sharedFolder, $share->getId()); + } catch (UnsupportedOperation|DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { + } + } + } + + /** + * Makes sure that the current members of a group or circle share have the shared folder and that + * former members don't. Deletes the share if its group or circle doesn't exist anymore. + * + * @return array{added: int, removed: int, deleted: bool} + * @throws DoesNotExistException + * @throws MultipleObjectsReturnedException + * @throws UnsupportedOperation + * @throws Exception + */ + public function syncShareParticipants(Share $share): array { + $result = ['added' => 0, 'removed' => 0, 'deleted' => false]; + $userIds = null; + if ($share->getType() === IShare::TYPE_GROUP) { + $group = $this->groupManager->get($share->getParticipant()); + if ($group !== null) { + $userIds = array_map(static fn ($user) => $user->getUID(), $group->getUsers()); + } + } elseif ($share->getType() === IShare::TYPE_CIRCLE) { + $circleExists = $this->circlesService->circleExists($share->getParticipant()); + if ($circleExists === null) { + // We can't tell right now, so better not touch anything + return $result; + } + if ($circleExists) { + $userIds = $this->circlesService->getUserIdsOfCircle($share->getParticipant()); + if (count($userIds) === 0) { + // A circle always has an owner, so looking up its members probably failed + return $result; + } + } + } else { + throw new UnsupportedOperation('Only group and circle shares can be synced'); + } + + if ($userIds === null) { + $this->deleteShare($share->getId()); + $result['deleted'] = true; + return $result; + } + + $formerUserIds = []; + foreach ($this->sharedFolderMapper->findByShare($share->getId()) as $sharedFolder) { + if (!in_array($sharedFolder->getUserId(), $userIds, true)) { + $formerUserIds[] = $sharedFolder->getUserId(); + } + } + foreach (array_unique($formerUserIds) as $userId) { + $this->removeSharedFolderOfUser($share, $userId); + $result['removed']++; + } + + $folder = $this->folderMapper->find($share->getFolderId()); + foreach ($userIds as $userId) { + if ($userId === $folder->getUserId()) { + continue; + } + // Users who have the folder through another share get their shared folder added to this share + if ($this->mountSharedFolderOfFolder($share, $userId, false)) { + continue; + } + // If this folder already contains a share from this user, don't share it back. Would cause a loop. + if ($this->treeMapper->containsSharedFolderFromUser($folder, $userId)) { + continue; + } + $this->addSharedFolder($share, $folder, $userId); + $result['added']++; + } + + return $result; + } + + /** + * Gives a user who just became a participant of a group or circle share the shared folder. + * If they already have the folder through another share, that shared folder is added to this share. + * + * @throws DoesNotExistException + * @throws MultipleObjectsReturnedException + * @throws UnsupportedOperation + * @throws Exception + */ + public function addParticipantToShare(Share $share, string $userId): void { + if ($share->getOwner() === $userId) { + return; + } + if ($this->mountSharedFolderOfFolder($share, $userId, false)) { + return; + } + $folder = $this->folderMapper->find($share->getFolderId()); + // If this folder already contains a share from this user, don't share it back. Would cause a loop. + if ($this->treeMapper->containsSharedFolderFromUser($folder, $userId)) { + return; + } + $this->addSharedFolder($share, $folder, $userId); + } + + /** + * If the user already has a shared folder of exactly this folder, through any share, it is added to the given share. + * Shared ancestor folders don't count, because they can be unshared independently. + * + * @param bool $restore If the user removed the shared folder from their tree earlier, which keeps the SharedFolder around, + * put it back into their root folder. Only use this when a share is created: that's an explicit action, + * unlike membership changes. + * @return bool Whether the user has a shared folder of exactly this folder + * @throws Exception + * @throws UnsupportedOperation + * @throws DoesNotExistException + * @throws MultipleObjectsReturnedException + */ + private function mountSharedFolderOfFolder(Share $share, string $userId, bool $restore): bool { + try { + $sharedFolder = $this->sharedFolderMapper->findByFolderAndUser($share->getFolderId(), $userId); + } catch (DoesNotExistException) { + return false; + } catch (MultipleObjectsReturnedException) { + // Older versions could create several + return true; + } + $shareIds = array_map(static fn (Share $s) => $s->getId(), $this->shareMapper->findBySharedFolder($sharedFolder->getId())); + if (!in_array($share->getId(), $shareIds, true)) { + $this->sharedFolderMapper->mount($sharedFolder->getId(), $share->getId()); + } + if ($restore) { + try { + $this->treeMapper->findParentOf(TreeMapper::TYPE_SHARE, $sharedFolder->getId()); + } catch (DoesNotExistException) { + $rootFolder = $this->folderMapper->findRootFolder($userId); + $this->treeMapper->move(TreeMapper::TYPE_SHARE, $sharedFolder->getId(), $rootFolder->getId()); + } + } + return true; + } + /** * @throws UnsupportedOperation * @throws MultipleObjectsReturnedException @@ -341,7 +492,10 @@ public function createShare($folderId, $participant, int $type, bool $canWrite = throw new UnsupportedOperation('Cannot share this with user that shared some of its contents'); } $this->shareMapper->insert($share); - $this->addSharedFolder($share, $folder, $participant); + // If the user already has this folder, e.g. through a group, don't add it twice. + if (!$this->mountSharedFolderOfFolder($share, $participant, true)) { + $this->addSharedFolder($share, $folder, $participant); + } } else { $this->addSharedFolderForParticipant($share, $folder, $type, $participant); } @@ -365,9 +519,8 @@ public function addSharedFolderForParticipant(Share $share, Folder $folder, int $this->shareMapper->insert($share); } - $members = $circle->getMembers(); - foreach ($members as $member) { - $this->addSharedFolderForParticipant($share, $folder, $member->getUserType(), $member->getUserId(), false); + foreach ($this->circlesService->getUserIdsOfCircle($participant) as $userId) { + $this->addSharedFolderForParticipant($share, $folder, IShare::TYPE_USER, $userId, false); } } if ($type === IShare::TYPE_GROUP) { @@ -386,7 +539,7 @@ public function addSharedFolderForParticipant(Share $share, Folder $folder, int continue; } // If this folder is already shared with the user, don't add it twice. - if ($this->treeMapper->isFolderSharedWithUser($folder->getId(), $user->getUID())) { + if ($this->mountSharedFolderOfFolder($share, $user->getUID(), true)) { continue; } @@ -403,10 +556,6 @@ public function addSharedFolderForParticipant(Share $share, Folder $folder, int if ($participant === $folder->getUserId()) { return; } - // If this folder is already shared with the user, don't add it twice. - if ($this->treeMapper->isFolderSharedWithUser($folder->getId(), $participant)) { - return; - } // If this folder already contains a share from this user, don't share it back. Would cause a loop. if ($this->treeMapper->containsSharedFolderFromUser($folder, $participant)) { @@ -417,6 +566,11 @@ public function addSharedFolderForParticipant(Share $share, Folder $folder, int $this->shareMapper->insert($share); } + // If this folder is already shared with the user, don't add it twice. + if ($this->mountSharedFolderOfFolder($share, $participant, true)) { + return; + } + $this->addSharedFolder($share, $folder, $participant); } } diff --git a/lib/Service/LinkExplorer.php b/lib/Service/LinkExplorer.php index fa3f75c237..d5f7c04ed9 100644 --- a/lib/Service/LinkExplorer.php +++ b/lib/Service/LinkExplorer.php @@ -20,19 +20,14 @@ class LinkExplorer { private $linkPreview; - private $logger; - - /** - * @var string - */ - private $enabled; - - public function __construct(IClientService $clientService, LoggerInterface $logger, IConfig $config) { + public function __construct( + IClientService $clientService, + private LoggerInterface $logger, + private IConfig $config, + ) { $client = $clientService->newClient(); $this->linkPreview = new LinkPreview(new Client($client), new RequestFactory()); $this->linkPreview->getParser('general')->setMinimumImageDimensions(150, 550); - $this->logger = $logger; - $this->enabled = $config->getAppValue('bookmarks', 'privacy.enableScraping', 'false'); } /** @@ -43,7 +38,7 @@ public function __construct(IClientService $clientService, LoggerInterface $logg public function get($url): array { $data = ['url' => $url]; - if ($this->enabled === 'false') { + if ($this->config->getAppValue('bookmarks', 'privacy.enableScraping', 'false') === 'false') { return $data; } diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 0e487c5b94..45394cf238 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,16 +1,5 @@ - - - - - - - - - - - - + @@ -66,21 +55,6 @@ - - - - - - - - - - - - - - - @@ -107,11 +81,6 @@ - - - - - diff --git a/tests/AuthorizerTest.php b/tests/AuthorizerTest.php new file mode 100644 index 0000000000..8d7564be00 --- /dev/null +++ b/tests/AuthorizerTest.php @@ -0,0 +1,145 @@ +cleanUp(); + + $this->userManager = \OCP\Server::get(IUserManager::class); + $this->groupManager = \OCP\Server::get(IGroupManager::class); + $this->folderMapper = \OCP\Server::get(Db\FolderMapper::class); + $this->treeMapper = \OCP\Server::get(Db\TreeMapper::class); + $this->sharedFolderMapper = \OCP\Server::get(Db\SharedFolderMapper::class); + $this->folders = \OCP\Server::get(FolderService::class); + $this->authorizer = \OCP\Server::get(Authorizer::class); + } + + private function createUser(string $uid): string { + if (!$this->userManager->userExists($uid)) { + $this->userManager->createUser($uid, 'password'); + } + return $this->userManager->get($uid)->getUID(); + } + + private function createFolder(string $userId, int $parentFolderId): Db\Folder { + $folder = new Db\Folder(); + $folder->setTitle('folder'); + $folder->setUserId($userId); + $this->folderMapper->insert($folder); + $this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $folder->getId(), $parentFolderId); + return $folder; + } + + /** + * A user who has a folder through a read-only group share and is also given + * write access directly only has one shared folder, which is part of both shares, + * and gets the permissions of both shares. + */ + public function testPermissionsOfAllSharesAreCombined(): void { + $ownerId = $this->createUser('perms_combined_owner'); + $memberId = $this->createUser('perms_combined_member'); + $outsiderId = $this->createUser('perms_combined_outsider'); + $group = $this->groupManager->createGroup('perms_combined'); + $group->addUser($this->userManager->get($memberId)); + try { + $folder = $this->createFolder($ownerId, $this->folderMapper->findRootFolder($ownerId)->getId()); + $subFolder = $this->createFolder($ownerId, $folder->getId()); + + $groupShare = $this->folders->createShare($folder->getId(), $group->getGID(), IShare::TYPE_GROUP, false, false); + $this->assertEquals( + Authorizer::PERM_READ | Authorizer::PERM_EDIT, + $this->authorizer->getUserPermissionsForFolder($memberId, $folder->getId()) + ); + $this->assertEquals(Authorizer::PERM_READ, $this->authorizer->getUserPermissionsForFolder($memberId, $subFolder->getId())); + + $userShare = $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER, true, true); + // The member still only has one shared folder, which is now part of both shares + $groupSharedFolders = $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId); + $userSharedFolders = $this->sharedFolderMapper->findByShareAndUser($userShare->getId(), $memberId); + $this->assertCount(1, $groupSharedFolders); + $this->assertCount(1, $userSharedFolders); + $this->assertEquals($groupSharedFolders[0]->getId(), $userSharedFolders[0]->getId()); + + $this->assertEquals(Authorizer::PERM_ALL, $this->authorizer->getUserPermissionsForFolder($memberId, $folder->getId())); + $this->assertEquals( + Authorizer::PERM_READ | Authorizer::PERM_EDIT | Authorizer::PERM_WRITE | Authorizer::PERM_RESHARE, + $this->authorizer->getUserPermissionsForFolder($memberId, $subFolder->getId()) + ); + $this->assertEquals(Authorizer::PERM_NONE, $this->authorizer->getUserPermissionsForFolder($outsiderId, $folder->getId())); + } finally { + $group->delete(); + } + } + + /** + * A user who has a folder directly with read-only access, and with write access + * through someone who re-shared a folder containing it, gets the permissions of both. + */ + public function testPermissionsThroughReshareAreCombinedWithDirectAccess(): void { + $ownerId = $this->createUser('perms_reshare_owner'); + $resharerId = $this->createUser('perms_reshare_resharer'); + $memberId = $this->createUser('perms_reshare_member'); + + $folder = $this->createFolder($ownerId, $this->folderMapper->findRootFolder($ownerId)->getId()); + $subFolder = $this->createFolder($ownerId, $folder->getId()); + $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER, false, false); + $this->assertEquals(Authorizer::PERM_READ, $this->authorizer->getUserPermissionsForFolder($memberId, $subFolder->getId())); + + // The resharer puts the folder they received into one of their own folders and shares that with the member + $resharerShare = $this->folders->createShare($folder->getId(), $resharerId, IShare::TYPE_USER, true, true); + $resharerFolder = $this->createFolder($resharerId, $this->folderMapper->findRootFolder($resharerId)->getId()); + $sharedFolder = $this->sharedFolderMapper->findByShareAndUser($resharerShare->getId(), $resharerId)[0]; + $this->treeMapper->move(Db\TreeMapper::TYPE_SHARE, $sharedFolder->getId(), $resharerFolder->getId()); + $this->folders->createShare($resharerFolder->getId(), $memberId, IShare::TYPE_USER, true, false); + + $this->assertEquals( + Authorizer::PERM_READ | Authorizer::PERM_EDIT | Authorizer::PERM_WRITE, + $this->authorizer->getUserPermissionsForFolder($memberId, $subFolder->getId()) + ); + } + + /** + * Being a participant of a share without having a shared folder for it must + * not grant access. This happens when sharing would create a loop: a folder + * that contains a folder shared by a group member is shared with that group. + */ + public function testParticipantWithoutSharedFolderHasNoAccess(): void { + $ownerId = $this->createUser('perms_loop_owner'); + $memberId = $this->createUser('perms_loop_member'); + $group = $this->groupManager->createGroup('perms_loop'); + $group->addUser($this->userManager->get($memberId)); + try { + // The member shares one of their folders with the owner, who puts it into their own folder + $memberFolder = $this->createFolder($memberId, $this->folderMapper->findRootFolder($memberId)->getId()); + $memberShare = $this->folders->createShare($memberFolder->getId(), $ownerId, IShare::TYPE_USER); + $ownerFolder = $this->createFolder($ownerId, $this->folderMapper->findRootFolder($ownerId)->getId()); + $sharedFolder = $this->sharedFolderMapper->findByShareAndUser($memberShare->getId(), $ownerId)[0]; + $this->treeMapper->move(Db\TreeMapper::TYPE_SHARE, $sharedFolder->getId(), $ownerFolder->getId()); + + // Sharing the owner's folder with the group skips the member + $groupShare = $this->folders->createShare($ownerFolder->getId(), $group->getGID(), IShare::TYPE_GROUP, true, true); + $this->assertCount(0, $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId)); + + $this->assertEquals(Authorizer::PERM_NONE, $this->authorizer->getUserPermissionsForFolder($memberId, $ownerFolder->getId())); + } finally { + $group->delete(); + } + } +} diff --git a/tests/CirclesSharingTest.php b/tests/CirclesSharingTest.php new file mode 100644 index 0000000000..6bcc4f8487 --- /dev/null +++ b/tests/CirclesSharingTest.php @@ -0,0 +1,458 @@ +isEnabledForAnyone('circles')) { + $this->markTestSkipped('The circles app is not enabled'); + } + $this->cleanUp(); + + $this->userManager = \OCP\Server::get(IUserManager::class); + $this->groupManager = \OCP\Server::get(IGroupManager::class); + $this->folderMapper = \OCP\Server::get(Db\FolderMapper::class); + $this->treeMapper = \OCP\Server::get(Db\TreeMapper::class); + $this->sharedFolderMapper = \OCP\Server::get(Db\SharedFolderMapper::class); + $this->shareMapper = \OCP\Server::get(Db\ShareMapper::class); + $this->folders = \OCP\Server::get(FolderService::class); + $this->circlesManager = \OCP\Server::get(CirclesManager::class); + + $this->ownerId = $this->createUser('circle_share_owner'); + } + + protected function tearDown(): void { + if (isset($this->circlesManager)) { + foreach (array_reverse($this->circles) as $circleId) { + try { + $this->startCirclesSession(); + $this->circlesManager->destroyCircle($circleId); + } catch (\Throwable $e) { + // already destroyed + } + } + $this->circlesManager->stopSession(); + } + $this->circles = []; + parent::tearDown(); + } + + private function createUser(string $uid): string { + if (!$this->userManager->userExists($uid)) { + $this->userManager->createUser($uid, 'password'); + } + return $this->userManager->get($uid)->getUID(); + } + + private function startCirclesSession(): void { + $this->circlesManager->startSession($this->circlesManager->getLocalFederatedUser($this->ownerId), true); + } + + private function createCircle(string $name): Circle { + $this->startCirclesSession(); + $circle = $this->circlesManager->createCircle($name . '_' . uniqid(), null, false, false, false); + $this->circles[] = $circle->getSingleId(); + return $circle; + } + + private function addUserToCircle(Circle $circle, string $userId): Member { + $this->startCirclesSession(); + return $this->circlesManager->addMember($circle->getSingleId(), $this->circlesManager->getLocalFederatedUser($userId)); + } + + private function addGroupToCircle(Circle $circle, string $groupId): Member { + $this->startCirclesSession(); + return $this->circlesManager->addMember($circle->getSingleId(), $this->circlesManager->getFederatedUser($groupId, Member::TYPE_GROUP)); + } + + private function addCircleToCircle(Circle $parent, Circle $child): Member { + $this->startCirclesSession(); + return $this->circlesManager->addMember($parent->getSingleId(), $this->circlesManager->getFederatedUser($child->getSingleId(), Member::TYPE_CIRCLE)); + } + + private function createFolder(): Db\Folder { + $folder = new Db\Folder(); + $folder->setTitle('shared with circle'); + $folder->setUserId($this->ownerId); + $this->folderMapper->insert($folder); + $this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $folder->getId(), $this->folderMapper->findRootFolder($this->ownerId)->getId()); + return $folder; + } + + private function shareWithCircle(Circle $circle): Db\Share { + return $this->folders->createShare($this->createFolder()->getId(), $circle->getSingleId(), IShare::TYPE_CIRCLE); + } + + private function assertHasSharedFolder(Db\Share $share, string $userId): void { + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($share->getId(), $userId), "$userId should have the shared folder"); + } + + private function assertHasNoSharedFolder(Db\Share $share, string $userId): void { + $this->assertCount(0, $this->sharedFolderMapper->findByShareAndUser($share->getId(), $userId), "$userId should not have the shared folder"); + } + + public function testCircleShareGivesUserMembersSharedFolder(): void { + $memberId = $this->createUser('circle_share_user_member'); + $circle = $this->createCircle('user_members'); + $this->addUserToCircle($circle, $memberId); + + $share = $this->shareWithCircle($circle); + + $this->assertHasSharedFolder($share, $memberId); + $this->assertHasNoSharedFolder($share, $this->ownerId); + } + + public function testCircleShareGivesGroupMembersSharedFolder(): void { + $memberId = $this->createUser('circle_share_group_member'); + $group = $this->groupManager->createGroup('circle_share_group'); + $group->addUser($this->userManager->get($memberId)); + try { + $circle = $this->createCircle('group_members'); + $this->addGroupToCircle($circle, $group->getGID()); + + $share = $this->shareWithCircle($circle); + + $this->assertHasSharedFolder($share, $memberId); + } finally { + $group->delete(); + } + } + + public function testCircleShareGivesNestedCircleMembersSharedFolder(): void { + $memberId = $this->createUser('circle_share_nested_member'); + $inner = $this->createCircle('nested_inner'); + $this->addUserToCircle($inner, $memberId); + $outer = $this->createCircle('nested_outer'); + $this->addCircleToCircle($outer, $inner); + + $share = $this->shareWithCircle($outer); + + $this->assertHasSharedFolder($share, $memberId); + } + + public function testAddedCircleMemberGetsSharedFolder(): void { + $memberId = $this->createUser('circle_share_added_member'); + $circle = $this->createCircle('added_member'); + $share = $this->shareWithCircle($circle); + $this->assertHasNoSharedFolder($share, $memberId); + + $this->addUserToCircle($circle, $memberId); + + $this->assertHasSharedFolder($share, $memberId); + } + + public function testRemovedCircleMemberLosesSharedFolder(): void { + $memberId = $this->createUser('circle_share_removed_member'); + $circle = $this->createCircle('removed_member'); + $member = $this->addUserToCircle($circle, $memberId); + $share = $this->shareWithCircle($circle); + $this->assertHasSharedFolder($share, $memberId); + + $this->startCirclesSession(); + $this->circlesManager->removeMember($member->getId()); + + $this->assertHasNoSharedFolder($share, $memberId); + } + + public function testMemberAddedToNestedCircleGetsSharedFolder(): void { + $memberId = $this->createUser('circle_share_nested_added_member'); + $inner = $this->createCircle('nested_added_inner'); + $outer = $this->createCircle('nested_added_outer'); + $this->addCircleToCircle($outer, $inner); + $share = $this->shareWithCircle($outer); + $this->assertHasNoSharedFolder($share, $memberId); + + $this->addUserToCircle($inner, $memberId); + + $this->assertHasSharedFolder($share, $memberId); + } + + public function testMemberRemovedFromNestedCircleLosesSharedFolder(): void { + $memberId = $this->createUser('circle_share_nested_removed_member'); + $inner = $this->createCircle('nested_removed_inner'); + $member = $this->addUserToCircle($inner, $memberId); + $outer = $this->createCircle('nested_removed_outer'); + $this->addCircleToCircle($outer, $inner); + $share = $this->shareWithCircle($outer); + $this->assertHasSharedFolder($share, $memberId); + + $this->startCirclesSession(); + $this->circlesManager->removeMember($member->getId()); + + $this->assertHasNoSharedFolder($share, $memberId); + } + + public function testDestroyedCircleRemovesShareAndSharedFolders(): void { + $memberId = $this->createUser('circle_share_destroyed_member'); + $circle = $this->createCircle('destroyed'); + $this->addUserToCircle($circle, $memberId); + $share = $this->shareWithCircle($circle); + $this->assertHasSharedFolder($share, $memberId); + + $this->startCirclesSession(); + $this->circlesManager->destroyCircle($circle->getSingleId()); + + try { + $this->shareMapper->find($share->getId()); + $this->fail('Share should have been deleted together with the circle'); + } catch (DoesNotExistException $e) { + // expected + } + $this->assertCount(0, $this->sharedFolderMapper->findByShare($share->getId())); + } + + public function testUserAddedToGroupInCircleGetsSharedFolder(): void { + $memberId = $this->createUser('circle_share_group_added_member'); + $group = $this->groupManager->createGroup('circle_share_group_added'); + try { + $circle = $this->createCircle('group_added'); + $this->addGroupToCircle($circle, $group->getGID()); + $share = $this->shareWithCircle($circle); + $this->assertHasNoSharedFolder($share, $memberId); + + $group->addUser($this->userManager->get($memberId)); + + $this->assertHasSharedFolder($share, $memberId); + } finally { + $group->delete(); + } + } + + public function testUserRemovedFromGroupInCircleLosesSharedFolder(): void { + $memberId = $this->createUser('circle_share_group_removed_member'); + $group = $this->groupManager->createGroup('circle_share_group_removed'); + $group->addUser($this->userManager->get($memberId)); + try { + $circle = $this->createCircle('group_removed'); + $this->addGroupToCircle($circle, $group->getGID()); + $share = $this->shareWithCircle($circle); + $this->assertHasSharedFolder($share, $memberId); + + $group->removeUser($this->userManager->get($memberId)); + + $this->assertHasNoSharedFolder($share, $memberId); + } finally { + $group->delete(); + } + } + + public function testUserRemovedFromGroupInCircleKeepsSharedFolderAsDirectMember(): void { + $memberId = $this->createUser('circle_share_group_and_direct_member'); + $group = $this->groupManager->createGroup('circle_share_group_and_direct'); + $group->addUser($this->userManager->get($memberId)); + try { + $circle = $this->createCircle('group_and_direct'); + $this->addGroupToCircle($circle, $group->getGID()); + $this->addUserToCircle($circle, $memberId); + $share = $this->shareWithCircle($circle); + $this->assertHasSharedFolder($share, $memberId); + + $group->removeUser($this->userManager->get($memberId)); + + $this->assertHasSharedFolder($share, $memberId); + } finally { + $group->delete(); + } + } + + /** + * A user who leaves a group but is still covered by a circle share of the same + * folder must keep their shared folder, which is part of both shares, and + * vice versa. + */ + public function testUserKeepsFolderSharedWithGroupAndCircleUntilRemovedFromBoth(): void { + $memberId = $this->createUser('circle_share_group_or_circle_member'); + $group = $this->groupManager->createGroup('circle_share_group_or_circle'); + $group->addUser($this->userManager->get($memberId)); + try { + $circle = $this->createCircle('group_or_circle'); + $member = $this->addUserToCircle($circle, $memberId); + $folder = $this->createFolder(); + $groupShare = $this->folders->createShare($folder->getId(), $group->getGID(), IShare::TYPE_GROUP); + $circleShare = $this->folders->createShare($folder->getId(), $circle->getSingleId(), IShare::TYPE_CIRCLE); + $this->assertHasSharedFolder($groupShare, $memberId); + $this->assertHasSharedFolder($circleShare, $memberId); + $this->assertCount(1, $this->sharedFolderMapper->findByUser($memberId)); + $sharedFolderId = $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId)[0]->getId(); + + $group->removeUser($this->userManager->get($memberId)); + + $this->assertHasNoSharedFolder($groupShare, $memberId); + $this->assertHasSharedFolder($circleShare, $memberId); + $this->assertEquals($sharedFolderId, $this->sharedFolderMapper->findByShareAndUser($circleShare->getId(), $memberId)[0]->getId()); + + $this->startCirclesSession(); + $this->circlesManager->removeMember($member->getId()); + + $this->assertHasNoSharedFolder($circleShare, $memberId); + } finally { + $group->delete(); + } + } + + /** + * A user who already has a folder through a direct share and then joins a circle + * the folder is also shared with must not get a second shared folder. + */ + public function testAddedCircleMemberKeepsSingleSharedFolderOfDirectShare(): void { + $memberId = $this->createUser('circle_share_direct_then_circle_member'); + $circle = $this->createCircle('direct_then_circle'); + $folder = $this->createFolder(); + $userShare = $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER); + $circleShare = $this->folders->createShare($folder->getId(), $circle->getSingleId(), IShare::TYPE_CIRCLE); + $this->assertHasSharedFolder($userShare, $memberId); + + $this->addUserToCircle($circle, $memberId); + + // The existing shared folder becomes part of the circle share + $this->assertHasSharedFolder($circleShare, $memberId); + $this->assertCount(1, $this->sharedFolderMapper->findByUser($memberId)); + } + + /** + * A user who removed a folder shared with them directly must get it back + * when the folder is then shared with a circle they are a member of. + */ + public function testCircleShareRestoresFolderRemovedFromDirectShare(): void { + $memberId = $this->createUser('circle_share_removed_then_circle_member'); + $circle = $this->createCircle('removed_then_circle'); + $this->addUserToCircle($circle, $memberId); + $folder = $this->createFolder(); + $userShare = $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER); + $sharedFolderId = $this->sharedFolderMapper->findByShareAndUser($userShare->getId(), $memberId)[0]->getId(); + + $this->folders->deleteSharedFolderOrFolder($memberId, $folder->getId(), true); + + $this->folders->createShare($folder->getId(), $circle->getSingleId(), IShare::TYPE_CIRCLE); + + $sharedFolders = $this->sharedFolderMapper->findByUser($memberId); + $this->assertCount(1, $sharedFolders); + $this->assertEquals($sharedFolderId, $sharedFolders[0]->getId()); + $rootFolder = $this->folderMapper->findRootFolder($memberId); + $this->assertEquals($rootFolder->getId(), $this->treeMapper->findParentOf(Db\TreeMapper::TYPE_SHARE, $sharedFolderId)->getId()); + } + + /** + * Sharing a subfolder with a circle must give a member who already has its parent folder + * a shared folder of the subfolder, so they keep it when the parent is unshared. + */ + public function testCircleShareOfSubfolderSurvivesUnsharingParentFolder(): void { + $memberId = $this->createUser('circle_share_nested_folders_member'); + $circle = $this->createCircle('nested_folders'); + $this->addUserToCircle($circle, $memberId); + $parentFolder = $this->createFolder(); + $subFolder = $this->createFolder(); + $this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $subFolder->getId(), $parentFolder->getId()); + $parentShare = $this->folders->createShare($parentFolder->getId(), $memberId, IShare::TYPE_USER); + $circleShare = $this->folders->createShare($subFolder->getId(), $circle->getSingleId(), IShare::TYPE_CIRCLE); + $this->assertHasSharedFolder($circleShare, $memberId); + + $this->folders->deleteShare($parentShare->getId()); + + $this->assertHasSharedFolder($circleShare, $memberId); + } + + public function testDeletedUserLosesFolderSharedWithCircle(): void { + $memberId = $this->createUser('circle_share_deleted_member'); + $remainingId = $this->createUser('circle_share_remaining_member'); + $circle = $this->createCircle('deleted_member'); + $this->addUserToCircle($circle, $memberId); + $this->addUserToCircle($circle, $remainingId); + $share = $this->shareWithCircle($circle); + $this->assertHasSharedFolder($share, $memberId); + + $this->userManager->get($memberId)->delete(); + + $this->assertHasNoSharedFolder($share, $memberId); + $this->assertCount(0, $this->sharedFolderMapper->findByUser($memberId)); + // The share and the other members' shared folders are untouched + $this->shareMapper->find($share->getId()); + $this->assertHasSharedFolder($share, $remainingId); + } + + /** + * Circle shares created by earlier versions gave none of the circle's members + * the shared folder. The repair step brings them in line with the circle. + */ + public function testCircleSharesUpdateRepairStepSyncsMembers(): void { + $missingId = $this->createUser('circle_share_repair_missing_member'); + $formerId = $this->createUser('circle_share_repair_former_member'); + $circle = $this->createCircle('repair'); + $this->addUserToCircle($circle, $missingId); + $share = $this->shareWithCircle($circle); + $folder = $this->folders->findById($share->getFolderId()); + // what earlier versions left behind: a member without the folder, and a non-member with it + $sharedFolder = $this->sharedFolderMapper->findByShareAndUser($share->getId(), $missingId)[0]; + $this->treeMapper->deleteEntry(Db\TreeMapper::TYPE_SHARE, $sharedFolder->getId()); + $this->sharedFolderMapper->delete($sharedFolder); + $this->folders->addSharedFolder($share, $folder, $formerId); + $this->assertHasNoSharedFolder($share, $missingId); + $this->assertHasSharedFolder($share, $formerId); + + \OCP\Server::get(CircleSharesUpdateRepairStep::class)->run($this->createMock(IOutput::class)); + + $this->assertHasSharedFolder($share, $missingId); + $this->assertHasNoSharedFolder($share, $formerId); + $this->assertHasNoSharedFolder($share, $this->ownerId); + } + + public function testCircleSharesUpdateRepairStepDeletesSharesOfMissingCircles(): void { + $memberId = $this->createUser('circle_share_repair_missing_circle_member'); + $folder = $this->createFolder(); + // A share of a circle that was destroyed while earlier versions didn't notice + $share = new Db\Share(); + $share->setFolderId($folder->getId()); + $share->setOwner($this->ownerId); + $share->setParticipant('nonexistentcircle' . uniqid()); + $share->setType(IShare::TYPE_CIRCLE); + $share->setCanWrite(false); + $share->setCanShare(false); + $this->shareMapper->insert($share); + $this->folders->addSharedFolder($share, $folder, $memberId); + $this->assertHasSharedFolder($share, $memberId); + + \OCP\Server::get(CircleSharesUpdateRepairStep::class)->run($this->createMock(IOutput::class)); + + try { + $this->shareMapper->find($share->getId()); + $this->fail('Share of a missing circle should have been deleted'); + } catch (DoesNotExistException $e) { + // expected + } + $this->assertCount(0, $this->sharedFolderMapper->findByUser($memberId)); + } +} diff --git a/tests/ShareTest.php b/tests/ShareTest.php new file mode 100644 index 0000000000..947de21475 --- /dev/null +++ b/tests/ShareTest.php @@ -0,0 +1,31 @@ +setFolderId(1); + $share->setOwner('share_test_owner'); + $share->setParticipant($participant); + $share->setType($type); + $share->setCanWrite(false); + $share->setCanShare(false); + + $this->assertEquals($participant, $share->toArray()['participantDisplayName']); + } +} diff --git a/tests/SharesRepairStepsTest.php b/tests/SharesRepairStepsTest.php new file mode 100644 index 0000000000..ca0aa77b85 --- /dev/null +++ b/tests/SharesRepairStepsTest.php @@ -0,0 +1,225 @@ +cleanUp(); + + $this->userManager = \OCP\Server::get(IUserManager::class); + $this->groupManager = \OCP\Server::get(IGroupManager::class); + $this->db = \OCP\Server::get(IDBConnection::class); + $this->folderMapper = \OCP\Server::get(Db\FolderMapper::class); + $this->treeMapper = \OCP\Server::get(Db\TreeMapper::class); + $this->sharedFolderMapper = \OCP\Server::get(Db\SharedFolderMapper::class); + $this->shareMapper = \OCP\Server::get(Db\ShareMapper::class); + $this->folders = \OCP\Server::get(FolderService::class); + } + + private function createUser(string $uid): string { + if (!$this->userManager->userExists($uid)) { + $this->userManager->createUser($uid, 'password'); + } + return $this->userManager->get($uid)->getUID(); + } + + private function createFolder(string $userId): Db\Folder { + $folder = new Db\Folder(); + $folder->setTitle('shared'); + $folder->setUserId($userId); + $this->folderMapper->insert($folder); + $this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $folder->getId(), $this->folderMapper->findRootFolder($userId)->getId()); + return $folder; + } + + private function countRows(string $table, string $column, int $id, ?string $type = null): int { + $qb = $this->db->getQueryBuilder(); + $qb->select($column) + ->from($table) + ->where($qb->expr()->eq($column, $qb->createPositionalParameter($id))); + if ($type !== null) { + $qb->andWhere($qb->expr()->eq('type', $qb->createPositionalParameter($type))); + } + $result = $qb->executeQuery(); + $count = count($result->fetchAll()); + $result->closeCursor(); + return $count; + } + + /** Removes a shared folder completely, as if it had never been created */ + private function removeSharedFolder(Db\SharedFolder $sharedFolder): void { + $this->treeMapper->deleteEntry(Db\TreeMapper::TYPE_SHARE, $sharedFolder->getId()); + $this->sharedFolderMapper->delete($sharedFolder); + } + + /** + * Earlier versions only deleted the share row when a group was deleted, leaving + * the members' shared folders, their tree entries and their links behind. + */ + public function testOrphanedSharesRepairStepRemovesSharedFoldersWithoutShare(): void { + $ownerId = $this->createUser('repair_orphan_owner'); + $memberId = $this->createUser('repair_orphan_member'); + $group = $this->groupManager->createGroup('repair_orphan'); + $group->addUser($this->userManager->get($memberId)); + try { + $folder = $this->createFolder($ownerId); + $share = $this->folders->createShare($folder->getId(), $group->getGID(), IShare::TYPE_GROUP); + $sharedFolderId = $this->sharedFolderMapper->findByShareAndUser($share->getId(), $memberId)[0]->getId(); + // what earlier versions did on group deletion + $this->shareMapper->delete($share); + + \OCP\Server::get(OrphanedSharesRepairStep::class)->run($this->createMock(IOutput::class)); + + $this->assertCount(0, $this->sharedFolderMapper->findByUser($memberId)); + $this->assertEquals(0, $this->countRows('bookmarks_tree', 'id', $sharedFolderId, Db\TreeMapper::TYPE_SHARE)); + $this->assertEquals(0, $this->countRows('bookmarks_shared_to_shares', 'shared_folder_id', $sharedFolderId)); + } finally { + $group->delete(); + } + } + + public function testOrphanedSharesRepairStepRemovesSharesOfMissingFolders(): void { + $ownerId = $this->createUser('repair_missing_folder_owner'); + $memberId = $this->createUser('repair_missing_folder_member'); + $folder = $this->createFolder($ownerId); + $share = $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER); + $sharedFolderId = $this->sharedFolderMapper->findByShareAndUser($share->getId(), $memberId)[0]->getId(); + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_folders') + ->where($qb->expr()->eq('id', $qb->createPositionalParameter($folder->getId()))) + ->executeStatement(); + + \OCP\Server::get(OrphanedSharesRepairStep::class)->run($this->createMock(IOutput::class)); + + $this->assertEquals(0, $this->countRows('bookmarks_shares', 'id', $share->getId())); + $this->assertCount(0, $this->sharedFolderMapper->findByUser($memberId)); + $this->assertEquals(0, $this->countRows('bookmarks_tree', 'id', $sharedFolderId, Db\TreeMapper::TYPE_SHARE)); + $this->assertEquals(0, $this->countRows('bookmarks_shared_to_shares', 'shared_folder_id', $sharedFolderId)); + } + + public function testOrphanedSharesRepairStepRemovesEntriesOfMissingSharedFolders(): void { + $ownerId = $this->createUser('repair_missing_sf_owner'); + $memberId = $this->createUser('repair_missing_sf_member'); + $folder = $this->createFolder($ownerId); + $share = $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER); + $sharedFolderId = $this->sharedFolderMapper->findByShareAndUser($share->getId(), $memberId)[0]->getId(); + $qb = $this->db->getQueryBuilder(); + $qb->delete('bookmarks_shared_folders') + ->where($qb->expr()->eq('id', $qb->createPositionalParameter($sharedFolderId))) + ->executeStatement(); + + \OCP\Server::get(OrphanedSharesRepairStep::class)->run($this->createMock(IOutput::class)); + + $this->assertEquals(0, $this->countRows('bookmarks_tree', 'id', $sharedFolderId, Db\TreeMapper::TYPE_SHARE)); + $this->assertEquals(0, $this->countRows('bookmarks_shared_to_shares', 'shared_folder_id', $sharedFolderId)); + // the share itself is still valid + $this->shareMapper->find($share->getId()); + } + + public function testGroupSharesUpdateRepairStepAddsMissingMembersWithoutDuplicates(): void { + $ownerId = $this->createUser('repair_group_owner'); + $missingId = $this->createUser('repair_group_missing_member'); + $coveredId = $this->createUser('repair_group_covered_member'); + $firstGroup = $this->groupManager->createGroup('repair_group_first'); + $secondGroup = $this->groupManager->createGroup('repair_group_second'); + $firstGroup->addUser($this->userManager->get($missingId)); + $firstGroup->addUser($this->userManager->get($coveredId)); + $secondGroup->addUser($this->userManager->get($coveredId)); + try { + $folder = $this->createFolder($ownerId); + $firstShare = $this->folders->createShare($folder->getId(), $firstGroup->getGID(), IShare::TYPE_GROUP); + $secondShare = $this->folders->createShare($folder->getId(), $secondGroup->getGID(), IShare::TYPE_GROUP); + // The covered member has a single shared folder, which is part of both shares + $this->assertCount(1, $this->sharedFolderMapper->findByUser($coveredId)); + // What earlier versions did: the shared folder is only part of the first share + $coveredSharedFolder = $this->sharedFolderMapper->findByShareAndUser($secondShare->getId(), $coveredId)[0]; + $this->sharedFolderMapper->unmount($coveredSharedFolder->getId(), $secondShare->getId()); + $this->removeSharedFolder($this->sharedFolderMapper->findByShareAndUser($firstShare->getId(), $missingId)[0]); + $this->assertCount(0, $this->sharedFolderMapper->findByUser($missingId)); + + \OCP\Server::get(GroupSharesUpdateRepairStep::class)->run($this->createMock(IOutput::class)); + + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($firstShare->getId(), $missingId)); + $this->assertCount(1, $this->sharedFolderMapper->findByUser($coveredId)); + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($secondShare->getId(), $coveredId)); + } finally { + $firstGroup->delete(); + $secondGroup->delete(); + } + } + + /** + * A member who has the parent folder of a group share's folder through another share + * must still be given a shared folder of the subfolder, as it can be unshared independently. + */ + public function testGroupSharesUpdateRepairStepAddsSubfolderForMemberWithParentFolder(): void { + $ownerId = $this->createUser('repair_subfolder_owner'); + $memberId = $this->createUser('repair_subfolder_member'); + $group = $this->groupManager->createGroup('repair_subfolder'); + $group->addUser($this->userManager->get($memberId)); + try { + $parentFolder = $this->createFolder($ownerId); + $subFolder = $this->createFolder($ownerId); + $this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $subFolder->getId(), $parentFolder->getId()); + $this->folders->createShare($parentFolder->getId(), $memberId, IShare::TYPE_USER); + $groupShare = $this->folders->createShare($subFolder->getId(), $group->getGID(), IShare::TYPE_GROUP); + $this->removeSharedFolder($this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId)[0]); + + \OCP\Server::get(GroupSharesUpdateRepairStep::class)->run($this->createMock(IOutput::class)); + + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId)); + } finally { + $group->delete(); + } + } + + /** + * Older versions could give a user several shared folders of the same folder, one per share. + * The kept shared folder must become part of all of these shares. + */ + public function testDeduplicateSharedFoldersRepairStepKeepsAllShares(): void { + $ownerId = $this->createUser('repair_dedup_owner'); + $memberId = $this->createUser('repair_dedup_member'); + $firstGroup = $this->groupManager->createGroup('repair_dedup_first'); + $secondGroup = $this->groupManager->createGroup('repair_dedup_second'); + try { + $folder = $this->createFolder($ownerId); + $firstShare = $this->folders->createShare($folder->getId(), $firstGroup->getGID(), IShare::TYPE_GROUP); + $secondShare = $this->folders->createShare($folder->getId(), $secondGroup->getGID(), IShare::TYPE_GROUP); + $this->folders->addSharedFolder($firstShare, $folder, $memberId); + $this->folders->addSharedFolder($secondShare, $folder, $memberId); + $this->assertCount(2, $this->sharedFolderMapper->findByUser($memberId)); + + \OCP\Server::get(DeduplicateSharedFoldersRepairStep::class)->run($this->createMock(IOutput::class)); + + $sharedFolders = $this->sharedFolderMapper->findByUser($memberId); + $this->assertCount(1, $sharedFolders); + $this->assertEquals($sharedFolders[0]->getId(), $this->sharedFolderMapper->findByShareAndUser($firstShare->getId(), $memberId)[0]->getId()); + $this->assertEquals($sharedFolders[0]->getId(), $this->sharedFolderMapper->findByShareAndUser($secondShare->getId(), $memberId)[0]->getId()); + } finally { + $firstGroup->delete(); + $secondGroup->delete(); + } + } +} diff --git a/tests/UsersGroupsCirclesListenerTest.php b/tests/UsersGroupsCirclesListenerTest.php new file mode 100644 index 0000000000..8dc31e01be --- /dev/null +++ b/tests/UsersGroupsCirclesListenerTest.php @@ -0,0 +1,506 @@ +cleanUp(); + + $this->userManager = \OCP\Server::get(IUserManager::class); + $this->groupManager = \OCP\Server::get(IGroupManager::class); + $this->folderMapper = \OCP\Server::get(Db\FolderMapper::class); + $this->treeMapper = \OCP\Server::get(Db\TreeMapper::class); + $this->sharedFolderMapper = \OCP\Server::get(Db\SharedFolderMapper::class); + $this->shareMapper = \OCP\Server::get(Db\ShareMapper::class); + $this->folders = \OCP\Server::get(FolderService::class); + } + + private function createUser(string $uid): string { + if (!$this->userManager->userExists($uid)) { + $this->userManager->createUser($uid, 'password'); + } + return $this->userManager->get($uid)->getUID(); + } + + private function shareTreeRowExists(int $sharedFolderId): bool { + $qb = \OCP\Server::get(IDBConnection::class)->getQueryBuilder(); + $qb->select('id') + ->from('bookmarks_tree') + ->where($qb->expr()->eq('id', $qb->createPositionalParameter($sharedFolderId))) + ->andWhere($qb->expr()->eq('type', $qb->createPositionalParameter(Db\TreeMapper::TYPE_SHARE))); + $result = $qb->executeQuery(); + $exists = $result->fetchOne() !== false; + $result->closeCursor(); + return $exists; + } + + /** + * Creates an owner and two members of a fresh group, and shares one of the + * owner's folders with that group. + * + * @return array{0: IGroup, 1: Db\Share, 2: string, 3: string} [group, share, firstMemberId, secondMemberId] + */ + private function createGroupShare(string $suffix): array { + $ownerId = $this->createUser('group_share_owner_' . $suffix); + $firstMemberId = $this->createUser('group_share_member1_' . $suffix); + $secondMemberId = $this->createUser('group_share_member2_' . $suffix); + + $group = $this->groupManager->createGroup('group_share_' . $suffix); + $group->addUser($this->userManager->get($firstMemberId)); + $group->addUser($this->userManager->get($secondMemberId)); + + $folder = new Db\Folder(); + $folder->setTitle('shared with group'); + $folder->setUserId($ownerId); + $this->folderMapper->insert($folder); + $this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $folder->getId(), $this->folderMapper->findRootFolder($ownerId)->getId()); + + $share = $this->folders->createShare($folder->getId(), $group->getGID(), IShare::TYPE_GROUP); + + return [$group, $share, $firstMemberId, $secondMemberId]; + } + + /** + * Deleting a user who received a folder through a group share must remove + * their SharedFolder and its tree row, while leaving the share and the other + * group members' SharedFolders intact. + */ + public function testDeletedGroupMemberLosesSharedFolder(): void { + [$group, $share, $deletedId, $remainingId] = $this->createGroupShare('userdel'); + + $deletedSharedFolders = $this->sharedFolderMapper->findByShareAndUser($share->getId(), $deletedId); + $this->assertCount(1, $deletedSharedFolders); + $deletedSharedFolderId = $deletedSharedFolders[0]->getId(); + $this->assertTrue($this->shareTreeRowExists($deletedSharedFolderId)); + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($share->getId(), $remainingId)); + + try { + $this->userManager->get($deletedId)->delete(); + + $this->assertCount(0, $this->sharedFolderMapper->findByShareAndUser($share->getId(), $deletedId)); + $this->assertFalse($this->shareTreeRowExists($deletedSharedFolderId)); + + // The share itself and the other member's SharedFolder are untouched + $this->shareMapper->find($share->getId()); + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($share->getId(), $remainingId)); + } finally { + $group->delete(); + } + } + + /** + * Deleting a group that a folder was shared with must remove the share and + * every member's SharedFolder and tree row. + */ + public function testDeletedGroupRemovesShareAndSharedFolders(): void { + [$group, $share, $firstMemberId, $secondMemberId] = $this->createGroupShare('groupdel'); + + $sharedFolderIds = []; + foreach ([$firstMemberId, $secondMemberId] as $memberId) { + $sharedFolders = $this->sharedFolderMapper->findByShareAndUser($share->getId(), $memberId); + $this->assertCount(1, $sharedFolders); + $this->assertTrue($this->shareTreeRowExists($sharedFolders[0]->getId())); + $sharedFolderIds[] = $sharedFolders[0]->getId(); + } + + $group->delete(); + + try { + $this->shareMapper->find($share->getId()); + $this->fail('Share should have been deleted together with the group'); + } catch (DoesNotExistException $e) { + // expected + } + $this->assertCount(0, $this->sharedFolderMapper->findByShare($share->getId())); + foreach ([$firstMemberId, $secondMemberId] as $memberId) { + $this->assertCount(0, $this->sharedFolderMapper->findByShareAndUser($share->getId(), $memberId)); + } + foreach ($sharedFolderIds as $sharedFolderId) { + $this->assertFalse($this->shareTreeRowExists($sharedFolderId)); + } + } + + /** + * Deleting a user that a folder was shared with directly must remove that + * share along with their SharedFolder and tree row, while leaving a share of + * the same folder with another user intact. + */ + public function testDeletedUserRemovesDirectShareAndSharedFolder(): void { + $ownerId = $this->createUser('user_share_owner'); + $deletedId = $this->createUser('user_share_deleted_recipient'); + $remainingId = $this->createUser('user_share_remaining_recipient'); + + $folder = new Db\Folder(); + $folder->setTitle('shared with users'); + $folder->setUserId($ownerId); + $this->folderMapper->insert($folder); + $this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $folder->getId(), $this->folderMapper->findRootFolder($ownerId)->getId()); + + $deletedShare = $this->folders->createShare($folder->getId(), $deletedId, IShare::TYPE_USER); + $remainingShare = $this->folders->createShare($folder->getId(), $remainingId, IShare::TYPE_USER); + + $deletedSharedFolders = $this->sharedFolderMapper->findByShareAndUser($deletedShare->getId(), $deletedId); + $this->assertCount(1, $deletedSharedFolders); + $deletedSharedFolderId = $deletedSharedFolders[0]->getId(); + $this->assertTrue($this->shareTreeRowExists($deletedSharedFolderId)); + + $this->userManager->get($deletedId)->delete(); + + try { + $this->shareMapper->find($deletedShare->getId()); + $this->fail('Share should have been deleted together with the recipient'); + } catch (DoesNotExistException $e) { + // expected + } + $this->assertCount(0, $this->sharedFolderMapper->findByShare($deletedShare->getId())); + $this->assertFalse($this->shareTreeRowExists($deletedSharedFolderId)); + + // The other recipient's share is untouched + $this->shareMapper->find($remainingShare->getId()); + $remainingSharedFolders = $this->sharedFolderMapper->findByShareAndUser($remainingShare->getId(), $remainingId); + $this->assertCount(1, $remainingSharedFolders); + $this->assertTrue($this->shareTreeRowExists($remainingSharedFolders[0]->getId())); + } + + private function createFolder(string $ownerId): Db\Folder { + $folder = new Db\Folder(); + $folder->setTitle('shared twice'); + $folder->setUserId($ownerId); + $this->folderMapper->insert($folder); + $this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $folder->getId(), $this->folderMapper->findRootFolder($ownerId)->getId()); + return $folder; + } + + /** + * A user who leaves a group but still has access to the folder through a share + * with another group must keep their shared folder, which is part of both shares. + */ + public function testUserRemovedFromGroupKeepsFolderSharedWithOtherGroup(): void { + $ownerId = $this->createUser('two_groups_owner'); + $memberId = $this->createUser('two_groups_member'); + $firstGroup = $this->groupManager->createGroup('two_groups_first'); + $secondGroup = $this->groupManager->createGroup('two_groups_second'); + $firstGroup->addUser($this->userManager->get($memberId)); + $secondGroup->addUser($this->userManager->get($memberId)); + try { + $folder = $this->createFolder($ownerId); + $firstShare = $this->folders->createShare($folder->getId(), $firstGroup->getGID(), IShare::TYPE_GROUP); + $secondShare = $this->folders->createShare($folder->getId(), $secondGroup->getGID(), IShare::TYPE_GROUP); + // The member only gets one shared folder, which is part of both shares + $sharedFolders = $this->sharedFolderMapper->findByShareAndUser($firstShare->getId(), $memberId); + $this->assertCount(1, $sharedFolders); + $sharedFolderId = $sharedFolders[0]->getId(); + $this->assertEquals($sharedFolderId, $this->sharedFolderMapper->findByShareAndUser($secondShare->getId(), $memberId)[0]->getId()); + $this->assertCount(1, $this->sharedFolderMapper->findByUser($memberId)); + + $firstGroup->removeUser($this->userManager->get($memberId)); + + $this->assertCount(0, $this->sharedFolderMapper->findByShareAndUser($firstShare->getId(), $memberId)); + $sharedFolders = $this->sharedFolderMapper->findByShareAndUser($secondShare->getId(), $memberId); + $this->assertCount(1, $sharedFolders); + // It's the same shared folder, still in place + $this->assertEquals($sharedFolderId, $sharedFolders[0]->getId()); + $this->assertTrue($this->shareTreeRowExists($sharedFolderId)); + + $secondGroup->removeUser($this->userManager->get($memberId)); + + $this->assertCount(0, $this->sharedFolderMapper->findByShareAndUser($secondShare->getId(), $memberId)); + $this->assertFalse($this->shareTreeRowExists($sharedFolderId)); + } finally { + $firstGroup->delete(); + $secondGroup->delete(); + } + } + + /** + * A user who leaves a group but also has the folder shared with them directly + * must keep exactly one shared folder. + */ + public function testUserRemovedFromGroupKeepsFolderSharedDirectly(): void { + $ownerId = $this->createUser('group_and_user_owner'); + $memberId = $this->createUser('group_and_user_member'); + $group = $this->groupManager->createGroup('group_and_user'); + $group->addUser($this->userManager->get($memberId)); + try { + $folder = $this->createFolder($ownerId); + $groupShare = $this->folders->createShare($folder->getId(), $group->getGID(), IShare::TYPE_GROUP); + $userShare = $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER); + // The member already has the folder through the group, so the direct share doesn't add it again, + // but the existing shared folder becomes part of the direct share + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId)); + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($userShare->getId(), $memberId)); + $this->assertCount(1, $this->sharedFolderMapper->findByUser($memberId)); + + $group->removeUser($this->userManager->get($memberId)); + + $this->assertCount(0, $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId)); + $sharedFolders = $this->sharedFolderMapper->findByShareAndUser($userShare->getId(), $memberId); + $this->assertCount(1, $sharedFolders); + $this->assertTrue($this->shareTreeRowExists($sharedFolders[0]->getId())); + } finally { + $group->delete(); + } + } + + /** + * A user who removed a folder shared through a group must get it back + * when the folder is then shared with them directly. + */ + public function testDirectShareRestoresFolderRemovedFromGroupShare(): void { + $ownerId = $this->createUser('removed_then_direct_owner'); + $memberId = $this->createUser('removed_then_direct_member'); + $group = $this->groupManager->createGroup('removed_then_direct'); + $group->addUser($this->userManager->get($memberId)); + try { + $folder = $this->createFolder($ownerId); + $groupShare = $this->folders->createShare($folder->getId(), $group->getGID(), IShare::TYPE_GROUP); + $sharedFolderId = $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId)[0]->getId(); + + $this->folders->deleteSharedFolderOrFolder($memberId, $folder->getId(), true); + $this->assertFalse($this->shareTreeRowExists($sharedFolderId)); + + $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER); + + $sharedFolders = $this->sharedFolderMapper->findByUser($memberId); + $this->assertCount(1, $sharedFolders); + $this->assertEquals($sharedFolderId, $sharedFolders[0]->getId()); + $this->assertTrue($this->shareTreeRowExists($sharedFolderId)); + $rootFolder = $this->folderMapper->findRootFolder($memberId); + $this->assertEquals($rootFolder->getId(), $this->treeMapper->findParentOf(Db\TreeMapper::TYPE_SHARE, $sharedFolderId)->getId()); + } finally { + $group->delete(); + } + } + + /** + * A user who removed a folder shared with them directly must get it back + * when the folder is then shared with a group they are a member of. + */ + public function testGroupShareRestoresFolderRemovedFromDirectShare(): void { + $ownerId = $this->createUser('removed_then_group_owner'); + $memberId = $this->createUser('removed_then_group_member'); + $group = $this->groupManager->createGroup('removed_then_group'); + $group->addUser($this->userManager->get($memberId)); + try { + $folder = $this->createFolder($ownerId); + $userShare = $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER); + $sharedFolderId = $this->sharedFolderMapper->findByShareAndUser($userShare->getId(), $memberId)[0]->getId(); + + $this->folders->deleteSharedFolderOrFolder($memberId, $folder->getId(), true); + $this->assertFalse($this->shareTreeRowExists($sharedFolderId)); + + $this->folders->createShare($folder->getId(), $group->getGID(), IShare::TYPE_GROUP); + + $sharedFolders = $this->sharedFolderMapper->findByUser($memberId); + $this->assertCount(1, $sharedFolders); + $this->assertEquals($sharedFolderId, $sharedFolders[0]->getId()); + $rootFolder = $this->folderMapper->findRootFolder($memberId); + $this->assertEquals($rootFolder->getId(), $this->treeMapper->findParentOf(Db\TreeMapper::TYPE_SHARE, $sharedFolderId)->getId()); + } finally { + $group->delete(); + } + } + + /** + * Deleting a share must keep the shared folder of participants who still have + * access to the folder through another share, and remove everyone else's. + */ + public function testDeletedShareKeepsFolderForUsersCoveredByOtherShare(): void { + $ownerId = $this->createUser('deleted_share_owner'); + $coveredId = $this->createUser('deleted_share_covered_member'); + $otherId = $this->createUser('deleted_share_other_member'); + $group = $this->groupManager->createGroup('deleted_share'); + $group->addUser($this->userManager->get($coveredId)); + $group->addUser($this->userManager->get($otherId)); + try { + $folder = $this->createFolder($ownerId); + $groupShare = $this->folders->createShare($folder->getId(), $group->getGID(), IShare::TYPE_GROUP); + $userShare = $this->folders->createShare($folder->getId(), $coveredId, IShare::TYPE_USER); + $coveredSharedFolderId = $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $coveredId)[0]->getId(); + $otherSharedFolderId = $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $otherId)[0]->getId(); + + $this->folders->deleteShare($groupShare->getId()); + + try { + $this->shareMapper->find($groupShare->getId()); + $this->fail('Share should have been deleted'); + } catch (DoesNotExistException $e) { + // expected + } + $this->assertCount(0, $this->sharedFolderMapper->findByShare($groupShare->getId())); + // The covered member keeps the same shared folder, which is still part of the direct share + $sharedFolders = $this->sharedFolderMapper->findByShareAndUser($userShare->getId(), $coveredId); + $this->assertCount(1, $sharedFolders); + $this->assertEquals($coveredSharedFolderId, $sharedFolders[0]->getId()); + $this->assertTrue($this->shareTreeRowExists($coveredSharedFolderId)); + // The other member loses it + $this->assertCount(0, $this->sharedFolderMapper->findByUser($otherId)); + $this->assertFalse($this->shareTreeRowExists($otherSharedFolderId)); + } finally { + $group->delete(); + } + } + + /** + * A user who already has a folder through a direct share and then joins a group + * the folder is also shared with must not get a second shared folder. + */ + public function testUserAddedToGroupKeepsSingleSharedFolderOfDirectShare(): void { + $ownerId = $this->createUser('direct_then_group_owner'); + $memberId = $this->createUser('direct_then_group_member'); + $group = $this->groupManager->createGroup('direct_then_group'); + try { + $folder = $this->createFolder($ownerId); + $userShare = $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER); + $groupShare = $this->folders->createShare($folder->getId(), $group->getGID(), IShare::TYPE_GROUP); + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($userShare->getId(), $memberId)); + + $group->addUser($this->userManager->get($memberId)); + + // The existing shared folder becomes part of the group share + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId)); + $this->assertCount(1, $this->sharedFolderMapper->findByUser($memberId)); + // Looking up the user's shared folder of this folder still works + $this->sharedFolderMapper->findByFolderAndUser($folder->getId(), $memberId); + } finally { + $group->delete(); + } + } + + /** + * A user who joins a group must not get a folder shared with that group if it contains + * a folder they shared themselves. Would cause a loop. + */ + public function testUserAddedToGroupDoesNotGetFolderContainingTheirOwnShare(): void { + $ownerId = $this->createUser('loop_group_owner'); + $memberId = $this->createUser('loop_group_member'); + $group = $this->groupManager->createGroup('loop_group'); + try { + // The member shares one of their folders with the owner, who puts it into a folder of their own + $memberFolder = $this->createFolder($memberId); + $memberShare = $this->folders->createShare($memberFolder->getId(), $ownerId, IShare::TYPE_USER); + $ownerSharedFolder = $this->sharedFolderMapper->findByShareAndUser($memberShare->getId(), $ownerId)[0]; + $folder = $this->createFolder($ownerId); + $this->treeMapper->move(Db\TreeMapper::TYPE_SHARE, $ownerSharedFolder->getId(), $folder->getId()); + $groupShare = $this->folders->createShare($folder->getId(), $group->getGID(), IShare::TYPE_GROUP); + + $group->addUser($this->userManager->get($memberId)); + + $this->assertCount(0, $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId)); + $this->assertCount(0, $this->sharedFolderMapper->findByUser($memberId)); + } finally { + $group->delete(); + } + } + + /** + * Sharing a subfolder directly with a user who already has its parent folder must give + * them a shared folder of the subfolder, so they keep it when the parent is unshared. + */ + public function testDirectShareOfSubfolderSurvivesUnsharingParentFolder(): void { + $ownerId = $this->createUser('nested_shares_owner'); + $memberId = $this->createUser('nested_shares_member'); + + $parentFolder = $this->createFolder($ownerId); + $subFolder = new Db\Folder(); + $subFolder->setTitle('subfolder'); + $subFolder->setUserId($ownerId); + $this->folderMapper->insert($subFolder); + $this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $subFolder->getId(), $parentFolder->getId()); + + $parentShare = $this->folders->createShare($parentFolder->getId(), $memberId, IShare::TYPE_USER); + $subShare = $this->folders->createShare($subFolder->getId(), $memberId, IShare::TYPE_USER, true); + $subSharedFolders = $this->sharedFolderMapper->findByShareAndUser($subShare->getId(), $memberId); + $this->assertCount(1, $subSharedFolders); + + $this->folders->deleteShare($parentShare->getId()); + + $this->assertCount(0, $this->sharedFolderMapper->findByShareAndUser($parentShare->getId(), $memberId)); + $sharedFolders = $this->sharedFolderMapper->findByShareAndUser($subShare->getId(), $memberId); + $this->assertCount(1, $sharedFolders); + $this->assertEquals($subSharedFolders[0]->getId(), $sharedFolders[0]->getId()); + $this->assertTrue($this->shareTreeRowExists($sharedFolders[0]->getId())); + } + + /** + * Sharing a folder directly with a user who has several shared folders of it, as older + * versions could create, must work and not add yet another one. + */ + public function testDirectShareWithUserWhoHasDuplicateSharedFolders(): void { + $ownerId = $this->createUser('duplicates_owner'); + $memberId = $this->createUser('duplicates_member'); + $firstGroup = $this->groupManager->createGroup('duplicates_first'); + $secondGroup = $this->groupManager->createGroup('duplicates_second'); + try { + $folder = $this->createFolder($ownerId); + $firstShare = $this->folders->createShare($folder->getId(), $firstGroup->getGID(), IShare::TYPE_GROUP); + $secondShare = $this->folders->createShare($folder->getId(), $secondGroup->getGID(), IShare::TYPE_GROUP); + $this->folders->addSharedFolder($firstShare, $folder, $memberId); + $this->folders->addSharedFolder($secondShare, $folder, $memberId); + $this->assertCount(2, $this->sharedFolderMapper->findByUser($memberId)); + + $userShare = $this->folders->createShare($folder->getId(), $memberId, IShare::TYPE_USER); + + $this->shareMapper->find($userShare->getId()); + $this->assertCount(0, $this->sharedFolderMapper->findByShareAndUser($userShare->getId(), $memberId)); + $this->assertCount(2, $this->sharedFolderMapper->findByUser($memberId)); + } finally { + $firstGroup->delete(); + $secondGroup->delete(); + } + } + + /** + * Sharing a subfolder with a group must give a member who already has its parent folder + * a shared folder of the subfolder, so they keep it when the parent is unshared. + * The same goes for a user who joins the group afterwards. + */ + public function testGroupShareOfSubfolderSurvivesUnsharingParentFolder(): void { + $ownerId = $this->createUser('nested_group_share_owner'); + $memberId = $this->createUser('nested_group_share_member'); + $joinerId = $this->createUser('nested_group_share_joiner'); + $group = $this->groupManager->createGroup('nested_group_share'); + $group->addUser($this->userManager->get($memberId)); + try { + $parentFolder = $this->createFolder($ownerId); + $subFolder = $this->createFolder($ownerId); + $this->treeMapper->move(Db\TreeMapper::TYPE_FOLDER, $subFolder->getId(), $parentFolder->getId()); + + $memberParentShare = $this->folders->createShare($parentFolder->getId(), $memberId, IShare::TYPE_USER); + $joinerParentShare = $this->folders->createShare($parentFolder->getId(), $joinerId, IShare::TYPE_USER); + $groupShare = $this->folders->createShare($subFolder->getId(), $group->getGID(), IShare::TYPE_GROUP); + $group->addUser($this->userManager->get($joinerId)); + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $memberId)); + $this->assertCount(1, $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $joinerId)); + + $this->folders->deleteShare($memberParentShare->getId()); + $this->folders->deleteShare($joinerParentShare->getId()); + + foreach ([$memberId, $joinerId] as $userId) { + $sharedFolders = $this->sharedFolderMapper->findByShareAndUser($groupShare->getId(), $userId); + $this->assertCount(1, $sharedFolders); + $this->assertTrue($this->shareTreeRowExists($sharedFolders[0]->getId())); + } + } finally { + $group->delete(); + } + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 8c617f8972..e3431ef998 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -17,4 +17,8 @@ require_once __DIR__ . '/../../../lib/base.php'; require_once __DIR__ . '/../../../tests/autoload.php'; -Server::get(IAppManager::class)->loadApp('bookmarks'); +$appManager = Server::get(IAppManager::class); +if ($appManager->isEnabledForAnyone('circles')) { + $appManager->loadApp('circles'); +} +$appManager->loadApp('bookmarks'); diff --git a/tests/stub.phpstub b/tests/stub.phpstub index acad5c60ad..b261d96f48 100644 --- a/tests/stub.phpstub +++ b/tests/stub.phpstub @@ -36,3 +36,44 @@ namespace OC\Files\Cache { public function whereParentInParameter(string $parameter):CacheQueryBuilder; } } + +namespace OCA\Circles\Model { + class Circle { + public function getSingleId(): string; + } + + class Membership { + public function getSingleId(): string; + public function getCircleId(): string; + } +} + +namespace OCA\Circles\Model\Probes { + class CircleProbe { + public function mustBeMember(bool $must = true): self; + } +} + +namespace OCA\Circles\Events { + class CircleGenericEvent extends \OCP\EventDispatcher\Event { + public function getCircle(): \OCA\Circles\Model\Circle; + } + + class DestroyingCircleEvent extends CircleGenericEvent { + } + + class MembershipsCreatedEvent extends \OCP\EventDispatcher\Event { + /** @return list<\OCA\Circles\Model\Membership> */ + public function getMemberships(): array; + } + + class MembershipsRemovedEvent extends \OCP\EventDispatcher\Event { + /** @return list<\OCA\Circles\Model\Membership> */ + public function getMemberships(): array; + } +} + +namespace OCA\Circles\Exceptions { + class FederatedUserNotFoundException extends \Exception { + } +}