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

feat(Security): Allow defining a password context for password validation and generation

Author: susnux

apps/files_sharing/lib/Controller/ShareController.php

@@ -33,13 +33,14 @@
 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 @@ protected function validateIdentity(?string $identityToken = null): bool {
 	 * 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);
 

apps/sharebymail/lib/ShareByMailProvider.php

@@ -28,6 +28,7 @@
 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 @@ protected function autoGeneratePassword(IShare $share): string {
 			);
 		}
 
-		$passwordEvent = new GenerateSecurePasswordEvent();
+		$passwordEvent = new GenerateSecurePasswordEvent(PasswordContext::SHARING);
 		$this->eventDispatcher->dispatchTyped($passwordEvent);
 
 		$password = $passwordEvent->getPassword();

apps/sharebymail/tests/ShareByMailProviderTest.php

@@ -9,10 +9,12 @@
 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\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 @@
  * @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 @@ private function getInstance(array $mockedMethods = []) {
 	}
 
 	protected function tearDown(): void {
-		$this->connection->getQueryBuilder()->delete('share')->execute();
+		$this->connection
+			->getQueryBuilder()
+			->delete('share')
+			->executeStatement();
 
 		parent::tearDown();
 	}
@@ -305,7 +283,11 @@ public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswo
 		// 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 @@ public function testCreateSendPasswordByMailWithEnforcedPasswordProtectionWithPe
 			->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 @@ public function dataUpdateSendPassword() {
 	 * @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 @@ public function testGetShareByPath() {
 		$permissions = 1;
 		$token = 'token';
 
-		$node = $this->getMockBuilder('OCP\Files\Node')->getMock();
+		$node = $this->createMock(Node::class);
 		$node->expects($this->once())->method('getId')->willReturn($itemSource);
 
 

lib/composer/composer/autoload_classmap.php

@@ -678,6 +678,7 @@
     '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',

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',

lib/public/Security/Events/GenerateSecurePasswordEvent.php

@@ -9,25 +9,55 @@
 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 {
 		return $this->password;
 	}
 
 	/**
+	 * 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;
+	}
 }

lib/public/Security/Events/ValidatePasswordPolicyEvent.php

@@ -9,26 +9,41 @@
 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;
+	}
 }

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;
+}

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());
+	}
+}