From fc4127dd62bdd1d9bd9339797607615a250ba33f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 22 Apr 2015 11:18:18 +0200 Subject: [PATCH 1/9] add $encryptionModuleId to methods of Keys/IStorage --- apps/encryption/appinfo/application.php | 18 +-- apps/encryption/lib/keymanager.php | 54 +++++--- .../encryption/settings/settings-personal.php | 2 +- apps/encryption_dummy/lib/dummymodule.php | 4 +- lib/private/encryption/keys/factory.php | 50 ------- lib/private/encryption/keys/storage.php | 128 ++++++------------ .../files/storage/wrapper/encryption.php | 9 +- lib/private/server.php | 25 ++-- lib/public/encryption/keys/istorage.php | 33 +++-- lib/public/iservercontainer.php | 4 +- settings/changepassword/controller.php | 2 +- tests/lib/encryption/keys/storage.php | 27 ++-- .../lib/files/storage/wrapper/encryption.php | 2 +- 13 files changed, 140 insertions(+), 218 deletions(-) delete mode 100644 lib/private/encryption/keys/factory.php diff --git a/apps/encryption/appinfo/application.php b/apps/encryption/appinfo/application.php index 243e227b6b..fa620992c8 100644 --- a/apps/encryption/appinfo/application.php +++ b/apps/encryption/appinfo/application.php @@ -24,8 +24,10 @@ namespace OCA\Encryption\AppInfo; -use OC\Files\Filesystem; use OC\Files\View; +use OCA\Encryption\Controller\RecoveryController; +use OCA\Encryption\Controller\SettingsController; +use OCA\Encryption\Controller\StatusController; use OCA\Encryption\Crypto\Crypt; use OCA\Encryption\Crypto\Encryption; use OCA\Encryption\HookManager; @@ -126,11 +128,11 @@ class Application extends \OCP\AppFramework\App { function (IAppContainer $c) { $server = $c->getServer(); - return new KeyManager($server->getEncryptionKeyStorage(\OCA\Encryption\Crypto\Encryption::ID), + return new KeyManager($server->getEncryptionKeyStorage(), $c->query('Crypt'), $server->getConfig(), $server->getUserSession(), - new \OCA\Encryption\Session($server->getSession()), + new Session($server->getSession()), $server->getLogger(), $c->query('Util') ); @@ -146,14 +148,14 @@ class Application extends \OCP\AppFramework\App { $server->getSecureRandom(), $c->query('KeyManager'), $server->getConfig(), - $server->getEncryptionKeyStorage(\OCA\Encryption\Crypto\Encryption::ID), + $server->getEncryptionKeyStorage(), $server->getEncryptionFilesHelper(), - new \OC\Files\View()); + new View()); }); $container->registerService('RecoveryController', function (IAppContainer $c) { $server = $c->getServer(); - return new \OCA\Encryption\Controller\RecoveryController( + return new RecoveryController( $c->getAppName(), $server->getRequest(), $server->getConfig(), @@ -163,7 +165,7 @@ class Application extends \OCP\AppFramework\App { $container->registerService('StatusController', function (IAppContainer $c) { $server = $c->getServer(); - return new \OCA\Encryption\Controller\StatusController( + return new StatusController( $c->getAppName(), $server->getRequest(), $server->getL10N($c->getAppName()), @@ -173,7 +175,7 @@ class Application extends \OCP\AppFramework\App { $container->registerService('SettingsController', function (IAppContainer $c) { $server = $c->getServer(); - return new \OCA\Encryption\Controller\SettingsController( + return new SettingsController( $c->getAppName(), $server->getRequest(), $server->getL10N($c->getAppName()), diff --git a/apps/encryption/lib/keymanager.php b/apps/encryption/lib/keymanager.php index b451b5c25a..1e6f3d29be 100644 --- a/apps/encryption/lib/keymanager.php +++ b/apps/encryption/lib/keymanager.php @@ -23,6 +23,7 @@ namespace OCA\Encryption; use OC\Encryption\Exceptions\DecryptionFailedException; +use OCA\Encryption\Crypto\Encryption; use OCA\Encryption\Exceptions\PrivateKeyMissingException; use OCA\Encryption\Exceptions\PublicKeyMissingException; use OCA\Encryption\Crypto\Crypt; @@ -136,7 +137,8 @@ class KeyManager { // Save public key $this->keyStorage->setSystemUserKey( - $this->publicShareKeyId . '.publicKey', $keyPair['publicKey']); + $this->publicShareKeyId . '.publicKey', $keyPair['publicKey'], + Encryption::ID); // Encrypt private key empty passphrase $encryptedKey = $this->crypt->symmetricEncryptFileContent($keyPair['privateKey'], ''); @@ -162,7 +164,7 @@ class KeyManager { * @return string */ public function getRecoveryKey() { - return $this->keyStorage->getSystemUserKey($this->recoveryKeyId . '.publicKey'); + return $this->keyStorage->getSystemUserKey($this->recoveryKeyId . '.publicKey', Encryption::ID); } /** @@ -179,7 +181,7 @@ class KeyManager { * @return bool */ public function checkRecoveryPassword($password) { - $recoveryKey = $this->keyStorage->getSystemUserKey($this->recoveryKeyId . '.privateKey'); + $recoveryKey = $this->keyStorage->getSystemUserKey($this->recoveryKeyId . '.privateKey', Encryption::ID); $decryptedRecoveryKey = $this->crypt->decryptPrivateKey($recoveryKey, $password); @@ -217,7 +219,10 @@ class KeyManager { */ public function setRecoveryKey($password, $keyPair) { // Save Public Key - $this->keyStorage->setSystemUserKey($this->getRecoveryKeyId(). '.publicKey', $keyPair['publicKey']); + $this->keyStorage->setSystemUserKey($this->getRecoveryKeyId(). + '.publicKey', + $keyPair['publicKey'], + Encryption::ID); $encryptedKey = $this->crypt->symmetricEncryptFileContent($keyPair['privateKey'], $password); @@ -236,7 +241,7 @@ class KeyManager { * @return bool */ public function setPublicKey($userId, $key) { - return $this->keyStorage->setUserKey($userId, $this->publicKeyId, $key); + return $this->keyStorage->setUserKey($userId, $this->publicKeyId, $key, Encryption::ID); } /** @@ -247,7 +252,8 @@ class KeyManager { public function setPrivateKey($userId, $key) { return $this->keyStorage->setUserKey($userId, $this->privateKeyId, - $key); + $key, + Encryption::ID); } /** @@ -258,7 +264,7 @@ class KeyManager { * @return boolean */ public function setFileKey($path, $key) { - return $this->keyStorage->setFileKey($path, $this->fileKeyId, $key); + return $this->keyStorage->setFileKey($path, $this->fileKeyId, $key, Encryption::ID); } /** @@ -284,7 +290,7 @@ class KeyManager { */ public function setShareKey($path, $uid, $key) { $keyId = $uid . '.' . $this->shareKeyId; - return $this->keyStorage->setFileKey($path, $keyId, $key); + return $this->keyStorage->setFileKey($path, $keyId, $key, Encryption::ID); } /** @@ -324,7 +330,7 @@ class KeyManager { */ public function getPrivateKey($userId) { $privateKey = $this->keyStorage->getUserKey($userId, - $this->privateKeyId); + $this->privateKeyId, Encryption::ID); if (strlen($privateKey) !== 0) { return $privateKey; @@ -338,12 +344,12 @@ class KeyManager { * @return string */ public function getFileKey($path, $uid) { - $encryptedFileKey = $this->keyStorage->getFileKey($path, $this->fileKeyId); + $encryptedFileKey = $this->keyStorage->getFileKey($path, $this->fileKeyId, Encryption::ID); if (is_null($uid)) { $uid = $this->getPublicShareKeyId(); $shareKey = $this->getShareKey($path, $uid); - $privateKey = $this->keyStorage->getSystemUserKey($this->publicShareKeyId . '.privateKey'); + $privateKey = $this->keyStorage->getSystemUserKey($this->publicShareKeyId . '.privateKey', Encryption::ID); $privateKey = $this->crypt->decryptPrivateKey($privateKey); } else { $shareKey = $this->getShareKey($path, $uid); @@ -367,7 +373,7 @@ class KeyManager { */ public function getEncryptedFileKey($path) { $encryptedFileKey = $this->keyStorage->getFileKey($path, - $this->fileKeyId); + $this->fileKeyId, Encryption::ID); return $encryptedFileKey; } @@ -380,7 +386,10 @@ class KeyManager { * @return boolean */ public function deleteShareKey($path, $keyId) { - return $this->keyStorage->deleteFileKey($path, $keyId . '.' . $this->shareKeyId); + return $this->keyStorage->deleteFileKey( + $path, + $keyId . '.' . $this->shareKeyId, + Encryption::ID); } @@ -391,7 +400,7 @@ class KeyManager { */ public function getShareKey($path, $uid) { $keyId = $uid . '.' . $this->shareKeyId; - return $this->keyStorage->getFileKey($path, $keyId); + return $this->keyStorage->getFileKey($path, $keyId, Encryption::ID); } /** @@ -416,7 +425,7 @@ class KeyManager { * @throws PublicKeyMissingException */ public function getPublicKey($userId) { - $publicKey = $this->keyStorage->getUserKey($userId, $this->publicKeyId); + $publicKey = $this->keyStorage->getUserKey($userId, $this->publicKeyId, Encryption::ID); if (strlen($publicKey) !== 0) { return $publicKey; @@ -434,7 +443,7 @@ class KeyManager { * @return string */ public function getPublicShareKey() { - return $this->keyStorage->getSystemUserKey($this->publicShareKeyId . '.publicKey'); + return $this->keyStorage->getSystemUserKey($this->publicShareKeyId . '.publicKey', Encryption::ID); } /** @@ -460,7 +469,7 @@ class KeyManager { * @return bool */ public function deletePublicKey($uid) { - return $this->keyStorage->deleteUserKey($uid, $this->publicKeyId); + return $this->keyStorage->deleteUserKey($uid, $this->publicKeyId, Encryption::ID); } /** @@ -468,11 +477,11 @@ class KeyManager { * @return bool */ private function deletePrivateKey($uid) { - return $this->keyStorage->deleteUserKey($uid, $this->privateKeyId); + return $this->keyStorage->deleteUserKey($uid, $this->privateKeyId, Encryption::ID); } public function deleteAllFileKeys($path) { - return $this->keyStorage->deleteAllFileKeys($path); + return $this->keyStorage->deleteAllFileKeys($path, Encryption::ID); } /** @@ -500,7 +509,7 @@ class KeyManager { * @return string returns openssl key */ public function getSystemPrivateKey($keyId) { - return $this->keyStorage->getSystemUserKey($keyId . '.' . $this->privateKeyId); + return $this->keyStorage->getSystemUserKey($keyId . '.' . $this->privateKeyId, Encryption::ID); } /** @@ -509,7 +518,10 @@ class KeyManager { * @return string returns openssl key */ public function setSystemPrivateKey($keyId, $key) { - return $this->keyStorage->setSystemUserKey($keyId . '.' . $this->privateKeyId, $key); + return $this->keyStorage->setSystemUserKey( + $keyId . '.' . $this->privateKeyId, + $key, + Encryption::ID); } /** diff --git a/apps/encryption/settings/settings-personal.php b/apps/encryption/settings/settings-personal.php index abbe62af61..01e1bdab0e 100644 --- a/apps/encryption/settings/settings-personal.php +++ b/apps/encryption/settings/settings-personal.php @@ -38,7 +38,7 @@ $util = new \OCA\Encryption\Util( \OC::$server->getConfig()); $keyManager = new \OCA\Encryption\KeyManager( - \OC::$server->getEncryptionKeyStorage(\OCA\Encryption\Crypto\Encryption::ID), + \OC::$server->getEncryptionKeyStorage(), $crypt, \OC::$server->getConfig(), $userSession, diff --git a/apps/encryption_dummy/lib/dummymodule.php b/apps/encryption_dummy/lib/dummymodule.php index 813b50edcb..e974ee468e 100644 --- a/apps/encryption_dummy/lib/dummymodule.php +++ b/apps/encryption_dummy/lib/dummymodule.php @@ -76,8 +76,8 @@ class DummyModule implements IEncryptionModule { public function end($path) { if ($this->isWriteOperation) { - $storage = \OC::$server->getEncryptionKeyStorage($this->getId()); - $storage->setFileKey($path, 'fileKey', 'foo'); + $storage = \OC::$server->getEncryptionKeyStorage(); + $storage->setFileKey($path, 'fileKey', 'foo', $this->getId()); } return ''; } diff --git a/lib/private/encryption/keys/factory.php b/lib/private/encryption/keys/factory.php deleted file mode 100644 index 0e2b0292a6..0000000000 --- a/lib/private/encryption/keys/factory.php +++ /dev/null @@ -1,50 +0,0 @@ - - * - * @copyright Copyright (c) 2015, ownCloud, Inc. - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * 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, version 3, - * along with this program. If not, see - * - */ - -namespace OC\Encryption\Keys; - -use OC\Encryption\Util; -use OC\Files\View; -use OC\User; - -/** - * Factory provides KeyStorage for different encryption modules - */ -class Factory { - /** @var array */ - protected $instances = array(); - - /** - * get a KeyStorage instance - * - * @param string $encryptionModuleId - * @param View $view - * @param Util $util - * @return Storage - */ - public function get($encryptionModuleId,View $view, Util $util) { - if (!isset($this->instances[$encryptionModuleId])) { - $this->instances[$encryptionModuleId] = new Storage($encryptionModuleId, $view, $util); - } - return $this->instances[$encryptionModuleId]; - } - -} diff --git a/lib/private/encryption/keys/storage.php b/lib/private/encryption/keys/storage.php index 925c20c74c..cd4aa7e56c 100644 --- a/lib/private/encryption/keys/storage.php +++ b/lib/private/encryption/keys/storage.php @@ -23,10 +23,12 @@ namespace OC\Encryption\Keys; use OC\Encryption\Util; +use OC\Files\Filesystem; use OC\Files\View; use OCP\Encryption\Exceptions\GenericEncryptionException; +use OCP\Encryption\Keys\IStorage; -class Storage implements \OCP\Encryption\Keys\IStorage { +class Storage implements IStorage { /** @var View */ private $view; @@ -40,152 +42,100 @@ class Storage implements \OCP\Encryption\Keys\IStorage { private $keyCache = array(); - /** @var string */ - private $encryptionModuleId; - /** * @param string $encryptionModuleId * @param View $view * @param Util $util */ - public function __construct($encryptionModuleId, View $view, Util $util) { + public function __construct(View $view, Util $util) { $this->view = $view; $this->util = $util; - $this->encryptionModuleId = $encryptionModuleId; $this->encryption_base_dir = '/files_encryption'; $this->keys_base_dir = $this->encryption_base_dir .'/keys'; } /** - * get user specific key - * - * @param string $uid ID if the user for whom we want the key - * @param string $keyId id of the key - * - * @return mixed key + * @inheritdoc */ - public function getUserKey($uid, $keyId) { - $path = $this->constructUserKeyPath($keyId, $uid); + public function getUserKey($uid, $keyId, $encryptionModuleId) { + $path = $this->constructUserKeyPath($encryptionModuleId, $keyId, $uid); return $this->getKey($path); } /** - * get file specific key - * - * @param string $path path to file - * @param string $keyId id of the key - * - * @return mixed key + * @inheritdoc */ - public function getFileKey($path, $keyId) { - $keyDir = $this->getFileKeyDir($path); + public function getFileKey($path, $keyId, $encryptionModuleId) { + $keyDir = $this->getFileKeyDir($encryptionModuleId, $path); return $this->getKey($keyDir . $keyId); } /** - * get system-wide encryption keys not related to a specific user, - * e.g something like a key for public link shares - * - * @param string $keyId id of the key - * - * @return mixed key + * @inheritdoc */ - public function getSystemUserKey($keyId) { - $path = $this->constructUserKeyPath($keyId); + public function getSystemUserKey($keyId, $encryptionModuleId) { + $path = $this->constructUserKeyPath($encryptionModuleId, $keyId, null); return $this->getKey($path); } /** - * set user specific key - * - * @param string $uid ID if the user for whom we want the key - * @param string $keyId id of the key - * @param mixed $key + * @inheritdoc */ - public function setUserKey($uid, $keyId, $key) { - $path = $this->constructUserKeyPath($keyId, $uid); + public function setUserKey($uid, $keyId, $key, $encryptionModuleId) { + $path = $this->constructUserKeyPath($encryptionModuleId, $keyId, $uid); return $this->setKey($path, $key); } /** - * set file specific key - * - * @param string $path path to file - * @param string $keyId id of the key - * @param boolean + * @inheritdoc */ - public function setFileKey($path, $keyId, $key) { - $keyDir = $this->getFileKeyDir($path); + public function setFileKey($path, $keyId, $key, $encryptionModuleId) { + $keyDir = $this->getFileKeyDir($encryptionModuleId, $path); return $this->setKey($keyDir . $keyId, $key); } /** - * set system-wide encryption keys not related to a specific user, - * e.g something like a key for public link shares - * - * @param string $keyId id of the key - * @param mixed $key - * - * @return mixed key + * @inheritdoc */ - public function setSystemUserKey($keyId, $key) { - $path = $this->constructUserKeyPath($keyId); + public function setSystemUserKey($keyId, $key, $encryptionModuleId) { + $path = $this->constructUserKeyPath($encryptionModuleId, $keyId, null); return $this->setKey($path, $key); } /** - * delete user specific key - * - * @param string $uid ID if the user for whom we want to delete the key - * @param string $keyId id of the key - * - * @return boolean False when the key could not be deleted + * @inheritdoc */ - public function deleteUserKey($uid, $keyId) { - $path = $this->constructUserKeyPath($keyId, $uid); + public function deleteUserKey($uid, $keyId, $encryptionModuleId) { + $path = $this->constructUserKeyPath($encryptionModuleId, $keyId, $uid); return !$this->view->file_exists($path) || $this->view->unlink($path); } /** - * delete file specific key - * - * @param string $path path to file - * @param string $keyId id of the key - * - * @return boolean False when the key could not be deleted + * @inheritdoc */ - public function deleteFileKey($path, $keyId) { - $keyDir = $this->getFileKeyDir($path); + public function deleteFileKey($path, $keyId, $encryptionModuleId) { + $keyDir = $this->getFileKeyDir($encryptionModuleId, $path); return !$this->view->file_exists($keyDir . $keyId) || $this->view->unlink($keyDir . $keyId); } /** - * delete all file keys for a given file - * - * @param string $path to the file - * @return boolean False when the key could not be deleted + * @inheritdoc */ - public function deleteAllFileKeys($path) { - $keyDir = $this->getFileKeyDir($path); + public function deleteAllFileKeys($path, $encryptionModuleId) { + $keyDir = $this->getFileKeyDir($encryptionModuleId, $path); $path = dirname($keyDir); return !$this->view->file_exists($path) || $this->view->deleteAll($path); } /** - * delete system-wide encryption keys not related to a specific user, - * e.g something like a key for public link shares - * - * @param string $keyId id of the key - * - * @return boolean False when the key could not be deleted + * @inheritdoc */ - public function deleteSystemUserKey($keyId) { - $path = $this->constructUserKeyPath($keyId); + public function deleteSystemUserKey($keyId, $encryptionModuleId) { + $path = $this->constructUserKeyPath($encryptionModuleId, $keyId, null); return !$this->view->file_exists($path) || $this->view->unlink($path); } - /** * construct path to users key * @@ -193,13 +143,13 @@ class Storage implements \OCP\Encryption\Keys\IStorage { * @param string $uid * @return string */ - protected function constructUserKeyPath($keyId, $uid = null) { + protected function constructUserKeyPath($encryptionModuleId, $keyId, $uid) { if ($uid === null) { - $path = $this->encryption_base_dir . '/' . $this->encryptionModuleId . '/' . $keyId; + $path = $this->encryption_base_dir . '/' . $encryptionModuleId . '/' . $keyId; } else { $path = '/' . $uid . $this->encryption_base_dir . '/' - . $this->encryptionModuleId . '/' . $uid . '.' . $keyId; + . $encryptionModuleId . '/' . $uid . '.' . $keyId; } return $path; @@ -256,7 +206,7 @@ class Storage implements \OCP\Encryption\Keys\IStorage { * @throws GenericEncryptionException * @internal param string $keyId */ - private function getFileKeyDir($path) { + private function getFileKeyDir($encryptionModuleId, $path) { if ($this->view->is_dir($path)) { throw new GenericEncryptionException("file was expected but directory was given: $path"); @@ -272,7 +222,7 @@ class Storage implements \OCP\Encryption\Keys\IStorage { $keyPath = '/' . $owner . $this->keys_base_dir . $filename . '/'; } - return \OC\Files\Filesystem::normalizePath($keyPath . $this->encryptionModuleId . '/', false); + return Filesystem::normalizePath($keyPath . $encryptionModuleId . '/', false); } /** diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index e5c96286f0..14c9df9c6f 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -187,8 +187,9 @@ class Encryption extends Wrapper { $encryptionModule = $this->getEncryptionModule($path); if ($encryptionModule) { - $keyStorage = $this->getKeyStorage($encryptionModule->getId()); - $keyStorage->deleteAllFileKeys($this->getFullPath($path)); + $keyStorage = $this->getKeyStorage(); + $keyStorage->deleteAllFileKeys($this->getFullPath($path), + $encryptionModule->getId()); } return $this->storage->unlink($path); @@ -436,8 +437,8 @@ class Encryption extends Wrapper { * @param string $encryptionModuleId * @return \OCP\Encryption\Keys\IStorage */ - protected function getKeyStorage($encryptionModuleId) { - $keyStorage = \OC::$server->getEncryptionKeyStorage($encryptionModuleId); + protected function getKeyStorage() { + $keyStorage = \OC::$server->getEncryptionKeyStorage(); return $keyStorage; } diff --git a/lib/private/server.php b/lib/private/server.php index d321ecb68b..8fdeec5281 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -97,8 +97,16 @@ class Server extends SimpleContainer implements IServerContainer { return new Encryption\File($util); }); - $this->registerService('EncryptionKeyStorageFactory', function ($c) { - return new Encryption\Keys\Factory(); + $this->registerService('EncryptionKeyStorage', function (Server $c) { + $view = new \OC\Files\View(); + $util = new \OC\Encryption\Util( + $view, + $c->getUserManager(), + $c->getGroupManager(), + $c->getConfig() + ); + + return new Encryption\Keys\Storage($view, $util); }); $this->registerService('TagMapper', function(Server $c) { return new TagMapper($c->getDatabaseConnection()); @@ -436,19 +444,10 @@ class Server extends SimpleContainer implements IServerContainer { } /** - * @param string $encryptionModuleId encryption module ID - * * @return \OCP\Encryption\Keys\IStorage */ - public function getEncryptionKeyStorage($encryptionModuleId) { - $view = new \OC\Files\View(); - $util = new \OC\Encryption\Util( - $view, - \OC::$server->getUserManager(), - \OC::$server->getGroupManager(), - \OC::$server->getConfig() - ); - return $this->query('EncryptionKeyStorageFactory')->get($encryptionModuleId, $view, $util); + public function getEncryptionKeyStorage() { + return $this->query('EncryptionKeyStorage'); } /** diff --git a/lib/public/encryption/keys/istorage.php b/lib/public/encryption/keys/istorage.php index 3e497ed2c7..696d537331 100644 --- a/lib/public/encryption/keys/istorage.php +++ b/lib/public/encryption/keys/istorage.php @@ -35,33 +35,36 @@ interface IStorage { * * @param string $uid ID if the user for whom we want the key * @param string $keyId id of the key + * @param string $encryptionModuleId * * @return mixed key * @since 8.1.0 */ - public function getUserKey($uid, $keyId); + public function getUserKey($uid, $keyId, $encryptionModuleId); /** * get file specific key * * @param string $path path to file * @param string $keyId id of the key + * @param string $encryptionModuleId * * @return mixed key * @since 8.1.0 */ - public function getFileKey($path, $keyId); + public function getFileKey($path, $keyId, $encryptionModuleId); /** * get system-wide encryption keys not related to a specific user, * e.g something like a key for public link shares * * @param string $keyId id of the key + * @param string $encryptionModuleId * * @return mixed key * @since 8.1.0 */ - public function getSystemUserKey($keyId); + public function getSystemUserKey($keyId, $encryptionModuleId); /** * set user specific key @@ -69,19 +72,21 @@ interface IStorage { * @param string $uid ID if the user for whom we want the key * @param string $keyId id of the key * @param mixed $key + * @param string $encryptionModuleId * @since 8.1.0 */ - public function setUserKey($uid, $keyId, $key); + public function setUserKey($uid, $keyId, $key, $encryptionModuleId); /** * set file specific key * * @param string $path path to file * @param string $keyId id of the key - * @param boolean + * @param mixed $key + * @param string $encryptionModuleId * @since 8.1.0 */ - public function setFileKey($path, $keyId, $key); + public function setFileKey($path, $keyId, $key, $encryptionModuleId); /** * set system-wide encryption keys not related to a specific user, @@ -89,53 +94,59 @@ interface IStorage { * * @param string $keyId id of the key * @param mixed $key + * @param string $encryptionModuleId * * @return mixed key * @since 8.1.0 */ - public function setSystemUserKey($keyId, $key); + public function setSystemUserKey($keyId, $key, $encryptionModuleId); /** * delete user specific key * * @param string $uid ID if the user for whom we want to delete the key * @param string $keyId id of the key + * @param string $encryptionModuleId * * @return boolean False when the key could not be deleted * @since 8.1.0 */ - public function deleteUserKey($uid, $keyId); + public function deleteUserKey($uid, $keyId, $encryptionModuleId); /** * delete file specific key * * @param string $path path to file * @param string $keyId id of the key + * @param string $encryptionModuleId * * @return boolean False when the key could not be deleted * @since 8.1.0 */ - public function deleteFileKey($path, $keyId); + public function deleteFileKey($path, $keyId, $encryptionModuleId); /** * delete all file keys for a given file * * @param string $path to the file + * @param string $encryptionModuleId + * * @return boolean False when the keys could not be deleted * @since 8.1.0 */ - public function deleteAllFileKeys($path); + public function deleteAllFileKeys($path, $encryptionModuleId); /** * delete system-wide encryption keys not related to a specific user, * e.g something like a key for public link shares * * @param string $keyId id of the key + * @param string $encryptionModuleId * * @return boolean False when the key could not be deleted * @since 8.1.0 */ - public function deleteSystemUserKey($keyId); + public function deleteSystemUserKey($keyId, $encryptionModuleId); /** * copy keys if a file was renamed diff --git a/lib/public/iservercontainer.php b/lib/public/iservercontainer.php index 9af1582dae..428c91429e 100644 --- a/lib/public/iservercontainer.php +++ b/lib/public/iservercontainer.php @@ -211,12 +211,10 @@ interface IServerContainer { public function getEncryptionFilesHelper(); /** - * @param string $encryptionModuleId encryption module ID - * * @return \OCP\Encryption\Keys\IStorage * @since 8.1.0 */ - public function getEncryptionKeyStorage($encryptionModuleId); + public function getEncryptionKeyStorage(); /** * Returns the URL generator diff --git a/settings/changepassword/controller.php b/settings/changepassword/controller.php index f041cb5b29..4a68636d3f 100644 --- a/settings/changepassword/controller.php +++ b/settings/changepassword/controller.php @@ -83,7 +83,7 @@ class Controller { \OC::$server->getLogger(), \OC::$server->getUserSession(), \OC::$server->getConfig()); - $keyStorage = \OC::$server->getEncryptionKeyStorage(\OCA\Encryption\Crypto\Encryption::ID); + $keyStorage = \OC::$server->getEncryptionKeyStorage(); $util = new \OCA\Encryption\Util( new \OC\Files\View(), $crypt, diff --git a/tests/lib/encryption/keys/storage.php b/tests/lib/encryption/keys/storage.php index bcf1c0f762..e67103fb6a 100644 --- a/tests/lib/encryption/keys/storage.php +++ b/tests/lib/encryption/keys/storage.php @@ -48,8 +48,7 @@ class StorageTest extends TestCase { ->disableOriginalConstructor() ->getMock(); - $this->storage = new Storage('encModule', $this->view, $this->util); - + $this->storage = new Storage($this->view, $this->util); } public function testSetFileKey() { @@ -69,7 +68,7 @@ class StorageTest extends TestCase { ->willReturn(strlen('key')); $this->assertTrue( - $this->storage->setFileKey('user1/files/foo.txt', 'fileKey', 'key') + $this->storage->setFileKey('user1/files/foo.txt', 'fileKey', 'key', 'encModule') ); } @@ -93,7 +92,7 @@ class StorageTest extends TestCase { ->willReturn(true); $this->assertSame('key', - $this->storage->getFileKey('user1/files/foo.txt', 'fileKey') + $this->storage->getFileKey('user1/files/foo.txt', 'fileKey', 'encModule') ); } @@ -114,7 +113,7 @@ class StorageTest extends TestCase { ->willReturn(strlen('key')); $this->assertTrue( - $this->storage->setFileKey('user1/files/foo.txt', 'fileKey', 'key') + $this->storage->setFileKey('user1/files/foo.txt', 'fileKey', 'key', 'encModule') ); } @@ -138,7 +137,7 @@ class StorageTest extends TestCase { ->willReturn(true); $this->assertSame('key', - $this->storage->getFileKey('user1/files/foo.txt', 'fileKey') + $this->storage->getFileKey('user1/files/foo.txt', 'fileKey', 'encModule') ); } @@ -150,7 +149,7 @@ class StorageTest extends TestCase { ->willReturn(strlen('key')); $this->assertTrue( - $this->storage->setSystemUserKey('shareKey_56884', 'key') + $this->storage->setSystemUserKey('shareKey_56884', 'key', 'encModule') ); } @@ -162,7 +161,7 @@ class StorageTest extends TestCase { ->willReturn(strlen('key')); $this->assertTrue( - $this->storage->setUserKey('user1', 'publicKey', 'key') + $this->storage->setUserKey('user1', 'publicKey', 'key', 'encModule') ); } @@ -177,7 +176,7 @@ class StorageTest extends TestCase { ->willReturn(true); $this->assertSame('key', - $this->storage->getSystemUserKey('shareKey_56884') + $this->storage->getSystemUserKey('shareKey_56884', 'encModule') ); } @@ -192,7 +191,7 @@ class StorageTest extends TestCase { ->willReturn(true); $this->assertSame('key', - $this->storage->getUserKey('user1', 'publicKey') + $this->storage->getUserKey('user1', 'publicKey', 'encModule') ); } @@ -207,7 +206,7 @@ class StorageTest extends TestCase { ->willReturn(true); $this->assertTrue( - $this->storage->deleteUserKey('user1', 'publicKey') + $this->storage->deleteUserKey('user1', 'publicKey', 'encModule') ); } @@ -222,7 +221,7 @@ class StorageTest extends TestCase { ->willReturn(true); $this->assertTrue( - $this->storage->deleteSystemUserKey('shareKey_56884') + $this->storage->deleteSystemUserKey('shareKey_56884', 'encModule') ); } @@ -246,7 +245,7 @@ class StorageTest extends TestCase { ->willReturn(true); $this->assertTrue( - $this->storage->deleteFileKey('user1/files/foo.txt', 'fileKey') + $this->storage->deleteFileKey('user1/files/foo.txt', 'fileKey', 'encModule') ); } @@ -270,7 +269,7 @@ class StorageTest extends TestCase { ->willReturn(true); $this->assertTrue( - $this->storage->deleteFileKey('user1/files/foo.txt', 'fileKey') + $this->storage->deleteFileKey('user1/files/foo.txt', 'fileKey', 'encModule') ); } diff --git a/tests/lib/files/storage/wrapper/encryption.php b/tests/lib/files/storage/wrapper/encryption.php index 3256f772df..1082cafbd3 100644 --- a/tests/lib/files/storage/wrapper/encryption.php +++ b/tests/lib/files/storage/wrapper/encryption.php @@ -112,7 +112,7 @@ class EncryptionWrapper extends \OC\Files\Storage\Wrapper\Encryption { parent::__construct($parameters, $encryptionManager, $util, $logger, $fileHelper, $uid); } - protected function getKeyStorage($encryptionModuleId) { + protected function getKeyStorage() { return $this->keyStore; } From 987bc138df89be2d0afdf268ab5c991b1bbe830b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 22 Apr 2015 12:12:27 +0200 Subject: [PATCH 2/9] calling renameKeys() on directory level as well - fixes #15778 --- .../files/storage/wrapper/encryption.php | 10 +++------- tests/lib/files/storage/wrapper/encryption.php | 17 +++++++++++++++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index 14c9df9c6f..04eb5fceed 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -215,11 +215,8 @@ class Encryption extends Wrapper { if (isset($this->unencryptedSize[$source])) { $this->unencryptedSize[$target] = $this->unencryptedSize[$source]; } - $encryptionModule = $this->getEncryptionModule($path2); - if ($encryptionModule) { - $keyStorage = $this->getKeyStorage($encryptionModule->getId()); - $keyStorage->renameKeys($source, $target); - } + $keyStorage = $this->getKeyStorage(); + $keyStorage->renameKeys($source, $target); } return $result; @@ -438,8 +435,7 @@ class Encryption extends Wrapper { * @return \OCP\Encryption\Keys\IStorage */ protected function getKeyStorage() { - $keyStorage = \OC::$server->getEncryptionKeyStorage(); - return $keyStorage; + return \OC::$server->getEncryptionKeyStorage(); } } diff --git a/tests/lib/files/storage/wrapper/encryption.php b/tests/lib/files/storage/wrapper/encryption.php index 1082cafbd3..228c7b08d6 100644 --- a/tests/lib/files/storage/wrapper/encryption.php +++ b/tests/lib/files/storage/wrapper/encryption.php @@ -12,6 +12,11 @@ class Encryption extends \Test\Files\Storage\Storage { */ private $sourceStorage; + /** + * @var \OC\Encryption\Keys\Storage | \PHPUnit_Framework_MockObject_MockObject + */ + private $keyStore; + public function setUp() { parent::setUp(); @@ -54,7 +59,7 @@ class Encryption extends \Test\Files\Storage\Storage { $logger = $this->getMock('\OC\Log'); $this->sourceStorage = new Temporary(array()); - $keyStore = $this->getMockBuilder('\OC\Encryption\Keys\Storage') + $this->keyStore = $this->getMockBuilder('\OC\Encryption\Keys\Storage') ->disableOriginalConstructor()->getMock(); $mount = $this->getMockBuilder('\OC\Files\Mount\MountPoint') ->disableOriginalConstructor() @@ -67,7 +72,7 @@ class Encryption extends \Test\Files\Storage\Storage { 'mountPoint' => '/', 'mount' => $mount ], - $encryptionManager, $util, $logger, $file, null, $keyStore + $encryptionManager, $util, $logger, $file, null, $this->keyStore ); } @@ -91,6 +96,14 @@ class Encryption extends \Test\Files\Storage\Storage { $encryptionModule->expects($this->any())->method('getUnencryptedBlockSize')->willReturn(8192); return $encryptionModule; } + + public function testRename() { + $this->keyStore + ->expects($this->once()) + ->method('renameKeys'); + $this->instance->mkdir('folder'); + $this->instance->rename('folder', 'flodder'); + } } // From 225cde21830459db044d6564c7a9cf62b7dd5944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 22 Apr 2015 13:09:42 +0200 Subject: [PATCH 3/9] pass KeyStorage via ctor --- lib/private/encryption/manager.php | 3 ++- .../files/storage/wrapper/encryption.php | 25 +++++++---------- .../lib/files/storage/wrapper/encryption.php | 27 +------------------ 3 files changed, 12 insertions(+), 43 deletions(-) diff --git a/lib/private/encryption/manager.php b/lib/private/encryption/manager.php index 7a3f17519f..97203b7756 100644 --- a/lib/private/encryption/manager.php +++ b/lib/private/encryption/manager.php @@ -221,7 +221,8 @@ class Manager implements IManager { $logger = \OC::$server->getLogger(); $uid = $user ? $user->getUID() : null; $fileHelper = \OC::$server->getEncryptionFilesHelper(); - return new Encryption($parameters, $manager, $util, $logger, $fileHelper, $uid); + $keyStorage = \OC::$server->getEncryptionKeyStorage(); + return new Encryption($parameters, $manager, $util, $logger, $fileHelper, $uid, $keyStorage); } else { return $storage; } diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index 04eb5fceed..0a9e6d61d2 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -56,6 +56,9 @@ class Encryption extends Wrapper { /** @var IMountPoint */ private $mount; + /** @var \OCP\Encryption\Keys\IStorage */ + private $keyStorage; + /** * @param array $parameters * @param \OC\Encryption\Manager $encryptionManager @@ -70,7 +73,8 @@ class Encryption extends Wrapper { \OC\Encryption\Util $util = null, \OC\Log $logger = null, File $fileHelper = null, - $uid = null + $uid = null, + $keyStorage = null ) { $this->mountPoint = $parameters['mountPoint']; @@ -80,6 +84,7 @@ class Encryption extends Wrapper { $this->logger = $logger; $this->uid = $uid; $this->fileHelper = $fileHelper; + $this->keyStorage = $keyStorage; $this->unencryptedSize = array(); parent::__construct($parameters); } @@ -187,8 +192,7 @@ class Encryption extends Wrapper { $encryptionModule = $this->getEncryptionModule($path); if ($encryptionModule) { - $keyStorage = $this->getKeyStorage(); - $keyStorage->deleteAllFileKeys($this->getFullPath($path), + $this->keyStorage->deleteAllFileKeys($this->getFullPath($path), $encryptionModule->getId()); } @@ -215,8 +219,7 @@ class Encryption extends Wrapper { if (isset($this->unencryptedSize[$source])) { $this->unencryptedSize[$target] = $this->unencryptedSize[$source]; } - $keyStorage = $this->getKeyStorage(); - $keyStorage->renameKeys($source, $target); + $this->keyStorage->renameKeys($source, $target); } return $result; @@ -241,8 +244,7 @@ class Encryption extends Wrapper { $target = $this->getFullPath($path2); $encryptionModule = $this->getEncryptionModule($path2); if ($encryptionModule) { - $keyStorage = $this->getKeyStorage($encryptionModule->getId()); - $keyStorage->copyKeys($source, $target); + $this->keyStorage->copyKeys($source, $target); } } @@ -429,13 +431,4 @@ class Encryption extends Wrapper { public function updateUnencryptedSize($path, $unencryptedSize) { $this->unencryptedSize[$path] = $unencryptedSize; } - - /** - * @param string $encryptionModuleId - * @return \OCP\Encryption\Keys\IStorage - */ - protected function getKeyStorage() { - return \OC::$server->getEncryptionKeyStorage(); - } - } diff --git a/tests/lib/files/storage/wrapper/encryption.php b/tests/lib/files/storage/wrapper/encryption.php index 228c7b08d6..1d77655550 100644 --- a/tests/lib/files/storage/wrapper/encryption.php +++ b/tests/lib/files/storage/wrapper/encryption.php @@ -66,7 +66,7 @@ class Encryption extends \Test\Files\Storage\Storage { ->setMethods(['getOption']) ->getMock(); $mount->expects($this->any())->method('getOption')->willReturn(true); - $this->instance = new EncryptionWrapper([ + $this->instance = new \OC\Files\Storage\Wrapper\Encryption([ 'storage' => $this->sourceStorage, 'root' => 'foo', 'mountPoint' => '/', @@ -105,28 +105,3 @@ class Encryption extends \Test\Files\Storage\Storage { $this->instance->rename('folder', 'flodder'); } } - -// -// FIXME: this is too bad and needs adjustment -// -class EncryptionWrapper extends \OC\Files\Storage\Wrapper\Encryption { - private $keyStore; - - public function __construct( - $parameters, - \OC\Encryption\Manager $encryptionManager = null, - \OC\Encryption\Util $util = null, - \OC\Log $logger = null, - \OC\Encryption\File $fileHelper = null, - $uid = null, - $keyStore = null - ) { - $this->keyStore = $keyStore; - parent::__construct($parameters, $encryptionManager, $util, $logger, $fileHelper, $uid); - } - - protected function getKeyStorage() { - return $this->keyStore; - } - -} From 0042bdd2e758f8b514acc86c3c72c3b1e5f5911b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 22 Apr 2015 13:12:52 +0200 Subject: [PATCH 4/9] fix PHPDoc --- lib/private/encryption/keys/storage.php | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/private/encryption/keys/storage.php b/lib/private/encryption/keys/storage.php index cd4aa7e56c..e34d7370ef 100644 --- a/lib/private/encryption/keys/storage.php +++ b/lib/private/encryption/keys/storage.php @@ -37,13 +37,16 @@ class Storage implements IStorage { private $util; // base dir where all the file related keys are stored + /** @var string */ private $keys_base_dir; + + /** @var string */ private $encryption_base_dir; - private $keyCache = array(); + /** @var array */ + private $keyCache = []; /** - * @param string $encryptionModuleId * @param View $view * @param Util $util */ @@ -139,6 +142,7 @@ class Storage implements IStorage { /** * construct path to users key * + * @param string $encryptionModuleId * @param string $keyId * @param string $uid * @return string @@ -201,6 +205,7 @@ class Storage implements IStorage { /** * get path to key folder for a given file * + * @param string $encryptionModuleId * @param string $path path to the file, relative to data/ * @return string * @throws GenericEncryptionException @@ -230,8 +235,6 @@ class Storage implements IStorage { * * @param string $source * @param string $target - * @param string $owner - * @param bool $systemWide */ public function renameKeys($source, $target) { @@ -258,8 +261,6 @@ class Storage implements IStorage { * * @param string $source * @param string $target - * @param string $owner - * @param bool $systemWide */ public function copyKeys($source, $target) { @@ -282,7 +283,7 @@ class Storage implements IStorage { } /** - * Make preparations to filesystem for saving a keyfile + * Make preparations to filesystem for saving a key file * * @param string $path relative to the views root */ From c81bc152d7bd649b24b00482e0d57b6ce24643c4 Mon Sep 17 00:00:00 2001 From: Clark Tomlinson Date: Mon, 20 Apr 2015 10:23:09 -0400 Subject: [PATCH 5/9] fixing return values and adding tests --- .../controller/recoverycontroller.php | 70 ++++--- .../controller/RecoveryControllerTest.php | 182 ++++++++++++++++++ 2 files changed, 220 insertions(+), 32 deletions(-) create mode 100644 apps/encryption/tests/controller/RecoveryControllerTest.php diff --git a/apps/encryption/controller/recoverycontroller.php b/apps/encryption/controller/recoverycontroller.php index 9c07bda62e..550190e952 100644 --- a/apps/encryption/controller/recoverycontroller.php +++ b/apps/encryption/controller/recoverycontroller.php @@ -72,31 +72,36 @@ class RecoveryController extends Controller { public function adminRecovery($recoveryPassword, $confirmPassword, $adminEnableRecovery) { // Check if both passwords are the same if (empty($recoveryPassword)) { - $errorMessage = (string) $this->l->t('Missing recovery key password'); - return new DataResponse(['data' => ['message' => $errorMessage]], 500); + $errorMessage = (string)$this->l->t('Missing recovery key password'); + return new DataResponse(['data' => ['message' => $errorMessage]], + 500); } if (empty($confirmPassword)) { - $errorMessage = (string) $this->l->t('Please repeat the recovery key password'); - return new DataResponse(['data' => ['message' => $errorMessage]], 500); + $errorMessage = (string)$this->l->t('Please repeat the recovery key password'); + return new DataResponse(['data' => ['message' => $errorMessage]], + 500); } if ($recoveryPassword !== $confirmPassword) { - $errorMessage = (string) $this->l->t('Repeated recovery key password does not match the provided recovery key password'); - return new DataResponse(['data' => ['message' => $errorMessage]], 500); + $errorMessage = (string)$this->l->t('Repeated recovery key password does not match the provided recovery key password'); + return new DataResponse(['data' => ['message' => $errorMessage]], + 500); } if (isset($adminEnableRecovery) && $adminEnableRecovery === '1') { if ($this->recovery->enableAdminRecovery($recoveryPassword)) { - return new DataResponse(['status' =>'success', 'data' => array('message' => (string) $this->l->t('Recovery key successfully enabled'))]); + return new DataResponse(['status' => 'success', 'data' => array('message' => (string)$this->l->t('Recovery key successfully enabled'))]); } - return new DataResponse(['data' => array('message' => (string) $this->l->t('Could not enable recovery key. Please check your recovery key password!'))]); + return new DataResponse(['data' => array('message' => (string)$this->l->t('Could not enable recovery key. Please check your recovery key password!'))]); } elseif (isset($adminEnableRecovery) && $adminEnableRecovery === '0') { if ($this->recovery->disableAdminRecovery($recoveryPassword)) { - return new DataResponse(['data' => array('message' => (string) $this->l->t('Recovery key successfully disabled'))]); + return new DataResponse(['data' => array('message' => (string)$this->l->t('Recovery key successfully disabled'))]); } - return new DataResponse(['data' => array('message' => (string) $this->l->t('Could not disable recovery key. Please check your recovery key password!'))]); + return new DataResponse(['data' => array('message' => (string)$this->l->t('Could not disable recovery key. Please check your recovery key password!'))]); } + // this response should never be sent but just in case. + return new DataResponse(['data' => ['message' => (string)$this->l->t('Missing parameters')]]); } /** @@ -108,42 +113,43 @@ class RecoveryController extends Controller { public function changeRecoveryPassword($newPassword, $oldPassword, $confirmPassword) { //check if both passwords are the same if (empty($oldPassword)) { - $errorMessage = (string) $this->l->t('Please provide the old recovery password'); + $errorMessage = (string)$this->l->t('Please provide the old recovery password'); return new DataResponse(array('data' => array('message' => $errorMessage))); } if (empty($newPassword)) { - $errorMessage = (string) $this->l->t('Please provide a new recovery password'); + $errorMessage = (string)$this->l->t('Please provide a new recovery password'); return new DataResponse (array('data' => array('message' => $errorMessage))); } if (empty($confirmPassword)) { - $errorMessage = (string) $this->l->t('Please repeat the new recovery password'); + $errorMessage = (string)$this->l->t('Please repeat the new recovery password'); return new DataResponse(array('data' => array('message' => $errorMessage))); } if ($newPassword !== $confirmPassword) { - $errorMessage = (string) $this->l->t('Repeated recovery key password does not match the provided recovery key password'); + $errorMessage = (string)$this->l->t('Repeated recovery key password does not match the provided recovery key password'); return new DataResponse(array('data' => array('message' => $errorMessage))); } - $result = $this->recovery->changeRecoveryKeyPassword($newPassword, $oldPassword); + $result = $this->recovery->changeRecoveryKeyPassword($newPassword, + $oldPassword); if ($result) { return new DataResponse( array( - 'status' => 'success' , + 'status' => 'success', 'data' => array( - 'message' => (string) $this->l->t('Password successfully changed.')) - ) - ); + 'message' => (string)$this->l->t('Password successfully changed.')) + ) + ); } else { return new DataResponse( array( 'data' => array - ('message' => (string) $this->l->t('Could not change the password. Maybe the old password was not correct.')) - ) - ); + ('message' => (string)$this->l->t('Could not change the password. Maybe the old password was not correct.')) + ) + ); } } @@ -161,19 +167,19 @@ class RecoveryController extends Controller { if ($result) { return new DataResponse( array( - 'status' => 'success', - 'data' => array( - 'message' => (string) $this->l->t('Recovery Key enabled')) - ) - ); - } else { - return new DataResponse( - array( - 'data' => array - ('message' => (string) $this->l->t('Could not enable the recovery key, please try again or contact your administrator')) + 'status' => 'success', + 'data' => array( + 'message' => (string)$this->l->t('Recovery Key enabled')) ) ); } + + return new DataResponse( + array( + 'data' => array + ('message' => (string)$this->l->t('Could not enable the recovery key, please try again or contact your administrator')) + ) + ); } } diff --git a/apps/encryption/tests/controller/RecoveryControllerTest.php b/apps/encryption/tests/controller/RecoveryControllerTest.php new file mode 100644 index 0000000000..289fe60e88 --- /dev/null +++ b/apps/encryption/tests/controller/RecoveryControllerTest.php @@ -0,0 +1,182 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + */ + + +namespace OC\apps\encryption\tests\lib\controller; + + +use OCA\Encryption\Controller\RecoveryController; +use Test\TestCase; + +class RecoveryControllerTest extends TestCase { + /** + * @var RecoveryController + */ + private $controller; + private $appName; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + private $requestMock; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + private $configMock; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + private $l10nMock; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + private $recoveryMock; + + public function testAdminRecovery() { + + $recoveryPassword = 'test'; + $enableRecovery = '1'; + + $this->recoveryMock->expects($this->any()) + ->method('enableAdminRecovery') + ->willReturn(true); + + $response = $this->controller->adminRecovery($recoveryPassword, + $recoveryPassword, + $enableRecovery)->getData(); + + + $this->assertEquals('Recovery key successfully enabled', + $response['data']['message']); + + $response = $this->controller->adminRecovery('', + $recoveryPassword, + $enableRecovery)->getData(); + + $this->assertEquals('Missing recovery key password', + $response['data']['message']); + + $response = $this->controller->adminRecovery($recoveryPassword, + '', + $enableRecovery)->getData(); + + $this->assertEquals('Please repeat the recovery key password', + $response['data']['message']); + + $response = $this->controller->adminRecovery($recoveryPassword, + 'something that doesn\'t match', + $enableRecovery)->getData(); + + $this->assertEquals('Repeated recovery key password does not match the provided recovery key password', + $response['data']['message']); + + $this->recoveryMock->expects($this->once()) + ->method('disableAdminRecovery') + ->willReturn(true); + + $response = $this->controller->adminRecovery($recoveryPassword, + $recoveryPassword, + '0')->getData(); + + $this->assertEquals('Recovery key successfully disabled', + $response['data']['message']); + } + + public function testChangeRecoveryPassword() { + $password = 'test'; + $oldPassword = 'oldtest'; + + $data = $this->controller->changeRecoveryPassword($password, + $oldPassword, + $password)->getData(); + + $this->assertEquals('Could not change the password. Maybe the old password was not correct.', + $data['data']['message']); + + $this->recoveryMock->expects($this->once()) + ->method('changeRecoveryKeyPassword') + ->with($password, $oldPassword) + ->willReturn(true); + + $data = $this->controller->changeRecoveryPassword($password, + $oldPassword, + $password)->getData(); + + $this->assertEquals('Password successfully changed.', + $data['data']['message']); + + $data = $this->controller->changeRecoveryPassword($password, + $oldPassword, + 'not match')->getData(); + + $this->assertEquals('Repeated recovery key password does not match the provided recovery key password', + $data['data']['message']); + + $data = $this->controller->changeRecoveryPassword('', + $oldPassword, + $password)->getData(); + + $this->assertEquals('Please provide a new recovery password', + $data['data']['message']); + + $data = $this->controller->changeRecoveryPassword($password, + '', + $password)->getData(); + + $this->assertEquals('Please provide the old recovery password', + $data['data']['message']); + } + + public function testUserSetRecovery() { + $this->recoveryMock->expects($this->exactly(2)) + ->method('setRecoveryForUser') + ->willReturnOnConsecutiveCalls(true, false); + + $data = $this->controller->userSetRecovery('1')->getData(); + + $this->assertEquals('Recovery Key enabled', $data['data']['message']); + + $data = $this->controller->userSetRecovery('1')->getData(); + + $this->assertEquals('Could not enable the recovery key, please try again or contact your administrator', + $data['data']['message']); + + } + + protected function setUp() { + parent::setUp(); + + $this->appName = 'encryption'; + $this->requestMock = $this->getMockBuilder('\OCP\IRequest') + ->disableOriginalConstructor() + ->getMock(); + + $this->configMock = $this->getMockBuilder('OCP\IConfig') + ->disableOriginalConstructor() + ->getMock(); + + $this->l10nMock = $this->getMockBuilder('OCP\IL10N') + ->disableOriginalConstructor() + ->getMock(); + + // Make l10n work in our tests + $this->l10nMock->expects($this->any()) + ->method('t') + ->willReturnArgument(0); + + $this->recoveryMock = $this->getMockBuilder('OCA\Encryption\Recovery') + ->disableOriginalConstructor() + ->getMock(); + + $this->controller = new RecoveryController($this->appName, + $this->requestMock, + $this->configMock, + $this->l10nMock, + $this->recoveryMock); + } + +} From 1747117edfd337d50075e3612ca56cac18b96f5a Mon Sep 17 00:00:00 2001 From: Clark Tomlinson Date: Mon, 20 Apr 2015 13:49:21 -0400 Subject: [PATCH 6/9] destupify tests --- .../controller/recoverycontroller.php | 47 +++-- .../controller/RecoveryControllerTest.php | 161 +++++++++--------- 2 files changed, 102 insertions(+), 106 deletions(-) diff --git a/apps/encryption/controller/recoverycontroller.php b/apps/encryption/controller/recoverycontroller.php index 550190e952..bf548f24fc 100644 --- a/apps/encryption/controller/recoverycontroller.php +++ b/apps/encryption/controller/recoverycontroller.php @@ -93,15 +93,15 @@ class RecoveryController extends Controller { if ($this->recovery->enableAdminRecovery($recoveryPassword)) { return new DataResponse(['status' => 'success', 'data' => array('message' => (string)$this->l->t('Recovery key successfully enabled'))]); } - return new DataResponse(['data' => array('message' => (string)$this->l->t('Could not enable recovery key. Please check your recovery key password!'))]); + return new DataResponse(['data' => array('message' => (string)$this->l->t('Could not enable recovery key. Please check your recovery key password!'))], 500); } elseif (isset($adminEnableRecovery) && $adminEnableRecovery === '0') { if ($this->recovery->disableAdminRecovery($recoveryPassword)) { return new DataResponse(['data' => array('message' => (string)$this->l->t('Recovery key successfully disabled'))]); } - return new DataResponse(['data' => array('message' => (string)$this->l->t('Could not disable recovery key. Please check your recovery key password!'))]); + return new DataResponse(['data' => array('message' => (string)$this->l->t('Could not disable recovery key. Please check your recovery key password!'))], 500); } // this response should never be sent but just in case. - return new DataResponse(['data' => ['message' => (string)$this->l->t('Missing parameters')]]); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Missing parameters')]], 500); } /** @@ -114,22 +114,22 @@ class RecoveryController extends Controller { //check if both passwords are the same if (empty($oldPassword)) { $errorMessage = (string)$this->l->t('Please provide the old recovery password'); - return new DataResponse(array('data' => array('message' => $errorMessage))); + return new DataResponse(array('data' => array('message' => $errorMessage)), 500); } if (empty($newPassword)) { $errorMessage = (string)$this->l->t('Please provide a new recovery password'); - return new DataResponse (array('data' => array('message' => $errorMessage))); + return new DataResponse (array('data' => array('message' => $errorMessage)), 500); } if (empty($confirmPassword)) { $errorMessage = (string)$this->l->t('Please repeat the new recovery password'); - return new DataResponse(array('data' => array('message' => $errorMessage))); + return new DataResponse(array('data' => array('message' => $errorMessage)), 500); } if ($newPassword !== $confirmPassword) { $errorMessage = (string)$this->l->t('Repeated recovery key password does not match the provided recovery key password'); - return new DataResponse(array('data' => array('message' => $errorMessage))); + return new DataResponse(array('data' => array('message' => $errorMessage)), 500); } $result = $this->recovery->changeRecoveryKeyPassword($newPassword, @@ -139,18 +139,17 @@ class RecoveryController extends Controller { return new DataResponse( array( 'status' => 'success', - 'data' => array( - 'message' => (string)$this->l->t('Password successfully changed.')) - ) - ); - } else { - return new DataResponse( - array( - 'data' => array - ('message' => (string)$this->l->t('Could not change the password. Maybe the old password was not correct.')) + 'data' => [ + 'message' => (string)$this->l->t('Password successfully changed.')] ) ); } + return new DataResponse( + array( + 'data' => [ + 'message' => (string)$this->l->t('Could not change the password. Maybe the old password was not correct.') + ] + ), 500); } /** @@ -168,19 +167,19 @@ class RecoveryController extends Controller { return new DataResponse( array( 'status' => 'success', - 'data' => array( - 'message' => (string)$this->l->t('Recovery Key enabled')) + 'data' => [ + 'message' => (string)$this->l->t('Recovery Key enabled')] ) ); } - return new DataResponse( - array( - 'data' => array - ('message' => (string)$this->l->t('Could not enable the recovery key, please try again or contact your administrator')) - ) - ); } + return new DataResponse( + array( + 'data' => [ + 'message' => (string)$this->l->t('Could not enable the recovery key, please try again or contact your administrator') + ] + ), 500); } } diff --git a/apps/encryption/tests/controller/RecoveryControllerTest.php b/apps/encryption/tests/controller/RecoveryControllerTest.php index 289fe60e88..0ac76774c5 100644 --- a/apps/encryption/tests/controller/RecoveryControllerTest.php +++ b/apps/encryption/tests/controller/RecoveryControllerTest.php @@ -7,10 +7,11 @@ */ -namespace OC\apps\encryption\tests\lib\controller; +namespace OCA\Encryption\Tests\Controller; use OCA\Encryption\Controller\RecoveryController; +use OCP\AppFramework\Http; use Test\TestCase; class RecoveryControllerTest extends TestCase { @@ -36,114 +37,110 @@ class RecoveryControllerTest extends TestCase { */ private $recoveryMock; - public function testAdminRecovery() { + public function adminRecoveryProvider() { + return [ + ['test', 'test', '1', 'Recovery key successfully enabled', HTTP::STATUS_OK], + ['', 'test', '1', 'Missing recovery key password', HTTP::STATUS_INTERNAL_SERVER_ERROR], + ['test', '', '1', 'Please repeat the recovery key password', HTTP::STATUS_INTERNAL_SERVER_ERROR], + ['test', 'soimething that doesn\'t match', '1', 'Repeated recovery key password does not match the provided recovery key password', HTTP::STATUS_INTERNAL_SERVER_ERROR], + ['test', 'test', '0', 'Recovery key successfully disabled', HTTP::STATUS_OK], + ]; + } + + /** + * @dataProvider adminRecoveryProvider + * @param $recoveryPassword + * @param $passconfirm + * @param $enableRecovery + * @param $expectedMessage + * @param $expectedStatus + */ + public function testAdminRecovery($recoveryPassword, $passconfirm, $enableRecovery, $expectedMessage, $expectedStatus) { - $recoveryPassword = 'test'; - $enableRecovery = '1'; $this->recoveryMock->expects($this->any()) ->method('enableAdminRecovery') ->willReturn(true); - $response = $this->controller->adminRecovery($recoveryPassword, - $recoveryPassword, - $enableRecovery)->getData(); - - - $this->assertEquals('Recovery key successfully enabled', - $response['data']['message']); - - $response = $this->controller->adminRecovery('', - $recoveryPassword, - $enableRecovery)->getData(); - - $this->assertEquals('Missing recovery key password', - $response['data']['message']); - - $response = $this->controller->adminRecovery($recoveryPassword, - '', - $enableRecovery)->getData(); - - $this->assertEquals('Please repeat the recovery key password', - $response['data']['message']); - - $response = $this->controller->adminRecovery($recoveryPassword, - 'something that doesn\'t match', - $enableRecovery)->getData(); - - $this->assertEquals('Repeated recovery key password does not match the provided recovery key password', - $response['data']['message']); - - $this->recoveryMock->expects($this->once()) + $this->recoveryMock->expects($this->any()) ->method('disableAdminRecovery') ->willReturn(true); $response = $this->controller->adminRecovery($recoveryPassword, - $recoveryPassword, - '0')->getData(); + $passconfirm, + $enableRecovery); + + + $this->assertEquals($expectedMessage, $response->getData()['data']['message']); + $this->assertEquals($expectedStatus, $response->getStatus()); + - $this->assertEquals('Recovery key successfully disabled', - $response['data']['message']); } - public function testChangeRecoveryPassword() { - $password = 'test'; - $oldPassword = 'oldtest'; + public function changeRecoveryPasswordProvider() { + return [ + ['test', 'test', 'oldtestFail', 'Could not change the password. Maybe the old password was not correct.', HTTP::STATUS_INTERNAL_SERVER_ERROR], + ['test', 'test', 'oldtest', 'Password successfully changed.', HTTP::STATUS_OK], + ['test', 'notmatch', 'oldtest', 'Repeated recovery key password does not match the provided recovery key password', HTTP::STATUS_INTERNAL_SERVER_ERROR], + ['', 'test', 'oldtest', 'Please provide a new recovery password', HTTP::STATUS_INTERNAL_SERVER_ERROR], + ['test', 'test', '', 'Please provide the old recovery password', HTTP::STATUS_INTERNAL_SERVER_ERROR] + ]; + } - $data = $this->controller->changeRecoveryPassword($password, - $oldPassword, - $password)->getData(); - - $this->assertEquals('Could not change the password. Maybe the old password was not correct.', - $data['data']['message']); - - $this->recoveryMock->expects($this->once()) + /** + * @dataProvider changeRecoveryPasswordProvider + * @param $password + * @param $confirmPassword + * @param $oldPassword + * @param $expectedMessage + * @param $expectedStatus + */ + public function testChangeRecoveryPassword($password, $confirmPassword, $oldPassword, $expectedMessage, $expectedStatus) { + $this->recoveryMock->expects($this->any()) ->method('changeRecoveryKeyPassword') ->with($password, $oldPassword) - ->willReturn(true); + ->will($this->returnValueMap([ + ['test', 'oldTestFail', false], + ['test', 'oldtest', true] + ])); - $data = $this->controller->changeRecoveryPassword($password, + $response = $this->controller->changeRecoveryPassword($password, $oldPassword, - $password)->getData(); + $confirmPassword); - $this->assertEquals('Password successfully changed.', - $data['data']['message']); + $this->assertEquals($expectedMessage, $response->getData()['data']['message']); + $this->assertEquals($expectedStatus, $response->getStatus()); - $data = $this->controller->changeRecoveryPassword($password, - $oldPassword, - 'not match')->getData(); - $this->assertEquals('Repeated recovery key password does not match the provided recovery key password', - $data['data']['message']); - - $data = $this->controller->changeRecoveryPassword('', - $oldPassword, - $password)->getData(); - - $this->assertEquals('Please provide a new recovery password', - $data['data']['message']); - - $data = $this->controller->changeRecoveryPassword($password, - '', - $password)->getData(); - - $this->assertEquals('Please provide the old recovery password', - $data['data']['message']); } - public function testUserSetRecovery() { - $this->recoveryMock->expects($this->exactly(2)) + public function userSetRecoveryProvider() { + return [ + ['1', 'Recovery Key enabled', Http::STATUS_OK], + ['0', 'Could not enable the recovery key, please try again or contact your administrator', Http::STATUS_INTERNAL_SERVER_ERROR] + ]; + } + + /** + * @dataProvider userSetRecoveryProvider + * @param $enableRecovery + * @param $expectedMessage + * @param $expectedStatus + */ + public function testUserSetRecovery($enableRecovery, $expectedMessage, $expectedStatus) { + $this->recoveryMock->expects($this->any()) ->method('setRecoveryForUser') - ->willReturnOnConsecutiveCalls(true, false); + ->with($enableRecovery) + ->will($this->returnValueMap([ + ['1', true], + ['0', false] + ])); - $data = $this->controller->userSetRecovery('1')->getData(); - $this->assertEquals('Recovery Key enabled', $data['data']['message']); + $response = $this->controller->userSetRecovery($enableRecovery); - $data = $this->controller->userSetRecovery('1')->getData(); - - $this->assertEquals('Could not enable the recovery key, please try again or contact your administrator', - $data['data']['message']); + $this->assertEquals($expectedMessage, $response->getData()['data']['message']); + $this->assertEquals($expectedStatus, $response->getStatus()); } From e3ec1a8bb8b913bd176b76bd59f8c0c209aff5cb Mon Sep 17 00:00:00 2001 From: Clark Tomlinson Date: Wed, 22 Apr 2015 10:41:47 -0400 Subject: [PATCH 7/9] remove status's and adjust js --- .../controller/recoverycontroller.php | 42 +++++++----- apps/encryption/js/settings-admin.js | 67 ++++++++++--------- apps/encryption/js/settings-personal.js | 48 +++++++------ 3 files changed, 89 insertions(+), 68 deletions(-) diff --git a/apps/encryption/controller/recoverycontroller.php b/apps/encryption/controller/recoverycontroller.php index bf548f24fc..8ae37d97ec 100644 --- a/apps/encryption/controller/recoverycontroller.php +++ b/apps/encryption/controller/recoverycontroller.php @@ -91,14 +91,14 @@ class RecoveryController extends Controller { if (isset($adminEnableRecovery) && $adminEnableRecovery === '1') { if ($this->recovery->enableAdminRecovery($recoveryPassword)) { - return new DataResponse(['status' => 'success', 'data' => array('message' => (string)$this->l->t('Recovery key successfully enabled'))]); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Recovery key successfully enabled')]]); } - return new DataResponse(['data' => array('message' => (string)$this->l->t('Could not enable recovery key. Please check your recovery key password!'))], 500); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Could not enable recovery key. Please check your recovery key password!')]], 500); } elseif (isset($adminEnableRecovery) && $adminEnableRecovery === '0') { if ($this->recovery->disableAdminRecovery($recoveryPassword)) { - return new DataResponse(['data' => array('message' => (string)$this->l->t('Recovery key successfully disabled'))]); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Recovery key successfully disabled')]]); } - return new DataResponse(['data' => array('message' => (string)$this->l->t('Could not disable recovery key. Please check your recovery key password!'))], 500); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Could not disable recovery key. Please check your recovery key password!')]], 500); } // this response should never be sent but just in case. return new DataResponse(['data' => ['message' => (string)$this->l->t('Missing parameters')]], 500); @@ -114,22 +114,22 @@ class RecoveryController extends Controller { //check if both passwords are the same if (empty($oldPassword)) { $errorMessage = (string)$this->l->t('Please provide the old recovery password'); - return new DataResponse(array('data' => array('message' => $errorMessage)), 500); + return new DataResponse(['data' => ['message' => $errorMessage]], 500); } if (empty($newPassword)) { $errorMessage = (string)$this->l->t('Please provide a new recovery password'); - return new DataResponse (array('data' => array('message' => $errorMessage)), 500); + return new DataResponse (['data' => ['message' => $errorMessage]], 500); } if (empty($confirmPassword)) { $errorMessage = (string)$this->l->t('Please repeat the new recovery password'); - return new DataResponse(array('data' => array('message' => $errorMessage)), 500); + return new DataResponse(['data' => ['message' => $errorMessage]], 500); } if ($newPassword !== $confirmPassword) { $errorMessage = (string)$this->l->t('Repeated recovery key password does not match the provided recovery key password'); - return new DataResponse(array('data' => array('message' => $errorMessage)), 500); + return new DataResponse(['data' => ['message' => $errorMessage]], 500); } $result = $this->recovery->changeRecoveryKeyPassword($newPassword, @@ -137,19 +137,18 @@ class RecoveryController extends Controller { if ($result) { return new DataResponse( - array( - 'status' => 'success', + [ 'data' => [ 'message' => (string)$this->l->t('Password successfully changed.')] - ) + ] ); } return new DataResponse( - array( + [ 'data' => [ 'message' => (string)$this->l->t('Could not change the password. Maybe the old password was not correct.') ] - ), 500); + ], 500); } /** @@ -164,22 +163,29 @@ class RecoveryController extends Controller { $result = $this->recovery->setRecoveryForUser($userEnableRecovery); if ($result) { + if ($userEnableRecovery === '0') { + return new DataResponse( + [ + 'data' => [ + 'message' => (string)$this->l->t('Recovery Key disabled')] + ] + ); + } return new DataResponse( - array( - 'status' => 'success', + [ 'data' => [ 'message' => (string)$this->l->t('Recovery Key enabled')] - ) + ] ); } } return new DataResponse( - array( + [ 'data' => [ 'message' => (string)$this->l->t('Could not enable the recovery key, please try again or contact your administrator') ] - ), 500); + ], 500); } } diff --git a/apps/encryption/js/settings-admin.js b/apps/encryption/js/settings-admin.js index bb539f6a4e..fdc53c5215 100644 --- a/apps/encryption/js/settings-admin.js +++ b/apps/encryption/js/settings-admin.js @@ -7,52 +7,59 @@ * See the COPYING-README file. */ -$(document).ready(function(){ +$(document).ready(function () { - $( 'input:radio[name="adminEnableRecovery"]' ).change( - function() { - var recoveryStatus = $( this ).val(); - var oldStatus = (1+parseInt(recoveryStatus, 10)) % 2; - var recoveryPassword = $( '#encryptionRecoveryPassword' ).val(); - var confirmPassword = $( '#repeatEncryptionRecoveryPassword' ).val(); + $('input:radio[name="adminEnableRecovery"]').change( + function () { + var recoveryStatus = $(this).val(); + var oldStatus = (1 + parseInt(recoveryStatus)) % 2; + var recoveryPassword = $('#encryptionRecoveryPassword').val(); + var confirmPassword = $('#repeatEncryptionRecoveryPassword').val(); OC.msg.startSaving('#encryptionSetRecoveryKey .msg'); $.post( OC.generateUrl('/apps/encryption/ajax/adminRecovery'), - { adminEnableRecovery: recoveryStatus, + { + adminEnableRecovery: recoveryStatus, recoveryPassword: recoveryPassword, - confirmPassword: confirmPassword }, - function( result ) { - OC.msg.finishedSaving('#encryptionSetRecoveryKey .msg', result); - if (result.status === "error") { - $('input:radio[name="adminEnableRecovery"][value="'+oldStatus.toString()+'"]') - .attr("checked", "true"); - } else { - if (recoveryStatus === "0") { - $('p[name="changeRecoveryPasswordBlock"]').addClass("hidden"); - } else { - $('input:password[name="changeRecoveryPassword"]').val(""); - $('p[name="changeRecoveryPasswordBlock"]').removeClass("hidden"); - } - } + confirmPassword: confirmPassword } - ); + ).done(function (data) { + OC.msg.finishedSuccess('#encryptionSetRecoveryKey .msg', data.data.message); + + if (recoveryStatus === "0") { + $('p[name="changeRecoveryPasswordBlock"]').addClass("hidden"); + } else { + $('input:password[name="changeRecoveryPassword"]').val(""); + $('p[name="changeRecoveryPasswordBlock"]').removeClass("hidden"); + } + }) + .fail(function (jqXHR) { + $('input:radio[name="adminEnableRecovery"][value="' + oldStatus.toString() + '"]').attr("checked", "true"); + OC.msg.finishedError('#encryptionSetRecoveryKey .msg', JSON.parse(jqXHR.responseText).data.message); + }); } ); // change recovery password - $('button:button[name="submitChangeRecoveryKey"]').click(function() { + $('button:button[name="submitChangeRecoveryKey"]').click(function () { var oldRecoveryPassword = $('#oldEncryptionRecoveryPassword').val(); var newRecoveryPassword = $('#newEncryptionRecoveryPassword').val(); var confirmNewPassword = $('#repeatedNewEncryptionRecoveryPassword').val(); OC.msg.startSaving('#encryptionChangeRecoveryKey .msg'); $.post( - OC.generateUrl('/apps/encryption/ajax/changeRecoveryPassword'), - { oldPassword: oldRecoveryPassword, newPassword: newRecoveryPassword, confirmPassword: confirmNewPassword }, - function( data ) { - OC.msg.finishedSaving('#encryptionChangeRecoveryKey .msg', data); - } - ); + OC.generateUrl('/apps/encryption/ajax/changeRecoveryPassword'), + { + oldPassword: oldRecoveryPassword, + newPassword: newRecoveryPassword, + confirmPassword: confirmNewPassword + } + ).done(function (data) { + OC.msg.finishedSuccess('#encryptionChangeRecoveryKey .msg', data.data.message); + }) + .fail(function (jqXHR) { + OC.msg.finishedError('#encryptionChangeRecoveryKey .msg', JSON.parse(jqXHR.responseText).data.message); + }); }); }); diff --git a/apps/encryption/js/settings-personal.js b/apps/encryption/js/settings-personal.js index e36f10a244..4728da8708 100644 --- a/apps/encryption/js/settings-personal.js +++ b/apps/encryption/js/settings-personal.js @@ -9,35 +9,43 @@ if (!OC.Encryption) { } OC.Encryption = { - updatePrivateKeyPassword: function() { + updatePrivateKeyPassword: function () { var oldPrivateKeyPassword = $('input:password[id="oldPrivateKeyPassword"]').val(); var newPrivateKeyPassword = $('input:password[id="newPrivateKeyPassword"]').val(); OC.msg.startSaving('#encryption .msg'); $.post( OC.generateUrl('/apps/encryption/ajax/updatePrivateKeyPassword'), - {oldPassword: oldPrivateKeyPassword, newPassword: newPrivateKeyPassword} - ).success(function (response) { - OC.msg.finishedSuccess('#encryption .msg', response.message); - }).fail(function (response) { - OC.msg.finishedError('#encryption .msg', response.responseJSON.message); - }); + { + oldPassword: oldPrivateKeyPassword, + newPassword: newPrivateKeyPassword + } + ).done(function (data) { + OC.msg.finishedSuccess('#encryption .msg', data.data.message); + }) + .fail(function (jqXHR) { + OC.msg.finishedError('#encryption .msg', JSON.parse(jqXHR.responseText).data.message); + }); } }; -$(document).ready(function(){ +$(document).ready(function () { // Trigger ajax on recoveryAdmin status change - $( 'input:radio[name="userEnableRecovery"]' ).change( - function() { - var recoveryStatus = $( this ).val(); + $('input:radio[name="userEnableRecovery"]').change( + function () { + var recoveryStatus = $(this).val(); OC.msg.startAction('#userEnableRecovery .msg', 'Updating recovery keys. This can take some time...'); $.post( - OC.generateUrl('/apps/encryption/ajax/userSetRecovery'), - { userEnableRecovery: recoveryStatus }, - function( data ) { - OC.msg.finishedAction('#userEnableRecovery .msg', data); + OC.generateUrl('/apps/encryption/ajax/userSetRecovery'), + { + userEnableRecovery: recoveryStatus } - ); + ).done(function (data) { + OC.msg.finishedSuccess('#userEnableRecovery .msg', data.data.message); + }) + .fail(function (jqXHR) { + OC.msg.finishedError('#userEnableRecovery .msg', JSON.parse(jqXHR.responseText).data.message); + }); // Ensure page is not reloaded on form submit return false; } @@ -45,12 +53,12 @@ $(document).ready(function(){ // update private key password - $('input:password[name="changePrivateKeyPassword"]').keyup(function(event) { + $('input:password[name="changePrivateKeyPassword"]').keyup(function (event) { var oldPrivateKeyPassword = $('input:password[id="oldPrivateKeyPassword"]').val(); var newPrivateKeyPassword = $('input:password[id="newPrivateKeyPassword"]').val(); - if (newPrivateKeyPassword !== '' && oldPrivateKeyPassword !== '' ) { + if (newPrivateKeyPassword !== '' && oldPrivateKeyPassword !== '') { $('button:button[name="submitChangePrivateKeyPassword"]').removeAttr("disabled"); - if(event.which === 13) { + if (event.which === 13) { OC.Encryption.updatePrivateKeyPassword(); } } else { @@ -58,7 +66,7 @@ $(document).ready(function(){ } }); - $('button:button[name="submitChangePrivateKeyPassword"]').click(function() { + $('button:button[name="submitChangePrivateKeyPassword"]').click(function () { OC.Encryption.updatePrivateKeyPassword(); }); From 29168665cb8acb3296ba734500a869a70313abdc Mon Sep 17 00:00:00 2001 From: Clark Tomlinson Date: Wed, 22 Apr 2015 13:26:06 -0400 Subject: [PATCH 8/9] fix messages from settings crontroller --- .../controller/recoverycontroller.php | 25 ++++++++++--------- apps/encryption/js/settings-personal.js | 4 +-- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/apps/encryption/controller/recoverycontroller.php b/apps/encryption/controller/recoverycontroller.php index 8ae37d97ec..f163b8fe64 100644 --- a/apps/encryption/controller/recoverycontroller.php +++ b/apps/encryption/controller/recoverycontroller.php @@ -26,6 +26,7 @@ namespace OCA\Encryption\Controller; use OCA\Encryption\Recovery; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -74,34 +75,34 @@ class RecoveryController extends Controller { if (empty($recoveryPassword)) { $errorMessage = (string)$this->l->t('Missing recovery key password'); return new DataResponse(['data' => ['message' => $errorMessage]], - 500); + Http::STATUS_INTERNAL_SERVER_ERROR); } if (empty($confirmPassword)) { $errorMessage = (string)$this->l->t('Please repeat the recovery key password'); return new DataResponse(['data' => ['message' => $errorMessage]], - 500); + Http::STATUS_INTERNAL_SERVER_ERROR); } if ($recoveryPassword !== $confirmPassword) { $errorMessage = (string)$this->l->t('Repeated recovery key password does not match the provided recovery key password'); return new DataResponse(['data' => ['message' => $errorMessage]], - 500); + Http::STATUS_INTERNAL_SERVER_ERROR); } if (isset($adminEnableRecovery) && $adminEnableRecovery === '1') { if ($this->recovery->enableAdminRecovery($recoveryPassword)) { return new DataResponse(['data' => ['message' => (string)$this->l->t('Recovery key successfully enabled')]]); } - return new DataResponse(['data' => ['message' => (string)$this->l->t('Could not enable recovery key. Please check your recovery key password!')]], 500); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Could not enable recovery key. Please check your recovery key password!')]], Http::STATUS_INTERNAL_SERVER_ERROR); } elseif (isset($adminEnableRecovery) && $adminEnableRecovery === '0') { if ($this->recovery->disableAdminRecovery($recoveryPassword)) { return new DataResponse(['data' => ['message' => (string)$this->l->t('Recovery key successfully disabled')]]); } - return new DataResponse(['data' => ['message' => (string)$this->l->t('Could not disable recovery key. Please check your recovery key password!')]], 500); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Could not disable recovery key. Please check your recovery key password!')]], Http::STATUS_INTERNAL_SERVER_ERROR); } // this response should never be sent but just in case. - return new DataResponse(['data' => ['message' => (string)$this->l->t('Missing parameters')]], 500); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Missing parameters')]], Http::STATUS_INTERNAL_SERVER_ERROR); } /** @@ -114,22 +115,22 @@ class RecoveryController extends Controller { //check if both passwords are the same if (empty($oldPassword)) { $errorMessage = (string)$this->l->t('Please provide the old recovery password'); - return new DataResponse(['data' => ['message' => $errorMessage]], 500); + return new DataResponse(['data' => ['message' => $errorMessage]], Http::STATUS_INTERNAL_SERVER_ERROR); } if (empty($newPassword)) { $errorMessage = (string)$this->l->t('Please provide a new recovery password'); - return new DataResponse (['data' => ['message' => $errorMessage]], 500); + return new DataResponse (['data' => ['message' => $errorMessage]], Http::STATUS_INTERNAL_SERVER_ERROR); } if (empty($confirmPassword)) { $errorMessage = (string)$this->l->t('Please repeat the new recovery password'); - return new DataResponse(['data' => ['message' => $errorMessage]], 500); + return new DataResponse(['data' => ['message' => $errorMessage]], Http::STATUS_INTERNAL_SERVER_ERROR); } if ($newPassword !== $confirmPassword) { $errorMessage = (string)$this->l->t('Repeated recovery key password does not match the provided recovery key password'); - return new DataResponse(['data' => ['message' => $errorMessage]], 500); + return new DataResponse(['data' => ['message' => $errorMessage]], Http::STATUS_INTERNAL_SERVER_ERROR); } $result = $this->recovery->changeRecoveryKeyPassword($newPassword, @@ -148,7 +149,7 @@ class RecoveryController extends Controller { 'data' => [ 'message' => (string)$this->l->t('Could not change the password. Maybe the old password was not correct.') ] - ], 500); + ], Http::STATUS_INTERNAL_SERVER_ERROR); } /** @@ -185,7 +186,7 @@ class RecoveryController extends Controller { 'data' => [ 'message' => (string)$this->l->t('Could not enable the recovery key, please try again or contact your administrator') ] - ], 500); + ], Http::STATUS_INTERNAL_SERVER_ERROR); } } diff --git a/apps/encryption/js/settings-personal.js b/apps/encryption/js/settings-personal.js index 4728da8708..658ba2a86e 100644 --- a/apps/encryption/js/settings-personal.js +++ b/apps/encryption/js/settings-personal.js @@ -20,10 +20,10 @@ OC.Encryption = { newPassword: newPrivateKeyPassword } ).done(function (data) { - OC.msg.finishedSuccess('#encryption .msg', data.data.message); + OC.msg.finishedSuccess('#encryption .msg', data.message); }) .fail(function (jqXHR) { - OC.msg.finishedError('#encryption .msg', JSON.parse(jqXHR.responseText).data.message); + OC.msg.finishedError('#encryption .msg', JSON.parse(jqXHR.responseText).message); }); } }; From 8c0856779bccb41014f677c5ebdec79aec0a5602 Mon Sep 17 00:00:00 2001 From: Clark Tomlinson Date: Fri, 24 Apr 2015 09:42:02 -0400 Subject: [PATCH 9/9] change error codes to 400 --- .../controller/recoverycontroller.php | 24 +++++++++---------- .../controller/RecoveryControllerTest.php | 16 ++++++------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/apps/encryption/controller/recoverycontroller.php b/apps/encryption/controller/recoverycontroller.php index f163b8fe64..f1a2651443 100644 --- a/apps/encryption/controller/recoverycontroller.php +++ b/apps/encryption/controller/recoverycontroller.php @@ -75,34 +75,34 @@ class RecoveryController extends Controller { if (empty($recoveryPassword)) { $errorMessage = (string)$this->l->t('Missing recovery key password'); return new DataResponse(['data' => ['message' => $errorMessage]], - Http::STATUS_INTERNAL_SERVER_ERROR); + Http::STATUS_BAD_REQUEST); } if (empty($confirmPassword)) { $errorMessage = (string)$this->l->t('Please repeat the recovery key password'); return new DataResponse(['data' => ['message' => $errorMessage]], - Http::STATUS_INTERNAL_SERVER_ERROR); + Http::STATUS_BAD_REQUEST); } if ($recoveryPassword !== $confirmPassword) { $errorMessage = (string)$this->l->t('Repeated recovery key password does not match the provided recovery key password'); return new DataResponse(['data' => ['message' => $errorMessage]], - Http::STATUS_INTERNAL_SERVER_ERROR); + Http::STATUS_BAD_REQUEST); } if (isset($adminEnableRecovery) && $adminEnableRecovery === '1') { if ($this->recovery->enableAdminRecovery($recoveryPassword)) { return new DataResponse(['data' => ['message' => (string)$this->l->t('Recovery key successfully enabled')]]); } - return new DataResponse(['data' => ['message' => (string)$this->l->t('Could not enable recovery key. Please check your recovery key password!')]], Http::STATUS_INTERNAL_SERVER_ERROR); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Could not enable recovery key. Please check your recovery key password!')]], Http::STATUS_BAD_REQUEST); } elseif (isset($adminEnableRecovery) && $adminEnableRecovery === '0') { if ($this->recovery->disableAdminRecovery($recoveryPassword)) { return new DataResponse(['data' => ['message' => (string)$this->l->t('Recovery key successfully disabled')]]); } - return new DataResponse(['data' => ['message' => (string)$this->l->t('Could not disable recovery key. Please check your recovery key password!')]], Http::STATUS_INTERNAL_SERVER_ERROR); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Could not disable recovery key. Please check your recovery key password!')]], Http::STATUS_BAD_REQUEST); } // this response should never be sent but just in case. - return new DataResponse(['data' => ['message' => (string)$this->l->t('Missing parameters')]], Http::STATUS_INTERNAL_SERVER_ERROR); + return new DataResponse(['data' => ['message' => (string)$this->l->t('Missing parameters')]], Http::STATUS_BAD_REQUEST); } /** @@ -115,22 +115,22 @@ class RecoveryController extends Controller { //check if both passwords are the same if (empty($oldPassword)) { $errorMessage = (string)$this->l->t('Please provide the old recovery password'); - return new DataResponse(['data' => ['message' => $errorMessage]], Http::STATUS_INTERNAL_SERVER_ERROR); + return new DataResponse(['data' => ['message' => $errorMessage]], Http::STATUS_BAD_REQUEST); } if (empty($newPassword)) { $errorMessage = (string)$this->l->t('Please provide a new recovery password'); - return new DataResponse (['data' => ['message' => $errorMessage]], Http::STATUS_INTERNAL_SERVER_ERROR); + return new DataResponse (['data' => ['message' => $errorMessage]], Http::STATUS_BAD_REQUEST); } if (empty($confirmPassword)) { $errorMessage = (string)$this->l->t('Please repeat the new recovery password'); - return new DataResponse(['data' => ['message' => $errorMessage]], Http::STATUS_INTERNAL_SERVER_ERROR); + return new DataResponse(['data' => ['message' => $errorMessage]], Http::STATUS_BAD_REQUEST); } if ($newPassword !== $confirmPassword) { $errorMessage = (string)$this->l->t('Repeated recovery key password does not match the provided recovery key password'); - return new DataResponse(['data' => ['message' => $errorMessage]], Http::STATUS_INTERNAL_SERVER_ERROR); + return new DataResponse(['data' => ['message' => $errorMessage]], Http::STATUS_BAD_REQUEST); } $result = $this->recovery->changeRecoveryKeyPassword($newPassword, @@ -149,7 +149,7 @@ class RecoveryController extends Controller { 'data' => [ 'message' => (string)$this->l->t('Could not change the password. Maybe the old password was not correct.') ] - ], Http::STATUS_INTERNAL_SERVER_ERROR); + ], Http::STATUS_BAD_REQUEST); } /** @@ -186,7 +186,7 @@ class RecoveryController extends Controller { 'data' => [ 'message' => (string)$this->l->t('Could not enable the recovery key, please try again or contact your administrator') ] - ], Http::STATUS_INTERNAL_SERVER_ERROR); + ], Http::STATUS_BAD_REQUEST); } } diff --git a/apps/encryption/tests/controller/RecoveryControllerTest.php b/apps/encryption/tests/controller/RecoveryControllerTest.php index 0ac76774c5..89b541e7bd 100644 --- a/apps/encryption/tests/controller/RecoveryControllerTest.php +++ b/apps/encryption/tests/controller/RecoveryControllerTest.php @@ -40,9 +40,9 @@ class RecoveryControllerTest extends TestCase { public function adminRecoveryProvider() { return [ ['test', 'test', '1', 'Recovery key successfully enabled', HTTP::STATUS_OK], - ['', 'test', '1', 'Missing recovery key password', HTTP::STATUS_INTERNAL_SERVER_ERROR], - ['test', '', '1', 'Please repeat the recovery key password', HTTP::STATUS_INTERNAL_SERVER_ERROR], - ['test', 'soimething that doesn\'t match', '1', 'Repeated recovery key password does not match the provided recovery key password', HTTP::STATUS_INTERNAL_SERVER_ERROR], + ['', 'test', '1', 'Missing recovery key password', HTTP::STATUS_BAD_REQUEST], + ['test', '', '1', 'Please repeat the recovery key password', HTTP::STATUS_BAD_REQUEST], + ['test', 'soimething that doesn\'t match', '1', 'Repeated recovery key password does not match the provided recovery key password', HTTP::STATUS_BAD_REQUEST], ['test', 'test', '0', 'Recovery key successfully disabled', HTTP::STATUS_OK], ]; } @@ -79,11 +79,11 @@ class RecoveryControllerTest extends TestCase { public function changeRecoveryPasswordProvider() { return [ - ['test', 'test', 'oldtestFail', 'Could not change the password. Maybe the old password was not correct.', HTTP::STATUS_INTERNAL_SERVER_ERROR], + ['test', 'test', 'oldtestFail', 'Could not change the password. Maybe the old password was not correct.', HTTP::STATUS_BAD_REQUEST], ['test', 'test', 'oldtest', 'Password successfully changed.', HTTP::STATUS_OK], - ['test', 'notmatch', 'oldtest', 'Repeated recovery key password does not match the provided recovery key password', HTTP::STATUS_INTERNAL_SERVER_ERROR], - ['', 'test', 'oldtest', 'Please provide a new recovery password', HTTP::STATUS_INTERNAL_SERVER_ERROR], - ['test', 'test', '', 'Please provide the old recovery password', HTTP::STATUS_INTERNAL_SERVER_ERROR] + ['test', 'notmatch', 'oldtest', 'Repeated recovery key password does not match the provided recovery key password', HTTP::STATUS_BAD_REQUEST], + ['', 'test', 'oldtest', 'Please provide a new recovery password', HTTP::STATUS_BAD_REQUEST], + ['test', 'test', '', 'Please provide the old recovery password', HTTP::STATUS_BAD_REQUEST] ]; } @@ -117,7 +117,7 @@ class RecoveryControllerTest extends TestCase { public function userSetRecoveryProvider() { return [ ['1', 'Recovery Key enabled', Http::STATUS_OK], - ['0', 'Could not enable the recovery key, please try again or contact your administrator', Http::STATUS_INTERNAL_SERVER_ERROR] + ['0', 'Could not enable the recovery key, please try again or contact your administrator', Http::STATUS_BAD_REQUEST] ]; }