From 2316cb1f8b9704621b47e6eb1c2cbf30c06c552d Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 29 Jan 2016 10:27:39 +0100 Subject: [PATCH] [Share 2.0] Allow moving of shares * Only recipient can move a share * Unit tests --- lib/private/share20/defaultshareprovider.php | 58 +++++++++++++++ lib/private/share20/manager.php | 19 +++++ lib/public/share/imanager.php | 20 ++++- lib/public/share/ishareprovider.php | 13 ++++ .../lib/share20/defaultshareprovidertest.php | 69 ++++++++++++++++++ tests/lib/share20/managertest.php | 73 +++++++++++++++++++ 6 files changed, 249 insertions(+), 3 deletions(-) diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php index ed9c03020b..224dddf401 100644 --- a/lib/private/share20/defaultshareprovider.php +++ b/lib/private/share20/defaultshareprovider.php @@ -20,6 +20,7 @@ */ namespace OC\Share20; +use OCP\Files\File; use OCP\Share\IShareProvider; use OC\Share20\Exception\InvalidShare; use OC\Share20\Exception\ProviderException; @@ -383,6 +384,63 @@ class DefaultShareProvider implements IShareProvider { } } + /** + * @inheritdoc + */ + public function move(\OCP\Share\IShare $share, IUser $recipient) { + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER) { + // Just update the target + $qb = $this->dbConn->getQueryBuilder(); + $qb->update('share') + ->set('file_target', $qb->createNamedParameter($share->getTarget())) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId()))) + ->execute(); + + } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) { + + // Check if there is a usergroup share + $qb = $this->dbConn->getQueryBuilder(); + $stmt = $qb->select('id') + ->from('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_USERGROUP))) + ->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($recipient->getUID()))) + ->andWhere($qb->expr()->eq('parent', $qb->createNamedParameter($share->getId()))) + ->setMaxResults(1) + ->execute(); + + $data = $stmt->fetch(); + $stmt->closeCursor(); + + if ($data === false) { + // No usergroup share yet. Create one. + $qb = $this->dbConn->getQueryBuilder(); + $qb->insert('share') + ->values([ + 'share_type' => $qb->createNamedParameter(self::SHARE_TYPE_USERGROUP), + 'share_with' => $qb->createNamedParameter($recipient->getUID()), + 'uid_owner' => $qb->createNamedParameter($share->getShareOwner()->getUID()), + 'uid_initiator' => $qb->createNamedParameter($share->getSharedBy()->getUID()), + 'parent' => $qb->createNamedParameter($share->getId()), + 'item_type' => $qb->createNamedParameter($share->getNode() instanceof File ? 'file' : 'folder'), + 'item_source' => $qb->createNamedParameter($share->getNode()->getId()), + 'file_source' => $qb->createNamedParameter($share->getNode()->getId()), + 'file_target' => $qb->createNamedParameter($share->getTarget()), + 'permissions' => $qb->createNamedParameter($share->getPermissions()), + 'stime' => $qb->createNamedParameter($share->getShareTime()->getTimestamp()), + ])->execute(); + } else { + // Already a usergroup share. Update it. + $qb = $this->dbConn->getQueryBuilder(); + $qb->update('share') + ->set('file_target', $qb->createNamedParameter($share->getTarget())) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($data['id']))) + ->execute(); + } + } + + return $share; + } + /** * Get all shares by the given user. Sharetype and path can be used to filter. * diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index 3c5bd197ae..438396501e 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -714,6 +714,25 @@ class Manager implements IManager { $provider->deleteFromSelf($share, $recipient); } + /** + * @inheritdoc + */ + public function moveShare(\OCP\Share\IShare $share, IUser $recipient) { + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) { + throw new \InvalidArgumentException('Can\'t change target of link share'); + } + + if (($share->getShareType() === \OCP\Share::SHARE_TYPE_USER && $share->getSharedWith() !== $recipient) || + ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP && !$share->getSharedWith()->inGroup($recipient))) { + throw new \InvalidArgumentException('Invalid recipient'); + } + + list($providerId, ) = $this->splitFullId($share->getId()); + $provider = $this->factory->getProvider($providerId); + + $provider->move($share, $recipient); + } + /** * Get shares shared by (initiated) by the provided user. * diff --git a/lib/public/share/imanager.php b/lib/public/share/imanager.php index 6919fea00d..15b9f34764 100644 --- a/lib/public/share/imanager.php +++ b/lib/public/share/imanager.php @@ -37,13 +37,15 @@ interface IManager { * Create a Share * * @param IShare $share - * @return Share The share object + * @return IShare The share object * @since 9.0.0 */ public function createShare(IShare $share); /** - * Update a share + * Update a share. + * The target of the share can't be changed this way: use moveShare + * The share can't be removed this way (permission 0): use deleteShare * * @param IShare $share * @return IShare The share object @@ -72,6 +74,18 @@ interface IManager { */ public function deleteFromSelf(IShare $share, IUser $recipient); + /** + * Move the share as a recipient of the share. + * This is updating the share target. So where the recipient has the share mounted. + * + * @param IShare $share + * @param IUser $recipient + * @return IShare + * @throws \InvalidArgumentException If $share is a link share or the $recipient does not match + * @since 9.0.0 + */ + public function moveShare(IShare $share, IUser $recipient); + /** * Get shares shared by (initiated) by the provided user. * @@ -118,7 +132,7 @@ interface IManager { * Get the share by token possible with password * * @param string $token - * @return Share + * @return IShare * @throws ShareNotFound * @since 9.0.0 */ diff --git a/lib/public/share/ishareprovider.php b/lib/public/share/ishareprovider.php index 9dc56dc37a..42a2881718 100644 --- a/lib/public/share/ishareprovider.php +++ b/lib/public/share/ishareprovider.php @@ -79,6 +79,19 @@ interface IShareProvider { */ public function deleteFromSelf(\OCP\Share\IShare $share, IUser $recipient); + /** + * Move a share as a recipient. + * This is updating the share target. Thus the mount point of the recipient. + * This may require special handling. If a user moves a group share + * the target should only be changed for them. + * + * @param \OCP\Share\IShare $share + * @param IUser $recipient + * @return \OCP\Share\IShare + * @since 9.0.0 + */ + public function move(\OCP\Share\IShare $share, IUser $recipient); + /** * Get all shares by the given user * diff --git a/tests/lib/share20/defaultshareprovidertest.php b/tests/lib/share20/defaultshareprovidertest.php index 4145e9e5ec..504bd77603 100644 --- a/tests/lib/share20/defaultshareprovidertest.php +++ b/tests/lib/share20/defaultshareprovidertest.php @@ -1892,6 +1892,75 @@ class DefaultShareProviderTest extends \Test\TestCase { $stmt->closeCursor(); + } + public function testMoveUserShare() { + $id = $this->addShareToDB(\OCP\Share::SHARE_TYPE_USER, 'user0', 'user1', 'user1', 'file', + 42, 'mytaret', 31, null, null); + + $user0 = $this->getMock('\OCP\IUser'); + $user0->method('getUID')->willReturn('user0'); + $user1 = $this->getMock('\OCP\IUser'); + $user1->method('getUID')->willReturn('user1'); + + $this->userManager->method('get')->will($this->returnValueMap([ + ['user0', $user0], + ['user1', $user1], + ])); + + $file = $this->getMock('\OCP\Files\File'); + $file->method('getId')->willReturn(42); + + $this->rootFolder->method('getUserFolder')->with('user1')->will($this->returnSelf()); + $this->rootFolder->method('getById')->willReturn([$file]); + + $share = $this->provider->getShareById($id, null); + + $share->setTarget('/newTarget'); + $this->provider->move($share, $user0); + + $share = $this->provider->getShareById($id, null); + $this->assertSame('/newTarget', $share->getTarget()); + } + + public function testMoveGroupShare() { + $id = $this->addShareToDB(\OCP\Share::SHARE_TYPE_GROUP, 'group0', 'user1', 'user1', 'file', + 42, 'mytaret', 31, null, null); + + $user0 = $this->getMock('\OCP\IUser'); + $user0->method('getUID')->willReturn('user0'); + $user1 = $this->getMock('\OCP\IUser'); + $user1->method('getUID')->willReturn('user1'); + + $group0 = $this->getMock('\OCP\IGroup'); + $group0->method('getGID')->willReturn('group0'); + $group0->method('inGroup')->with($user0)->willReturn(true); + + $this->groupManager->method('get')->with('group0')->willReturn($group0); + + $this->userManager->method('get')->will($this->returnValueMap([ + ['user0', $user0], + ['user1', $user1], + ])); + + $folder = $this->getMock('\OCP\Files\Folder'); + $folder->method('getId')->willReturn(42); + + $this->rootFolder->method('getUserFolder')->with('user1')->will($this->returnSelf()); + $this->rootFolder->method('getById')->willReturn([$folder]); + + $share = $this->provider->getShareById($id, $user0); + + $share->setTarget('/newTarget'); + $this->provider->move($share, $user0); + + $share = $this->provider->getShareById($id, $user0); + $this->assertSame('/newTarget', $share->getTarget()); + + $share->setTarget('/ultraNewTarget'); + $this->provider->move($share, $user0); + + $share = $this->provider->getShareById($id, $user0); + $this->assertSame('/ultraNewTarget', $share->getTarget()); } } diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php index b5559bb517..1a6e8200ad 100644 --- a/tests/lib/share20/managertest.php +++ b/tests/lib/share20/managertest.php @@ -1887,6 +1887,79 @@ class ManagerTest extends \Test\TestCase { $manager->updateShare($share); } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Can't change target of link share + */ + public function testMoveShareLink() { + $share = $this->manager->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_LINK); + + $recipient = $this->getMock('\OCP\IUser'); + + $this->manager->moveShare($share, $recipient); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Invalid recipient + */ + public function testMoveShareUserNotRecipient() { + $share = $this->manager->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_USER); + + $sharedWith = $this->getMock('\OCP\IUser'); + $share->setSharedWith($sharedWith); + + $recipient = $this->getMock('\OCP\IUser'); + + $this->manager->moveShare($share, $recipient); + } + + public function testMoveShareUser() { + $share = $this->manager->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_USER); + + $recipient = $this->getMock('\OCP\IUser'); + $share->setSharedWith($recipient); + + $this->defaultProvider->method('move')->with($share, $recipient)->will($this->returnArgument(0)); + + $this->manager->moveShare($share, $recipient); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Invalid recipient + */ + public function testMoveShareGroupNotRecipient() { + $share = $this->manager->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_GROUP); + + $sharedWith = $this->getMock('\OCP\IGroup'); + $share->setSharedWith($sharedWith); + + $recipient = $this->getMock('\OCP\IUser'); + $sharedWith->method('inGroup')->with($recipient)->willReturn(false); + + $this->manager->moveShare($share, $recipient); + } + + public function testMoveShareGroup() { + $share = $this->manager->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_GROUP); + + $sharedWith = $this->getMock('\OCP\IGroup'); + $share->setSharedWith($sharedWith); + + $recipient = $this->getMock('\OCP\IUser'); + $sharedWith->method('inGroup')->with($recipient)->willReturn(true); + + $this->defaultProvider->method('move')->with($share, $recipient)->will($this->returnArgument(0)); + + $this->manager->moveShare($share, $recipient); + } } class DummyPassword {