Merge pull request #1376 from nextcloud/upstream-25966

Allow increasing permissions for share owner
nextcloud · Sep 13, 2016 · f8450e5 · f8450e5
2 parents feb8598 + 56d37e2
commit f8450e5
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 2 deletions.
diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php
@@ -661,7 +661,7 @@ public function updateShare(
 			}
 		}
 
-		if ($permissions !== null) {
+		if ($permissions !== null && $share->getShareOwner() !== $this->currentUser->getUID()) {
 			/* Check if this is an incomming share */
 			$incomingShares = $this->shareManager->getSharedWith($this->currentUser->getUID(), \OCP\Share::SHARE_TYPE_USER, $share->getNode(), -1, 0);
 			$incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser->getUID(), \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0));

diff --git a/apps/files_sharing/tests/API/Share20OCSTest.php b/apps/files_sharing/tests/API/Share20OCSTest.php
@@ -24,6 +24,8 @@
 namespace OCA\Files_Sharing\Tests\API;
 
 use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\OCS\OCSNotFoundException;
+use OCP\Files\Folder;
 use OCP\IL10N;
 use OCA\Files_Sharing\API\Share20OCS;
 use OCP\Files\NotFoundException;
@@ -108,8 +110,11 @@ protected function setUp() {
 		);
 	}
 
+	/**
+	 * @return Share20OCS|\PHPUnit_Framework_MockObject_MockObject
+	 */
 	private function mockFormatShare() {
-		return $this->getMockBuilder('OCA\Files_Sharing\API\Share20OCS')
+		return $this->getMockBuilder(Share20OCS::class)
 			->setConstructorArgs([
 				$this->appName,
 				$this->request,
@@ -1569,6 +1574,103 @@ public function testUpdateOtherPermissions() {
 		$this->assertEquals($expected->getData(), $result->getData());
 	}
 
+	public function testUpdateShareCannotIncreasePermissions() {
+		$ocs = $this->mockFormatShare();
+
+		$folder = $this->createMock(Folder::class);
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share
+			->setId(42)
+			->setSharedBy($this->currentUser->getUID())
+			->setShareOwner('anotheruser')
+			->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
+			->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->getUID())
+			->setShareOwner('anotheruser')
+			->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
+			->setSharedWith('group1')
+			->setPermissions(\OCP\Constants::PERMISSION_READ)
+			->setNode($folder);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['permissions', null, '31'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+
+		$this->shareManager->expects($this->any(0))
+			->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, [$incomingShare]]
+			]));
+
+		$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 testUpdateShareCanIncreasePermissionsIfOwner() {
+		$ocs = $this->mockFormatShare();
+
+		$folder = $this->createMock(Folder::class);
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share
+			->setId(42)
+			->setSharedBy($this->currentUser->getUID())
+			->setShareOwner($this->currentUser->getUID())
+			->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
+			->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->getUID())
+			->setShareOwner($this->currentUser->getUID())
+			->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
+			->setSharedWith('group1')
+			->setPermissions(\OCP\Constants::PERMISSION_READ)
+			->setNode($folder);
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+
+		$this->shareManager->expects($this->any(0))
+			->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, [$incomingShare]]
+			]));
+
+		$this->shareManager->expects($this->once())
+			->method('updateShare')
+			->with($share)
+			->willReturn($share);
+
+		$result = $ocs->updateShare(42, 31);
+		$this->assertInstanceOf(DataResponse::class, $result);
+	}
+
 	public function dataFormatShare() {
 		$file = $this->getMockBuilder('\OCP\Files\File')->getMock();
 		$folder = $this->getMockBuilder('\OCP\Files\Folder')->getMock();