From 67ae121b2e36120133be7fd700a5fa861d4f1995 Mon Sep 17 00:00:00 2001 From: Patrick Reimers Date: Fri, 22 Feb 2019 14:56:26 +0100 Subject: [PATCH] [Security] Change FormAuthenticator if condition --- .../SimpleFormAuthenticationListener.php | 2 +- ...namePasswordFormAuthenticationListener.php | 2 +- ...PasswordFormAuthenticationListenerTest.php | 83 ++++++++++++++++++- 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Firewall/SimpleFormAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/SimpleFormAuthenticationListener.php index 63d73d235aa1..99c8fcab4e64 100644 --- a/src/Symfony/Component/Security/Http/Firewall/SimpleFormAuthenticationListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/SimpleFormAuthenticationListener.php @@ -107,7 +107,7 @@ protected function attemptAuthentication(Request $request) $password = ParameterBagUtils::getRequestParameterValue($request, $this->options['password_parameter']); } - if (!\is_string($username) || (\is_object($username) && !\method_exists($username, '__toString'))) { + if (!\is_string($username) && (!\is_object($username) || !\method_exists($username, '__toString'))) { throw new BadRequestHttpException(sprintf('The key "%s" must be a string, "%s" given.', $this->options['username_parameter'], \gettype($username))); } diff --git a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php index aea883a04c78..2f5415673294 100644 --- a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php @@ -85,7 +85,7 @@ protected function attemptAuthentication(Request $request) $password = ParameterBagUtils::getRequestParameterValue($request, $this->options['password_parameter']); } - if (!\is_string($username) || (\is_object($username) && !\method_exists($username, '__toString'))) { + if (!\is_string($username) && (!\is_object($username) || !\method_exists($username, '__toString'))) { throw new BadRequestHttpException(sprintf('The key "%s" must be a string, "%s" given.', $this->options['username_parameter'], \gettype($username))); } diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php index 198ee6566dfd..f1536db6d25a 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php @@ -81,7 +81,7 @@ public function testHandleWhenUsernameLength($username, $ok) * @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException * @expectedExceptionMessage The key "_username" must be a string, "array" given. */ - public function testHandleNonStringUsername($postOnly) + public function testHandleNonStringUsernameWithArray($postOnly) { $request = Request::create('/login_check', 'POST', ['_username' => []]); $request->setSession($this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock()); @@ -99,6 +99,79 @@ public function testHandleNonStringUsername($postOnly) $listener->handle($event); } + /** + * @dataProvider postOnlyDataProvider + * @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException + * @expectedExceptionMessage The key "_username" must be a string, "integer" given. + */ + public function testHandleNonStringUsernameWithInt($postOnly) + { + $request = Request::create('/login_check', 'POST', ['_username' => 42]); + $request->setSession($this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock()); + $listener = new UsernamePasswordFormAuthenticationListener( + new TokenStorage(), + $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(), + new SessionAuthenticationStrategy(SessionAuthenticationStrategy::NONE), + $httpUtils = new HttpUtils(), + 'foo', + new DefaultAuthenticationSuccessHandler($httpUtils), + new DefaultAuthenticationFailureHandler($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $httpUtils), + ['require_previous_session' => false, 'post_only' => $postOnly] + ); + $event = new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST); + $listener->handle($event); + } + + /** + * @dataProvider postOnlyDataProvider + * @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException + * @expectedExceptionMessage The key "_username" must be a string, "object" given. + */ + public function testHandleNonStringUsernameWithObject($postOnly) + { + $request = Request::create('/login_check', 'POST', ['_username' => new \stdClass()]); + $request->setSession($this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock()); + $listener = new UsernamePasswordFormAuthenticationListener( + new TokenStorage(), + $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(), + new SessionAuthenticationStrategy(SessionAuthenticationStrategy::NONE), + $httpUtils = new HttpUtils(), + 'foo', + new DefaultAuthenticationSuccessHandler($httpUtils), + new DefaultAuthenticationFailureHandler($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $httpUtils), + ['require_previous_session' => false, 'post_only' => $postOnly] + ); + $event = new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST); + $listener->handle($event); + } + + /** + * @dataProvider postOnlyDataProvider + */ + public function testHandleNonStringUsernameWith__toString($postOnly) + { + $usernameClass = $this->getMockBuilder(DummyUserClass::class)->getMock(); + $usernameClass + ->expects($this->atLeastOnce()) + ->method('__toString') + ->will($this->returnValue('someUsername')); + + $request = Request::create('/login_check', 'POST', ['_username' => $usernameClass]); + $request->setSession($this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock()); + $listener = new UsernamePasswordFormAuthenticationListener( + new TokenStorage(), + $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(), + new SessionAuthenticationStrategy(SessionAuthenticationStrategy::NONE), + $httpUtils = new HttpUtils(), + 'foo', + new DefaultAuthenticationSuccessHandler($httpUtils), + new DefaultAuthenticationFailureHandler($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $httpUtils), + ['require_previous_session' => false, 'post_only' => $postOnly] + ); + $event = new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST); + $listener->handle($event); + } + public function postOnlyDataProvider() { return [ @@ -115,3 +188,11 @@ public function getUsernameForLength() ]; } } + +class DummyUserClass +{ + public function __toString() + { + return ''; + } +}