diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #22569 [Security] Handle bad request format in json auth listener (ogizanagi)

Browse Source 
This PR was merged into the 3.3-dev branch.

Discussion
----------

[Security] Handle bad request format in json auth listener

| Q             | A
| ------------- | ---
| Branch?       | master (3.3)
| Bug fix?      | yesish
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

In https://github.com/symfony/symfony/pull/22034, I wondered myself if we shouldn't throw a dedicated exception to handle bad formatted requests and give more inputs to the client by returning a 400 response with an explicit message.

~~Here is a suggestion, introducing a new `BadRequestFormatException` and using it in `UsernamePasswordJsonAuthenticationListener` whenever there is no custom failure handler set (but someone using its own handler should be able to treat the failure properly too).~~

As discussed with @chalasr , it seems better to directly throw a `BadRequestHttpException` as it's actually out of the whole security process. PR updated.

Commits
-------

93a8cb9cd4 [Security] Handle bad request format in json auth listener
This commit is contained in:
Fabien Potencier 2017-04-29 08:39:09 -07:00
commit 460fcbf23b
2 changed files with 35 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php
View File

@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\PropertyAccess\Exception\AccessException;
use Symfony\Component\PropertyAccess\PropertyAccess;
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
@ -83,23 +84,23 @@ class UsernamePasswordJsonAuthenticationListener implements ListenerInterface
try {
if (!$data instanceof \stdClass) {
throw new BadCredentialsException('Invalid JSON.');
throw new BadRequestHttpException('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']));
throw new BadRequestHttpException(sprintf('The key "%s" must be provided.', $this->options['username_path']), $e);
}
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']));
throw new BadRequestHttpException(sprintf('The key "%s" must be provided.', $this->options['password_path']), $e);
}
if (!is_string($username)) {
throw new BadCredentialsException(sprintf('The key "%s" must be a string.', $this->options['username_path']));
throw new BadRequestHttpException(sprintf('The key "%s" must be a string.', $this->options['username_path']));
}
if (strlen($username) > Security::MAX_USERNAME_LENGTH) {
@ -107,7 +108,7 @@ class UsernamePasswordJsonAuthenticationListener implements ListenerInterface
}
if (!is_string($password)) {
throw new BadCredentialsException(sprintf('The key "%s" must be a string.', $this->options['password_path']));
throw new BadRequestHttpException(sprintf('The key "%s" must be a string.', $this->options['password_path']));
}
$token = new UsernamePasswordToken($username, $password, $this->providerKey);

33
src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordJsonAuthenticationListenerTest.php
View File

@ -93,6 +93,23 @@ class UsernamePasswordJsonAuthenticationListenerTest extends TestCase
$this->assertEquals('ok', $event->getResponse()->getContent());
}
/**
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
* @expectedExceptionMessage Invalid JSON
*/
public function testAttemptAuthenticationNoJson()
{
$this->createListener();
$request = new Request();
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
$this->listener->handle($event);
}
/**
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
* @expectedExceptionMessage The key "username" must be provided
*/
public function testAttemptAuthenticationNoUsername()
{
$this->createListener();
@ -100,9 +117,12 @@ class UsernamePasswordJsonAuthenticationListenerTest extends TestCase
$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\HttpKernel\Exception\BadRequestHttpException
* @expectedExceptionMessage The key "password" must be provided
*/
public function testAttemptAuthenticationNoPassword()
{
$this->createListener();
@ -110,9 +130,12 @@ class UsernamePasswordJsonAuthenticationListenerTest extends TestCase
$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\HttpKernel\Exception\BadRequestHttpException
* @expectedExceptionMessage The key "username" must be a string.
*/
public function testAttemptAuthenticationUsernameNotAString()
{
$this->createListener();
@ -120,9 +143,12 @@ class UsernamePasswordJsonAuthenticationListenerTest extends TestCase
$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\HttpKernel\Exception\BadRequestHttpException
* @expectedExceptionMessage The key "password" must be a string.
*/
public function testAttemptAuthenticationPasswordNotAString()
{
$this->createListener();
@ -130,7 +156,6 @@ class UsernamePasswordJsonAuthenticationListenerTest extends TestCase
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
$this->listener->handle($event);
$this->assertSame('ko', $event->getResponse()->getContent());
}
public function testAttemptAuthenticationUsernameTooLong()