Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable18] shortcut in reading nested group members when IN_CHAIN is available #22204

Merged
merged 3 commits into from Aug 12, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions apps/user_ldap/lib/Configuration.php
Expand Up @@ -44,6 +44,10 @@ class Configuration {
const AVATAR_PREFIX_NONE = 'none';
const AVATAR_PREFIX_DATA_ATTRIBUTE = 'data:';

public const LDAP_SERVER_FEATURE_UNKNOWN = 'unknown';
public const LDAP_SERVER_FEATURE_AVAILABLE = 'available';
public const LDAP_SERVER_FEATURE_UNAVAILABLE = 'unavailable';

protected $configPrefix = null;
protected $configRead = false;
/**
Expand Down Expand Up @@ -109,6 +113,7 @@ class Configuration {
'ldapDynamicGroupMemberURL' => null,
'ldapDefaultPPolicyDN' => null,
'ldapExtStorageHomeAttribute' => null,
'ldapMatchingRuleInChainState' => self::LDAP_SERVER_FEATURE_UNKNOWN,
);

/**
Expand Down Expand Up @@ -481,6 +486,7 @@ public function getDefaults() {
'ldap_default_ppolicy_dn' => '',
'ldap_user_avatar_rule' => 'default',
'ldap_ext_storage_home_attribute' => '',
'ldap_matching_rule_in_chain_state' => self::LDAP_SERVER_FEATURE_UNKNOWN,
);
}

Expand Down Expand Up @@ -542,6 +548,7 @@ public function getConfigTranslationArray() {
'ldap_dynamic_group_member_url' => 'ldapDynamicGroupMemberURL',
'ldap_default_ppolicy_dn' => 'ldapDefaultPPolicyDN',
'ldap_ext_storage_home_attribute' => 'ldapExtStorageHomeAttribute',
'ldap_matching_rule_in_chain_state' => 'ldapMatchingRuleInChainState',
'ldapIgnoreNamingRules' => 'ldapIgnoreNamingRules', // sysconfig
);
return $array;
Expand Down
1 change: 1 addition & 0 deletions apps/user_ldap/lib/Connection.php
Expand Up @@ -66,6 +66,7 @@
* @property string[] ldapBaseGroups
* @property string ldapGroupFilter
* @property string ldapGroupDisplayName
* @property string ldapMatchingRuleInChainState
*/
class Connection extends LDAPUtility {
private $ldapConnectionRes = null;
Expand Down
38 changes: 38 additions & 0 deletions apps/user_ldap/lib/Group_LDAP.php
Expand Up @@ -232,6 +232,37 @@ private function _groupMembers($dnGroup, &$seen = null) {
if($groupMembers !== null) {
return $groupMembers;
}

if ($this->access->connection->ldapNestedGroups
&& $this->access->connection->useMemberOfToDetectMembership
&& $this->access->connection->hasMemberOfFilterSupport
&& $this->access->connection->ldapMatchingRuleInChainState !== Configuration::LDAP_SERVER_FEATURE_UNAVAILABLE
) {
$attemptedLdapMatchingRuleInChain = true;
// compatibility hack with servers supporting :1.2.840.113556.1.4.1941:, and others)
$filter = $this->access->combineFilterWithAnd([
$this->access->connection->ldapUserFilter,
$this->access->connection->ldapUserDisplayName . '=*',
'memberof:1.2.840.113556.1.4.1941:=' . $dnGroup
]);
$memberRecords = $this->access->fetchListOfUsers(
$filter,
$this->access->userManager->getAttributes(true)
);
$result = array_reduce($memberRecords, function ($carry, $record) {
$carry[] = $record['dn'][0];
return $carry;
}, []);
if ($this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_AVAILABLE) {
return $result;
} elseif (!empty($memberRecords)) {
$this->access->connection->ldapMatchingRuleInChainState = Configuration::LDAP_SERVER_FEATURE_AVAILABLE;
$this->access->connection->saveConfiguration();
return $result;
}
// when feature availability is unknown, and the result is empty, continue and test with original approach
}

$seen[$dnGroup] = 1;
$members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr);
if (is_array($members)) {
Expand All @@ -244,6 +275,13 @@ private function _groupMembers($dnGroup, &$seen = null) {
$allMembers += $this->getDynamicGroupMembers($dnGroup);

$this->access->connection->writeToCache($cacheKey, $allMembers);
if (isset($attemptedLdapMatchingRuleInChain)
&& $this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_UNKNOWN
&& !empty($allMembers)
) {
$this->access->connection->ldapMatchingRuleInChainState = Configuration::LDAP_SERVER_FEATURE_UNAVAILABLE;
$this->access->connection->saveConfiguration();
}
return $allMembers;
}

Expand Down
24 changes: 22 additions & 2 deletions apps/user_ldap/tests/Group_LDAPTest.php
Expand Up @@ -72,6 +72,8 @@ private function getAccessMock() {
->method('getConnection')
->will($this->returnValue($connector));

$access->userManager = $this->createMock(Manager::class);

return $access;
}

Expand Down Expand Up @@ -125,6 +127,10 @@ public function testCountEmptySearchString() {
->method('countUsers')
->will($this->returnValue(2));

$access->userManager->expects($this->any())
->method('getAttributes')
->willReturn(['displayName', 'mail']);

$groupBackend = new GroupLDAP($access, $pluginManager);
$users = $groupBackend->countUsersInGroup('group');

Expand Down Expand Up @@ -165,6 +171,10 @@ public function testCountWithSearchString() {
return 'foobar' . \OC::$server->getSecureRandom()->generate(7);
}));

$access->userManager->expects($this->any())
->method('getAttributes')
->willReturn(['displayName', 'mail']);

$groupBackend = new GroupLDAP($access,$pluginManager);
$users = $groupBackend->countUsersInGroup('group', '3');

Expand Down Expand Up @@ -533,7 +543,10 @@ public function testUsersInGroupPrimaryMembersOnly() {
$access->expects($this->exactly(2))
->method('nextcloudUserNames')
->willReturnOnConsecutiveCalls(['lisa', 'bart', 'kira', 'brad'], ['walle', 'dino', 'xenia']);
$access->userManager = $this->createMock(Manager::class);

$access->userManager->expects($this->any())
->method('getAttributes')
->willReturn(['displayName', 'mail']);

$groupBackend = new GroupLDAP($access, $pluginManager);
$users = $groupBackend->usersInGroup('foobar');
Expand Down Expand Up @@ -568,7 +581,10 @@ public function testUsersInGroupPrimaryAndUnixMembers() {
$access->expects($this->once())
->method('nextcloudUserNames')
->will($this->returnValue(array('lisa', 'bart', 'kira', 'brad')));
$access->userManager = $this->createMock(Manager::class);

$access->userManager->expects($this->any())
->method('getAttributes')
->willReturn(['displayName', 'mail']);

$groupBackend = new GroupLDAP($access, $pluginManager);
$users = $groupBackend->usersInGroup('foobar');
Expand Down Expand Up @@ -607,6 +623,10 @@ public function testCountUsersInGroupPrimaryMembersOnly() {
->method('countUsers')
->will($this->returnValue(4));

$access->userManager->expects($this->any())
->method('getAttributes')
->willReturn(['displayName', 'mail']);

$groupBackend = new GroupLDAP($access, $pluginManager);
$users = $groupBackend->countUsersInGroup('foobar');

Expand Down