diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 1358663ea2b6c..90274beba492c 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -692,6 +692,7 @@ public function updateShare( if ($newPermissions !== null) { $share->setPermissions($newPermissions); + $permissions = $newPermissions; } if ($expireDate === '') { diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 890fdb6eda0e4..ed4aa1dba9e5d 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -1205,7 +1205,7 @@ public function testUpdateNoParametersOther() { public function testUpdateLinkShareClear() { $ocs = $this->mockFormatShare(); - $node = $this->getMockBuilder('\OCP\Files\Folder')->getMock(); + $node = $this->getMockBuilder(Folder::class)->getMock(); $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser) @@ -1229,6 +1229,9 @@ public function testUpdateLinkShareClear() { }) )->will($this->returnArgument(0)); + $this->shareManager->method('getSharedWith') + ->willReturn([]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42, null, '', 'false', ''); @@ -1261,6 +1264,9 @@ public function testUpdateLinkShareSet() { }) )->will($this->returnArgument(0)); + $this->shareManager->method('getSharedWith') + ->willReturn([]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42, null, 'password', 'true', '2000-01-01'); @@ -1483,6 +1489,9 @@ public function testUpdateLinkSharePublicUploadDoesNotChangeOther() { }) )->will($this->returnArgument(0)); + $this->shareManager->method('getSharedWith') + ->willReturn([]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42, null, null, 'true', null); @@ -1633,6 +1642,52 @@ public function testUpdateShareCannotIncreasePermissions() { } } + public function testUpdateShareCannotIncreasePermissionsLinkShare() { + $ocs = $this->mockFormatShare(); + + $folder = $this->createMock(Folder::class); + + $share = \OC::$server->getShareManager()->newShare(); + $share + ->setId(42) + ->setSharedBy($this->currentUser) + ->setShareOwner('anotheruser') + ->setShareType(\OCP\Share::SHARE_TYPE_LINK) + ->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) + ->setShareOwner('anotheruser') + ->setShareType(\OCP\Share::SHARE_TYPE_USER) + ->setSharedWith('currentUser') + ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setNode($folder); + + $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); + + $this->shareManager->expects($this->any()) + ->method('getSharedWith') + ->will($this->returnValueMap([ + ['currentUser', \OCP\Share::SHARE_TYPE_USER, $share->getNode(), -1, 0, [$incomingShare]], + ['currentUser', \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0, []] + ])); + + $this->shareManager->expects($this->never())->method('updateShare'); + $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); + + try { + $ocs->updateShare(42, null, null, 'true'); + $this->fail(); + } catch (OCSNotFoundException $e) { + $this->assertEquals('Cannot increase permissions', $e->getMessage()); + } + } + public function testUpdateShareCanIncreasePermissionsIfOwner() { $ocs = $this->mockFormatShare(); diff --git a/build/integration/features/sharing-v1.feature b/build/integration/features/sharing-v1.feature index 07237ac7218b6..acfeb0e611ffa 100644 --- a/build/integration/features/sharing-v1.feature +++ b/build/integration/features/sharing-v1.feature @@ -971,3 +971,20 @@ Feature: sharing When Deleting last share Then etag of element "/" of user "user1" has changed And etag of element "/PARENT" of user "user0" has not changed + + Scenario: do not allow to increase link share permissions on reshare + Given As an "admin" + And user "admin" created a folder "/TMP" + And user "user0" exists + And creating a share with + | path | TMP | + | shareType | 0 | + | shareWith | user0 | + | permissions | 17 | + When As an "user0" + And creating a share with + | path | TMP | + | shareType | 3 | + And Updating last share with + | publicUpload | true | + Then the OCS status code should be "404"