create new encryption keys on password reset and backup the old one

Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>

4 files changed, 85 insertions, 37 deletions

diff --git a/apps/encryption/lib/Hooks/UserHooks.php b/apps/encryption/lib/Hooks/UserHooks.php
index 16e4e962432..d189ce3eeef 100644
--- a/apps/encryption/lib/Hooks/UserHooks.php
+++ b/apps/encryption/lib/Hooks/UserHooks.php
@@ -40,6 +40,13 @@ use OCA\Encryption\Session;
 use OCA\Encryption\Recovery;
 
 class UserHooks implements IHook {
+
+	/**
+	 * list of user for which we perform a password reset
+	 * @var array
+	 */
+	protected static $passwordResetUsers = [];
+
 	/**
 	 * @var KeyManager
 	 */
@@ -132,6 +139,16 @@ class UserHooks implements IHook {
 				$this,
 				'preSetPassphrase');
 
+			OCUtil::connectHook('\OC\Core\LostPassword\Controller\LostController',
+				'post_passwordReset',
+				$this,
+				'postPasswordReset');
+
+			OCUtil::connectHook('\OC\Core\LostPassword\Controller\LostController',
+				'pre_passwordReset',
+				$this,
+				'prePasswordReset');
+
 			OCUtil::connectHook('OC_User',
 				'post_createUser',
 				$this,
@@ -202,6 +219,22 @@ class UserHooks implements IHook {
 		}
 	}
 
+	public function prePasswordReset($params) {
+		if (App::isEnabled('encryption')) {
+			$user = $params['uid'];
+			self::$passwordResetUsers[$user] = true;
+		}
+	}
+
+	public function postPasswordReset($params) {
+		$uid = $params['uid'];
+		$password = $params['password'];
+		$this->keyManager->backupUserKeys('passwordReset', $uid);
+		$this->keyManager->deleteUserKeys($uid);
+		$this->userSetup->setupUser($uid, $password);
+		unset(self::$passwordResetUsers[$uid]);
+	}
+
 	/**
 	 * If the password can't be changed within ownCloud, than update the key password in advance.
 	 *
@@ -209,13 +242,10 @@ class UserHooks implements IHook {
 	 * @return boolean|null
 	 */
 	public function preSetPassphrase($params) {
-		if (App::isEnabled('encryption')) {
-
-			$user = $this->userManager->get($params['uid']);
+		$user = $this->userManager->get($params['uid']);
 
-			if ($user && !$user->canChangePassword()) {
-				$this->setPassphrase($params);
-			}
+		if ($user && !$user->canChangePassword()) {
+			$this->setPassphrase($params);
 		}
 	}
 
@@ -227,6 +257,12 @@ class UserHooks implements IHook {
 	 */
 	public function setPassphrase($params) {
 
+		// if we are in the process to resetting a user password, we have nothing
+		// to do here
+		if (isset(self::$passwordResetUsers[$params['uid']])) {
+			return true;
+		}
+
 		// Get existing decrypted private key
 		$privateKey = $this->session->getPrivateKey();
 		$user = $this->user->getUser();
@@ -299,19 +335,6 @@ class UserHooks implements IHook {
 		Filesystem::initMountPoints($user);
 	}
 
-
-	/**
-	 * after password reset we create a new key pair for the user
-	 *
-	 * @param array $params
-	 */
-	public function postPasswordReset($params) {
-		$password = $params['password'];
-
-		$this->keyManager->deleteUserKeys($params['uid']);
-		$this->userSetup->setupUser($params['uid'], $password);
-	}
-
 	/**
 	 * setup file system for user
 	 *
diff --git a/apps/encryption/lib/KeyManager.php b/apps/encryption/lib/KeyManager.php
index 26f023ed8f9..caae154b2d3 100644
--- a/apps/encryption/lib/KeyManager.php
+++ b/apps/encryption/lib/KeyManager.php
@@ -560,11 +560,10 @@ class KeyManager {
 
 	/**
 	 * @param string $purpose
-	 * @param bool $timestamp
-	 * @param bool $includeUserKeys
+	 * @param string $uid
 	 */
-	public function backupAllKeys($purpose, $timestamp = true, $includeUserKeys = true) {
-//		$backupDir = $this->keyStorage->;
+	public function backupUserKeys($purpose, $uid) {
+		$this->keyStorage->backupUserKeys(Encryption::ID, $purpose, $uid);
 	}
 
 	/**
@@ -573,7 +572,6 @@ class KeyManager {
 	 * @param string $uid
 	 */
 	public function deleteUserKeys($uid) {
-		$this->backupAllKeys('password_reset');
 		$this->deletePublicKey($uid);
 		$this->deletePrivateKey($uid);
 	}
diff --git a/apps/encryption/tests/Hooks/UserHooksTest.php b/apps/encryption/tests/Hooks/UserHooksTest.php
index 43cc54f8901..f9477e3e038 100644
--- a/apps/encryption/tests/Hooks/UserHooksTest.php
+++ b/apps/encryption/tests/Hooks/UserHooksTest.php
@@ -120,6 +120,31 @@ class UserHooksTest extends TestCase {
 		$this->assertTrue(true);
 	}
 
+	public function testPrePasswordReset() {
+		$params = ['uid' => 'user1'];
+		$expected = ['user1' => true];
+		$this->instance->prePasswordReset($params);
+		$passwordResetUsers = $this->invokePrivate($this->instance, 'passwordResetUsers');
+
+		$this->assertSame($expected, $passwordResetUsers);
+	}
+
+	public function testPostPasswordReset() {
+		$params = ['uid' => 'user1', 'password' => 'password'];
+		$this->invokePrivate($this->instance, 'passwordResetUsers', [['user1' => true]]);
+		$this->keyManagerMock->expects($this->once())->method('backupUserKeys')
+			->with('passwordReset', 'user1');
+		$this->keyManagerMock->expects($this->once())->method('deleteUserKeys')
+			->with('user1');
+		$this->userSetupMock->expects($this->once())->method('setupUser')
+			->with('user1', 'password');
+
+		$this->instance->postPasswordReset($params);
+		$passwordResetUsers = $this->invokePrivate($this->instance, 'passwordResetUsers');
+		$this->assertEmpty($passwordResetUsers);
+
+	}
+
 	/**
 	 * @dataProvider dataTestPreSetPassphrase
 	 */
@@ -252,6 +277,15 @@ class UserHooksTest extends TestCase {
 		$this->assertNull($this->instance->setPassphrase($this->params));
 	}
 
+	public function testSetPassphraseResetUserMode() {
+		$params = ['uid' => 'user1', 'password' => 'password'];
+		$this->invokePrivate($this->instance, 'passwordResetUsers', [[$params['uid'] => true]]);
+		$this->sessionMock->expects($this->never())->method('getPrivateKey');
+		$this->keyManagerMock->expects($this->never())->method('setPrivateKey');
+		$this->assertTrue($this->instance->setPassphrase($params));
+		$this->invokePrivate($this->instance, 'passwordResetUsers', [[]]);
+	}
+
 	public function testSetPasswordNoUser() {
 		$this->sessionMock->expects($this->once())
 			->method('getPrivateKey')
@@ -287,19 +321,6 @@ class UserHooksTest extends TestCase {
 		$this->assertNull($userHooks->setPassphrase($this->params));
 	}
 
-	public function testPostPasswordReset() {
-		$this->keyManagerMock->expects($this->once())
-			->method('deleteUserKeys')
-			->with('testUser');
-
-		$this->userSetupMock->expects($this->once())
-			->method('setupUser')
-			->with('testUser', 'password');
-
-		$this->instance->postPasswordReset($this->params);
-		$this->assertTrue(true);
-	}
-
 	protected function setUp() {
 		parent::setUp();
 		$this->loggerMock = $this->createMock(ILogger::class);
diff --git a/apps/encryption/tests/KeyManagerTest.php b/apps/encryption/tests/KeyManagerTest.php
index fec311afa35..40def135816 100644
--- a/apps/encryption/tests/KeyManagerTest.php
+++ b/apps/encryption/tests/KeyManagerTest.php
@@ -657,4 +657,10 @@ class KeyManagerTest extends TestCase {
 		$this->instance->setVersion('/admin/files/myfile.txt', 5, $view);
 	}
 
+	public function testBackupUserKeys() {
+		$this->keyStorageMock->expects($this->once())->method('backupUserKeys')
+			->with('OC_DEFAULT_MODULE', 'test', 'user1');
+		$this->instance->backupUserKeys('test', 'user1');
+	}
+
 }