Improve LDAP error reporting

4 files changed, 77 insertions, 13 deletions

diff --git a/ChangeLog b/ChangeLog
index d15db403..701aebb5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,7 @@ Version 1.0.39 (unreleased)
 
 Improvements:
 
+* Improve LDAP error reporting
 * Add configuration parameter to disable email configuration from user interface
 * Add email address field for projects
 * Improve forget password behaviour (notify the user that an email has been sent or not)
diff --git a/app/Core/Ldap/Client.php b/app/Core/Ldap/Client.php
index 867d67fe..7df744aa 100644
--- a/app/Core/Ldap/Client.php
+++ b/app/Core/Ldap/Client.php
@@ -69,12 +69,14 @@ class Client
      * Establish server connection
      *
      * @access public
-     * @throws ClientException
-     * @param  string   $server  LDAP server hostname or IP
-     * @param  integer  $port    LDAP port
-     * @param  boolean  $tls     Start TLS
-     * @param  boolean  $verify  Skip SSL certificate verification
+     *
+     * @param  string $server LDAP server hostname or IP
+     * @param  int    $port   LDAP port
+     * @param  bool   $tls    Start TLS
+     * @param  bool   $verify Skip SSL certificate verification
      * @return Client
+     * @throws ClientException
+     * @throws ConnectionException
      */
     public function open($server, $port = LDAP_PORT, $tls = LDAP_START_TLS, $verify = LDAP_SSL_VERIFY)
     {
@@ -86,10 +88,10 @@ class Client
             putenv('LDAPTLS_REQCERT=never');
         }
 
-        $this->ldap = ldap_connect($server, $port);
+        $this->ldap = @ldap_connect($server, $port);
 
         if ($this->ldap === false) {
-            throw new ClientException('LDAP: Unable to connect to the LDAP server');
+            throw new ConnectionException('Malformed LDAP server hostname or LDAP server port');
         }
 
         ldap_set_option($this->ldap, LDAP_OPT_PROTOCOL_VERSION, 3);
@@ -98,7 +100,7 @@ class Client
         ldap_set_option($this->ldap, LDAP_OPT_TIMELIMIT, 1);
 
         if ($tls && ! @ldap_start_tls($this->ldap)) {
-            throw new ClientException('LDAP: Unable to start TLS');
+            throw new ConnectionException('Unable to start LDAP TLS (' . $this->getLdapError() . ')');
         }
 
         return $this;
@@ -114,7 +116,8 @@ class Client
     public function useAnonymousAuthentication()
     {
         if (! @ldap_bind($this->ldap)) {
-            throw new ClientException('Unable to perform anonymous binding');
+            $this->checkForServerConnectionError();
+            throw new ClientException('Unable to perform anonymous binding => '.$this->getLdapError());
         }
 
         return true;
@@ -132,7 +135,8 @@ class Client
     public function authenticate($bind_rdn, $bind_password)
     {
         if (! @ldap_bind($this->ldap, $bind_rdn, $bind_password)) {
-            throw new ClientException('LDAP authentication failure for "'.$bind_rdn.'"');
+            $this->checkForServerConnectionError();
+            throw new ClientException('LDAP authentication failure for "'.$bind_rdn.'" => '.$this->getLdapError());
         }
 
         return true;
@@ -209,4 +213,31 @@ class Client
     {
         return $this->logger !== null;
     }
+
+    /**
+     * Raise ConnectionException if the application is not able to connect to LDAP server
+     *
+     * @access protected
+     * @throws ConnectionException
+     */
+    protected function checkForServerConnectionError()
+    {
+        if (ldap_errno($this->ldap) === -1) {
+            throw new ConnectionException('Unable to connect to LDAP server (' . $this->getLdapError() . ')');
+        }
+    }
+
+    /**
+     * Get extended LDAP error message
+     *
+     * @return string
+     */
+    protected function getLdapError()
+    {
+        ldap_get_option($this->ldap, LDAP_OPT_ERROR_STRING, $extendedErrorMessage);
+        $errorMessage = ldap_error($this->ldap);
+        $errorCode = ldap_errno($this->ldap);
+
+        return 'Code="'.$errorCode.'"; Error="'.$errorMessage.'"; ExtendedError="'.$extendedErrorMessage.'"';
+    }
 }
diff --git a/app/Core/Ldap/ConnectionException.php b/app/Core/Ldap/ConnectionException.php
new file mode 100644
index 00000000..8d6a4959
--- /dev/null
+++ b/app/Core/Ldap/ConnectionException.php
@@ -0,0 +1,15 @@
+<?php
+
+namespace Kanboard\Core\Ldap;
+
+use Exception;
+
+/**
+ * LDAP Connection Exception
+ *
+ * @package ldap
+ * @author  Frederic Guillot
+ */
+class ConnectionException extends Exception
+{
+}
diff --git a/tests/units/Core/Ldap/ClientTest.php b/tests/units/Core/Ldap/ClientTest.php
index d149500e..9f007c28 100644
--- a/tests/units/Core/Ldap/ClientTest.php
+++ b/tests/units/Core/Ldap/ClientTest.php
@@ -13,6 +13,21 @@ function ldap_set_option()
 {
 }
 
+function ldap_get_option($link_identifier, $option, &$error)
+{
+    $error = 'some extended error';
+}
+
+function ldap_error($link_identifier)
+{
+    return 'some error';
+}
+
+function ldap_errno($link_identifier)
+{
+    return -100;
+}
+
 function ldap_bind($link_identifier, $bind_rdn = null, $bind_password = null)
 {
     return ClientTest::$functions->ldap_bind($link_identifier, $bind_rdn, $bind_password);
@@ -26,7 +41,6 @@ function ldap_start_tls($link_identifier)
 class ClientTest extends \Base
 {
     public static $functions;
-    private $ldap;
 
     public function setUp()
     {
@@ -37,8 +51,11 @@ class ClientTest extends \Base
             ->setMethods(array(
                 'ldap_connect',
                 'ldap_set_option',
+                'ldap_get_option',
                 'ldap_bind',
                 'ldap_start_tls',
+                'ldap_error',
+                'ldap_errno'
             ))
             ->getMock();
     }
@@ -83,7 +100,7 @@ class ClientTest extends \Base
             )
             ->will($this->returnValue(false));
 
-        $this->setExpectedException('\Kanboard\Core\Ldap\ClientException');
+        $this->setExpectedException('\Kanboard\Core\Ldap\ConnectionException');
 
         $ldap = new Client;
         $ldap->open('my_ldap_server');
@@ -133,7 +150,7 @@ class ClientTest extends \Base
             )
             ->will($this->returnValue(false));
 
-        $this->setExpectedException('\Kanboard\Core\Ldap\ClientException');
+        $this->setExpectedException('\Kanboard\Core\Ldap\ConnectionException');
 
         $ldap = new Client;
         $ldap->open('my_ldap_server', 389, true);