diff options
author | Ferdinand Thiessen <opensource@fthiessen.de> | 2024-08-23 00:01:39 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-08-23 00:01:39 +0200 |
commit | 920a74118ce560c2d66940bad3b8a9d60bf86584 (patch) | |
tree | 83a5c12999d212bd63b2bb36a02ff93695f0467d | |
parent | d78bfb718120201c0f253d7980a0e5c531bb9d31 (diff) | |
parent | dd58e5290f0790157df6077471327ebd9affb03f (diff) | |
download | nextcloud-server-920a74118ce560c2d66940bad3b8a9d60bf86584.tar.gz nextcloud-server-920a74118ce560c2d66940bad3b8a9d60bf86584.zip |
Merge pull request #47403 from nextcloud/feat/password-context
feat(Security): Allow defining a password context for password validation and generation
10 files changed, 184 insertions, 63 deletions
diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 642065c9983..25af4862ca4 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -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); diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index b21f6440c64..d4f7189a3cc 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -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(); diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index e03fba72312..382a65154eb 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -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; - - /** @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 IDBConnection $connection; + + 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); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 1a3a86fd207..eed05ce73d6 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -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', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index c636ba769f9..ac293dd7b36 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -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', diff --git a/lib/public/Security/Events/GenerateSecurePasswordEvent.php b/lib/public/Security/Events/GenerateSecurePasswordEvent.php index 8adddd529b0..419e7b40ee4 100644 --- a/lib/public/Security/Events/GenerateSecurePasswordEvent.php +++ b/lib/public/Security/Events/GenerateSecurePasswordEvent.php @@ -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; + } } diff --git a/lib/public/Security/Events/ValidatePasswordPolicyEvent.php b/lib/public/Security/Events/ValidatePasswordPolicyEvent.php index 0aa8b516f70..d7ac9442392 100644 --- a/lib/public/Security/Events/ValidatePasswordPolicyEvent.php +++ b/lib/public/Security/Events/ValidatePasswordPolicyEvent.php @@ -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; + } } diff --git a/lib/public/Security/PasswordContext.php b/lib/public/Security/PasswordContext.php new file mode 100644 index 00000000000..909070c09ff --- /dev/null +++ b/lib/public/Security/PasswordContext.php @@ -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; +} diff --git a/tests/lib/Security/Events/GenerateSecurePasswordEventTest.php b/tests/lib/Security/Events/GenerateSecurePasswordEventTest.php new file mode 100644 index 00000000000..29002a90488 --- /dev/null +++ b/tests/lib/Security/Events/GenerateSecurePasswordEventTest.php @@ -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()); + } +} diff --git a/tests/lib/Security/Events/ValidatePasswordPolicyEventTest.php b/tests/lib/Security/Events/ValidatePasswordPolicyEventTest.php new file mode 100644 index 00000000000..8a7bc3b2f29 --- /dev/null +++ b/tests/lib/Security/Events/ValidatePasswordPolicyEventTest.php @@ -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()); + } +} |