Merge pull request #16015 from nextcloud/backport/16011/stable16

[stable16] (LDAP) API: return one base properly when multiple are configured
nextcloud · Jun 19, 2019 · d3199cf · d3199cf
2 parents 0c79118 + 15c77de
commit d3199cf
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 20 deletions.
diff --git a/apps/user_ldap/lib/ILDAPUserPlugin.php b/apps/user_ldap/lib/ILDAPUserPlugin.php
@@ -40,7 +40,7 @@ public function respondToActions();
 	 *
 	 * @param string $uid The UID of the user to create
 	 * @param string $password The password of the new user
-	 * @return bool
+	 * @return bool|string
 	 */
 	public function createUser($uid, $password);
 

diff --git a/apps/user_ldap/lib/LDAPProvider.php b/apps/user_ldap/lib/LDAPProvider.php
@@ -182,8 +182,25 @@ public function getGroupLDAPConnection($gid) {
 	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 @@ public function getLDAPBaseGroups($uid) {
 		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
@@ -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 @@ public function testGetLDAPBaseUsersUserIDNotFound() {
 	}
 
 	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 @@ public function testGetLDAPBaseGroupsUserIDNotFound() {
 	}
 
 	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'));
 	}
 
 	/**