Migrate personal certificate handling into AppFramework controllers

Also added unit-tests and better error-handling

8 files changed, 124 insertions, 94 deletions

diff --git a/settings/ajax/addRootCertificate.php b/settings/ajax/addRootCertificate.php
deleted file mode 100644
index 64a55eaede9..00000000000
--- a/settings/ajax/addRootCertificate.php
+++ /dev/null
@@ -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'));
-}
diff --git a/settings/ajax/removeRootCertificate.php b/settings/ajax/removeRootCertificate.php
deleted file mode 100644
index 4ef5fe32aed..00000000000
--- a/settings/ajax/removeRootCertificate.php
+++ /dev/null
@@ -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);
diff --git a/settings/application.php b/settings/application.php
index 59fe9f6b65a..920d172c93d 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 00000000000..c8d05a51240
--- /dev/null
+++ b/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();
+	}
+
+}
diff --git a/settings/js/personal.js b/settings/js/personal.js
index 687b02399a7..c251d1572b2 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 = $('<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.'));
 		}
 	});
 
diff --git a/settings/personal.php b/settings/personal.php
index 12b320ac001..7bf1110c03e 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 462b4ab543f..52b320cbdb5 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 dfdc6191805..02ee261cd1d 100644
--- a/settings/templates/personal.php
+++ b/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>