trim by default, add unit tests

2 files changed, 140 insertions, 16 deletions

diff --git a/apps/user_ldap/lib/configuration.php b/apps/user_ldap/lib/configuration.php
index fe3740108c2..6e62f5730b9 100644
--- a/apps/user_ldap/lib/configuration.php
+++ b/apps/user_ldap/lib/configuration.php
@@ -145,17 +145,18 @@ class Configuration {
 			}
 
 			$setMethod = 'setValue';
-			$trim = false;
 			switch($key) {
+				case 'ldapAgentPassword':
+					$setMethod = 'setRawValue';
+					break;
 				case 'homeFolderNamingRule':
-					if(!empty($val) && strpos($val, 'attr:') === false) {
-						$val = 'attr:'.$val;
+					if(!empty(trim($val)) && strpos($val, 'attr:') === false) {
+						$val = 'attr:'.trim($val);
 					}
 					break;
 				case 'ldapBase':
 				case 'ldapBaseUsers':
 				case 'ldapBaseGroups':
-					$trim = true;// Prevent login errors due to whitespace
 				case 'ldapAttributesForUserSearch':
 				case 'ldapAttributesForGroupSearch':
 				case 'ldapUserFilterObjectclass':
@@ -166,7 +167,7 @@ class Configuration {
 					$setMethod = 'setMultiLine';
 					break;
 			}
-			$this->$setMethod($key, $val, $trim);
+			$this->$setMethod($key, $val);
 			if(is_array($applied)) {
 				$applied[] = $inputKey;
 			}
@@ -280,7 +281,7 @@ class Configuration {
 	 * @param array|string $value to set
 	 * @param boolean $trim Trim value? (default: false)
 	 */
-	protected function setMultiLine($varName, $value, $trim = false) {
+	protected function setMultiLine($varName, $value) {
 		if(empty($value)) {
 			$value = '';
 		} else if (!is_array($value)) {
@@ -290,17 +291,25 @@ class Configuration {
 			}
 		}
 
-		if($trim) {
-			if(!is_array($value)) {
-				$value = trim($value);
-			} else {
-				foreach($value as $key => $val) {
-					$value[$key] = trim($val);
+		if(!is_array($value)) {
+			$finalValue = trim($value);
+		} else {
+			$finalValue = [];
+			foreach($value as $key => $val) {
+				if(is_string($val)) {
+					$val = trim($val);
+					if(!empty($val)) {
+						//accidental line breaks are not wanted and can cause
+						// odd behaviour. Thus, away with them.
+						$finalValue[] = $val;
+					}
+				} else {
+					$finalValue[] = $val;
 				}
 			}
 		}
 
-		$this->setValue($varName, $value);
+		$this->setRawValue($varName, $finalValue);
 	}
 
 	/**
@@ -347,16 +356,25 @@ class Configuration {
 	 * 
 	 * @param string $varName name of config key
 	 * @param mixed $value to set
-	 * @param boolean $trim Trim value? (default: false)
 	 */
-	protected function setValue($varName, $value, $trim = false) {
-		if($trim && is_string($value)) {
+	protected function setValue($varName, $value) {
+		if(is_string($value)) {
 			$value = trim($value);
 		}
 		$this->config[$varName] = $value;
 	}
 
 	/**
+	 * Sets a scalar value without trimming.
+	 *
+	 * @param string $varName name of config key
+	 * @param mixed $value to set
+	 */
+	protected function setRawValue($varName, $value) {
+		$this->config[$varName] = $value;
+	}
+
+	/**
 	 * @param string $varName
 	 * @param string $value
 	 * @return bool
diff --git a/apps/user_ldap/tests/configuration.php b/apps/user_ldap/tests/configuration.php
new file mode 100644
index 00000000000..efe4d7eec83
--- /dev/null
+++ b/apps/user_ldap/tests/configuration.php
@@ -0,0 +1,106 @@
+<?php
+/**
+ * @author Arthur Schiwon <blizzz@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 OCA\user_ldap\tests;
+
+class Test_Configuration extends \Test\TestCase {
+
+	public function configurationDataProvider() {
+		$inputWithDN = array(
+			'cn=someUsers,dc=example,dc=org',
+			'  ',
+			' cn=moreUsers,dc=example,dc=org '
+		);
+		$expectWithDN = array(
+			'cn=someUsers,dc=example,dc=org',
+			'cn=moreUsers,dc=example,dc=org'
+		);
+
+		$inputNames = array(
+			'  uid  ',
+			'cn ',
+			' ',
+			'',
+			' whats my name',
+		    '	'
+		);
+		$expectedNames = array('uid', 'cn', 'whats my name');
+
+		$inputString = ' alea iacta est ';
+		$expectedString = 'alea iacta est';
+
+		$inputHomeFolder = array(
+			' homeDirectory ',
+			' attr:homeDirectory ',
+			' '
+		);
+
+		$expectedHomeFolder = array(
+			'attr:homeDirectory', 'attr:homeDirectory', ''
+		);
+
+		$password = ' such a passw0rd ';
+
+		return array(
+			'set general base' => array('ldapBase', $inputWithDN, $expectWithDN),
+			'set user base'    => array('ldapBaseUsers', $inputWithDN, $expectWithDN),
+			'set group base'   => array('ldapBaseGroups', $inputWithDN, $expectWithDN),
+
+			'set search attributes users'  => array('ldapAttributesForUserSearch', $inputNames, $expectedNames),
+			'set search attributes groups' => array('ldapAttributesForGroupSearch', $inputNames, $expectedNames),
+
+			'set user filter objectclasses'  => array('ldapUserFilterObjectclass', $inputNames, $expectedNames),
+			'set user filter groups'         => array('ldapUserFilterGroups', $inputNames, $expectedNames),
+			'set group filter objectclasses' => array('ldapGroupFilterObjectclass', $inputNames, $expectedNames),
+			'set group filter groups'        => array('ldapGroupFilterGroups', $inputNames, $expectedNames),
+			'set login filter attributes'    => array('ldapLoginFilterAttributes', $inputNames, $expectedNames),
+
+			'set agent password' => array('ldapAgentPassword', $password, $password),
+
+			'set home folder, variant 1' => array('homeFolderNamingRule', $inputHomeFolder[0], $expectedHomeFolder[0]),
+			'set home folder, variant 2' => array('homeFolderNamingRule', $inputHomeFolder[1], $expectedHomeFolder[1]),
+			'set home folder, empty'     => array('homeFolderNamingRule', $inputHomeFolder[2], $expectedHomeFolder[2]),
+
+			// default behaviour, one case is enough, special needs must be tested
+			// individually
+			'set string value' => array('ldapHost', $inputString, $expectedString),
+		);
+	}
+
+	/**
+	 * @dataProvider configurationDataProvider
+	 */
+	public function testSetValue($key, $input, $expected) {
+		$configuration = new \OCA\user_ldap\lib\Configuration('t01', false);
+
+		$settingsInput = array(
+			'ldapBaseUsers' => array(
+				'cn=someUsers,dc=example,dc=org',
+				'  ',
+				' cn=moreUsers,dc=example,dc=org '
+			)
+		);
+
+		$configuration->setConfiguration([$key => $input]);
+		$this->assertSame($configuration->$key, $expected);
+	}
+
+}