8000 feature #58300 [Security][SecurityBundle] Show user account status er… · symfony/symfony@a7a32f7 · GitHub
[go: up one dir, main page]
Skip to content

Commit a7a32f7

Browse files
chalasrchalasr
committed
feature #58300 [Security][SecurityBundle] Show user account status errors (core23)
This PR was squashed before being merged into the 7.3 branch. Discussion ---------- [Security][SecurityBundle] Show user account status errors | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Fix #50028 | License | MIT According to the `hideUserNotFoundExceptions` flag, only `UserNotFoundException` should be hidden, but the implementation was also silencing account specific errors. To fix the issue in a BC way, a new `hide_account_status_exceptions` config key was introduced to show account exceptions. This key should be changed to **false** with an upcoming version (7.3 or 7.4?), so that the existing `hide_user_not_found` config key works as intended with the upcoming symfony 8 release. Commits ------- 5a11de5 [Security][SecurityBundle] Show user account status errors
2 parents eff9b52 + 5a11de5 commit a7a32f7

File tree

12 files changed

+683
-22
lines changed

12 files changed

+683
-22
lines changed

‎UPGRADE-7.3.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ FrameworkBundle
3939
because its default value will change in version 8.0
4040
* Deprecate the `--show-arguments` option of the `container:debug` command, as arguments are now always shown
4141

42+
43+
SecurityBundle
44+
--------------
45+
46+
* Deprecate the `security.hide_user_not_found` config option in favor of `security.expose_security_errors`
47+
4248
Serializer
4349
----------
4450

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ CHANGELOG
66

77
* Add `Security::isGrantedForUser()` to test user authorization without relying on the session. For example, users not currently logged in, or while processing a message from a message queue
88
* Add encryption support to `OidcTokenHandler` (JWE)
9+
* Add `expose_security_errors` config option to display `AccountStatusException`
10+
* Deprecate the `security.hide_user_not_found` config option in favor of `security.expose_security_errors`
911

1012
7.2
1113
---

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
1818
use Symfony\Component\Config\Definition\ConfigurationInterface;
1919
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
20+
use Symfony\Component\Security\Http\Authentication\ExposeSecurityLevel;
2021
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
2122
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy;
2223

@@ -54,13 +55,30 @@ public function getConfigTreeBuilder(): TreeBuilder
5455
$rootNode = $tb->getRootNode();
5556

5657
$rootNode
58+
->beforeNormalization()
59+
->always()
60+
->then(function ($v) {
61+
if (isset($v['hide_user_not_found']) && !isset($v['expose_security_errors'])) {
62+
$v['expose_security_errors'] = $v['hide_user_not_found'] ? ExposeSecurityLevel::None : ExposeSecurityLevel::All;
63+
}
64+
65+
return $v;
66+
})
67+
->end()
5768
->children()
5869
->scalarNode('access_denied_url')->defaultNull()->example('/foo/error403')->end()
5970
->enumNode('session_fixation_strategy')
6071
->values([SessionAuthenticationStrategy::NONE, SessionAuthenticationStrategy::MIGRATE, SessionAuthenticationStrategy::INVALIDATE])
6172
->defaultValue(SessionAuthenticationStrategy::MIGRATE)
6273
->end()
63-
->booleanNode('hide_user_not_found')->defaultTrue()->end()
74+
->booleanNode('hide_user_not_found')
75+
->setDeprecated('symfony/security-bundle', '7.3', 'The "%node%" option is deprecated and will be removed in 8.0. Use the "expose_security_errors" option instead.')
76+
->end()
77+
->enumNode('expose_security_errors')
78+
->beforeNormalization()->ifString()->then(fn ($v) => ['value' => ExposeSecurityLevel::tryFrom($v)])->end()
79+
->values(ExposeSecurityLevel::cases())
80+
->defaultValue(ExposeSecurityLevel::None)
81+
->end()
6482
->booleanNode('erase_credentials')->defaultTrue()->end()
6583
->arrayNode('access_decision_manager')
6684
->addDefaultsIfNotSet()

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
use Symfony\Component\Security\Core\User\ChainUserProvider;
5959
use Symfony\Component\Security\Core\User\UserCheckerInterface;
6060
use Symfony\Component\Security\Core\User\UserProviderInterface;
61+
use Symfony\Component\Security\Http\Authentication\ExposeSecurityLevel;
6162
use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticator;
6263
use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticatorManagerListener;
6364
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
@@ -154,7 +155,8 @@ public function load(array $configs, ContainerBuilder $container): void
154155
));
155156
}
156157

157-
$container->setParameter('security.authentication.hide_user_not_found', $config['hide_user_not_found']);
158+
$container->setParameter('security.authentication.hide_user_not_found', ExposeSecurityLevel::All !== $config['expose_security_errors']);
159+
$container->setParameter('.security.authentication.expose_security_errors', $config['expose_security_errors']);
158160

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

‎src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
<xsd:attribute name="strategy" type="access_decision_manager_strategy" />
5656
<xsd:attribute name="service" type="xsd:string" />
5757
<xsd:attribute name="strategy-service" type="xsd:string" />
58+
<xsd:attribute name="expose-security-errors" type="access_decision_manager_expose_security_level" />
5859
<xsd:attribute name="allow-if-all-abstain" type="xsd:boolean" />
5960
<xsd:attribute name="allow-if-equal-granted-denied" type="xsd:boolean" />
6061
</xsd:complexType>
@@ -68,6 +69,14 @@
6869
</xsd:restriction>
6970
</xsd:simpleType>
7071

72+
<xsd:simpleType name="access_decision_manager_expose_security_level">
73+
<xsd:restriction base="xsd:string">
74+
<xsd:enumeration value="none" />
75+
<xsd:enumeration value="account_status" />
76+
<xsd:enumeration value="all" />
77+
</xsd:restriction>
78+
</xsd:simpleType>
79+
7180
<xsd:complexType name="password_hasher">
7281
<xsd:sequence>
7382
<xsd:element name="migrate-from" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
abstract_arg('provider key'),
4343
service('logger')->nullOnInvalid(),
4444
param('security.authentication.manager.erase_credentials'),
45-
param('security.authentication.hide_user_not_found'),
45+
param('.security.authentication.expose_security_errors'),
4646
abstract_arg('required badges'),
4747
])
4848
->tag('monolog.logger', ['channel' => 'security'])

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,17 @@
1212
namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bridge\PhpUnit\ExpectUserDeprecationMessageTrait;
1516
use Symfony\Bundle\SecurityBundle\DependencyInjection\MainConfiguration;
1617
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AuthenticatorFactoryInterface;
1718
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
1819
use Symfony\Component\Config\Definition\Processor;
20+
use Symfony\Component\Security\Http\Authentication\ExposeSecurityLevel;
1921

2022
class MainConfigurationTest extends TestCase
2123
{
24+
use ExpectUserDeprecationMessageTrait;
25+
2226
/**
2327
* The minimal, required config needed to not have any required validation
2428
* issues.
@@ -228,4 +232,52 @@ public function testFirewalls()
228232
$configuration = new MainConfiguration(['stub' => $factory], []);
229233
$configuration->getConfigTreeBuilder();
230234
}
235+
236+
/**
237+
* @dataProvider provideHideUserNotFoundData
238+
*/
239+
public function testExposeSecurityErrors(array $config, ExposeSecurityLevel $expectedExposeSecurityErrors)
240+
{
241+
$config = array_merge(static::$minimalConfig, $config);
242+
243+
$processor = new Processor();
244+
$configuration = new MainConfiguration([], []);
245+
$processedConfig = $processor->processConfiguration($configuration, [$config]);
246+
247+
$this->assertEquals($expectedExposeSecurityErrors, $processedConfig['expose_security_errors']);
248+
$this->assertArrayNotHasKey('hide_user_not_found', $processedConfig);
249+
}
250+
251+
public static function provideHideUserNotFoundData(): iterable
252+
{
253+
yield [[], ExposeSecurityLevel::None];
254+
yield [['expose_security_errors' => ExposeSecurityLevel::None], ExposeSecurityLevel::None];
255+
yield [['expose_security_errors' => ExposeSecurityLevel::AccountStatus], ExposeSecurityLevel::AccountStatus];
256+
yield [['expose_security_errors' => ExposeSecurityLevel::All], ExposeSecurityLevel::All];
257+
}
258+
259+
/**
260+
* @dataProvider provideHideUserNotFoundLegacyData
261+
*
262+
* @group legacy
263+
*/
264+
public function testExposeSecurityErrorsWithLegacyConfig(array $config, ExposeSecurityLevel $expectedExposeSecurityErrors, ?bool $expectedHideUserNotFound)
265+
{
266+
$this->expectUserDeprecationMessage('Since symfony/security-bundle 7.3: The "hide_user_not_found" option is deprecated and will be removed in 8.0. Use the "expose_security_errors" option instead.');
267+
268+
$config = array_merge(static::$minimalConfig, $config);
269+
270+
$processor = new Processor();
271+
$configuration = new MainConfiguration([], []);
272+
$processedConfig = $processor->processConfiguration($configuration, [$config]);
273+
274+
$this->assertEquals($expectedExposeSecurityErrors, $processedConfig['expose_security_errors']);
275+
$this->assertEquals($expectedHideUserNotFound, $processedConfig['hide_user_not_found']);
276+
}
277+
278+
public static function provideHideUserNotFoundLegacyData(): iterable
279+
{
280+
yield [['hide_user_not_found' => true], ExposeSecurityLevel::None, true];
281+
yield [['hide_user_not_found' => false], ExposeSecurityLevel::All, false];
282+
}
231283
}

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

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
*/
4747
class AuthenticatorManager implements AuthenticatorManagerInterface, UserAuthenticatorInterface
4848
{
49+
private ExposeSecurityLevel $exposeSecurityErrors;
50+
4951
/**
5052
* @param iterable<mixed, AuthenticatorInterface> $authenticators
5153
*/
@@ -56,9 +58,17 @@ public function __construct(
5658
private string $firewallName,
5759
private ?LoggerInterface $logger = null,
5860
private bool $eraseCredentials = true,
59-
private bool $hideUserNotFoundExceptions = true,
61+
ExposeSecurityLevel|bool $exposeSecurityErrors = ExposeSecurityLevel::None,
6062
private array $requiredBadges = [],
6163
) {
64+
if (\is_bool($exposeSecurityErrors)) {
65+
trigger_deprecation('symfony/security-http', '7.3', 'Passing a boolean as "exposeSecurityErrors" parameter is deprecated, use %s value instead.', ExposeSecurityLevel::class);
66+
67+
// The old parameter had an inverted meaning ($hideUserNotFoundExceptions), for that reason the current name does not reflect the behavior
68+
$exposeSecurityErrors = $exposeSecurityErrors ? ExposeSecurityLevel::None : ExposeSecurityLevel::All;
69+
}
70+
71+
$this->exposeSecurityErrors = $exposeSecurityErrors;
6272
}
6373

6474
/**
@@ -250,7 +260,7 @@ private function handleAuthenticationFailure(AuthenticationException $authentica
250260

251261
// Avoid leaking error details in case of invalid user (e.g. user not found or invalid account status)
252262
// to prevent user enumeration via response content comparison
253-
if ($this->hideUserNotFoundExceptions && ($authenticationException instanceof UserNotFoundException || ($authenticationException instanceof AccountStatusException && !$authenticationException instanceof CustomUserMessageAccountStatusException))) {
263+
if ($this->isSensitiveException($authenticationException)) {
254264
$authenticationException = new BadCredentialsException('Bad credentials.', 0, $authenticationException);
255265
}
256266

@@ -264,4 +274,17 @@ private function handleAuthenticationFailure(AuthenticationException $authentica
264274
// returning null is ok, it means they want the request to continue
265275
return $loginFailureEvent->getResponse();
266276
}
277+
278+
private function isSensitiveException(AuthenticationException $exception): bool
279+
{
280+
if (ExposeSecurityLevel::All !== $this->exposeSecurityErrors && $exception instanceof UserNotFoundException) {
281+
return true;
282+
}
283+
284+
if (ExposeSecurityLevel::None === $this->exposeSecurityErrors && $exception instanceof AccountStatusException && !$exception instanceof CustomUserMessageAccountStatusException) {
285+
return true;
286+
}
287+
288+
return false;
289+
}
267290
}

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

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\Security\Http\Authentication;
13+
14+
/**
15+
* @author Christian Gripp <mail@core23.de>
16+
*/
17+
enum ExposeSecurityLevel: string
18+
{
19+
case None = 'none';
20+
case AccountStatus = 'account_status';
21+
case All = 'all';
22+
}

‎src/Symfony/Component/Security/Http/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ CHANGELOG
55
---
66

77
* Add encryption support to `OidcTokenHandler` (JWE)
8+
* Replace `$hideAccountStatusExceptions` argument with `$exposeSecurityErrors` in `AuthenticatorManager` constructor
89

910
7.2
1011
---

0 commit comments

Comments
 (0)
0