From 9ef1c59c61249d1818087526f0d3b01ab4790196 Mon Sep 17 00:00:00 2001 From: Cyrille Bollu Date: Wed, 23 Feb 2022 12:03:38 +0100 Subject: [PATCH] Makes sure password hasn't expired when verifying a share's password. This also deletes the ResetExpiredPasswordsJob.php as it is not needed anymore. This commit is part of https://github.com/nextcloud/server/issues/31005 Signed-off-by: Cyrille Bollu --- apps/files_sharing/appinfo/info.xml | 1 - .../ResetExpiredPasswordsJob.php | 108 ------------------ apps/sharebymail/lib/ShareByMailProvider.php | 1 + lib/private/Share20/Manager.php | 10 ++ lib/private/Share20/Share.php | 17 +++ lib/public/Share/IShare.php | 13 +++ 6 files changed, 41 insertions(+), 109 deletions(-) delete mode 100644 apps/files_sharing/lib/BackgroundJob/ResetExpiredPasswordsJob.php diff --git a/apps/files_sharing/appinfo/info.xml b/apps/files_sharing/appinfo/info.xml index a2812becf0a6f..7ab8a1fcfa252 100644 --- a/apps/files_sharing/appinfo/info.xml +++ b/apps/files_sharing/appinfo/info.xml @@ -30,7 +30,6 @@ Turning the feature off removes shared files and folders on the server for all s OCA\Files_Sharing\DeleteOrphanedSharesJob OCA\Files_Sharing\ExpireSharesJob OCA\Files_Sharing\BackgroundJob\FederatedSharesDiscoverJob - OCA\Files_Sharing\BackgroundJob\ResetExpiredPasswordsJob diff --git a/apps/files_sharing/lib/BackgroundJob/ResetExpiredPasswordsJob.php b/apps/files_sharing/lib/BackgroundJob/ResetExpiredPasswordsJob.php deleted file mode 100644 index 11617a8b88d87..0000000000000 --- a/apps/files_sharing/lib/BackgroundJob/ResetExpiredPasswordsJob.php +++ /dev/null @@ -1,108 +0,0 @@ - - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * - */ -namespace OCA\Files_Sharing\BackgroundJob; - -use \OCP\AppFramework\Utility\ITimeFactory; -use \OCP\BackgroundJob\TimedJob; -use \OCP\DB\QueryBuilder\IQueryBuilder; -use \OCP\EventDispatcher\IEventDispatcher; -use \OCP\IConfig; -use \OCP\IDBConnection; -use \OCP\Security\IHasher; -use \OCP\Security\ISecureRandom; - -class ResetExpiredPasswordsJob extends TimedJob { - - /** @var IConfig */ - private $config; - - /** @var IDBConnection */ - private $connection; - - /** @var IEventDispatcher */ - private $eventDispatcher; - - /** @var IHasher */ - private $hasher; - - /** @var ISecureRandom */ - private $secureRandom; - - public function __construct(IConfig $config, IDBConnection $connection, IEventDispatcher $eventDispatcher, - IHasher $hasher, ISecureRandom $secureRandom, ITimeFactory $time) { - - parent::__construct($time); - - $this->config = $config; - $this->connection = $connection; - $this->eventDispatcher = $eventDispatcher; - $this->hasher = $hasher; - $this->secureRandom = $secureRandom; - - // Runs at most every 5 minutes - parent::setInterval(300); - } - - // Sets a random password to shares whose password has expired - protected function run($argument) { - $qb = $this->connection->getQueryBuilder(); - - // QUESTION: DOES THE DATETIME COMPARAISON WORK WELL WHEN TIMEZONES ENTER THE GAME? - // I THINK SO, BECAUSE EVERYTHING HAPPENS ON THE SERVER, HENCE ON THE SAME TZ - $qb->select('id') - ->from('share') - ->where($qb->expr()->lte('password_expiration_time', $qb->createNamedParameter((new \DateTime())->format('Y-m-d H:i:s'), IQueryBuilder::PARAM_DATE))); - - $result = $qb->execute(); - while ($row = $result->fetch()) { - - // Generates a random password respecting any password policy defined - $event = new \OCP\Security\Events\GenerateSecurePasswordEvent(); - $this->eventDispatcher->dispatchTyped($event); - $password = $event->getPassword() ?? $this->hasher->hash($this->secureRandom->generate(20)); - - // Gets password expiration interval. Default to 15 minutes - $expirationInterval = $this->config->getSystemValue('share_temporary_password_expiration_interval'); - if ($expirationInterval === '') { - $expirationInterval = 'P0DT15M'; - } - - // Computes new password expiration time. - $now = new \DateTime(); - try { - $expirationTime = $now->add(new \DateInterval($expirationInterval)); - } catch (\Exception $e) { - // Catches invalid format for system value 'share_temporary_password_expiration_interval' - $expirationTime = $now->add(new \DateInterval('P0DT15M')); - } finally { - - // Updates share password and expiration time - $qb->update('share') - ->where($qb->expr()->eq('id', $qb->createNamedParameter($row['id']))) - ->set('password', $qb->createNamedParameter($password)) - ->set('password_expiration_time', $qb->createNamedParameter($expirationTime->format('Y-m-d H:i:s'), IQueryBuilder::PARAM_DATE)) - ->execute(); - } - } - - } - -} diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index e3df9e3f213f1..89a56e645c369 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -1033,6 +1033,7 @@ protected function createShareObject($data) { $share->setShareTime($shareTime); $share->setSharedWith($data['share_with']); $share->setPassword($data['password']); + $share->setPasswordExpirationTime($data['password_expiration_time']); $share->setLabel($data['label']); $share->setSendPasswordByTalk((bool)$data['password_by_talk']); $share->setHideDownload((bool)$data['hide_download']); diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index e5c15b632d1a4..38a1cd6a270e2 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1556,6 +1556,16 @@ public function checkPassword(IShare $share, $password) { return false; } + // Makes sure password hasn't expired + $expirationTime = $share->getPasswordExpirationTime(); + if ($expirationTime !== null) { + $expirationDateTime = new \DateTime($expirationTime); + $now = new \DateTime(); + if ($expirationDateTime < $now) { + return false; + } + } + $newHash = ''; if (!$this->hasher->verify($password, $share->getPassword(), $newHash)) { return false; diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index f1df71b1143f4..f52dd43ff0c23 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -73,6 +73,8 @@ class Share implements IShare { private $expireDate; /** @var string */ private $password; + /** @var string */ + private $passwordExpirationTime; /** @var bool */ private $sendPasswordByTalk = false; /** @var string */ @@ -461,6 +463,21 @@ public function getPassword() { return $this->password; } + /** + * @inheritdoc + */ + public function setPasswordExpirationTime($passwordExpirationTime) { + $this->passwordExpirationTime = $passwordExpirationTime; + return $this; + } + + /** + * @inheritdoc + */ + public function getPasswordExpirationTime() { + return $this->passwordExpirationTime; + } + /** * @inheritdoc */ diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index 8ff3f5f0394ea..02030c76e638d 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -448,6 +448,19 @@ public function setPassword($password); */ public function getPassword(); + /** + * Set the password's expiration time of this share. + * + * @return \OCP\Share\IShare The modified object + */ + public function setPasswordExpirationTime($passwordExpirationTime); + + /** + * Get the password's expiration time of this share. + * + * @return string + */ + public function getPasswordExpirationTime(); /** * Set if the recipient can start a conversation with the owner to get the