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 <sharidasan@owncloud.com>
owncloud · Apr 20, 2018 · be1d907 · be1d907
1 parent 06c30ef
commit be1d907
Show file tree

Hide file tree

Showing 4 changed files with 194 additions and 29 deletions.
diff --git 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('<error>Share with id ' . $share->getId() . ' points at deleted file, skipping</error>');
 			} catch (\Exception $e) {

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
@@ -44,6 +44,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 +676,88 @@ 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 = []) {
+		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
@@ -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
@@ -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'])