8000 [Security][Guard] Prevent user enumeration via response content · symfony/symfony@f012eee · GitHub
[go: up one dir, main page]
Skip to content

Commit f012eee

Browse files
chalasrchalasr
committed
[Security][Guard] Prevent user enumeration via response content
1 parent 83093d5 commit f012eee

File tree

5 files changed

+71
-7
lines changed

5 files changed

+71
-7
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
<argument type="service" id="security.authentication.session_strategy" />
1818
</call>
1919
</service>
20-
20+
2121
<service id="Symfony\Component\Security\Guard\GuardAuthenticatorHandler" alias="security.authentication.guard_handler" />
2222

2323
<!-- See GuardAuthenticationFactory -->
@@ -41,6 +41,7 @@
4141
<argument /> <!-- Provider-shared Key -->
4242
<argument /> <!-- Authenticator -->
4343
<argument type="service" id="logger" on-invalid="null" />
44+
<argument>%security.authentication.hide_user_not_found%</argument>
4445
</service>
4546
</services>
4647
</container>

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1515
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
16+
use Symfony\Component\Security\Core\Exception\AccountStatusException;
1617
use Symfony\Component\Security\Core\Exception\AuthenticationException;
1718
use Symfony\Component\Security\Core\Exception\AuthenticationServiceException;
1819
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
@@ -83,7 +84,7 @@ public function authenticate(TokenInterface $token)
8384
$this->userChecker->checkPreAuth($user);
8485
$this->checkAuthentication($user, $token);
8586
$this->userChecker->checkPostAuth($user);
86-
} catch (BadCredentialsException $e) {
87+
} catch (AccountStatusException $e) {
8788
if ($this->hideUserNotFoundExceptions) {
8889
throw new BadCredentialsException('Bad credentials.', 0, $e);
8990
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public function testAuthenticateWhenProviderDoesNotReturnAnUserInterface()
7979

8080
public function testAuthenticateWhenPreChecksFails()
8181
{
82-
$this->expectException('Symfony\Component\Security\Core\Exception\CredentialsExpiredException');
82+
$this->expectException(BadCredentialsException::class);
8383
$userChecker = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserCheckerInterface')->getMock();
8484
$userChecker->expects($this->once())
8585
->method('checkPreAuth')
@@ -97,7 +97,7 @@ public function testAuthenticateWhenPreChecksFails()
9797

9898
public function testAuthenticateWhenPostChecksFails()
9999
{
100-
$this->expectException('Symfony\Component\Security\Core\Exception\AccountExpiredException');
100+
$this->expectException(BadCredentialsException::class);
101101
$userChecker = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserCheckerInterface')->getMock();
102102
$userChecker->expects($this->once())
103103
->method('checkPostAuth')
@@ -116,15 +116,15 @@ public function testAuthenticateWhenPostChecksFails()
116116
public function testAuthenticateWhenPostCheckAuthenticationFails()
117117
{
118118
$this->expectException('Symfony\Component\Security\Core\Exception\BadCredentialsException');
119-
$this->expectExceptionMessage('Bad credentials');
119+
$this->expectExceptionMessage('Bad credentials.');
120120
$provider = $this->getProvider();
121121
$provider->expects($this->once())
122122
->method('retrieveUser')
123123
->willReturn($this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock())
124124
;
125125
$provider->expects($this->once())
126126
->method('checkAuthentication')
127-
->willThrowException(new BadCredentialsException())
127+
->willThrowException(new CredentialsExpiredException())
128128
;
129129

130130
$provider->authenticate($this->getSupportedToken());

‎src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@
1717
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
1818
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
1919
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
20+
use Symfony\Component\Security\Core\Exception\AccountStatusException;
2021
use Symfony\Component\Security\Core\Exception\AuthenticationException;
22+
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
23+
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
2124
use Symfony\Component\Security\Guard\AbstractGuardAuthenticator;
2225
use Symfony\Component\Security\Guard\AuthenticatorInterface;
2326
use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
@@ -40,6 +43,7 @@ class GuardAuthenticationListener implements ListenerInterface
4043
private $guardAuthenticators;
4144
private $logger;
4245
private $rememberMeServices;
46+
private $hideUserNotFoundExceptions;
4347

4448
/**
4549
* @param GuardAuthenticatorHandler $guardHandler The Guard handler
@@ -48,7 +52,7 @@ class GuardAuthenticationListener implements ListenerInterface
4852
* @param iterable|AuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider
4953
* @param LoggerInterface $logger A LoggerInterface instance
5054
*/
51-
public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null)
55+
public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null, $hideUserNotFoundExceptions = true)
5256
{
5357
if (empty($providerKey)) {
5458
throw new \InvalidArgumentException('$providerKey must not be empty.');
@@ -59,6 +63,7 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat
5963
$this->providerKey = $providerKey;
6064
$this->guardAuthenticators = $guardAuthenticators;
6165
$this->logger = $logger;
66+
$this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions;
6267
}
6368

6469
/**
@@ -163,6 +168,12 @@ private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorIn
163168
$this->logger->info('Guard authentication failed.', ['exception' => $e, 'authenticator' => \get_class($guardAuthenticator)]);
164169
}
165170

171+
// Avoid leaking error details in case of invalid user (e.g. user not found or invalid account status)
172+
// to prevent user enumeration via response content
173+
if ($this->hideUserNotFoundExceptions && ($e instanceof UsernameNotFoundException || $e instanceof AccountStatusException)) {
174+
$e = new BadCredentialsException('Bad credentials.', 0, $e);
175+
}
176+
166177
$response = $this->guardHandler->handleAuthenticationFailure($e, $request, $guardAuthenticator, $this->providerKey);
167178

168179
if ($response instanceof Response) {

‎src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
use Symfony\Component\HttpFoundation\Response;
1717
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1818
use Symfony\Component\Security\Core\Exception\AuthenticationException;
19+
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
20+
use Symfony\Component\Security\Core\Exception\LockedException;
21+
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
1922
use Symfony\Component\Security\Guard\AbstractGuardAuthenticator;
2023
use Symfony\Component\Security\Guard\AuthenticatorInterface;
2124
use Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener;
@@ -208,6 +211,54 @@ public function testHandleCatchesAuthenticationException()
208211
$listener->handle($this->event);
209212
}
210213

214+
/**
215+
* @dataProvider exceptionsToHide
216+
*/
217+
public function testHandleHidesInvalidUserExceptions(AuthenticationException $exceptionToHide)
218+
{
219+
$authenticator = $this->createMock(AuthenticatorInterface::class);
220+
$providerKey = 'my_firewall2';
221+
222+
$authenticator
223+
->expects($this->once())
224+
->method('supports')
225+
->willReturn(true);
226+
$authenticator
227+< 527 div class="diff-text-inner"> ->expects($this->once())
228+
->method('getCredentials')
229+
->willReturn(['username' => 'robin', 'password' => 'hood']);
230+
231+
$this->authenticationManager
232+
->expects($this->once())
233+
->method('authenticate')
234+
->willThrowException($exceptionToHide);
235+
236+
$this->guardAuthenticatorHandler
237+
->expects($this->once())
238+
->method('handleAuthenticationFailure')
239+
->with($this->callback(function ($e) use ($exceptionToHide) {
240+
return $e instanceof BadCredentialsException && $exceptionToHide === $e->getPrevious();
241+
}), $this->request, $authenticator, $providerKey);
242+
243+
$listener = new GuardAuthenticationListener(
244+
$this->guardAuthenticatorHandler,
245+
$this->authenticationManager,
246+
$providerKey,
247+
[$authenticator],
248+
$this->logger
249+
);
250+
251+
$listener->handle($this->event);
252+
}
253+
254+
public function exceptionsToHide()
255+
{
256+
return [
257+
[new UsernameNotFoundException()],
258+
[new LockedException()],
259+
];
260+
}
261+
211262
/**
212263
* @group legacy
213264
*/

0 commit comments

Comments
 (0)
0