From fb52b1af67320e221fc66c13b32f98e5d5438229 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 29 Aug 2016 11:21:44 +0200 Subject: [PATCH 1/2] Allow increasing permissions for share owner In some cases, the owner of the share is also recipient through a group share. The owner must still be able to increase permissions in that situation. --- apps/files_sharing/lib/API/Share20OCS.php | 2 +- .../tests/API/Share20OCSTest.php | 108 ++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php index 62a947ee2c..34f73c7ac0 100644 --- a/apps/files_sharing/lib/API/Share20OCS.php +++ b/apps/files_sharing/lib/API/Share20OCS.php @@ -661,7 +661,7 @@ class Share20OCS extends OCSController { } } - if ($permissions !== null) { + if ($permissions !== null && $share->getShareOwner() !== $this->currentUser->getUID()) { /* Check if this is an incomming share */ $incomingShares = $this->shareManager->getSharedWith($this->currentUser->getUID(), \OCP\Share::SHARE_TYPE_USER, $share->getNode(), -1, 0); $incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser->getUID(), \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0)); diff --git a/apps/files_sharing/tests/API/Share20OCSTest.php b/apps/files_sharing/tests/API/Share20OCSTest.php index 1f0b4855a0..2621c890b3 100644 --- a/apps/files_sharing/tests/API/Share20OCSTest.php +++ b/apps/files_sharing/tests/API/Share20OCSTest.php @@ -1569,6 +1569,114 @@ class Share20OCSTest extends \Test\TestCase { $this->assertEquals($expected->getData(), $result->getData()); } + public function testUpdateShareCannotIncreasePermissions() { + $ocs = $this->mockFormatShare(); + + $date = new \DateTime('2000-01-01'); + + $folder = $this->getMock('\OCP\Files\Folder'); + + $share = \OC::$server->getShareManager()->newShare(); + $share + ->setId(42) + ->setSharedBy($this->currentUser->getUID()) + ->setShareOwner('anotheruser') + ->setShareType(\OCP\Share::SHARE_TYPE_GROUP) + ->setSharedWith('group1') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder); + + // note: updateShare will modify the received instance but getSharedWith will reread from the database, + // so their values will be different + $incomingShare = \OC::$server->getShareManager()->newShare(); + $incomingShare + ->setId(42) + ->setSharedBy($this->currentUser->getUID()) + ->setShareOwner('anotheruser') + ->setShareType(\OCP\Share::SHARE_TYPE_GROUP) + ->setSharedWith('group1') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder); + + $this->request + ->method('getParam') + ->will($this->returnValueMap([ + ['permissions', null, '31'], + ])); + + $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); + + $this->shareManager->expects($this->any(0)) + ->method('getSharedWith') + ->will($this->returnValueMap([ + ['currentUser', \OCP\Share::SHARE_TYPE_USER, $share->getNode(), -1, 0, []], + ['currentUser', \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0, [$incomingShare]] + ])); + + $this->shareManager->expects($this->never())->method('updateShare'); + + $expected = new \OC_OCS_Result(null, 404, 'Cannot increase permissions'); + $result = $ocs->updateShare(42); + + $this->assertEquals($expected->getMeta(), $result->getMeta()); + $this->assertEquals($expected->getData(), $result->getData()); + } + + public function testUpdateShareCanIncreasePermissionsIfOwner() { + $ocs = $this->mockFormatShare(); + + $date = new \DateTime('2000-01-01'); + + $folder = $this->getMock('\OCP\Files\Folder'); + + $share = \OC::$server->getShareManager()->newShare(); + $share + ->setId(42) + ->setSharedBy($this->currentUser->getUID()) + ->setShareOwner($this->currentUser->getUID()) + ->setShareType(\OCP\Share::SHARE_TYPE_GROUP) + ->setSharedWith('group1') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder); + + // note: updateShare will modify the received instance but getSharedWith will reread from the database, + // so their values will be different + $incomingShare = \OC::$server->getShareManager()->newShare(); + $incomingShare + ->setId(42) + ->setSharedBy($this->currentUser->getUID()) + ->setShareOwner($this->currentUser->getUID()) + ->setShareType(\OCP\Share::SHARE_TYPE_GROUP) + ->setSharedWith('group1') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder); + + $this->request + ->method('getParam') + ->will($this->returnValueMap([ + ['permissions', null, '31'], + ])); + + $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); + + $this->shareManager->expects($this->any(0)) + ->method('getSharedWith') + ->will($this->returnValueMap([ + ['currentUser', \OCP\Share::SHARE_TYPE_USER, $share->getNode(), -1, 0, []], + ['currentUser', \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0, [$incomingShare]] + ])); + + $this->shareManager->expects($this->once()) + ->method('updateShare') + ->with($share) + ->willReturn($share); + + $expected = new \OC_OCS_Result(); + $result = $ocs->updateShare(42); + + $this->assertEquals($expected->getMeta(), $result->getMeta()); + $this->assertEquals($expected->getData(), $result->getData()); + } public function dataFormatShare() { $file = $this->getMockBuilder('\OCP\Files\File')->getMock(); $folder = $this->getMockBuilder('\OCP\Files\Folder')->getMock(); From 56d37e27bc543a1f7710cdde231891ee5a31926a Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 13 Sep 2016 09:26:00 +0200 Subject: [PATCH 2/2] Fix tests --- .../tests/API/Share20OCSTest.php | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/apps/files_sharing/tests/API/Share20OCSTest.php b/apps/files_sharing/tests/API/Share20OCSTest.php index 2621c890b3..f9579b8993 100644 --- a/apps/files_sharing/tests/API/Share20OCSTest.php +++ b/apps/files_sharing/tests/API/Share20OCSTest.php @@ -24,6 +24,8 @@ namespace OCA\Files_Sharing\Tests\API; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCS\OCSNotFoundException; +use OCP\Files\Folder; use OCP\IL10N; use OCA\Files_Sharing\API\Share20OCS; use OCP\Files\NotFoundException; @@ -108,8 +110,11 @@ class Share20OCSTest extends \Test\TestCase { ); } + /** + * @return Share20OCS|\PHPUnit_Framework_MockObject_MockObject + */ private function mockFormatShare() { - return $this->getMockBuilder('OCA\Files_Sharing\API\Share20OCS') + return $this->getMockBuilder(Share20OCS::class) ->setConstructorArgs([ $this->appName, $this->request, @@ -1572,9 +1577,7 @@ class Share20OCSTest extends \Test\TestCase { public function testUpdateShareCannotIncreasePermissions() { $ocs = $this->mockFormatShare(); - $date = new \DateTime('2000-01-01'); - - $folder = $this->getMock('\OCP\Files\Folder'); + $folder = $this->createMock(Folder::class); $share = \OC::$server->getShareManager()->newShare(); $share @@ -1615,19 +1618,18 @@ class Share20OCSTest extends \Test\TestCase { $this->shareManager->expects($this->never())->method('updateShare'); - $expected = new \OC_OCS_Result(null, 404, 'Cannot increase permissions'); - $result = $ocs->updateShare(42); - - $this->assertEquals($expected->getMeta(), $result->getMeta()); - $this->assertEquals($expected->getData(), $result->getData()); + try { + $ocs->updateShare(42, 31); + $this->fail(); + } catch (OCSNotFoundException $e) { + $this->assertEquals('Cannot increase permissions', $e->getMessage()); + } } public function testUpdateShareCanIncreasePermissionsIfOwner() { $ocs = $this->mockFormatShare(); - $date = new \DateTime('2000-01-01'); - - $folder = $this->getMock('\OCP\Files\Folder'); + $folder = $this->createMock(Folder::class); $share = \OC::$server->getShareManager()->newShare(); $share @@ -1651,12 +1653,6 @@ class Share20OCSTest extends \Test\TestCase { ->setPermissions(\OCP\Constants::PERMISSION_READ) ->setNode($folder); - $this->request - ->method('getParam') - ->will($this->returnValueMap([ - ['permissions', null, '31'], - ])); - $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); $this->shareManager->expects($this->any(0)) @@ -1671,12 +1667,10 @@ class Share20OCSTest extends \Test\TestCase { ->with($share) ->willReturn($share); - $expected = new \OC_OCS_Result(); - $result = $ocs->updateShare(42); - - $this->assertEquals($expected->getMeta(), $result->getMeta()); - $this->assertEquals($expected->getData(), $result->getData()); + $result = $ocs->updateShare(42, 31); + $this->assertInstanceOf(DataResponse::class, $result); } + public function dataFormatShare() { $file = $this->getMockBuilder('\OCP\Files\File')->getMock(); $folder = $this->getMockBuilder('\OCP\Files\Folder')->getMock();