feature #31560 [Ldap][Security] LdapBindAuthenticationProvider does not bind before search query (Simperfit)
This PR was merged into the 4.4 branch.
Discussion
----------
[Ldap][Security] LdapBindAuthenticationProvider does not bind before search query
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yesish
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks? | no <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass? | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | #24210 <!-- #-prefixed issue number(s), if any -->
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.
Additionally (see https://symfony.com/roadmap):
- Bug fixes must be submitted against the lowest maintained branch where they apply
(lowest branches are regularly merged to upper ones so they get the fixes too).
- Features and deprecations must be submitted against the master branch.
-->
This is tagged as a bug but since we need to add new params to the differents login providers and a deprecation I think we need to consider it as a new feature, maybe this could go in 3.4 too but without the deprecation.
This allow using a searchDn and a sarchPassword when passing a queryString in order to do the query authenticated.
Commits
-------
cb2d97f92b
[Ldap][Security] LdapBindAuthenticationProvider does not bind before search query
This commit is contained in:
commit
3c9ff1f8ac
@ -1,6 +1,10 @@
|
||||
CHANGELOG
|
||||
=========
|
||||
|
||||
4.4.0
|
||||
|
||||
* Deprecated the usage of "query_string" without a "search_dn" and a "search_password" config key in Ldap factories.
|
||||
|
||||
4.3.0
|
||||
-----
|
||||
|
||||
|
@ -34,9 +34,14 @@ class FormLoginLdapFactory extends FormLoginFactory
|
||||
->replaceArgument(2, $id)
|
||||
->replaceArgument(3, new Reference($config['service']))
|
||||
->replaceArgument(4, $config['dn_string'])
|
||||
->replaceArgument(5, $config['search_dn'])
|
||||
->replaceArgument(6, $config['search_password'])
|
||||
;
|
||||
|
||||
if (!empty($config['query_string'])) {
|
||||
if ('' === $config['search_dn'] || '' === $config['search_password']) {
|
||||
@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);
|
||||
}
|
||||
$definition->addMethodCall('setQueryString', [$config['query_string']]);
|
||||
}
|
||||
|
||||
@ -52,6 +57,8 @@ class FormLoginLdapFactory extends FormLoginFactory
|
||||
->scalarNode('service')->defaultValue('ldap')->end()
|
||||
->scalarNode('dn_string')->defaultValue('{username}')->end()
|
||||
->scalarNode('query_string')->end()
|
||||
->scalarNode('search_dn')->defaultValue('')->end()
|
||||
->scalarNode('search_password')->defaultValue('')->end()
|
||||
->end()
|
||||
;
|
||||
}
|
||||
|
@ -35,12 +35,17 @@ class HttpBasicLdapFactory extends HttpBasicFactory
|
||||
->replaceArgument(2, $id)
|
||||
->replaceArgument(3, new Reference($config['service']))
|
||||
->replaceArgument(4, $config['dn_string'])
|
||||
->replaceArgument(5, $config['search_dn'])
|
||||
->replaceArgument(6, $config['search_password'])
|
||||
;
|
||||
|
||||
// entry point
|
||||
$entryPointId = $this->createEntryPoint($container, $id, $config, $defaultEntryPoint);
|
||||
|
||||
if (!empty($config['query_string'])) {
|
||||
if ('' === $config['search_dn'] || '' === $config['search_password']) {
|
||||
@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);
|
||||
}
|
||||
$definition->addMethodCall('setQueryString', [$config['query_string']]);
|
||||
}
|
||||
|
||||
@ -62,6 +67,8 @@ class HttpBasicLdapFactory extends HttpBasicFactory
|
||||
->scalarNode('service')->defaultValue('ldap')->end()
|
||||
->scalarNode('dn_string')->defaultValue('{username}')->end()
|
||||
->scalarNode('query_string')->end()
|
||||
->scalarNode('search_dn')->defaultValue('')->end()
|
||||
->scalarNode('search_password')->defaultValue('')->end()
|
||||
->end()
|
||||
;
|
||||
}
|
||||
|
@ -36,9 +36,14 @@ class JsonLoginLdapFactory extends JsonLoginFactory
|
||||
->replaceArgument(2, $id)
|
||||
->replaceArgument(3, new Reference($config['service']))
|
||||
->replaceArgument(4, $config['dn_string'])
|
||||
->replaceArgument(5, $config['search_dn'])
|
||||
->replaceArgument(6, $config['search_password'])
|
||||
;
|
||||
|
||||
if (!empty($config['query_string'])) {
|
||||
if ('' === $config['search_dn'] || '' === $config['search_password']) {
|
||||
@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);
|
||||
}
|
||||
$definition->addMethodCall('setQueryString', [$config['query_string']]);
|
||||
}
|
||||
|
||||
@ -54,6 +59,8 @@ class JsonLoginLdapFactory extends JsonLoginFactory
|
||||
->scalarNode('service')->defaultValue('ldap')->end()
|
||||
->scalarNode('dn_string')->defaultValue('{username}')->end()
|
||||
->scalarNode('query_string')->end()
|
||||
->scalarNode('search_dn')->defaultValue('')->end()
|
||||
->scalarNode('search_password')->defaultValue('')->end()
|
||||
->end()
|
||||
;
|
||||
}
|
||||
|
@ -195,6 +195,8 @@
|
||||
<argument /> <!-- UserChecker -->
|
||||
<argument /> <!-- Provider-shared Key -->
|
||||
<argument /> <!-- LDAP -->
|
||||
<argument /> <!-- search dn -->
|
||||
<argument /> <!-- search password -->
|
||||
<argument /> <!-- Base DN -->
|
||||
<argument>%security.authentication.hide_user_not_found%</argument>
|
||||
</service>
|
||||
|
@ -34,14 +34,18 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider
|
||||
private $ldap;
|
||||
private $dnString;
|
||||
private $queryString;
|
||||
private $searchDn;
|
||||
private $searchPassword;
|
||||
|
||||
public function __construct(UserProviderInterface $userProvider, UserCheckerInterface $userChecker, string $providerKey, LdapInterface $ldap, string $dnString = '{username}', bool $hideUserNotFoundExceptions = true)
|
||||
public function __construct(UserProviderInterface $userProvider, UserCheckerInterface $userChecker, string $providerKey, LdapInterface $ldap, string $dnString = '{username}', bool $hideUserNotFoundExceptions = true, string $searchDn = '', string $searchPassword = '')
|
||||
{
|
||||
parent::__construct($userChecker, $providerKey, $hideUserNotFoundExceptions);
|
||||
|
||||
$this->userProvider = $userProvider;
|
||||
$this->ldap = $ldap;
|
||||
$this->dnString = $dnString;
|
||||
$this->searchDn = $searchDn;
|
||||
$this->searchPassword = $searchPassword;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -82,6 +86,11 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider
|
||||
$username = $this->ldap->escape($username, '', LdapInterface::ESCAPE_DN);
|
||||
|
||||
if ($this->queryString) {
|
||||
if ('' !== $this->searchDn && '' !== $this->searchPassword) {
|
||||
$this->ldap->bind($this->searchDn, $this->searchPassword);
|
||||
} else {
|
||||
@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);
|
||||
}
|
||||
$query = str_replace('{username}', $username, $this->queryString);
|
||||
$result = $this->ldap->query($this->dnString, $query)->execute();
|
||||
if (1 !== $result->count()) {
|
||||
|
@ -123,6 +123,10 @@ class LdapBindAuthenticationProviderTest extends TestCase
|
||||
->with('foo', '')
|
||||
->willReturn('foo')
|
||||
;
|
||||
$ldap
|
||||
->expects($this->at(1))
|
||||
->method('bind')
|
||||
->with('elsa', 'test1234A$');
|
||||
$ldap
|
||||
->expects($this->once())
|
||||
->method('query')
|
||||
@ -131,7 +135,48 @@ class LdapBindAuthenticationProviderTest extends TestCase
|
||||
;
|
||||
$userChecker = $this->getMockBuilder(UserCheckerInterface::class)->getMock();
|
||||
|
||||
$provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap);
|
||||
$provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap, '{username}', true, 'elsa', 'test1234A$');
|
||||
$provider->setQueryString('{username}bar');
|
||||
$reflection = new \ReflectionMethod($provider, 'checkAuthentication');
|
||||
$reflection->setAccessible(true);
|
||||
|
||||
$reflection->invoke($provider, new User('foo', null), new UsernamePasswordToken('foo', 'bar', 'key'));
|
||||
}
|
||||
|
||||
public function testQueryWithUserForDn()
|
||||
{
|
||||
$userProvider = $this->getMockBuilder(UserProviderInterface::class)->getMock();
|
||||
|
||||
$collection = new \ArrayIterator([new Entry('')]);
|
||||
|
||||
$query = $this->getMockBuilder(QueryInterface::class)->getMock();
|
||||
$query
|
||||
->expects($this->once())
|
||||
->method('execute')
|
||||
->will($this->returnValue($collection))
|
||||
;
|
||||
|
||||
$ldap = $this->getMockBuilder(LdapInterface::class)->getMock();
|
||||
$ldap
|
||||
->expects($this->once())
|
||||
->method('escape')
|
||||
->with('foo', '')
|
||||
->will($this->returnValue('foo'))
|
||||
;
|
||||
$ldap
|
||||
->expects($this->at(1))
|
||||
->method('bind')
|
||||
->with('elsa', 'test1234A$');
|
||||
$ldap
|
||||
->expects($this->once())
|
||||
->method('query')
|
||||
->with('{username}', 'foobar')
|
||||
->will($this->returnValue($query))
|
||||
;
|
||||
|
||||
$userChecker = $this->getMockBuilder(UserCheckerInterface::class)->getMock();
|
||||
|
||||
$provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap, '{username}', true, 'elsa', 'test1234A$');
|
||||
$provider->setQueryString('{username}bar');
|
||||
$reflection = new \ReflectionMethod($provider, 'checkAuthentication');
|
||||
$reflection->setAccessible(true);
|
||||
@ -157,6 +202,10 @@ class LdapBindAuthenticationProviderTest extends TestCase
|
||||
;
|
||||
|
||||
$ldap = $this->getMockBuilder(LdapInterface::class)->getMock();
|
||||
$ldap
|
||||
->expects($this->at(1))
|
||||
->method('bind')
|
||||
->with('elsa', 'test1234A$');
|
||||
$ldap
|
||||
->expects($this->once())
|
||||
->method('query')
|
||||
@ -164,7 +213,7 @@ class LdapBindAuthenticationProviderTest extends TestCase
|
||||
;
|
||||
$userChecker = $this->getMockBuilder(UserCheckerInterface::class)->getMock();
|
||||
|
||||
$provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap);
|
||||
$provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap, '{username}', true, 'elsa', 'test1234A$');
|
||||
$provider->setQueryString('{username}bar');
|
||||
$reflection = new \ReflectionMethod($provider, 'checkAuthentication');
|
||||
$reflection->setAccessible(true);
|
||||
|
Reference in New Issue
Block a user