Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 8fb0ed7

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
Merge branch '5.2' into 5.x
* 5.2: [CI][Psalm] Install stable/released PHPUnit [Security] Add missing Finnish translations [Security][Guard] Prevent user enumeration via response content
2 parents 2ac23c6 + 293919f commit 8fb0ed7
Copy full SHA for 8fb0ed7

File tree

14 files changed

+118
-12
lines changed
Filter options

14 files changed

+118
-12
lines changed

‎.github/workflows/psalm.yml

Copy file name to clipboardExpand all lines: .github/workflows/psalm.yml
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ jobs:
3939
run: |
4040
echo "::group::modify composer.json"
4141
composer remove --no-update --no-interaction symfony/phpunit-bridge
42-
composer require --no-update psalm/phar phpunit/phpunit php-http/discovery psr/event-dispatcher
42+
composer require --no-update psalm/phar phpunit/phpunit:@stable php-http/discovery psr/event-dispatcher
4343
echo "::endgroup::"
4444
echo "::group::composer update"
4545
composer update --no-progress --ansi

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ private function createFirewall(ContainerBuilder $container, string $id, array $
502502
->replaceArgument(0, $authenticators)
503503
->replaceArgument(2, new Reference($firewallEventDispatcherId))
504504
->replaceArgument(3, $id)
505-
->replaceArgument(6, $firewall['required_badges'] ?? [])
505+
->replaceArgument(7, $firewall['required_badges'] ?? [])
506506
->addTag('monolog.logger', ['channel' => 'security'])
507507
;
508508

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Resources/config/guard.php
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
abstract_arg('Provider-shared Key'),
4646
abstract_arg('Authenticators'),
4747
service('logger')->nullOnInvalid(),
48+
param('security.authentication.hide_user_not_found'),
4849
])
4950
->tag('monolog.logger', ['channel' => 'security'])
5051
;

‎src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
abstract_arg('provider key'),
4545
service('logger')->nullOnInvalid(),
4646
param('security.authentication.manager.erase_credentials'),
47+
param('security.authentication.hide_user_not_found'),
4748
abstract_arg('required badges'),
4849
])
4950
->tag('monolog.logger', ['channel' => 'security'])

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public function testAuthenticatorManager()
4343
$this->assertEquals(AuthenticatorManager::class, $authenticatorManager->getClass());
4444

4545
// required badges
46-
$this->assertEquals([CsrfTokenBadge::class, RememberMeBadge::class], $authenticatorManager->getArgument(6));
46+
$this->assertEquals([CsrfTokenBadge::class, RememberMeBadge::class], $authenticatorManager->getArgument(7));
4747

4848
// login link
4949
$expiredStorage = $container->getDefinition($expiredStorageId = 'security.authenticator.expired_login_link_storage.main');

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/AuthenticatorTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/Functional/AuthenticatorTest.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public function testFirewallUserProvider($email, $withinFirewall)
4040
if ($withinFirewall) {
4141
$this->assertJsonStringEqualsJsonString('{"email":"'.$email.'"}', $client->getResponse()->getContent());
4242
} else {
43-
$this->assertJsonStringEqualsJsonString('{"error":"Username could not be found."}', $client->getResponse()->getContent());
43+
$this->assertJsonStringEqualsJsonString('{"error":"Invalid credentials."}', $client->getResponse()->getContent());
4444
}
4545
}
4646

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public function testLoginThrottling()
142142

143143
break;
144144
case 2: // Third attempt with unexisting username
145-
$this->assertStringContainsString('Username could not be found.', $text, 'Invalid response on 3rd attempt');
145+
$this->assertStringContainsString('Invalid credentials.', $text, 'Invalid response on 3rd attempt');
146146

147147
break;
148148
case 3: // Fourth attempt : still login throttling !

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
1515
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1616
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
17+
use Symfony\Component\Security\Core\Exception\AccountStatusException;
1718
use Symfony\Component\Security\Core\Exception\AuthenticationException;
1819
use Symfony\Component\Security\Core\Exception\AuthenticationServiceException;
1920
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
@@ -79,7 +80,7 @@ public function authenticate(TokenInterface $token)
7980
$this->userChecker->checkPreAuth($user);
8081
$this->checkAuthentication($user, $token);
8182
$this->userChecker->checkPostAuth($user);
82-
} catch (BadCredentialsException $e) {
83+
} catch (AccountStatusException $e) {
8384
if ($this->hideUserNotFoundExceptions) {
8485
throw new BadCredentialsException('Bad credentials.', 0, $e);
8586
}

‎src/Symfony/Component/Security/Core/Resources/translations/security.fi.xlf

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Resources/translations/security.fi.xlf
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@
7070
<source>Invalid or expired login link.</source>
7171
<target>Virheellinen tai vanhentunut kirjautumislinkki.</target>
7272
</trans-unit>
73+
<trans-unit id="19">
74+
<source>Too many failed login attempts, please try again in %minutes% minute.</source>
75+
<target>Liian monta epäonnistunutta kirjautumisyritystä, yritä uudelleen %minutes% minuutin kuluttua.</target>
76+
</trans-unit>
77+
<trans-unit id="20">
78+
<source>Too many failed login attempts, please try again in %minutes% minutes.</source>
79+
<target>Liian monta epäonnistunutta kirjautumisyritystä, yritä uudelleen %minutes% minuutin kuluttua.</target>
80+
</trans-unit>
7381
</body>
7482
</file>
7583
</xliff>

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public function testAuthenticateWhenProviderDoesNotReturnAnUserInterface()
8383

8484
public function testAuthenticateWhenPreChecksFails()
8585
{
86-
$this->expectException(CredentialsExpiredException::class);
86+
$this->expectException(BadCredentialsException::class);
8787
$userChecker = $this->createMock(UserCheckerInterface::class);
8888
$userChecker->expects($this->once())
8989
->method('checkPreAuth')
@@ -101,7 +101,7 @@ public function testAuthenticateWhenPreChecksFails()
101101

102102
public function testAuthenticateWhenPostChecksFails()
103103
{
104-
$this->expectException(AccountExpiredException::class);
104+
$this->expectException(BadCredentialsException::class);
105105
$userChecker = $this->createMock(UserCheckerInterface::class);
106106
$userChecker->expects($this->once())
107107
->method('checkPostAuth')
@@ -128,7 +128,7 @@ public function testAuthenticateWhenPostCheckAuthenticationFails()
128128
;
129129
$provider->expects($this->once())
130130
->method('checkAuthentication')
131-
->willThrowException(new BadCredentialsException())
131+
->willThrowException(new CredentialsExpiredException())
132132
;
133133

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

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php
+13-1Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@
1717
use Symfony\Component\HttpKernel\Event\RequestEvent;
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\CustomUserMessageAccountStatusException;
24+
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
2125
use Symfony\Component\Security\Guard\AuthenticatorInterface;
2226
use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
2327
use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken;
@@ -40,12 +44,13 @@ class GuardAuthenticationListener extends AbstractListener
4044
private $guardAuthenticators;
4145
private $logger;
4246
private $rememberMeServices;
47+
private $hideUserNotFoundExceptions;
4348

4449
/**
4550
* @param string $providerKey The provider (i.e. firewall) key
4651
* @param iterable|AuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider
4752
*/
48-
public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, string $providerKey, iterable $guardAuthenticators, LoggerInterface $logger = null)
53+
public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, string $providerKey, iterable $guardAuthenticators, LoggerInterface $logger = null, bool $hideUserNotFoundExceptions = true)
4954
{
5055
if (empty($providerKey)) {
5156
throw new \InvalidArgumentException('$providerKey must not be empty.');
@@ -56,6 +61,7 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat
5661
$this->providerKey = $providerKey;
5762
$this->guardAuthenticators = $guardAuthenticators;
5863
$this->logger = $logger;
64+
$this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions;
5965
}
6066

6167
/**
@@ -160,6 +166,12 @@ private function executeGuardAuthenticator(string $uniqueGuardKey, Authenticator
160166
$this->logger->info('Guard authentication failed.', ['exception' => $e, 'authenticator' => \get_class($guardAuthenticator)]);
161167
}
162168

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

165177
if ($response instanceof Response) {

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php
+51Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
use Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager;
2020
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2121
use Symfony\Component\Security\Core\Exception\AuthenticationException;
22+
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
23+
use Symfony\Component\Security\Core\Exception\LockedException;
24+
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
2225
use Symfony\Component\Security\Guard\AuthenticatorInterface;
2326
use Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener;
2427
use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
@@ -211,6 +214,54 @@ public function testHandleCatchesAuthenticationException()
211214
$listener($this->event);
212215
}
213216

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

‎src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php
+12-1Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@
1818
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1919
use Symfony\Component\Security\Core\AuthenticationEvents;
2020
use Symfony\Component\Security\Core\Event\AuthenticationSuccessEvent;
21+
use Symfony\Component\Security\Core\Exception\AccountStatusException;
2122
use Symfony\Component\Security\Core\Exception\AuthenticationException;
2223
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
24+
use Symfony\Component\Security\Core\Exception\CustomUserMessageAccountStatusException;
25+
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
2326
use Symfony\Component\Security\Core\User\UserInterface;
2427
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
2528
use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface;
@@ -48,19 +51,21 @@ class AuthenticatorManager implements AuthenticatorManagerInterface, UserAuthent
4851
private $eraseCredentials;
4952
private $logger;
5053
private $firewallName;
54+
private $hideUserNotFoundExceptions;
5155
private $requiredBadges;
5256

5357
/**
5458
* @param AuthenticatorInterface[] $authenticators
5559
*/
56-
public function __construct(iterable $authenticators, TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher, string $firewallName, ?LoggerInterface $logger = null, bool $eraseCredentials = true, array $requiredBadges = [])
60+
public function __construct(iterable $authenticators, TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher, string $firewallName, ?LoggerInterface $logger = null, bool $eraseCredentials = true, bool $hideUserNotFoundExceptions = true, array $requiredBadges = [])
5761
{
5862
$this->authenticators = $authenticators;
5963
$this->tokenStorage = $tokenStorage;
6064
$this->eventDispatcher = $eventDispatcher;
6165
$this->firewallName = $firewallName;
6266
$this->logger = $logger;
6367
$this->eraseCredentials = $eraseCredentials;
68+
$this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions;
6469
$this->requiredBadges = $requiredBadges;
6570
}
6671

@@ -251,6 +256,12 @@ private function handleAuthenticationFailure(AuthenticationException $authentica
251256
$this->logger->info('Authenticator failed.', ['exception' => $authenticationException, 'authenticator' => \get_class($authenticator)]);
252257
}
253258

259+
// Avoid leaking error details in case of invalid user (e.g. user not found or invalid account status)
260+
// to prevent user enumeration via response content comparison
261+
if ($this->hideUserNotFoundExceptions && ($authenticationException instanceof UsernameNotFoundException || ($authenticationException instanceof AccountStatusException && !$authenticationException instanceof CustomUserMessageAccountStatusException))) {
262+
$authenticationException = new BadCredentialsException('Bad credentials.', 0, $authenticationException);
263+
}
264+
254265
$response = $authenticator->onAuthenticationFailure($request, $authenticationException);
255266
if (null !== $response && null !== $this->logger) {
256267
$this->logger->debug('The "{authenticator}" authenticator set the failure response.', ['authenticator' => \get_class($authenticator)]);

‎src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerTest.php
+22-1Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2020
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2121
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
22+
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
2223
use Symfony\Component\Security\Core\User\InMemoryUser;
2324
use Symfony\Component\Security\Http\Authentication\AuthenticatorManager;
2425
use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface;
@@ -268,6 +269,26 @@ public function testInteractiveAuthenticator()
268269
$this->assertSame($this->response, $response);
269270
}
270271

272+
public function testAuthenticateRequestHidesInvalidUserExceptions()
273+
{
274+
$invalidUserException = new UsernameNotFoundException();
275+
$authenticator = $this->createMock(InteractiveAuthenticatorInterface::class);
276+
$this->request->attributes->set('_security_authenticators', [$authenticator]);
277+
278+
$authenticator->expects($this->any())->method('authenticate')->willThrowException($invalidUserException);
279+
280+
$authenticator->expects($this->any())
281+
->method('onAuthenticationFailure')
282+
->with($this->equalTo($this->request), $this->callback(function ($e) use ($invalidUserException) {
283+
return $e instanceof BadCredentialsException && $invalidUserException === $e->getPrevious();
284+
}))
285+
->willReturn($this->response);
286+
287+
$manager = $this->createManager([$authenticator]);
288+
$response = $manager->authenticateRequest($this->request);
289+
$this->assertSame($this->response, $response);
290+
}
291+
271292
private function createAuthenticator($supports = true)
272293
{
273294
$authenticator = $this->createMock(InteractiveAuthenticatorInterface::class);
@@ -278,6 +299,6 @@ private function createAuthenticator($supports = true)
278299

279300
private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = true, array $requiredBadges = [])
280301
{
281-
return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, null, $eraseCredentials, $requiredBadges);
302+
return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, null, $eraseCredentials, true, $requiredBadges);
282303
}
283304
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.