From 1c48e069494bde1f8ced39b1192c44b858f7c259 Mon Sep 17 00:00:00 2001 From: Bob van de Vijver Date: Wed, 23 Nov 2016 15:20:43 +0100 Subject: [PATCH 1/3] Always have a valid connection when using the EntryManager --- src/Symfony/Component/Ldap/Adapter/ExtLdap/Adapter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Ldap/Adapter/ExtLdap/Adapter.php b/src/Symfony/Component/Ldap/Adapter/ExtLdap/Adapter.php index 545d5d69a75d4..06dbc81524284 100644 --- a/src/Symfony/Component/Ldap/Adapter/ExtLdap/Adapter.php +++ b/src/Symfony/Component/Ldap/Adapter/ExtLdap/Adapter.php @@ -50,7 +50,7 @@ public function getConnection() public function getEntryManager() { if (null === $this->entryManager) { - $this->entryManager = new EntryManager($this->connection); + $this->entryManager = new EntryManager($this->getConnection()); } return $this->entryManager; From c87c3fd1565a4771aef9d971801d6e33d508d709 Mon Sep 17 00:00:00 2001 From: Bob van de Vijver Date: Sun, 18 Dec 2016 16:34:12 +0100 Subject: [PATCH 2/3] [LDAP] Check if bound before executing search/data edits. --- .../Ldap/Adapter/EntryManagerInterface.php | 12 +++++++++++ .../Ldap/Adapter/ExtLdap/EntryManager.php | 21 ++++++++++++++++--- .../Component/Ldap/Adapter/ExtLdap/Query.php | 8 ++++--- .../Component/Ldap/Adapter/QueryInterface.php | 6 ++++++ .../Ldap/Exception/NotBoundException.php | 21 +++++++++++++++++++ 5 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 src/Symfony/Component/Ldap/Exception/NotBoundException.php diff --git a/src/Symfony/Component/Ldap/Adapter/EntryManagerInterface.php b/src/Symfony/Component/Ldap/Adapter/EntryManagerInterface.php index b53e2e0662b3b..9538abfae2b25 100644 --- a/src/Symfony/Component/Ldap/Adapter/EntryManagerInterface.php +++ b/src/Symfony/Component/Ldap/Adapter/EntryManagerInterface.php @@ -12,11 +12,14 @@ namespace Symfony\Component\Ldap\Adapter; use Symfony\Component\Ldap\Entry; +use Symfony\Component\Ldap\Exception\LdapException; +use Symfony\Component\Ldap\Exception\NotBoundException; /** * Entry manager interface. * * @author Charles Sarrazin + * @author Bob van de Vijver */ interface EntryManagerInterface { @@ -24,6 +27,9 @@ interface EntryManagerInterface * Adds a new entry in the Ldap server. * * @param Entry $entry + * + * @throws NotBoundException + * @throws LdapException */ public function add(Entry $entry); @@ -31,6 +37,9 @@ public function add(Entry $entry); * Updates an entry from the Ldap server. * * @param Entry $entry + * + * @throws NotBoundException + * @throws LdapException */ public function update(Entry $entry); @@ -38,6 +47,9 @@ public function update(Entry $entry); * Removes an entry from the Ldap server. * * @param Entry $entry + * + * @throws NotBoundException + * @throws LdapException */ public function remove(Entry $entry); } diff --git a/src/Symfony/Component/Ldap/Adapter/ExtLdap/EntryManager.php b/src/Symfony/Component/Ldap/Adapter/ExtLdap/EntryManager.php index 18043ccc9919a..455602c5afa2e 100644 --- a/src/Symfony/Component/Ldap/Adapter/ExtLdap/EntryManager.php +++ b/src/Symfony/Component/Ldap/Adapter/ExtLdap/EntryManager.php @@ -14,9 +14,11 @@ use Symfony\Component\Ldap\Adapter\EntryManagerInterface; use Symfony\Component\Ldap\Entry; use Symfony\Component\Ldap\Exception\LdapException; +use Symfony\Component\Ldap\Exception\NotBoundException; /** * @author Charles Sarrazin + * @author Bob van de Vijver */ class EntryManager implements EntryManagerInterface { @@ -32,7 +34,7 @@ public function __construct(Connection $connection) */ public function add(Entry $entry) { - $con = $this->connection->getResource(); + $con = $this->getConnectionResource(); if (!@ldap_add($con, $entry->getDn(), $entry->getAttributes())) { throw new LdapException(sprintf('Could not add entry "%s": %s', $entry->getDn(), ldap_error($con))); @@ -46,7 +48,7 @@ public function add(Entry $entry) */ public function update(Entry $entry) { - $con = $this->connection->getResource(); + $con = $this->getConnectionResource(); if (!@ldap_modify($con, $entry->getDn(), $entry->getAttributes())) { throw new LdapException(sprintf('Could not update entry "%s": %s', $entry->getDn(), ldap_error($con))); @@ -58,10 +60,23 @@ public function update(Entry $entry) */ public function remove(Entry $entry) { - $con = $this->connection->getResource(); + $con = $this->getConnectionResource(); if (!@ldap_delete($con, $entry->getDn())) { throw new LdapException(sprintf('Could not remove entry "%s": %s', $entry->getDn(), ldap_error($con))); } } + + /** + * Get the connection resource, but first check if the connection is bound. + */ + private function getConnectionResource() + { + // 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.'); + } + + return $this->connection->getResource(); + } } diff --git a/src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php b/src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php index 0e8eae7d21d17..a268630529883 100644 --- a/src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php +++ b/src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php @@ -13,13 +13,15 @@ use Symfony\Component\Ldap\Adapter\AbstractQuery; use Symfony\Component\Ldap\Exception\LdapException; +use Symfony\Component\Ldap\Exception\NotBoundException; /** * @author Charles Sarrazin + * @author Bob van de Vijver */ class Query extends AbstractQuery { - /** @var Connection */ + /** @var Connection */ protected $connection; /** @var resource */ @@ -53,9 +55,9 @@ public function __destruct() public function execute() { if (null === $this->search) { - // If the connection is not bound, then we try an anonymous bind. + // If the connection is not bound, throw an exception. Users should use an explicit bind call first. if (!$this->connection->isBound()) { - $this->connection->bind(); + throw new NotBoundException('Query execution is not possible without binding the connection first.'); } $con = $this->connection->getResource(); diff --git a/src/Symfony/Component/Ldap/Adapter/QueryInterface.php b/src/Symfony/Component/Ldap/Adapter/QueryInterface.php index ba26de791efe8..c4cd4329bbe70 100644 --- a/src/Symfony/Component/Ldap/Adapter/QueryInterface.php +++ b/src/Symfony/Component/Ldap/Adapter/QueryInterface.php @@ -12,9 +12,12 @@ namespace Symfony\Component\Ldap\Adapter; use Symfony\Component\Ldap\Entry; +use Symfony\Component\Ldap\Exception\LdapException; +use Symfony\Component\Ldap\Exception\NotBoundException; /** * @author Charles Sarrazin + * @author Bob van de Vijver */ interface QueryInterface { @@ -27,6 +30,9 @@ interface QueryInterface * Executes a query and returns the list of Ldap entries. * * @return CollectionInterface|Entry[] + * + * @throws NotBoundException + * @throws LdapException */ public function execute(); } diff --git a/src/Symfony/Component/Ldap/Exception/NotBoundException.php b/src/Symfony/Component/Ldap/Exception/NotBoundException.php new file mode 100644 index 0000000000000..6eb904e8bbaaf --- /dev/null +++ b/src/Symfony/Component/Ldap/Exception/NotBoundException.php @@ -0,0 +1,21 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Ldap\Exception; + +/** + * NotBoundException is thrown if the connection with the LDAP server is not yet bound. + * + * @author Bob van de Vijver + */ +class NotBoundException extends \RuntimeException implements ExceptionInterface +{ +} From 3c2c3ba1f2d988bfefcdd04cc65f34af2fb87afe Mon Sep 17 00:00:00 2001 From: Bob van de Vijver Date: Sun, 18 Dec 2016 17:39:44 +0100 Subject: [PATCH 3/3] Added tests to check unbound exception --- .../Tests/Adapter/ExtLdap/AdapterTest.php | 12 +++++++ .../Tests/Adapter/ExtLdap/LdapManagerTest.php | 34 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/AdapterTest.php b/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/AdapterTest.php index f25d181896d35..a1a80231bd49b 100644 --- a/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/AdapterTest.php +++ b/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/AdapterTest.php @@ -14,6 +14,7 @@ use Symfony\Component\Ldap\Adapter\ExtLdap\Adapter; use Symfony\Component\Ldap\Adapter\ExtLdap\Collection; use Symfony\Component\Ldap\Entry; +use Symfony\Component\Ldap\Exception\NotBoundException; use Symfony\Component\Ldap\LdapInterface; /** @@ -65,4 +66,15 @@ public function testLdapQueryIterator() $this->assertEquals(array('Fabien Potencier'), $entry->getAttribute('cn')); $this->assertEquals(array('fabpot@symfony.com', 'fabien@potencier.com'), $entry->getAttribute('mail')); } + + /** + * @group functional + */ + public function testLdapQueryWithoutBind() + { + $ldap = new Adapter($this->getLdapConfig()); + $this->setExpectedException(NotBoundException::class); + $query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectclass=person)(ou=Maintainers))', array()); + $query->execute(); + } } diff --git a/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/LdapManagerTest.php b/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/LdapManagerTest.php index fa9c7ba156e81..846d6a313d812 100644 --- a/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/LdapManagerTest.php +++ b/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/LdapManagerTest.php @@ -15,6 +15,7 @@ use Symfony\Component\Ldap\Adapter\ExtLdap\Collection; use Symfony\Component\Ldap\Entry; use Symfony\Component\Ldap\Exception\LdapException; +use Symfony\Component\Ldap\Exception\NotBoundException; /** * @requires extension ldap @@ -98,6 +99,39 @@ public function testLdapUpdate() $this->assertNull($entry->getAttribute('email')); } + /** + * @group functional + */ + public function testLdapUnboundAdd() + { + $this->adapter = new Adapter($this->getLdapConfig()); + $this->setExpectedException(NotBoundException::class); + $em = $this->adapter->getEntryManager(); + $em->add(new Entry('')); + } + + /** + * @group functional + */ + public function testLdapUnboundRemove() + { + $this->adapter = new Adapter($this->getLdapConfig()); + $this->setExpectedException(NotBoundException::class); + $em = $this->adapter->getEntryManager(); + $em->remove(new Entry('')); + } + + /** + * @group functional + */ + public function testLdapUnboundUpdate() + { + $this->adapter = new Adapter($this->getLdapConfig()); + $this->setExpectedException(NotBoundException::class); + $em = $this->adapter->getEntryManager(); + $em->update(new Entry('')); + } + /** * @return Collection|Entry[] */