always use an LDAP URL when connecting to LDAP

apps/user_ldap/lib/connection.php

@@ -575,10 +575,6 @@ class Connection extends LDAPUtility {
 		if(empty($host)) {
 			return false;
 		}
-		if(strpos($host, '://') !== false) {
-			//ldap_connect ignores port parameter when URLs are passed
-			$host .= ':' . $port;
-		}
 		$this->ldapConnectionRes = $this->ldap->connect($host, $port);
 		if($this->ldap->setOption($this->ldapConnectionRes, LDAP_OPT_PROTOCOL_VERSION, 3)) {
 			if($this->ldap->setOption($this->ldapConnectionRes, LDAP_OPT_REFERRALS, 0)) {

apps/user_ldap/lib/ldap.php

@@ -48,7 +48,14 @@ class LDAP implements ILDAPWrapper {
 	 * @return mixed
 	 */
 	public function connect($host, $port) {
-		return $this->invokeLDAPMethod('connect', $host, $port);
+		if(strpos($host, '://') === false) {
+			$host = 'ldap://' . $host;
+		}
+		if(strpos($host, ':', strpos($host, '://') + 1) === false) {
+			//ldap_connect ignores port parameter when URLs are passed
+			$host .= ':' . $port;
+		}
+		return $this->invokeLDAPMethod('connect', $host);
 	}
 
 	/**

apps/user_ldap/lib/wizard.php

@@ -1035,13 +1035,6 @@ class Wizard extends LDAPUtility {
 		if(!$hostInfo) {
 			throw new \Exception($this->l->t('Invalid Host'));
 		}
-		if(isset($hostInfo['scheme'])) {
-			if(isset($hostInfo['port'])) {
-				//problem
-			} else {
-				$host .= ':' . $port;
-			}
-		}
 		\OCP\Util::writeLog('user_ldap', 'Wiz: Attempting to connect ', \OCP\Util::DEBUG);
 		$cr = $this->ldap->connect($host, $port);
 		if(!is_resource($cr)) {
@@ -1291,12 +1284,10 @@ class Wizard extends LDAPUtility {
 			return $this->cr;
 		}
 
-		$host = $this->configuration->ldapHost;
-		if(strpos($host, '://') !== false) {
-			//ldap_connect ignores port parameter when URLs are passed
-			$host .= ':' . $this->configuration->ldapPort;
-		}
-		$cr = $this->ldap->connect($host, $this->configuration->ldapPort);
+		$cr = $this->ldap->connect(
+			$this->configuration->ldapHost,
+			$this->configuration->ldapPort
+		);
 
 		$this->ldap->setOption($cr, LDAP_OPT_PROTOCOL_VERSION, 3);
 		$this->ldap->setOption($cr, LDAP_OPT_REFERRALS, 0);

apps/user_ldap/tests/integration/lib/integrationtestconnect.php

@@ -0,0 +1,166 @@
+<?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\integration\lib;
+
+use OCA\user_ldap\lib\user\Manager as LDAPUserManager;
+use OCA\user_ldap\tests\integration\AbstractIntegrationTest;
+use OCA\User_LDAP\Mapping\UserMapping;
+use OCA\user_ldap\USER_LDAP;
+
+require_once __DIR__  . '/../../../../../lib/base.php';
+
+class IntegrationConnect extends AbstractIntegrationTest {
+	/** @var  UserMapping */
+	protected $mapping;
+
+	/** @var USER_LDAP */
+	protected $backend;
+
+	/** @var  string */
+	protected $host;
+
+	/** @var  int */
+	protected $port;
+
+	public function __construct($host, $port, $bind, $pwd, $base) {
+		// make sure host is a simple host name
+		if(strpos($host, '://') !== false) {
+			$host = substr_replace($host, '', 0, strpos($host, '://') + 3);
+		}
+		if(strpos($host, ':') !== false) {
+			$host = substr_replace($host, '', strpos($host, ':'));
+		}
+		$this->host = $host;
+		$this->port = $port;
+		parent::__construct($host, $port, $bind, $pwd, $base);
+	}
+
+	/**
+	 * test that a faulty host will does not connect successfully
+	 *
+	 * @return bool
+	 */
+	protected function case1() {
+		// reset possible LDAP connection
+		$this->initConnection();
+		$this->connection->setConfiguration([
+			'ldapHost' => 'qwertz.uiop',
+		]);
+		try {
+			$this->connection->getConnectionResource();
+		} catch (\OC\ServerNotAvailableException $e) {
+			return true;
+		}
+		return false;
+	}
+
+	/**
+	 * tests that a connect succeeds when only a hostname is provided
+	 *
+	 * @return bool
+	 */
+	protected function case2() {
+		// reset possible LDAP connection
+		$this->initConnection();
+		$this->connection->setConfiguration([
+				'ldapHost' => $this->host,
+		]);
+		try {
+			$this->connection->getConnectionResource();
+		} catch (\OC\ServerNotAvailableException $e) {
+			return false;
+		}
+		return true;
+	}
+
+	/**
+	 * tests that a connect succeeds when an LDAP URL is provided
+	 *
+	 * @return bool
+	 */
+	protected function case3() {
+		// reset possible LDAP connection
+		$this->initConnection();
+		$this->connection->setConfiguration([
+				'ldapHost' => 'ldap://' . $this->host,
+		]);
+		try {
+			$this->connection->getConnectionResource();
+		} catch (\OC\ServerNotAvailableException $e) {
+			return false;
+		}
+		return true;
+	}
+
+	/**
+	 * tests that a connect succeeds when an LDAP URL with port is provided
+	 *
+	 * @return bool
+	 */
+	protected function case4() {
+		// reset possible LDAP connection
+		$this->initConnection();
+		$this->connection->setConfiguration([
+				'ldapHost' => 'ldap://' . $this->host  . ':' . $this->port,
+		]);
+		try {
+			$this->connection->getConnectionResource();
+		} catch (\OC\ServerNotAvailableException $e) {
+			return false;
+		}
+		return true;
+	}
+
+	/**
+	 * tests that a connect succeeds when a hostname with port is provided
+	 *
+	 * @return bool
+	 */
+	protected function case5() {
+		// reset possible LDAP connection
+		$this->initConnection();
+		$this->connection->setConfiguration([
+				'ldapHost' => $this->host  . ':' . $this->port,
+		]);
+		try {
+			$this->connection->getConnectionResource();
+		} catch (\OC\ServerNotAvailableException $e) {
+			return false;
+		}
+		return true;
+	}
+
+	/**
+	 * repeat case1, only to make sure that not a connection was reused by
+	 * accident.
+	 *
+	 * @return bool
+	 */
+	protected function case6() {
+		return $this->case1();
+	}
+}
+
+require_once(__DIR__ . '/../setup-scripts/config.php');
+$test = new IntegrationConnect($host, $port, $adn, $apwd, $bdn);
+$test->init();
+$test->run();