Merge pull request #35600 from owncloud/release-10.2.1-bfa66f7b146eb7…

…fa9299984494cbea8a5def3ff7 [release-10.2.1] Do not allow to set higher permissions on a public link share for a resource which was shared with limited permissions
owncloud · Jun 20, 2019 · 8e466af · 8e466af
2 parents 7510419 + 46371fd
commit 8e466af
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 20 deletions.
diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php
@@ -825,6 +825,14 @@ public function updateShare($id) {
 			}
 
 			if ($newPermissions !== null) {
+				$shareHome = $this->rootFolder->getUserFolder($share->getSharedBy());
+				$nodes = $shareHome->getById($share->getNode()->getId());
+				foreach ($nodes as $node) {
+					if (($node->getPermissions() | $newPermissions) !== $node->getPermissions()) {
+						return new Result(null, 404, 'Cannot increase permission of ' . $share->getTarget());
+					}
+				}
+
 				$share->setPermissions($newPermissions);
 				$permissions = $newPermissions;
 			}

diff --git a/apps/files_sharing/tests/Controller/Share20OcsControllerTest.php b/apps/files_sharing/tests/Controller/Share20OcsControllerTest.php
@@ -28,6 +28,7 @@
 use OCA\Files_Sharing\Controller\Share20OcsController;
 use OCA\Files_Sharing\Service\NotificationPublisher;
 use OCA\Files_Sharing\SharingBlacklist;
+use OCP\Constants;
 use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
 use OCP\IConfig;
@@ -39,6 +40,7 @@
 use OCP\IGroup;
 use OCP\IUserManager;
 use OCP\IUserSession;
+use OCP\Lock\ILockingProvider;
 use OCP\Lock\LockedException;
 use OCP\Share;
 use Symfony\Component\EventDispatcher\EventDispatcher;
@@ -1660,7 +1662,50 @@ public function testUpdateNoParametersOther() {
 		$this->assertEquals($expected->getData(), $result->getData());
 	}
 
+	public function testUpdateLinkHigherPermissions() {
+		$node = $this->createMock(Folder::class);
+		$share = $this->newShare();
+		$share->setPermissions(Constants::PERMISSION_READ)
+			->setSharedBy($this->currentUser->getUID())
+			->setShareType(Share::SHARE_TYPE_LINK)
+			->setNode($node)
+			->setTarget('/foo/bar');
+
+		$node->expects($this->once())
+			->method('lock')
+			->with(ILockingProvider::LOCK_SHARED);
+
+		$this->request
+			->method('getParam')
+			->willReturnMap([
+				['permissions', null, '15'],
+			]);
+
+		$originalNode = $this->createMock(File::class);
+		$originalNode->method('getPermissions')->willReturn(17);
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')->willReturn([$originalNode]);
+		$this->rootFolder
+			->method('getUserFolder')
+			->willReturn($userFolder);
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$expected = new Result(null, 404, 'Cannot increase permission of /foo/bar');
+		$result = $this->ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
 	public function testUpdateLinkShareClear() {
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')->willReturn([]);
+		$this->rootFolder
+			->method('getUserFolder')
+			->willReturn($userFolder);
+
 		$ocs = $this->mockFormatShare();
 
 		$node = $this->createMock('\OCP\Files\Folder');
@@ -1707,6 +1752,12 @@ public function testUpdateLinkShareClear() {
 	}
 
 	public function testUpdateLinkShareSet() {
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')->willReturn([]);
+		$this->rootFolder
+			->method('getUserFolder')
+			->willReturn($userFolder);
+
 		$ocs = $this->mockFormatShare();
 
 		$folder = $this->createMock('\OCP\Files\Folder');
@@ -1751,6 +1802,12 @@ public function testUpdateLinkShareSet() {
 	 * @dataProvider publicUploadParamsProvider
 	 */
 	public function testUpdateLinkShareEnablePublicUpload($params) {
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')->willReturn([]);
+		$this->rootFolder
+			->method('getUserFolder')
+			->willReturn($userFolder);
+
 		$ocs = $this->mockFormatShare();
 
 		$folder = $this->createMock('\OCP\Files\Folder');
@@ -1793,6 +1850,12 @@ public function testUpdateLinkShareEnablePublicUpload($params) {
 	}
 
 	public function testUpdateLinkShareInvalidDate() {
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')->willReturn([]);
+		$this->rootFolder
+			->method('getUserFolder')
+			->willReturn($userFolder);
+
 		$ocs = $this->mockFormatShare();
 
 		$folder = $this->createMock('\OCP\Files\Folder');
@@ -1991,6 +2054,12 @@ public function testUpdateLinkShareExpireDateDoesNotChangeOther() {
 	}
 
 	public function testUpdateLinkSharePublicUploadDoesNotChangeOther() {
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')->willReturn([]);
+		$this->rootFolder
+			->method('getUserFolder')
+			->willReturn($userFolder);
+
 		$ocs = $this->mockFormatShare();
 
 		$date = new \DateTime('2000-01-01');
@@ -2032,6 +2101,12 @@ public function testUpdateLinkSharePublicUploadDoesNotChangeOther() {
 	}
 
 	public function testUpdateLinkSharePermissions() {
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')->willReturn([]);
+		$this->rootFolder
+			->method('getUserFolder')
+			->willReturn($userFolder);
+
 		$ocs = $this->mockFormatShare();
 
 		$date = new \DateTime('2000-01-01');
@@ -2074,6 +2149,12 @@ public function testUpdateLinkSharePermissions() {
 	}
 
 	public function testUpdateLinkShareCreateOnly() {
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')->willReturn([]);
+		$this->rootFolder
+			->method('getUserFolder')
+			->willReturn($userFolder);
+
 		$ocs = $this->mockFormatShare();
 
 		$folder = $this->createMock('\OCP\Files\Folder');
@@ -2339,9 +2420,13 @@ public function testRegularShareRecipientCannotIncreasePermission($nodeType, $cu
 	 * @dataProvider publicUploadParamsProvider
 	 */
 	public function testUpdateShareCannotIncreasePermissionsPublicLink($params) {
-		$ocs = $this->mockFormatShare();
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')->willReturn([]);
+		$this->rootFolder
+			->method('getUserFolder')
+			->willReturn($userFolder);
 
-		$date = new \DateTime('2000-01-01');
+		$ocs = $this->mockFormatShare();
 
 		$folder = $this->createMock('\OCP\Files\Folder');
 

diff --git a/tests/acceptance/features/apiShareManagement/reShare.feature b/tests/acceptance/features/apiShareManagement/reShare.feature
@@ -359,7 +359,6 @@ Feature: sharing
       | 1               | 200              |
       | 2               | 404              |
 
-  @issue-enterprise-3364
   Scenario Outline: increasing permissions of a public link from a sub-folder of a share with share+read only permissions is not allowed
     Given using OCS API version "<ocs_api_version>"
     And user "user0" has created folder "/test"
@@ -372,24 +371,13 @@ Feature: sharing
     And publicly uploading a file should not work
     When user "user1" updates the last share using the sharing API with
       | permissions | 15 |
-    Then the OCS status code should be "<ocs_status_code>"
-    And the HTTP status code should be "200"
-    #Then the OCS status code should be "404"
-    #And the HTTP status code should be "<http_status_code>"
-    And publicly uploading a file should work
-    #And publicly uploading a file should not work
-    # Delete the following 4 steps when fixing the issue:
-    And the public should be able to upload file "file.txt" with content "some text" to the last public shared folder
-    And as "user0" file "/test/sub/file.txt" should exist
-    And as "user1" file "/test/sub/file.txt" should exist
-    And the content of file "/test/sub/file.txt" for user "user0" should be "some text"
+    Then the OCS status code should be "404"
+    And the HTTP status code should be "<http_status_code>"
+    And publicly uploading a file should not work
     Examples:
-      | ocs_api_version | ocs_status_code |
-      | 1               | 100             |
-      | 2               | 200             |
-      #| ocs_api_version | http_status_code |
-      #| 1               | 200              |
-      #| 2               | 404              |
+      | ocs_api_version | http_status_code |
+      | 1               | 200              |
+      | 2               | 404              |
 
   Scenario Outline: resharing a file is not allowed when allow resharing has been disabled
     Given using OCS API version "<ocs_api_version>"