From f012eee6c6034a94566dff596fe4e16dfc5d9c1f Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Sat, 2 Jan 2021 17:06:12 +0100 Subject: [PATCH 01/61] [Security][Guard] Prevent user enumeration via response content --- .../SecurityBundle/Resources/config/guard.xml | 3 +- .../Provider/UserAuthenticationProvider.php | 3 +- .../UserAuthenticationProviderTest.php | 8 +-- .../Firewall/GuardAuthenticationListener.php | 13 ++++- .../GuardAuthenticationListenerTest.php | 51 +++++++++++++++++++ 5 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml index 43321494e0194..9e27dcb575f49 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml @@ -17,7 +17,7 @@ - + @@ -41,6 +41,7 @@ + %security.authentication.hide_user_not_found% diff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php index 172556ac2868b..9557fa00047c1 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php @@ -13,6 +13,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Exception\AccountStatusException; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\AuthenticationServiceException; use Symfony\Component\Security\Core\Exception\BadCredentialsException; @@ -83,7 +84,7 @@ public function authenticate(TokenInterface $token) $this->userChecker->checkPreAuth($user); $this->checkAuthentication($user, $token); $this->userChecker->checkPostAuth($user); - } catch (BadCredentialsException $e) { + } catch (AccountStatusException $e) { if ($this->hideUserNotFoundExceptions) { throw new BadCredentialsException('Bad credentials.', 0, $e); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php index 7b984e304d814..c20b6ca2eaa1d 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php @@ -79,7 +79,7 @@ public function testAuthenticateWhenProviderDoesNotReturnAnUserInterface() public function testAuthenticateWhenPreChecksFails() { - $this->expectException('Symfony\Component\Security\Core\Exception\CredentialsExpiredException'); + $this->expectException(BadCredentialsException::class); $userChecker = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserCheckerInterface')->getMock(); $userChecker->expects($this->once()) ->method('checkPreAuth') @@ -97,7 +97,7 @@ public function testAuthenticateWhenPreChecksFails() public function testAuthenticateWhenPostChecksFails() { - $this->expectException('Symfony\Component\Security\Core\Exception\AccountExpiredException'); + $this->expectException(BadCredentialsException::class); $userChecker = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserCheckerInterface')->getMock(); $userChecker->expects($this->once()) ->method('checkPostAuth') @@ -116,7 +116,7 @@ public function testAuthenticateWhenPostChecksFails() public function testAuthenticateWhenPostCheckAuthenticationFails() { $this->expectException('Symfony\Component\Security\Core\Exception\BadCredentialsException'); - $this->expectExceptionMessage('Bad credentials'); + $this->expectExceptionMessage('Bad credentials.'); $provider = $this->getProvider(); $provider->expects($this->once()) ->method('retrieveUser') @@ -124,7 +124,7 @@ public function testAuthenticateWhenPostCheckAuthenticationFails() ; $provider->expects($this->once()) ->method('checkAuthentication') - ->willThrowException(new BadCredentialsException()) + ->willThrowException(new CredentialsExpiredException()) ; $provider->authenticate($this->getSupportedToken()); diff --git a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php index 11bda3cd180da..190290c46afb3 100644 --- a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php +++ b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php @@ -17,7 +17,10 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Exception\AccountStatusException; use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Security\Core\Exception\BadCredentialsException; +use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Guard\AbstractGuardAuthenticator; use Symfony\Component\Security\Guard\AuthenticatorInterface; use Symfony\Component\Security\Guard\GuardAuthenticatorHandler; @@ -40,6 +43,7 @@ class GuardAuthenticationListener implements ListenerInterface private $guardAuthenticators; private $logger; private $rememberMeServices; + private $hideUserNotFoundExceptions; /** * @param GuardAuthenticatorHandler $guardHandler The Guard handler @@ -48,7 +52,7 @@ class GuardAuthenticationListener implements ListenerInterface * @param iterable|AuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider * @param LoggerInterface $logger A LoggerInterface instance */ - public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null) + public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null, $hideUserNotFoundExceptions = true) { if (empty($providerKey)) { throw new \InvalidArgumentException('$providerKey must not be empty.'); @@ -59,6 +63,7 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat $this->providerKey = $providerKey; $this->guardAuthenticators = $guardAuthenticators; $this->logger = $logger; + $this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions; } /** @@ -163,6 +168,12 @@ private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorIn $this->logger->info('Guard authentication failed.', ['exception' => $e, 'authenticator' => \get_class($guardAuthenticator)]); } + // Avoid leaking error details in case of invalid user (e.g. user not found or invalid account status) + // to prevent user enumeration via response content + if ($this->hideUserNotFoundExceptions && ($e instanceof UsernameNotFoundException || $e instanceof AccountStatusException)) { + $e = new BadCredentialsException('Bad credentials.', 0, $e); + } + $response = $this->guardHandler->handleAuthenticationFailure($e, $request, $guardAuthenticator, $this->providerKey); if ($response instanceof Response) { diff --git a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php index 6572696d48fca..a2e4671d52fff 100644 --- a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php @@ -16,6 +16,9 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Security\Core\Exception\BadCredentialsException; +use Symfony\Component\Security\Core\Exception\LockedException; +use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Guard\AbstractGuardAuthenticator; use Symfony\Component\Security\Guard\AuthenticatorInterface; use Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener; @@ -208,6 +211,54 @@ public function testHandleCatchesAuthenticationException() $listener->handle($this->event); } + /** + * @dataProvider exceptionsToHide + */ + public function testHandleHidesInvalidUserExceptions(AuthenticationException $exceptionToHide) + { + $authenticator = $this->createMock(AuthenticatorInterface::class); + $providerKey = 'my_firewall2'; + + $authenticator + ->expects($this->once()) + ->method('supports') + ->willReturn(true); + $authenticator + ->expects($this->once()) + ->method('getCredentials') + ->willReturn(['username' => 'robin', 'password' => 'hood']); + + $this->authenticationManager + ->expects($this->once()) + ->method('authenticate') + ->willThrowException($exceptionToHide); + + $this->guardAuthenticatorHandler + ->expects($this->once()) + ->method('handleAuthenticationFailure') + ->with($this->callback(function ($e) use ($exceptionToHide) { + return $e instanceof BadCredentialsException && $exceptionToHide === $e->getPrevious(); + }), $this->request, $authenticator, $providerKey); + + $listener = new GuardAuthenticationListener( + $this->guardAuthenticatorHandler, + $this->authenticationManager, + $providerKey, + [$authenticator], + $this->logger + ); + + $listener->handle($this->event); + } + + public function exceptionsToHide() + { + return [ + [new UsernameNotFoundException()], + [new LockedException()], + ]; + } + /** * @group legacy */ From 9ad3720efcb5e3998667dce897ed7d9cacfe985e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Sat, 10 Apr 2021 12:08:01 +0200 Subject: [PATCH 02/61] [security] NullToken signature --- .../Component/Security/Core/Authentication/Token/NullToken.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php index 589ad1b47f9c7..b8f1c4632df7b 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php @@ -33,7 +33,7 @@ public function getCredentials() public function getUser() { - return null; + return ''; } public function setUser($user) From b7658aac3fc48a9c7762ed2040debf3015a428f8 Mon Sep 17 00:00:00 2001 From: Maciej Zgadzaj Date: Sat, 24 Apr 2021 23:53:14 +0200 Subject: [PATCH 03/61] PhpDocExtractor: Handle "true" and "false" property types --- .../Tests/Extractor/PhpDocExtractorTest.php | 4 +++ .../Extractor/ReflectionExtractorTest.php | 8 ++++++ .../Tests/Fixtures/ParentDummy.php | 28 +++++++++++++++++++ src/Symfony/Component/PropertyInfo/Type.php | 4 +++ 4 files changed, 44 insertions(+) diff --git a/src/Symfony/Component/PropertyInfo/Tests/Extractor/PhpDocExtractorTest.php b/src/Symfony/Component/PropertyInfo/Tests/Extractor/PhpDocExtractorTest.php index 3acadc6484be2..1a7dd664af4b8 100644 --- a/src/Symfony/Component/PropertyInfo/Tests/Extractor/PhpDocExtractorTest.php +++ b/src/Symfony/Component/PropertyInfo/Tests/Extractor/PhpDocExtractorTest.php @@ -109,7 +109,11 @@ public function typesProvider() ['a', [new Type(Type::BUILTIN_TYPE_INT)], 'A.', null], ['b', [new Type(Type::BUILTIN_TYPE_OBJECT, true, 'Symfony\Component\PropertyInfo\Tests\Fixtures\ParentDummy')], 'B.', null], ['c', [new Type(Type::BUILTIN_TYPE_BOOL, true)], null, null], + ['ct', [new Type(Type::BUILTIN_TYPE_TRUE, true)], null, null], + ['cf', [new Type(Type::BUILTIN_TYPE_FALSE, true)], null, null], ['d', [new Type(Type::BUILTIN_TYPE_BOOL)], null, null], + ['dt', [new Type(Type::BUILTIN_TYPE_TRUE)], null, null], + ['df', [new Type(Type::BUILTIN_TYPE_FALSE)], null, null], ['e', [new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_RESOURCE))], null, null], ['f', [new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_OBJECT, false, 'DateTime'))], null, null], ['g', [new Type(Type::BUILTIN_TYPE_ARRAY, true, null, true)], 'Nullable array.', null], diff --git a/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php b/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php index 1f65e57b62bd9..4fad83ef83a37 100644 --- a/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php +++ b/src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php @@ -88,7 +88,11 @@ public function testGetProperties() 'date', 'element', 'c', + 'ct', + 'cf', 'd', + 'dt', + 'df', 'e', 'f', ], @@ -134,7 +138,11 @@ public function testGetPropertiesWithCustomPrefixes() 'parentAnnotationNoParent', 'date', 'c', + 'ct', + 'cf', 'd', + 'dt', + 'df', 'e', 'f', ], diff --git a/src/Symfony/Component/PropertyInfo/Tests/Fixtures/ParentDummy.php b/src/Symfony/Component/PropertyInfo/Tests/Fixtures/ParentDummy.php index d698983254d75..a7c1f513a78c7 100644 --- a/src/Symfony/Component/PropertyInfo/Tests/Fixtures/ParentDummy.php +++ b/src/Symfony/Component/PropertyInfo/Tests/Fixtures/ParentDummy.php @@ -65,6 +65,20 @@ public function isC() { } + /** + * @return true|null + */ + public function isCt() + { + } + + /** + * @return false|null + */ + public function isCf() + { + } + /** * @return bool */ @@ -72,6 +86,20 @@ public function canD() { } + /** + * @return true + */ + public function canDt() + { + } + + /** + * @return false + */ + public function canDf() + { + } + /** * @param resource $e */ diff --git a/src/Symfony/Component/PropertyInfo/Type.php b/src/Symfony/Component/PropertyInfo/Type.php index 582b98d6411f5..02c387db47483 100644 --- a/src/Symfony/Component/PropertyInfo/Type.php +++ b/src/Symfony/Component/PropertyInfo/Type.php @@ -24,6 +24,8 @@ class Type public const BUILTIN_TYPE_FLOAT = 'float'; public const BUILTIN_TYPE_STRING = 'string'; public const BUILTIN_TYPE_BOOL = 'bool'; + public const BUILTIN_TYPE_TRUE = 'true'; + public const BUILTIN_TYPE_FALSE = 'false'; public const BUILTIN_TYPE_RESOURCE = 'resource'; public const BUILTIN_TYPE_OBJECT = 'object'; public const BUILTIN_TYPE_ARRAY = 'array'; @@ -41,6 +43,8 @@ class Type self::BUILTIN_TYPE_FLOAT, self::BUILTIN_TYPE_STRING, self::BUILTIN_TYPE_BOOL, + self::BUILTIN_TYPE_TRUE, + self::BUILTIN_TYPE_FALSE, self::BUILTIN_TYPE_RESOURCE, self::BUILTIN_TYPE_OBJECT, self::BUILTIN_TYPE_ARRAY, From 838f36b09f9dc5763b8b2f8007547d0b8b1de9a7 Mon Sep 17 00:00:00 2001 From: Pavel Kirpitsov Date: Tue, 27 Apr 2021 13:20:43 +0300 Subject: [PATCH 04/61] [Notifier] [Bridge] Store message id for slack transport's SendMessage --- .../Notifier/Bridge/Slack/SlackTransport.php | 5 ++++- .../Bridge/Slack/Tests/SlackTransportTest.php | 12 ++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Notifier/Bridge/Slack/SlackTransport.php b/src/Symfony/Component/Notifier/Bridge/Slack/SlackTransport.php index 5a98638ab30d5..740a4937ebd6b 100644 --- a/src/Symfony/Component/Notifier/Bridge/Slack/SlackTransport.php +++ b/src/Symfony/Component/Notifier/Bridge/Slack/SlackTransport.php @@ -91,6 +91,9 @@ protected function doSend(MessageInterface $message): SentMessage throw new TransportException(sprintf('Unable to post the Slack message: "%s".', $result['error']), $response); } - return new SentMessage($message, (string) $this); + $sentMessage = new SentMessage($message, (string) $this); + $sentMessage->setMessageId($result['ts']); + + return $sentMessage; } } diff --git a/src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportTest.php b/src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportTest.php index 3b7a3c6f8ff64..12e00e3f4a951 100644 --- a/src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportTest.php +++ b/src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportTest.php @@ -110,7 +110,7 @@ public function testSendWithOptions() $response->expects($this->once()) ->method('getContent') - ->willReturn(json_encode(['ok' => true])); + ->willReturn(json_encode(['ok' => true, 'ts' => '1503435956.000247'])); $expectedBody = json_encode(['channel' => $channel, 'text' => $message]); @@ -122,7 +122,9 @@ public function testSendWithOptions() $transport = $this->createTransport($client, $channel); - $transport->send(new ChatMessage('testMessage')); + $sentMessage = $transport->send(new ChatMessage('testMessage')); + + $this->assertSame('1503435956.000247', $sentMessage->getMessageId()); } public function testSendWithNotification() @@ -138,7 +140,7 @@ public function testSendWithNotification() $response->expects($this->once()) ->method('getContent') - ->willReturn(json_encode(['ok' => true])); + ->willReturn(json_encode(['ok' => true, 'ts' => '1503435956.000247'])); $notification = new Notification($message); $chatMessage = ChatMessage::fromNotification($notification); @@ -158,7 +160,9 @@ public function testSendWithNotification() $transport = $this->createTransport($client, $channel); - $transport->send($chatMessage); + $sentMessage = $transport->send($chatMessage); + + $this->assertSame('1503435956.000247', $sentMessage->getMessageId()); } public function testSendWithInvalidOptions() From 92a61b1cf60a90470d620a7f04377dd7e9d9d9f8 Mon Sep 17 00:00:00 2001 From: Nyholm Date: Fri, 23 Apr 2021 18:47:46 +0200 Subject: [PATCH 05/61] [Translation] Set default locale for IdentityTranslatorTest --- .../Component/Translation/Tests/IdentityTranslatorTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Translation/Tests/IdentityTranslatorTest.php b/src/Symfony/Component/Translation/Tests/IdentityTranslatorTest.php index a630a7a3ce4f0..16945d38a293a 100644 --- a/src/Symfony/Component/Translation/Tests/IdentityTranslatorTest.php +++ b/src/Symfony/Component/Translation/Tests/IdentityTranslatorTest.php @@ -23,6 +23,7 @@ protected function setUp(): void parent::setUp(); $this->defaultLocale = \Locale::getDefault(); + \Locale::setDefault('en'); } protected function tearDown(): void From a95bbaaaef1f706836a59f021ac42cd5af576609 Mon Sep 17 00:00:00 2001 From: Roman Anasal Date: Wed, 28 Apr 2021 22:58:08 +0200 Subject: [PATCH 06/61] [TwigBridge] Fix HTML for translatable custom-file label in Bootstrap 4 theme Bootstraps allows to translate the label of the button of a custom-file input with SCSS variables depending on the lang attribute of the *input*. This attribute was set on the label instead and thus had no effect. --- .../Twig/Resources/views/Form/bootstrap_4_layout.html.twig | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig b/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig index c990d81370f3c..b279477265bf8 100644 --- a/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig +++ b/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig @@ -121,11 +121,12 @@ {% block file_widget -%} <{{ element|default('div') }} class="custom-file"> {%- set type = type|default('file') -%} - {{- block('form_widget_simple') -}} - {%- set label_attr = label_attr|merge({ class: (label_attr.class|default('') ~ ' custom-file-label')|trim }) -%} {%- set input_lang = 'en' -%} {% if app is defined and app.request is defined %}{%- set input_lang = app.request.locale -%}{%- endif -%} -