8000 [Ldap] LDAP authentication should return a meaningful error when the … · symfony/symfony@ad2cc0e · GitHub
[go: up one dir, main page]

Skip to content

Commit ad2cc0e

Browse files
Jayfrownfabpot
authored andcommitted
[Ldap] LDAP authentication should return a meaningful error when the LDAP server is unavailable
1 parent 1a08c65 commit ad2cc0e

File tree

6 files changed

+47
-15
lines changed

6 files changed

+47
-15
lines changed

src/Symfony/Component/Ldap/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
CHANGELOG
22
=========
33

4+
6.1
5+
---
6+
7+
* Return a 500 Internal Server Error if LDAP server in unavailable during user enumeration / authentication
8+
* Introduce `InvalidSearchCredentialsException` to differentiate between cases where user-provided credentials are invalid and cases where the configured search credentials are invalid
9+
410
6.0
511
---
612

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
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+
* InvalidSearchCredentialsException is thrown if binding to ldap fails when
16+
* using the configured search_dn and search_password.
17+
*
18+
* @author Jeroen de Boer <info@jayfrown.nl>
19+
*/
20+
class InvalidSearchCredentialsException extends InvalidCredentialsException
21+
{
22+
}

src/Symfony/Component/Ldap/Security/CheckLdapCredentialsListener.php

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

1414
use Psr\Container\ContainerInterface;
1515
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
16-
use Symfony\Component\Ldap\Exception\ConnectionException;
16+
use Symfony\Component\Ldap\Exception\InvalidCredentialsException;
17+
use Symfony\Component\Ldap\Exception\InvalidSearchCredentialsException;
1718
use Symfony\Component\Ldap\LdapInterface;
1819
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
1920
use Symfony\Component\Security\Core\Exception\LogicException;
@@ -74,7 +75,11 @@ public function onCheckPassport(CheckPassportEvent $event)
7475
try {
7576
if ($ldapBadge->getQueryString()) {
7677
if ('' !== $ldapBadge->getSearchDn() && '' !== $ldapBadge->getSearchPassword()) {
77-
$ldap->bind($ldapBadge->getSearchDn(), $ldapBadge->getSearchPassword());
78+
try {
79+
$ldap->bind($ldapBadge->getSearchDn(), $ldapBadge->getSearchPassword());
80+
} catch (InvalidCredentialsException $e) {
81+
throw new InvalidSearchCredentialsException();
82+
}
7883
} else {
7984
throw new LogicException('Using the "query_string" config without using a "search_dn" and a "search_password" is not supported.');
8085
}
@@ -92,7 +97,7 @@ public function onCheckPassport(CheckPassportEvent $event)
9297
}
9398

9499
$ldap->bind($dn, $presentedPassword);
95-
} catch (ConnectionException $e) {
100+
} catch (InvalidCredentialsException $e) {
96101
throw new BadCredentialsException('The presented password is invalid.');
97102
}
98103

src/Symfony/Component/Ldap/Security/LdapUserProvider.php

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

1414
use Symfony\Component\Ldap\Entry;
15-
use Symfony\Component\Ldap\Exception\ConnectionException;
1615
use Symfony\Component\Ldap\Exception\ExceptionInterface;
16+
use Symfony\Component\Ldap\Exception\InvalidCredentialsException;
17+
use Symfony\Component\Ldap\Exception\InvalidSearchCredentialsException;
1718
use Symfony\Component\Ldap\LdapInterface;
1819
use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
1920
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
@@ -75,16 +76,14 @@ public function loadUserByIdentifier(string $identifier): UserInterface
7576
{
7677
try {
7778
$this->ldap->bind($this->searchDn, $this->searchPassword);
78-
$identifier = $this->ldap->escape($identifier, '', LdapInterface::ESCAPE_FILTER);
79-
$query = str_replace(['{username}', '{user_identifier}'], $identifier, $this->defaultSearch);
80-
$search = $this->ldap->query($this->baseDn, $query, ['filter' => 0 == \count($this->extraFields) ? '*' : $this->extraFields]);
81-
} catch (ConnectionException $e) {
82-
$e = new UserNotFoundException(sprintf('User "%s" not found.', $identifier), 0, $e);
83-
$e->setUserIdentifier($identifier);
84-
85-
throw $e;
79+
} catch (InvalidCredentialsException $e) {
80+
throw new InvalidSearchCredentialsException();
8681
}
8782

83+
$identifier = $this->ldap->escape($identifier, '', LdapInterface::ESCAPE_FILTER);
84+
$query = str_replace(['{username}', '{user_identifier}'], $identifier, $this->defaultSearch);
85+
$search = $this->ldap->query($this->baseDn, $query, ['filter' => 0 == \count($this->extraFields) ? '*' : $this->extraFields]);
86+
8887
$entries = $search->execute();
8988
$count = \count($entries);
9089

src/Symfony/Component/Ldap/Tests/Security/CheckLdapCredentialsListenerTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
use Symfony\Component\Ldap\Adapter\CollectionInterface;
1919
use Symfony\Component\Ldap\Adapter\QueryInterface;
2020
use Symfony\Component\Ldap\Entry;
21-
use Symfony\Component\Ldap\Exception\ConnectionException;
21+
use Symfony\Component\Ldap\Exception\InvalidCredentialsException;
2222
use Symfony\Component\Ldap\LdapInterface;
2323
use Symfony\Component\Ldap\Security\CheckLdapCredentialsListener;
2424
use Symfony\Component\Ldap\Security\LdapBadge;
@@ -133,7 +133,7 @@ public function testBindFailureShouldThrowAnException()
133133
$this->expectExceptionMessage('The presented password is invalid.');
134134

135135
$this->ldap->method('escape')->willReturnArgument(0);
136-
$this->ldap->expects($this->any())->method('bind')->willThrowException(new ConnectionException());
136+
$this->ldap->expects($this->any())->method('bind')->willThrowException(new InvalidCredentialsException());
137137

138138
$listener = $this->createListener();
139139
$listener->onCheckPassport($this->createEvent());

src/Symfony/Component/Ldap/Tests/Security/LdapUserProviderTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class LdapUserProviderTest extends TestCase
2929
{
3030
public function testLoadUserByUsernameFailsIfCantConnectToLdap()
3131
{
32-
$this->expectException(UserNotFoundException::class);
32+
$this->expectException(ConnectionException::class);
3333

3434
$ldap = $this->createMock(LdapInterface::class);
3535
$ldap

0 commit comments

Comments
 (0)
0