diff --git a/app/Access/Ldap.php b/app/Access/Ldap.php
index 12a3d1e71..702d629ce 100644
--- a/app/Access/Ldap.php
+++ b/app/Access/Ldap.php
@@ -52,13 +52,25 @@ class Ldap
*
* @param resource|\LDAP\Connection $ldapConnection
*
- * @return resource|\LDAP\Result
+ * @return \LDAP\Result|array|false
*/
public function search($ldapConnection, string $baseDn, string $filter, array $attributes = null)
{
return ldap_search($ldapConnection, $baseDn, $filter, $attributes);
}
+ /**
+ * Read an entry from the LDAP tree.
+ *
+ * @param resource|\Ldap\Connection $ldapConnection
+ *
+ * @return \LDAP\Result|array|false
+ */
+ public function read($ldapConnection, string $baseDn, string $filter, array $attributes = null)
+ {
+ return ldap_read($ldapConnection, $baseDn, $filter, $attributes);
+ }
+
/**
* Get entries from an LDAP search result.
*
diff --git a/app/Access/LdapService.php b/app/Access/LdapService.php
index e822b09a6..7c8d8b18f 100644
--- a/app/Access/LdapService.php
+++ b/app/Access/LdapService.php
@@ -321,94 +321,105 @@ class LdapService
return [];
}
- $userGroups = $this->groupFilter($user);
+ $userGroups = $this->extractGroupsFromSearchResponseEntry($user);
$allGroups = $this->getGroupsRecursive($userGroups, []);
+ $formattedGroups = $this->extractGroupNamesFromLdapGroupDns($allGroups);
if ($this->config['dump_user_groups']) {
throw new JsonDebugException([
- 'details_from_ldap' => $user,
- 'parsed_direct_user_groups' => $userGroups,
- 'parsed_recursive_user_groups' => $allGroups,
+ 'details_from_ldap' => $user,
+ 'parsed_direct_user_groups' => $userGroups,
+ 'parsed_recursive_user_groups' => $allGroups,
+ 'parsed_resulting_group_names' => $formattedGroups,
]);
}
return $allGroups;
}
- /**
- * Get the parent groups of an array of groups.
- *
- * @throws LdapException
- */
- private function getGroupsRecursive(array $groupsArray, array $checked): array
+ protected function extractGroupNamesFromLdapGroupDns(array $groupDNs): array
{
- $groupsToAdd = [];
- foreach ($groupsArray as $groupName) {
- if (in_array($groupName, $checked)) {
- continue;
+ $names = [];
+
+ foreach ($groupDNs as $groupDN) {
+ $exploded = $this->ldap->explodeDn($groupDN, 1);
+ if ($exploded !== false && count($exploded) > 0) {
+ $names[] = $exploded[0];
}
-
- $parentGroups = $this->getGroupGroups($groupName);
- $groupsToAdd = array_merge($groupsToAdd, $parentGroups);
- $checked[] = $groupName;
}
- $groupsArray = array_unique(array_merge($groupsArray, $groupsToAdd), SORT_REGULAR);
-
- if (empty($groupsToAdd)) {
- return $groupsArray;
- }
-
- return $this->getGroupsRecursive($groupsArray, $checked);
+ return array_unique($names);
}
/**
- * Get the parent groups of a single group.
+ * Build an array of all relevant groups DNs after recursively scanning
+ * across parents of the groups given.
*
* @throws LdapException
*/
- private function getGroupGroups(string $groupName): array
+ protected function getGroupsRecursive(array $groupDNs, array $checked): array
{
+ $groupsToAdd = [];
+ foreach ($groupDNs as $groupDN) {
+ if (in_array($groupDN, $checked)) {
+ continue;
+ }
+
+ $parentGroups = $this->getParentsOfGroup($groupDN);
+ $groupsToAdd = array_merge($groupsToAdd, $parentGroups);
+ $checked[] = $groupDN;
+ }
+
+ $uniqueDNs = array_unique(array_merge($groupDNs, $groupsToAdd), SORT_REGULAR);
+
+ if (empty($groupsToAdd)) {
+ return $uniqueDNs;
+ }
+
+ return $this->getGroupsRecursive($uniqueDNs, $checked);
+ }
+
+ /**
+ * @throws LdapException
+ */
+ protected function getParentsOfGroup(string $groupDN): array
+ {
+ $groupsAttr = strtolower($this->config['group_attribute']);
$ldapConnection = $this->getConnection();
$this->bindSystemUser($ldapConnection);
$followReferrals = $this->config['follow_referrals'] ? 1 : 0;
$this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals);
-
- $baseDn = $this->config['base_dn'];
- $groupsAttr = strtolower($this->config['group_attribute']);
-
- $groupFilter = 'CN=' . $this->ldap->escape($groupName);
- $groups = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, $groupFilter, [$groupsAttr]);
- if ($groups['count'] === 0) {
+ $read = $this->ldap->read($ldapConnection, $groupDN, '(objectClass=*)', [$groupsAttr]);
+ $results = $this->ldap->getEntries($ldapConnection, $read);
+ if ($results['count'] === 0) {
return [];
}
- return $this->groupFilter($groups[0]);
+ return $this->extractGroupsFromSearchResponseEntry($results[0]);
}
/**
- * Filter out LDAP CN and DN language in a ldap search return.
- * Gets the base CN (common name) of the string.
+ * Extract an array of group DN values from the given LDAP search response entry
*/
- protected function groupFilter(array $userGroupSearchResponse): array
+ protected function extractGroupsFromSearchResponseEntry(array $ldapEntry): array
{
$groupsAttr = strtolower($this->config['group_attribute']);
- $ldapGroups = [];
+ $groupDNs = [];
$count = 0;
- if (isset($userGroupSearchResponse[$groupsAttr]['count'])) {
- $count = (int) $userGroupSearchResponse[$groupsAttr]['count'];
+ if (isset($ldapEntry[$groupsAttr]['count'])) {
+ $count = (int) $ldapEntry[$groupsAttr]['count'];
}
for ($i = 0; $i < $count; $i++) {
- $dnComponents = $this->ldap->explodeDn($userGroupSearchResponse[$groupsAttr][$i], 1);
- if (!in_array($dnComponents[0], $ldapGroups)) {
- $ldapGroups[] = $dnComponents[0];
+ $dn = $ldapEntry[$groupsAttr][$i];
+ if (!in_array($dn, $groupDNs)) {
+ $groupDNs[] = $dn;
}
}
- return $ldapGroups;
+ return $groupDNs;
}
/**
diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php
index 3f80f00f4..7c2510eb1 100644
--- a/tests/Auth/LdapTest.php
+++ b/tests/Auth/LdapTest.php
@@ -435,6 +435,44 @@ class LdapTest extends TestCase
]);
}
+ public function test_recursive_group_search_queries_via_full_dn()
+ {
+ app('config')->set([
+ 'services.ldap.user_to_groups' => true,
+ 'services.ldap.group_attribute' => 'memberOf',
+ ]);
+
+ $userResp = ['count' => 1, 0 => [
+ 'uid' => [$this->mockUser->name],
+ 'cn' => [$this->mockUser->name],
+ 'dn' => 'dc=test,' . config('services.ldap.base_dn'),
+ 'mail' => [$this->mockUser->email],
+ ]];
+ $groupResp = ['count' => 1,
+ 0 => [
+ 'dn' => 'dc=test,' . config('services.ldap.base_dn'),
+ 'memberof' => [
+ 'count' => 1,
+ 0 => 'cn=ldaptester,ou=groups,dc=example,dc=com',
+ ],
+ ],
+ ];
+
+ $this->commonLdapMocks(1, 1, 3, 4, 3, 1);
+
+ $escapedName = ldap_escape($this->mockUser->name);
+ $this->mockLdap->shouldReceive('searchAndGetEntries')->twice()
+ ->with($this->resourceId, config('services.ldap.base_dn'), "(&(uid={$escapedName}))", \Mockery::type('array'))
+ ->andReturn($userResp, $groupResp);
+
+ $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
+ ->with($this->resourceId, config('services.ldap.base_dn'), $groupResp[0]['dn'], ['memberof'])
+ ->andReturn(['count' => 0]);
+
+ $resp = $this->mockUserLogin();
+ $resp->assertRedirect('/');
+ }
+
public function test_external_auth_id_visible_in_roles_page_when_ldap_active()
{
$role = Role::factory()->create(['display_name' => 'ldaptester', 'external_auth_id' => 'ex-auth-a, test-second-param']);