[Ldap][Security] LdapBindAuthenticationProvider does not bind before search query

This commit is contained in:
Amrouche Hamza 2019-05-21 08:08:02 +02:00 committed by Fabien Potencier
parent 950db8e85b
commit cb2d97f92b
7 changed files with 88 additions and 3 deletions

View File

@ -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
-----

View File

@ -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()
;
}

View File

@ -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()
;
}

View File

@ -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()
;
}

View File

@ -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>

View File

@ -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()) {

View File

@ -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);