Do not sync user role if LDAP groups are not configured

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)

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
 

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
      *

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')

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()