From 20a6073a9feb6b0eb4c004e58bb47912c447fef9 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 15 Apr 2015 14:21:23 +0200 Subject: [PATCH] Migrate personal certificate handling into AppFramework controllers Also added unit-tests and better error-handling --- apps/files_external/appinfo/routes.php | 6 - settings/ajax/addRootCertificate.php | 52 ------ settings/ajax/removeRootCertificate.php | 28 --- settings/application.php | 12 ++ settings/controller/certificatecontroller.php | 93 ++++++++++ settings/js/personal.js | 22 ++- settings/personal.php | 2 + settings/routes.php | 6 +- settings/templates/personal.php | 3 +- .../controller/CertificateControllerTest.php | 174 ++++++++++++++++++ 10 files changed, 298 insertions(+), 100 deletions(-) delete mode 100644 settings/ajax/addRootCertificate.php delete mode 100644 settings/ajax/removeRootCertificate.php create mode 100644 settings/controller/certificatecontroller.php create mode 100644 tests/settings/controller/CertificateControllerTest.php diff --git a/apps/files_external/appinfo/routes.php b/apps/files_external/appinfo/routes.php index 98eb2fcccb..8c6dff7a90 100644 --- a/apps/files_external/appinfo/routes.php +++ b/apps/files_external/appinfo/routes.php @@ -46,12 +46,6 @@ $application->registerRoutes( ) ); -// TODO: move these to app framework -$this->create('files_external_add_root_certificate', 'ajax/addRootCertificate.php') - ->actionInclude('files_external/ajax/addRootCertificate.php'); -$this->create('files_external_remove_root_certificate', 'ajax/removeRootCertificate.php') - ->actionInclude('files_external/ajax/removeRootCertificate.php'); - $this->create('files_external_dropbox', 'ajax/dropbox.php') ->actionInclude('files_external/ajax/dropbox.php'); $this->create('files_external_google', 'ajax/google.php') diff --git a/settings/ajax/addRootCertificate.php b/settings/ajax/addRootCertificate.php deleted file mode 100644 index 64a55eaede..0000000000 --- a/settings/ajax/addRootCertificate.php +++ /dev/null @@ -1,52 +0,0 @@ - - * @author Robin Appelman - * - * @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 - * - */ -OCP\JSON::checkLoggedIn(); -OCP\JSON::callCheck(); - -$l = new OC_L10N('core'); - -if (!isset($_FILES['rootcert_import'])) { - OCP\JSON::error(array('error' => 'No certificate uploaded')); - exit; -} - -$data = file_get_contents($_FILES['rootcert_import']['tmp_name']); -$filename = basename($_FILES['rootcert_import']['name']); - -$certificateManager = \OC::$server->getCertificateManager(); - -try { - $cert = $certificateManager->addCertificate($data, $filename); - OCP\JSON::success(array( - 'name' => $cert->getName(), - 'commonName' => $cert->getCommonName(), - 'organization' => $cert->getOrganization(), - 'validFrom' => $cert->getIssueDate()->getTimestamp(), - 'validTill' => $cert->getExpireDate()->getTimestamp(), - 'validFromString' => $l->l('date', $cert->getIssueDate()), - 'validTillString' => $l->l('date', $cert->getExpireDate()), - 'issuer' => $cert->getIssuerName(), - 'issuerOrganization' => $cert->getIssuerOrganization() - )); -} catch(\Exception $e) { - OCP\JSON::error(array('error' => 'Couldn\'t import SSL root certificate, allowed formats: PEM and DER')); -} diff --git a/settings/ajax/removeRootCertificate.php b/settings/ajax/removeRootCertificate.php deleted file mode 100644 index 4ef5fe32ae..0000000000 --- a/settings/ajax/removeRootCertificate.php +++ /dev/null @@ -1,28 +0,0 @@ - - * @author Lukas Reschke - * @author Robin Appelman - * - * @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 - * - */ -OCP\JSON::checkLoggedIn(); -OCP\JSON::callCheck(); - -$name = (string)$_POST['cert']; -$certificateManager = \OC::$server->getCertificateManager(); -$certificateManager->removeCertificate($name); diff --git a/settings/application.php b/settings/application.php index 59fe9f6b65..920d172c93 100644 --- a/settings/application.php +++ b/settings/application.php @@ -25,6 +25,7 @@ namespace OC\Settings; use OC\Files\View; use OC\Settings\Controller\AppSettingsController; +use OC\Settings\Controller\CertificateController; use OC\Settings\Controller\CheckSetupController; use OC\Settings\Controller\EncryptionController; use OC\Settings\Controller\GroupsController; @@ -97,6 +98,14 @@ class Application extends App { $c->query('Config') ); }); + $container->registerService('CertificateController', function(IContainer $c) { + return new CertificateController( + $c->query('AppName'), + $c->query('Request'), + $c->query('CertificateManager'), + $c->query('L10N') + ); + }); $container->registerService('GroupsController', function(IContainer $c) { return new GroupsController( $c->query('AppName'), @@ -223,5 +232,8 @@ class Application extends App { $container->registerService('DatabaseConnection', function(IContainer $c) { return $c->query('ServerContainer')->getDatabaseConnection(); }); + $container->registerService('CertificateManager', function(IContainer $c){ + return $c->query('ServerContainer')->getCertificateManager(); + }); } } diff --git a/settings/controller/certificatecontroller.php b/settings/controller/certificatecontroller.php new file mode 100644 index 0000000000..c8d05a5124 --- /dev/null +++ b/settings/controller/certificatecontroller.php @@ -0,0 +1,93 @@ + + * + * @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\Settings\Controller; + +use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\DataResponse; +use OCP\ICertificateManager; +use OCP\IL10N; +use OCP\IRequest; + +/** + * @package OC\Settings\Controller + */ +class CertificateController extends Controller { + /** @var ICertificateManager */ + private $certificateManager; + /** @var IL10N */ + private $l10n; + + /** + * @param string $appName + * @param IRequest $request + * @param ICertificateManager $certificateManager + * @param IL10N $l10n + */ + public function __construct($appName, + IRequest $request, + ICertificateManager $certificateManager, + IL10N $l10n) { + parent::__construct($appName, $request); + $this->certificateManager = $certificateManager; + $this->l10n = $l10n; + } + + /** + * Add a new personal root certificate to the users' trust store + * @return array + */ + public function addPersonalRootCertificate() { + $file = $this->request->getUploadedFile('rootcert_import'); + if(empty($file)) { + return new DataResponse(['message' => 'No file uploaded'], Http::STATUS_UNPROCESSABLE_ENTITY); + } + + try { + $certificate = $this->certificateManager->addCertificate(file_get_contents($file['tmp_name']), $file['name']); + return new DataResponse([ + 'name' => $certificate->getName(), + 'commonName' => $certificate->getCommonName(), + 'organization' => $certificate->getOrganization(), + 'validFrom' => $certificate->getIssueDate()->getTimestamp(), + 'validTill' => $certificate->getExpireDate()->getTimestamp(), + 'validFromString' => $this->l10n->l('date', $certificate->getIssueDate()), + 'validTillString' => $this->l10n->l('date', $certificate->getExpireDate()), + 'issuer' => $certificate->getIssuerName(), + 'issuerOrganization' => $certificate->getIssuerOrganization(), + ]); + } catch (\Exception $e) { + return new DataResponse('An error occurred.', Http::STATUS_INTERNAL_SERVER_ERROR); + } + } + + /** + * Removes a personal root certificate from the users' trust store + * @param string $certificateIdentifier + * @return DataResponse + */ + public function removePersonalRootCertificate($certificateIdentifier) { + $this->certificateManager->removeCertificate($certificateIdentifier); + return new DataResponse(); + } + +} diff --git a/settings/js/personal.js b/settings/js/personal.js index 687b02399a..c251d1572b 100644 --- a/settings/js/personal.js +++ b/settings/js/personal.js @@ -297,8 +297,8 @@ $(document).ready(function () { $('#sslCertificate').on('click', 'td.remove > img', function () { var row = $(this).parent().parent(); - $.post(OC.generateUrl('settings/ajax/removeRootCertificate'), { - cert: row.data('name') + $.ajax(OC.generateUrl('settings/personal/certificate/{certificate}', {certificate: row.data('name')}), { + type: 'DELETE' }); row.remove(); return true; @@ -307,18 +307,19 @@ $(document).ready(function () { $('#sslCertificate tr > td').tipsy({gravity: 'n', live: true}); $('#rootcert_import').fileupload({ - done: function (e, data) { - var issueDate = new Date(data.result.validFrom * 1000); - var expireDate = new Date(data.result.validTill * 1000); + success: function (data) { + var issueDate = new Date(data.validFrom * 1000); + var expireDate = new Date(data.validTill * 1000); var now = new Date(); var isExpired = !(issueDate <= now && now <= expireDate); var row = $(''); + row.data('name', data.name); row.addClass(isExpired? 'expired': 'valid'); - row.append($('').attr('title', data.result.organization).text(data.result.commonName)); - row.append($('').attr('title', t('core,', 'Valid until {date}', {date: data.result.validFromString})) - .text(data.result.validTillString)); - row.append($('').attr('title', data.result.issuerOrganization).text(data.result.issuer)); + row.append($('').attr('title', data.organization).text(data.commonName)); + row.append($('').attr('title', t('core,', 'Valid until {date}', {date: data.validFromString})) + .text(data.validTillString)); + row.append($('').attr('title', data.issuerOrganization).text(data.issuer)); row.append($('').addClass('remove').append( $('').attr({ alt: t('core', 'Delete'), @@ -328,6 +329,9 @@ $(document).ready(function () { )); $('#sslCertificate tbody').append(row); + }, + fail: function (e, data) { + OC.Notification.showTemporary(t('settings', 'An error occured. Please upload an ASCII-encoded PEM certificate.')); } }); diff --git a/settings/personal.php b/settings/personal.php index 12b320ac00..7bf1110c03 100644 --- a/settings/personal.php +++ b/settings/personal.php @@ -37,6 +37,7 @@ OC_Util::checkLoggedIn(); $defaults = new OC_Defaults(); // initialize themable default strings and urls $certificateManager = \OC::$server->getCertificateManager(); $config = \OC::$server->getConfig(); +$urlGenerator = \OC::$server->getURLGenerator(); // Highlight navigation entry OC_Util::addScript( 'settings', 'personal' ); @@ -118,6 +119,7 @@ $tmpl->assign('displayName', OC_User::getDisplayName()); $tmpl->assign('enableAvatars', $config->getSystemValue('enable_avatars', true)); $tmpl->assign('avatarChangeSupported', OC_User::canUserChangeAvatar(OC_User::getUser())); $tmpl->assign('certs', $certificateManager->listCertificates()); +$tmpl->assign('urlGenerator', $urlGenerator); // Get array of group ids for this user $groups = \OC::$server->getGroupManager()->getUserIdGroups(OC_User::getUser()); diff --git a/settings/routes.php b/settings/routes.php index 462b4ab543..52b320cbdb 100644 --- a/settings/routes.php +++ b/settings/routes.php @@ -53,6 +53,8 @@ $application->registerRoutes($this, [ ['name' => 'LogSettings#getEntries', 'url' => '/settings/admin/log/entries', 'verb' => 'GET'], ['name' => 'LogSettings#download', 'url' => '/settings/admin/log/download', 'verb' => 'GET'], ['name' => 'CheckSetup#check', 'url' => '/settings/ajax/checksetup', 'verb' => 'GET'], + ['name' => 'Certificate#addPersonalRootCertificate', 'url' => '/settings/personal/certificate', 'verb' => 'POST'], + ['name' => 'Certificate#removePersonalRootCertificate', 'url' => '/settings/personal/certificate/{certificateIdentifier}', 'verb' => 'DELETE'], ] ]); @@ -90,10 +92,6 @@ $this->create('settings_personal_changepassword', '/settings/personal/changepass ->action('OC\Settings\ChangePassword\Controller', 'changePersonalPassword'); $this->create('settings_ajax_setlanguage', '/settings/ajax/setlanguage.php') ->actionInclude('settings/ajax/setlanguage.php'); -$this->create('settings_cert_post', '/settings/ajax/addRootCertificate') - ->actionInclude('settings/ajax/addRootCertificate.php'); -$this->create('settings_cert_remove', '/settings/ajax/removeRootCertificate') - ->actionInclude('settings/ajax/removeRootCertificate.php'); // apps $this->create('settings_ajax_enableapp', '/settings/ajax/enableapp.php') ->actionInclude('settings/ajax/enableapp.php'); diff --git a/settings/templates/personal.php b/settings/templates/personal.php index dfdc619180..02ee261cd1 100644 --- a/settings/templates/personal.php +++ b/settings/templates/personal.php @@ -5,6 +5,7 @@ */ /** @var $_ array */ +/** @var $_['urlGenerator'] */ ?>
@@ -236,7 +237,7 @@ if($_['passwordChangeSupported']) { -
+
diff --git a/tests/settings/controller/CertificateControllerTest.php b/tests/settings/controller/CertificateControllerTest.php new file mode 100644 index 0000000000..363175cca4 --- /dev/null +++ b/tests/settings/controller/CertificateControllerTest.php @@ -0,0 +1,174 @@ + + * + * @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\Settings\Controller; + +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\DataResponse; +use OCP\IRequest; +use OCP\IL10N; +use OCP\ICertificateManager; + +/** + * Class CertificateControllerTest + * + * @package OC\Settings\Controller + */ +class CertificateControllerTest extends \Test\TestCase { + /** @var CertificateController */ + private $certificateController; + /** @var IRequest */ + private $request; + /** @var ICertificateManager */ + private $certificateManager; + /** @var IL10N */ + private $l10n; + + public function setUp() { + parent::setUp(); + + $this->request = $this->getMock('\OCP\IRequest'); + $this->certificateManager = $this->getMock('\OCP\ICertificateManager'); + $this->l10n = $this->getMock('\OCP\IL10N'); + + $this->certificateController = new CertificateController( + 'settings', + $this->request, + $this->certificateManager, + $this->l10n + ); + } + + public function testAddPersonalRootCertificateWithEmptyFile() { + $this->request + ->expects($this->once()) + ->method('getUploadedFile') + ->with('rootcert_import') + ->will($this->returnValue(null)); + + $expected = new DataResponse(['message' => 'No file uploaded'], Http::STATUS_UNPROCESSABLE_ENTITY); + $this->assertEquals($expected, $this->certificateController->addPersonalRootCertificate()); + } + + public function testAddPersonalRootCertificateValidCertificate() { + $uploadedFile = [ + 'tmp_name' => __DIR__ . '/../../data/certificates/goodCertificate.crt', + 'name' => 'goodCertificate.crt', + ]; + + $certificate = $this->getMock('\OCP\ICertificate'); + $certificate + ->expects($this->once()) + ->method('getName') + ->will($this->returnValue('Name')); + $certificate + ->expects($this->once()) + ->method('getCommonName') + ->will($this->returnValue('CommonName')); + $certificate + ->expects($this->once()) + ->method('getOrganization') + ->will($this->returnValue('Organization')); + $certificate + ->expects($this->exactly(2)) + ->method('getIssueDate') + ->will($this->returnValue(new \DateTime('@1429099555'))); + $certificate + ->expects($this->exactly(2)) + ->method('getExpireDate') + ->will($this->returnValue(new \DateTime('@1529099555'))); + $certificate + ->expects($this->once()) + ->method('getIssuerName') + ->will($this->returnValue('Issuer')); + $certificate + ->expects($this->once()) + ->method('getIssuerOrganization') + ->will($this->returnValue('IssuerOrganization')); + + $this->request + ->expects($this->once()) + ->method('getUploadedFile') + ->with('rootcert_import') + ->will($this->returnValue($uploadedFile)); + $this->certificateManager + ->expects($this->once()) + ->method('addCertificate') + ->with(file_get_contents($uploadedFile['tmp_name'], 'goodCertificate.crt')) + ->will($this->returnValue($certificate)); + + $this->l10n + ->expects($this->at(0)) + ->method('l') + ->with('date', new \DateTime('@1429099555')) + ->will($this->returnValue('Valid From as String')); + $this->l10n + ->expects($this->at(1)) + ->method('l') + ->with('date', new \DateTime('@1529099555')) + ->will($this->returnValue('Valid Till as String')); + + + $expected = new DataResponse([ + 'name' => 'Name', + 'commonName' => 'CommonName', + 'organization' => 'Organization', + 'validFrom' => 1429099555, + 'validTill' => 1529099555, + 'validFromString' => 'Valid From as String', + 'validTillString' => 'Valid Till as String', + 'issuer' => 'Issuer', + 'issuerOrganization' => 'IssuerOrganization', + ]); + $this->assertEquals($expected, $this->certificateController->addPersonalRootCertificate()); + } + + public function testAddPersonalRootCertificateInValidCertificate() { + $uploadedFile = [ + 'tmp_name' => __DIR__ . '/../../data/certificates/badCertificate.crt', + 'name' => 'badCertificate.crt', + ]; + + $this->request + ->expects($this->once()) + ->method('getUploadedFile') + ->with('rootcert_import') + ->will($this->returnValue($uploadedFile)); + $this->certificateManager + ->expects($this->once()) + ->method('addCertificate') + ->with(file_get_contents($uploadedFile['tmp_name'], 'goodCertificate.crt')) + ->will($this->throwException(new \Exception())); + + $expected = new DataResponse('An error occurred.', Http::STATUS_INTERNAL_SERVER_ERROR); + $this->assertEquals($expected, $this->certificateController->addPersonalRootCertificate()); + } + + public function testRemoveCertificate() { + $this->certificateManager + ->expects($this->once()) + ->method('removeCertificate') + ->with('CertificateToRemove'); + + $this->assertEquals(new DataResponse(), $this->certificateController->removePersonalRootCertificate('CertificateToRemove')); + } + +}