From 544b24653f67ef66d73536ccf3f98d496aa0f1dd Mon Sep 17 00:00:00 2001 From: deajan Date: Tue, 27 Nov 2018 11:35:02 +0100 Subject: [PATCH 1/5] Faster MS AD group filter --- lib/SP/Providers/Auth/Ldap/LdapActions.php | 12 ++++++++---- lib/SP/Providers/Auth/Ldap/LdapMsAds.php | 13 +++++-------- lib/SP/Providers/Auth/Ldap/LdapMsAzureAd.php | 12 +++++------- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/lib/SP/Providers/Auth/Ldap/LdapActions.php b/lib/SP/Providers/Auth/Ldap/LdapActions.php index 1f43476b..cfb122ee 100644 --- a/lib/SP/Providers/Auth/Ldap/LdapActions.php +++ b/lib/SP/Providers/Auth/Ldap/LdapActions.php @@ -159,7 +159,7 @@ final class LdapActions * * @return bool|array */ - protected function getResults($filter, array $attributes = null) + protected function getResults($filter, array $attributes = null, $searchBase = false) { $cookie = ''; $results = []; @@ -167,7 +167,11 @@ final class LdapActions do { ldap_control_paged_result($this->ldapHandler, 1000, false, $cookie); - $searchRes = @ldap_search($this->ldapHandler, $this->ldapParams->getSearchBase(), $filter, $attributes); + if ($searchBase != false) { + $searchRes = ldap_search($this->ldapHandler, $searchBase, $filter, $attributes); + } else { + $searchRes = @ldap_search($this->ldapHandler, $this->ldapParams->getSearchBase(), $filter, $attributes); + } if (!$searchRes) { return false; @@ -249,9 +253,9 @@ final class LdapActions * @return array * @throws LdapException */ - public function getObjects($filter, array $attributes = self::USER_ATTRIBUTES) + public function getObjects($filter, array $attributes = self::USER_ATTRIBUTES, $searchBase = false) { - $searchResults = $this->getResults($filter, $attributes); + $searchResults = $this->getResults($filter, $attributes, $searchBase); if ($searchResults === false) { $this->eventDispatcher->notifyEvent('ldap.search', diff --git a/lib/SP/Providers/Auth/Ldap/LdapMsAds.php b/lib/SP/Providers/Auth/Ldap/LdapMsAds.php index c51d2290..22324242 100644 --- a/lib/SP/Providers/Auth/Ldap/LdapMsAds.php +++ b/lib/SP/Providers/Auth/Ldap/LdapMsAds.php @@ -116,7 +116,7 @@ final class LdapMsAds extends Ldap return true; } - return $this->checkUserInGroupByFilter($userLogin); + return $this->checkUserInGroupByFilter($userLogin, $userDn); } /** @@ -125,17 +125,14 @@ final class LdapMsAds extends Ldap * @return bool * @throws LdapException */ - private function checkUserInGroupByFilter(string $userLogin): bool + private function checkUserInGroupByFilter(string $userLogin, string $userDn): bool { $groupDn = $this->getGroupDn(); - - $filter = '(&(|' - . LdapUtil::getAttributesForFilter(self::FILTER_USER_ATTRIBUTES, $userLogin) - . ')(|' + $filter = '(|' . LdapUtil::getAttributesForFilter(self::FILTER_GROUP_ATTRIBUTES, $groupDn) - . '))'; + . ')'; - $searchResults = $this->ldapActions->getObjects($filter, ['dn']); + $searchResults = $this->ldapActions->getObjects($filter, ['dn'], $userDn); if (isset($searchResults['count']) && (int)$searchResults['count'] === 0 diff --git a/lib/SP/Providers/Auth/Ldap/LdapMsAzureAd.php b/lib/SP/Providers/Auth/Ldap/LdapMsAzureAd.php index 4454484b..ededc16c 100644 --- a/lib/SP/Providers/Auth/Ldap/LdapMsAzureAd.php +++ b/lib/SP/Providers/Auth/Ldap/LdapMsAzureAd.php @@ -116,7 +116,7 @@ final class LdapMsAzureAd extends Ldap return true; } - return $this->checkUserInGroupByFilter($userLogin); + return $this->checkUserInGroupByFilter($userLogin, $userDn); } /** @@ -125,17 +125,15 @@ final class LdapMsAzureAd extends Ldap * @return bool * @throws LdapException */ - private function checkUserInGroupByFilter(string $userLogin): bool + private function checkUserInGroupByFilter(string $userLogin, string $userDn): bool { $groupDn = $this->getGroupDn(); - $filter = '(&(|' - . LdapUtil::getAttributesForFilter(self::FILTER_USER_ATTRIBUTES, $userLogin) - . ')(|' + $filter = '(|' . LdapUtil::getAttributesForFilter(self::FILTER_GROUP_ATTRIBUTES, $groupDn) - . '))'; + . ')'; - $searchResults = $this->ldapActions->getObjects($filter, ['dn']); + $searchResults = $this->ldapActions->getObjects($filter, ['dn'], $userDn); if (isset($searchResults['count']) && (int)$searchResults['count'] === 0 From d9ac1f93dfdb65b05449345593ba8cfd2eef0bf1 Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Tue, 27 Nov 2018 12:07:13 +0100 Subject: [PATCH 2/5] Added @ to ldap_search again since debugging is finished --- lib/SP/Providers/Auth/Ldap/LdapActions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/SP/Providers/Auth/Ldap/LdapActions.php b/lib/SP/Providers/Auth/Ldap/LdapActions.php index cfb122ee..77d34e38 100644 --- a/lib/SP/Providers/Auth/Ldap/LdapActions.php +++ b/lib/SP/Providers/Auth/Ldap/LdapActions.php @@ -168,7 +168,7 @@ final class LdapActions ldap_control_paged_result($this->ldapHandler, 1000, false, $cookie); if ($searchBase != false) { - $searchRes = ldap_search($this->ldapHandler, $searchBase, $filter, $attributes); + $searchRes = @ldap_search($this->ldapHandler, $searchBase, $filter, $attributes); } else { $searchRes = @ldap_search($this->ldapHandler, $this->ldapParams->getSearchBase(), $filter, $attributes); } From 0b5b3806d3930e8b3cdfbb5f28cd10acb745008a Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Tue, 27 Nov 2018 21:49:05 +0100 Subject: [PATCH 3/5] More elegant code --- lib/SP/Providers/Auth/Ldap/LdapActions.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/SP/Providers/Auth/Ldap/LdapActions.php b/lib/SP/Providers/Auth/Ldap/LdapActions.php index 77d34e38..a4d48d73 100644 --- a/lib/SP/Providers/Auth/Ldap/LdapActions.php +++ b/lib/SP/Providers/Auth/Ldap/LdapActions.php @@ -167,12 +167,11 @@ final class LdapActions do { ldap_control_paged_result($this->ldapHandler, 1000, false, $cookie); - if ($searchBase != false) { - $searchRes = @ldap_search($this->ldapHandler, $searchBase, $filter, $attributes); - } else { - $searchRes = @ldap_search($this->ldapHandler, $this->ldapParams->getSearchBase(), $filter, $attributes); + if ($searchBase == false) { + $searchBase = $this->ldapParams->getSearchBase(); } - + $searchRes = @ldap_search($this->ldapHandler, $searchBase, $filter, $attributes); + if (!$searchRes) { return false; } From 7aa19b43aa65f79045f3b5716cd3505c81584c7d Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Thu, 29 Nov 2018 17:30:02 +0100 Subject: [PATCH 4/5] Switched from default searchBase = false to null --- lib/SP/Providers/Auth/Ldap/LdapActions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/SP/Providers/Auth/Ldap/LdapActions.php b/lib/SP/Providers/Auth/Ldap/LdapActions.php index a4d48d73..0f808ce1 100644 --- a/lib/SP/Providers/Auth/Ldap/LdapActions.php +++ b/lib/SP/Providers/Auth/Ldap/LdapActions.php @@ -159,7 +159,7 @@ final class LdapActions * * @return bool|array */ - protected function getResults($filter, array $attributes = null, $searchBase = false) + protected function getResults($filter, array $attributes = null, $searchBase = null) { $cookie = ''; $results = []; @@ -167,7 +167,7 @@ final class LdapActions do { ldap_control_paged_result($this->ldapHandler, 1000, false, $cookie); - if ($searchBase == false) { + if (empty($searchBase)) { $searchBase = $this->ldapParams->getSearchBase(); } $searchRes = @ldap_search($this->ldapHandler, $searchBase, $filter, $attributes); From 1c3989c8fb30b4710e660db2803f1ccce2ed64dc Mon Sep 17 00:00:00 2001 From: Orsiris de Jong Date: Thu, 29 Nov 2018 17:30:52 +0100 Subject: [PATCH 5/5] Another false to null switch --- lib/SP/Providers/Auth/Ldap/LdapActions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/SP/Providers/Auth/Ldap/LdapActions.php b/lib/SP/Providers/Auth/Ldap/LdapActions.php index 0f808ce1..e81a9e7e 100644 --- a/lib/SP/Providers/Auth/Ldap/LdapActions.php +++ b/lib/SP/Providers/Auth/Ldap/LdapActions.php @@ -252,7 +252,7 @@ final class LdapActions * @return array * @throws LdapException */ - public function getObjects($filter, array $attributes = self::USER_ATTRIBUTES, $searchBase = false) + public function getObjects($filter, array $attributes = self::USER_ATTRIBUTES, $searchBase = null) { $searchResults = $this->getResults($filter, $attributes, $searchBase);