diff options
| -rw-r--r-- | ChangeLog | 1 | ||||
| -rw-r--r-- | app/Core/Ldap/Client.php | 51 | ||||
| -rw-r--r-- | app/Core/Ldap/ConnectionException.php | 15 | ||||
| -rw-r--r-- | tests/units/Core/Ldap/ClientTest.php | 23 |
4 files changed, 77 insertions, 13 deletions
@@ -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); |