diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php index 5ae97d70df00..680e983d1ed2 100644 --- a/apps/files_sharing/lib/Controller/Share20OcsController.php +++ b/apps/files_sharing/lib/Controller/Share20OcsController.php @@ -848,6 +848,10 @@ public function updateShare($id) { $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new Result(null, 400, $this->l->t('Wrong or no update parameter given')); } else { + if (($share->getPermissions() < $permissions) && + !$this->canIncreasePermission($share)) { + return new Result(null, 404, 'Cannot increase permission of ' . $share->getTarget()); + } $permissions = (int)$permissions; $share->setPermissions($permissions); } @@ -889,6 +893,31 @@ public function updateShare($id) { return new Result($this->formatShare($share)); } + /** + * If the user is not the owner of the share, then lets not allow to increase + * the permission of the share. + * + * @param IShare $share + * @return bool , true if permission is allowed to increase, else false. Also false if no user session available + */ + private function canIncreasePermission(IShare $share) { + $userObj = $this->userSession->getUser(); + if ($userObj === null) { + return false; + } + + $shareHome = $this->rootFolder->getUserFolder($share->getSharedBy()); + $nodes = $shareHome->getById($share->getNode()->getId()); + foreach ($nodes as $node) { + $nodeOwner = $node->getOwner()->getUID(); + if ($nodeOwner !== $userObj->getUID()) { + return false; + } + } + + return true; + } + /** * @NoCSRFRequired * @NoAdminRequired diff --git a/apps/files_sharing/tests/Controller/Share20OcsControllerTest.php b/apps/files_sharing/tests/Controller/Share20OcsControllerTest.php index 667501d7f36d..f009814d34a1 100644 --- a/apps/files_sharing/tests/Controller/Share20OcsControllerTest.php +++ b/apps/files_sharing/tests/Controller/Share20OcsControllerTest.php @@ -2232,7 +2232,8 @@ public function testUpdateShareCannotIncreasePermissions() { ->setShareType(Share::SHARE_TYPE_GROUP) ->setSharedWith('group1') ->setPermissions(\OCP\Constants::PERMISSION_READ) - ->setNode($folder); + ->setNode($folder) + ->setTarget('/folderShare'); $this->request ->method('getParam') @@ -2251,13 +2252,89 @@ public function testUpdateShareCannotIncreasePermissions() { $this->shareManager->expects($this->never())->method('updateShare'); - $expected = new \OC\OCS\Result(null, 404, 'Cannot increase permissions'); + $folderOwner = $this->createMock(IUser::class); + $folderOwner->method('getUID') + ->willReturn('foo1234'); + $userHomeFolder = $this->createMock(Folder::class); + $userNode = $this->createMock(Folder::class); + $userNode->method('getOwner') + ->willReturn($folderOwner); + $userHomeFolder->method('getById') + ->willReturn([$userNode]); + $this->rootFolder->method('getUserFolder') + ->willReturn($userHomeFolder); + + $expected = new Result(null, 404, 'Cannot increase permission of ' . $share->getTarget()); $result = $ocs->updateShare(42); $this->assertEquals($expected->getMeta(), $result->getMeta()); $this->assertEquals($expected->getData(), $result->getData()); } + public function providesDataForTestingIncreasePermissionRegularShare() { + return [ + ['file', 16, 17], + ['file', 17, 19], + ['folder', 17, 19], + ['folder', 19, 21], + ['folder', 21, 23], + ]; + } + + /** + * @dataProvider providesDataForTestingIncreasePermissionRegularShare + * + * @param string $nodeType + * @param int $currentPermission + * @param string $updatePermissionTo + */ + public function testRegularShareRecipientCannotIncreasePermission($nodeType, $currentPermission, $updatePermissionTo) { + $ocs = $this->mockFormatShare(); + + $share = \OC::$server->getShareManager()->newShare(); + $share + ->setId(42) + ->setSharedBy($this->currentUser->getUID()) + ->setShareOwner('anotheruser') + ->setShareType(Share::SHARE_TYPE_USER) + ->setSharedWith('user1') + ->setPermissions($currentPermission); + + if ($nodeType === 'file') { + $file = $this->createMock(File::class); + $share->setNode($file); + $share->setTarget('/testFile'); + } else { + $folder = $this->createMock(Folder::class); + $share->setNode($folder); + $share->setTarget('/testFolder'); + } + + $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); + + $this->request + ->method('getParam') + ->will($this->returnValueMap([ + ['permissions', null, $updatePermissionTo], + ])); + + $folderOwner = $this->createMock(IUser::class); + $folderOwner->method('getUID') + ->willReturn('foo1234'); + $userHomeFolder = $this->createMock(Folder::class); + $userNode = $this->createMock(Folder::class); + $userNode->method('getOwner') + ->willReturn($folderOwner); + $userHomeFolder->method('getById') + ->willReturn([$userNode]); + $this->rootFolder->method('getUserFolder') + ->willReturn($userHomeFolder); + + $result = $ocs->updateShare(42); + $this->assertEquals(404, $result->getStatusCode()); + $this->assertEquals('Cannot increase permission of ' . $share->getTarget(), $result->getMeta()['message']); + } + /** * @dataProvider publicUploadParamsProvider */ @@ -2360,7 +2437,16 @@ public function testUpdateShareCanIncreasePermissionsIfOwner() { ->with($share) ->willReturn($share); - $expected = new \OC\OCS\Result(); + $userHomeFolder = $this->createMock(Folder::class); + $userNode = $this->createMock(Folder::class); + $userNode->method('getOwner') + ->willReturn($this->currentUser); + $userHomeFolder->method('getById') + ->willReturn([$userNode]); + $this->rootFolder->method('getUserFolder') + ->willReturn($userHomeFolder); + + $expected = new Result(); $result = $ocs->updateShare(42); $this->assertEquals($expected->getMeta(), $result->getMeta());