Skip to content

Commit e322653

Browse files
MorrisJobkeMorrisJobke
authored
Merge pull request #22204 from nextcloud/backport/21559/stable18
[stable18] shortcut in reading nested group members when IN_CHAIN is available
2 parents 7ffd98d + 268fff8 commit e322653

File tree

4 files changed

+95
-11
lines changed

4 files changed

+95
-11
lines changed

‎apps/user_ldap/lib/Configuration.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ class Configuration {
4444
const AVATAR_PREFIX_NONE = 'none';
4545
const AVATAR_PREFIX_DATA_ATTRIBUTE = 'data:';
4646

47+
public const LDAP_SERVER_FEATURE_UNKNOWN = 'unknown';
48+
public const LDAP_SERVER_FEATURE_AVAILABLE = 'available';
49+
public const LDAP_SERVER_FEATURE_UNAVAILABLE = 'unavailable';
50+
4751
protected $configPrefix = null;
4852
protected $configRead = false;
4953
/**
@@ -109,6 +113,7 @@ class Configuration {
109113
'ldapDynamicGroupMemberURL' => null,
110114
'ldapDefaultPPolicyDN' => null,
111115
'ldapExtStorageHomeAttribute' => null,
116+
'ldapMatchingRuleInChainState' => self::LDAP_SERVER_FEATURE_UNKNOWN,
112117
);
113118

114119
/**
@@ -481,6 +486,7 @@ public function getDefaults() {
481486
'ldap_default_ppolicy_dn' => '',
482487
'ldap_user_avatar_rule' => 'default',
483488
'ldap_ext_storage_home_attribute' => '',
489+
'ldap_matching_rule_in_chain_state' => self::LDAP_SERVER_FEATURE_UNKNOWN,
484490
);
485491
}
486492

@@ -542,6 +548,7 @@ public function getConfigTranslationArray() {
542548
'ldap_dynamic_group_member_url' => 'ldapDynamicGroupMemberURL',
543549
'ldap_default_ppolicy_dn' => 'ldapDefaultPPolicyDN',
544550
'ldap_ext_storage_home_attribute' => 'ldapExtStorageHomeAttribute',
551+
'ldap_matching_rule_in_chain_state' => 'ldapMatchingRuleInChainState',
545552
'ldapIgnoreNamingRules' => 'ldapIgnoreNamingRules', // sysconfig
546553
);
547554
return $array;

‎apps/user_ldap/lib/Connection.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
* @property string[] ldapBaseGroups
6767
* @property string ldapGroupFilter
6868
* @property string ldapGroupDisplayName
69+
* @property string ldapMatchingRuleInChainState
6970
*/
7071
class Connection extends LDAPUtility {
7172
private $ldapConnectionRes = null;

‎apps/user_ldap/lib/Group_LDAP.php‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,37 @@ private function _groupMembers($dnGroup, &$seen = null) {
232232
if($groupMembers !== null) {
233233
return $groupMembers;
234234
}
235+
236+
if ($this->access->connection->ldapNestedGroups
237+
&& $this->access->connection->useMemberOfToDetectMembership
238+
&& $this->access->connection->hasMemberOfFilterSupport
239+
&& $this->access->connection->ldapMatchingRuleInChainState !== Configuration::LDAP_SERVER_FEATURE_UNAVAILABLE
240+
) {
241+
$attemptedLdapMatchingRuleInChain = true;
242+
// compatibility hack with servers supporting :1.2.840.113556.1.4.1941:, and others)
243+
$filter = $this->access->combineFilterWithAnd([
244+
$this->access->connection->ldapUserFilter,
245+
$this->access->connection->ldapUserDisplayName . '=*',
246+
'memberof:1.2.840.113556.1.4.1941:=' . $dnGroup
247+
]);
248+
$memberRecords = $this->access->fetchListOfUsers(
249+
$filter,
250+
$this->access->userManager->getAttributes(true)
251+
);
252+
$result = array_reduce($memberRecords, function ($carry, $record) {
253+
$carry[] = $record['dn'][0];
254+
return $carry;
255+
}, []);
256+
if ($this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_AVAILABLE) {
257+
return $result;
258+
} elseif (!empty($memberRecords)) {
259+
$this->access->connection->ldapMatchingRuleInChainState = Configuration::LDAP_SERVER_FEATURE_AVAILABLE;
260+
$this->access->connection->saveConfiguration();
261+
return $result;
262+
}
263+
// when feature availability is unknown, and the result is empty, continue and test with original approach
264+
}
265+
235266
$seen[$dnGroup] = 1;
236267
$members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr);
237268
if (is_array($members)) {
@@ -244,6 +275,13 @@ private function _groupMembers($dnGroup, &$seen = null) {
244275
$allMembers += $this->getDynamicGroupMembers($dnGroup);
245276

246277
$this->access->connection->writeToCache($cacheKey, $allMembers);
278+
if (isset($attemptedLdapMatchingRuleInChain)
279+
&& $this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_UNKNOWN
280+
&& !empty($allMembers)
281+
) {
282+
$this->access->connection->ldapMatchingRuleInChainState = Configuration::LDAP_SERVER_FEATURE_UNAVAILABLE;
283+
$this->access->connection->saveConfiguration();
284+
}
247285
return $allMembers;
248286
}
249287

‎apps/user_ldap/tests/Group_LDAPTest.php‎

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,15 @@ private function getAccessMock() {
7272
->method('getConnection')
7373
->will($this->returnValue($connector));
7474

75+
$access->userManager = $this->createMock(Manager::class);
76+
7577
return $access;
7678
}
7779

7880
private function getPluginManagerMock() {
7981
return $this->getMockBuilder('\OCA\User_LDAP\GroupPluginManager')->getMock();
8082
}
81-
83+
8284
/**
8385
* @param Access|\PHPUnit_Framework_MockObject_MockObject $access
8486
*/
@@ -119,12 +121,21 @@ public function testCountEmptySearchString() {
119121
}
120122
return [];
121123
});
122-
124+
$access->expects($this->any())
125+
->method('combineFilterWithAnd')
126+
->willReturn('pseudo=filter');
127+
$access->expects($this->any())
128+
->method('fetchListOfUsers')
129+
->will($this->returnValue([])); // return val does not matter here
123130
// for primary groups
124131
$access->expects($this->once())
125132
->method('countUsers')
126133
->will($this->returnValue(2));
127134

135+
$access->userManager->expects($this->any())
136+
->method('getAttributes')
137+
->willReturn(['displayName', 'mail']);
138+
128139
$groupBackend = new GroupLDAP($access, $pluginManager);
129140
$users = $groupBackend->countUsersInGroup('group');
130141

@@ -164,6 +175,13 @@ public function testCountWithSearchString() {
164175
->will($this->returnCallback(function() {
165176
return 'foobar' . \OC::$server->getSecureRandom()->generate(7);
166177
}));
178+
$access->expects($this->any())
179+
->method('combineFilterWithAnd')
180+
->willReturn('pseudo=filter');
181+
182+
$access->userManager->expects($this->any())
183+
->method('getAttributes')
184+
->willReturn(['displayName', 'mail']);
167185

168186
$groupBackend = new GroupLDAP($access,$pluginManager);
169187
$users = $groupBackend->countUsersInGroup('group', '3');
@@ -193,7 +211,7 @@ public function testCountUsersWithPlugin() {
193211
$ldap = new GroupLDAP($access, $pluginManager);
194212

195213
$this->assertEquals($ldap->countUsersInGroup('gid', 'search'),42);
196-
}
214+
}
197215

198216
public function testGidNumber2NameSuccess() {
199217
$access = $this->getAccessMock();
@@ -533,7 +551,13 @@ public function testUsersInGroupPrimaryMembersOnly() {
533551
$access->expects($this->exactly(2))
534552
->method('nextcloudUserNames')
535553
->willReturnOnConsecutiveCalls(['lisa', 'bart', 'kira', 'brad'], ['walle', 'dino', 'xenia']);
536-
$access->userManager = $this->createMock(Manager::class);
554+
$access->expects($this->any())
555+
->method('fetchListOfUsers')
556+
->will($this->returnValue([])); // return val does not matter here
557+
558+
$access->userManager->expects($this->any())
559+
->method('getAttributes')
560+
->willReturn(['displayName', 'mail']);
537561

538562
$groupBackend = new GroupLDAP($access, $pluginManager);
539563
$users = $groupBackend->usersInGroup('foobar');
@@ -568,7 +592,12 @@ public function testUsersInGroupPrimaryAndUnixMembers() {
568592
$access->expects($this->once())
569593
->method('nextcloudUserNames')
570594
->will($this->returnValue(array('lisa', 'bart', 'kira', 'brad')));
571-
$access->userManager = $this->createMock(Manager::class);
595+
$access->expects($this->any())
596+
->method('fetchListOfUsers')
597+
->will($this->returnValue([])); // return val does not matter here
598+
$access->userManager->expects($this->any())
599+
->method('getAttributes')
600+
->willReturn(['displayName', 'mail']);
572601

573602
$groupBackend = new GroupLDAP($access, $pluginManager);
574603
$users = $groupBackend->usersInGroup('foobar');
@@ -602,10 +631,19 @@ public function testCountUsersInGroupPrimaryMembersOnly() {
602631
$access->expects($this->any())
603632
->method('groupname2dn')
604633
->will($this->returnValue('cn=foobar,dc=foo,dc=bar'));
605-
634+
$access->expects($this->any())
635+
->method('fetchListOfUsers')
636+
->will($this->returnValue([])); // return val does not matter here
606637
$access->expects($this->once())
607638
->method('countUsers')
608639
->will($this->returnValue(4));
640+
$access->expects($this->any())
641+
->method('combineFilterWithAnd')
642+
->willReturn('pseudo=filter');
643+
644+
$access->userManager->expects($this->any())
645+
->method('getAttributes')
646+
->willReturn(['displayName', 'mail']);
609647

610648
$groupBackend = new GroupLDAP($access, $pluginManager);
611649
$users = $groupBackend->countUsersInGroup('foobar');
@@ -770,7 +808,7 @@ public function testCreateGroupWithPlugin() {
770808
$this->assertEquals($ldap->createGroup('gid'),true);
771809
}
772810

773-
811+
774812
public function testCreateGroupFailing() {
775813
$this->expectException(\Exception::class);
776814

@@ -825,7 +863,7 @@ public function testDeleteGroupWithPlugin() {
825863
$this->assertEquals($ldap->deleteGroup('gid'),'result');
826864
}
827865

828-
866+
829867
public function testDeleteGroupFailing() {
830868
$this->expectException(\Exception::class);
831869

@@ -871,7 +909,7 @@ public function testAddToGroupWithPlugin() {
871909
$this->assertEquals($ldap->addToGroup('uid', 'gid'),'result');
872910
}
873911

874-
912+
875913
public function testAddToGroupFailing() {
876914
$this->expectException(\Exception::class);
877915

@@ -917,7 +955,7 @@ public function testRemoveFromGroupWithPlugin() {
917955
$this->assertEquals($ldap->removeFromGroup('uid', 'gid'),'result');
918956
}
919957

920-
958+
921959
public function testRemoveFromGroupFailing() {
922960
$this->expectException(\Exception::class);
923961

@@ -963,7 +1001,7 @@ public function testGetGroupDetailsWithPlugin() {
9631001
$this->assertEquals($ldap->getGroupDetails('gid'),'result');
9641002
}
9651003

966-
1004+
9671005
public function testGetGroupDetailsFailing() {
9681006
$this->expectException(\Exception::class);
9691007

0 commit comments

Comments
 (0)