Merge pull request #25101 from nextcloud/fix/noid/ldap-known-groups

LDAP: make actually use of batch read known groups
nextcloud · Jan 29, 2021 · 6a3321c · 6a3321c
2 parents 395826b + 02b7031
commit 6a3321c
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 1 deletion.
diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php
@@ -945,7 +945,7 @@ public function fetchListOfGroups($filter, $attr, $limit = null, $offset = null)
 
 		array_walk($groupRecords, function ($record) use ($idsByDn) {
 			$newlyMapped = false;
-			$gid = $uidsByDn[$record['dn'][0]] ?? null;
+			$gid = $idsByDn[$record['dn'][0]] ?? null;
 			if ($gid === null) {
 				$gid = $this->dn2ocname($record['dn'][0], null, false, $newlyMapped, $record);
 			}

diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php
@@ -42,6 +42,7 @@
 use OCA\User_LDAP\ILDAPWrapper;
 use OCA\User_LDAP\LDAP;
 use OCA\User_LDAP\LogWrapper;
+use OCA\User_LDAP\Mapping\GroupMapping;
 use OCA\User_LDAP\Mapping\UserMapping;
 use OCA\User_LDAP\User\Manager;
 use OCA\User_LDAP\User\OfflineUser;
@@ -66,6 +67,8 @@ class AccessTest extends TestCase {
 	protected $userMapper;
 	/** @var IManager|\PHPUnit\Framework\MockObject\MockObject */
 	protected $shareManager;
+	/** @var GroupMapping|\PHPUnit\Framework\MockObject\MockObject */
+	protected $groupMapper;
 	/** @var Connection|\PHPUnit\Framework\MockObject\MockObject */
 	private $connection;
 	/** @var LDAP|\PHPUnit\Framework\MockObject\MockObject */
@@ -88,6 +91,7 @@ protected function setUp(): void {
 		$this->helper = $this->createMock(Helper::class);
 		$this->config = $this->createMock(IConfig::class);
 		$this->userMapper = $this->createMock(UserMapping::class);
+		$this->groupMapper = $this->createMock(GroupMapping::class);
 		$this->ncUserManager = $this->createMock(IUserManager::class);
 		$this->shareManager = $this->createMock(IManager::class);
 
@@ -100,6 +104,7 @@ protected function setUp(): void {
 			$this->ncUserManager
 		);
 		$this->access->setUserMapper($this->userMapper);
+		$this->access->setGroupMapper($this->groupMapper);
 	}
 
 	private function getConnectorAndLdapMock() {
@@ -641,6 +646,45 @@ public function testFetchListOfUsers() {
 		$this->assertSame($expected, $list);
 	}
 
+	public function testFetchListOfGroupsKnown() {
+		$filter  = 'objectClass=nextcloudGroup';
+		$attributes = ['cn', 'gidNumber', 'dn'];
+		$base = 'ou=SomeGroups,dc=my,dc=directory';
+
+		$fakeConnection = ldap_connect();
+		$fakeSearchResultResource = ldap_connect();
+		$fakeLdapEntries = [
+			'count' => 2,
+			[
+				'dn' => 'cn=Good Team,' . $base,
+				'cn' => ['Good Team'],
+			],
+			[
+				'dn' => 'cn=Another Good Team,' . $base,
+				'cn' => ['Another Good Team'],
+			]
+		];
+
+		$this->prepareMocksForSearchTests($base, $fakeConnection, $fakeSearchResultResource, $fakeLdapEntries);
+
+		$this->groupMapper->expects($this->any())
+			->method('getListOfIdsByDn')
+			->willReturn([
+				'cn=Good Team,' . $base => 'Good_Team',
+				'cn=Another Good Team,' . $base => 'Another_Good_Team',
+			]);
+		$this->groupMapper->expects($this->never())
+			->method('getNameByDN');
+
+		$this->connection->expects($this->exactly(2))
+			->method('writeToCache');
+
+		$groups = $this->access->fetchListOfGroups($filter, $attributes);
+		$this->assertSame(2, count($groups));
+		$this->assertSame('Good Team', $groups[0]['cn'][0]);
+		$this->assertSame('Another Good Team', $groups[1]['cn'][0]);
+	}
+
 	public function intUsernameProvider() {
 		// system dependent :-/
 		$translitExpected = @iconv('UTF-8', 'ASCII//TRANSLIT', 'fränk') ? 'frank' : 'frnk';