8000 Show user account status errors · symfony/symfony@80060ea · GitHub
[go: up one dir, main page]
Skip to content

Commit 80060ea

Browse files
core23core23
committed
Show user account status errors
1 parent 7b41875 commit 80060ea

File tree

5 files changed

+61
-3
lines changed

5 files changed

+61
-3
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ public function getConfigTreeBuilder(): TreeBuilder
6161
->defaultValue(SessionAuthenticationStrategy::MIGRATE)
6262
->end()
6363
->booleanNode('hide_user_not_found')->defaultTrue()->end()
64+
->booleanNode('show_account_status_exceptions')->defaultFalse()->end()
6465
->booleanNode('erase_credentials')->defaultTrue()->end()
6566
->arrayNode('access_decision_manager')
6667
->addDefaultsIfNotSet()

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ public function load(array $configs, ContainerBuilder $container): void
155155
}
156156

157157
$container->setParameter('security.authentication.hide_user_not_found', $config['hide_user_not_found']);
158+
$container->setParameter('security.authentication.show_account_status_exceptions', $config['show_account_status_exceptions']);
158159

159160
if (class_exists(Application::class)) {
160161
$loader->load('debug_console.php');

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
param('security.authentication.manager.erase_credentials'),
4545
param('security.authentication.hide_user_not_found'),
4646
abstract_arg('required badges'),
47+
param('security.authentication.show_account_status_exceptions'),
4748
])
4849
->tag('monolog.logger', ['channel' => 'security'])
4950

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public function __construct(
5858
private bool $eraseCredentials = true,
5959
private bool $hideUserNotFoundExceptions = true,
6060
private array $requiredBadges = [],
61+
private bool $showAccountStatusExceptions = false,
6162
) {
6263
}
6364

@@ -243,7 +244,7 @@ private function handleAuthenticationFailure(AuthenticationException $authentica
243244

244245
// Avoid leaking error details in case of invalid user (e.g. user not found or invalid account status)
245246
// to prevent user enumeration via response content comparison
246-
if ($this->hideUserNotFoundExceptions && ($authenticationException instanceof UserNotFoundException || ($authenticationException instanceof AccountStatusException && !$authenticationException instanceof CustomUserMessageAccountStatusException))) {
247+
if ($this->isFilteredException($authenticationException)) {
247248
$authenticationException = new BadCredentialsException('Bad credentials.', 0, $authenticationException);
248249
}
249250

@@ -257,4 +258,21 @@ private function handleAuthenticationFailure(AuthenticationException $authentica
257258
// returning null is ok, it means they want the request to continue
258259
return $loginFailureEvent->getResponse();
259260
}
261+
262+
private function isFilteredException(AuthenticationException $exception): bool
263+
{
264+
if (!$this->hideUserNotFoundExceptions) {
265+
return false;
266+
}
267+
268+
if ($exception instanceof UserNotFoundException) {
269+
return true;
270+
}
271+
272+
if ($this->showAccountStatusExceptions) {
273+
return false;
274+
}
275+
276+
return $exception instanceof AccountStatusException && !$exception instanceof CustomUserMessageAccountStatusException;
277+
}
260278
}

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

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2323
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2424
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
25+
use Symfony\Component\Security\Core\Exception\LockedException;
2526
use Symfony\Component\Security\Core\Exception\UserNotFoundException;
2627
use Symfony\Component\Security\Core\User\InMemoryUser;
2728
use Symfony\Component\Security\Http\Authentication\AuthenticatorManager;
@@ -322,6 +323,42 @@ public function testAuthenticateRequestHidesInvalidUserExceptions()
322323
$this->assertSame($this->response, $response);
323324
}
324325

326+
public function testAuthenticateRequestShowsAccountStatusException()
327+
{
328+
$invalidUserException = new LockedException();
329+
$authenticator = $this->createMock(TestInteractiveAuthenticator::class);
330+
$this->request->attributes->set('_security_authenticators', [$authenticator]);
331+
332+
$authenticator->expects($this->any())->method('authenticate')->willThrowException($invalidUserException);
333+
334+
$authenticator->expects($this->any())
335+
->method('onAuthenticationFailure')
336+
->with($this->equalTo($this->request), $this->callback(fn ($e) => $e === $invalidUserException))
337+
->willReturn($this->response);
338+
339+
$manager = $this->createManager([$authenticator], showAccountStatusExceptions: true);
340+
$response = $manager->authenticateRequest($this->request);
341+
$this->assertSame($this->response, $response);
342+
}
343+
344+
public function testAuthenticateRequestHidesInvalidAccountStatusExceptiot()
345+
{
346+
$invalidUserException = new LockedException();
347+
$authenticator = $this->createMock(TestInteractiveAuthenticator::class);
348+
$this->request->attributes->set('_security_authenticators', [$authenticator]);
349+
350+
$authenticator->expects($this->any())->method('authenticate')->willThrowException($invalidUserException);
351+
352+
$authenticator->expects($this->any())
353+
->method('onAuthenticationFailure')
354+
->with($this->equalTo($this->request), $this->callback(fn ($e) => $e instanceof BadCredentialsException && $invalidUserException === $e->getPrevious()))
355+
->willReturn($this->response);
356+
357+
$manager = $this->createManager([$authenticator]);
358+
$response = $manager->authenticateRequest($this->request);
359+
$this->assertSame($this->response, $response);
360+
}
361+
325362
public function testLogsUseTheDecoratedAuthenticatorWhenItIsTraceable()
326363
{
327364
$authenticator = $this->createMock(TestInteractiveAuthenticator::class);
@@ -373,9 +410,9 @@ private static function createDummySupportsAuthenticator(?bool $supports = true)
373410
return new DummySupportsAuthenticator($supports);
374411
}
375412

376-
private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = true, array $requiredBadges = [], ?LoggerInterface $logger = null)
413+
private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = true, array $requiredBadges = [], ?LoggerInterface $logger = null, bool $showAccountStatusExceptions = false)
377414
{
378-
return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, $logger, $eraseCredentials, true, $requiredBadges);
415+
return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, $logger, $eraseCredentials, true, $requiredBadges, showAccountStatusExceptions: $showAccountStatusExceptions);
379416
}
380417
}
381418

0 commit comments

Comments
 (0)
0