Merge pull request #18019 from nextcloud/enhancement/password-policy-events

Add typed events for password_policy

lib/composer/composer/autoload_classmap.php

@@ -386,6 +386,8 @@ return array(
     'OCP\\Search\\Provider' => $baseDir . '/lib/public/Search/Provider.php',
     'OCP\\Search\\Result' => $baseDir . '/lib/public/Search/Result.php',
     'OCP\\Security\\CSP\\AddContentSecurityPolicyEvent' => $baseDir . '/lib/public/Security/CSP/AddContentSecurityPolicyEvent.php',
+    'OCP\\Security\\Events\\GenerateSecurePasswordEvent' => $baseDir . '/lib/public/Security/Events/GenerateSecurePasswordEvent.php',
+    'OCP\\Security\\Events\\ValidatePasswordPolicyEvent' => $baseDir . '/lib/public/Security/Events/ValidatePasswordPolicyEvent.php',
     'OCP\\Security\\FeaturePolicy\\AddFeaturePolicyEvent' => $baseDir . '/lib/public/Security/FeaturePolicy/AddFeaturePolicyEvent.php',
     'OCP\\Security\\IContentSecurityPolicyManager' => $baseDir . '/lib/public/Security/IContentSecurityPolicyManager.php',
     'OCP\\Security\\ICredentialsManager' => $baseDir . '/lib/public/Security/ICredentialsManager.php',

lib/composer/composer/autoload_static.php

@@ -415,6 +415,8 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OCP\\Search\\Provider' => __DIR__ . '/../../..' . '/lib/public/Search/Provider.php',
         'OCP\\Search\\Result' => __DIR__ . '/../../..' . '/lib/public/Search/Result.php',
         'OCP\\Security\\CSP\\AddContentSecurityPolicyEvent' => __DIR__ . '/../../..' . '/lib/public/Security/CSP/AddContentSecurityPolicyEvent.php',
+        'OCP\\Security\\Events\\GenerateSecurePasswordEvent' => __DIR__ . '/../../..' . '/lib/public/Security/Events/GenerateSecurePasswordEvent.php',
+        'OCP\\Security\\Events\\ValidatePasswordPolicyEvent' => __DIR__ . '/../../..' . '/lib/public/Security/Events/ValidatePasswordPolicyEvent.php',
         'OCP\\Security\\FeaturePolicy\\AddFeaturePolicyEvent' => __DIR__ . '/../../..' . '/lib/public/Security/FeaturePolicy/AddFeaturePolicyEvent.php',
         'OCP\\Security\\IContentSecurityPolicyManager' => __DIR__ . '/../../..' . '/lib/public/Security/IContentSecurityPolicyManager.php',
         'OCP\\Security\\ICredentialsManager' => __DIR__ . '/../../..' . '/lib/public/Security/ICredentialsManager.php',

lib/private/Share20/Manager.php

@@ -55,6 +55,7 @@ use OCP\IUser;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
 use OCP\Mail\IMailer;
+use OCP\Security\Events\ValidatePasswordPolicyEvent;
 use OCP\Security\IHasher;
 use OCP\Security\ISecureRandom;
 use OCP\Share;
@@ -191,8 +192,7 @@ class Manager implements IManager {
 
 		// Let others verify the password
 		try {
-			$event = new GenericEvent($password);
+			$this->eventDispatcher->dispatch(new ValidatePasswordPolicyEvent($password));
-			$this->eventDispatcher->dispatch('OCP\PasswordPolicy::validate', $event);
 		} catch (HintException $e) {
 			throw new \Exception($e->getHint());
 		}

lib/private/User/Database.php

@@ -58,7 +58,9 @@ declare(strict_types=1);
 namespace OC\User;
 
 use OC\Cache\CappedMemoryCache;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IDBConnection;
+use OCP\Security\Events\ValidatePasswordPolicyEvent;
 use OCP\User\Backend\ABackend;
 use OCP\User\Backend\ICheckPasswordBackend;
 use OCP\User\Backend\ICountUsersBackend;
@@ -68,7 +70,6 @@ use OCP\User\Backend\IGetHomeBackend;
 use OCP\User\Backend\IGetRealUIDBackend;
 use OCP\User\Backend\ISetDisplayNameBackend;
 use OCP\User\Backend\ISetPasswordBackend;
-use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\GenericEvent;
 
 /**
@@ -86,7 +87,7 @@ class Database extends ABackend
 	/** @var CappedMemoryCache */
 	private $cache;
 
-	/** @var EventDispatcherInterface */
+	/** @var IEventDispatcher */
 	private $eventDispatcher;
 
 	/** @var IDBConnection */
@@ -98,13 +99,13 @@ class Database extends ABackend
 	/**
 	 * \OC\User\Database constructor.
 	 *
-	 * @param EventDispatcherInterface $eventDispatcher
+	 * @param IEventDispatcher $eventDispatcher
 	 * @param string $table
 	 */
 	public function __construct($eventDispatcher = null, $table = 'users') {
 		$this->cache = new CappedMemoryCache();
 		$this->table = $table;
-		$this->eventDispatcher = $eventDispatcher ? $eventDispatcher : \OC::$server->getEventDispatcher();
+		$this->eventDispatcher = $eventDispatcher ? $eventDispatcher : \OC::$server->query(IEventDispatcher::class);
 	}
 
 	/**
@@ -130,8 +131,7 @@ class Database extends ABackend
 		$this->fixDI();
 
 		if (!$this->userExists($uid)) {
-			$event = new GenericEvent($password);
+			$this->eventDispatcher->dispatchTyped(new ValidatePasswordPolicyEvent($password));
-			$this->eventDispatcher->dispatch('OCP\PasswordPolicy::validate', $event);
 
 			$qb = $this->dbConn->getQueryBuilder();
 			$qb->insert($this->table)
@@ -199,8 +199,7 @@ class Database extends ABackend
 		$this->fixDI();
 
 		if ($this->userExists($uid)) {
-			$event = new GenericEvent($password);
+			$this->eventDispatcher->dispatchTyped(new ValidatePasswordPolicyEvent($password));
-			$this->eventDispatcher->dispatch('OCP\PasswordPolicy::validate', $event);
 
 			$hasher = \OC::$server->getHasher();
 			$hashedPassword = $hasher->hash($password);

lib/public/Security/Events/GenerateSecurePasswordEvent.php

@@ -0,0 +1,50 @@
+<?php declare(strict_types=1);
+
+/**
+ * @copyright 2019 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @author 2019 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @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 OCP\Security\Events;
+
+use OCP\EventDispatcher\Event;
+
+/**
+ * @since 18.0.0
+ */
+class GenerateSecurePasswordEvent extends Event {
+
+	/** @var null|string */
+	private $password;
+
+	/**
+	 * @since 18.0.0
+	 */
+	public function getPassword(): ?string {
+		return $this->password;
+	}
+
+	/**
+	 * @since 18.0.0
+	 */
+	public function setPassword(string $password): void {
+		$this->password = $password;
+	}
+
+}

lib/public/Security/Events/ValidatePasswordPolicyEvent.php

@@ -0,0 +1,51 @@
+<?php declare(strict_types=1);
+
+/**
+ * @copyright 2019 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @author 2019 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @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 OCP\Security\Events;
+
+use OCP\EventDispatcher\Event;
+
+/**
+ * @since 18.0.0
+ */
+class ValidatePasswordPolicyEvent extends Event {
+
+	/** @var string */
+	private $password;
+
+	/**
+	 * @since 18.0.0
+	 */
+	public function __construct(string $password) {
+		parent::__construct();
+		$this->password = $password;
+	}
+
+	/**
+	 * @since 18.0.0
+	 */
+	public function getPassword(): string {
+		return $this->password;
+	}
+
+}

tests/lib/Share20/ManagerTest.php

@@ -27,6 +27,7 @@ use OC\Share20\DefaultShareProvider;
 use OC\Share20\Exception;
 use OC\Share20\Manager;
 use OC\Share20\Share;
+use OCP\EventDispatcher\Event;
 use OCP\Files\File;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
@@ -42,16 +43,18 @@ use OCP\ILogger;
 use OCP\IServerContainer;
 use OCP\IURLGenerator;
 use OCP\IUser;
-
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
 use OCP\Mail\IMailer;
+use OCP\Security\Events\ValidatePasswordPolicyEvent;
 use OCP\Security\IHasher;
 use OCP\Security\ISecureRandom;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IProviderFactory;
 use OCP\Share\IShare;
 use OCP\Share\IShareProvider;
+use PHPUnit\Framework\MockObject\MockBuilder;
+use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\GenericEvent;
 
@@ -65,37 +68,37 @@ class ManagerTest extends \Test\TestCase {
 
 	/** @var Manager */
 	protected $manager;
-	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var ILogger|MockObject */
 	protected $logger;
-	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IConfig|MockObject */
 	protected $config;
-	/** @var ISecureRandom|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var ISecureRandom|MockObject */
 	protected $secureRandom;
-	/** @var IHasher|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IHasher|MockObject */
 	protected $hasher;
-	/** @var IShareProvider|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IShareProvider|MockObject */
 	protected $defaultProvider;
-	/** @var  IMountManager|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var  IMountManager|MockObject */
 	protected $mountManager;
-	/** @var  IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var  IGroupManager|MockObject */
 	protected $groupManager;
-	/** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IL10N|MockObject */
 	protected $l;
-	/** @var IFactory|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IFactory|MockObject */
 	protected $l10nFactory;
 	/** @var DummyFactory */
 	protected $factory;
-	/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IUserManager|MockObject */
 	protected $userManager;
-	/** @var IRootFolder | \PHPUnit_Framework_MockObject_MockObject */
+	/** @var IRootFolder | MockObject */
 	protected $rootFolder;
-	/** @var  EventDispatcherInterface | \PHPUnit_Framework_MockObject_MockObject */
+	/** @var  EventDispatcherInterface | MockObject */
 	protected $eventDispatcher;
-	/** @var  IMailer|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var  IMailer|MockObject */
 	protected $mailer;
-	/** @var  IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var  IURLGenerator|MockObject */
 	protected $urlGenerator;
-	/** @var  \OC_Defaults|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var  \OC_Defaults|MockObject */
 	protected $defaults;
 
 	public function setUp() {
@@ -146,7 +149,7 @@ class ManagerTest extends \Test\TestCase {
 	}
 
 	/**
-	 * @return \PHPUnit_Framework_MockObject_MockBuilder
+	 * @return MockBuilder
 	 */
 	private function createManagerMock() {
 		return 	$this->getMockBuilder('\OC\Share20\Manager')
@@ -496,11 +499,12 @@ class ManagerTest extends \Test\TestCase {
 		]));
 
 		$this->eventDispatcher->expects($this->once())->method('dispatch')
-			->willReturnCallback(function($eventName, GenericEvent $event) {
+			->willReturnCallback(function(Event $event) {
-				$this->assertSame('OCP\PasswordPolicy::validate', $eventName);
+				$this->assertInstanceOf(ValidatePasswordPolicyEvent::class, $event);
-				$this->assertSame('password', $event->getSubject());
+				/** @var ValidatePasswordPolicyEvent $event */
+				$this->assertSame('password', $event->getPassword());
 			}
-			);
+		);
 
 		$result = self::invokePrivate($this->manager, 'verifyPassword', ['password']);
 		$this->assertNull($result);
@@ -516,12 +520,13 @@ class ManagerTest extends \Test\TestCase {
 		]));
 
 		$this->eventDispatcher->expects($this->once())->method('dispatch')
-			->willReturnCallback(function($eventName, GenericEvent $event) {
+			->willReturnCallback(function(Event $event) {
-				$this->assertSame('OCP\PasswordPolicy::validate', $eventName);
+				$this->assertInstanceOf(ValidatePasswordPolicyEvent::class, $event);
-				$this->assertSame('password', $event->getSubject());
+				/** @var ValidatePasswordPolicyEvent $event */
+				$this->assertSame('password', $event->getPassword());
 				throw new HintException('message', 'password not accepted');
 			}
-			);
+		);
 
 		self::invokePrivate($this->manager, 'verifyPassword', ['password']);
 	}

tests/lib/User/DatabaseTest.php

@@ -21,10 +21,14 @@
 */
 
 namespace Test\User;
+
 use OC\HintException;
 use OC\User\User;
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Security\Events\ValidatePasswordPolicyEvent;
+use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
-use Symfony\Component\EventDispatcher\GenericEvent;
 
 /**
  * Class DatabaseTest
@@ -34,7 +38,7 @@ use Symfony\Component\EventDispatcher\GenericEvent;
 class DatabaseTest extends Backend {
 	/** @var array */
 	private $users;
-	/** @var EventDispatcherInterface|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IEventDispatcher|MockObject */
 	private $eventDispatcher;
 
 	public function getUser() {
@@ -46,7 +50,7 @@ class DatabaseTest extends Backend {
 	protected function setUp() {
 		parent::setUp();
 
-		$this->eventDispatcher = $this->createMock(EventDispatcherInterface::class);
+		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
 
 		$this->backend=new \OC\User\Database($this->eventDispatcher);
 	}
@@ -65,11 +69,12 @@ class DatabaseTest extends Backend {
 		$user = $this->getUser();
 		$this->backend->createUser($user, 'pass1');
 
-		$this->eventDispatcher->expects($this->once())->method('dispatch')
+		$this->eventDispatcher->expects($this->once())->method('dispatchTyped')
 			->willReturnCallback(
-				function ($eventName, GenericEvent $event) {
+				function (Event $event) {
-					$this->assertSame('OCP\PasswordPolicy::validate',  $eventName);
+					$this->assertInstanceOf(ValidatePasswordPolicyEvent::class, $event);
-					$this->assertSame('newpass', $event->getSubject());
+					/** @var ValidatePasswordPolicyEvent $event */
+					$this->assertSame('newpass', $event->getPassword());
 				}
 			);
 
@@ -85,11 +90,12 @@ class DatabaseTest extends Backend {
 		$user = $this->getUser();
 		$this->backend->createUser($user, 'pass1');
 
-		$this->eventDispatcher->expects($this->once())->method('dispatch')
+		$this->eventDispatcher->expects($this->once())->method('dispatchTyped')
 			->willReturnCallback(
-				function ($eventName, GenericEvent $event) {
+				function (Event $event) {
-					$this->assertSame('OCP\PasswordPolicy::validate', $eventName);
+					$this->assertInstanceOf(ValidatePasswordPolicyEvent::class, $event);
-					$this->assertSame('newpass', $event->getSubject());
+					/** @var ValidatePasswordPolicyEvent $event */
+					$this->assertSame('newpass', $event->getPassword());
 					throw new HintException('password change failed', 'password change failed');
 				}
 			);
@@ -124,8 +130,8 @@ class DatabaseTest extends Backend {
 		$user2 = $this->getUser();
 		$this->backend->createUser($user2, 'pass1');
 
-		$user1Obj = new User($user1, $this->backend, $this->eventDispatcher);
+		$user1Obj = new User($user1, $this->backend, $this->createMock(EventDispatcherInterface::class));
-		$user2Obj = new User($user2, $this->backend, $this->eventDispatcher);
+		$user2Obj = new User($user2, $this->backend, $this->createMock(EventDispatcherInterface::class));
 		$emailAddr1 = "$user1@nextcloud.com";
 		$emailAddr2 = "$user2@nextcloud.com";