From 6eb2adc59b86d78ca1825009f2f27f1881c77740 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 18 Jan 2017 17:02:45 +0100 Subject: [PATCH 1/2] Revert "Use preDelete hook to delete encryption keys" This reverts commit 4559dad3e421f330eb8cbc718aa19e573a5ae2f7. --- apps/encryption/lib/Hooks/UserHooks.php | 6 +++--- apps/encryption/tests/Hooks/UserHooksTest.php | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/encryption/lib/Hooks/UserHooks.php b/apps/encryption/lib/Hooks/UserHooks.php index 03de88e3f187..8dabe5c33104 100644 --- a/apps/encryption/lib/Hooks/UserHooks.php +++ b/apps/encryption/lib/Hooks/UserHooks.php @@ -137,9 +137,9 @@ public function addHooks() { 'postCreateUser'); OCUtil::connectHook('OC_User', - 'pre_deleteUser', + 'post_deleteUser', $this, - 'preDeleteUser'); + 'postDeleteUser'); } } @@ -194,7 +194,7 @@ public function postCreateUser($params) { * @param array $params : uid, password * @note This method should never be called for users using client side encryption */ - public function preDeleteUser($params) { + public function postDeleteUser($params) { if (App::isEnabled('encryption')) { $this->keyManager->deletePublicKey($params['uid']); diff --git a/apps/encryption/tests/Hooks/UserHooksTest.php b/apps/encryption/tests/Hooks/UserHooksTest.php index 118587baf8c1..16b958716d56 100644 --- a/apps/encryption/tests/Hooks/UserHooksTest.php +++ b/apps/encryption/tests/Hooks/UserHooksTest.php @@ -108,12 +108,12 @@ public function testPostCreateUser() { $this->assertTrue(true); } - public function testPreDeleteUser() { + public function testPostDeleteUser() { $this->keyManagerMock->expects($this->once()) ->method('deletePublicKey') ->with('testUser'); - $this->instance->preDeleteUser($this->params); + $this->instance->postDeleteUser($this->params); $this->assertTrue(true); } From 933ee8c6ab8fb94801ddc4a74f0c75bf3066af12 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 18 Jan 2017 17:42:07 +0100 Subject: [PATCH 2/2] Ignore exception when deleting keys of deleted user Whenever a user was deleted for encryption where the keys are stored in the home, we can ignore user existence exceptions because it means the keys are already gone. --- lib/private/Encryption/Keys/Storage.php | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/private/Encryption/Keys/Storage.php b/lib/private/Encryption/Keys/Storage.php index 19d947090b9c..3c87d43132c3 100644 --- a/lib/private/Encryption/Keys/Storage.php +++ b/lib/private/Encryption/Keys/Storage.php @@ -28,6 +28,7 @@ use OC\Files\View; use OCP\Encryption\Keys\IStorage; use OCP\IUserSession; +use OC\User\NoUserException; class Storage implements IStorage { @@ -138,8 +139,21 @@ public function setSystemUserKey($keyId, $key, $encryptionModuleId) { * @inheritdoc */ public function deleteUserKey($uid, $keyId, $encryptionModuleId) { - $path = $this->constructUserKeyPath($encryptionModuleId, $keyId, $uid); - return !$this->view->file_exists($path) || $this->view->unlink($path); + try { + $path = $this->constructUserKeyPath($encryptionModuleId, $keyId, $uid); + return !$this->view->file_exists($path) || $this->view->unlink($path); + } catch (NoUserException $e) { + // this exception can come from initMountPoints() from setupUserMounts() + // for a deleted user. + // + // It means, that: + // - we are not running in alternative storage mode because we don't call + // initMountPoints() in that mode + // - the keys were in the user's home but since the user was deleted, the + // user's home is gone and so are the keys + // + // So there is nothing to do, just ignore. + } } /**