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>

2 files changed, 82 insertions, 19 deletions

diff --git a/apps/user_ldap/lib/LDAPProvider.php b/apps/user_ldap/lib/LDAPProvider.php
index 4121bdd9d2e..41a4bc6d658 100644
--- 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 1d540c42557..054a3adf787 100644
--- 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'));
 	}
 	
 	/**