From 5549641f1f977ab2b105b8f9d7b8c6829c0e6d02 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 27 May 2015 10:37:12 +0200 Subject: [PATCH 1/2] improve error messages displayed to the user --- apps/encryption/lib/crypto/encryption.php | 2 +- .../exceptions/decryptionfailedexception.php | 11 ----------- lib/private/encryption/manager.php | 11 +++++++++-- lib/private/server.php | 2 +- .../exceptions/genericencryptionexception.php | 17 +++++------------ tests/lib/encryption/managertest.php | 6 +++++- 6 files changed, 21 insertions(+), 28 deletions(-) diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php index f527955b49..9094a84d4c 100644 --- a/apps/encryption/lib/crypto/encryption.php +++ b/apps/encryption/lib/crypto/encryption.php @@ -387,7 +387,7 @@ class Encryption implements IEncryptionModule { '" is not able to read ' . $path; $hint = $this->l->t('Can not read this file, probably this is a shared file. Please ask the file owner to reshare the file with you.'); $this->logger->warning($msg); - throw new DecryptionFailedException($msg, 0, null, $hint); + throw new DecryptionFailedException($msg, $hint); } return false; } diff --git a/lib/private/encryption/exceptions/decryptionfailedexception.php b/lib/private/encryption/exceptions/decryptionfailedexception.php index 7e9fa21eae..406ae12968 100644 --- a/lib/private/encryption/exceptions/decryptionfailedexception.php +++ b/lib/private/encryption/exceptions/decryptionfailedexception.php @@ -27,15 +27,4 @@ use OCP\Encryption\Exceptions\GenericEncryptionException; class DecryptionFailedException extends GenericEncryptionException { - /** - * @param string $message - * @param int $code - * @param \Exception $previous - * @param string $hint - */ - public function __construct($message = '', $code = 0, \Exception $previous = null, $hint = '') { - parent::__construct($message, $code, $previous, $hint); - -} - } diff --git a/lib/private/encryption/manager.php b/lib/private/encryption/manager.php index 45f4504564..6942376f0b 100644 --- a/lib/private/encryption/manager.php +++ b/lib/private/encryption/manager.php @@ -30,6 +30,7 @@ use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; use OCP\Files\Mount\IMountPoint; use OCP\IConfig; +use OCP\IL10N; use OCP\ILogger; class Manager implements IManager { @@ -43,14 +44,19 @@ class Manager implements IManager { /** @var ILogger */ protected $logger; + /** @var Il10n */ + protected $l; + /** * @param IConfig $config * @param ILogger $logger + * @param IL10N $l10n */ - public function __construct(IConfig $config, ILogger $logger) { + public function __construct(IConfig $config, ILogger $logger, IL10N $l10n) { $this->encryptionModules = array(); $this->config = $config; $this->logger = $logger; + $this->l = $l10n; } /** @@ -145,7 +151,8 @@ class Manager implements IManager { return call_user_func($this->encryptionModules[$moduleId]['callback']); } else { $message = "Module with id: $moduleId does not exists."; - throw new Exceptions\ModuleDoesNotExistsException($message); + $hint = $this->l->t('Module with id: %s does not exists. Please enable it in your apps settings or contact your administrator.', [$moduleId]); + throw new Exceptions\ModuleDoesNotExistsException($message, $hint); } } else { return $this->getDefaultEncryptionModule(); diff --git a/lib/private/server.php b/lib/private/server.php index 7fa668b222..aeea4a6485 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -84,7 +84,7 @@ class Server extends SimpleContainer implements IServerContainer { }); $this->registerService('EncryptionManager', function (Server $c) { - return new Encryption\Manager($c->getConfig(), $c->getLogger()); + return new Encryption\Manager($c->getConfig(), $c->getLogger(), $c->getL10N('core')); }); $this->registerService('EncryptionFileHelper', function (Server $c) { diff --git a/lib/public/encryption/exceptions/genericencryptionexception.php b/lib/public/encryption/exceptions/genericencryptionexception.php index e97f00c88b..5648e5edf7 100644 --- a/lib/public/encryption/exceptions/genericencryptionexception.php +++ b/lib/public/encryption/exceptions/genericencryptionexception.php @@ -21,6 +21,7 @@ */ namespace OCP\Encryption\Exceptions; +use OC\HintException; /** * Class GenericEncryptionException @@ -28,28 +29,20 @@ namespace OCP\Encryption\Exceptions; * @package OCP\Encryption\Exceptions * @since 8.1.0 */ -class GenericEncryptionException extends \Exception { - - /** @var string */ - protected $hint; +class GenericEncryptionException extends HintException { /** * @param string $message + * @param string $hint * @param int $code * @param \Exception $previous * @since 8.1.0 */ - public function __construct($message = '', $code = 0, \Exception $previous = null, $hint = '') { + public function __construct($message = '', $hint = '', $code = 0, \Exception $previous = null) { if (empty($message)) { $message = 'Unspecified encryption exception'; } - parent::__construct($message, $code, $previous); - - $this->hint = $hint; - } - - public function getHint() { - return $this->hint; + parent::__construct($message, $hint, $code, $previous); } } diff --git a/tests/lib/encryption/managertest.php b/tests/lib/encryption/managertest.php index faca647450..3b1e07ffd6 100644 --- a/tests/lib/encryption/managertest.php +++ b/tests/lib/encryption/managertest.php @@ -16,11 +16,15 @@ class ManagerTest extends TestCase { /** @var \PHPUnit_Framework_MockObject_MockObject */ private $logger; + /** @var \PHPUnit_Framework_MockObject_MockObject */ + private $l10n; + public function setUp() { parent::setUp(); $this->config = $this->getMock('\OCP\IConfig'); $this->logger = $this->getMock('\OCP\ILogger'); - $this->manager = new Manager($this->config, $this->logger); + $this->l10n = $this->getMock('\OCP\Il10n'); + $this->manager = new Manager($this->config, $this->logger, $this->l10n); } public function testManagerIsDisabled() { From 68db3059eec904e90c41a8452799222e21c9c460 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 27 May 2015 11:10:06 +0200 Subject: [PATCH 2/2] detect migration status --- apps/encryption/appinfo/app.php | 6 +- apps/encryption/appinfo/application.php | 8 +- .../controller/statuscontroller.php | 15 +++- apps/encryption/js/encryption.js | 2 +- apps/encryption/lib/session.php | 1 + .../tests/controller/StatusControllerTest.php | 90 +++++++++++++++++++ 6 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 apps/encryption/tests/controller/StatusControllerTest.php diff --git a/apps/encryption/appinfo/app.php b/apps/encryption/appinfo/app.php index 0c7c231aef..2eb12f638e 100644 --- a/apps/encryption/appinfo/app.php +++ b/apps/encryption/appinfo/app.php @@ -25,8 +25,10 @@ namespace OCA\Encryption\AppInfo; \OCP\Util::addscript('encryption', 'encryption'); -$app = new Application(); -if (\OC::$server->getEncryptionManager()->isReady()) { +$encryptionSystemReady = \OC::$server->getEncryptionManager()->isReady(); + +$app = new Application([], $encryptionSystemReady); +if ($encryptionSystemReady) { $app->registerEncryptionModule(); $app->registerHooks(); $app->registerSettings(); diff --git a/apps/encryption/appinfo/application.php b/apps/encryption/appinfo/application.php index 10ad610cd4..cb9c33cfe5 100644 --- a/apps/encryption/appinfo/application.php +++ b/apps/encryption/appinfo/application.php @@ -52,12 +52,18 @@ class Application extends \OCP\AppFramework\App { /** * @param array $urlParams + * @param bool $encryptionSystemReady */ - public function __construct($urlParams = array()) { + public function __construct($urlParams = array(), $encryptionSystemReady = true) { parent::__construct('encryption', $urlParams); $this->encryptionManager = \OC::$server->getEncryptionManager(); $this->config = \OC::$server->getConfig(); $this->registerServices(); + if($encryptionSystemReady === false) { + /** @var Session $session */ + $session = $this->getContainer()->query('Session'); + $session->setStatus(Session::RUN_MIGRATION); + } } /** diff --git a/apps/encryption/controller/statuscontroller.php b/apps/encryption/controller/statuscontroller.php index ef3d70a0b4..cdc4b2e92e 100644 --- a/apps/encryption/controller/statuscontroller.php +++ b/apps/encryption/controller/statuscontroller.php @@ -60,20 +60,29 @@ class StatusController extends Controller { public function getStatus() { $status = 'error'; - $message = ''; + $message = 'no valid init status'; switch( $this->session->getStatus()) { + case Session::RUN_MIGRATION: + $status = 'interactionNeeded'; + $message = (string)$this->l->t( + 'You need to migrate your encryption keys from the old encryption (ownCloud <= 8.0) to the new one. Please run \'occ encryption:migrate\' or contact your administrator' + ); + break; case Session::INIT_EXECUTED: - $status = 'success'; + $status = 'interactionNeeded'; $message = (string)$this->l->t( 'Invalid private key for Encryption App. Please update your private key password in your personal settings to recover access to your encrypted files.' ); break; case Session::NOT_INITIALIZED: - $status = 'success'; + $status = 'interactionNeeded'; $message = (string)$this->l->t( 'Encryption App is enabled but your keys are not initialized, please log-out and log-in again' ); break; + case Session::INIT_SUCCESSFUL: + $status = 'success'; + $message = (string)$this->l->t('Encryption App is enabled and ready'); } return new DataResponse( diff --git a/apps/encryption/js/encryption.js b/apps/encryption/js/encryption.js index ea6a5596f2..a6c1bea89b 100644 --- a/apps/encryption/js/encryption.js +++ b/apps/encryption/js/encryption.js @@ -22,7 +22,7 @@ OC.Encryption = { $.get( OC.generateUrl('/apps/encryption/ajax/getStatus'), function (result) { - if (result.status === "success") { + if (result.status === "interactionNeeded") { OC.Notification.show(result.data.message); } } diff --git a/apps/encryption/lib/session.php b/apps/encryption/lib/session.php index 85d2a7698e..9709518a27 100644 --- a/apps/encryption/lib/session.php +++ b/apps/encryption/lib/session.php @@ -33,6 +33,7 @@ class Session { const NOT_INITIALIZED = '0'; const INIT_EXECUTED = '1'; const INIT_SUCCESSFUL = '2'; + const RUN_MIGRATION = '3'; /** * @param ISession $session diff --git a/apps/encryption/tests/controller/StatusControllerTest.php b/apps/encryption/tests/controller/StatusControllerTest.php new file mode 100644 index 0000000000..b57fd1cc33 --- /dev/null +++ b/apps/encryption/tests/controller/StatusControllerTest.php @@ -0,0 +1,90 @@ + + * + * @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 OCA\Encryption\Tests\Controller; + + +use OCA\Encryption\Controller\StatusController; +use OCA\Encryption\Session; +use Test\TestCase; + +class StatusControllerTest extends TestCase { + + /** @var \PHPUnit_Framework_MockObject_MockObject */ + protected $requestMock; + + /** @var \PHPUnit_Framework_MockObject_MockObject */ + protected $l10nMock; + + /** @var \OCA\Encryption\Session | \PHPUnit_Framework_MockObject_MockObject */ + protected $sessionMock; + + /** @var StatusController */ + protected $controller; + + protected function setUp() { + + parent::setUp(); + + $this->sessionMock = $this->getMockBuilder('OCA\Encryption\Session') + ->disableOriginalConstructor()->getMock(); + $this->requestMock = $this->getMock('OCP\IRequest'); + + $this->l10nMock = $this->getMockBuilder('OCP\IL10N') + ->disableOriginalConstructor()->getMock(); + $this->l10nMock->expects($this->any()) + ->method('t') + ->will($this->returnCallback(function($message) { + return $message; + })); + + $this->controller = new StatusController('encryptionTest', + $this->requestMock, + $this->l10nMock, + $this->sessionMock); + + } + + /** + * @dataProvider dataTestGetStatus + * + * @param string $status + * @param string $expectedStatus + */ + public function testGetStatus($status, $expectedStatus) { + $this->sessionMock->expects($this->once()) + ->method('getStatus')->willReturn($status); + $result = $this->controller->getStatus(); + $data = $result->getData(); + $this->assertSame($expectedStatus, $data['status']); + } + + public function dataTestGetStatus() { + return array( + array(Session::RUN_MIGRATION, 'interactionNeeded'), + array(Session::INIT_EXECUTED, 'interactionNeeded'), + array(Session::INIT_SUCCESSFUL, 'success'), + array(Session::NOT_INITIALIZED, 'interactionNeeded'), + array('unknown', 'error'), + ); + } +}