From 0c9c7737b4b03f17af957b5036c143351a445a1b Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 25 Feb 2016 10:30:03 +0100 Subject: [PATCH] Remove delete permissions for read-only federated reshares An incomming federated share is just a mount point. Therefor if we request the permissions on the mountpoint DELETE permissions will be returned (among others). Since we can always remove a mountpoint, update a mount point. However now when trying to reshare we will try to reshare with DELETE permissions. Which is false. This PR removes the delete permissions if it is a shared storage. Basically a quick hack. Fixes #22587 --- apps/files_sharing/api/share20ocs.php | 9 ++ .../tests/api/share20ocstest.php | 120 ++++++++++++++++++ 2 files changed, 129 insertions(+) diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php index 309e1159ff..588538fbb6 100644 --- a/apps/files_sharing/api/share20ocs.php +++ b/apps/files_sharing/api/share20ocs.php @@ -271,6 +271,15 @@ class Share20OCS { $permissions &= ~\OCP\Constants::PERMISSION_CREATE; } + /* + * Hack for https://github.com/owncloud/core/issues/22587 + * We check the permissions via webdav. But the permissions of the mount point + * do not equal the share permissions. Here we fix that for federated mounts. + */ + if ($path->getStorage()->instanceOfStorage('OCA\Files_Sharing\External\Storage')) { + $permissions &= ~($permissions & ~$path->getPermissions()); + } + $shareWith = $this->request->getParam('shareWith', null); $shareType = (int)$this->request->getParam('shareType', '-1'); diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php index 44d94868a3..057df20a5a 100644 --- a/apps/files_sharing/tests/api/share20ocstest.php +++ b/apps/files_sharing/tests/api/share20ocstest.php @@ -553,6 +553,11 @@ class Share20OCSTest extends \Test\TestCase { ->willReturn($userFolder); $path = $this->getMock('\OCP\Files\File'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') ->with('valid-path') @@ -586,6 +591,11 @@ class Share20OCSTest extends \Test\TestCase { ->willReturn($userFolder); $path = $this->getMock('\OCP\Files\File'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') ->with('valid-path') @@ -632,6 +642,11 @@ class Share20OCSTest extends \Test\TestCase { ->willReturn($userFolder); $path = $this->getMock('\OCP\Files\File'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') ->with('valid-path') @@ -678,6 +693,11 @@ class Share20OCSTest extends \Test\TestCase { ->willReturn($userFolder); $path = $this->getMock('\OCP\Files\File'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') ->with('valid-path') @@ -724,6 +744,11 @@ class Share20OCSTest extends \Test\TestCase { ->willReturn($userFolder); $path = $this->getMock('\OCP\Files\Folder'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') ->with('valid-path') @@ -754,6 +779,11 @@ class Share20OCSTest extends \Test\TestCase { ])); $path = $this->getMock('\OCP\Files\Folder'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf()); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -776,6 +806,11 @@ class Share20OCSTest extends \Test\TestCase { ])); $path = $this->getMock('\OCP\Files\Folder'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf()); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -799,6 +834,11 @@ class Share20OCSTest extends \Test\TestCase { ])); $path = $this->getMock('\OCP\Files\File'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf()); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -827,6 +867,11 @@ class Share20OCSTest extends \Test\TestCase { ])); $path = $this->getMock('\OCP\Files\Folder'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf()); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -866,6 +911,11 @@ class Share20OCSTest extends \Test\TestCase { ])); $path = $this->getMock('\OCP\Files\Folder'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf()); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -905,6 +955,11 @@ class Share20OCSTest extends \Test\TestCase { ])); $path = $this->getMock('\OCP\Files\Folder'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf()); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -947,6 +1002,11 @@ class Share20OCSTest extends \Test\TestCase { ])); $path = $this->getMock('\OCP\Files\Folder'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf()); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -961,6 +1021,66 @@ class Share20OCSTest extends \Test\TestCase { $this->assertEquals($expected->getData(), $result->getData()); } + /** + * Test for https://github.com/owncloud/core/issues/22587 + * TODO: Remove once proper solution is in place + */ + public function testCreateReshareOfFederatedMountNoDeletePermissions() { + $share = \OC::$server->getShareManager()->newShare(); + $this->shareManager->method('newShare')->willReturn($share); + + $ocs = $this->getMockBuilder('OCA\Files_Sharing\API\Share20OCS') + ->setConstructorArgs([ + $this->shareManager, + $this->groupManager, + $this->userManager, + $this->request, + $this->rootFolder, + $this->urlGenerator, + $this->currentUser + ])->setMethods(['formatShare']) + ->getMock(); + + $this->request + ->method('getParam') + ->will($this->returnValueMap([ + ['path', null, 'valid-path'], + ['permissions', null, \OCP\Constants::PERMISSION_ALL], + ['shareType', $this->any(), \OCP\Share::SHARE_TYPE_USER], + ['shareWith', null, 'validUser'], + ])); + + $userFolder = $this->getMock('\OCP\Files\Folder'); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('currentUser') + ->willReturn($userFolder); + + $path = $this->getMock('\OCP\Files\Folder'); + $storage = $this->getMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(true); + $path->method('getStorage')->willReturn($storage); + $path->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ); + $userFolder->expects($this->once()) + ->method('get') + ->with('valid-path') + ->willReturn($path); + + $this->userManager->method('userExists')->with('validUser')->willReturn(true); + + $this->shareManager + ->expects($this->once()) + ->method('createShare') + ->with($this->callback(function (\OCP\Share\IShare $share) { + return $share->getPermissions() === \OCP\Constants::PERMISSION_READ; + })) + ->will($this->returnArgument(0)); + + $ocs->createShare(); + } + public function testUpdateShareCantAccess() { $share = \OC::$server->getShareManager()->newShare();