From bca48e088b2757d76e944d5974029f0ec77c649e Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 19 Jun 2019 13:39:15 +0200 Subject: [PATCH] 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 --- apps/user_ldap/lib/LDAPProvider.php | 24 ++++++- apps/user_ldap/tests/LDAPProviderTest.php | 77 ++++++++++++++++++----- 2 files changed, 82 insertions(+), 19 deletions(-) diff --git a/apps/user_ldap/lib/LDAPProvider.php b/apps/user_ldap/lib/LDAPProvider.php index 94793980b3..7a0c3cc951 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 1d540c4255..054a3adf78 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')); } /**