summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--ChangeLog1
-rw-r--r--app/Core/Ldap/Client.php51
-rw-r--r--app/Core/Ldap/ConnectionException.php15
-rw-r--r--tests/units/Core/Ldap/ClientTest.php23
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);