Fixes not allowed increasing of link share permissions

Fixes the following:

1. user0 shares folder with user1 (RO but with sharing permissions)
2. user1 shares by link
3. user1 send 'publicUpload=true' OCS request to the link share

before this increased the permissions of the link share. Which should
not happen.

now: API reponds with an error that the permissions can't be increased.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
index 1358663ea2b6c2d180803c92f4b1097572112ef1..90274beba492cbfda1faa3815cba4ad7555e4fb5 100644 (file)
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -692,6 +692,7 @@ class ShareAPIController extends OCSController {
 
                        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 890fdb6eda0e493e9c488328f4e22e150bfd192f..ed4aa1dba9e5d5c2146a2efd3ead038bba67e93a 100644 (file)
--- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
@@ -1205,7 +1205,7 @@ class ShareAPIControllerTest extends \Test\TestCase {
        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 @@ class ShareAPIControllerTest extends \Test\TestCase {
                        })
                )->will($this->returnArgument(0));
 
+               $this->shareManager->method('getSharedWith')
+                       ->willReturn([]);
+
                $expected = new DataResponse(null);
                $result = $ocs->updateShare(42, null, '', 'false', '');
 
@@ -1261,6 +1264,9 @@ class ShareAPIControllerTest extends \Test\TestCase {
                        })
                )->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 @@ class ShareAPIControllerTest extends \Test\TestCase {
                        })
                )->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 @@ class ShareAPIControllerTest extends \Test\TestCase {
                }
        }
 
+       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();