Migrate personal certificate handling into AppFramework controllers

Also added unit-tests and better error-handling

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')

settings/ajax/addRootCertificate.php

@@ -1,52 +0,0 @@
-<?php
-/**
- * @author Lukas Reschke <lukas@owncloud.com>
- * @author Robin Appelman <icewind@owncloud.com>
- *
- * @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 <http://www.gnu.org/licenses/>
- *
- */
-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'));
-}

settings/ajax/removeRootCertificate.php

@@ -1,28 +0,0 @@
-<?php
-/**
- * @author Björn Schießle <schiessle@owncloud.com>
- * @author Lukas Reschke <lukas@owncloud.com>
- * @author Robin Appelman <icewind@owncloud.com>
- *
- * @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 <http://www.gnu.org/licenses/>
- *
- */
-OCP\JSON::checkLoggedIn();
-OCP\JSON::callCheck();
-
-$name = (string)$_POST['cert'];
-$certificateManager = \OC::$server->getCertificateManager();
-$certificateManager->removeCertificate($name);

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();
+		});
 	}
 }

settings/controller/certificatecontroller.php

@@ -0,0 +1,93 @@
+<?php
+/**
+ * @author Lukas Reschke <lukas@owncloud.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>
+ *
+ */
+
+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();
+	}
+
+}

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 = $('<tr/>');
+			row.data('name', data.name);
 			row.addClass(isExpired? 'expired': 'valid');
-			row.append($('<td/>').attr('title', data.result.organization).text(data.result.commonName));
-			row.append($('<td/>').attr('title', t('core,', 'Valid until {date}', {date: data.result.validFromString}))
-				.text(data.result.validTillString));
-			row.append($('<td/>').attr('title', data.result.issuerOrganization).text(data.result.issuer));
+			row.append($('<td/>').attr('title', data.organization).text(data.commonName));
+			row.append($('<td/>').attr('title', t('core,', 'Valid until {date}', {date: data.validFromString}))
+				.text(data.validTillString));
+			row.append($('<td/>').attr('title', data.issuerOrganization).text(data.issuer));
 			row.append($('<td/>').addClass('remove').append(
 				$('<img/>').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.'));
 		}
 	});
 

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());

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');

settings/templates/personal.php

@@ -5,6 +5,7 @@
  */
 
 /** @var $_ array */
+/** @var $_['urlGenerator'] */
 ?>
 
 <div id="app-navigation">
@@ -236,7 +237,7 @@ if($_['passwordChangeSupported']) {
 			<?php endforeach; ?>
 		</tbody>
 	</table>
-	<form class="uploadButton" method="post" action="<?php p(\OC_Helper::linkToRoute('settings_cert_post')); ?>" target="certUploadFrame">
+	<form class="uploadButton" method="post" action="<?php p($_['urlGenerator']->linkToRoute('settings.Certificate.addPersonalRootCertificate')); ?>" target="certUploadFrame">
 		<input type="file" id="rootcert_import" name="rootcert_import" class="hidden">
 		<input type="button" id="rootcert_import_button" value="<?php p($l->t('Import root certificate')); ?>"/>
 	</form>

tests/settings/controller/CertificateControllerTest.php

@@ -0,0 +1,174 @@
+<?php
+/**
+ * @author Lukas Reschke <lukas@owncloud.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>
+ *
+ */
+
+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'));
+	}
+
+}