Skip to content

Commit

Permalink
Adds a "Request password" button to the public share authentication p…
Browse files Browse the repository at this point in the history
…age for shares

of type TYPE_EMAIL, when the "video verification" checkbox isn't checked.

This to support sending temporary passwords to recipients for non-anonymous shares
(TYPE_EMAIL shares).

See #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Implements the "Request password" functionality in AuthPublicShareController.php

Users accessing non-anonymous public shares can now request a temporary password
themselves.

This commit is part of #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Creates a migration step for the files_sharing app to add the 'password_expiration_time'
attribute to the oc_shares table.

This is needed to support the temporary password feature.

This commit is part of #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Hides the password input field for shares shared with an email recipient and whose
'video verification' checkbox isn't checked.

This supports the 'temporary password for email shares' feature, and this commit
relates to #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Fixes a typo in a comment in apps/files_sharing/src/components/SharingEntryLink.vue

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

When updating a share of type EMAIL, only send the password to the recipient and
set a password expiration time when the password has requested explicitly (either
via a Talk session or by the user having followed the temporary password self-
provisioning process).

This commit is part of #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Creates a new background job to reset expired share temporary passwords.

This commit is part of #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Respect password policies (if any) when generating a temporary password.

This commit is part of #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Moves the newly created validateIdentity function away from the IManager interface and
rather implement it in ShareController, so as to avoid the need for creating a new
version of the IManager interface.

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Makes share temporary passwords' expiration time configurable via a system value.

This commit is part of #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

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 #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Refactors the publicshareauth template to have the Enter key working in the
email input field and add a 'Back' button.

This commit is part of #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Fixes "Migration step 'OCA\Files_Sharing\Migration\Version24000Date20220208195521' is unknown."
(by running build/autoloadchecker.sh)

This commit is part of #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Adds an option to the "Create a new Share" OCS Share API to allow not sending
the share password by email.

Rationale:

With the introduction of temporary share passwords, it's not so much usefull to
send the password to the guest at share creation since she may request it later
anyway.

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Adds a system config value to allow permanent share passwords

This commit is part of #31005

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

fixes DefaultShareProvider class is not compatible with IShareProvider

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Fixes FederatedShareProvider class is not compatible with IShareProvider

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Reverts the addition of a 'sendPassword' parameter to the ShareAPIController::create() method.

The behaviour is now: If the password is a temporary one, do not send it to the guest, else send it.

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>

Updates ShareByMailProviderTest tests to take into account the fact that temporary
passwords shall not be sent by email anymore upon share creation.

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
  • Loading branch information
StCyr authored and PVince81 committed Apr 7, 2022
1 parent d7e0948 commit 1ae14b4
Show file tree
Hide file tree
Showing 26 changed files with 11,701 additions and 78 deletions.
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
*
* @param IShare $share
*
*/
private function setSharePasswordExpirationTime($share) {
if ($this->config->getSystemValue('allow_mail_share_permanent_password')) {
// Sets password expiration date to NULL
$share->setPasswordExpirationTime();
} else {
// 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'
$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
62 changes: 62 additions & 0 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 Down Expand Up @@ -109,6 +110,8 @@ class ShareController extends AuthPublicShareController {
protected $defaults;
/** @var ShareManager */
protected $shareManager;
/** @var ISecureRandom */
protected $secureRandom;

/** @var Share\IShare */
protected $share;
Expand All @@ -129,6 +132,7 @@ class ShareController extends AuthPublicShareController {
* @param IAccountManager $accountManager
* @param IEventDispatcher $eventDispatcher
* @param IL10N $l10n
* @param ISecureRandom $secureRandom
* @param Defaults $defaults
*/
public function __construct(string $appName,
Expand All @@ -146,6 +150,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 +164,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 +215,62 @@ 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;
}

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

return true;

}

/**
* Generates a password for the share, respecting any password policy defined
*/
protected function generatePassword() {
$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);
return;
}

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);

/**
* @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

0 comments on commit 1ae14b4

Please sign in to comment.