8000 bug #22569 [Security] Handle bad request format in json auth listener… · symfony/symfony@460fcbf · GitHub
[go: up one dir, main page]
Skip to content

Commit 460fcbf

Browse files
fabpotfabpot
committed
bug #22569 [Security] Handle bad request format in json auth listener (ogizanagi)
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 #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 ------- 93a8cb9 [Security] Handle bad request format in json auth listener
2 parents 288d55f + 93a8cb9 commit 460fcbf

File tree

2 files changed

+35
-9
lines changed

2 files changed

+35
-9
lines changed

‎src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\HttpFoundation\Request;
1818
use Symfony\Component\HttpFoundation\Response;
1919
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
20+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
2021
use Symfony\Component\PropertyAccess\Exception\AccessException;
2122
use Symfony\Component\PropertyAccess\PropertyAccess;
2223
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
@@ -83,31 +84,31 @@ public function handle(GetResponseEvent $event)
8384

8485
try {
8586
if (!$data instanceof \stdClass) {
86-
throw new BadCredentialsException('Invalid JSON.');
87+
throw new BadRequestHttpException('Invalid JSON.');
8788
}
8889

8990
try {
9091
$username = $this->propertyAccessor->getValue($data, $this->options['username_path']);
9192
} catch (AccessException $e) {
92-
throw new BadCredentialsException(sprintf('The key "%s" must be provided.', $this->options['username_path']));
93+
throw new BadRequestHttpException(sprintf('The key "%s" must be provided.', $this->options['username_path']), $e);
9394
}
9495

9596
try {
9697
$password = $this->propertyAccessor->getValue($data, $this->options['password_path']);
9798
} catch (AccessException $e) {
98-
throw new BadCredentialsException(sprintf('The key "%s" must be provided.', $this->options['password_path']));
99+
throw new BadRequestHttpException(sprintf('The key "%s" must be provided.', $this->options['password_path']), $e);
99100
}
100101

101102
if (!is_string($username)) {
102-
throw new BadCredentialsException(sprintf('The key "%s" must be a string.', $this->options['username_path']));
103+
throw new BadRequestHttpException(sprintf('The key "%s" must be a string.', $this->options['username_path']));
103104
}
104105

105106
if (strlen($username) > Security::MAX_USERNAME_LENGTH) {
106107
throw new BadCredentialsException('Invalid username.');
107108
}
108109

109110
if (!is_string($password)) {
110-
throw new BadCredentialsException(sprintf('The key "%s" must be a string.', $this->options['password_path']));
111+
throw new BadRequestHttpException(sprintf('The key "%s" must be a string.', $this->options['password_path']));
111112
}
112113

113114
$token = new UsernamePasswordToken($username, $password, $this->providerKey);

‎src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordJsonAuthenticationListenerTest.php

Lines changed: 29 additions & 4 deletions
Original file line numberDiff l 10000 ine numberDiff line change
@@ -93,44 +93,69 @@ public function testUsePath()
9393
$this->assertEquals('ok', $event->getResponse()->getContent());
9494
}
9595

96+
/**
97+
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
98+
* @expectedExceptionMessage Invalid JSON
99+
*/
100+
public function testAttemptAuthenticationNoJson()
101+
{
102+
$this->createListener();
103+
$request = new Request();
104+
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
105+
106+
$this->listener->handle($event);
107+
}
108+
109+
/**
110+
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
111+
* @expectedExceptionMessage The key "username" must be provided
112+
*/
96113
public function testAttemptAuthenticationNoUsername()
97114
{
98115
$this->createListener();
99116
$request = new Request(array(), array(), array(), array(), array(), array(), '{"usr": "dunglas", "password": "foo"}');
100117
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
101118

102119
$this->listener->handle($event);
103-
$this->assertSame('ko', $event->getResponse()->getContent());
104120
}
105121

122+
/**
123+
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
124+
* @expectedExceptionMessage The key "password" must be provided
125+
*/
106126
public function testAttemptAuthenticationNoPassword()
107127
{
108128
$this->createListener();
109129
$request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "pass": "foo"}');
110130
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
111131

112132
$this->listener->handle($event);
113-
$this->assertSame('ko', $event->getResponse()->getContent());
114133
}
115134

135+
/**
136+
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
137+
* @expectedExceptionMessage The key "username" must be a string.
138+
*/
116139
public function testAttemptAuthenticationUsernameNotAString()
117140
{
118141
$this->createListener();
119142
$request = new Request(array(), array(), array(), array(), array(), array(), '{"username": 1, "password": "foo"}');
120143
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
121144

122145
$this->listener->handle($event);
123-
$this->assertSame('ko', $event->getResponse()->getContent());
124146
}
125147

148+
/**
149+
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
150+
* @expectedExceptionMessage The key "password" must be a string.
151+
*/
126152
public function testAttemptAuthenticationPasswordNotAString()
127153
{
128154
$this->createListener();
129155
$request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": 1}');
130156
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
131157

132158
$this->listener->handle($event);
133-
$this->assertSame('ko', $event->getResponse()->getContent());
134159
}
135160

136161
public function testAttemptAuthenticationUsernameTooLong()

0 commit comments

Comments
 (0)
0