diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php
index 5444f0815e3230cb663019e1c2ba855dda0eb51e..be2fbecad853e563abaf3b326e2ce7e5337d842c 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 74dd2b467cf9b0215d5fc9cd6516bf139d49c55d..bc582ab90b51b29d0c036bec09dffd1ff90924d0 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],
];
}