fixes returning the base when multiple are specified

* reading the config directly will return the value with line breaks
* using the proper accessor gives us all bases in an array
* returns the first matching one
* having user id provided for the group base is strange and does not let
  us operate like this. here we return the first one. might change in
  future, a backportable fix won't have an API change however.

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

diff --git a/apps/user_ldap/lib/LDAPProvider.php b/apps/user_ldap/lib/LDAPProvider.php
index 94793980b396f8c096dbb9e1c3fc5ce5149357c1..7a0c3cc9517e3e3f34fe67519387f801f7bbd1bf 100644 (file)
--- a/apps/user_ldap/lib/LDAPProvider.php
+++ b/apps/user_ldap/lib/LDAPProvider.php
@@ -182,8 +182,25 @@ class LDAPProvider implements ILDAPProvider, IDeletionFlagSupport {
        public function getLDAPBaseUsers($uid) {
                if(!$this->userBackend->userExists($uid)){
                        throw new \Exception('User id not found in LDAP');
-               }       
-               return $this->userBackend->getLDAPAccess($uid)->getConnection()->getConfiguration()['ldap_base_users'];
+               }
+               $access = $this->userBackend->getLDAPAccess($uid);
+               $bases = $access->getConnection()->ldapBaseUsers;
+               $dn = $this->getUserDN($uid);
+               foreach ($bases as $base) {
+                       if($access->isDNPartOfBase($dn, [$base])) {
+                               return $base;
+                       }
+               }
+               // should not occur, because the user does not qualify to use NC in this case
+               $this->logger->info(
+                       'No matching user base found for user {dn}, available: {bases}.',
+                       [
+                               'app' => 'user_ldap',
+                               'dn' => $dn,
+                               'bases' => $bases,
+                       ]
+               );
+               return array_shift($bases);
        }
        
        /**
@@ -196,7 +213,8 @@ class LDAPProvider implements ILDAPProvider, IDeletionFlagSupport {
                if(!$this->userBackend->userExists($uid)){
                        throw new \Exception('User id not found in LDAP');
                }
-               return $this->userBackend->getLDAPAccess($uid)->getConnection()->getConfiguration()['ldap_base_groups'];
+               $bases = $this->userBackend->getLDAPAccess($uid)->getConnection()->ldapBaseGroups;
+               return array_shift($bases);
        }
        
        /**

diff --git a/apps/user_ldap/tests/LDAPProviderTest.php b/apps/user_ldap/tests/LDAPProviderTest.php
index 1d540c42557a6cfe4d0994b2f66049617da2e15b..054a3adf787988e2e1d5f1f6bc9113310f05b076 100644 (file)
--- a/apps/user_ldap/tests/LDAPProviderTest.php
+++ b/apps/user_ldap/tests/LDAPProviderTest.php
@@ -26,6 +26,8 @@
 namespace OCA\User_LDAP\Tests;
 
 use OC\User\Manager;
+use OCA\User_LDAP\Access;
+use OCA\User_LDAP\Connection;
 use OCA\User_LDAP\IGroupLDAP;
 use OCP\IConfig;
 use OCP\IServerContainer;
@@ -337,24 +339,49 @@ class LDAPProviderTest extends \Test\TestCase {
        }
        
        public function testGetLDAPBaseUsers() {
+               $bases = [
+                       'ou=users,ou=foobar,dc=example,dc=org',
+                       'ou=users,ou=barfoo,dc=example,dc=org',
+               ];
+               $dn = 'uid=malik,' . $bases[1];
+
+               $connection = $this->createMock(Connection::class);
+               $connection->expects($this->any())
+                       ->method('__get')
+                       ->willReturnCallback(function ($key) use ($bases) {
+                               switch($key) {
+                                       case 'ldapBaseUsers':
+                                               return $bases;
+                               }
+                               return null;
+                       });
+
+               $access = $this->createMock(Access::class);
+               $access->expects($this->any())
+                       ->method('getConnection')
+                       ->willReturn($connection);
+               $access->expects($this->exactly(2))
+                       ->method('isDNPartOfBase')
+                       ->willReturnOnConsecutiveCalls(false, true);
+               $access->expects($this->atLeastOnce())
+                       ->method('username2dn')
+                       ->willReturn($dn);
+
                $userBackend = $this->getMockBuilder('OCA\User_LDAP\User_LDAP')
                         ->setMethods(['userExists', 'getLDAPAccess', 'getConnection', 'getConfiguration'])
                         ->disableOriginalConstructor()
                         ->getMock();
-               $userBackend->expects($this->at(0))
+               $userBackend->expects($this->atLeastOnce())
             ->method('userExists')
             ->willReturn(true);
-               $userBackend->expects($this->at(3))
-            ->method('getConfiguration')
-            ->willReturn(array('ldap_base_users'=>'ou=users,dc=example,dc=org'));
                $userBackend->expects($this->any())
-            ->method($this->anything())
-            ->willReturnSelf();
-               
+                       ->method('getLDAPAccess')
+                       ->willReturn($access);
+
                $server = $this->getServerMock($userBackend, $this->getDefaultGroupBackendMock());
                        
                $ldapProvider = $this->getLDAPProvider($server);
-               $this->assertEquals('ou=users,dc=example,dc=org', $ldapProvider->getLDAPBaseUsers('existing_user'));
+               $this->assertEquals($bases[1], $ldapProvider->getLDAPBaseUsers('existing_user'));
        }
        
        /**
@@ -375,24 +402,42 @@ class LDAPProviderTest extends \Test\TestCase {
        }
        
        public function testGetLDAPBaseGroups() {
+               $bases = [
+                       'ou=groupd,ou=foobar,dc=example,dc=org',
+                       'ou=groups,ou=barfoo,dc=example,dc=org',
+               ];
+
+               $connection = $this->createMock(Connection::class);
+               $connection->expects($this->any())
+                       ->method('__get')
+                       ->willReturnCallback(function ($key) use ($bases) {
+                               switch($key) {
+                                       case 'ldapBaseGroups':
+                                               return $bases;
+                               }
+                               return null;
+                       });
+
+               $access = $this->createMock(Access::class);
+               $access->expects($this->any())
+                       ->method('getConnection')
+                       ->willReturn($connection);
+
                $userBackend = $this->getMockBuilder('OCA\User_LDAP\User_LDAP')
                         ->setMethods(['userExists', 'getLDAPAccess', 'getConnection', 'getConfiguration'])
                         ->disableOriginalConstructor()
                         ->getMock();
-               $userBackend->expects($this->at(0))
+               $userBackend->expects($this->any())
             ->method('userExists')
             ->willReturn(true);
-               $userBackend->expects($this->at(3))
-            ->method('getConfiguration')
-            ->willReturn(array('ldap_base_groups'=>'ou=groups,dc=example,dc=org'));
                $userBackend->expects($this->any())
-            ->method($this->anything())
-            ->willReturnSelf();
-               
+                       ->method('getLDAPAccess')
+                       ->willReturn($access);
+
                $server = $this->getServerMock($userBackend, $this->getDefaultGroupBackendMock());
                        
                $ldapProvider = $this->getLDAPProvider($server);
-               $this->assertEquals('ou=groups,dc=example,dc=org', $ldapProvider->getLDAPBaseGroups('existing_user'));
+               $this->assertEquals($bases[0], $ldapProvider->getLDAPBaseGroups('existing_user'));
        }
        
        /**