From 41c334bf2986c890f3d6032002d83e37f0f79df8 Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sun, 6 Sep 2015 18:07:30 -0400 Subject: Add LDAP group sync --- ChangeLog | 1 + app/Auth/Ldap.php | 110 +++++++++++++++++++++++++++------------ app/constants.php | 5 +- config.default.php | 11 ++++ doc/config.markdown | 11 ++++ doc/index.markdown | 1 + doc/ldap-authentication.markdown | 11 ++++ doc/ldap-group-sync.markdown | 36 +++++++++++++ tests/units/Auth/LdapTest.php | 69 ++++++++++++------------ 9 files changed, 187 insertions(+), 68 deletions(-) create mode 100644 doc/ldap-group-sync.markdown diff --git a/ChangeLog b/ChangeLog index b5acf71e..f74fbd5f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,7 @@ Version 1.0.19 (unreleased) New features: +* Add LDAP group sync * Add swimlane description Improvements: diff --git a/app/Auth/Ldap.php b/app/Auth/Ldap.php index cc1b3f95..2f8f791e 100644 --- a/app/Auth/Ldap.php +++ b/app/Auth/Ldap.php @@ -46,7 +46,7 @@ class Ldap extends Base else { // We create automatically a new user - if (LDAP_ACCOUNT_CREATION && $this->createUser($username, $result['name'], $result['email'])) { + if (LDAP_ACCOUNT_CREATION && $this->user->create($result) !== false) { $user = $this->user->getByUsername($username); } else { @@ -64,28 +64,6 @@ class Ldap extends Base return false; } - /** - * Create a new local user after the LDAP authentication - * - * @access public - * @param string $username Username - * @param string $name Name of the user - * @param string $email Email address - * @return bool - */ - public function createUser($username, $name, $email) - { - $values = array( - 'username' => $username, - 'name' => $name, - 'email' => $email, - 'is_admin' => 0, - 'is_ldap_user' => 1, - ); - - return $this->user->create($values); - } - /** * Find the user from the LDAP server * @@ -99,7 +77,7 @@ class Ldap extends Base $ldap = $this->connect(); if ($ldap !== false && $this->bind($ldap, $username, $password)) { - return $this->search($ldap, $username, $password); + return $this->getProfile($ldap, $username, $password); } return false; @@ -180,7 +158,7 @@ class Ldap extends Base } /** - * LDAP user lookup + * Get LDAP user profile * * @access public * @param resource $ldap @@ -188,12 +166,11 @@ class Ldap extends Base * @param string $password * @param string $base_dn * @param string $user_pattern - * @param array $attributes * @return boolean|array */ - public function search($ldap, $username, $password, $base_dn = LDAP_ACCOUNT_BASE, $user_pattern = LDAP_USER_PATTERN, array $attributes = array(LDAP_ACCOUNT_FULLNAME, LDAP_ACCOUNT_EMAIL)) + public function getProfile($ldap, $username, $password, $base_dn = LDAP_ACCOUNT_BASE, $user_pattern = LDAP_USER_PATTERN) { - $sr = ldap_search($ldap, $base_dn, sprintf($user_pattern, $username), $attributes); + $sr = ldap_search($ldap, $base_dn, sprintf($user_pattern, $username), $this->getProfileAttributes()); if ($sr === false) { return false; @@ -206,11 +183,62 @@ class Ldap extends Base } if (@ldap_bind($ldap, $entries[0]['dn'], $password)) { - return array( - 'username' => $username, - 'name' => $this->getEntry($entries, LDAP_ACCOUNT_FULLNAME), - 'email' => $this->getEntry($entries, LDAP_ACCOUNT_EMAIL), - ); + return $this->prepareProfile($ldap, $entries, $username); + } + + return false; + } + + /** + * Build user profile from LDAP information + * + * @access public + * @param resource $ldap + * @param array $entries + * @param string $username + * @return boolean|array + */ + public function prepareProfile($ldap, array $entries, $username) + { + return array( + 'username' => $username, + 'name' => $this->getEntry($entries, LDAP_ACCOUNT_FULLNAME), + 'email' => $this->getEntry($entries, LDAP_ACCOUNT_EMAIL), + 'is_admin' => (int) $this->isMemberOf($this->getEntries($entries, LDAP_ACCOUNT_MEMBEROF), LDAP_GROUP_ADMIN_DN), + 'is_project_admin' => (int) $this->isMemberOf($this->getEntries($entries, LDAP_ACCOUNT_MEMBEROF), LDAP_GROUP_PROJECT_ADMIN_DN), + 'is_ldap_user' => 1, + ); + } + + /** + * Ge the list of attributes to fetch when reading the LDAP user entry + * + * @access public + * @return array + */ + public function getProfileAttributes() + { + return array(LDAP_ACCOUNT_FULLNAME, LDAP_ACCOUNT_EMAIL, LDAP_ACCOUNT_MEMBEROF); + } + + /** + * Check group membership + * + * @access public + * @param array $group_entries + * @param string $group_dn + * @return boolean + */ + public function isMemberOf(array $group_entries, $group_dn) + { + if (! isset($group_entries['count']) || empty($group_dn)) { + return false; + } + + for ($i = 0; $i < $group_entries['count']; $i++) { + if ($group_entries[$i] === $group_dn) { + return true; + } } return false; @@ -286,7 +314,7 @@ class Ldap extends Base } /** - * Return a value from the LDAP info + * Return one entry from a list of entries * * @access private * @param array $entries LDAP entries @@ -298,4 +326,18 @@ class Ldap extends Base { return isset($entries[0][$key][0]) ? $entries[0][$key][0] : $default; } + + /** + * Return subset of entries + * + * @access private + * @param array $entries + * @param string $key + * @param array $default + * @return array + */ + private function getEntries(array $entries, $key, $default = array()) + { + return isset($entries[0][$key]) ? $entries[0][$key] : $default; + } } diff --git a/app/constants.php b/app/constants.php index cf515932..f25bd903 100644 --- a/app/constants.php +++ b/app/constants.php @@ -34,8 +34,11 @@ defined('LDAP_USER_PATTERN') or define('LDAP_USER_PATTERN', ''); defined('LDAP_ACCOUNT_FULLNAME') or define('LDAP_ACCOUNT_FULLNAME', 'displayname'); defined('LDAP_ACCOUNT_EMAIL') or define('LDAP_ACCOUNT_EMAIL', 'mail'); defined('LDAP_ACCOUNT_ID') or define('LDAP_ACCOUNT_ID', ''); -defined('LDAP_USERNAME_CASE_SENSITIVE') or define('LDAP_USERNAME_CASE_SENSITIVE', false); +defined('LDAP_ACCOUNT_MEMBEROF') or define('LDAP_ACCOUNT_MEMBEROF', 'memberof'); defined('LDAP_ACCOUNT_CREATION') or define('LDAP_ACCOUNT_CREATION', true); +defined('LDAP_GROUP_ADMIN_DN') or define('LDAP_GROUP_ADMIN_DN', ''); +defined('LDAP_GROUP_PROJECT_ADMIN_DN') or define('LDAP_GROUP_PROJECT_ADMIN_DN', ''); +defined('LDAP_USERNAME_CASE_SENSITIVE') or define('LDAP_USERNAME_CASE_SENSITIVE', false); // Google authentication defined('GOOGLE_AUTH') or define('GOOGLE_AUTH', false); diff --git a/config.default.php b/config.default.php index d5d1b30d..6cfe260d 100644 --- a/config.default.php +++ b/config.default.php @@ -105,6 +105,17 @@ define('LDAP_ACCOUNT_EMAIL', 'mail'); // Example for OpenLDAP: 'uid' define('LDAP_ACCOUNT_ID', 'samaccountname'); +// LDAP Attribute for group membership +define('LDAP_ACCOUNT_MEMBEROF', 'memberof'); + +// DN for administrators +// Example: CN=Kanboard Admins,CN=Users,DC=kanboard,DC=local +define('LDAP_GROUP_ADMIN_DN', ''); + +// DN for project administrators +// Example: CN=Kanboard Project Admins,CN=Users,DC=kanboard,DC=local +define('LDAP_GROUP_PROJECT_ADMIN_DN', ''); + // By default Kanboard lowercase the ldap username to avoid duplicate users (the database is case sensitive) // Set to true if you want to preserve the case define('LDAP_USERNAME_CASE_SENSITIVE', false); diff --git a/doc/config.markdown b/doc/config.markdown index b5c3ce0d..5473ef9b 100644 --- a/doc/config.markdown +++ b/doc/config.markdown @@ -132,6 +132,17 @@ define('LDAP_ACCOUNT_EMAIL', 'mail'); // Example for OpenLDAP: 'uid' define('LDAP_ACCOUNT_ID', 'samaccountname'); +// LDAP Attribute for group membership +define('LDAP_ACCOUNT_MEMBEROF', 'memberof'); + +// DN for administrators +// Example: CN=Kanboard Admins,CN=Users,DC=kanboard,DC=local +define('LDAP_GROUP_ADMIN_DN', ''); + +// DN for project administrators +// Example: CN=Kanboard Project Admins,CN=Users,DC=kanboard,DC=local +define('LDAP_GROUP_PROJECT_ADMIN_DN', ''); + // By default Kanboard lowercase the ldap username to avoid duplicate users (the database is case sensitive) // Set to true if you want to preserve the case define('LDAP_USERNAME_CASE_SENSITIVE', false); diff --git a/doc/index.markdown b/doc/index.markdown index 0c33bfaa..bc3cc23c 100644 --- a/doc/index.markdown +++ b/doc/index.markdown @@ -120,6 +120,7 @@ Technical details ### Authentication - [LDAP authentication](ldap-authentication.markdown) +- [LDAP group sync](ldap-group-sync.markdown) - [Google authentication](google-authentication.markdown) - [Github authentication](github-authentication.markdown) - [Gitlab authentication](gitlab-authentication.markdown) diff --git a/doc/ldap-authentication.markdown b/doc/ldap-authentication.markdown index 53b3d012..136aa9ac 100644 --- a/doc/ldap-authentication.markdown +++ b/doc/ldap-authentication.markdown @@ -225,6 +225,17 @@ define('LDAP_ACCOUNT_EMAIL', 'mail'); // Example for OpenLDAP: 'uid' define('LDAP_ACCOUNT_ID', 'samaccountname'); +// LDAP Attribute for group membership +define('LDAP_ACCOUNT_MEMBEROF', 'memberof'); + +// DN for administrators +// Example: CN=Kanboard Admins,CN=Users,DC=kanboard,DC=local +define('LDAP_GROUP_ADMIN_DN', ''); + +// DN for project administrators +// Example: CN=Kanboard Project Admins,CN=Users,DC=kanboard,DC=local +define('LDAP_GROUP_PROJECT_ADMIN_DN', ''); + // By default Kanboard lowercase the ldap username to avoid duplicate users (the database is case sensitive) // Set to true if you want to preserve the case define('LDAP_USERNAME_CASE_SENSITIVE', false); diff --git a/doc/ldap-group-sync.markdown b/doc/ldap-group-sync.markdown new file mode 100644 index 00000000..355a1cde --- /dev/null +++ b/doc/ldap-group-sync.markdown @@ -0,0 +1,36 @@ +LDAP Group Synchronization +========================== + +Requirements +------------ + +- Have LDAP authentication properly configured +- Use a LDAP server that supports `memberOf` + +Automatically define Kanboard groups based on LDAP groups +--------------------------------------------------------- + +In your config file, define the constants `LDAP_GROUP_ADMIN_DN` and `LDAP_GROUP_PROJECT_ADMIN_DN`. Here an example, replace the values according to your own LDAP configuration: + +```php +define('LDAP_GROUP_ADMIN_DN', 'CN=Kanboard Admins,CN=Users,DC=kanboard,DC=local'); +define('LDAP_GROUP_PROJECT_ADMIN_DN', 'CN=Kanboard Project Admins,CN=Users,DC=kanboard,DC=local'); +``` + +- People member of "Kanboard Admins" will be "Kanboard Administrators" +- People member of "Kanboard Project Admins" will be "Kanboard Project Administrators" +- Everybody else will be Kanboard Standard Users + +Note: At the moment, that works only at account creation. + +Filter Kanboard access based on the LDAP group +---------------------------------------------- + +To allow only some users to use Kanboard, use the existing `LDAP_USER_PATTERN` constant: + +```php +define('LDAP_USER_PATTERN', '(&(objectClass=user)(sAMAccountName=%s)(memberOf=CN=Kanboard Users,CN=Users,DC=kanboard,DC=local))'); +``` + +This example allow only people member of the group "Kanboard Users" to connect to Kanboard. + diff --git a/tests/units/Auth/LdapTest.php b/tests/units/Auth/LdapTest.php index 5c453b39..0f2ad24e 100644 --- a/tests/units/Auth/LdapTest.php +++ b/tests/units/Auth/LdapTest.php @@ -155,6 +155,9 @@ class LdapTest extends \Base 'username' => 'my_user', 'name' => 'My user', 'email' => 'user1@localhost', + 'is_admin' => 0, + 'is_project_admin' => 0, + 'is_ldap_user' => 1, ); self::$functions @@ -164,7 +167,7 @@ class LdapTest extends \Base $this->equalTo('my_ldap_connection'), $this->equalTo('ou=People,dc=kanboard,dc=local'), $this->equalTo('uid=my_user'), - $this->equalTo(array(LDAP_ACCOUNT_FULLNAME, LDAP_ACCOUNT_EMAIL)) + $this->equalTo($this->ldap->getProfileAttributes()) ) ->will($this->returnValue('my_result_identifier')); @@ -187,7 +190,7 @@ class LdapTest extends \Base ) ->will($this->returnValue(true)); - $this->assertEquals($expected, $this->ldap->search('my_ldap_connection', 'my_user', 'my_password', 'ou=People,dc=kanboard,dc=local', 'uid=%s')); + $this->assertEquals($expected, $this->ldap->getProfile('my_ldap_connection', 'my_user', 'my_password', 'ou=People,dc=kanboard,dc=local', 'uid=%s')); } public function testSearchWithBadPassword() @@ -218,7 +221,7 @@ class LdapTest extends \Base $this->equalTo('my_ldap_connection'), $this->equalTo('ou=People,dc=kanboard,dc=local'), $this->equalTo('uid=my_user'), - $this->equalTo(array(LDAP_ACCOUNT_FULLNAME, LDAP_ACCOUNT_EMAIL)) + $this->equalTo($this->ldap->getProfileAttributes()) ) ->will($this->returnValue('my_result_identifier')); @@ -241,7 +244,7 @@ class LdapTest extends \Base ) ->will($this->returnValue(false)); - $this->assertFalse($this->ldap->search('my_ldap_connection', 'my_user', 'my_password', 'ou=People,dc=kanboard,dc=local', 'uid=%s')); + $this->assertFalse($this->ldap->getProfile('my_ldap_connection', 'my_user', 'my_password', 'ou=People,dc=kanboard,dc=local', 'uid=%s')); } public function testSearchWithUserNotFound() @@ -253,7 +256,7 @@ class LdapTest extends \Base $this->equalTo('my_ldap_connection'), $this->equalTo('ou=People,dc=kanboard,dc=local'), $this->equalTo('uid=my_user'), - $this->equalTo(array(LDAP_ACCOUNT_FULLNAME, LDAP_ACCOUNT_EMAIL)) + $this->equalTo($this->ldap->getProfileAttributes()) ) ->will($this->returnValue('my_result_identifier')); @@ -266,7 +269,7 @@ class LdapTest extends \Base ) ->will($this->returnValue(array())); - $this->assertFalse($this->ldap->search('my_ldap_connection', 'my_user', 'my_password', 'ou=People,dc=kanboard,dc=local', 'uid=%s')); + $this->assertFalse($this->ldap->getProfile('my_ldap_connection', 'my_user', 'my_password', 'ou=People,dc=kanboard,dc=local', 'uid=%s')); } public function testSuccessfulAuthentication() @@ -359,6 +362,8 @@ class LdapTest extends \Base public function testAuthenticationWithAutomaticAccountCreation() { + $ldap_profile = array('username' => 'user', 'name' => 'My user', 'email' => 'user@here'); + $this->container['userSession'] = $this ->getMockBuilder('\Model\UserSession') ->setConstructorArgs(array($this->container)) @@ -368,13 +373,13 @@ class LdapTest extends \Base $this->container['user'] = $this ->getMockBuilder('\Model\User') ->setConstructorArgs(array($this->container)) - ->setMethods(array('getByUsername')) + ->setMethods(array('getByUsername', 'create')) ->getMock(); $ldap = $this ->getMockBuilder('\Auth\Ldap') ->setConstructorArgs(array($this->container)) - ->setMethods(array('findUser', 'createUser')) + ->setMethods(array('findUser')) ->getMock(); $ldap @@ -384,17 +389,7 @@ class LdapTest extends \Base $this->equalTo('user'), $this->equalTo('password') ) - ->will($this->returnValue(array('username' => 'user', 'name' => 'My user', 'email' => 'user@here'))); - - $ldap - ->expects($this->at(1)) - ->method('createUser') - ->with( - $this->equalTo('user'), - $this->equalTo('My user'), - $this->equalTo('user@here') - ) - ->will($this->returnValue(true)); + ->will($this->returnValue($ldap_profile)); $this->container['user'] ->expects($this->at(0)) @@ -406,6 +401,14 @@ class LdapTest extends \Base $this->container['user'] ->expects($this->at(1)) + ->method('create') + ->with( + $this->equalTo($ldap_profile) + ) + ->will($this->returnValue(true)); + + $this->container['user'] + ->expects($this->at(2)) ->method('getByUsername') ->with( $this->equalTo('user') @@ -421,6 +424,8 @@ class LdapTest extends \Base public function testAuthenticationWithAutomaticAccountCreationFailed() { + $ldap_profile = array('username' => 'user', 'name' => 'My user', 'email' => 'user@here'); + $this->container['userSession'] = $this ->getMockBuilder('\Model\UserSession') ->setConstructorArgs(array($this->container)) @@ -430,13 +435,13 @@ class LdapTest extends \Base $this->container['user'] = $this ->getMockBuilder('\Model\User') ->setConstructorArgs(array($this->container)) - ->setMethods(array('getByUsername')) + ->setMethods(array('getByUsername', 'create')) ->getMock(); $ldap = $this ->getMockBuilder('\Auth\Ldap') ->setConstructorArgs(array($this->container)) - ->setMethods(array('findUser', 'createUser')) + ->setMethods(array('findUser')) ->getMock(); $ldap @@ -446,26 +451,24 @@ class LdapTest extends \Base $this->equalTo('user'), $this->equalTo('password') ) - ->will($this->returnValue(array('username' => 'user', 'name' => 'My user', 'email' => 'user@here'))); - - $ldap - ->expects($this->at(1)) - ->method('createUser') - ->with( - $this->equalTo('user'), - $this->equalTo('My user'), - $this->equalTo('user@here') - ) - ->will($this->returnValue(false)); + ->will($this->returnValue($ldap_profile)); $this->container['user'] - ->expects($this->once()) + ->expects($this->at(0)) ->method('getByUsername') ->with( $this->equalTo('user') ) ->will($this->returnValue(null)); + $this->container['user'] + ->expects($this->at(1)) + ->method('create') + ->with( + $this->equalTo($ldap_profile) + ) + ->will($this->returnValue(false)); + $this->container['userSession'] ->expects($this->never()) ->method('refresh'); -- cgit v1.2.3