diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 5444f0815e..be2fbecad8 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -55,7 +55,7 @@ use OCP\ILogger; class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend { protected $enabled = false; - /** @var string[] $cachedGroupMembers array of users with gid as key */ + /** @var string[][] $cachedGroupMembers array of users with gid as key */ protected $cachedGroupMembers; /** @var string[] $cachedGroupsByMember array of groups with uid as key */ protected $cachedGroupsByMember; @@ -136,17 +136,13 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I //usually, LDAP attributes are said to be case insensitive. But there are exceptions of course. $members = $this->_groupMembers($groupDN); - if (!is_array($members) || count($members) === 0) { - $this->access->connection->writeToCache($cacheKey, false); - return false; - } //extra work if we don't get back user DNs switch ($this->ldapGroupMemberAssocAttr) { case 'memberuid': case 'zimbramailforwardingaddress': $requestAttributes = $this->access->userManager->getAttributes(true); - $dns = []; + $users = []; $filterParts = []; $bytes = 0; foreach ($members as $mid) { @@ -160,22 +156,37 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I if ($bytes >= 9000000) { // AD has a default input buffer of 10 MB, we do not want // to take even the chance to exceed it + // so we fetch results with the filterParts we collected so far $filter = $this->access->combineFilterWithOr($filterParts); - $users = $this->access->fetchListOfUsers($filter, $requestAttributes, count($filterParts)); + $search = $this->access->fetchListOfUsers($filter, $requestAttributes, count($filterParts)); $bytes = 0; $filterParts = []; - $dns = array_merge($dns, $users); + $users = array_merge($users, $search); } } + if (count($filterParts) > 0) { + // if there are filterParts left we need to add their result $filter = $this->access->combineFilterWithOr($filterParts); - $users = $this->access->fetchListOfUsers($filter, $requestAttributes, count($filterParts)); - $dns = array_merge($dns, $users); + $search = $this->access->fetchListOfUsers($filter, $requestAttributes, count($filterParts)); + $users = array_merge($users, $search); } - $members = $dns; + + // now we cleanup the users array to get only dns + $dns = []; + foreach ($users as $record) { + $dns[$record['dn'][0]] = 1; + } + $members = array_keys($dns); + break; } + if (count($members) === 0) { + $this->access->connection->writeToCache($cacheKey, false); + return false; + } + $isInGroup = in_array($userDN, $members); $this->access->connection->writeToCache($cacheKey, $isInGroup); $this->access->connection->writeToCache($cacheKeyMembers, $members); diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index 74dd2b467c..bc582ab90b 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -50,6 +50,47 @@ use Test\TestCase; * @package OCA\User_LDAP\Tests */ class Group_LDAPTest extends TestCase { + public function testCountEmptySearchString() { + $access = $this->getAccessMock(); + $pluginManager = $this->getPluginManagerMock(); + $groupDN = 'cn=group,dc=foo,dc=bar'; + + $this->enableGroups($access); + + $access->expects($this->any()) + ->method('groupname2dn') + ->willReturn($groupDN); + $access->expects($this->any()) + ->method('readAttribute') + ->willReturnCallback(function ($dn) use ($groupDN) { + if ($dn === $groupDN) { + return [ + 'uid=u11,ou=users,dc=foo,dc=bar', + 'uid=u22,ou=users,dc=foo,dc=bar', + 'uid=u33,ou=users,dc=foo,dc=bar', + 'uid=u34,ou=users,dc=foo,dc=bar' + ]; + } + return []; + }); + $access->expects($this->any()) + ->method('isDNPartOfBase') + ->willReturn(true); + // for primary groups + $access->expects($this->once()) + ->method('countUsers') + ->willReturn(2); + + $access->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['displayName', 'mail']); + + $groupBackend = new GroupLDAP($access, $pluginManager); + $users = $groupBackend->countUsersInGroup('group'); + + $this->assertSame(6, $users); + } + /** * @return MockObject|Access */ @@ -98,47 +139,6 @@ class Group_LDAPTest extends TestCase { }); } - public function testCountEmptySearchString() { - $access = $this->getAccessMock(); - $pluginManager = $this->getPluginManagerMock(); - $groupDN = 'cn=group,dc=foo,dc=bar'; - - $this->enableGroups($access); - - $access->expects($this->any()) - ->method('groupname2dn') - ->willReturn($groupDN); - $access->expects($this->any()) - ->method('readAttribute') - ->willReturnCallback(function ($dn) use ($groupDN) { - if ($dn === $groupDN) { - return [ - 'uid=u11,ou=users,dc=foo,dc=bar', - 'uid=u22,ou=users,dc=foo,dc=bar', - 'uid=u33,ou=users,dc=foo,dc=bar', - 'uid=u34,ou=users,dc=foo,dc=bar' - ]; - } - return []; - }); - $access->expects($this->any()) - ->method('isDNPartOfBase') - ->willReturn(true); - // for primary groups - $access->expects($this->once()) - ->method('countUsers') - ->willReturn(2); - - $access->userManager->expects($this->any()) - ->method('getAttributes') - ->willReturn(['displayName', 'mail']); - - $groupBackend = new GroupLDAP($access, $pluginManager); - $users = $groupBackend->countUsersInGroup('group'); - - $this->assertSame(6, $users); - } - public function testCountWithSearchString() { $access = $this->getAccessMock(); $pluginManager = $this->getPluginManagerMock(); @@ -503,6 +503,179 @@ class Group_LDAPTest extends TestCase { $groupBackend->inGroup($uid, $gid); } + public function groupWithMembersProvider() { + return [ + [ + 'someGroup', + 'cn=someGroup,ou=allTheGroups,ou=someDepartment,dc=someDomain,dc=someTld', + [ + 'uid=oneUser,ou=someTeam,ou=someDepartment,dc=someDomain,dc=someTld', + 'uid=someUser,ou=someTeam,ou=someDepartment,dc=someDomain,dc=someTld', + 'uid=anotherUser,ou=someTeam,ou=someDepartment,dc=someDomain,dc=someTld', + 'uid=differentUser,ou=someTeam,ou=someDepartment,dc=someDomain,dc=someTld', + ], + ], + ]; + } + + /** + * @dataProvider groupWithMembersProvider + */ + public function testInGroupMember(string $gid, string $groupDn, array $memberDNs) { + $access = $this->getAccessMock(); + $pluginManager = $this->getPluginManagerMock(); + + $access->connection = $this->createMock(Connection::class); + + $uid = 'someUser'; + $userDn = $memberDNs[0]; + + $access->connection->expects($this->any()) + ->method('__get') + ->willReturnCallback(function ($name) { + switch ($name) { + case 'ldapGroupMemberAssocAttr': + return 'member'; + case 'ldapDynamicGroupMemberURL': + return ''; + case 'hasPrimaryGroups': + case 'ldapNestedGroups': + return 0; + default: + return 1; + } + }); + $access->connection->expects($this->any()) + ->method('getFromCache') + ->willReturn(null); + + $access->expects($this->once()) + ->method('username2dn') + ->with($uid) + ->willReturn($userDn); + $access->expects($this->once()) + ->method('groupname2dn') + ->willReturn($groupDn); + $access->expects($this->any()) + ->method('readAttribute') + ->willReturn($memberDNs); + + $groupBackend = new GroupLDAP($access, $pluginManager); + $this->assertTrue($groupBackend->inGroup($uid, $gid)); + } + + /** + * @dataProvider groupWithMembersProvider + */ + public function testInGroupMemberNot(string $gid, string $groupDn, array $memberDNs) { + $access = $this->getAccessMock(); + $pluginManager = $this->getPluginManagerMock(); + + $access->connection = $this->createMock(Connection::class); + + $uid = 'unelatedUser'; + $userDn = 'uid=unrelatedUser,ou=unrelatedTeam,ou=unrelatedDepartment,dc=someDomain,dc=someTld'; + + $access->connection->expects($this->any()) + ->method('__get') + ->willReturnCallback(function ($name) { + switch ($name) { + case 'ldapGroupMemberAssocAttr': + return 'member'; + case 'ldapDynamicGroupMemberURL': + return ''; + case 'hasPrimaryGroups': + case 'ldapNestedGroups': + return 0; + default: + return 1; + } + }); + $access->connection->expects($this->any()) + ->method('getFromCache') + ->willReturn(null); + + $access->expects($this->once()) + ->method('username2dn') + ->with($uid) + ->willReturn($userDn); + $access->expects($this->once()) + ->method('groupname2dn') + ->willReturn($groupDn); + $access->expects($this->any()) + ->method('readAttribute') + ->willReturn($memberDNs); + + $groupBackend = new GroupLDAP($access, $pluginManager); + $this->assertFalse($groupBackend->inGroup($uid, $gid)); + } + + /** + * @dataProvider groupWithMembersProvider + */ + public function testInGroupMemberUid(string $gid, string $groupDn, array $memberDNs) { + $access = $this->getAccessMock(); + $pluginManager = $this->getPluginManagerMock(); + + $memberUids = []; + $userRecords = []; + foreach ($memberDNs as $dn) { + $memberUids[] = ldap_explode_dn($dn, false)[0]; + $userRecords[] = ['dn' => [$dn]]; + } + + + $access->connection = $this->createMock(Connection::class); + + $uid = 'someUser'; + $userDn = $memberDNs[0]; + + $access->connection->expects($this->any()) + ->method('__get') + ->willReturnCallback(function ($name) { + switch ($name) { + case 'ldapGroupMemberAssocAttr': + return 'memberUid'; + case 'ldapDynamicGroupMemberURL': + return ''; + case 'ldapLoginFilter': + return 'uid=%uid'; + case 'hasPrimaryGroups': + case 'ldapNestedGroups': + return 0; + default: + return 1; + } + }); + $access->connection->expects($this->any()) + ->method('getFromCache') + ->willReturn(null); + + $access->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['uid', 'mail', 'displayname']); + + $access->expects($this->once()) + ->method('username2dn') + ->with($uid) + ->willReturn($userDn); + $access->expects($this->once()) + ->method('groupname2dn') + ->willReturn($groupDn); + $access->expects($this->any()) + ->method('readAttribute') + ->willReturn($memberUids); + $access->expects($this->any()) + ->method('fetchListOfUsers') + ->willReturn($userRecords); + $access->expects($this->any()) + ->method('combineFilterWithOr') + ->willReturn('(|(pseudo=filter)(filter=pseudo))'); + + $groupBackend = new GroupLDAP($access, $pluginManager); + $this->assertTrue($groupBackend->inGroup($uid, $gid)); + } + public function testGetGroupsWithOffset() { $access = $this->getAccessMock(); $pluginManager = $this->getPluginManagerMock(); @@ -721,8 +894,8 @@ class Group_LDAPTest extends TestCase { public function nestedGroupsProvider(): array { return [ - [ true ], - [ false ], + [true], + [false], ]; }