From b96347485c862767257ea79726982fbcb3321219 Mon Sep 17 00:00:00 2001 From: Kyle Evans Date: Thu, 6 Dec 2018 22:37:05 -0600 Subject: [PATCH] [Ldap] Implement pagination --- .travis.yml | 3 + .../Component/Ldap/Adapter/AbstractQuery.php | 1 + .../Ldap/Adapter/ExtLdap/Collection.php | 33 ++-- .../Component/Ldap/Adapter/ExtLdap/Query.php | 152 ++++++++++++++---- src/Symfony/Component/Ldap/CHANGELOG.md | 5 + .../Tests/Adapter/ExtLdap/AdapterTest.php | 124 ++++++++++++++ .../Ldap/Tests/Fixtures/conf/slapd.conf | 5 +- 7 files changed, 280 insertions(+), 43 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8f023685bb..c8ab525f1d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -62,6 +62,9 @@ before_install: set -e stty cols 120 mkdir /tmp/slapd + if [ ! -e /tmp/slapd-modules ]; then + [ -d /usr/lib/openldap ] && ln -s /usr/lib/openldap /tmp/slapd-modules || ln -s /usr/lib/ldap /tmp/slapd-modules + fi slapd -f src/Symfony/Component/Ldap/Tests/Fixtures/conf/slapd.conf -h ldap://localhost:3389 & [ -d ~/.composer ] || mkdir ~/.composer cp .composer/* ~/.composer/ diff --git a/src/Symfony/Component/Ldap/Adapter/AbstractQuery.php b/src/Symfony/Component/Ldap/Adapter/AbstractQuery.php index 442936b578..b2b777d845 100644 --- a/src/Symfony/Component/Ldap/Adapter/AbstractQuery.php +++ b/src/Symfony/Component/Ldap/Adapter/AbstractQuery.php @@ -35,6 +35,7 @@ abstract class AbstractQuery implements QueryInterface 'deref' => static::DEREF_NEVER, 'attrsOnly' => 0, 'scope' => static::SCOPE_SUB, + 'pageSize' => 0, ]); $resolver->setAllowedValues('deref', [static::DEREF_ALWAYS, static::DEREF_NEVER, static::DEREF_FINDING, static::DEREF_SEARCHING]); $resolver->setAllowedValues('scope', [static::SCOPE_BASE, static::SCOPE_ONE, static::SCOPE_SUB]); diff --git a/src/Symfony/Component/Ldap/Adapter/ExtLdap/Collection.php b/src/Symfony/Component/Ldap/Adapter/ExtLdap/Collection.php index fade430753..4aa9663dc9 100644 --- a/src/Symfony/Component/Ldap/Adapter/ExtLdap/Collection.php +++ b/src/Symfony/Component/Ldap/Adapter/ExtLdap/Collection.php @@ -44,31 +44,40 @@ class Collection implements CollectionInterface public function count() { - if (false !== $count = ldap_count_entries($this->connection->getResource(), $this->search->getResource())) { - return $count; + $con = $this->connection->getResource(); + $searches = $this->search->getResources(); + $count = 0; + foreach ($searches as $search) { + $searchCount = ldap_count_entries($con, $search); + if (false === $searchCount) { + throw new LdapException(sprintf('Error while retrieving entry count: %s.', ldap_error($con))); + } + $count += $searchCount; } - throw new LdapException(sprintf('Error while retrieving entry count: %s.', ldap_error($this->connection->getResource()))); + return $count; } public function getIterator() { - $con = $this->connection->getResource(); - $search = $this->search->getResource(); - $current = ldap_first_entry($con, $search); - if (0 === $this->count()) { return; } - if (false === $current) { - throw new LdapException(sprintf('Could not rewind entries array: %s.', ldap_error($con))); - } + $con = $this->connection->getResource(); + $searches = $this->search->getResources(); + foreach ($searches as $search) { + $current = ldap_first_entry($con, $search); - yield $this->getSingleEntry($con, $current); + if (false === $current) { + throw new LdapException(sprintf('Could not rewind entries array: %s.', ldap_error($con))); + } - while (false !== $current = ldap_next_entry($con, $current)) { yield $this->getSingleEntry($con, $current); + + while (false !== $current = ldap_next_entry($con, $current)) { + yield $this->getSingleEntry($con, $current); + } } } diff --git a/src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php b/src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php index 2147acb4d1..44ad077d72 100644 --- a/src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php +++ b/src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php @@ -21,11 +21,14 @@ use Symfony\Component\Ldap\Exception\NotBoundException; */ class Query extends AbstractQuery { + // As of PHP 7.2, we can use LDAP_CONTROL_PAGEDRESULTS instead of this + const PAGINATION_OID = '1.2.840.113556.1.4.319'; + /** @var Connection */ protected $connection; - /** @var resource */ - private $search; + /** @var resource[] */ + private $results; public function __construct(Connection $connection, string $dn, string $query, array $options = []) { @@ -37,16 +40,19 @@ class Query extends AbstractQuery $con = $this->connection->getResource(); $this->connection = null; - if (null === $this->search || false === $this->search) { + if (null === $this->results) { return; } - $success = ldap_free_result($this->search); - $this->search = null; - - if (!$success) { - throw new LdapException(sprintf('Could not free results: %s.', ldap_error($con))); + foreach ($this->results as $result) { + if (false === $result || null === $result) { + continue; + } + if (!ldap_free_result($result)) { + throw new LdapException(sprintf('Could not free results: %s.', ldap_error($con))); + } } + $this->results = null; } /** @@ -54,12 +60,13 @@ class Query extends AbstractQuery */ public function execute() { - if (null === $this->search) { + if (null === $this->results) { // If the connection is not bound, throw an exception. Users should use an explicit bind call first. if (!$this->connection->isBound()) { throw new NotBoundException('Query execution is not possible without binding the connection first.'); } + $this->results = []; $con = $this->connection->getResource(); switch ($this->options['scope']) { @@ -76,39 +83,126 @@ class Query extends AbstractQuery throw new LdapException(sprintf('Could not search in scope "%s".', $this->options['scope'])); } - $this->search = @$func( - $con, - $this->dn, - $this->query, - $this->options['filter'], - $this->options['attrsOnly'], - $this->options['maxItems'], - $this->options['timeout'], - $this->options['deref'] - ); - } - - if (false === $this->search) { - $ldapError = ''; - if ($errno = ldap_errno($con)) { - $ldapError = sprintf(' LDAP error was [%d] %s', $errno, ldap_error($con)); + $itemsLeft = $maxItems = $this->options['maxItems']; + $pageSize = $this->options['pageSize']; + // Deal with the logic to handle maxItems properly. If we can satisfy it in + // one request based on pageSize, we don't need to bother sending page control + // to the server so that it can determine what we already know. + if (0 !== $maxItems && $pageSize > $maxItems) { + $pageSize = 0; + } elseif (0 !== $maxItems) { + $pageSize = min($maxItems, $pageSize); } + $pageControl = $this->options['scope'] != static::SCOPE_BASE && $pageSize > 0; + $cookie = ''; + do { + if ($pageControl) { + ldap_control_paged_result($con, $pageSize, true, $cookie); + } + $sizeLimit = $itemsLeft; + if ($pageSize > 0 && $sizeLimit >= $pageSize) { + $sizeLimit = 0; + } + $search = @$func( + $con, + $this->dn, + $this->query, + $this->options['filter'], + $this->options['attrsOnly'], + $sizeLimit, + $this->options['timeout'], + $this->options['deref'] + ); - throw new LdapException(sprintf('Could not complete search with dn "%s", query "%s" and filters "%s".%s', $this->dn, $this->query, implode(',', $this->options['filter']), $ldapError)); + if (false === $search) { + $ldapError = ''; + if ($errno = ldap_errno($con)) { + $ldapError = sprintf(' LDAP error was [%d] %s', $errno, ldap_error($con)); + } + if ($pageControl) { + $this->resetPagination(); + } + + throw new LdapException(sprintf('Could not complete search with dn "%s", query "%s" and filters "%s".%s', $this->dn, $this->query, implode(',', $this->options['filter']), $ldapError)); + } + + $this->results[] = $search; + $itemsLeft -= min($itemsLeft, $pageSize); + + if (0 !== $maxItems && 0 === $itemsLeft) { + break; + } + if ($pageControl) { + ldap_control_paged_result_response($con, $search, $cookie); + } + } while (null !== $cookie && '' !== $cookie); + + if ($pageControl) { + $this->resetPagination(); + } } return new Collection($this->connection, $this); } /** - * Returns a LDAP search resource. + * Returns a LDAP search resource. If this query resulted in multiple searches, only the first + * page will be returned. * * @return resource * * @internal */ - public function getResource() + public function getResource($idx = 0) { - return $this->search; + if (null === $this->results || $idx >= \count($this->results)) { + return null; + } + + return $this->results[$idx]; + } + + /** + * Returns all LDAP search resources. + * + * @return resource[] + * + * @internal + */ + public function getResources() + { + return $this->results; + } + + /** + * Resets pagination on the current connection. + * + * @internal + */ + private function resetPagination() + { + $con = $this->connection->getResource(); + ldap_control_paged_result($con, 0); + + // This is a workaround for a bit of a bug in the above invocation + // of ldap_control_paged_result. Instead of indicating to extldap that + // we no longer wish to page queries on this link, this invocation sets + // the LDAP_CONTROL_PAGEDRESULTS OID with a page size of 0. This isn't + // well defined by RFC 2696 if there is no cookie present, so some servers + // will interpret it differently and do the wrong thing. Forcefully remove + // the OID for now until a fix can make its way through the versions of PHP + // the we support. + // + // This is not supported in PHP < 7.2, so these versions will remain broken. + $ctl = []; + ldap_get_option($con, LDAP_OPT_SERVER_CONTROLS, $ctl); + if (!empty($ctl)) { + foreach ($ctl as $idx => $info) { + if (static::PAGINATION_OID == $info['oid']) { + unset($ctl[$idx]); + } + } + ldap_set_option($con, LDAP_OPT_SERVER_CONTROLS, $ctl); + } } } diff --git a/src/Symfony/Component/Ldap/CHANGELOG.md b/src/Symfony/Component/Ldap/CHANGELOG.md index 7dc0c81b3d..05b46af235 100644 --- a/src/Symfony/Component/Ldap/CHANGELOG.md +++ b/src/Symfony/Component/Ldap/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +4.3.0 +----- + + * Added pagination support to the ExtLdap adapter with the pageSize query option. + 4.2.0 ----- diff --git a/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/AdapterTest.php b/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/AdapterTest.php index f5c856f9d5..96faaedd80 100644 --- a/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/AdapterTest.php +++ b/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/AdapterTest.php @@ -15,6 +15,7 @@ use Symfony\Component\Ldap\Adapter\ExtLdap\Adapter; use Symfony\Component\Ldap\Adapter\ExtLdap\Collection; use Symfony\Component\Ldap\Adapter\ExtLdap\Query; use Symfony\Component\Ldap\Entry; +use Symfony\Component\Ldap\Exception\LdapException; use Symfony\Component\Ldap\Exception\NotBoundException; use Symfony\Component\Ldap\LdapInterface; @@ -23,6 +24,12 @@ use Symfony\Component\Ldap\LdapInterface; */ class AdapterTest extends LdapTestCase { + const PAGINATION_REQUIRED_CONFIG = [ + 'options' => [ + 'protocol_version' => 3, + ], + ]; + public function testLdapEscape() { $ldap = new Adapter(); @@ -111,4 +118,121 @@ class AdapterTest extends LdapTestCase $this->assertEquals($one_level_result->count(), 1); $this->assertEquals($one_level_result[0]->getAttribute('ou'), ['Ldap']); } + + public function testLdapPagination() + { + $ldap = new Adapter(array_merge($this->getLdapConfig(), static::PAGINATION_REQUIRED_CONFIG)); + $ldap->getConnection()->bind('cn=admin,dc=symfony,dc=com', 'symfony'); + $entries = $this->setupTestUsers($ldap); + + $unpaged_query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectClass=applicationProcess)(cn=user*))', [ + 'scope' => Query::SCOPE_ONE, + ]); + $fully_paged_query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectClass=applicationProcess)(cn=user*))', [ + 'scope' => Query::SCOPE_ONE, + 'pageSize' => 25, + ]); + $paged_query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectClass=applicationProcess)(cn=user*))', [ + 'scope' => Query::SCOPE_ONE, + 'pageSize' => 5, + ]); + + try { + $unpaged_results = $unpaged_query->execute(); + $fully_paged_results = $fully_paged_query->execute(); + $paged_results = $paged_query->execute(); + + // All four of the above queries should result in the 25 'users' being returned + $this->assertEquals($unpaged_results->count(), 25); + $this->assertEquals($fully_paged_results->count(), 25); + $this->assertEquals($paged_results->count(), 25); + + // They should also result in 1 or 25 / pageSize results + $this->assertEquals(\count($unpaged_query->getResources()), 1); + $this->assertEquals(\count($fully_paged_query->getResources()), 1); + $this->assertEquals(\count($paged_query->getResources()), 5); + + if (PHP_MAJOR_VERSION > 7 || (PHP_MAJOR_VERSION == 7 && PHP_MINOR_VERSION >= 2)) { + // This last query is to ensure that we haven't botched the state of our connection + // by not resetting pagination properly. extldap <= PHP 7.1 do not implement the necessary + // bits to work around an implementation flaw, so we simply can't guarantee this to work there. + $final_query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectClass=applicationProcess)(cn=user*))', [ + 'scope' => Query::SCOPE_ONE, + ]); + + $final_results = $final_query->execute(); + + $this->assertEquals($final_results->count(), 25); + $this->assertEquals(\count($final_query->getResources()), 1); + } + } catch (LdapException $exc) { + $this->markTestSkipped('Test LDAP server does not support pagination'); + } + + $this->destroyEntries($ldap, $entries); + } + + private function setupTestUsers($ldap) + { + $entries = []; + + // Create 25 'users' that we'll query for in different page sizes + $em = $ldap->getEntryManager(); + for ($i = 0; $i < 25; ++$i) { + $cn = sprintf('user%d', $i); + $entry = new Entry(sprintf('cn=%s,dc=symfony,dc=com', $cn)); + $entry->setAttribute('objectClass', ['applicationProcess']); + $entry->setAttribute('cn', [$cn]); + try { + $em->add($entry); + } catch (LdapException $exc) { + // ignored + } + $entries[] = $entry; + } + + return $entries; + } + + private function destroyEntries($ldap, $entries) + { + $em = $ldap->getEntryManager(); + foreach ($entries as $entry) { + $em->remove($entry); + } + } + + public function testLdapPaginationLimits() + { + $ldap = new Adapter(array_merge($this->getLdapConfig(), static::PAGINATION_REQUIRED_CONFIG)); + $ldap->getConnection()->bind('cn=admin,dc=symfony,dc=com', 'symfony'); + + $entries = $this->setupTestUsers($ldap); + + $low_max_query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectClass=applicationProcess)(cn=user*))', [ + 'scope' => Query::SCOPE_ONE, + 'pageSize' => 10, + 'maxItems' => 5, + ]); + $high_max_query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectClass=applicationProcess)(cn=user*))', [ + 'scope' => Query::SCOPE_ONE, + 'pageSize' => 10, + 'maxItems' => 13, + ]); + + try { + $low_max_results = $low_max_query->execute(); + $high_max_results = $high_max_query->execute(); + + $this->assertEquals($low_max_results->count(), 5); + $this->assertEquals($high_max_results->count(), 13); + + $this->assertEquals(\count($low_max_query->getResources()), 1); + $this->assertEquals(\count($high_max_query->getResources()), 2); + } catch (LdapException $exc) { + $this->markTestSkipped('Test LDAP server does not support pagination'); + } + + $this->destroyEntries($ldap, $entries); + } } diff --git a/src/Symfony/Component/Ldap/Tests/Fixtures/conf/slapd.conf b/src/Symfony/Component/Ldap/Tests/Fixtures/conf/slapd.conf index 35f7fe5652..24eebcb2db 100644 --- a/src/Symfony/Component/Ldap/Tests/Fixtures/conf/slapd.conf +++ b/src/Symfony/Component/Ldap/Tests/Fixtures/conf/slapd.conf @@ -7,9 +7,10 @@ include /etc/ldap/schema/nis.schema pidfile /tmp/slapd/slapd.pid argsfile /tmp/slapd/slapd.args -modulepath /usr/lib/openldap +modulepath /tmp/slapd-modules +moduleload back_hdb -database ldif +database hdb directory /tmp/slapd suffix "dc=symfony,dc=com"