8000 bug #20605 [Ldap] Always have a valid connection when using the Entry… · symfony/symfony@5c68c69 · GitHub
[go: up one dir, main page]

Skip to content

Commit 5c68c69

Browse files
committed
bug #20605 [Ldap] Always have a valid connection when using the EntryManager (bobvandevijver)
This PR was submitted for the 3.2 branch but it was merged into the 3.1 branch instead (closes #20605). Discussion ---------- [Ldap] Always have a valid connection when using the EntryManager | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | They should | Fixed tickets | | License | MIT | Doc PR | The connection might be `null` when calling the `getEntryManager` function as it uses the `$this->connection` instead of retrieved it from it's own method which checks (and constructs if necessary) it first. Commits ------- 7775ec2 [Ldap] Always have a valid connection when using the EntryManager
2 parents 027d588 + 7775ec2 commit 5c68c69

File tree

8 files changed

+109
-7
lines changed

8 files changed

+109
-7
lines changed

src/Symfony/Component/Ldap/Adapter/EntryManagerInterface.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,32 +12,44 @@
1212
namespace Symfony\Component\Ldap\Adapter;
1313

1414
use Symfony\Component\Ldap\Entry;
15+
use Symfony\Component\Ldap\Exception\LdapException;
16+
use Symfony\Component\Ldap\Exception\NotBoundException;
1517

1618
/**
1719
* Entry manager interface.
1820
*
1921
* @author Charles Sarrazin <charles@sarraz.in>
22+
* @author Bob van de Vijver <bobvandevijver@hotmail.com>
2023
*/
2124
interface EntryManagerInterface
2225
{
2326
/**
2427
* Adds a new entry in the Ldap server.
2528
*
2629
* @param Entry $entry
30+
*
31+
* @throws NotBoundException
32+
* @throws LdapException
2733
*/
2834
public function add(Entry $entry);
2935

3036
/**
3137
* Updates an entry from the Ldap server.
3238
*
3339
* @param Entry $entry
40+
*
41+
* @throws NotBoundException
42+
* @throws LdapException
3443
*/
3544
public function update(Entry $entry);
3645

3746
/**
3847
* Removes an entry from the Ldap ser 8000 ver.
3948
*
4049
* @param Entry $entry
50+
*
51+
* @throws NotBoundException
52+
* @throws LdapException
4153
*/
4254
public function remove(Entry $entry);
4355
}

src/Symfony/Component/Ldap/Adapter/ExtLdap/Adapter.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function getConnection()
5050
public function getEntryManager()
5151
{
5252
if (null === $this->entryManager) {
53-
$this->entryManager = new EntryManager($this->connection);
53+
$this->entryManager = new EntryManager($this->getConnection());
5454
}
5555

5656
return $this->entryManager;

src/Symfony/Component/Ldap/Adapter/ExtLdap/EntryManager.php

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
use Symfony\Component\Ldap\Adapter\EntryManagerInterface;
1515
use Symfony\Component\Ldap\Entry;
1616
use Symfony\Component\Ldap\Exception\LdapException;
17+
use Symfony\Component\Ldap\Exception\NotBoundException;
1718

1819
/**
1920
* @author Charles Sarrazin <charles@sarraz.in>
21+
* @author Bob van de Vijver <bobvandevijver@hotmail.com>
2022
*/
2123
class EntryManager implements EntryManagerInterface
2224
{
@@ -32,7 +34,7 @@ public function __construct(Connection $connection)
3234
*/
3335
public function add(Entry $entry)
3436
{
35-
$con = $this->connection->getResource();
37+
$con = $this->getConnectionResource();
3638

3739
if (!@ldap_add($con, $entry->getDn(), $entry->getAttributes())) {
3840
throw new LdapException(sprintf('Could not add entry "%s": %s', $entry->getDn(), ldap_error($con)));
@@ -46,7 +48,7 @@ public function add(Entry $entry)
4648
*/
4749
public function update(Entry $entry)
4850
{
49-
$con = $this->connection->getResource();
51+
$con = $this->getConnectionResource();
5052

5153
if (!@ldap_modify($con, $entry->getDn(), $entry->getAttributes())) {
5254
throw new LdapException(sprintf('Could not update entry "%s": %s', $entry->getDn(), ldap_error($con)));
@@ -58,10 +60,23 @@ public function update(Entry $entry)
5860
*/
5961
public function remove(Entry $entry)
6062
{
61-
$con = $this->connection->getResource();
63+
$con = $this->getConnectionResource();
6264

6365
if (!@ldap_delete($con, $entry->getDn())) {
6466
throw new LdapException(sprintf('Could not remove entry "%s": %s', $entry->getDn(), ldap_error($con)));
6567
}
6668
}
69+
70+
/**
71+
* Get the connection resource, but first check if the connection is bound.
72+
*/
73+
private function getConnectionResource()
74+
{
75+
// If the connection is not bound, throw an exception. Users should use an explicit bind call first.
76+
if (!$this->connection->isBound()) {
77+
throw new NotBoundException('Query execution is not possible without binding the connection first.');
78+
}
79+
80+
return $this->connection->getResource();
81+
}
6782
}

src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@
1313

1414
use Symfony\Component\Ldap\Adapter\AbstractQuery;
1515
use Symfony\Component\Ldap\Exception\LdapException;
16+
use Symfony\Component\Ldap\Exception\NotBoundException;
1617

1718
/**
1819
* @author Charles Sarrazin <charles@sarraz.in>
20+
* @author Bob van de Vijver <bobvandevijver@hotmail.com>
1921
*/
2022
class Query extends AbstractQuery
2123
{
22-
/** @var Connection */
24+
/** @var Connection */
2325
protected $connection;
2426

2527
/** @var resource */
@@ -53,9 +55,9 @@ public function __destruct()
5355
public function execute()
5456
{
5557
if (null === $this->search) {
56-
// If the connection is not bound, then we try an anonymous bind.
58+
// If the connection is not bound, throw an exception. Users should use an explicit bind call first.
5759
if (!$this->connection->isBound()) {
58-
$this->connection->bind();
60+
throw new NotBoundException('Query execution is not possible without binding the connection first.');
5961
}
6062

6163
$con = $this->connection->getResource();

src/Symfony/Component/Ldap/Adapter/QueryInterface.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@
1212
namespace Symfony\Component\Ldap\Adapter;
1313

1414
use Symfony\Component\Ldap\Entry;
15+
use Symfony\Component\Ldap\Exception\LdapException;
16+
use Symfony\Component\Ldap\Exception\NotBoundException;
1517

1618
/**
1719
* @author Charles Sarrazin <charles@sarraz.in>
20+
* @author Bob van de Vijver <bobvandevijver@hotmail.com>
1821
*/
1922
interface QueryInterface
2023
{
@@ -27,6 +30,9 @@ interface QueryInterface
2730
* Executes a query and returns the list of Ldap entries.
2831
*
2932
* @return CollectionInterface|Entry[]
33+
*
34+
* @throws NotBoundException
35+
* @throws LdapException
3036
*/
3137
public function execute();
3238
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Ldap\Exception;
13+
14+
/**
15+
* NotBoundException is thrown if the connection with the LDAP server is not yet bound.
16+
*
17+
* @author Bob van de Vijver <bobvandevijver@hotmail.com>
18+
*/
19+
class NotBoundException extends \RuntimeException implements ExceptionInterface
20+
{
21+
}

src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/AdapterTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\Ldap\Adapter\ExtLdap\Adapter;
1515
use Symfony\Component\Ldap\Adapter\ExtLdap\Collection;
1616
use Symfony\Component\Ldap\Entry;
17+
use Symfony\Component\Ldap\Exception\NotBoundException;
1718
use Symfony\Component\Ldap\LdapInterface;
1819

1920
/**
@@ -65,4 +66,15 @@ public function testLdapQueryIterator()
6566
$this->assertEquals(array('Fabien Potencier'), $entry->getAttribute('cn'));
6667
$this->assertEquals(array('fabpot@symfony.com', 'fabien@potencier.com'), $entry->getAttribute('mail'));
6768
}
69+
70+
/**
71+
* @group functional
72+
*/
73+
public function testLdapQueryWithoutBind()
74+
{
75+
$ldap = new Adapter($this->getLdapConfig());
76+
$this->setExpectedException(NotBoundException::class);
77+
$query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectclass=person)(ou=Maintainers))', array());
78+
$query->execute();
79+
}
6880
}

src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/LdapManagerTest.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\Ldap\Adapter\ExtLdap\Collection;
1616
use Symfony\Component\Ldap\Entry;
1717
use Symfony\Component\Ldap\Exception\LdapException;
18+
use Symfony\Component\Ldap\Exception\NotBoundException;
1819

1920
/**
2021
* @requires extension ldap
@@ -98,6 +99,39 @@ public function testLdapUpdate()
9899
$this->assertNull($entry->getAttribute('email'));
99100
}
100101

102+
/**
103+
* @group functional
104+
*/
105+
public function testLdapUnboundAdd()
106+
{
107+
$this->adapter = new Adapter($this->getLdapConfig());
108+
$this->setExpectedException(NotBoundException::class);
109+
$em = $this->adapter->getEntryManager();
110+
$em->add(new Entry(''));
111+
}
112+
113+
/**
114+
* @group functional
115+
*/
116+
public function testLdapUnboundRemove()
117+
{
118+
$this->adapter = new Adapter($this->getLdapConfig());
119+
$this->setExpectedException(NotBoundException::class);
120+
$em = $this->adapter->getEntryManager();
121+
$em->remove(new Entry(''));
122+
}
123+
124+
/**
125+
* @group functional
126+
*/
127+
public function testLdapUnboundUpdate()
128+
{
129+
$this->adapter = new Adapter($this->getLdapConfig());
130+
$this->setExpectedException(NotBoundException::class);
131+
$em = $this->adapter->getEntryManager();
132+
$em->update(new Entry(''));
133+
}
134+
101135
/**
102136
* @return Collection|Entry[]
103137
*/

0 commit comments

Comments
 (0)
0