From 35a3432793919303726a7ea03d6a714db4b40707 Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <rullzer@owncloud.com>
Date: Wed, 27 Jan 2016 15:42:11 +0100
Subject: [PATCH 1/4] [Share 2.0] When passing empty strings don't fail

The password and expiration date can be set to empty strings when
created. This is now handled gracefully.
---
 apps/files_sharing/api/share20ocs.php         |  14 +-
 .../tests/api/share20ocstest.php              | 246 +++++++++++++++++-
 2 files changed, 250 insertions(+), 10 deletions(-)

diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php
index 3048eb7cd1a..081c3b16999 100644
--- a/apps/files_sharing/api/share20ocs.php
+++ b/apps/files_sharing/api/share20ocs.php
@@ -272,12 +272,12 @@ class Share20OCS {
 			if ($publicUpload === 'true') {
 				// Check if public upload is allowed
 				if (!$this->shareManager->shareApiLinkAllowPublicUpload()) {
-					return new \OC_OCS_Result(null, 403, '"public upload disabled by the administrator');
+					return new \OC_OCS_Result(null, 403, 'public upload disabled by the administrator');
 				}
 
 				// Public upload can only be set for folders
 				if ($path instanceof \OCP\Files\File) {
-					return new \OC_OCS_Result(null, 404, '"public upload is only possible for public shared folders');
+					return new \OC_OCS_Result(null, 404, 'public upload is only possible for public shared folders');
 				}
 
 				$share->setPermissions(
@@ -290,12 +290,16 @@ class Share20OCS {
 			}
 
 			// Set password
-			$share->setPassword($this->request->getParam('password', null));
+			$password = $this->request->getParam('password', '');
+
+			if ($password !== '') {
+				$share->setPassword($password);
+			}
 
 			//Expire date
-			$expireDate = $this->request->getParam('expireDate', null);
+			$expireDate = $this->request->getParam('expireDate', '');
 
-			if ($expireDate !== null) {
+			if ($expireDate !== '') {
 				try {
 					$expireDate = $this->parseDate($expireDate);
 					$share->setExpirationDate($expireDate);
diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php
index f38af988155..4bbcfb5a600 100644
--- a/apps/files_sharing/tests/api/share20ocstest.php
+++ b/apps/files_sharing/tests/api/share20ocstest.php
@@ -31,19 +31,19 @@ use OCP\Files\IRootFolder;
 
 class Share20OCSTest extends \Test\TestCase {
 
-	/** @var \OC\Share20\Manager */
+	/** @var \OC\Share20\Manager | \PHPUnit_Framework_MockObject_MockObject */
 	private $shareManager;
 
-	/** @var IGroupManager */
+	/** @var IGroupManager | \PHPUnit_Framework_MockObject_MockObject */
 	private $groupManager;
 
-	/** @var IUserManager */
+	/** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject */
 	private $userManager;
 
-	/** @var IRequest */
+	/** @var IRequest | \PHPUnit_Framework_MockObject_MockObject */
 	private $request;
 
-	/** @var IRootFolder */
+	/** @var IRootFolder | \PHPUnit_Framework_MockObject_MockObject */
 	private $rootFolder;
 
 	/** @var IURLGenerator */
@@ -78,6 +78,20 @@ class Share20OCSTest extends \Test\TestCase {
 		);
 	}
 
+	private function mockFormatShare() {
+		return $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();
+	}
+
 	public function testDeleteShareShareNotFound() {
 		$this->shareManager
 			->expects($this->once())
@@ -696,4 +710,226 @@ class Share20OCSTest extends \Test\TestCase {
 		$this->assertEquals($expected->getMeta(), $result->getMeta());
 		$this->assertEquals($expected->getData(), $result->getData());
 	}
+
+	public function testCreateShareLinkNoLinksAllowed() {
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['path', null, 'valid-path'],
+				['shareType', '-1', \OCP\Share::SHARE_TYPE_LINK],
+			]));
+
+		$path = $this->getMock('\OCP\Files\Folder');
+		$this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf());
+		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+
+		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+
+		$expected = new \OC_OCS_Result(null, 404, 'public link sharing is disabled by the administrator');
+		$result = $this->ocs->createShare();
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testCreateShareLinkNoPublicUpload() {
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['path', null, 'valid-path'],
+				['shareType', '-1', \OCP\Share::SHARE_TYPE_LINK],
+				['publicUpload', null, 'true'],
+			]));
+
+		$path = $this->getMock('\OCP\Files\Folder');
+		$this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf());
+		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+
+		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
+
+		$expected = new \OC_OCS_Result(null, 403, 'public upload disabled by the administrator');
+		$result = $this->ocs->createShare();
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testCreateShareLinkPublicUploadFile() {
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['path', null, 'valid-path'],
+				['shareType', '-1', \OCP\Share::SHARE_TYPE_LINK],
+				['publicUpload', null, 'true'],
+			]));
+
+		$path = $this->getMock('\OCP\Files\File');
+		$this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf());
+		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+
+		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$expected = new \OC_OCS_Result(null, 404, 'public upload is only possible for public shared folders');
+		$result = $this->ocs->createShare();
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testCreateShareLinkPublicUploadFolder() {
+		$ocs = $this->mockFormatShare();
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['path', null, 'valid-path'],
+				['shareType', '-1', \OCP\Share::SHARE_TYPE_LINK],
+				['publicUpload', null, 'true'],
+				['expireDate', '', ''],
+				['password', '', ''],
+			]));
+
+		$path = $this->getMock('\OCP\Files\Folder');
+		$this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf());
+		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+
+		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$currentUser = $this->currentUser;
+
+		$this->shareManager->expects($this->once())->method('createShare')->with(
+			$this->callback(function (IShare $share) use ($path, $currentUser) {
+				return $share->getPath() === $path &&
+					$share->getShareType() === \OCP\Share::SHARE_TYPE_LINK &&
+					$share->getPermissions() === \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_DELETE &&
+					$share->getSharedBy() === $currentUser &&
+					$share->getPassword() === null &&
+					$share->getExpirationDate() === null;
+			})
+		);
+
+		$expected = new \OC_OCS_Result(null);
+		$result = $ocs->createShare();
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testCreateShareLinkPassword() {
+		$ocs = $this->mockFormatShare();
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['path', null, 'valid-path'],
+				['shareType', '-1', \OCP\Share::SHARE_TYPE_LINK],
+				['publicUpload', null, 'false'],
+				['expireDate', '', ''],
+				['password', '', 'password'],
+			]));
+
+		$path = $this->getMock('\OCP\Files\Folder');
+		$this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf());
+		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+
+		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$currentUser = $this->currentUser;
+
+		$this->shareManager->expects($this->once())->method('createShare')->with(
+			$this->callback(function (IShare $share) use ($path, $currentUser) {
+				return $share->getPath() === $path &&
+				$share->getShareType() === \OCP\Share::SHARE_TYPE_LINK &&
+				$share->getPermissions() === \OCP\Constants::PERMISSION_READ &&
+				$share->getSharedBy() === $currentUser &&
+				$share->getPassword() === 'password' &&
+				$share->getExpirationDate() === null;
+			})
+		);
+
+		$expected = new \OC_OCS_Result(null);
+		$result = $ocs->createShare();
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testCreateShareValidExpireDate() {
+		$ocs = $this->mockFormatShare();
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['path', null, 'valid-path'],
+				['shareType', '-1', \OCP\Share::SHARE_TYPE_LINK],
+				['publicUpload', null, 'false'],
+				['expireDate', '', '2000-01-01'],
+				['password', '', ''],
+			]));
+
+		$path = $this->getMock('\OCP\Files\Folder');
+		$this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf());
+		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+
+		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$currentUser = $this->currentUser;
+
+		$this->shareManager->expects($this->once())->method('createShare')->with(
+			$this->callback(function (IShare $share) use ($path, $currentUser) {
+				$date = new \DateTime('2000-01-01');
+				$date->setTime(0,0,0);
+
+				return $share->getPath() === $path &&
+				$share->getShareType() === \OCP\Share::SHARE_TYPE_LINK &&
+				$share->getPermissions() === \OCP\Constants::PERMISSION_READ &&
+				$share->getSharedBy() === $currentUser &&
+				$share->getPassword() === null &&
+				$share->getExpirationDate() == $date;
+			})
+		);
+
+		$expected = new \OC_OCS_Result(null);
+		$result = $ocs->createShare();
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testCreateShareInvalidExpireDate() {
+		$ocs = $this->mockFormatShare();
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['path', null, 'valid-path'],
+				['shareType', '-1', \OCP\Share::SHARE_TYPE_LINK],
+				['publicUpload', null, 'false'],
+				['expireDate', '', 'a1b2d3'],
+				['password', '', ''],
+			]));
+
+		$path = $this->getMock('\OCP\Files\Folder');
+		$this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf());
+		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+
+		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$expected = new \OC_OCS_Result(null, 404, 'Invalid Date. Format must be YYYY-MM-DD.');
+		$result = $ocs->createShare();
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
 }

From b321ceef60d98f3c8224c6bacc402c1ec1d08920 Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <rullzer@owncloud.com>
Date: Wed, 27 Jan 2016 16:46:48 +0100
Subject: [PATCH 2/4] [Share 2.0] Also handle empty parameter in updateShare

* More sanity checks
* More unit tests
---
 apps/files_sharing/api/share20ocs.php         |  73 ++++--
 .../tests/api/share20ocstest.php              | 238 ++++++++++++++++++
 2 files changed, 285 insertions(+), 26 deletions(-)

diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php
index 081c3b16999..d49ef7ad45f 100644
--- a/apps/files_sharing/api/share20ocs.php
+++ b/apps/files_sharing/api/share20ocs.php
@@ -452,41 +452,62 @@ class Share20OCS {
 		}
 
 		if (!$this->canAccessShare($share)) {
-			return new \OC_OCS_Result(null, 404, "wrong share Id, share doesn't exist.");
+			return new \OC_OCS_Result(null, 404, 'wrong share Id, share doesn\'t exist.');
 		}
 
 		$permissions = $this->request->getParam('permissions', null);
-		$password = $this->request->getParam('password', null);
+		$password = $this->request->getParam('password', '');
 		$publicUpload = $this->request->getParam('publicUpload', null);
-		$expireDate = $this->request->getParam('expireDate', null);
+		$expireDate = $this->request->getParam('expireDate', '');
 
-		if ($permissions === null && $password === null && $publicUpload === null && $expireDate === null) {
-			return new \OC_OCS_Result(null, 400, 'Wrong or no update parameter given');
-		}
-
-		if ($expireDate !== null) {
-			try {
-				$expireDate = $this->parseDate($expireDate);
-			} catch (\Exception $e) {
-				return new \OC_OCS_Result(null, 400, $e->getMessage());
+		/*
+		 * expirationdate, password and publicUpload only make sense for link shares
+		 */
+		if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
+			if ($password === null && $publicUpload === null && $expireDate === null) {
+				return new \OC_OCS_Result(null, 400, 'Wrong or no update parameter given');
+			}
+
+			if ($expireDate === '') {
+				$share->setExpirationDate(null);
+			} else {
+				try {
+					$expireDate = $this->parseDate($expireDate);
+				} catch (\Exception $e) {
+					return new \OC_OCS_Result(null, 400, $e->getMessage());
+				}
+				$share->setExpirationDate($expireDate);
+			}
+
+			if ($password === '') {
+				$share->setPassword(null);
+			} else {
+				$share->setPassword($password);
+			}
+
+			if ($publicUpload === 'true') {
+				if(!$this->shareManager->shareApiLinkAllowPublicUpload()) {
+					return new \OC_OCS_Result(null, 403, "public upload disabled by the administrator");
+				}
+
+				if (!($share->getPath() instanceof \OCP\Files\Folder)) {
+					return new \OC_OCS_Result(null, 400, "public upload is only possible for public shared folders");
+				}
+
+				$share->setPermissions(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE);
+			} else if ($publicUpload === 'false') {
+				$share->setPermissions(\OCP\Constants::PERMISSION_READ);
+			}
+		} else {
+			if ($permissions === null) {
+				return new \OC_OCS_Result(null, 400, 'Wrong or no update parameter given');
+			} else {
+				$permissions = (int)$permissions;
+				$share->setPermissions($permissions);
 			}
-			$share->setExpirationDate($expireDate);
 		}
 
-		if ($permissions !== null) {
-			$permissions = (int)$permissions;
-			$share->setPermissions($permissions);
-		}
 
-		if ($password !== null) {
-			$share->setPassword($password);
-		}
-
-		if ($publicUpload === 'true') {
-			$share->setPermissions(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE);
-		} else if ($publicUpload === 'false') {
-			$share->setPermissions(\OCP\Constants::PERMISSION_READ);
-		}
 
 		try {
 			$share = $this->shareManager->updateShare($share);
diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php
index 4bbcfb5a600..ce21e9675e2 100644
--- a/apps/files_sharing/tests/api/share20ocstest.php
+++ b/apps/files_sharing/tests/api/share20ocstest.php
@@ -932,4 +932,242 @@ class Share20OCSTest extends \Test\TestCase {
 		$this->assertEquals($expected->getMeta(), $result->getMeta());
 		$this->assertEquals($expected->getData(), $result->getData());
 	}
+
+	public function testUpdateShareCantAccess() {
+		$share = \OC::$server->getShareManager()->newShare();
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+
+		$expected = new \OC_OCS_Result(null, 404, 'wrong share Id, share doesn\'t exist.');
+		$result = $this->ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateNoParametersLink() {
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK);
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+
+		$expected = new \OC_OCS_Result(null, 400, 'Wrong or no update parameter given');
+		$result = $this->ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateNoParametersOther() {
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_GROUP);
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+
+		$expected = new \OC_OCS_Result(null, 400, 'Wrong or no update parameter given');
+		$result = $this->ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateLinkShareClear() {
+		$ocs = $this->mockFormatShare();
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPassword('password')
+			->setExpirationDate(new \DateTime())
+			->setPermissions(\OCP\Constants::PERMISSION_ALL);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['publicUpload', null, 'false'],
+				['expireDate', '', ''],
+				['password', '', ''],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+
+		$this->shareManager->expects($this->once())->method('updateShare')->with(
+			$this->callback(function (IShare $share) {
+				return $share->getPermissions() === \OCP\Constants::PERMISSION_READ &&
+				$share->getPassword() === null &&
+				$share->getExpirationDate() === null;
+			})
+		);
+
+		$expected = new \OC_OCS_Result(null);
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateLinkShareSet() {
+		$ocs = $this->mockFormatShare();
+
+		$folder = $this->getMock('\OCP\Files\Folder');
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPath($folder);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['publicUpload', null, 'true'],
+				['expireDate', '', '2000-01-01'],
+				['password', '', 'password'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$this->shareManager->expects($this->once())->method('updateShare')->with(
+			$this->callback(function (IShare $share) {
+				$date = new \DateTime('2000-01-01');
+				$date->setTime(0,0,0);
+
+				return $share->getPermissions() === \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE && \OCP\Constants::PERMISSION_DELETE &&
+				$share->getPassword() === 'password' &&
+				$share->getExpirationDate() == $date;
+			})
+		);
+
+		$expected = new \OC_OCS_Result(null);
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateLinkShareInvalidDate() {
+		$ocs = $this->mockFormatShare();
+
+		$folder = $this->getMock('\OCP\Files\Folder');
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPath($folder);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['publicUpload', null, 'true'],
+				['expireDate', '', '2000-01-a'],
+				['password', '', 'password'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$expected = new \OC_OCS_Result(null, 400, 'Invalid date. Format must be YYYY-MM-DD');
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateLinkSharePublicUploadNotAllowed() {
+		$ocs = $this->mockFormatShare();
+
+		$folder = $this->getMock('\OCP\Files\Folder');
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPath($folder);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['publicUpload', null, 'true'],
+				['expireDate', '', ''],
+				['password', '', 'password'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(false);
+
+		$expected = new \OC_OCS_Result(null, 403, 'public upload disabled by the administrator');
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateLinkSharePublicUploadOnFile() {
+		$ocs = $this->mockFormatShare();
+
+		$file = $this->getMock('\OCP\Files\File');
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPath($file);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['publicUpload', null, 'true'],
+				['expireDate', '', ''],
+				['password', '', 'password'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$expected = new \OC_OCS_Result(null, 400, 'public upload is only possible for public shared folders');
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateOtherPermissions() {
+		$ocs = $this->mockFormatShare();
+
+		$file = $this->getMock('\OCP\Files\File');
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_USER)
+			->setPath($file);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['permissions', null, '31'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$this->shareManager->expects($this->once())->method('updateShare')->with(
+			$this->callback(function (IShare $share) {
+				return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL;
+			})
+		);
+
+		$expected = new \OC_OCS_Result(null);
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
 }

From f5c45dfe7b2aecbeb9b7193ce14bc28511006d18 Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <rullzer@owncloud.com>
Date: Wed, 27 Jan 2016 17:09:02 +0100
Subject: [PATCH 3/4] [Share 2.0] Still allow isolated updates

Still allow isolated updates of parameters
---
 apps/files_sharing/api/share20ocs.php         |   8 +-
 .../tests/api/share20ocstest.php              | 128 +++++++++++++++++-
 2 files changed, 125 insertions(+), 11 deletions(-)

diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php
index d49ef7ad45f..48aca9b9c1b 100644
--- a/apps/files_sharing/api/share20ocs.php
+++ b/apps/files_sharing/api/share20ocs.php
@@ -456,9 +456,9 @@ class Share20OCS {
 		}
 
 		$permissions = $this->request->getParam('permissions', null);
-		$password = $this->request->getParam('password', '');
+		$password = $this->request->getParam('password', null);
 		$publicUpload = $this->request->getParam('publicUpload', null);
-		$expireDate = $this->request->getParam('expireDate', '');
+		$expireDate = $this->request->getParam('expireDate', null);
 
 		/*
 		 * expirationdate, password and publicUpload only make sense for link shares
@@ -470,7 +470,7 @@ class Share20OCS {
 
 			if ($expireDate === '') {
 				$share->setExpirationDate(null);
-			} else {
+			} else if ($expireDate !== null) {
 				try {
 					$expireDate = $this->parseDate($expireDate);
 				} catch (\Exception $e) {
@@ -481,7 +481,7 @@ class Share20OCS {
 
 			if ($password === '') {
 				$share->setPassword(null);
-			} else {
+			} else if ($password !== null) {
 				$share->setPassword($password);
 			}
 
diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php
index ce21e9675e2..b440ba96e04 100644
--- a/apps/files_sharing/tests/api/share20ocstest.php
+++ b/apps/files_sharing/tests/api/share20ocstest.php
@@ -990,8 +990,8 @@ class Share20OCSTest extends \Test\TestCase {
 			->method('getParam')
 			->will($this->returnValueMap([
 				['publicUpload', null, 'false'],
-				['expireDate', '', ''],
-				['password', '', ''],
+				['expireDate', null, ''],
+				['password', null, ''],
 			]));
 
 		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
@@ -1026,8 +1026,8 @@ class Share20OCSTest extends \Test\TestCase {
 			->method('getParam')
 			->will($this->returnValueMap([
 				['publicUpload', null, 'true'],
-				['expireDate', '', '2000-01-01'],
-				['password', '', 'password'],
+				['expireDate', null, '2000-01-01'],
+				['password', null, 'password'],
 			]));
 
 		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
@@ -1066,8 +1066,8 @@ class Share20OCSTest extends \Test\TestCase {
 			->method('getParam')
 			->will($this->returnValueMap([
 				['publicUpload', null, 'true'],
-				['expireDate', '', '2000-01-a'],
-				['password', '', 'password'],
+				['expireDate', null, '2000-01-a'],
+				['password', null, 'password'],
 			]));
 
 		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
@@ -1095,7 +1095,7 @@ class Share20OCSTest extends \Test\TestCase {
 			->method('getParam')
 			->will($this->returnValueMap([
 				['publicUpload', null, 'true'],
-				['expireDate', '', ''],
+				['expireDate', '', null],
 				['password', '', 'password'],
 			]));
 
@@ -1138,6 +1138,120 @@ class Share20OCSTest extends \Test\TestCase {
 		$this->assertEquals($expected->getData(), $result->getData());
 	}
 
+	public function testUpdateLinkSharePasswordDoesNotChangeOther() {
+		$ocs = $this->mockFormatShare();
+
+		$date = new \DateTime('2000-01-01');
+		$date->setTime(0,0,0);
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPassword('password')
+			->setExpirationDate($date)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['password', null, 'newpassword'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+
+		$this->shareManager->expects($this->once())->method('updateShare')->with(
+			$this->callback(function (IShare $share) use ($date) {
+				return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL &&
+				$share->getPassword() === 'newpassword' &&
+				$share->getExpirationDate() === $date;
+			})
+		);
+
+		$expected = new \OC_OCS_Result(null);
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateLinkShareExpireDateDoesNotChangeOther() {
+		$ocs = $this->mockFormatShare();
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPassword('password')
+			->setExpirationDate(new \DateTime())
+			->setPermissions(\OCP\Constants::PERMISSION_ALL);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['expireDate', null, '2010-12-23'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+
+		$this->shareManager->expects($this->once())->method('updateShare')->with(
+			$this->callback(function (IShare $share) {
+				$date = new \DateTime('2010-12-23');
+				$date->setTime(0,0,0);
+
+				return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL &&
+				$share->getPassword() === 'password' &&
+				$share->getExpirationDate() == $date;
+			})
+		);
+
+		$expected = new \OC_OCS_Result(null);
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateLinkSharePublicUploadDoesNotChangeOther() {
+		$ocs = $this->mockFormatShare();
+
+		$date = new \DateTime('2000-01-01');
+
+		$folder = $this->getMock('\OCP\Files\Folder');
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPassword('password')
+			->setExpirationDate($date)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setPath($folder);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['publicUpload', null, 'true'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$this->shareManager->expects($this->once())->method('updateShare')->with(
+			$this->callback(function (IShare $share) use ($date) {
+				return $share->getPermissions() === \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_DELETE &&
+				$share->getPassword() === 'password' &&
+				$share->getExpirationDate() === $date;
+			})
+		);
+
+		$expected = new \OC_OCS_Result(null);
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
 	public function testUpdateOtherPermissions() {
 		$ocs = $this->mockFormatShare();
 

From 6957917b2038f71e4f87e8f3074e6ba81dd250b7 Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <rullzer@owncloud.com>
Date: Wed, 27 Jan 2016 20:32:04 +0100
Subject: [PATCH 4/4] [Share 2.0] Allow using permissions to update link share

---
 apps/files_sharing/api/share20ocs.php         | 55 ++++++++++----
 .../tests/api/share20ocstest.php              | 72 +++++++++++++++++++
 2 files changed, 113 insertions(+), 14 deletions(-)

diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php
index 48aca9b9c1b..1d84aefa764 100644
--- a/apps/files_sharing/api/share20ocs.php
+++ b/apps/files_sharing/api/share20ocs.php
@@ -464,10 +464,41 @@ class Share20OCS {
 		 * expirationdate, password and publicUpload only make sense for link shares
 		 */
 		if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
-			if ($password === null && $publicUpload === null && $expireDate === null) {
+			if ($permissions === null && $password === null && $publicUpload === null && $expireDate === null) {
 				return new \OC_OCS_Result(null, 400, 'Wrong or no update parameter given');
 			}
 
+			$newPermissions = null;
+			if ($publicUpload === 'true') {
+				$newPermissions = \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE;
+			} else if ($publicUpload === 'false') {
+				$newPermissions = \OCP\Constants::PERMISSION_READ;
+			}
+
+			if ($permissions !== null) {
+				$newPermissions = (int)$permissions;
+			}
+
+			if ($newPermissions !== null &&
+				$newPermissions !== \OCP\Constants::PERMISSION_READ &&
+				$newPermissions !== (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE)) {
+				return new \OC_OCS_Result(null, 400, 'can\'t change permission for public link share');
+			}
+
+			if ($newPermissions === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE)) {
+				if (!$this->shareManager->shareApiLinkAllowPublicUpload()) {
+					return new \OC_OCS_Result(null, 403, 'public upload disabled by the administrator');
+				}
+
+				if (!($share->getPath() instanceof \OCP\Files\Folder)) {
+					return new \OC_OCS_Result(null, 400, "public upload is only possible for public shared folders");
+				}
+			}
+
+			if ($newPermissions !== null) {
+				$share->setPermissions($newPermissions);
+			}
+
 			if ($expireDate === '') {
 				$share->setExpirationDate(null);
 			} else if ($expireDate !== null) {
@@ -485,20 +516,8 @@ class Share20OCS {
 				$share->setPassword($password);
 			}
 
-			if ($publicUpload === 'true') {
-				if(!$this->shareManager->shareApiLinkAllowPublicUpload()) {
-					return new \OC_OCS_Result(null, 403, "public upload disabled by the administrator");
-				}
-
-				if (!($share->getPath() instanceof \OCP\Files\Folder)) {
-					return new \OC_OCS_Result(null, 400, "public upload is only possible for public shared folders");
-				}
-
-				$share->setPermissions(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE);
-			} else if ($publicUpload === 'false') {
-				$share->setPermissions(\OCP\Constants::PERMISSION_READ);
-			}
 		} else {
+			// For other shares only permissions is valid.
 			if ($permissions === null) {
 				return new \OC_OCS_Result(null, 400, 'Wrong or no update parameter given');
 			} else {
@@ -518,6 +537,14 @@ class Share20OCS {
 		return new \OC_OCS_Result($this->formatShare($share));
 	}
 
+	public function validatePermissions($permissions) {
+		if ($permissions < 0 || $permissions > \OCP\Constants::PERMISSION_ALL) {
+			return false;
+		}
+
+
+	}
+
 	/**
 	 * @param IShare $share
 	 * @return bool
diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php
index b440ba96e04..18f05b7867d 100644
--- a/apps/files_sharing/tests/api/share20ocstest.php
+++ b/apps/files_sharing/tests/api/share20ocstest.php
@@ -1252,6 +1252,78 @@ class Share20OCSTest extends \Test\TestCase {
 		$this->assertEquals($expected->getData(), $result->getData());
 	}
 
+	public function testUpdateLinkSharePermissions() {
+		$ocs = $this->mockFormatShare();
+
+		$date = new \DateTime('2000-01-01');
+
+		$folder = $this->getMock('\OCP\Files\Folder');
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPassword('password')
+			->setExpirationDate($date)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setPath($folder);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['permissions', null, '7'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$this->shareManager->expects($this->once())->method('updateShare')->with(
+			$this->callback(function (IShare $share) use ($date) {
+				return $share->getPermissions() === \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_DELETE &&
+				$share->getPassword() === 'password' &&
+				$share->getExpirationDate() === $date;
+			})
+		);
+
+		$expected = new \OC_OCS_Result(null);
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testUpdateLinkShareInvalidPermissions() {
+		$ocs = $this->mockFormatShare();
+
+		$date = new \DateTime('2000-01-01');
+
+		$folder = $this->getMock('\OCP\Files\Folder');
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPassword('password')
+			->setExpirationDate($date)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setPath($folder);
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['permissions', null, '31'],
+			]));
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+
+		$expected = new \OC_OCS_Result(null, 400, 'can\'t change permission for public link share');
+		$result = $ocs->updateShare(42);
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
 	public function testUpdateOtherPermissions() {
 		$ocs = $this->mockFormatShare();