From a32b002cff4ace48a06fe7f45be85cb64862fbbe Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 28 Jul 2015 12:14:05 +0200 Subject: [PATCH] always use an LDAP URL when connecting to LDAP --- apps/user_ldap/lib/connection.php | 4 - apps/user_ldap/lib/ldap.php | 9 +- apps/user_ldap/lib/wizard.php | 17 +- .../lib/integrationtestconnect.php | 166 ++++++++++++++++++ 4 files changed, 178 insertions(+), 18 deletions(-) create mode 100644 apps/user_ldap/tests/integration/lib/integrationtestconnect.php diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php index 3f3953bb28..57dc060f38 100644 --- a/apps/user_ldap/lib/connection.php +++ b/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)) { diff --git a/apps/user_ldap/lib/ldap.php b/apps/user_ldap/lib/ldap.php index 4d45db2e15..e730bff82c 100644 --- a/apps/user_ldap/lib/ldap.php +++ b/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); } /** diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index e53ff35cfd..20926fb06a 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/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); diff --git a/apps/user_ldap/tests/integration/lib/integrationtestconnect.php b/apps/user_ldap/tests/integration/lib/integrationtestconnect.php new file mode 100644 index 0000000000..878aa08f5a --- /dev/null +++ b/apps/user_ldap/tests/integration/lib/integrationtestconnect.php @@ -0,0 +1,166 @@ + + * + * @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 + * + */ + +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();