Skip to content

Commit 49f0e64

Browse files
SimperfitSimperfit
committed
[Ldap][Security] LdapBindAuthenticationProvider does not bind before search query
1 parent beb6036 commit 49f0e64

File tree

7 files changed

+88
-3
lines changed

7 files changed

+88
-3
lines changed

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
CHANGELOG
22
=========
33

4+
4.4.0
5+
6+
* Deprecated the usage of "query_string" without a "search_dn" and a "search_password" config key in Ldap factories.
7+
48
4.3.0
59
-----
610

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/FormLoginLdapFactory.php

+7
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,14 @@ protected function createAuthProvider(ContainerBuilder $container, $id, $config,
3434
->replaceArgument(2, $id)
3535
->replaceArgument(3, new Reference($config['service']))
3636
->replaceArgument(4, $config['dn_string'])
37+
->replaceArgument(5, $config['search_dn'])
38+
->replaceArgument(6, $config['search_password'])
3739
;
3840

3941
if (!empty($config['query_string'])) {
42+
if ('' === $config['search_dn'] || '' === $config['search_password']) {
43+
@trigger_error('Using the "query_string" config without using a "search_dn" and a "search_password" is deprecated since Symfony 4.4 and will throw in Symfony 5.0.', E_USER_DEPRECATED);
44+
}
4045
$definition->addMethodCall('setQueryString', [$config['query_string']]);
4146
}
4247

@@ -52,6 +57,8 @@ public function addConfiguration(NodeDefinition $node)
5257
->scalarNode('service')->defaultValue('ldap')->end()
5358
->scalarNode('dn_string')->defaultValue('{username}')->end()
5459
->scalarNode('query_string')->end()
60+
->scalarNode('search_dn')->defaultValue('')->end()
61+
->scalarNode('search_password')->defaultValue('')->end()
5562
->end()
5663
;
5764
}

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/HttpBasicLdapFactory.php

+7
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,17 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
3535
->replaceArgument(2, $id)
3636
->replaceArgument(3, new Reference($config['service']))
3737
->replaceArgument(4, $config['dn_string'])
38+
->replaceArgument(5, $config['search_dn'])
39+
->replaceArgument(6, $config['search_password'])
3840
;
3941

4042
// entry point
4143
$entryPointId = $this->createEntryPoint($container, $id, $config, $defaultEntryPoint);
4244

4345
if (!empty($config['query_string'])) {
46+
if ('' === $config['search_dn'] || '' === $config['search_password']) {
47+
@trigger_error('Using the "query_string" config without using a "search_dn" and a "search_password" is deprecated since Symfony 4.4 and will throw in Symfony 5.0.', E_USER_DEPRECATED);
48+
}
4449
$definition->addMethodCall('setQueryString', [$config['query_string']]);
4550
}
4651

@@ -62,6 +67,8 @@ public function addConfiguration(NodeDefinition $node)
6267
->scalarNode('service')->defaultValue('ldap')->end()
6368
->scalarNode('dn_string')->defaultValue('{username}')->end()
6469
->scalarNode('query_string')->end()
70+
->scalarNode('search_dn')->defaultValue('')->end()
71+
->scalarNode('search_password')->defaultValue('')->end()
6572
->end()
6673
;
6774
}

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginLdapFactory.php

+7
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,14 @@ protected function createAuthProvider(ContainerBuilder $container, $id, $config,
3636
->replaceArgument(2, $id)
3737
->replaceArgument(3, new Reference($config['service']))
3838
->replaceArgument(4, $config['dn_string'])
39+
->replaceArgument(5, $config['search_dn'])
40+
->replaceArgument(6, $config['search_password'])
3941
;
4042

4143
if (!empty($config['query_string'])) {
44+
if ('' === $config['search_dn'] || '' === $config['search_password']) {
45+
@trigger_error('Using the "query_string" config without using a "search_dn" and a "search_password" is deprecated since Symfony 4.4 and will throw in Symfony 5.0.', E_USER_DEPRECATED);
46+
}
4247
$definition->addMethodCall('setQueryString', [$config['query_string']]);
4348
}
4449

@@ -54,6 +59,8 @@ public function addConfiguration(NodeDefinition $node)
5459
->scalarNode('service')->defaultValue('ldap')->end()
5560
->scalarNode('dn_string')->defaultValue('{username}')->end()
5661
->scalarNode('query_string')->end()
62+
->scalarNode('search_dn')->defaultValue('')->end()
63+
->scalarNode('search_password')->defaultValue('')->end()
5764
->end()
5865
;
5966
}

‎src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml

+2
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,8 @@
195195
<argument /> <!-- UserChecker -->
196196
<argument /> <!-- Provider-shared Key -->
197197
<argument /> <!-- LDAP -->
198+
<argument /> <!-- search dn -->
199+
<argument /> <!-- search password -->
198200
<argument /> <!-- Base DN -->
199201
<argument>%security.authentication.hide_user_not_found%</argument>
200202
</service>

‎src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php

+10-1
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,18 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider
3434
private $ldap;
3535
private $dnString;
3636
private $queryString;
37+
private $searchDn;
38+
private $searchPassword;
3739

38-
public function __construct(UserProviderInterface $userProvider, UserCheckerInterface $userChecker, string $providerKey, LdapInterface $ldap, string $dnString = '{username}', bool $hideUserNotFoundExceptions = true)
40+
public function __construct(UserProviderInterface $userProvider, UserCheckerInterface $userChecker, string $providerKey, LdapInterface $ldap, string $dnString = '{username}', bool $hideUserNotFoundExceptions = true, string $searchDn = '', string $searchPassword = '')
3941
{
4042
parent::__construct($userChecker, $providerKey, $hideUserNotFoundExceptions);
4143

4244
$this->userProvider = $userProvider;
4345
$this->ldap = $ldap;
4446
$this->dnString = $dnString;
47+
$this->searchDn = $searchDn;
48+
$this->searchPassword = $searchPassword;
4549
}
4650

4751
/**
@@ -82,6 +86,11 @@ protected function checkAuthentication(UserInterface $user, UsernamePasswordToke
8286
$username = $this->ldap->escape($username, '', LdapInterface::ESCAPE_DN);
8387

8488
if ($this->queryString) {
89+
if ('' !== $this->searchDn && '' !== $this->searchPassword) {
90+
$this->ldap->bind($this->searchDn, $this->searchPassword);
91+
} else {
92+
@trigger_error('Using the "query_string" config without using a "search_dn" and a "search_password" is deprecated since Symfony 4.4 and will throw in Symfony 5.0.', E_USER_DEPRECATED);
93+
}
8594
$query = str_replace('{username}', $username, $this->queryString);
8695
$result = $this->ldap->query($this->dnString, $query)->execute();
8796
if (1 !== $result->count()) {

‎src/Symfony/Component/Security/Core/Tests/Authentication/Provider/LdapBindAuthenticationProviderTest.php

+51-2
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ public function testQueryForDn()
123123
->with('foo', '')
124124
->will($this->returnValue('foo'))
125125
;
126+
$ldap
127+
->expects($this->at(1))
128+
->method('bind')
129+
->with('elsa', 'test1234A$');
126130
$ldap
127131
->expects($this->once())
128132
->method('query')
@@ -131,7 +135,48 @@ public function testQueryForDn()
131135
;
132136
$userChecker = $this->getMockBuilder(UserCheckerInterface::class)->getMock();
133137

134-
$provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap);
138+
$provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap, '{username}', true, 'elsa', 'test1234A$');
139+
$provider->setQueryString('{username}bar');
140+
$reflection = new \ReflectionMethod($provider, 'checkAuthentication');
141+
$reflection->setAccessible(true);
142+
143+
$reflection->invoke($provider, new User('foo', null), new UsernamePasswordToken('foo', 'bar', 'key'));
144+
}
145+
146+
public function testQueryWithUserForDn()
147+
{
148+
$userProvider = $this->getMockBuilder(UserProviderInterface::class)->getMock();
149+
150+
$collection = new \ArrayIterator([new Entry('')]);
151+
152+
$query = $this->getMockBuilder(QueryInterface::class)->getMock();
153+
$query
154+
->expects($this->once())
155+
->method('execute')
156+
->will($this->returnValue($collection))
157+
;
158+
159+
$ldap = $this->getMockBuilder(LdapInterface::class)->getMock();
160+
$ldap
161+
->expects($this->once())
162+
->method('escape')
163+
->with('foo', '')
164+
->will($this->returnValue('foo'))
165+
;
166+
$ldap
167+
->expects($this->at(1))
168+
->method('bind')
169+
->with('elsa', 'test1234A$');
170+
$ldap
171+
->expects($this->once())
172+
->method('query')
173+
->with('{username}', 'foobar')
174+
->will($this->returnValue($query))
175+
;
176+
177+
$userChecker = $this->getMockBuilder(UserCheckerInterface::class)->getMock();
178+
179+
$provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap, '{username}', true, 'elsa', 'test1234A$');
135180
$provider->setQueryString('{username}bar');
136181
$reflection = new \ReflectionMethod($provider, 'checkAuthentication');
137182
$reflection->setAccessible(true);
@@ -157,14 +202,18 @@ public function testEmptyQueryResultShouldThrowAnException()
157202
;
158203

159204
$ldap = $this->getMockBuilder(LdapInterface::class)->getMock();
205+
$ldap
206+
->expects($this->at(1))
207+
->method('bind')
208+
->with('elsa', 'test1234A$');
160209
$ldap
161210
->expects($this->once())
162211
->method('query')
163212
->will($this->returnValue($query))
164213
;
165214
$userChecker = $this->getMockBuilder(UserCheckerInterface::class)->getMock();
166215

167-
$provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap);
216+
$provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap, '{username}', true, 'elsa', 'test1234A$');
168217
$provider->setQueryString('{username}bar');
169218
$reflection = new \ReflectionMethod($provider, 'checkAuthentication');
170219
$reflection->setAccessible(true);

0 commit comments

Comments
 (0)