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

Temporary passwords for public non-anonymous protected shares (ie: files shared with an email recipient). #31220

Merged
merged 1 commit into from
Apr 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/files_sharing/appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
Turning the feature off removes shared files and folders on the server for all share recipients, and also on the sync clients and mobile apps. More information is available in the Nextcloud Documentation.

</description>
<version>1.16.1</version>
<version>1.16.2</version>
<licence>agpl</licence>
<author>Michael Gapczynski</author>
<author>Bjoern Schiessle</author>
Expand Down
1 change: 1 addition & 0 deletions apps/files_sharing/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
'OCA\\Files_Sharing\\Migration\\Version11300Date20201120141438' => $baseDir . '/../lib/Migration/Version11300Date20201120141438.php',
'OCA\\Files_Sharing\\Migration\\Version21000Date20201223143245' => $baseDir . '/../lib/Migration/Version21000Date20201223143245.php',
'OCA\\Files_Sharing\\Migration\\Version22000Date20210216084241' => $baseDir . '/../lib/Migration/Version22000Date20210216084241.php',
'OCA\\Files_Sharing\\Migration\\Version24000Date20220208195521' => $baseDir . '/../lib/Migration/Version24000Date20220208195521.php',
'OCA\\Files_Sharing\\Migration\\Version24000Date20220404142216' => $baseDir . '/../lib/Migration/Version24000Date20220404142216.php',
'OCA\\Files_Sharing\\MountProvider' => $baseDir . '/../lib/MountProvider.php',
'OCA\\Files_Sharing\\Notification\\Listener' => $baseDir . '/../lib/Notification/Listener.php',
Expand Down
1 change: 1 addition & 0 deletions apps/files_sharing/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class ComposerStaticInitFiles_Sharing
'OCA\\Files_Sharing\\Migration\\Version11300Date20201120141438' => __DIR__ . '/..' . '/../lib/Migration/Version11300Date20201120141438.php',
'OCA\\Files_Sharing\\Migration\\Version21000Date20201223143245' => __DIR__ . '/..' . '/../lib/Migration/Version21000Date20201223143245.php',
'OCA\\Files_Sharing\\Migration\\Version22000Date20210216084241' => __DIR__ . '/..' . '/../lib/Migration/Version22000Date20210216084241.php',
'OCA\\Files_Sharing\\Migration\\Version24000Date20220208195521' => __DIR__ . '/..' . '/../lib/Migration/Version24000Date20220208195521.php',
'OCA\\Files_Sharing\\Migration\\Version24000Date20220404142216' => __DIR__ . '/..' . '/../lib/Migration/Version24000Date20220404142216.php',
'OCA\\Files_Sharing\\MountProvider' => __DIR__ . '/..' . '/../lib/MountProvider.php',
'OCA\\Files_Sharing\\Notification\\Listener' => __DIR__ . '/..' . '/../lib/Notification/Listener.php',
Expand Down
4 changes: 2 additions & 2 deletions apps/files_sharing/composer/composer/installed.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
'type' => 'library',
'install_path' => __DIR__ . '/../',
'aliases' => array(),
'reference' => 'c6429e6cd19c57582364338362e543580821cf99',
'reference' => 'ea4531aaaa6eb9fb3859e05b69ab773bfbfe7437',
'name' => '__root__',
'dev' => false,
),
Expand All @@ -16,7 +16,7 @@
'type' => 'library',
'install_path' => __DIR__ . '/../',
'aliases' => array(),
'reference' => 'c6429e6cd19c57582364338362e543580821cf99',
'reference' => 'ea4531aaaa6eb9fb3859e05b69ab773bfbfe7437',
'dev_requirement' => false,
),
),
Expand Down
37 changes: 37 additions & 0 deletions apps/files_sharing/lib/Controller/ShareAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ protected function formatShare(IShare $share, Node $recipientNode = null): array
} elseif ($share->getShareType() === IShare::TYPE_EMAIL) {
$result['share_with'] = $share->getSharedWith();
$result['password'] = $share->getPassword();
$result['password_expiration_time'] = $share->getPasswordExpirationTime();
$result['send_password_by_talk'] = $share->getSendPasswordByTalk();
$result['share_with_displayname'] = $this->getDisplayNameFromAddressBook($share->getSharedWith(), 'EMAIL');
$result['token'] = $share->getToken();
Expand Down Expand Up @@ -570,6 +571,10 @@ public function createShare(
// Set password
if ($password !== '') {
$share->setPassword($password);
// Shares shared by email have temporary passwords by default
if ($shareType === IShare::TYPE_EMAIL) {
$this->setSharePasswordExpirationTime($share);
}
}

// Only share by mail have a recipient
Expand Down Expand Up @@ -1177,6 +1182,9 @@ public function updateShare(
$share->setPassword(null);
} elseif ($password !== null) {
$share->setPassword($password);
if ($share->getShareType() === IShare::TYPE_EMAIL) {
$this->setSharePasswordExpirationTime($share);
}
}

if ($label !== null) {
Expand Down Expand Up @@ -1513,6 +1521,35 @@ private function parseDate(string $expireDate): \DateTime {
return $date;
}

/**
* Set the share's password expiration time
*/
private function setSharePasswordExpirationTime(IShare $share): void {
if ($this->config->getSystemValue('allow_mail_share_permanent_password')) {
// Sets password expiration date to NULL
$share->setPasswordExpirationTime();
return;
}
// Sets password expiration date
$expirationTime = null;
try {
$now = new \DateTime();
$expirationInterval = $this->config->getSystemValue('share_temporary_password_expiration_interval');
if ($expirationInterval === '' || is_null($expirationInterval)) {
$expirationInterval = 'P0DT15M';
}
$expirationTime = $now->add(new \DateInterval($expirationInterval));
} catch (\Exception $e) {
// Catches invalid format for system value 'share_temporary_password_expiration_interval'
PVince81 marked this conversation as resolved.
Show resolved Hide resolved
\OC::$server->getLogger()->logException($e, [
'message' => 'The \'share_temporary_password_expiration_interval\' system setting does not respect the DateInterval::__construct() format. Setting it to \'P0DT15M\''
]);
$expirationTime = $now->add(new \DateInterval('P0DT15M'));
} finally {
$share->setPasswordExpirationTime($expirationTime);
}
}

/**
* Since we have multiple providers but the OCS Share API v1 does
* not support this we need to check all backends.
Expand Down
113 changes: 67 additions & 46 deletions apps/files_sharing/lib/Controller/ShareController.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Security\ISecureRandom;
use OCP\Share;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager as ShareManager;
Expand All @@ -84,53 +85,21 @@
* @package OCA\Files_Sharing\Controllers
*/
class ShareController extends AuthPublicShareController {
protected IConfig $config;
protected IUserManager $userManager;
protected ILogger $logger;
protected \OCP\Activity\IManager $activityManager;
protected IPreview $previewManager;
protected IRootFolder $rootFolder;
protected FederatedShareProvider $federatedShareProvider;
protected IAccountManager $accountManager;
protected IEventDispatcher $eventDispatcher;
protected IL10N $l10n;
protected Defaults $defaults;
protected ShareManager $shareManager;
protected ISecureRandom $secureRandom;
protected ?Share\IShare $share = null;

/** @var IConfig */
protected $config;
/** @var IUserManager */
protected $userManager;
/** @var ILogger */
protected $logger;
/** @var \OCP\Activity\IManager */
protected $activityManager;
/** @var IPreview */
protected $previewManager;
/** @var IRootFolder */
protected $rootFolder;
/** @var FederatedShareProvider */
protected $federatedShareProvider;
/** @var IAccountManager */
protected $accountManager;
/** @var IEventDispatcher */
protected $eventDispatcher;
/** @var IL10N */
protected $l10n;
/** @var Defaults */
protected $defaults;
/** @var ShareManager */
protected $shareManager;

/** @var Share\IShare */
protected $share;

/**
* @param string $appName
* @param IRequest $request
* @param IConfig $config
* @param IURLGenerator $urlGenerator
* @param IUserManager $userManager
* @param ILogger $logger
* @param \OCP\Activity\IManager $activityManager
* @param \OCP\Share\IManager $shareManager
* @param ISession $session
* @param IPreview $previewManager
* @param IRootFolder $rootFolder
* @param FederatedShareProvider $federatedShareProvider
* @param IAccountManager $accountManager
* @param IEventDispatcher $eventDispatcher
* @param IL10N $l10n
* @param Defaults $defaults
*/
public function __construct(string $appName,
IRequest $request,
IConfig $config,
Expand All @@ -146,6 +115,7 @@ public function __construct(string $appName,
IAccountManager $accountManager,
IEventDispatcher $eventDispatcher,
IL10N $l10n,
ISecureRandom $secureRandom,
Defaults $defaults) {
parent::__construct($appName, $request, $session, $urlGenerator);

Expand All @@ -159,6 +129,7 @@ public function __construct(string $appName,
$this->accountManager = $accountManager;
$this->eventDispatcher = $eventDispatcher;
$this->l10n = $l10n;
$this->secureRandom = $secureRandom;
$this->defaults = $defaults;
$this->shareManager = $shareManager;
}
Expand Down Expand Up @@ -209,6 +180,56 @@ protected function showAuthFailed(): TemplateResponse {
return $response;
}

/**
* The template to show after user identification
*/
protected function showIdentificationResult(bool $success = false): TemplateResponse {
$templateParameters = ['share' => $this->share, 'identityOk' => $success];

$this->eventDispatcher->dispatchTyped(new BeforeTemplateRenderedEvent($this->share, BeforeTemplateRenderedEvent::SCOPE_PUBLIC_SHARE_AUTH));

$response = new TemplateResponse('core', 'publicshareauth', $templateParameters, 'guest');
if ($this->share->getSendPasswordByTalk()) {
$csp = new ContentSecurityPolicy();
$csp->addAllowedConnectDomain('*');
$csp->addAllowedMediaDomain('blob:');
$response->setContentSecurityPolicy($csp);
}

return $response;
}

/**
* Validate the identity token of a public share
*
* @param ?string $identityToken
* @return bool
*/
protected function validateIdentity(?string $identityToken = null): bool {

if ($this->share->getShareType() !== IShare::TYPE_EMAIL) {
return false;
}

if ($identityToken === null || $this->share->getSharedWith() === null) {
return false;
}

return $identityToken === $this->share->getSharedWith();
}

/**
* Generates a password for the share, respecting any password policy defined
*/
protected function generatePassword(): void {
$event = new \OCP\Security\Events\GenerateSecurePasswordEvent();
$this->eventDispatcher->dispatchTyped($event);
$password = $event->getPassword() ?? $this->secureRandom->generate(20);

$this->share->setPassword($password);
$this->shareManager->updateShare($this->share);
}

protected function verifyPassword(string $password): bool {
return $this->shareManager->checkPassword($this->share, $password);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

declare(strict_types=1);

PVince81 marked this conversation as resolved.
Show resolved Hide resolved
/**
* @copyright Copyright (c) 2022 Vincent Petry <vincent@nextloud.com>
*
* @author Vincent Petry <vincent@nextcloud.com>
*
* @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 <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\Files_Sharing\Migration;

use Closure;
use OCP\DB\Types;
use OCP\DB\ISchemaWrapper;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

class Version24000Date20220208195521 extends SimpleMigrationStep {

/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
$schema = $schemaClosure();
$table = $schema->getTable('share');
$table->addColumn('password_expiration_time', Types::DATETIME, [
'notnull' => false,
]);
return $schema;
}

}
4 changes: 2 additions & 2 deletions apps/files_sharing/src/components/SharingEntryLink.vue
Original file line number Diff line number Diff line change
Expand Up @@ -780,8 +780,8 @@ export default {
/**
* Uncheck password protection
* We need this method because @update:checked
* is ran simultaneously as @uncheck, so
* so we cannot ensure data is up-to-date
* is ran simultaneously as @uncheck, so we
* cannot ensure data is up-to-date
*/
onPasswordDisable() {
this.share.password = ''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4410,6 +4410,7 @@ public function dataFormatShare() {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
'password_expiration_time' => null,
], $share, [], false
];

Expand Down Expand Up @@ -4459,6 +4460,7 @@ public function dataFormatShare() {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
'password_expiration_time' => null,
], $share, [], false
];

Expand Down
4 changes: 4 additions & 0 deletions apps/files_sharing/tests/Controller/ShareControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ class ShareControllerTest extends \Test\TestCase {
private $eventDispatcher;
/** @var IL10N */
private $l10n;
/** @var ISecureRandom */
private $secureRandom;
/** @var Defaults|MockObject */
private $defaults;

Expand All @@ -127,6 +129,7 @@ protected function setUp(): void {
$this->accountManager = $this->createMock(IAccountManager::class);
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->l10n = $this->createMock(IL10N::class);
$this->secureRandom = $this->createMock(ISecureRandom::class);
$this->defaults = $this->createMock(Defaults::class);

$this->shareController = new \OCA\Files_Sharing\Controller\ShareController(
Expand All @@ -145,6 +148,7 @@ protected function setUp(): void {
$this->accountManager,
$this->eventDispatcher,
$this->l10n,
$this->secureRandom,
$this->defaults
);

Expand Down
Loading