diff --git a/apps/files/lib/Service/OwnershipTransferService.php b/apps/files/lib/Service/OwnershipTransferService.php index 283a49932f723..e28fbfe19bd56 100644 --- a/apps/files/lib/Service/OwnershipTransferService.php +++ b/apps/files/lib/Service/OwnershipTransferService.php @@ -176,16 +176,6 @@ public function transfer( $output ); - $destinationPath = $finalTarget . '/' . $path; - // restore the shares - $this->restoreShares( - $sourceUid, - $destinationUid, - $destinationPath, - $shares, - $output - ); - // transfer the incoming shares if ($transferIncomingShares === true) { $sourceShares = $this->collectIncomingShares( @@ -210,6 +200,16 @@ public function transfer( $move ); } + + $destinationPath = $finalTarget . '/' . $path; + // restore the shares + $this->restoreShares( + $sourceUid, + $destinationUid, + $destinationPath, + $shares, + $output + ); } private function walkFiles(View $view, $path, Closure $callBack) { diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index d965a7c2f2b66..648ffcf266f19 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -52,6 +52,7 @@ use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountManager; use OCP\Files\Node; +use OCP\Files\NotFoundException; use OCP\HintException; use OCP\IConfig; use OCP\IDateTimeZone; @@ -896,6 +897,7 @@ protected function sendMailNotification(IL10N $l, * @param IShare $share * @return IShare The share object * @throws \InvalidArgumentException + * @throws GenericShareException */ public function updateShare(IShare $share) { $expirationDateUpdated = false; @@ -1153,6 +1155,89 @@ protected function deleteChildren(IShare $share) { return $deletedShares; } + /** Promote re-shares into direct shares so that target user keeps access */ + protected function promoteReshares(IShare $share): void { + try { + $node = $share->getNode(); + } catch (NotFoundException) { + /* Skip if node not found */ + return; + } + + $userIds = []; + + if ($share->getShareType() === IShare::TYPE_USER) { + $userIds[] = $share->getSharedWith(); + } elseif ($share->getShareType() === IShare::TYPE_GROUP) { + $group = $this->groupManager->get($share->getSharedWith()); + $users = $group?->getUsers() ?? []; + + foreach ($users as $user) { + /* Skip share owner */ + if ($user->getUID() === $share->getShareOwner() || $user->getUID() === $share->getSharedBy()) { + continue; + } + $userIds[] = $user->getUID(); + } + } else { + /* We only support user and group shares */ + return; + } + + $reshareRecords = []; + $shareTypes = [ + IShare::TYPE_GROUP, + IShare::TYPE_USER, + IShare::TYPE_LINK, + IShare::TYPE_REMOTE, + IShare::TYPE_EMAIL, + ]; + + foreach ($userIds as $userId) { + foreach ($shareTypes as $shareType) { + $provider = $this->factory->getProviderForType($shareType); + if ($node instanceof Folder) { + /* We need to get all shares by this user to get subshares */ + $shares = $provider->getSharesBy($userId, $shareType, null, false, -1, 0); + + foreach ($shares as $share) { + try { + $path = $share->getNode()->getPath(); + } catch (NotFoundException) { + /* Ignore share of non-existing node */ + continue; + } + if ($node->getRelativePath($path) !== null) { + /* If relative path is not null it means the shared node is the same or in a subfolder */ + $reshareRecords[] = $share; + } + } + } else { + $shares = $provider->getSharesBy($userId, $shareType, $node, false, -1, 0); + foreach ($shares as $child) { + $reshareRecords[] = $child; + } + } + } + } + + foreach ($reshareRecords as $child) { + try { + /* Check if the share is still valid (means the resharer still has access to the file through another mean) */ + $this->generalCreateChecks($child); + } catch (GenericShareException $e) { + /* The check is invalid, promote it to a direct share from the sharer of parent share */ + $this->logger->debug('Promote reshare because of exception ' . $e->getMessage(), ['exception' => $e, 'fullId' => $child->getFullId()]); + try { + $child->setSharedBy($share->getSharedBy()); + $this->updateShare($child); + } catch (GenericShareException|\InvalidArgumentException $e) { + $this->logger->warning('Failed to promote reshare because of exception ' . $e->getMessage(), ['exception' => $e, 'fullId' => $child->getFullId()]); + } + } + } + } + /** * Delete a share * @@ -1177,6 +1262,9 @@ public function deleteShare(IShare $share) { $provider->delete($share); $this->dispatcher->dispatchTyped(new ShareDeletedEvent($share)); + + // Promote reshares of the deleted share + $this->promoteReshares($share); } diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index e352c088d514e..c25a6b02c4329 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -23,6 +23,7 @@ use DateTimeZone; use OC\Files\Mount\MoveableMount; +use OC\Files\Utils\PathHelper; use OC\KnownUser\KnownUserService; use OC\Share20\DefaultShareProvider; use OC\Share20\Exception; @@ -60,6 +61,7 @@ use OCP\Share\Events\ShareDeletedEvent; use OCP\Share\Events\ShareDeletedFromSelfEvent; use OCP\Share\Exceptions\AlreadySharedException; +use OCP\Share\Exceptions\GenericShareException; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IProviderFactory; @@ -212,6 +214,14 @@ private function createManagerMock() { ]); } + private function createFolderMock(string $folderPath): MockObject&Folder { + $folder = $this->createMock(Folder::class); + $folder->method('getPath')->willReturn($folderPath); + $folder->method('getRelativePath')->willReturnCallback( + fn (string $path): ?string => PathHelper::getRelativePath($folderPath, $path) + ); + return $folder; + } public function testDeleteNoShareId() { $this->expectException(\InvalidArgumentException::class); @@ -241,7 +251,7 @@ public function dataTestDelete() { */ public function testDelete($shareType, $sharedWith) { $manager = $this->createManagerMock() - ->setMethods(['getShareById', 'deleteChildren']) + ->setMethods(['getShareById', 'deleteChildren', 'promoteReshares']) ->getMock(); $manager->method('deleteChildren')->willReturn([]); @@ -259,6 +269,7 @@ public function testDelete($shareType, $sharedWith) { ->setTarget('myTarget'); $manager->expects($this->once())->method('deleteChildren')->with($share); + $manager->expects($this->once())->method('promoteReshares')->with($share); $this->defaultProvider ->expects($this->once()) @@ -283,7 +294,7 @@ public function testDelete($shareType, $sharedWith) { public function testDeleteLazyShare() { $manager = $this->createManagerMock() - ->setMethods(['getShareById', 'deleteChildren']) + ->setMethods(['getShareById', 'deleteChildren', 'promoteReshares']) ->getMock(); $manager->method('deleteChildren')->willReturn([]); @@ -302,6 +313,7 @@ public function testDeleteLazyShare() { $this->rootFolder->expects($this->never())->method($this->anything()); $manager->expects($this->once())->method('deleteChildren')->with($share); + $manager->expects($this->once())->method('promoteReshares')->with($share); $this->defaultProvider ->expects($this->once()) @@ -326,7 +338,7 @@ public function testDeleteLazyShare() { public function testDeleteNested() { $manager = $this->createManagerMock() - ->setMethods(['getShareById']) + ->setMethods(['getShareById', 'promoteReshares']) ->getMock(); $path = $this->createMock(File::class);