Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow updating of password on public share #14868

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 1 addition & 8 deletions apps/files_sharing/api/local.php
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,6 @@ private static function updatePassword($share, $params) {
foreach ($items as $item) {
if($item['share_type'] === \OCP\Share::SHARE_TYPE_LINK) {
$checkExists = true;
$permissions = $item['permissions'];
}
}

Expand All @@ -487,13 +486,7 @@ private static function updatePassword($share, $params) {
}

try {
$result = \OCP\Share::shareItem(
$itemType,
$itemSource,
\OCP\Share::SHARE_TYPE_LINK,
$shareWith,
$permissions
);
$result = \OCP\Share::setPassword($itemType, $itemSource, $shareWith);
} catch (\Exception $e) {
return new \OC_OCS_Result(null, 403, $e->getMessage());
}
Expand Down
10 changes: 10 additions & 0 deletions core/ajax/share.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,16 @@
}
}
break;
case 'setPassword':
if (isset($_POST['shareWith'])) {
try {
$return = OCP\Share::setPassword((string)$_POST['itemType'], (string)$_POST['itemSource'], (string)$_POST['shareWith']);
($return) ? OC_JSON::success() : OC_JSON::error();
} catch (\Exception $e) {
OC_JSON::error(array('data' => array('message' => $e->getMessage())));
}
}
break;
case 'informRecipients':
$l = \OC::$server->getL10N('core');
$shareType = (int) $_POST['shareType'];
Expand Down
41 changes: 27 additions & 14 deletions core/js/share.js
Original file line number Diff line number Diff line change
Expand Up @@ -1080,10 +1080,17 @@ $(document).ready(function() {
}

$loading.removeClass('hidden');
OC.Share.share(itemType, itemSource, OC.Share.SHARE_TYPE_LINK, '', permissions, itemSourceName).then(function() {
$loading.addClass('hidden');
$('#linkPassText').attr('placeholder', t('core', 'Choose a password for the public link'));
});
$.post(OC.filePath('core', 'ajax', 'share.php'),
{
action: 'setPassword',
itemType: itemType,
itemSource: itemSource,
shareWith: ''
},
function() {
$loading.addClass('hidden');
$('#linkPassText').attr('placeholder', t('core', 'Choose a password for the public link'));
});
} else {
$('#linkPassText').focus();
}
Expand All @@ -1109,17 +1116,23 @@ $(document).ready(function() {
}

$loading.removeClass('hidden');
OC.Share.share(itemType, itemSource, OC.Share.SHARE_TYPE_LINK, $('#linkPassText').val(), permissions, itemSourceName, function(data) {
$loading.addClass('hidden');
linkPassText.val('');
linkPassText.attr('placeholder', t('core', 'Password protected'));

if (oc_appconfig.core.enforcePasswordForPublicLink) {
OC.Share.showLink(data.token, "password set", itemSource);
OC.Share.updateIcon(itemType, itemSource);
}
});
$.post(OC.filePath('core', 'ajax', 'share.php'),
{
action: 'setPassword',
itemType: itemType,
itemSource: itemSource,
shareWith: $('#linkPassText').val()
},
function(data) {
$loading.addClass('hidden');
linkPassText.val('');
linkPassText.attr('placeholder', t('core', 'Password protected'));

if (oc_appconfig.core.enforcePasswordForPublicLink) {
OC.Share.showLink(data.token, "password set", itemSource);
OC.Share.updateIcon(itemType, itemSource);
}
});
}
});

Expand Down
92 changes: 67 additions & 25 deletions lib/private/share/share.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@

namespace OC\Share;

use OCP\IUserSession;
use OCP\IDBConnection;
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.
Expand Down Expand Up @@ -535,6 +539,7 @@ public static function shareItem($itemType, $itemSource, $shareType, $shareWith,

$backend = self::getBackend($itemType);
$l = \OC::$server->getL10N('lib');
$logger = \OC::$server->getLogger();

if ($backend->isShareTypeAllowed($shareType) === false) {
$message = 'Sharing %s failed, because the backend does not allow shares from type %i';
Expand Down Expand Up @@ -660,31 +665,21 @@ public static function shareItem($itemType, $itemSource, $shareType, $shareWith,
$shareWith['group'] = $group;
$shareWith['users'] = array_diff(\OC_Group::usersInGroup($group), array($uidOwner));
} else if ($shareType === self::SHARE_TYPE_LINK) {
$updateExistingShare = false;
if (\OC_Appconfig::getValue('core', 'shareapi_allow_links', 'yes') == 'yes') {

// when updating a link share
// FIXME Don't delete link if we update it
if ($checkExists = self::getItems($itemType, $itemSource, self::SHARE_TYPE_LINK, null,
// Only 1 public share allowed
if (self::getItems($itemType, $itemSource, self::SHARE_TYPE_LINK, null,
$uidOwner, self::FORMAT_NONE, null, 1)) {
// remember old token
$oldToken = $checkExists['token'];
$oldPermissions = $checkExists['permissions'];
//delete the old share
Helper::delete($checkExists['id']);
$updateExistingShare = true;
$message = "Only 1 public share allowed";
$message_t = $l->t("Only 1 public share allowed");
$logger->error($message, ['app' => 'OCP\Share::shareItem']);
throw new \Exception($message_t);
}

// Generate hash of password - same method as user passwords
if (!empty($shareWith)) {
$shareWith = \OC::$server->getHasher()->hash($shareWith);
} else {
// reuse the already set password, but only if we change permissions
// otherwise the user disabled the password protection
if ($checkExists && (int)$permissions !== (int)$oldPermissions) {
$shareWith = $checkExists['share_with'];
}
}
}

if (\OCP\Util::isPublicLinkPasswordRequired() && empty($shareWith)) {
$message = 'You need to provide a password to create a public link, only protected links are allowed';
Expand All @@ -693,21 +688,17 @@ public static function shareItem($itemType, $itemSource, $shareType, $shareWith,
throw new \Exception($message_t);
}

if ($updateExistingShare === false &&
self::isDefaultExpireDateEnabled() &&
if (self::isDefaultExpireDateEnabled() &&
empty($expirationDate)) {
$expirationDate = Helper::calcExpireDate();
}

// Generate token
if (isset($oldToken)) {
$token = $oldToken;
} else {
$token = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(self::TOKEN_LENGTH,
$token = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(self::TOKEN_LENGTH,
\OCP\Security\ISecureRandom::CHAR_LOWER.\OCP\Security\ISecureRandom::CHAR_UPPER.
\OCP\Security\ISecureRandom::CHAR_DIGITS
);
}
);

$result = self::put($itemType, $itemSource, $shareType, $shareWith, $uidOwner, $permissions,
null, $token, $itemSourceName, $expirationDate);
if ($result) {
Expand Down Expand Up @@ -1122,6 +1113,49 @@ public static function setExpirationDate($itemType, $itemSource, $date, $shareTi
return true;
}

/**
* Set expiration date for a share
*
* @param IUserSession $userSession
* @param IDBConnection $connection
* @param IConfig $config
* @param string $itemType
* @param string $itemSource
* @param string $password
* @throws \Exception
* @return boolean
*/
public static function setPassword(IUserSession $userSession,
IDBConnection $connection,
IConfig $config,
$itemType, $itemSource, $password) {
$user = $userSession->getUser();
if (is_null($user)) {
throw new \Exception("User not logged in");
}
$user = $user->getUID();

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('item_type', $qb->expr()->literal($itemType)))
->andWhere($qb->expr()->eq('item_source', $qb->expr()->literal($itemSource)))
->andWhere($qb->expr()->eq('uid_owner', $qb->expr()->literal($user)))
->andWhere($qb->expr()->eq('share_type', $qb->expr()->literal(\OCP\Share::SHARE_TYPE_LINK)));
$qb->execute();

return true;
}

/**
* Checks whether a share has expired, calls unshareItem() if yes.
* @param array $item Share data (usually database row)
Expand Down Expand Up @@ -2368,4 +2402,12 @@ public static function getExpireInterval() {
return (int)\OCP\Config::getAppValue('core', 'shareapi_expire_after_n_days', '7');
}

/**
* @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;
}
}
7 changes: 7 additions & 0 deletions lib/public/idbconnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,4 +174,11 @@ public function dropTable($table);
* @return bool
*/
public function tableExists($table);

/**
* Gets the QueryBuilder
*
* @return \Doctrine\DBAL\Query\QueryBuilder
*/
public function createQueryBuilder();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interface change, @icewind1991 @DeepDiver1975 @schiesbn I guess we need this anyway ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. as on my todo list to ask more people about this. We somehow need it if we want to support the QueryBuilder or we can't use the IDBConnection to obtain it

}
15 changes: 15 additions & 0 deletions lib/public/share.php
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,21 @@ public static function setExpirationDate($itemType, $itemSource, $date, $shareTi
return \OC\Share\Share::setExpirationDate($itemType, $itemSource, $date, $shareTime);
}

/**
* Set expiration date for a share
* @param string $itemType
* @param string $itemSource
* @param string $password
* @return boolean
*/
public static function setPassword($itemType, $itemSource, $password) {
$userSession = \OC::$server->getUserSession();
$connection = \OC::$server->getDatabaseConnection();
$config = \OC::$server->getConfig();
return \OC\Share\Share::setPassword($userSession, $connection, $config, $itemType, $itemSource, $password);
}


/**
* Get the backend class for the specified item type
* @param string $itemType
Expand Down
110 changes: 110 additions & 0 deletions tests/lib/share/share.php
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,116 @@ function dataProviderTestGroupItems() {
),
);
}

/**
* 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('\OCP\IDBConnection')
->disableOriginalConstructor()
->getMock();

$config = $this->getMockBuilder('\OCP\IConfig')
->disableOriginalConstructor()
->getMock();

\OC\Share\Share::setPassword($userSession, $connection, $config, 'a', 'b', 'c');
}

/**
* 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('expr')->willReturn($ex);


$connection = $this->getMockBuilder('\OCP\IDBConnection')
->disableOriginalConstructor()
->getMock();
$connection->method('createQueryBuilder')->willReturn($qb);

$config = $this->getMockBuilder('\OCP\IConfig')
->disableOriginalConstructor()
->getMock();


$res = \OC\Share\Share::setPassword($userSession, $connection, $config, 'a', 'b', 'c');

$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('set')->will($this->returnSelf());
$qb->method('where')->will($this->returnSelf());
$qb->method('andWhere')->will($this->returnSelf());
$qb->method('expr')->willReturn($ex);


$connection = $this->getMockBuilder('\OCP\IDBConnection')
->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, 'a', 'b', '');
}

}

class DummyShareClass extends \OC\Share\Share {
Expand Down