From c5002e809a7fc137a968168458f9d00f2337a20c Mon Sep 17 00:00:00 2001 From: Sujith H Date: Tue, 17 Apr 2018 22:36:08 +0530 Subject: [PATCH] Update public link share in trasnsfer ownership command The public link share wasn't updated in the command. This resulted in failure, when the public links were accessed after the files were transferred. This change helps to update the share for public links. And hence access to the links won't cause any failure. Signed-off-by: Sujith H --- apps/files/lib/Command/TransferOwnership.php | 30 +----- lib/private/Share20/Manager.php | 91 +++++++++++++++++ lib/public/Share/IManager.php | 10 ++ tests/lib/Share20/ManagerTest.php | 100 +++++++++++++++++++ 4 files changed, 202 insertions(+), 29 deletions(-) diff --git a/apps/files/lib/Command/TransferOwnership.php b/apps/files/lib/Command/TransferOwnership.php index 15772f1eb2cd..f53fc5ce1e4c 100644 --- a/apps/files/lib/Command/TransferOwnership.php +++ b/apps/files/lib/Command/TransferOwnership.php @@ -299,35 +299,7 @@ private function restoreShares(OutputInterface $output) { foreach($this->shares as $share) { try { - if ($share->getSharedWith() === $this->destinationUser) { - // Unmount the shares before deleting, so we don't try to get the storage later on. - $shareMountPoint = $this->mountManager->find('/' . $this->destinationUser . '/files' . $share->getTarget()); - if ($shareMountPoint) { - $this->mountManager->removeMount($shareMountPoint->getMountPoint()); - } - $this->shareManager->deleteShare($share); - } else { - if ($share->getShareOwner() === $this->sourceUser) { - $share->setShareOwner($this->destinationUser); - } - if ($share->getSharedBy() === $this->sourceUser) { - $share->setSharedBy($this->destinationUser); - } - /* - * If the share is already moved then updateShare would cause exception - * This can happen if the folder is shared and file(s) inside the folder - * has shares, for example public link - */ - if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) { - $sharePath = ltrim($share->getNode()->getPath(), '/'); - if (strpos($sharePath, $this->finalTarget) !== false) { - //The share is already moved - continue; - } - } - - $this->shareManager->updateShare($share); - } + $this->shareManager->transferShares($share, $this->sourceUser, $this->destinationUser, ['finalTarget' => $this->finalTarget]); } catch (\OCP\Files\NotFoundException $e) { $output->writeln('Share with id ' . $share->getId() . ' points at deleted file, skipping'); } catch (\Exception $e) { diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 36402c16b138..104235108c28 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -28,6 +28,7 @@ use OC\Cache\CappedMemoryCache; use OC\Files\Mount\MoveableMount; +use OC\Files\View; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IRootFolder; @@ -44,6 +45,7 @@ use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IProviderFactory; +use OCP\Share\IShare; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\GenericEvent; @@ -675,6 +677,95 @@ public function createShare(\OCP\Share\IShare $share) { return $share; } + /** + * Transfer shares from oldOwner to newOwner. Both old and new owners are uid + * The extraArgs array holds the information of the folder where the share + * target should be shifted to + * + * @param IShare $share + * @param string $oldOwner + * @param string $newOwner + * @param array $extraArgs + * @throws \Exception + */ + public function transferShares(IShare $share, $oldOwner, $newOwner, $extraArgs = []) { + $view = new View('/'); + //If the destination location, i.e finalTarget is not present, then + //throw an exception + if (!$view->file_exists($extraArgs['finalTarget'])) { + throw new \Exception('The target location doesn\'t exist'); + } + + if ($oldOwner === $newOwner) { + throw new \Exception('No point in transferring to same user'); + } + + /** + * If the share was already shared with new owner, then we can delete it + */ + if ($share->getSharedWith() === $newOwner) { + // Unmount the shares before deleting, so we don't try to get the storage later on. + $shareMountPoint = $this->mountManager->find('/' . $newOwner . '/files' . $share->getTarget()); + if ($shareMountPoint) { + $this->mountManager->removeMount($shareMountPoint->getMountPoint()); + } + $this->deleteShare($share); + } else { + $sharedWith = $share->getSharedWith(); + if (!isset($extraArgs['finalTarget'])) { + throw new \Exception('finalTarget is not set'); + } + + $targetFile = '/' . rtrim(basename($extraArgs['finalTarget']), '/') . '/' . ltrim(basename($share->getTarget()), '/'); + /** + * Scenario where share is made by old owner to a user different + * from new owner + */ + if (($sharedWith !== null) && ($sharedWith !== $oldOwner) && ($sharedWith !== $newOwner)) { + $sharedBy = $share->getSharedBy(); + $sharedOwner = $share->getShareOwner(); + //The origin of the share now has to be the destination user. + if (($sharedBy === $oldOwner) && ($sharedOwner === $oldOwner)) { + $share->setSharedBy($newOwner); + $share->setShareOwner($newOwner); + $share->setTarget($targetFile); + } + $this->updateShare($share); + } else { + if ($share->getShareOwner() === $oldOwner) { + $share->setShareOwner($newOwner); + } + if ($share->getSharedBy() === $oldOwner) { + $share->setSharedBy($newOwner); + } + } + /* + * If the share is already moved then updateShare would cause exception + * This can happen if the folder is shared and file(s) inside the folder + * has shares, for example public link + */ + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) { + $sharePath = ltrim($share->getNode()->getPath(), '/'); + if (strpos($sharePath, $extraArgs['finalTarget']) !== false) { + /** + * Create a new share and update the share with the + * details from old share. + */ + $token = $share->getToken(); + $share->setTarget($targetFile); + if (($share->getSharedBy() === $share->getShareOwner()) && ($share->getSharedWith() === null)) { + $share = $this->createShare($share); + } + $share->setToken($token); + } else { + throw new \Exception('The path is not found in share'); + } + } + + $this->updateShare($share); + } + } + /** * Update a share * diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 2e71f80a3f98..e23744fd19b9 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -112,6 +112,16 @@ public function getAllSharesBy($userId, $shareTypes, $nodeIDs, $reshares = false */ public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0); + /** + * Transfer shares from oldOwner to newOwner. Both old and new owners are uid + * + * @param IShare $share + * @param string $oldOwner + * @param string $newOwner + * @param array $args + * @since 10.0.9 + */ + public function transferShares(IShare $share, $oldOwner, $newOwner, $args = []); /** * Get shares shared with $userId for specified share types. diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index cca321c1edec..be438b776add 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1675,6 +1675,106 @@ public function testCanShare($expected, $sharingEnabled, $disabledForUser) { $this->assertEquals($expected, !$exception); } + public function provideTransferShareData() { + return [ + ['oldShareOwner', 'newShareOwner', '/transferred from oldShareOwner on 12345', 'myPath'], + ]; + } + + /** + * @dataProvider provideTransferShareData + */ + public function testTransferShare($oldShareOwner, $newShareOwner, $finalTarget, $file) { + $manager = $this->createManagerMock() + ->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks']) + ->getMock(); + + $shareNewOwner = $this->createMock(IUser::class); + $shareNewOwner->method('getUID')->willReturn($newShareOwner); + + $shareOwner = $this->createMock('\OCP\IUser'); + $shareOwner->method('getUID')->willReturn($oldShareOwner); + + $storage = $this->createMock('\OCP\Files\Storage'); + $path = $this->createMock('\OCP\Files\File'); + $path->method('getOwner')->willReturn($shareOwner); + $path->method('getName')->willReturn($finalTarget); + $path->method('getStorage')->willReturn($storage); + + $share = $this->manager->newShare(); + /*$share = $this->createShare( + null, + \OCP\Share::SHARE_TYPE_USER, + $path, + 'sharedWith', + 'sharedBy', + null, + \OCP\Constants::PERMISSION_ALL);*/ + //$node = $this->createMock('\OCP\Files\File'); + $path->method('getId')->willReturn(100); + $path->method('getPath')->willReturn('/'.$newShareOwner.'/files/'. $finalTarget. '/'. $file); + $share->setProviderId('foo') + ->setId('42') + ->setShareType(\OCP\Share::SHARE_TYPE_LINK) + ->setSharedBy($oldShareOwner) + ->setShareOwner($oldShareOwner) + ->setNode($path) + ->setTarget('/'.$file) + ->setPassword('pass') + ->setPermissions(15); + + $manager->expects($this->any()) + ->method('canShare') + ->with($share) + ->willReturn(true); + $manager->expects($this->any()) + ->method('generalCreateChecks') + ->with($share);; + $manager->expects($this->any()) + ->method('userCreateChecks') + ->with($share); + $manager->expects($this->any()) + ->method('pathCreateChecks') + ->with($path); + + $this->defaultProvider + ->expects($this->any()) + ->method('create') + ->with($share) + ->will($this->returnArgument(0)); + + $this->defaultProvider + ->expects($this->any()) + ->method('getShareById') + ->willReturn($share); + + $this->defaultProvider + ->expects($this->any()) + ->method('update') + ->willReturn($share); + + $this->config + ->method('getAppValue') + ->will($this->returnValueMap([ + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['core', 'shareapi_allow_public_upload', 'yes', 'yes'], + ])); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['listner'])->getMock(); + \OCP\Util::connectHook('\OC\Share', 'verifyPassword', $hookListner, 'listner'); + $hookListner->expects($this->any()) + ->method('listner') + ->will($this->returnCallback(function (array $array) { + $array['accepted'] = true; + $array['message'] = 'password accepted'; + })); + + $newShare = $manager->createShare($share); + + $finalTarget = $finalTarget . '/' . $file; + $manager->transferShares($newShare, $oldShareOwner, $newShareOwner, ['finalTarget' => $finalTarget]); + } + public function testCreateShareUser() { $manager = $this->createManagerMock() ->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks'])