Merge pull request #47403 from nextcloud/feat/password-context

feat(Security): Allow defining a password context for password validation and generation
This commit is contained in:
Ferdinand Thiessen 2024-08-23 00:01:39 +02:00 committed by GitHub
commit 920a74118c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 183 additions and 62 deletions

View file

@ -33,13 +33,14 @@ use OCP\IRequest;
use OCP\ISession;
use OCP\IURLGenerator;
use OCP\IUserManager;
use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\ISecureRandom;
use OCP\Security\PasswordContext;
use OCP\Share;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager as ShareManager;
use OCP\Share\IPublicShareTemplateFactory;
use OCP\Share\IShare;
use OCP\Template;
/**
* @package OCA\Files_Sharing\Controllers
@ -156,7 +157,7 @@ class ShareController extends AuthPublicShareController {
* Generates a password for the share, respecting any password policy defined
*/
protected function generatePassword(): void {
$event = new \OCP\Security\Events\GenerateSecurePasswordEvent();
$event = new GenerateSecurePasswordEvent(PasswordContext::SHARING);
$this->eventDispatcher->dispatchTyped($event);
$password = $event->getPassword() ?? $this->secureRandom->generate(20);

View file

@ -28,6 +28,7 @@ use OCP\Mail\IMailer;
use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\IHasher;
use OCP\Security\ISecureRandom;
use OCP\Security\PasswordContext;
use OCP\Share\Exceptions\GenericShareException;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IAttributes;
@ -131,7 +132,7 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
);
}
$passwordEvent = new GenerateSecurePasswordEvent();
$passwordEvent = new GenerateSecurePasswordEvent(PasswordContext::SHARING);
$this->eventDispatcher->dispatchTyped($passwordEvent);
$password = $passwordEvent->getPassword();

View file

@ -9,10 +9,12 @@ use DateTime;
use OC\Mail\Message;
use OCA\ShareByMail\Settings\SettingsManager;
use OCA\ShareByMail\ShareByMailProvider;
use OCP\Activity\IManager as IActivityManager;
use OCP\Defaults;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\File;
use OCP\Files\IRootFolder;
use OCP\Files\Node;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IL10N;
@ -25,9 +27,11 @@ use OCP\Mail\IMessage;
use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\IHasher;
use OCP\Security\ISecureRandom;
use OCP\Security\PasswordContext;
use OCP\Share\IAttributes;
use OCP\Share\IManager;
use OCP\Share\IShare;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;
@ -38,65 +42,36 @@ use Test\TestCase;
* @group DB
*/
class ShareByMailProviderTest extends TestCase {
/** @var IConfig */
private $config;
private IDBConnection $connection;
/** @var IDBConnection */
private $connection;
/** @var IManager | \PHPUnit\Framework\MockObject\MockObject */
private $shareManager;
/** @var IL10N | \PHPUnit\Framework\MockObject\MockObject */
private $l;
/** @var LoggerInterface | \PHPUnit\Framework\MockObject\MockObject */
private $logger;
/** @var IRootFolder | \PHPUnit\Framework\MockObject\MockObject */
private $rootFolder;
/** @var IUserManager | \PHPUnit\Framework\MockObject\MockObject */
private $userManager;
/** @var ISecureRandom | \PHPUnit\Framework\MockObject\MockObject */
private $secureRandom;
/** @var IMailer | \PHPUnit\Framework\MockObject\MockObject */
private $mailer;
/** @var IURLGenerator | \PHPUnit\Framework\MockObject\MockObject */
private $urlGenerator;
/** @var IShare | \PHPUnit\Framework\MockObject\MockObject */
private $share;
/** @var \OCP\Activity\IManager | \PHPUnit\Framework\MockObject\MockObject */
private $activityManager;
/** @var SettingsManager | \PHPUnit\Framework\MockObject\MockObject */
private $settingsManager;
/** @var Defaults|\PHPUnit\Framework\MockObject\MockObject */
private $defaults;
/** @var IHasher | \PHPUnit\Framework\MockObject\MockObject */
private $hasher;
/** @var IEventDispatcher */
private $eventDispatcher;
private IL10N&MockObject $l;
private IShare&MockObject $share;
private IConfig&MockObject $config;
private IMailer&MockObject $mailer;
private IHasher&MockObject $hasher;
private Defaults&MockObject $defaults;
private IManager&MockObject $shareManager;
private LoggerInterface&MockObject $logger;
private IRootFolder&MockObject $rootFolder;
private IUserManager&MockObject $userManager;
private ISecureRandom&MockObject $secureRandom;
private IURLGenerator&MockObject $urlGenerator;
private SettingsManager&MockObject $settingsManager;
private IActivityManager&MockObject $activityManager;
private IEventDispatcher&MockObject $eventDispatcher;
protected function setUp(): void {
parent::setUp();
$this->config = $this->getMockBuilder(IConfig::class)->getMock();
$this->connection = \OC::$server->getDatabaseConnection();
$this->connection = \OCP\Server::get(IDBConnection::class);
$this->l = $this->getMockBuilder(IL10N::class)->getMock();
$this->l->method('t')
->willReturnCallback(function ($text, $parameters = []) {
return vsprintf($text, $parameters);
});
$this->config = $this->getMockBuilder(IConfig::class)->getMock();
$this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
$this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock();
$this->userManager = $this->getMockBuilder(IUserManager::class)->getMock();
@ -165,7 +140,10 @@ class ShareByMailProviderTest extends TestCase {
}
protected function tearDown(): void {
$this->connection->getQueryBuilder()->delete('share')->execute();
$this->connection
->getQueryBuilder()
->delete('share')
->executeStatement();
parent::tearDown();
}
@ -305,7 +283,11 @@ class ShareByMailProviderTest extends TestCase {
// Assume the mail address is valid.
$this->mailer->expects($this->any())->method('validateMailAddress')->willReturn(true);
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity', 'sendEmail', 'sendPassword', 'sendPasswordToOwner']);
$instance = $this->getInstance([
'getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject',
'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity',
'sendEmail', 'sendPassword', 'sendPasswordToOwner',
]);
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
@ -361,7 +343,7 @@ class ShareByMailProviderTest extends TestCase {
->willReturn('autogeneratedPassword');
$this->eventDispatcher->expects($this->once())
->method('dispatchTyped')
->with(new GenerateSecurePasswordEvent());
->with(new GenerateSecurePasswordEvent(PasswordContext::SHARING));
// Assume the mail address is valid.
$this->mailer->expects($this->any())->method('validateMailAddress')->willReturn(true);
@ -822,7 +804,7 @@ class ShareByMailProviderTest extends TestCase {
* @param bool sendMail
*/
public function testUpdateSendPassword($plainTextPassword, string $originalPassword, string $newPassword, $originalSendPasswordByTalk, $newSendPasswordByTalk, bool $sendMail) {
$node = $this->getMockBuilder(File::class)->getMock();
$node = $this->createMock(File::class);
$node->expects($this->any())->method('getName')->willReturn('filename');
$this->settingsManager->method('sendPasswordByMail')->willReturn(true);
@ -927,7 +909,7 @@ class ShareByMailProviderTest extends TestCase {
$permissions = 1;
$token = 'token';
$node = $this->getMockBuilder('OCP\Files\Node')->getMock();
$node = $this->createMock(Node::class);
$node->expects($this->once())->method('getId')->willReturn($itemSource);

View file

@ -678,6 +678,7 @@ return array(
'OCP\\Security\\Ip\\IFactory' => $baseDir . '/lib/public/Security/Ip/IFactory.php',
'OCP\\Security\\Ip\\IRange' => $baseDir . '/lib/public/Security/Ip/IRange.php',
'OCP\\Security\\Ip\\IRemoteAddress' => $baseDir . '/lib/public/Security/Ip/IRemoteAddress.php',
'OCP\\Security\\PasswordContext' => $baseDir . '/lib/public/Security/PasswordContext.php',
'OCP\\Security\\RateLimiting\\ILimiter' => $baseDir . '/lib/public/Security/RateLimiting/ILimiter.php',
'OCP\\Security\\RateLimiting\\IRateLimitExceededException' => $baseDir . '/lib/public/Security/RateLimiting/IRateLimitExceededException.php',
'OCP\\Security\\VerificationToken\\IVerificationToken' => $baseDir . '/lib/public/Security/VerificationToken/IVerificationToken.php',

View file

@ -711,6 +711,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OCP\\Security\\Ip\\IFactory' => __DIR__ . '/../../..' . '/lib/public/Security/Ip/IFactory.php',
'OCP\\Security\\Ip\\IRange' => __DIR__ . '/../../..' . '/lib/public/Security/Ip/IRange.php',
'OCP\\Security\\Ip\\IRemoteAddress' => __DIR__ . '/../../..' . '/lib/public/Security/Ip/IRemoteAddress.php',
'OCP\\Security\\PasswordContext' => __DIR__ . '/../../..' . '/lib/public/Security/PasswordContext.php',
'OCP\\Security\\RateLimiting\\ILimiter' => __DIR__ . '/../../..' . '/lib/public/Security/RateLimiting/ILimiter.php',
'OCP\\Security\\RateLimiting\\IRateLimitExceededException' => __DIR__ . '/../../..' . '/lib/public/Security/RateLimiting/IRateLimitExceededException.php',
'OCP\\Security\\VerificationToken\\IVerificationToken' => __DIR__ . '/../../..' . '/lib/public/Security/VerificationToken/IVerificationToken.php',

View file

@ -9,15 +9,34 @@ declare(strict_types=1);
namespace OCP\Security\Events;
use OCP\EventDispatcher\Event;
use OCP\Security\PasswordContext;
/**
* Event to request a secure password to be generated
* @since 18.0.0
*/
class GenerateSecurePasswordEvent extends Event {
/** @var null|string */
private $password;
private ?string $password;
/**
* Request a secure password to be generated.
*
* By default passwords are generated for the user account context,
* this can be adjusted by passing another `PasswordContext`.
* @since 31.0.0
*/
public function __construct(
private PasswordContext $context = PasswordContext::ACCOUNT,
) {
parent::__construct();
$this->password = null;
}
/**
* Get the generated password.
*
* If a password generator is registered and successfully generated a password
* that password can get read back. Otherwise `null` is returned.
* @since 18.0.0
*/
public function getPassword(): ?string {
@ -25,9 +44,20 @@ class GenerateSecurePasswordEvent extends Event {
}
/**
* Set the generated password.
*
* This is used by password generators to set the generated password.
* @since 18.0.0
*/
public function setPassword(string $password): void {
$this->password = $password;
}
/**
* Get the context this password should generated for.
* @since 31.0.0
*/
public function getContext(): PasswordContext {
return $this->context;
}
}

View file

@ -9,26 +9,41 @@ declare(strict_types=1);
namespace OCP\Security\Events;
use OCP\EventDispatcher\Event;
use OCP\Security\PasswordContext;
/**
* This event can be emitted to request a validation of a password.
*
* If a password policy app is installed and the password
* is invalid, an `\OCP\HintException` will be thrown.
* @since 18.0.0
*/
class ValidatePasswordPolicyEvent extends Event {
/** @var string */
private $password;
/**
* @since 18.0.0
* @since 31.0.0 - $context parameter added
*/
public function __construct(string $password) {
public function __construct(
private string $password,
private PasswordContext $context = PasswordContext::ACCOUNT,
) {
parent::__construct();
$this->password = $password;
}
/**
* Get the password that should be validated.
* @since 18.0.0
*/
public function getPassword(): string {
return $this->password;
}
/**
* Get the context this password should validated for.
* @since 31.0.0
*/
public function getContext(): PasswordContext {
return $this->context;
}
}

View file

@ -0,0 +1,29 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCP\Security;
/**
* Define the context in which a password is used.
* This allows setting a context for password validation and password generation.
*
* @package OCP\Security
* @since 31.0.0
*/
enum PasswordContext {
/**
* Password used for an user account
* @since 31.0.0
*/
case ACCOUNT;
/**
* Password used for (public) shares
* @since 31.0.0
*/
case SHARING;
}

View file

@ -0,0 +1,33 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace Test\Security\Events;
use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\PasswordContext;
class GenerateSecurePasswordEventTest extends \Test\TestCase {
public function testDefaultProperties() {
$event = new GenerateSecurePasswordEvent();
$this->assertNull($event->getPassword());
$this->assertEquals(PasswordContext::ACCOUNT, $event->getContext());
}
public function testSettingPassword() {
$event = new GenerateSecurePasswordEvent();
$event->setPassword('example');
$this->assertEquals('example', $event->getPassword());
}
public function testSettingContext() {
$event = new GenerateSecurePasswordEvent(PasswordContext::SHARING);
$this->assertEquals(PasswordContext::SHARING, $event->getContext());
}
}

View file

@ -0,0 +1,28 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace Test\Security\Events;
use OCP\Security\Events\ValidatePasswordPolicyEvent;
use OCP\Security\PasswordContext;
class ValidatePasswordPolicyEventTest extends \Test\TestCase {
public function testDefaultProperties() {
$password = 'example';
$event = new ValidatePasswordPolicyEvent($password);
$this->assertEquals($password, $event->getPassword());
$this->assertEquals(PasswordContext::ACCOUNT, $event->getContext());
}
public function testSettingContext() {
$event = new ValidatePasswordPolicyEvent('example', PasswordContext::SHARING);
$this->assertEquals(PasswordContext::SHARING, $event->getContext());
}
}