owncloud · JammingBen · Jan 18, 2021 · Dec 16, 2020 · Dec 22, 2020 · Jan 13, 2021
diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php
@@ -92,7 +92,9 @@ public function registerHooks() {
 					$container->query('Util'),
 					$container->query('Session'),
 					$container->query('Crypt'),
-					$container->query('Recovery'), $server->getConfig())
+					$container->query('Recovery'),
+					$server->getConfig(),
+					$server->getEventDispatcher())
 			]);
 
 			$hookManager->fireHooks();

diff --git a/lib/Hooks/UserHooks.php b/lib/Hooks/UserHooks.php
@@ -38,6 +38,8 @@
 use OCA\Encryption\Util;
 use OCA\Encryption\Session;
 use OCA\Encryption\Recovery;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\EventDispatcher\GenericEvent;
 
 class UserHooks implements IHook {
 	/**
@@ -80,6 +82,10 @@ class UserHooks implements IHook {
 	 * @var IConfig
 	 */
 	private $config;
+	/**
+	 * @var EventDispatcherInterface
+	 */
+	private $eventDispatcher;
 
 	/**
 	 * UserHooks constructor.
@@ -94,6 +100,7 @@ class UserHooks implements IHook {
 	 * @param Crypt $crypt
 	 * @param Recovery $recovery
 	 * @param IConfig $config
+	 * @param EventDispatcherInterface $eventDispatcher
 	 */
 	public function __construct(KeyManager $keyManager,
 								IUserManager $userManager,
@@ -103,7 +110,9 @@ public function __construct(KeyManager $keyManager,
 								Util $util,
 								Session $session,
 								Crypt $crypt,
-								Recovery $recovery, IConfig $config) {
+								Recovery $recovery,
+								IConfig $config,
+								EventDispatcherInterface $eventDispatcher) {
 		$this->keyManager = $keyManager;
 		$this->userManager = $userManager;
 		$this->logger = $logger;
@@ -114,6 +123,7 @@ public function __construct(KeyManager $keyManager,
 		$this->recovery = $recovery;
 		$this->crypt = $crypt;
 		$this->config = $config;
+		$this->eventDispatcher = $eventDispatcher;
 	}
 
 	/**
@@ -122,59 +132,44 @@ public function __construct(KeyManager $keyManager,
 	 * @return void|null
 	 */
 	public function addHooks() {
-		OCUtil::connectHook('OC_User', 'post_login', $this, 'login');
-		OCUtil::connectHook('OC_User', 'logout', $this, 'logout');
+		$this->eventDispatcher->addListener('user.afterlogin', [$this, 'login']);
+		$this->eventDispatcher->addListener('user.beforelogout', [$this, 'logout']);
 
 		// this hooks only make sense if no master key is used
 		if ($this->util->isMasterKeyEnabled() === false) {
-			OCUtil::connectHook('OC_User',
-				'post_setPassword',
-				$this,
-				'setPassphrase');
-
-			OCUtil::connectHook('OC_User',
-				'pre_setPassword',
-				$this,
-				'preSetPassphrase');
-
-			OCUtil::connectHook('OC_User',
-				'post_createUser',
-				$this,
-				'postCreateUser');
-
-			OCUtil::connectHook('OC_User',
-				'post_deleteUser',
-				$this,
-				'postDeleteUser');
+			$this->eventDispatcher->addListener('user.aftersetpassword', [$this, 'setPassphrase']);
+			$this->eventDispatcher->addListener('user.beforesetpassword', [$this, 'preSetPassphrase']);
+			$this->eventDispatcher->addListener('user.aftercreate', [$this, 'postCreateUser']);
+			$this->eventDispatcher->addListener('user.afterdelete', [$this, 'postDeleteUser']);
 		}
 	}
 
 	/**
 	 * Startup encryption backend upon user login
 	 *
 	 * @note This method should never be called for users using client side encryption
-	 * @param array $params
+	 * @param GenericEvent $params
 	 * @return boolean|null|void
 	 */
-	public function login($params) {
+	public function login(GenericEvent $params) {
 		if (!App::isEnabled('encryption')) {
 			return true;
 		}
 
 		// ensure filesystem is loaded
 		if (!\OC\Files\Filesystem::$loaded) {
-			$this->setupFS($params['uid']);
+			$this->setupFS($params->getArgument('uid'));
 		}
 		if ($this->util->isMasterKeyEnabled() === false) {
-			$this->userSetup->setupUser($params['uid'], $params['password']);
+			$this->userSetup->setupUser($params->getArgument('uid'), $params->getArgument('password'));
 		}
 
 		if (($this->util->isMasterKeyEnabled() === false) &&
 			($this->config->getAppValue('encryption', 'userSpecificKey', '') === '')) {
 			$this->config->setAppValue('encryption', 'userSpecificKey', '1');
 		}
 
-		$this->keyManager->init($params['uid'], $params['password']);
+		$this->keyManager->init($params->getArgument('uid'), $params->getArgument('password'));
 	}
 
 	/**
@@ -188,43 +183,44 @@ public function logout() {
 	 * setup encryption backend upon user created
 	 *
 	 * @note This method should never be called for users using client side encryption
-	 * @param array $params
+	 * @param GenericEvent $params
+	 * @return void
 	 */
 	public function postCreateUser($params) {
 		if (App::isEnabled('encryption')) {
-			$this->userSetup->setupUser($params['uid'], $params['password']);
+			$this->userSetup->setupUser($params->getArgument('uid'), $params->getArgument('password'));
 		}
 	}
 
 	/**
 	 * cleanup encryption backend upon user deleted
 	 *
-	 * @param array $params : uid, password
 	 * @note This method should never be called for users using client side encryption
+	 * @param GenericEvent $params
+	 * @return void
 	 */
-	public function postDeleteUser($params) {
+	public function postDeleteUser(GenericEvent $params) {
 		if (App::isEnabled('encryption')) {
 			/**
 			 * Adding a safe condition to make sure the uid is not
 			 * empty or null.
 			 */
-			if (($params['uid'] !== null) && ($params['uid'] !== '')) {
-				$this->keyManager->deletePublicKey($params['uid']);
-				\OC::$server->getEncryptionKeyStorage()->deleteAltUserStorageKeys($params['uid']);
+			if ($params->getArgument('uid') !== null && $params->getArgument('uid') !== '') {
+				$this->keyManager->deletePublicKey($params->getArgument('uid'));
+				\OC::$server->getEncryptionKeyStorage()->deleteAltUserStorageKeys($params->getArgument('uid'));
 			}
 		}
 	}
 
 	/**
 	 * If the password can't be changed within ownCloud, than update the key password in advance.
 	 *
-	 * @param array $params : uid, password
+	 * @param GenericEvent $params : uid, password
 	 * @return void
 	 */
-	public function preSetPassphrase($params) {
+	public function preSetPassphrase(GenericEvent $params) {
 		if (App::isEnabled('encryption')) {
-			$user = $this->userManager->get($params['uid']);
-
+			$user = $params->getArgument('user');
 			if ($user && !$user->canChangePassword()) {
 				$this->setPassphrase($params);
 			}
@@ -234,12 +230,15 @@ public function preSetPassphrase($params) {
 	/**
 	 * Change a user's encryption passphrase
 	 *
-	 * @param array $params keys: uid, password
+	 * @param GenericEvent $params keys: uid, password
 	 * @return void
 	 */
-	public function setPassphrase($params) {
+	public function setPassphrase(GenericEvent $params) {
 		$privateKey = null;
 		$user = null;
+		$userFromParams = $params->getArgument('user');
+		$userIdFromParams = $userFromParams->getUID();
+
 		//Check if the session is there or not
 		if ($this->user->getUser() !== null) {
 			// Get existing decrypted private key
@@ -248,10 +247,9 @@ public function setPassphrase($params) {
 		}
 
 		// current logged in user changes his own password
-		if ($user !== null && $params['uid'] === $user->getUID() && $privateKey) {
-
+		if ($user !== null && $userIdFromParams === $user->getUID() && $privateKey) {
 			// Encrypt private key with new user pwd as passphrase
-			$encryptedPrivateKey = $this->crypt->encryptPrivateKey($privateKey, $params['password'], $params['uid']);
+			$encryptedPrivateKey = $this->crypt->encryptPrivateKey($privateKey, $params->getArgument('password'), $userIdFromParams);
 
 			// Save private key
 			if ($encryptedPrivateKey) {
@@ -265,9 +263,9 @@ public function setPassphrase($params) {
 			// private key has not changed, only the passphrase
 			// used to decrypt it has changed
 		} else { // admin changed the password for a different user, create new keys and re-encrypt file keys
-			$user = $params['uid'];
+			$user = $userIdFromParams;
 			$this->initMountPoints($user);
-			$recoveryPassword = isset($params['recoveryPassword']) ? $params['recoveryPassword'] : null;
+			$recoveryPassword = $params->hasArgument('recoveryPassword') ? $params->getArgument('recoveryPassword') : null;
 
 			// we generate new keys if...
 			// ...we have a recovery password and the user enabled the recovery key
@@ -278,11 +276,10 @@ public function setPassphrase($params) {
 				|| !$this->keyManager->userHasKeys($user)
 				|| !$this->util->userHasFiles($user)
 			) {
-
 				// backup old keys
 				//$this->backupAllKeys('recovery');
 
-				$newUserPassword = $params['password'];
+				$newUserPassword = $params->getArgument('password');
 
 				$keyPair = $this->crypt->createKeyPair();
 

diff --git a/tests/unit/Command/FixEncryptedVersionTest.php b/tests/unit/Command/FixEncryptedVersionTest.php
@@ -78,6 +78,7 @@ public static function setUpBeforeClass(): void {
 
 	public static function tearDownAfterClass(): void {
 		parent::tearDownAfterClass();
+		\OC\Files\Filesystem::clearMounts();
 		$user = \OC::$server->getUserManager()->get(self::TEST_ENCRYPTION_VERSION_AFFECTED_USER);
 		if ($user !== null) {
 			$user->delete();

diff --git a/tests/unit/Hooks/UserHooksTest.php b/tests/unit/Hooks/UserHooksTest.php
@@ -38,6 +38,8 @@
 use OCP\IUserManager;
 use OCP\IUserSession;
 use PHPUnit\Framework\MockObject\MockObject;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\EventDispatcher\GenericEvent;
 use Test\TestCase;
 
 /**
@@ -91,7 +93,10 @@ class UserHooksTest extends TestCase {
 	 */
 	private $instance;
 
-	private $params = ['uid' => 'testUser', 'password' => 'password'];
+	/** @var  EventDispatcher */
+	private $eventDispatcher;
+
+	private $params;
 
 	private $keyPair = ['publicKey' => 'abcd', 'privateKey' => 'efgh'];
 
@@ -136,19 +141,20 @@ public function testPostDeleteUser() {
 		$this->assertNull($this->instance->postDeleteUser($this->params));
 	}
 
+	public function dataTestPreSetPassphrase() {
+		return [
+			[true],
+			[false]
+		];
+	}
+
 	/**
 	 * @dataProvider dataTestPreSetPassphrase
 	 */
 	public function testPreSetPassphrase($canChange) {
 		/** @var UserHooks | MockObject  $instance */
 		$instance = $this->getInstanceMock(['setPassphrase']);
-
-		$userMock = $this->createMock(IUser::class);
-
-		$this->userManagerMock->expects($this->once())
-			->method('get')
-			->with($this->params['uid'])
-			->willReturn($userMock);
+		$userMock = $this->params['user'];
 		$userMock->expects($this->once())
 			->method('canChangePassword')
 			->willReturn($canChange);
@@ -167,13 +173,6 @@ public function testPreSetPassphrase($canChange) {
 		$this->assertNull($instance->preSetPassphrase($this->params));
 	}
 
-	public function dataTestPreSetPassphrase() {
-		return [
-			[true],
-			[false]
-		];
-	}
-
 	public function testSetPassphrase() {
 		$this->sessionMock->expects($this->exactly(4))
 			->method('getPrivateKey')
@@ -341,7 +340,8 @@ protected function getInstanceMock($methods) {
 					$this->sessionMock,
 					$this->cryptMock,
 					$this->recoveryMock,
-					$this->config
+					$this->config,
+					$this->eventDispatcher
 			])
 			->onlyMethods($methods)
 			->getMock();
@@ -396,7 +396,11 @@ protected function setUp(): void {
 		$this->recoveryMock = $recoveryMock;
 		$this->utilMock = $this->createMock(Util::class);
 		$this->utilMock->expects($this->any())->method('isMasterKeyEnabled')->willReturn(false);
+		$this->eventDispatcher = $this->createMock(EventDispatcher::class);
 
+		$userMock = $this->createMock(IUser::class);
+		$userMock->expects($this->any())->method('getUID')->willReturn('testUser');
+		$this->params = new GenericEvent(null, ['uid' => 'testUser', 'password' => 'password', 'user' => $userMock]);
 		$this->instance = $this->getInstanceMock(['setupFS']);
 	}