From dc0749ecce232a5a68d83fbde965ee4ee8e36d00 Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sun, 11 Oct 2015 14:44:16 -0400 Subject: Improve LDAP auth --- app/Auth/Ldap.php | 39 +++++++++++++++++------- config.default.php | 14 ++++----- doc/ldap-authentication.markdown | 65 +++++++++++++++------------------------- tests/units/Auth/LdapTest.php | 33 +++++++++++++++----- 4 files changed, 86 insertions(+), 65 deletions(-) diff --git a/app/Auth/Ldap.php b/app/Auth/Ldap.php index a3c7522c..3a48c402 100644 --- a/app/Auth/Ldap.php +++ b/app/Auth/Ldap.php @@ -30,6 +30,17 @@ class Ldap extends Base return LDAP_SERVER; } + /** + * Get LDAP bind type + * + * @access public + * @return integer + */ + public function getLdapBindType() + { + return LDAP_BIND_TYPE; + } + /** * Get LDAP server port * @@ -265,7 +276,7 @@ class Ldap extends Base public function connect() { if (! function_exists('ldap_connect')) { - $this->logger->error('The PHP LDAP extension is required'); + $this->logger->error('LDAP: The PHP LDAP extension is required'); return false; } @@ -277,7 +288,7 @@ class Ldap extends Base $ldap = ldap_connect($this->getLdapServer(), $this->getLdapPort()); if ($ldap === false) { - $this->logger->error('Unable to connect to the LDAP server'); + $this->logger->error('LDAP: Unable to connect to the LDAP server'); return false; } @@ -287,7 +298,7 @@ class Ldap extends Base ldap_set_option($ldap, LDAP_OPT_TIMELIMIT, 1); if (LDAP_START_TLS && ! @ldap_start_tls($ldap)) { - $this->logger->error('Unable to use ldap_start_tls()'); + $this->logger->error('LDAP: Unable to use ldap_start_tls()'); return false; } @@ -301,16 +312,15 @@ class Ldap extends Base * @param resource $ldap * @param string $username * @param string $password - * @param string $ldap_type * @return boolean */ - public function bind($ldap, $username, $password, $ldap_type = LDAP_BIND_TYPE) + public function bind($ldap, $username, $password) { - if ($ldap_type === 'user') { - $ldap_username = $this->getLdapUserPattern($username); + if ($this->getLdapBindType() === 'user') { + $ldap_username = sprintf($this->getLdapUsername(), $username); $ldap_password = $password; } - else if ($ldap_type === 'proxy') { + else if ($this->getLdapBindType() === 'proxy') { $ldap_username = $this->getLdapUsername(); $ldap_password = $this->getLdapPassword(); } @@ -320,6 +330,8 @@ class Ldap extends Base } if (! @ldap_bind($ldap, $ldap_username, $ldap_password)) { + $this->logger->error('LDAP: Unable to bind to server with: '.$ldap_username); + $this->logger->error('LDAP: bind type='.$this->getLdapBindType()); return false; } @@ -337,8 +349,11 @@ class Ldap extends Base */ public function getProfile($ldap, $username, $password) { - $entries = $this->executeQuery($ldap, $this->getLdapUserPattern($username)); + $user_pattern = $this->getLdapUserPattern($username); + $entries = $this->executeQuery($ldap, $user_pattern); + if ($entries === false) { + $this->logger->error('LDAP: Unable to get user profile: '.$user_pattern); return false; } @@ -346,6 +361,10 @@ class Ldap extends Base return $this->prepareProfile($ldap, $entries, $username); } + if (DEBUG) { + $this->logger->debug('LDAP: wrong password for '.$entries[0]['dn']); + } + return false; } @@ -442,7 +461,7 @@ class Ldap extends Base */ private function executeQuery($ldap, $query) { - $sr = ldap_search($ldap, $this->getLdapBaseDn(), $query, $this->getProfileAttributes()); + $sr = @ldap_search($ldap, $this->getLdapBaseDn(), $query, $this->getProfileAttributes()); if ($sr === false) { return false; } diff --git a/config.default.php b/config.default.php index 2b7da60d..38359d9c 100644 --- a/config.default.php +++ b/config.default.php @@ -72,20 +72,20 @@ define('LDAP_SERVER', ''); // LDAP server port (389 by default) define('LDAP_PORT', 389); -// By default, require certificate to be verified for ldaps:// style URL. Set to false to skip the verification. +// By default, require certificate to be verified for ldaps:// style URL. Set to false to skip the verification define('LDAP_SSL_VERIFY', true); // Enable LDAP START_TLS define('LDAP_START_TLS', false); -// LDAP bind type: "anonymous", "user" (use the given user/password from the form) and "proxy" (a specific user to browse the LDAP directory) +// LDAP bind type: "anonymous", "user" or "proxy" define('LDAP_BIND_TYPE', 'anonymous'); -// LDAP username to connect with. null for anonymous bind (by default). -// Or for user bind type, you can use a pattern: %s@kanboard.local +// LDAP username to use with proxy mode +// LDAP username pattern to use with user mode define('LDAP_USERNAME', null); -// LDAP password to connect with. null for anonymous bind (by default). +// LDAP password to use for proxy mode define('LDAP_PASSWORD', null); // LDAP account base, i.e. root of all user account @@ -97,10 +97,10 @@ define('LDAP_ACCOUNT_BASE', ''); // Example for OpenLDAP: 'uid=%s' define('LDAP_USER_PATTERN', ''); -// Name of an attribute of the user account object which should be used as the full name of the user. +// Name of an attribute of the user account object which should be used as the full name of the user define('LDAP_ACCOUNT_FULLNAME', 'displayname'); -// Name of an attribute of the user account object which should be used as the email of the user. +// Name of an attribute of the user account object which should be used as the email of the user define('LDAP_ACCOUNT_EMAIL', 'mail'); // Name of an attribute of the user account object which should be used as the id of the user. (optional) diff --git a/doc/ldap-authentication.markdown b/doc/ldap-authentication.markdown index 136aa9ac..f2e4869a 100644 --- a/doc/ldap-authentication.markdown +++ b/doc/ldap-authentication.markdown @@ -4,7 +4,7 @@ LDAP authentication Requirements ------------ -- LDAP extension for PHP +- PHP LDAP extension enabled - LDAP server: - OpenLDAP - Microsoft Active Directory @@ -23,8 +23,6 @@ When the LDAP authentication is activated, the login process work like that: - LDAP users have no local passwords - LDAP users can't modify their password with the user interface -- By default, all LDAP users have no admin privileges -- To become administrator, a LDAP user must be promoted by another administrator The full name and the email address are automatically fetched from the LDAP server. @@ -36,9 +34,9 @@ This file must be stored in the root directory of Kanboard. ### LDAP bind type -There is 3 possible ways to browse the LDAP directory: +There are 3 possible ways to browse the LDAP directory: -#### Anonymous browsing +#### Anonymous mode ```php define('LDAP_BIND_TYPE', 'anonymous'); @@ -48,10 +46,9 @@ define('LDAP_PASSWORD', null); This is the default value but some LDAP servers don't allow that. -#### Proxy user +#### Proxy mode -A specific user is used to browse the LDAP directory. -By example, Novell eDirectory use that method. +A specific user is used to browse the LDAP directory: ```php define('LDAP_BIND_TYPE', 'proxy'); @@ -59,33 +56,28 @@ define('LDAP_USERNAME', 'my proxy user'); define('LDAP_PASSWORD', 'my proxy password'); ``` -#### User credentials +#### User mode + +This method uses the credentials provided by the end-user. -This method use the credentials provided by the end-user. By example, Microsoft Active Directory doesn't allow anonymous browsing by default and if you don't want to use a proxy user you can use this method. ```php define('LDAP_BIND_TYPE', 'user'); -define('LDAP_USERNAME', '%s@mydomain.local'); +define('LDAP_USERNAME', '%s@kanboard.local'); define('LDAP_PASSWORD', null); ``` -Here, the `LDAP_USERNAME` is use to define a replacement pattern: - -```php -define('LDAP_USERNAME', '%s@mydomain.local'); - -// Another way to do the same: +In this case, the constant `LDAP_USERNAME` is used as a pattern to the ldap username, examples: -define('LDAP_USERNAME', 'MYDOMAIN\\%s'); -``` +- `%s@kanboard.local` will be replaced by `my_user@kanboard.local` +- `KANBOARD\\%s` will be replaced by `KANBOARD\my_user` ### Example for Microsoft Active Directory Let's say we have a domain `KANBOARD` (kanboard.local) and the primary controller is `myserver.kanboard.local`. -Microsoft Active Directory doesn't allow anonymous binding by default. -First example with a proxy user: +First example with proxy mode: ```php container); + $ldap = $this + ->getMockBuilder('\Auth\Ldap') + ->setConstructorArgs(array($this->container)) + ->setMethods(array('getLdapBindType')) + ->getMock(); + + $ldap + ->expects($this->any()) + ->method('getLdapBindType') + ->will($this->returnValue('anonymous')); self::$functions ->expects($this->once()) @@ -128,7 +137,7 @@ class LdapTest extends \Base ) ->will($this->returnValue(true)); - $this->assertTrue($ldap->bind('my_ldap_connection', 'my_user', 'my_password', 'anonymous')); + $this->assertTrue($ldap->bind('my_ldap_connection', 'my_user', 'my_password')); } public function testBindUser() @@ -136,14 +145,19 @@ class LdapTest extends \Base $ldap = $this ->getMockBuilder('\Auth\Ldap') ->setConstructorArgs(array($this->container)) - ->setMethods(array('getLdapUserPattern')) + ->setMethods(array('getLdapUsername', 'getLdapBindType')) ->getMock(); $ldap ->expects($this->once()) - ->method('getLdapUserPattern') + ->method('getLdapUsername') ->will($this->returnValue('uid=my_user')); + $ldap + ->expects($this->any()) + ->method('getLdapBindType') + ->will($this->returnValue('user')); + self::$functions ->expects($this->once()) ->method('ldap_bind') @@ -154,7 +168,7 @@ class LdapTest extends \Base ) ->will($this->returnValue(true)); - $this->assertTrue($ldap->bind('my_ldap_connection', 'my_user', 'my_password', 'user')); + $this->assertTrue($ldap->bind('my_ldap_connection', 'my_user', 'my_password')); } public function testBindProxy() @@ -162,7 +176,7 @@ class LdapTest extends \Base $ldap = $this ->getMockBuilder('\Auth\Ldap') ->setConstructorArgs(array($this->container)) - ->setMethods(array('getLdapUsername', 'getLdapPassword')) + ->setMethods(array('getLdapUsername', 'getLdapPassword', 'getLdapBindType')) ->getMock(); $ldap @@ -175,6 +189,11 @@ class LdapTest extends \Base ->method('getLdapPassword') ->will($this->returnValue('something')); + $ldap + ->expects($this->any()) + ->method('getLdapBindType') + ->will($this->returnValue('proxy')); + self::$functions ->expects($this->once()) ->method('ldap_bind') @@ -185,7 +204,7 @@ class LdapTest extends \Base ) ->will($this->returnValue(true)); - $this->assertTrue($ldap->bind('my_ldap_connection', 'my_user', 'my_password', 'proxy')); + $this->assertTrue($ldap->bind('my_ldap_connection', 'my_user', 'my_password')); } public function testSearchSuccess() -- cgit v1.2.3