From 2dcf887cceca8dd25b3d664fb0726cb6d69739ef Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 18 Feb 2016 13:38:38 +0100 Subject: [PATCH] When (re-)sharing an incomming federated share set the corrent owner Incomming federated shares are a special kind. We mount them as normal webdav shares but we do supply owner info with the federated cloud id of the share owner. Since we do not yet have the new resharing behaviour on federated shares we need to set the correct owner. Which will allow sharing and proper mounting for other users. fixes #22500 --- lib/private/share20/manager.php | 16 ++++- tests/lib/share20/managertest.php | 97 ++++++++++++++++++++++++++++--- 2 files changed, 102 insertions(+), 11 deletions(-) diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index 4cff3dc818..9b33e94755 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -517,8 +517,20 @@ class Manager implements IManager { // Verify if there are any issues with the path $this->pathCreateChecks($share->getNode()); - // On creation of a share the owner is always the owner of the path - $share->setShareOwner($share->getNode()->getOwner()->getUID()); + /* + * On creation of a share the owner is always the owner of the path + * Except for mounted federated shares. + */ + $storage = $share->getNode()->getStorage(); + if ($storage->instanceOfStorage('OCA\Files_Sharing\External\Storage')) { + $parent = $share->getNode()->getParent(); + while($parent->getStorage()->instanceOfStorage('OCA\Files_Sharing\External\Storage')) { + $parent = $parent->getParent(); + } + $share->setShareOwner($parent->getOwner()->getUID()); + } else { + $share->setShareOwner($share->getNode()->getOwner()->getUID()); + } // Cannot share with the owner if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER && diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php index fe94b72c4e..c41f075439 100644 --- a/tests/lib/share20/managertest.php +++ b/tests/lib/share20/managertest.php @@ -1429,9 +1429,11 @@ class ManagerTest extends \Test\TestCase { $shareOwner = $this->getMock('\OCP\IUser'); $shareOwner->method('getUID')->willReturn('shareOwner'); + $storage = $this->getMock('\OCP\Files\Storage'); $path = $this->getMock('\OCP\Files\File'); $path->method('getOwner')->willReturn($shareOwner); $path->method('getName')->willReturn('target'); + $path->method('getStorage')->willReturn($storage); $share = $this->createShare( null, @@ -1480,9 +1482,11 @@ class ManagerTest extends \Test\TestCase { $shareOwner = $this->getMock('\OCP\IUser'); $shareOwner->method('getUID')->willReturn('shareOwner'); + $storage = $this->getMock('\OCP\Files\Storage'); $path = $this->getMock('\OCP\Files\File'); $path->method('getOwner')->willReturn($shareOwner); $path->method('getName')->willReturn('target'); + $path->method('getStorage')->willReturn($storage); $share = $this->createShare( null, @@ -1539,10 +1543,12 @@ class ManagerTest extends \Test\TestCase { $shareOwner = $this->getMock('\OCP\IUser'); $shareOwner->method('getUID')->willReturn('shareOwner'); + $storage = $this->getMock('\OCP\Files\Storage'); $path = $this->getMock('\OCP\Files\File'); $path->method('getOwner')->willReturn($shareOwner); $path->method('getName')->willReturn('target'); $path->method('getId')->willReturn(1); + $path->method('getStorage')->willReturn($storage); $date = new \DateTime(); @@ -1663,9 +1669,11 @@ class ManagerTest extends \Test\TestCase { $shareOwner = $this->getMock('\OCP\IUser'); $shareOwner->method('getUID')->willReturn('shareOwner'); + $storage = $this->getMock('\OCP\Files\Storage'); $path = $this->getMock('\OCP\Files\File'); $path->method('getOwner')->willReturn($shareOwner); $path->method('getName')->willReturn('target'); + $path->method('getStorage')->willReturn($storage); $share = $this->createShare( null, @@ -1697,8 +1705,86 @@ class ManagerTest extends \Test\TestCase { ->method('setTarget') ->with('/target'); - $dummy = new DummyCreate(); - \OCP\Util::connectHook('OCP\Share', 'pre_shared', $dummy, 'listner'); + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['pre'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'pre_shared', $hookListner, 'pre'); + $hookListner->expects($this->once()) + ->method('pre') + ->will($this->returnCallback(function (array $data) { + $data['run'] = false; + $data['error'] = 'I won\'t let you share!'; + })); + + $manager->createShare($share); + } + + public function testCreateShareOfIncommingFederatedShare() { + $manager = $this->createManagerMock() + ->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks']) + ->getMock(); + + $shareOwner = $this->getMock('\OCP\IUser'); + $shareOwner->method('getUID')->willReturn('shareOwner'); + + $storage = $this->getMock('\OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(true); + + $storage2 = $this->getMock('\OCP\Files\Storage'); + $storage2->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + + $path = $this->getMock('\OCP\Files\File'); + $path->expects($this->never())->method('getOwner'); + $path->method('getName')->willReturn('target'); + $path->method('getStorage')->willReturn($storage); + + $parent = $this->getMock('\OCP\Files\Folder'); + $parent->method('getStorage')->willReturn($storage); + + $parentParent = $this->getMock('\OCP\Files\Folder'); + $parentParent->method('getStorage')->willReturn($storage2); + $parentParent->method('getOwner')->willReturn($shareOwner); + + $path->method('getParent')->willReturn($parent); + $parent->method('getParent')->willReturn($parentParent); + + $share = $this->createShare( + null, + \OCP\Share::SHARE_TYPE_USER, + $path, + 'sharedWith', + 'sharedBy', + null, + \OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once()) + ->method('canShare') + ->with($share) + ->willReturn(true); + $manager->expects($this->once()) + ->method('generalCreateChecks') + ->with($share);; + $manager->expects($this->once()) + ->method('userCreateChecks') + ->with($share);; + $manager->expects($this->once()) + ->method('pathCreateChecks') + ->with($path); + + $this->defaultProvider + ->expects($this->once()) + ->method('create') + ->with($share) + ->will($this->returnArgument(0)); + + $share->expects($this->once()) + ->method('setShareOwner') + ->with('shareOwner'); + $share->expects($this->once()) + ->method('setTarget') + ->with('/target'); $manager->createShare($share); } @@ -2269,13 +2355,6 @@ class DummyPassword { } } -class DummyCreate { - public function listner($array) { - $array['run'] = false; - $array['error'] = 'I won\'t let you share!'; - } -} - class DummyFactory implements IProviderFactory { /** @var IShareProvider */