Merge pull request #2711 from Icinga/bugfix/improve-error-handling-and-validation-of-multiple-ldap-uris-2645

fixes #2645

application/forms/Config/Resource/LdapResourceForm.php

@@ -4,6 +4,7 @@
 namespace Icinga\Forms\Config\Resource;
 
 use Icinga\Web\Form;
+use Icinga\Web\Url;
 use Icinga\Protocol\Ldap\LdapConnection;
 
 /**
@@ -20,9 +21,7 @@ class LdapResourceForm extends Form
     }
 
     /**
-     * Create and add elements to this form
+     * {@inheritdoc}
-     *
-     * @param   array   $formData   The data sent by the user
      */
     public function createElements(array $formData)
     {
@@ -49,7 +48,38 @@ class LdapResourceForm extends Form
                     'The hostname or address of the LDAP server to use for authentication.'
                     . ' You can also provide multiple hosts separated by a space'
                 ),
-                'value'         => 'localhost'
+                'value'         => 'localhost',
+                'validators'    => array(
+                    array(
+                        'Callback',
+                        false,
+                        array(
+                            'callback'  => function ($v) {
+                                $withoutScheme = $withScheme = false;
+                                foreach (explode(' ', $v) as $uri) {
+                                    if (preg_match('~^(?<!://)[^:]+:\d+$~', $uri)) {
+                                        return false;
+                                    }
+
+                                    $url = Url::fromPath($uri);
+                                    if ($url->getScheme()) {
+                                        $withScheme = true;
+                                    } else {
+                                        $withoutScheme = true;
+                                    }
+                                }
+
+                                return $withScheme ^ $withoutScheme;
+                            },
+                            'messages'  => array(
+                                'callbackValue' => $this->translate(
+                                    'A protocol scheme such as ldap:// or ldaps:// is mandatory for URIs with a given'
+                                    . ' port and for all other URIs as well once a scheme is given for a single one.'
+                                )
+                            )
+                        )
+                    )
+                )
             )
         );
         $this->addElement(

library/Icinga/Protocol/Ldap/LdapCapabilities.php

@@ -291,8 +291,8 @@ class LdapCapabilities
         $result = @ldap_read($ds, '', (string) $connection->select()->from('*', $fields), $fields);
         if (! $result) {
             throw new LdapException(
-                'Capability query failed (%s:%d): %s. Check if hostname and port of the'
+                'Capability query failed (%s; Default port: %d): %s. Check if hostname and port'
-                . ' ldap resource are correct and if anonymous access is permitted.',
+                . ' of the ldap resource are correct and if anonymous access is permitted.',
                 $connection->getHostname(),
                 $connection->getPort(),
                 ldap_error($ds)
@@ -302,7 +302,7 @@ class LdapCapabilities
         $entry = ldap_first_entry($ds, $result);
         if ($entry === false) {
             throw new LdapException(
-                'Capabilities not available (%s:%d): %s. Discovery of root DSE probably not permitted.',
+                'Capabilities not available (%s; Default port: %d): %s. Discovery of root DSE probably not permitted.',
                 $connection->getHostname(),
                 $connection->getPort(),
                 ldap_error($ds)

library/Icinga/Protocol/Ldap/LdapConnection.php

@@ -18,6 +18,7 @@ use Icinga\Data\Inspection;
 use Icinga\Data\Selectable;
 use Icinga\Data\Sortable;
 use Icinga\Exception\ProgrammingError;
+use Icinga\Web\Url;
 
 /**
  * Encapsulate LDAP connections and query creation
@@ -316,11 +317,11 @@ class LdapConnection implements Selectable, Inspectable
         $success = @ldap_bind($ds, $this->bindDn, $this->bindPw);
         if (! $success) {
             throw new LdapException(
-                'LDAP connection to %s:%s (%s / %s) failed: %s',
+                'LDAP bind (%s / %s) to %s with default port %s failed: %s',
-                $this->hostname,
-                $this->port,
                 $this->bindDn,
                 '***' /* $this->bindPw */,
+                $this->hostname,
+                $this->port,
                 ldap_error($ds)
             );
         }
@@ -1155,7 +1156,18 @@ class LdapConnection implements Selectable, Inspectable
         $hostname = $this->hostname;
         if ($this->encryption === static::LDAPS) {
             $info->write('Connect using LDAPS');
-            $hostname = 'ldaps://' . $hostname;
+            $ldapUrls = explode(' ', $hostname);
+            if (count($ldapUrls) > 1) {
+                foreach ($ldapUrls as & $uri) {
+                    if (strpos($uri, '://') === false) {
+                        $uri = 'ldaps://' . $uri;
+                    }
+                }
+
+                $hostname = implode(' ', $ldapUrls);
+            } else {
+                $hostname = 'ldaps://' . $hostname;
+            }
         }
 
         $ds = ldap_connect($hostname, $this->port);
@@ -1209,12 +1221,27 @@ class LdapConnection implements Selectable, Inspectable
         $scope = $query->getScope();
 
         if (Logger::getInstance()->getLevel() === Logger::DEBUG) {
-            // We're checking the level by ourself to avoid rendering the ldapsearch commandline for nothing
+            // We're checking the level by ourselves to avoid rendering the ldapsearch commandline for nothing
             $starttlsParam = $this->encryption === static::STARTTLS ? ' -ZZ' : '';
-            $ldapUrl = ($this->encryption === static::LDAPS ? 'ldaps://' : 'ldap://')
-                . $this->hostname
-                . ($this->port ? ':' . $this->port : '');
 
+            $ldapUrls = array();
+            $defaultScheme = $this->encryption === static::LDAPS ? 'ldaps://' : 'ldap://';
+            foreach (explode(' ', $this->hostname) as $uri) {
+                $url = Url::fromPath($uri);
+                if (! $url->getScheme()) {
+                    $uri = $defaultScheme . $uri . ($this->port ? ':' . $this->port : '');
+                } else {
+                    if ($url->getPort() === null) {
+                        $url->setPort($this->port);
+                    }
+
+                    $uri = $url->getAbsoluteUrl();
+                }
+
+                $ldapUrls[] = $uri;
+            }
+
+            $bindParams = '';
             if ($this->bound) {
                 $bindParams = ' -D "' . $this->bindDn . '"' . ($this->bindPw ? ' -W' : '');
             }
@@ -1232,7 +1259,7 @@ class LdapConnection implements Selectable, Inspectable
             Logger::debug("Issueing LDAP search. Use '%s' to reproduce.", sprintf(
                 'ldapsearch -P 3%s -H "%s"%s -b "%s" -s "%s" -z %u -l %u -a "%s"%s%s%s',
                 $starttlsParam,
-                $ldapUrl,
+                implode(' ', $ldapUrls),
                 $bindParams,
                 $baseDn,
                 $scope,
@@ -1452,11 +1479,11 @@ class LdapConnection implements Selectable, Inspectable
         // Try a bind-command with the given user credentials, this must not fail
         $success = @ldap_bind($ds, $this->bindDn, $this->bindPw);
         $msg = sprintf(
-            'LDAP bind to %s:%s (%s / %s)',
+            'LDAP bind (%s / %s) to %s with default port %s',
-            $this->hostname,
-            $this->port,
             $this->bindDn,
-            '***' /* $this->bindPw */
+            '***' /* $this->bindPw */,
+            $this->hostname,
+            $this->port
         );
         if (! $success) {
             // ldap_error does not return any proper error messages in case of certificate errors. Connecting