diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 7efe21287..2f0a05894 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -13,6 +13,7 @@ Contributors: - [Anjar Febrianto](https://github.com/Lasut) - [Ashbike](https://github.com/ashbike) - [Ashish Kulkarni](https://github.com/ashkulz) +- [Bitcoin 333](https://github.com/bitcoin333) - [Busfreak](https://github.com/Busfreak) - [Christian González](https://github.com/nerdoc) - [Chorgroup](https://github.com/chorgroup) diff --git a/ChangeLog b/ChangeLog index ae028de11..64e446e3e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -22,6 +22,8 @@ Improvements: Bug fixes: +* Do not sync user role if LDAP groups are not configured +* Fixed issue with unicode handling for letter based avatars and user initials * Do not send notifications to disabled users * Fix wrong redirect when removing a task from the task view page diff --git a/app/Core/Ldap/User.php b/app/Core/Ldap/User.php index c54aa1ac2..91b485305 100644 --- a/app/Core/Ldap/User.php +++ b/app/Core/Ldap/User.php @@ -108,12 +108,18 @@ class User /** * Get role from LDAP groups * + * Note: Do not touch the current role if groups are not configured + * * @access protected * @param string[] $groupIds * @return string */ protected function getRole(array $groupIds) { + if ($this->hasGroupsNotConfigured()) { + return null; + } + foreach ($groupIds as $groupId) { $groupId = strtolower($groupId); @@ -271,6 +277,17 @@ class User return $this->getGroupUserFilter() !== '' && $this->getGroupUserFilter() !== null; } + /** + * Return true if LDAP Group mapping is not configured + * + * @access public + * @return boolean + */ + public function hasGroupsNotConfigured() + { + return !$this->getGroupAdminDn() && !$this->getGroupManagerDn(); + } + /** * Get LDAP admin group DN * diff --git a/tests/units/Core/Ldap/LdapUserTest.php b/tests/units/Core/Ldap/LdapUserTest.php index 02b9331ec..505b8a034 100644 --- a/tests/units/Core/Ldap/LdapUserTest.php +++ b/tests/units/Core/Ldap/LdapUserTest.php @@ -61,7 +61,7 @@ class LdapUserTest extends Base ->getMock(); } - public function testGetUser() + public function testGetUserWithNoGroupConfigured() { $entries = new Entries(array( 'count' => 1, @@ -136,7 +136,7 @@ class LdapUserTest extends Base $this->assertEquals('my_ldap_user', $user->getUsername()); $this->assertEquals('My LDAP user', $user->getName()); $this->assertEquals('user1@localhost', $user->getEmail()); - $this->assertEquals(Role::APP_USER, $user->getRole()); + $this->assertEquals(null, $user->getRole()); $this->assertSame('', $user->getPhoto()); $this->assertEquals(array(), $user->getExternalGroupIds()); $this->assertEquals(array('is_ldap_user' => 1), $user->getExtraAttributes()); @@ -744,6 +744,11 @@ class LdapUserTest extends Base ->method('getGroupUserFilter') ->will($this->returnValue('(&(objectClass=posixGroup)(memberUid=%s))')); + $this->user + ->expects($this->any()) + ->method('getGroupManagerDn') + ->will($this->returnValue('cn=Kanboard Managers,ou=Groups,dc=kanboard,dc=local')); + $this->user ->expects($this->any()) ->method('getBasDn') diff --git a/tests/units/Core/User/UserPropertyTest.php b/tests/units/Core/User/UserPropertyTest.php index a2f052f47..30e651cb8 100644 --- a/tests/units/Core/User/UserPropertyTest.php +++ b/tests/units/Core/User/UserPropertyTest.php @@ -56,6 +56,30 @@ class UserPropertyTest extends Base ); $this->assertEquals($expected, UserProperty::filterProperties($profile, $properties)); + + $profile = array( + 'id' => 123, + 'username' => 'bob', + 'name' => null, + 'email' => '', + 'other_column' => 'myvalue', + 'role' => Role::APP_ADMIN, + ); + + $properties = array( + 'external_id' => '456', + 'username' => 'bobby', + 'name' => 'Bobby', + 'email' => 'admin@localhost', + 'role' => null, + ); + + $expected = array( + 'name' => 'Bobby', + 'email' => 'admin@localhost', + ); + + $this->assertEquals($expected, UserProperty::filterProperties($profile, $properties)); } public function testFilterPropertiesOverrideExistingValueWhenNecessary()