From cb175a41c388f97d491ad4cde3bcab14fe665cec Mon Sep 17 00:00:00 2001 From: Maxime Steinhausser Date: Fri, 17 Mar 2017 11:48:56 +0100 Subject: [PATCH] [Security] json auth listener should not produce a 500 response on bad request format --- ...namePasswordJsonAuthenticationListener.php | 54 +++++++++---------- ...PasswordJsonAuthenticationListenerTest.php | 20 ++----- 2 files changed, 32 insertions(+), 42 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php index dfbb4a4c208be..8b40119632b36 100644 --- a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php @@ -70,35 +70,35 @@ public function handle(GetResponseEvent $event) $request = $event->getRequest(); $data = json_decode($request->getContent()); - if (!$data instanceof \stdClass) { - throw new BadCredentialsException('Invalid JSON.'); - } - try { - $username = $this->propertyAccessor->getValue($data, $this->options['username_path']); - } catch (AccessException $e) { - throw new BadCredentialsException(sprintf('The key "%s" must be provided.', $this->options['username_path'])); - } + if (!$data instanceof \stdClass) { + throw new BadCredentialsException('Invalid JSON.'); + } + + try { + $username = $this->propertyAccessor->getValue($data, $this->options['username_path']); + } catch (AccessException $e) { + throw new BadCredentialsException(sprintf('The key "%s" must be provided.', $this->options['username_path'])); + } + + try { + $password = $this->propertyAccessor->getValue($data, $this->options['password_path']); + } catch (AccessException $e) { + throw new BadCredentialsException(sprintf('The key "%s" must be provided.', $this->options['password_path'])); + } + + if (!is_string($username)) { + throw new BadCredentialsException(sprintf('The key "%s" must be a string.', $this->options['username_path'])); + } + + if (strlen($username) > Security::MAX_USERNAME_LENGTH) { + throw new BadCredentialsException('Invalid username.'); + } + + if (!is_string($password)) { + throw new BadCredentialsException(sprintf('The key "%s" must be a string.', $this->options['password_path'])); + } - try { - $password = $this->propertyAccessor->getValue($data, $this->options['password_path']); - } catch (AccessException $e) { - throw new BadCredentialsException(sprintf('The key "%s" must be provided.', $this->options['password_path'])); - } - - if (!is_string($username)) { - throw new BadCredentialsException(sprintf('The key "%s" must be a string.', $this->options['username_path'])); - } - - if (strlen($username) > Security::MAX_USERNAME_LENGTH) { - throw new BadCredentialsException('Invalid username.'); - } - - if (!is_string($password)) { - throw new BadCredentialsException(sprintf('The key "%s" must be a string.', $this->options['password_path'])); - } - - try { $token = new UsernamePasswordToken($username, $password, $this->providerKey); $authenticatedToken = $this->authenticationManager->authenticate($token); diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordJsonAuthenticationListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordJsonAuthenticationListenerTest.php index 5a8687d7f9465..e5435cb1b215e 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordJsonAuthenticationListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordJsonAuthenticationListenerTest.php @@ -86,9 +86,6 @@ public function testUsePath() $this->assertEquals('ok', $event->getResponse()->getContent()); } - /** - * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException - */ public function testAttemptAuthenticationNoUsername() { $this->createListener(); @@ -96,11 +93,9 @@ public function testAttemptAuthenticationNoUsername() $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST); $this->listener->handle($event); + $this->assertSame('ko', $event->getResponse()->getContent()); } - /** - * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException - */ public function testAttemptAuthenticationNoPassword() { $this->createListener(); @@ -108,11 +103,9 @@ public function testAttemptAuthenticationNoPassword() $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST); $this->listener->handle($event); + $this->assertSame('ko', $event->getResponse()->getContent()); } - /** - * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException - */ public function testAttemptAuthenticationUsernameNotAString() { $this->createListener(); @@ -120,11 +113,9 @@ public function testAttemptAuthenticationUsernameNotAString() $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST); $this->listener->handle($event); + $this->assertSame('ko', $event->getResponse()->getContent()); } - /** - * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException - */ public function testAttemptAuthenticationPasswordNotAString() { $this->createListener(); @@ -132,11 +123,9 @@ public function testAttemptAuthenticationPasswordNotAString() $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST); $this->listener->handle($event); + $this->assertSame('ko', $event->getResponse()->getContent()); } - /** - * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException - */ public function testAttemptAuthenticationUsernameTooLong() { $this->createListener(); @@ -145,5 +134,6 @@ public function testAttemptAuthenticationUsernameTooLong() $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST); $this->listener->handle($event); + $this->assertSame('ko', $event->getResponse()->getContent()); } }