From e73ccbd4cade0622615ee133496a571ac1d6dba7 Mon Sep 17 00:00:00 2001
From: Lukas Reschke <lukas@owncloud.com>
Date: Mon, 3 Nov 2014 10:55:52 +0100
Subject: [PATCH] Migrate "setsecurity.php" to the AppFramework

Add switch to enforce SSL for subdomains

Add unit tests

Add test for boolean values

Camel-case

Fix ugly JS
---
 config/config.sample.php                      |   6 +
 lib/base.php                                  |  15 +-
 settings/admin.php                            |   3 +-
 settings/ajax/setsecurity.php                 |  21 ---
 settings/application.php                      |   9 ++
 .../controller/securitysettingscontroller.php |  95 ++++++++++++
 settings/js/admin.js                          |  30 +++-
 settings/routes.php                           |   8 +-
 settings/templates/admin.php                  |  22 ++-
 .../securitysettingscontrollertest.php        | 138 ++++++++++++++++++
 10 files changed, 311 insertions(+), 36 deletions(-)
 delete mode 100644 settings/ajax/setsecurity.php
 create mode 100644 settings/controller/securitysettingscontroller.php
 create mode 100644 tests/settings/controller/securitysettingscontrollertest.php

diff --git a/config/config.sample.php b/config/config.sample.php
index a53521485e..1f1447e22e 100644
--- a/config/config.sample.php
+++ b/config/config.sample.php
@@ -663,6 +663,12 @@ $CONFIG = array(
  */
 'forcessl' => false,
 
+/**
+ * Change this to ``true`` to require HTTPS connections also for all subdomains.
+ * Works only together when `forcessl` is set to true.
+ */
+'forceSSLforSubdomains' => false,
+
 /**
  * Extra SSL options to be used for configuration.
  */
diff --git a/lib/base.php b/lib/base.php
index d428d45d90..78ab9580b2 100644
--- a/lib/base.php
+++ b/lib/base.php
@@ -229,11 +229,18 @@ class OC {
 
 	public static function checkSSL() {
 		// redirect to https site if configured
-		if (OC_Config::getValue("forcessl", false)) {
-			header('Strict-Transport-Security: max-age=31536000');
-			ini_set("session.cookie_secure", "on");
+		if (\OC::$server->getConfig()->getSystemValue('forcessl', false)) {
+			// Default HSTS policy
+			$header = 'Strict-Transport-Security: max-age=31536000';
+
+			// If SSL for subdomains is enabled add "; includeSubDomains" to the header
+			if(\OC::$server->getConfig()->getSystemValue('forceSSLforSubdomains', false)) {
+				$header .= '; includeSubDomains';
+			}
+			header($header);
+			ini_set('session.cookie_secure', 'on');
 			if (OC_Request::serverProtocol() <> 'https' and !OC::$CLI) {
-				$url = "https://" . OC_Request::serverHost() . OC_Request::requestUri();
+				$url = 'https://' . OC_Request::serverHost() . OC_Request::requestUri();
 				header("Location: $url");
 				exit();
 			}
diff --git a/settings/admin.php b/settings/admin.php
index d58b9a597b..1c1cbd9975 100644
--- a/settings/admin.php
+++ b/settings/admin.php
@@ -53,7 +53,8 @@ $template->assign('shareExcludedGroupsList', implode('|', $excludedGroupsList));
 
 // Check if connected using HTTPS
 $template->assign('isConnectedViaHTTPS', OC_Request::serverProtocol() === 'https');
-$template->assign('enforceHTTPSEnabled', $config->getSystemValue("forcessl", false));
+$template->assign('enforceHTTPSEnabled', $config->getSystemValue('forcessl', false));
+$template->assign('forceSSLforSubdomainsEnabled', $config->getSystemValue('forceSSLforSubdomains', false));
 
 // If the current web root is non-empty but the web root from the config is,
 // and system cron is used, the URL generator fails to build valid URLs.
diff --git a/settings/ajax/setsecurity.php b/settings/ajax/setsecurity.php
deleted file mode 100644
index f1f737a494..0000000000
--- a/settings/ajax/setsecurity.php
+++ /dev/null
@@ -1,21 +0,0 @@
-<?php
-/**
- * Copyright (c) 2013-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.
- */
-
-OC_Util::checkAdminUser();
-OCP\JSON::callCheck();
-
-if(isset($_POST['enforceHTTPS'])) {
-	 \OC::$server->getConfig()->setSystemValue('forcessl', filter_var($_POST['enforceHTTPS'], FILTER_VALIDATE_BOOLEAN));
-}
-
-if(isset($_POST['trustedDomain'])) {
-	$trustedDomains = \OC::$server->getConfig()->getSystemValue('trusted_domains');
-	$trustedDomains[] = $_POST['trustedDomain'];
-	\OC::$server->getConfig()->setSystemValue('trusted_domains', $trustedDomains);
-}
-
-echo 'true';
diff --git a/settings/application.php b/settings/application.php
index 99d78aff2c..64aa467122 100644
--- a/settings/application.php
+++ b/settings/application.php
@@ -13,6 +13,7 @@ namespace OC\Settings;
 use OC\AppFramework\Utility\SimpleContainer;
 use OC\Settings\Controller\AppSettingsController;
 use OC\Settings\Controller\MailSettingsController;
+use OC\Settings\Controller\SecuritySettingsController;
 use \OCP\AppFramework\App;
 use \OCP\Util;
 
@@ -53,6 +54,14 @@ class Application extends App {
 				$c->query('Config')
 			);
 		});
+		$container->registerService('SecuritySettingsController', function(SimpleContainer $c) {
+			return new SecuritySettingsController(
+				$c->query('AppName'),
+				$c->query('Request'),
+				$c->query('Config')
+			);
+		});
+
 		/**
 		 * Core class wrappers
 		 */
diff --git a/settings/controller/securitysettingscontroller.php b/settings/controller/securitysettingscontroller.php
new file mode 100644
index 0000000000..af60df8dc3
--- /dev/null
+++ b/settings/controller/securitysettingscontroller.php
@@ -0,0 +1,95 @@
+<?php
+/**
+ * @author Lukas Reschke
+ * @copyright 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.
+ */
+
+namespace OC\Settings\Controller;
+
+use \OCP\AppFramework\Controller;
+use OCP\IRequest;
+use OCP\IConfig;
+
+/**
+ * @package OC\Settings\Controller
+ */
+class SecuritySettingsController extends Controller {
+	/** @var \OCP\IConfig */
+	private $config;
+
+	/**
+	 * @param string $appName
+	 * @param IRequest $request
+	 * @param IConfig $config
+	 */
+	public function __construct($appName,
+								IRequest $request,
+								IConfig $config) {
+		parent::__construct($appName, $request);
+		$this->config = $config;
+	}
+
+	/**
+	 * @return array
+	 */
+	protected function returnSuccess() {
+		return array(
+			'status' => 'success'
+		);
+	}
+
+	/**
+	 * @return array
+	 */
+	protected function returnError() {
+		return array(
+			'status' => 'error'
+		);
+	}
+
+	/**
+	 * Enforce or disable the enforcement of SSL
+	 * @param boolean $enforceHTTPS Whether SSL should be enforced
+	 * @return array
+	 */
+	public function enforceSSL($enforceHTTPS = false) {
+		if(!is_bool($enforceHTTPS)) {
+			return $this->returnError();
+		}
+		$this->config->setSystemValue('forcessl', $enforceHTTPS);
+
+		return $this->returnSuccess();
+	}
+
+	/**
+	 * Enforce or disable the enforcement for SSL on subdomains
+	 * @param bool $forceSSLforSubdomains Whether SSL on subdomains should be enforced
+	 * @return array
+	 */
+	public function enforceSSLForSubdomains($forceSSLforSubdomains = false) {
+		if(!is_bool($forceSSLforSubdomains)) {
+			return $this->returnError();
+		}
+		$this->config->setSystemValue('forceSSLforSubdomains', $forceSSLforSubdomains);
+
+		return $this->returnSuccess();
+	}
+
+	/**
+	 * Add a new trusted domain
+	 * @param string $newTrustedDomain The newly to add trusted domain
+	 * @return array
+	 */
+	public function trustedDomains($newTrustedDomain) {
+		$trustedDomains = $this->config->getSystemValue('trusted_domains');
+		$trustedDomains[] = $newTrustedDomain;
+		$this->config->setSystemValue('trusted_domains', $trustedDomains);
+
+		return $this->returnSuccess();
+	}
+
+}
diff --git a/settings/js/admin.js b/settings/js/admin.js
index e3a092f71b..059e48ebab 100644
--- a/settings/js/admin.js
+++ b/settings/js/admin.js
@@ -9,8 +9,8 @@ $(document).ready(function(){
 				if(answer) {
 					$.ajax({
 						type: 'POST',
-						url: OC.generateUrl('settings/ajax/setsecurity.php'),
-						data: { trustedDomain: params.trustDomain }
+						url: OC.generateUrl('settings/admin/security/trustedDomains'),
+						data: { newTrustedDomain: params.trustDomain }
 					}).done(function() {
 						window.location.replace(OC.generateUrl('settings/admin'));
 					});
@@ -73,10 +73,32 @@ $(document).ready(function(){
 		$('#setDefaultExpireDate').toggleClass('hidden', !(this.checked && $('#shareapiDefaultExpireDate')[0].checked));
 	});
 
-	$('#security').change(function(){
-		$.post(OC.filePath('settings','ajax','setsecurity.php'), { enforceHTTPS: $('#forcessl').val() },function(){} );
+	$('#forcessl').change(function(){
+		$(this).val(($(this).val() !== 'true'));
+		var forceSSLForSubdomain = $('#forceSSLforSubdomainsSpan');
+
+		$.post(OC.generateUrl('settings/admin/security/ssl'), {
+			enforceHTTPS: $(this).val()
+		},function(){} );
+
+		if($(this).val() === 'true') {
+			forceSSLForSubdomain.prop('disabled', false);
+			forceSSLForSubdomain.removeClass('hidden');
+		} else {
+			forceSSLForSubdomain.prop('disabled', true);
+			forceSSLForSubdomain.addClass('hidden');
+		}
 	});
 
+	$('#forceSSLforSubdomains').change(function(){
+		$(this).val(($(this).val() !== 'true'));
+
+		$.post(OC.generateUrl('settings/admin/security/ssl/subdomains'), {
+			forceSSLforSubdomains: $(this).val()
+		},function(){} );
+	});
+
+
 	$('#mail_smtpauth').change(function() {
 		if (!this.checked) {
 			$('#mail_credentials').addClass('hidden');
diff --git a/settings/routes.php b/settings/routes.php
index 82167ea639..7ca33fc274 100644
--- a/settings/routes.php
+++ b/settings/routes.php
@@ -14,7 +14,11 @@ $application->registerRoutes($this, array('routes' =>array(
 	array('name' => 'MailSettings#storeCredentials', 'url' => '/settings/admin/mailsettings/credentials', 'verb' => 'POST'),
 	array('name' => 'MailSettings#sendTestMail', 'url' => '/settings/admin/mailtest', 'verb' => 'POST'),
 	array('name' => 'AppSettings#listCategories', 'url' => '/settings/apps/categories', 'verb' => 'GET'),
-	array('name' => 'AppSettings#listApps', 'url' => '/settings/apps/list', 'verb' => 'GET')
+	array('name' => 'AppSettings#listApps', 'url' => '/settings/apps/list', 'verb' => 'GET'),
+	array('name' => 'SecuritySettings#enforceSSL', 'url' => '/settings/admin/security/ssl', 'verb' => 'POST'),
+	array('name' => 'SecuritySettings#enforceSSLForSubdomains', 'url' => '/settings/admin/security/ssl/subdomains', 'verb' => 'POST'),
+	array('name' => 'SecuritySettings#trustedDomains', 'url' => '/settings/admin/security/trustedDomains', 'verb' => 'POST'),
+
 )));
 
 /** @var $this \OCP\Route\IRouter */
@@ -95,8 +99,6 @@ $this->create('settings_ajax_getlog', '/settings/ajax/getlog.php')
 	->actionInclude('settings/ajax/getlog.php');
 $this->create('settings_ajax_setloglevel', '/settings/ajax/setloglevel.php')
 	->actionInclude('settings/ajax/setloglevel.php');
-$this->create('settings_ajax_setsecurity', '/settings/ajax/setsecurity.php')
-	->actionInclude('settings/ajax/setsecurity.php');
 $this->create('settings_ajax_excludegroups', '/settings/ajax/excludegroups.php')
 	->actionInclude('settings/ajax/excludegroups.php');
 $this->create('settings_ajax_checksetup', '/settings/ajax/checksetup')
diff --git a/settings/templates/admin.php b/settings/templates/admin.php
index bcf57e12a8..206d2ada03 100644
--- a/settings/templates/admin.php
+++ b/settings/templates/admin.php
@@ -336,9 +336,9 @@ if ($_['suggestedOverwriteWebroot']) {
 		<input type="checkbox" name="forcessl"  id="forcessl"
 			<?php if ($_['enforceHTTPSEnabled']) {
 				print_unescaped('checked="checked" ');
-				print_unescaped('value="false"');
-			}  else {
 				print_unescaped('value="true"');
+			}  else {
+				print_unescaped('value="false"');
 			}
 			?>
 			<?php if (!$_['isConnectedViaHTTPS']) p('disabled'); ?> />
@@ -346,7 +346,23 @@ if ($_['suggestedOverwriteWebroot']) {
 		<em><?php p($l->t(
 			'Forces the clients to connect to %s via an encrypted connection.',
 			$theme->getName()
-		)); ?></em>
+		)); ?></em><br/>
+		<span id="forceSSLforSubdomainsSpan" <?php if(!$_['enforceHTTPSEnabled']) { print_unescaped('class="hidden"'); } ?>>
+			<input type="checkbox" name="forceSSLforSubdomains"  id="forceSSLforSubdomains"
+				<?php if ($_['forceSSLforSubdomainsEnabled']) {
+					print_unescaped('checked="checked" ');
+					print_unescaped('value="true"');
+				}  else {
+					print_unescaped('value="false"');
+				}
+				?>
+				<?php if (!$_['isConnectedViaHTTPS']) { p('disabled'); } ?> />
+			<label for="forceSSLforSubdomains"><?php p($l->t('Enforce HTTPS for subdomains'));?></label><br/>
+			<em><?php p($l->t(
+					'Forces the clients to connect to %s and subdomains via an encrypted connection.',
+					$theme->getName()
+				)); ?></em>
+		</span>
 		<?php if (!$_['isConnectedViaHTTPS']) {
 			print_unescaped("<br/><em>");
 			p($l->t(
diff --git a/tests/settings/controller/securitysettingscontrollertest.php b/tests/settings/controller/securitysettingscontrollertest.php
new file mode 100644
index 0000000000..d89e493236
--- /dev/null
+++ b/tests/settings/controller/securitysettingscontrollertest.php
@@ -0,0 +1,138 @@
+<?php
+/**
+ * @author Lukas Reschke
+ * @copyright 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.
+ */
+namespace OC\Settings\Controller;
+
+use \OC\Settings\Application;
+
+/**
+ * @package OC\Settings\Controller
+ */
+class SecuritySettingsControllerTest extends \PHPUnit_Framework_TestCase {
+
+	/** @var \OCP\AppFramework\IAppContainer */
+	private $container;
+
+	/** @var SecuritySettingsController */
+	private $securitySettingsController;
+
+	protected function setUp() {
+		$app = new Application();
+		$this->container = $app->getContainer();
+		$this->container['Config'] = $this->getMockBuilder('\OCP\IConfig')
+			->disableOriginalConstructor()->getMock();
+		$this->container['AppName'] = 'settings';
+		$this->securitySettingsController = $this->container['SecuritySettingsController'];
+	}
+
+
+	public function testEnforceSSLEmpty() {
+		$this->container['Config']
+			->expects($this->once())
+			->method('setSystemValue')
+			->with('forcessl', false);
+
+		$response = $this->securitySettingsController->enforceSSL();
+		$expectedResponse = array('status' => 'success');
+
+		$this->assertSame($expectedResponse, $response);
+	}
+
+	public function testEnforceSSL() {
+		$this->container['Config']
+			->expects($this->once())
+			->method('setSystemValue')
+			->with('forcessl', true);
+
+		$response = $this->securitySettingsController->enforceSSL(true);
+		$expectedResponse = array('status' => 'success');
+
+		$this->assertSame($expectedResponse, $response);
+	}
+
+	public function testEnforceSSLInvalid() {
+		$this->container['Config']
+			->expects($this->exactly(0))
+			->method('setSystemValue');
+
+		$response = $this->securitySettingsController->enforceSSL('blah');
+		$expectedResponse = array('status' => 'error');
+
+		$this->assertSame($expectedResponse, $response);
+	}
+
+	public function testEnforceSSLForSubdomainsEmpty() {
+		$this->container['Config']
+			->expects($this->once())
+			->method('setSystemValue')
+			->with('forceSSLforSubdomains', false);
+
+		$response = $this->securitySettingsController->enforceSSLForSubdomains();
+		$expectedResponse = array('status' => 'success');
+
+		$this->assertSame($expectedResponse, $response);
+	}
+
+	public function testEnforceSSLForSubdomains() {
+		$this->container['Config']
+			->expects($this->once())
+			->method('setSystemValue')
+			->with('forceSSLforSubdomains', true);
+
+		$response = $this->securitySettingsController->enforceSSLForSubdomains(true);
+		$expectedResponse = array('status' => 'success');
+
+		$this->assertSame($expectedResponse, $response);
+	}
+
+	public function testEnforceSSLForSubdomainsInvalid() {
+		$this->container['Config']
+			->expects($this->exactly(0))
+			->method('setSystemValue');
+
+		$response = $this->securitySettingsController->enforceSSLForSubdomains('blah');
+		$expectedResponse = array('status' => 'error');
+
+		$this->assertSame($expectedResponse, $response);
+	}
+
+	public function testTrustedDomainsWithExistingValues() {
+		$this->container['Config']
+			->expects($this->once())
+			->method('setSystemValue')
+			->with('trusted_domains', array('owncloud.org', 'owncloud.com', 'newdomain.com'));
+		$this->container['Config']
+			->expects($this->once())
+			->method('getSystemValue')
+			->with('trusted_domains')
+			->will($this->returnValue(array('owncloud.org', 'owncloud.com')));
+
+		$response = $this->securitySettingsController->trustedDomains('newdomain.com');
+		$expectedResponse = array('status' => 'success');
+
+		$this->assertSame($expectedResponse, $response);
+	}
+
+	public function testTrustedDomainsEmpty() {
+		$this->container['Config']
+			->expects($this->once())
+			->method('setSystemValue')
+			->with('trusted_domains', array('newdomain.com'));
+		$this->container['Config']
+			->expects($this->once())
+			->method('getSystemValue')
+			->with('trusted_domains')
+			->will($this->returnValue(''));
+
+		$response = $this->securitySettingsController->trustedDomains('newdomain.com');
+		$expectedResponse = array('status' => 'success');
+
+		$this->assertSame($expectedResponse, $response);
+	}
+}