Merge pull request #15637 from owncloud/migrate-certificate-stuff

Migrate personal certificate handling to AppFramework controllers

15 files changed, 359 insertions, 132 deletions

diff --git a/apps/files_external/appinfo/routes.php b/apps/files_external/appinfo/routes.php
index 98eb2fcccbb..8c6dff7a901 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/lib/private/security/certificate.php b/lib/private/security/certificate.php
index 468427d0702..0d7fcc4148d 100644
--- a/lib/private/security/certificate.php
+++ b/lib/private/security/certificate.php
@@ -49,18 +49,18 @@ class Certificate implements ICertificate {
 	 */
 	public function __construct($data, $name) {
 		$this->name = $name;
-		try {
-			$gmt = new \DateTimeZone('GMT');
-			$info = openssl_x509_parse($data);
-			$this->commonName = isset($info['subject']['CN']) ? $info['subject']['CN'] : null;
-			$this->organization = isset($info['subject']['O']) ? $info['subject']['O'] : null;
-			$this->issueDate = new \DateTime('@' . $info['validFrom_time_t'], $gmt);
-			$this->expireDate = new \DateTime('@' . $info['validTo_time_t'], $gmt);
-			$this->issuerName = isset($info['issuer']['CN']) ? $info['issuer']['CN'] : null;
-			$this->issuerOrganization = isset($info['issuer']['O']) ? $info['issuer']['O'] : null;
-		} catch (\Exception $e) {
+		$gmt = new \DateTimeZone('GMT');
+		$info = openssl_x509_parse($data);
+		if(!is_array($info)) {
 			throw new \Exception('Certificate could not get parsed.');
 		}
+
+		$this->commonName = isset($info['subject']['CN']) ? $info['subject']['CN'] : null;
+		$this->organization = isset($info['subject']['O']) ? $info['subject']['O'] : null;
+		$this->issueDate = new \DateTime('@' . $info['validFrom_time_t'], $gmt);
+		$this->expireDate = new \DateTime('@' . $info['validTo_time_t'], $gmt);
+		$this->issuerName = isset($info['issuer']['CN']) ? $info['issuer']['CN'] : null;
+		$this->issuerOrganization = isset($info['issuer']['O']) ? $info['issuer']['O'] : null;
 	}
 
 	/**
diff --git a/lib/private/security/certificatemanager.php b/lib/private/security/certificatemanager.php
index beeb87a8316..d61c7f29327 100644
--- a/lib/private/security/certificatemanager.php
+++ b/lib/private/security/certificatemanager.php
@@ -107,12 +107,12 @@ class CertificateManager implements ICertificateManager {
 	 *
 	 * @param string $certificate the certificate data
 	 * @param string $name the filename for the certificate
-	 * @return \OCP\ICertificate|void|bool
+	 * @return \OCP\ICertificate
 	 * @throws \Exception If the certificate could not get added
 	 */
 	public function addCertificate($certificate, $name) {
 		if (!Filesystem::isValidPath($name) or Filesystem::isFileBlacklisted($name)) {
-			return false;
+			throw new \Exception('Filename is not valid');
 		}
 
 		$dir = $this->getPathToCertificates() . 'uploads/';
diff --git a/lib/public/icertificatemanager.php b/lib/public/icertificatemanager.php
index ec88f32e291..3014cd8f633 100644
--- a/lib/public/icertificatemanager.php
+++ b/lib/public/icertificatemanager.php
@@ -38,7 +38,8 @@ interface ICertificateManager {
 	/**
 	 * @param string $certificate the certificate data
 	 * @param string $name the filename for the certificate
-	 * @return bool | \OCP\ICertificate
+	 * @return \OCP\ICertificate
+	 * @throws \Exception If the certificate could not get added
 	 * @since 8.0.0
 	 */
 	public function addCertificate($certificate, $name);
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..d9026cd42af
--- /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_UNPROCESSABLE_ENTITY);
+		}
+	}
+
+	/**
+	 * 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..165b55bcdae 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.validTillString}))
+				.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>
diff --git a/tests/lib/security/certificate.php b/tests/lib/security/certificate.php
index 361f2f8c38d..7fc8bbbdf25 100644
--- a/tests/lib/security/certificate.php
+++ b/tests/lib/security/certificate.php
@@ -1,9 +1,22 @@
 <?php
 /**
- * Copyright (c) 2014 Lukas Reschke <lukas@owncloud.com>
- * This file is licensed under the Affero General Public License version 3 or
- * later.
- * See the COPYING-README file.
+ * @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/>
+ *
  */
 
 use \OC\Security\Certificate;
@@ -32,33 +45,34 @@ class CertificateTest extends \Test\TestCase {
 	 * @expectedException \Exception
 	 * @expectedExceptionMessage Certificate could not get parsed.
 	 */
-	function testBogusData() {
-		new Certificate('foo', 'bar');
+	public function testBogusData() {
+		$certificate = new Certificate('foo', 'bar');
+		$certificate->getIssueDate();
 	}
 
-	function testGetName() {
+	public function testGetName() {
 		$this->assertSame('GoodCertificate', $this->goodCertificate->getName());
 		$this->assertSame('BadCertificate', $this->invalidCertificate->getName());
 	}
 
-	function testGetCommonName() {
+	public function testGetCommonName() {
 		$this->assertSame('security.owncloud.com', $this->goodCertificate->getCommonName());
 		$this->assertSame(null, $this->invalidCertificate->getCommonName());
 	}
 
-	function testGetOrganization() {
+	public function testGetOrganization() {
 		$this->assertSame('ownCloud Inc.', $this->goodCertificate->getOrganization());
 		$this->assertSame('Internet Widgits Pty Ltd', $this->invalidCertificate->getOrganization());
 	}
 
-	function testGetIssueDate() {
+	public function testGetIssueDate() {
 		$expected = new DateTime('2014-08-27 08:45:52 GMT');
 		$this->assertEquals($expected->getTimestamp(), $this->goodCertificate->getIssueDate()->getTimestamp());
 		$expected = new DateTime('2014-08-27 08:48:51 GMT');
 		$this->assertEquals($expected->getTimestamp(), $this->invalidCertificate->getIssueDate()->getTimestamp());
 	}
 
-	function testGetExpireDate() {
+	public function testGetExpireDate() {
 		$expected = new DateTime('2015-08-27 08:45:52 GMT');
 		$this->assertEquals($expected->getTimestamp(), $this->goodCertificate->getExpireDate()->getTimestamp());
 		$expected = new DateTime('2015-08-27 08:48:51 GMT');
@@ -70,19 +84,19 @@ class CertificateTest extends \Test\TestCase {
 	/**
 	 * Obviously the following test case might fail after 2015-08-27, just create a new certificate with longer validity then
 	 */
-	function testIsExpired() {
+	public function testIsExpired() {
 		$this->assertSame(false, $this->goodCertificate->isExpired());
 		$this->assertSame(false, $this->invalidCertificate->isExpired());
 		$this->assertSame(true, $this->expiredCertificate->isExpired());
 	}
 
-	function testGetIssuerName() {
+	public function testGetIssuerName() {
 		$this->assertSame('security.owncloud.com', $this->goodCertificate->getIssuerName());
 		$this->assertSame(null, $this->invalidCertificate->getIssuerName());
 		$this->assertSame(null, $this->expiredCertificate->getIssuerName());
 	}
 
-	function testGetIssuerOrganization() {
+	public function testGetIssuerOrganization() {
 		$this->assertSame('ownCloud Inc.', $this->goodCertificate->getIssuerOrganization());
 		$this->assertSame('Internet Widgits Pty Ltd', $this->invalidCertificate->getIssuerOrganization());
 		$this->assertSame('Internet Widgits Pty Ltd', $this->expiredCertificate->getIssuerOrganization());
diff --git a/tests/lib/security/certificatemanager.php b/tests/lib/security/certificatemanager.php
index 1167fe3d868..fab1c208443 100644
--- a/tests/lib/security/certificatemanager.php
+++ b/tests/lib/security/certificatemanager.php
@@ -14,8 +14,6 @@ class CertificateManagerTest extends \Test\TestCase {
 	private $certificateManager;
 	/** @var String */
 	private $username;
-	/** @var  \OC\User\User */
-	private $user;
 
 	protected function setUp() {
 		parent::setUp();
@@ -67,9 +65,25 @@ class CertificateManagerTest extends \Test\TestCase {
 		$this->certificateManager->addCertificate('InvalidCertificate', 'invalidCertificate');
 	}
 
-	function testAddDangerousFile() {
-		$this->assertFalse($this->certificateManager->addCertificate(file_get_contents(__DIR__.'/../../data/certificates/expiredCertificate.crt'), '.htaccess'));
-		$this->assertFalse($this->certificateManager->addCertificate(file_get_contents(__DIR__.'/../../data/certificates/expiredCertificate.crt'), '../../foo.txt'));
+	/**
+	 * @return array
+	 */
+	public function dangerousFileProvider() {
+		return [
+			['.htaccess'],
+			['../../foo.txt'],
+			['..\..\foo.txt'],
+		];
+	}
+
+	/**
+	 * @expectedException \Exception
+	 * @expectedExceptionMessage Filename is not valid
+	 * @dataProvider dangerousFileProvider
+	 * @param string $filename
+	 */
+	function testAddDangerousFile($filename) {
+		$this->certificateManager->addCertificate(file_get_contents(__DIR__.'/../../data/certificates/expiredCertificate.crt'), $filename);
 	}
 
 	function testRemoveDangerousFile() {
diff --git a/tests/settings/controller/CertificateControllerTest.php b/tests/settings/controller/CertificateControllerTest.php
new file mode 100644
index 00000000000..b6981195034
--- /dev/null
+++ b/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'], 'badCertificate.crt'))
+			->will($this->throwException(new \Exception()));
+
+		$expected = new DataResponse('An error occurred.', Http::STATUS_UNPROCESSABLE_ENTITY);
+		$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'));
+	}
+
+}