From b57aac0a89c55fb6b227a4271d6e0869f61a8924 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 4 Feb 2016 09:53:47 +0100 Subject: [PATCH 1/2] [Share 2.0] Add missing post_update_permissions hook --- lib/private/share20/manager.php | 12 +++++++++ lib/private/share20/share.php | 2 ++ tests/lib/share20/managertest.php | 41 +++++++++++++++++++++++++------ 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index 6ea638b84e..2a350f02d6 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -613,6 +613,18 @@ class Manager implements IManager { ]); } + if ($share->getPermissions() !== $originalShare->getPermissions()) { + \OC_Hook::emit('OCP\Share', 'post_update_permissions', array( + 'itemType' => $share->getNode() instanceof \OCP\Files\File ? 'file' : 'folder', + 'itemSource' => $share->getNode()->getId(), + 'shareType' => $share->getShareType(), + 'shareWith' => $share->getSharedWith(), + 'uidOwner' => $share->getSharedBy(), + 'permissions' => $share->getPermissions(), + 'path' => $share->getNode()->getPath(), + )); + } + return $share; } diff --git a/lib/private/share20/share.php b/lib/private/share20/share.php index 448b05d20a..cd30f24c42 100644 --- a/lib/private/share20/share.php +++ b/lib/private/share20/share.php @@ -105,6 +105,8 @@ class Share implements \OCP\Share\IShare { * @inheritdoc */ public function setNode(Node $node) { + $this->fileId = null; + $this->nodeType = null; $this->node = $node; return $this; } diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php index 270f5da33c..1cdf63aa7e 100644 --- a/tests/lib/share20/managertest.php +++ b/tests/lib/share20/managertest.php @@ -1870,19 +1870,24 @@ class ManagerTest extends \Test\TestCase { $originalShare = $this->manager->newShare(); $originalShare->setShareType(\OCP\Share::SHARE_TYPE_USER) - ->setSharedWith('origUser'); + ->setSharedWith('origUser') + ->setPermissions(1); + + $node = $this->getMock('\OCP\Files\File'); + $node->method('getId')->willReturn(100); + $node->method('getPath')->willReturn('myPath'); $manager->expects($this->once())->method('canShare')->willReturn(true); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); - $node = $this->getMock('\OCP\Files\File'); - $share = $this->manager->newShare(); $share->setProviderId('foo') ->setId('42') ->setShareType(\OCP\Share::SHARE_TYPE_USER) ->setSharedWith('origUser') ->setShareOwner('newUser') + ->setSharedBy('sharer') + ->setPermissions(31) ->setNode($node); $this->defaultProvider->expects($this->once()) @@ -1894,6 +1899,17 @@ class ManagerTest extends \Test\TestCase { \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); $hookListner->expects($this->never())->method('post'); + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner2, 'post'); + $hookListner2->expects($this->once())->method('post')->with([ + 'itemType' => 'file', + 'itemSource' => 100, + 'shareType' => \OCP\Share::SHARE_TYPE_USER, + 'shareWith' => 'origUser', + 'uidOwner' => 'sharer', + 'permissions' => 31, + 'path' => 'myPath', + ]); $manager->updateShare($share); } @@ -1911,7 +1927,8 @@ class ManagerTest extends \Test\TestCase { $originalShare = $this->manager->newShare(); $originalShare->setShareType(\OCP\Share::SHARE_TYPE_GROUP) - ->setSharedWith('origUser'); + ->setSharedWith('origUser') + ->setPermissions(31); $manager->expects($this->once())->method('canShare')->willReturn(true); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); @@ -1924,7 +1941,8 @@ class ManagerTest extends \Test\TestCase { ->setShareType(\OCP\Share::SHARE_TYPE_GROUP) ->setSharedWith('origUser') ->setShareOwner('owner') - ->setNode($node); + ->setNode($node) + ->setPermissions(31); $this->defaultProvider->expects($this->once()) ->method('update') @@ -1935,6 +1953,9 @@ class ManagerTest extends \Test\TestCase { \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); $hookListner->expects($this->never())->method('post'); + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); $manager->updateShare($share); } @@ -1953,7 +1974,8 @@ class ManagerTest extends \Test\TestCase { ->getMock(); $originalShare = $this->manager->newShare(); - $originalShare->setShareType(\OCP\Share::SHARE_TYPE_LINK); + $originalShare->setShareType(\OCP\Share::SHARE_TYPE_LINK) + ->setPermissions(15); $tomorrow = new \DateTime(); $tomorrow->setTime(0,0,0); @@ -1970,7 +1992,8 @@ class ManagerTest extends \Test\TestCase { ->setShareOwner('owner') ->setPassword('password') ->setExpirationDate($tomorrow) - ->setNode($file); + ->setNode($file) + ->setPermissions(15); $manager->expects($this->once())->method('canShare')->willReturn(true); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); @@ -1990,6 +2013,10 @@ class ManagerTest extends \Test\TestCase { 'uidOwner' => 'owner', ]); + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner2, 'post'); + $hookListner2->expects($this->never())->method('post'); + $manager->updateShare($share); } From 169874957a155bc01f5e573350a16f60ab586e0c Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 4 Feb 2016 13:25:24 +0100 Subject: [PATCH 2/2] Path should be relative --- lib/private/share20/manager.php | 4 +++- tests/lib/share20/managertest.php | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index 2a350f02d6..d65fb927f9 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -77,6 +77,7 @@ class Manager implements IManager { * @param IL10N $l * @param IProviderFactory $factory * @param IUserManager $userManager + * @param IRootFolder $rootFolder */ public function __construct( ILogger $logger, @@ -614,6 +615,7 @@ class Manager implements IManager { } if ($share->getPermissions() !== $originalShare->getPermissions()) { + $userFolder = $this->rootFolder->getUserFolder($share->getShareOwner()); \OC_Hook::emit('OCP\Share', 'post_update_permissions', array( 'itemType' => $share->getNode() instanceof \OCP\Files\File ? 'file' : 'folder', 'itemSource' => $share->getNode()->getId(), @@ -621,7 +623,7 @@ class Manager implements IManager { 'shareWith' => $share->getSharedWith(), 'uidOwner' => $share->getSharedBy(), 'permissions' => $share->getPermissions(), - 'path' => $share->getNode()->getPath(), + 'path' => $userFolder->getRelativePath($share->getNode()->getPath()), )); } diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php index 1cdf63aa7e..131bc7fbfd 100644 --- a/tests/lib/share20/managertest.php +++ b/tests/lib/share20/managertest.php @@ -1875,7 +1875,7 @@ class ManagerTest extends \Test\TestCase { $node = $this->getMock('\OCP\Files\File'); $node->method('getId')->willReturn(100); - $node->method('getPath')->willReturn('myPath'); + $node->method('getPath')->willReturn('/newUser/files/myPath'); $manager->expects($this->once())->method('canShare')->willReturn(true); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); @@ -1899,6 +1899,9 @@ class ManagerTest extends \Test\TestCase { \OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post'); $hookListner->expects($this->never())->method('post'); + $this->rootFolder->method('getUserFolder')->with('newUser')->will($this->returnSelf()); + $this->rootFolder->method('getRelativePath')->with('/newUser/files/myPath')->willReturn('/myPath'); + $hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); \OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner2, 'post'); $hookListner2->expects($this->once())->method('post')->with([ @@ -1908,7 +1911,7 @@ class ManagerTest extends \Test\TestCase { 'shareWith' => 'origUser', 'uidOwner' => 'sharer', 'permissions' => 31, - 'path' => 'myPath', + 'path' => '/myPath', ]); $manager->updateShare($share);