From 6044ad0e174a0d3c9db174115df9c8f61fd43dc3 Mon Sep 17 00:00:00 2001
From: Robin Appelman <icewind@owncloud.com>
Date: Mon, 18 Aug 2014 13:57:38 +0200
Subject: [PATCH] Cleanup certificate code

---
 lib/private/certificate/certificate.php       |  6 ++++-
 .../certificate/certificatemanager.php        | 26 ++++++++++++-------
 settings/ajax/addRootCertificate.php          | 13 +++++-----
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/lib/private/certificate/certificate.php b/lib/private/certificate/certificate.php
index 801afa7916..6b4021cf5e 100644
--- a/lib/private/certificate/certificate.php
+++ b/lib/private/certificate/certificate.php
@@ -27,6 +27,10 @@ class Certificate implements ICertificate {
 
 	protected $issuerOrganization;
 
+	/**
+	 * @param string $data base64 encoded certificate
+	 * @param string $name
+	 */
 	public function __construct($data, $name) {
 		$this->name = $name;
 		$info = openssl_x509_parse($data);
@@ -97,7 +101,7 @@ class Certificate implements ICertificate {
 	 */
 	public function isExpired() {
 		$now = new \DateTime();
-		return !($this->issueDate <= $now and $now <= $this->expireDate);
+		return $this->issueDate > $now or $now > $this->expireDate;
 	}
 
 	/**
diff --git a/lib/private/certificate/certificatemanager.php b/lib/private/certificate/certificatemanager.php
index c6207f057d..d7180f7f3f 100644
--- a/lib/private/certificate/certificatemanager.php
+++ b/lib/private/certificate/certificatemanager.php
@@ -8,6 +8,7 @@
 
 namespace OC\Certificate;
 
+use OC\Files\Filesystem;
 use OCP\ICertificateManager;
 
 /**
@@ -34,10 +35,7 @@ class CertificateManager implements ICertificateManager {
 	public function listCertificates() {
 		$path = $this->user->getHome() . '/files_external/uploads/';
 		if (!is_dir($path)) {
-			//path might not exist (e.g. non-standard OC_User::getHome() value)
-			//in this case create full path using 3rd (recursive=true) parameter.
-			//note that we use "normal" php filesystem functions here since the certs need to be local
-			mkdir($path, 0700, true);
+			return array();
 		}
 		$result = array();
 		$handle = opendir($path);
@@ -62,9 +60,7 @@ class CertificateManager implements ICertificateManager {
 		$fh_certs = fopen($path . '/rootcerts.crt', 'w');
 		foreach ($certs as $cert) {
 			$file = $path . '/uploads/' . $cert;
-			$fh = fopen($file, 'r');
-			$data = fread($fh, filesize($file));
-			fclose($fh);
+			$data = file_get_contents($file);
 			if (strpos($data, 'BEGIN CERTIFICATE')) {
 				fwrite($fh_certs, $data);
 				fwrite($fh_certs, "\r\n");
@@ -75,6 +71,8 @@ class CertificateManager implements ICertificateManager {
 	}
 
 	/**
+	 * Save the certificate and re-generate the certificate bundle
+	 *
 	 * @param string $certificate the certificate data
 	 * @param string $name the filename for the certificate
 	 * @return bool | \OCP\ICertificate
@@ -92,7 +90,14 @@ class CertificateManager implements ICertificateManager {
 		}
 
 		if ($isValid) {
-			$file = $this->user->getHome() . '/files_external/uploads/' . $name;
+			$dir = $this->user->getHome() . '/files_external/uploads/';
+			if (!file_exists($dir)) {
+				//path might not exist (e.g. non-standard OC_User::getHome() value)
+				//in this case create full path using 3rd (recursive=true) parameter.
+				//note that we use "normal" php filesystem functions here since the certs need to be local
+				mkdir($dir, 0700, true);
+			}
+			$file = $dir . $name;
 			file_put_contents($file, $certificate);
 			$this->createCertificateBundle();
 			return new Certificate($certificate, $name);
@@ -102,11 +107,13 @@ class CertificateManager implements ICertificateManager {
 	}
 
 	/**
+	 * Remove the certificate and re-generate the certificate bundle
+	 *
 	 * @param string $name
 	 * @return bool
 	 */
 	public function removeCertificate($name) {
-		if (!\OC\Files\Filesystem::isValidPath($name)) {
+		if (!Filesystem::isValidPath($name)) {
 			return false;
 		}
 		$path = $this->user->getHome() . '/files_external/uploads/';
@@ -114,6 +121,7 @@ class CertificateManager implements ICertificateManager {
 			unlink($path . $name);
 			$this->createCertificateBundle();
 		}
+		return true;
 	}
 
 	/**
diff --git a/settings/ajax/addRootCertificate.php b/settings/ajax/addRootCertificate.php
index f055a4066e..87b1460ef1 100644
--- a/settings/ajax/addRootCertificate.php
+++ b/settings/ajax/addRootCertificate.php
@@ -3,19 +3,18 @@ OCP\JSON::callCheck();
 
 $l = new OC_L10N('core');
 
-if (!($filename = $_FILES['rootcert_import']['name'])) {
-	header('Location:' . OCP\Util::linkToRoute("settings_personal"));
+if (!isset($_FILES['rootcert_import'])) {
+	OCP\JSON::error(array('error' => 'No certificate uploaded'));
 	exit;
 }
 
-$fh = fopen($_FILES['rootcert_import']['tmp_name'], 'r');
-$data = fread($fh, filesize($_FILES['rootcert_import']['tmp_name']));
-fclose($fh);
-$filename = $_FILES['rootcert_import']['name'];
+$data = file_get_contents($_FILES['rootcert_import']['tmp_name']);
+$filename = basename($_FILES['rootcert_import']['name']);
 
 $certificateManager = \OC::$server->getCertificateManager();
 
-if ($cert = $certificateManager->addCertificate($data, $filename)) {
+$cert = $certificateManager->addCertificate($data, $filename);
+if ($cert) {
 	OCP\JSON::success(array(
 		'name' => $cert->getName(),
 		'commonName' => $cert->getCommonName(),