diff options
author | Morris Jobke <hey@morrisjobke.de> | 2019-07-03 20:07:05 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-07-03 20:07:05 +0200 |
commit | c5c14d09b190d3e8e1fe5e8e6aff7e95b0ac6f20 (patch) | |
tree | fa85a64fb55855ef4f825fe611e4e333df7a51a0 /apps | |
parent | a528942c2473ce25f0252b70bcf6a613e17195a6 (diff) | |
parent | 87836472d377bacbb0194134173855e0d255ee75 (diff) | |
download | nextcloud-server-c5c14d09b190d3e8e1fe5e8e6aff7e95b0ac6f20.tar.gz nextcloud-server-c5c14d09b190d3e8e1fe5e8e6aff7e95b0ac6f20.zip |
Merge pull request #16186 from nextcloud/bugfix/noid/also-check-permissions-when-creating-a-share
Better check reshare permissions part2
Diffstat (limited to 'apps')
-rw-r--r-- | apps/files_sharing/lib/Controller/ShareAPIController.php | 34 | ||||
-rw-r--r-- | apps/files_sharing/tests/Controller/ShareAPIControllerTest.php | 155 |
2 files changed, 11 insertions, 178 deletions
diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 66e39bb0715..09489861e1c 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -971,41 +971,13 @@ class ShareAPIController extends OCSController { } $share->setExpirationDate($expireDate); } - } - if ($permissions !== null && $share->getShareOwner() !== $this->currentUser) { - - // Get the root mount point for the user and check the share permissions there - $userFolder = $this->rootFolder->getUserFolder($this->currentUser); - $userNodes = $userFolder->getById($share->getNodeId()); - $userNode = array_shift($userNodes); - - $userMountPointId = $userNode->getMountPoint()->getStorageRootId(); - $userMountPoints = $userFolder->getById($userMountPointId); - $userMountPoint = array_shift($userMountPoints); - - /* Check if this is an incoming share */ - $incomingShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_USER, $userMountPoint, -1, 0); - $incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_GROUP, $userMountPoint, -1, 0)); - $incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_ROOM, $userMountPoint, -1, 0)); - - /** @var \OCP\Share\IShare[] $incomingShares */ - if (!empty($incomingShares)) { - $maxPermissions = 0; - foreach ($incomingShares as $incomingShare) { - $maxPermissions |= $incomingShare->getPermissions(); - } - - if ($share->getPermissions() & ~$maxPermissions) { - throw new OCSNotFoundException($this->l->t('Cannot increase permissions')); - } - } - } - - try { $share = $this->shareManager->updateShare($share); + } catch (GenericShareException $e) { + $code = $e->getCode() === 0 ? 403 : $e->getCode(); + throw new OCSException($e->getHint(), $code); } catch (\Exception $e) { throw new OCSBadRequestException($e->getMessage(), $e); } diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index f00b5c424bf..dcf264aae66 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -28,6 +28,7 @@ namespace OCA\Files_Sharing\Tests\Controller; use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\Files\File; use OCP\Files\Folder; @@ -45,6 +46,7 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\Files\IRootFolder; use OCP\Lock\LockedException; +use OCP\Share\Exceptions\GenericShareException; use OCP\Share\IManager; use OCP\Share; use Test\TestCase; @@ -2418,157 +2420,16 @@ class ShareAPIControllerTest extends TestCase { $mountPoint->method('getStorageRootId') ->willReturn(42); - $this->shareManager->expects($this->never())->method('updateShare'); - - try { - $ocs->updateShare(42, 31); - $this->fail(); - } catch (OCSNotFoundException $e) { - $this->assertEquals('Cannot increase permissions', $e->getMessage()); - } - } - - public function testUpdateShareCannotIncreasePermissionsLinkShare() { - $ocs = $this->mockFormatShare(); - - $folder = $this->createMock(Folder::class); - $folder->method('getId') - ->willReturn(42); - - $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, []], - ['currentUser', \OCP\Share::SHARE_TYPE_ROOM, $share->getNode(), -1, 0, []] - ])); - - $userFolder = $this->createMock(Folder::class); - $this->rootFolder->method('getUserFolder') - ->with($this->currentUser) - ->willReturn($userFolder); - - $userFolder->method('getById') - ->with(42) - ->willReturn([$folder]); - - $mountPoint = $this->createMock(IMountPoint::class); - $folder->method('getMountPoint') - ->willReturn($mountPoint); - $mountPoint->method('getStorageRootId') - ->willReturn(42); - - $this->shareManager->expects($this->never())->method('updateShare'); - $this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true); - - try { - $ocs->updateShare(42, null, null, null, 'true'); - $this->fail(); - } catch (OCSNotFoundException $e) { - $this->assertEquals('Cannot increase permissions', $e->getMessage()); - } - } - - public function testUpdateShareCannotIncreasePermissionsRoomShare() { - $ocs = $this->mockFormatShare(); - - $folder = $this->createMock(Folder::class); - $folder->method('getId') - ->willReturn(42); - - $share = \OC::$server->getShareManager()->newShare(); - $share - ->setId(42) - ->setSharedBy($this->currentUser) - ->setShareOwner('anotheruser') - ->setShareType(\OCP\Share::SHARE_TYPE_ROOM) - ->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) - ->setShareOwner('anotheruser') - ->setShareType(\OCP\Share::SHARE_TYPE_ROOM) - ->setSharedWith('group1') - ->setPermissions(\OCP\Constants::PERMISSION_READ) - ->setNode($folder); - - $this->request - ->method('getParam') - ->will($this->returnValueMap([ - ['permissions', null, '31'], - ])); - - $this->shareManager - ->method('getShareById') - ->will($this->returnCallback( - function ($id) use ($share) { - if ($id !== 'ocRoomShare:42') { - throw new \OCP\Share\Exceptions\ShareNotFound(); - } - - return $share; - } - )); - - $userFolder = $this->createMock(Folder::class); - $this->rootFolder->method('getUserFolder') - ->with($this->currentUser) - ->willReturn($userFolder); - - $userFolder->method('getById') - ->with(42) - ->willReturn([$folder]); - - $mountPoint = $this->createMock(IMountPoint::class); - $folder->method('getMountPoint') - ->willReturn($mountPoint); - $mountPoint->method('getStorageRootId') - ->willReturn(42); - - $this->shareManager->expects($this->any()) - ->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, []], - ['currentUser', \OCP\Share::SHARE_TYPE_ROOM, $share->getNode(), -1, 0, [$incomingShare]] - ])); - - $this->shareManager->expects($this->never())->method('updateShare'); + $this->shareManager->expects($this->once()) + ->method('updateShare') + ->with($share) + ->willThrowException(new GenericShareException('Can’t increase permissions of path/file', 'Can’t increase permissions of path/file', 404)); try { $ocs->updateShare(42, 31); $this->fail(); - } catch (OCSNotFoundException $e) { - $this->assertEquals('Cannot increase permissions', $e->getMessage()); + } catch (OCSException $e) { + $this->assertEquals('Can’t increase permissions of path/file', $e->getMessage()); } } |