OCS Fixes to allow setting of password without removing additional settings

- Added setPassword to share.php
- Fixed OCS API call
- Added unit tests

apps/files_sharing/api/local.php

@@ -343,7 +343,7 @@ class Local {
 			if(isset($params['_put']['permissions'])) {
 				return self::updatePermissions($share, $params);
 			} elseif (isset($params['_put']['password'])) {
-				return self::updatePassword($share, $params);
+				return self::updatePassword($params['id'], (int)$share['share_type'], $params['_put']['password']);
 			} elseif (isset($params['_put']['publicUpload'])) {
 				return self::updatePublicUpload($share, $params);
 			} elseif (isset($params['_put']['expireDate'])) {
@@ -457,47 +457,22 @@ class Local {
 
 	/**
 	 * update password for public link share
-	 * @param array $share information about the share
+	 * @param int $shareId
-	 * @param array $params 'password'
+	 * @param int $shareType 
+	 * @param string $password
 	 * @return \OC_OCS_Result
 	 */
-	private static function updatePassword($share, $params) {
+	private static function updatePassword($shareId, $shareType, $password) {
-
+		if($shareType !== \OCP\Share::SHARE_TYPE_LINK) {
-		$itemSource = $share['item_source'];
-		$itemType = $share['item_type'];
-
-		if( (int)$share['share_type'] !== \OCP\Share::SHARE_TYPE_LINK) {
 			return  new \OC_OCS_Result(null, 400, "password protection is only supported for public shares");
 		}
 
-		$shareWith = isset($params['_put']['password']) ? $params['_put']['password'] : null;
+		if($password === '') {
-
+			$password = null;
-		if($shareWith === '') {
-			$shareWith = null;
-		}
-
-		$items = \OCP\Share::getItemShared($itemType, $itemSource);
-
-		$checkExists = false;
-		foreach ($items as $item) {
-			if($item['share_type'] === \OCP\Share::SHARE_TYPE_LINK) {
-				$checkExists = true;
-				$permissions = $item['permissions'];
-			}
-		}
-
-		if (!$checkExists) {
-			return  new \OC_OCS_Result(null, 404, "share doesn't exists, can't change password");
 		}
 
 		try {
-			$result = \OCP\Share::shareItem(
+			$result = \OCP\Share::setPassword($shareId, $password);
-					$itemType,
-					$itemSource,
-					\OCP\Share::SHARE_TYPE_LINK,
-					$shareWith,
-					$permissions
-					);
 		} catch (\Exception $e) {
 			return new \OC_OCS_Result(null, 403, $e->getMessage());
 		}

lib/private/share/share.php

@@ -37,6 +37,10 @@
 
 namespace OC\Share;
 
+use OCP\IUserSession;
+use OC\DB\Connection;
+use OCP\IConfig;
+
 /**
  * This class provides the ability for apps to share their content between users.
  * Apps must create a backend class that implements OCP\Share_Backend and register it with this class.
@@ -1150,6 +1154,74 @@ class Share extends \OC\Share\Constants {
 		return true;
 	}
 
+	/**
+	 * Retrieve the owner of a connection
+	 *
+	 * @param Connection $connection
+	 * @param int $shareId
+	 * @throws \Exception
+	 * @return string uid of share owner
+	 */
+	private static function getShareOwner(Connection $connection, $shareId) {
+		$qb = $connection->createQueryBuilder();
+
+		$qb->select('`uid_owner`')
+			->from('`*PREFIX*share`')
+			->where($qb->expr()->eq('`id`', $shareId));
+		$result = $qb->execute();
+		$result = $result->fetch();
+
+		if (empty($result)) {
+			throw new \Exception('Share not found');
+		}
+
+		return $result['uid_owner'];
+	}
+
+	/**
+	 * Set expiration date for a share
+	 *
+	 * @param IUserSession $userSession
+	 * @param Connection $connection
+	 * @param IConfig $config
+	 * @param int $shareId
+	 * @param string $password
+	 * @throws \Exception
+	 * @return boolean
+	 */
+	public static function setPassword(IUserSession $userSession, 
+	                                   Connection $connection,
+	                                   IConfig $config,
+	                                   $shareId, $password) {
+		$user = $userSession->getUser();
+		if (is_null($user)) {
+			throw new \Exception("User not logged in");
+		}
+
+		$uid = self::getShareOwner($connection, $shareId);
+
+		if ($uid !== $user->getUID()) {
+			throw new \Exception('Cannot update share of a different user');
+		}
+
+		if ($password === '') {
+			$password = null;
+		}
+
+		//If passwords are enforced the password can't be null
+		if (self::enforcePassword($config) && is_null($password)) {
+			throw new \Exception('Cannot remove password');
+		}
+
+		$qb = $connection->createQueryBuilder();
+		$qb->update('`*PREFIX*share`')
+			->set('`share_with`', is_null($password) ? 'NULL' : $qb->expr()->literal(\OC::$server->getHasher()->hash($password)))
+			->where($qb->expr()->eq('`id`', $shareId));
+		$qb->execute();
+
+		return true;
+	}
+
 	/**
 	 * Checks whether a share has expired, calls unshareItem() if yes.
 	 * @param array $item Share data (usually database row)
@@ -2429,4 +2501,12 @@ class Share extends \OC\Share\Constants {
 		return false;
 	}
 
+	/**
+	 * @param IConfig $config
+	 * @return bool 
+	 */
+	public static function enforcePassword(IConfig $config) {
+		$enforcePassword = $config->getAppValue('core', 'shareapi_enforce_links_password', 'no');
+		return ($enforcePassword === "yes") ? true : false;
+	}
 }

lib/public/share.php

@@ -317,6 +317,20 @@ class Share extends \OC\Share\Constants {
 		return \OC\Share\Share::setExpirationDate($itemType, $itemSource, $date, $shareTime);
 	}
 
+	/**
+	 * Set expiration date for a share
+	 * @param int $shareId
+	 * @param string $password
+	 * @return boolean
+	 */
+	public static function setPassword($shareId, $password) {
+		$userSession = \OC::$server->getUserSession();
+		$connection = \OC::$server->getDatabaseConnection();
+		$config = \OC::$server->getConfig();
+		return \OC\Share\Share::setPassword($userSession, $connection, $config, $shareId, $password);
+	}
+
+
 	/**
 	 * Get the backend class for the specified item type
 	 * @param string $itemType

tests/lib/share/share.php

@@ -1128,6 +1128,236 @@ class Test_Share extends \Test\TestCase {
 		\OC_Appconfig::deleteKey('core', 'shareapi_expire_after_n_days');
 		\OC_Appconfig::deleteKey('core', 'shareapi_enforce_expire_date');
 	}
+
+	/**
+	 * Cannot set password is there is no user
+	 *
+	 * @expectedException Exception
+	 * @expectedExceptionMessage User not logged in
+	 */
+	public function testSetPasswordNoUser() {
+		$userSession = $this->getMockBuilder('\OCP\IUserSession')
+		                    ->disableOriginalConstructor()
+		                    ->getMock();
+
+		$connection  = $this->getMockBuilder('\OC\DB\Connection')
+		                    ->disableOriginalConstructor()
+		                    ->getMock();
+
+		$config = $this->getMockBuilder('\OCP\IConfig')
+		               ->disableOriginalConstructor()
+		               ->getMock();
+
+		\OC\Share\Share::setPassword($userSession, $connection, $config, 1, 'pass');
+	}
+
+	/**
+	 * Test setting a password when everything is fine
+	 */
+	public function testSetPassword() {
+		$user = $this->getMockBuilder('\OCP\IUser')
+		             ->disableOriginalConstructor()
+		             ->getMock();
+		$user->method('getUID')->willReturn('user');
+
+		$userSession = $this->getMockBuilder('\OCP\IUserSession')
+		                    ->disableOriginalConstructor()
+		                    ->getMock();
+		$userSession->method('getUser')->willReturn($user);
+
+
+		$ex = $this->getMockBuilder('\Doctrine\DBAL\Query\Expression\ExpressionBuilder')
+		           ->disableOriginalConstructor()
+		           ->getMock();
+		$qb = $this->getMockBuilder('\Doctrine\DBAL\Query\QueryBuilder')
+		           ->disableOriginalConstructor()
+		           ->getMock();
+		$qb->method('update')->will($this->returnSelf());
+		$qb->method('set')->will($this->returnSelf());
+		$qb->method('where')->will($this->returnSelf());
+		$qb->method('andWhere')->will($this->returnSelf());
+		$qb->method('select')->will($this->returnSelf());
+		$qb->method('from')->will($this->returnSelf());
+		$qb->method('expr')->willReturn($ex);
+
+		$ret = $this->getMockBuilder('\Doctrine\DBAL\Driver\ResultStatement')
+		            ->disableOriginalConstructor()
+					->getMock();
+		$ret->method('fetch')->willReturn(['uid_owner' => 'user']);
+		$qb->method('execute')->willReturn($ret);
+
+
+		$connection  = $this->getMockBuilder('\OC\DB\Connection')
+		                    ->disableOriginalConstructor()
+		                    ->getMock();
+		$connection->method('createQueryBuilder')->willReturn($qb);
+
+		$config = $this->getMockBuilder('\OCP\IConfig')
+		               ->disableOriginalConstructor()
+		               ->getMock();
+
+
+		$res = \OC\Share\Share::setPassword($userSession, $connection, $config, 1, 'pass');
+
+		$this->assertTrue($res);
+	}
+
+	/**
+	 * @expectedException Exception
+	 * @expectedExceptionMessage Cannot remove password
+	 *
+	 * Test removing a password when password is enforced
+	 */
+	public function testSetPasswordRemove() {
+		$user = $this->getMockBuilder('\OCP\IUser')
+		             ->disableOriginalConstructor()
+		             ->getMock();
+		$user->method('getUID')->willReturn('user');
+
+		$userSession = $this->getMockBuilder('\OCP\IUserSession')
+		                    ->disableOriginalConstructor()
+		                    ->getMock();
+		$userSession->method('getUser')->willReturn($user);
+
+
+		$ex = $this->getMockBuilder('\Doctrine\DBAL\Query\Expression\ExpressionBuilder')
+		           ->disableOriginalConstructor()
+		           ->getMock();
+		$qb = $this->getMockBuilder('\Doctrine\DBAL\Query\QueryBuilder')
+		           ->disableOriginalConstructor()
+		           ->getMock();
+		$qb->method('update')->will($this->returnSelf());
+		$qb->method('select')->will($this->returnSelf());
+		$qb->method('from')->will($this->returnSelf());
+		$qb->method('set')->will($this->returnSelf());
+		$qb->method('where')->will($this->returnSelf());
+		$qb->method('andWhere')->will($this->returnSelf());
+		$qb->method('expr')->willReturn($ex);
+
+		$ret = $this->getMockBuilder('\Doctrine\DBAL\Driver\ResultStatement')
+		            ->disableOriginalConstructor()
+					->getMock();
+		$ret->method('fetch')->willReturn(['uid_owner' => 'user']);
+		$qb->method('execute')->willReturn($ret);
+
+
+		$connection  = $this->getMockBuilder('\OC\DB\Connection')
+		                    ->disableOriginalConstructor()
+		                    ->getMock();
+		$connection->method('createQueryBuilder')->willReturn($qb);
+
+		$config = $this->getMockBuilder('\OCP\IConfig')
+		               ->disableOriginalConstructor()
+		               ->getMock();
+		$config->method('getAppValue')->willReturn('yes');
+
+		\OC\Share\Share::setPassword($userSession, $connection, $config, 1, '');
+	}
+
+	/**
+	 * @expectedException Exception
+	 * @expectedExceptionMessage Share not found
+	 *
+	 * Test modification of invaid share
+	 */
+	public function testSetPasswordInvalidShare() {
+		$user = $this->getMockBuilder('\OCP\IUser')
+		             ->disableOriginalConstructor()
+		             ->getMock();
+		$user->method('getUID')->willReturn('user');
+
+		$userSession = $this->getMockBuilder('\OCP\IUserSession')
+		                    ->disableOriginalConstructor()
+		                    ->getMock();
+		$userSession->method('getUser')->willReturn($user);
+
+
+		$ex = $this->getMockBuilder('\Doctrine\DBAL\Query\Expression\ExpressionBuilder')
+		           ->disableOriginalConstructor()
+		           ->getMock();
+		$qb = $this->getMockBuilder('\Doctrine\DBAL\Query\QueryBuilder')
+		           ->disableOriginalConstructor()
+		           ->getMock();
+		$qb->method('update')->will($this->returnSelf());
+		$qb->method('set')->will($this->returnSelf());
+		$qb->method('where')->will($this->returnSelf());
+		$qb->method('andWhere')->will($this->returnSelf());
+		$qb->method('select')->will($this->returnSelf());
+		$qb->method('from')->will($this->returnSelf());
+		$qb->method('expr')->willReturn($ex);
+
+		$ret = $this->getMockBuilder('\Doctrine\DBAL\Driver\ResultStatement')
+		            ->disableOriginalConstructor()
+					->getMock();
+		$ret->method('fetch')->willReturn([]);
+		$qb->method('execute')->willReturn($ret);
+
+
+		$connection  = $this->getMockBuilder('\OC\DB\Connection')
+		                    ->disableOriginalConstructor()
+		                    ->getMock();
+		$connection->method('createQueryBuilder')->willReturn($qb);
+
+		$config = $this->getMockBuilder('\OCP\IConfig')
+		               ->disableOriginalConstructor()
+		               ->getMock();
+
+
+		\OC\Share\Share::setPassword($userSession, $connection, $config, 1, 'pass');
+	}
+
+	/**
+	 * @expectedException Exception
+	 * @expectedExceptionMessage Cannot update share of a different user
+	 *
+	 * Test modification of share of another user
+	 */
+	public function testSetPasswordShareOtherUser() {
+		$user = $this->getMockBuilder('\OCP\IUser')
+		             ->disableOriginalConstructor()
+		             ->getMock();
+		$user->method('getUID')->willReturn('user');
+
+		$userSession = $this->getMockBuilder('\OCP\IUserSession')
+		                    ->disableOriginalConstructor()
+		                    ->getMock();
+		$userSession->method('getUser')->willReturn($user);
+
+
+		$ex = $this->getMockBuilder('\Doctrine\DBAL\Query\Expression\ExpressionBuilder')
+		           ->disableOriginalConstructor()
+		           ->getMock();
+		$qb = $this->getMockBuilder('\Doctrine\DBAL\Query\QueryBuilder')
+		           ->disableOriginalConstructor()
+		           ->getMock();
+		$qb->method('update')->will($this->returnSelf());
+		$qb->method('set')->will($this->returnSelf());
+		$qb->method('where')->will($this->returnSelf());
+		$qb->method('andWhere')->will($this->returnSelf());
+		$qb->method('select')->will($this->returnSelf());
+		$qb->method('from')->will($this->returnSelf());
+		$qb->method('expr')->willReturn($ex);
+
+		$ret = $this->getMockBuilder('\Doctrine\DBAL\Driver\ResultStatement')
+		            ->disableOriginalConstructor()
+					->getMock();
+		$ret->method('fetch')->willReturn(['uid_owner' => 'user2']);
+		$qb->method('execute')->willReturn($ret);
+
+
+		$connection  = $this->getMockBuilder('\OC\DB\Connection')
+		                    ->disableOriginalConstructor()
+		                    ->getMock();
+		$connection->method('createQueryBuilder')->willReturn($qb);
+
+		$config = $this->getMockBuilder('\OCP\IConfig')
+		               ->disableOriginalConstructor()
+		               ->getMock();
+
+
+		\OC\Share\Share::setPassword($userSession, $connection, $config, 1, 'pass');
+	}
+
 }
 
 class DummyShareClass extends \OC\Share\Share {