Skip to content

Commit 35608f5

Browse files
fabpotfabpot
committed
minor symfony#22477 [Security] add Request type json check in json_login (lsmith77)
This PR was merged into the 3.3-dev branch. Discussion ---------- [Security] add Request type json check in json_login | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no, unreleased feature | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | - follow up to symfony#22425 to limit the `UsernamePasswordJsonAuthenticationListener` to only requests with appropriate JSON content type. I am not entirely happy with this implementation but mostly because Symfony out of the box only provides very limited content type negotiation. I guess anyone that wants to tweak the content negotiation will simply need to ensure the Request::$format is set accordingly before the code is triggered. Commits ------- 045a36b add Request type json check in json_login
2 parents 5863e4b + 045a36b commit 35608f5

File tree

3 files changed

+41
-14
lines changed

3 files changed

+41
-14
lines changed

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class JsonLoginTest extends WebTestCase
2121
public function testDefaultJsonLoginSuccess()
2222
{
2323
$client = $this->createClient(array('test_case' => 'JsonLogin', 'root_config' => 'config.yml'));
24-
$client->request('POST', '/chk', array(), array(), array(), '{"user": {"login": "dunglas", "password": "foo"}}');
24+
$client->request('POST', '/chk', array(), array(), array('CONTENT_TYPE' => 'application/json'), '{"user": {"login": "dunglas", "password": "foo"}}');
2525
$response = $client->getResponse();
2626

2727
$this->assertInstanceOf(JsonResponse::class, $response);
@@ -32,7 +32,7 @@ public function testDefaultJsonLoginSuccess()
3232
public function testDefaultJsonLoginFailure()
3333
{
3434
$client = $this->createClient(array('test_case' => 'JsonLogin', 'root_config' => 'config.yml'));
35-
$client->request('POST', '/chk', array(), array(), array(), '{"user": {"login": "dunglas", "password": "bad"}}');
35+
$client->request('POST', '/chk', array(), array(), array('CONTENT_TYPE' => 'application/json'), '{"user": {"login": "dunglas", "password": "bad"}}');
3636
$response = $client->getResponse();
3737

3838
$this->assertInstanceOf(JsonResponse::class, $response);
@@ -43,7 +43,7 @@ public function testDefaultJsonLoginFailure()
4343
public function testCustomJsonLoginSuccess()
4444
{
4545
$client = $this->createClient(array('test_case' => 'JsonLogin', 'root_config' => 'custom_handlers.yml'));
46-
$client->request('POST', '/chk', array(), array(), array(), '{"user": {"login": "dunglas", "password": "foo"}}');
46+
$client->request('POST', '/chk', array(), array(), array('CONTENT_TYPE' => 'application/json'), '{"user": {"login": "dunglas", "password": "foo"}}');
4747
$response = $client->getResponse();
4848

4949
$this->assertInstanceOf(JsonResponse::class, $response);
@@ -54,7 +54,7 @@ public function testCustomJsonLoginSuccess()
5454
public function testCustomJsonLoginFailure()
5555
{
5656
$client = $this->createClient(array('test_case' => 'JsonLogin', 'root_config' => 'custom_handlers.yml'));
57-
$client->request('POST', '/chk', array(), array(), array(), '{"user": {"login": "dunglas", "password": "bad"}}');
57+
$client->request('POST', '/chk', array(), array(), array('CONTENT_TYPE' => 'application/json'), '{"user": {"login": "dunglas", "password": "bad"}}');
5858
$response = $client->getResponse();
5959

6060
$this->assertInstanceOf(JsonResponse::class, $response);

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM
7575
public function handle(GetResponseEvent $event)
7676
{
7777
$request = $event->getRequest();
78+
if (false === strpos($request->getRequestFormat(), 'json')
79+
&& false === strpos($request->getContentType(), 'json')
80+
) {
81+
return;
82+
}
7883

7984
if (isset($this->options['check_path']) && !$this->httpUtils->checkRequestPath($request, $this->options['check_path'])) {
8085
return;

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

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,21 @@ private function createListener(array $options = array(), $success = true, $matc
6363
$this->listener = new UsernamePasswordJsonAuthenticationListener($tokenStorage, $authenticationManager, $httpUtils, 'providerKey', $authenticationSuccessHandler, $authenticationFailureHandler, $options);
6464
}
6565

66-
public function testHandleSuccess()
66+
public function testHandleSuccessIfRequestContentTypeIsJson()
67+
{
68+
$this->createListener();
69+
$request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": "dunglas", "password": "foo"}');
70+
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
71+
72+
$this->listener->handle($event);
73+
$this->assertEquals('ok', $event->getResponse()->getContent());
74+
}
75+
76+
public function testSuccessIfRequestFormatIsJsonLD()
6777
{
6878
$this->createListener();
6979
$request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": "foo"}');
80+
$request->setRequestFormat('json-ld');
7081
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
7182

7283
$this->listener->handle($event);
@@ -76,7 +87,7 @@ public function testHandleSuccess()
7687
public function testHandleFailure()
7788
{
7889
$this->createListener(array(), false);
79-
$request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": "foo"}');
90+
$request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": "dunglas", "password": "foo"}');
8091
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
8192

8293
$this->listener->handle($event);
@@ -86,7 +97,7 @@ public function testHandleFailure()
8697
public function testUsePath()
8798
{
8899
$this->createListener(array('username_path' => 'user.login', 'password_path' => 'user.pwd'));
89-
$request = new Request(array(), array(), array(), array(), array(), array(), '{"user": {"login": "dunglas", "pwd": "foo"}}');
100+
$request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"user": {"login": "dunglas", "pwd": "foo"}}');
90101
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
91102

92103
$this->listener->handle($event);
@@ -113,7 +124,7 @@ public function testAttemptAuthenticationNoJson()
113124
public function testAttemptAuthenticationNoUsername()
114125
{
115126
$this->createListener();
116-
$request = new Request(array(), array(), array(), array(), array(), array(), '{"usr": "dunglas", "password": "foo"}');
127+
$request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"usr": "dunglas", "password": "foo"}');
117128
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
118129

119130
$this->listener->handle($event);
@@ -126,7 +137,7 @@ public function testAttemptAuthenticationNoUsername()
126137
public function testAttemptAuthenticationNoPassword()
127138
{
128139
$this->createListener();
129-
$request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "pass": "foo"}');
140+
$request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": "dunglas", "pass": "foo"}');
130141
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
131142

132143
$this->listener->handle($event);
@@ -139,7 +150,7 @@ public function testAttemptAuthenticationNoPassword()
139150
public function testAttemptAuthenticationUsernameNotAString()
140151
{
141152
$this->createListener();
142-
$request = new Request(array(), array(), array(), array(), array(), array(), '{"username": 1, "password": "foo"}');
153+
$request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": 1, "password": "foo"}');
143154
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
144155

145156
$this->listener->handle($event);
@@ -152,7 +163,7 @@ public function testAttemptAuthenticationUsernameNotAString()
152163
public function testAttemptAuthenticationPasswordNotAString()
153164
{
154165
$this->createListener();
155-
$request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": 1}');
166+
$request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": "dunglas", "password": 1}');
156167
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
157168

158169
$this->listener->handle($event);
@@ -162,7 +173,7 @@ public function testAttemptAuthenticationUsernameTooLong()
162173
{
163174
$this->createListener();
164175
$username = str_repeat('x', Security::MAX_USERNAME_LENGTH + 1);
165-
$request = new Request(array(), array(), array(), array(), array(), array(), sprintf('{"username": "%s", "password": 1}', $username));
176+
$request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), sprintf('{"username": "%s", "password": 1}', $username));
166177
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
167178

168179
$this->listener->handle($event);
@@ -172,7 +183,18 @@ public function testAttemptAuthenticationUsernameTooLong()
172183
public function testDoesNotAttemptAuthenticationIfRequestPathDoesNotMatchCheckPath()
173184
{
174185
$this->createListener(array('check_path' => '/'), true, false);
175-
$request = new Request();
186+
$request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'));
187+
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
188+
$event->setResponse(new Response('original'));
189+
190+
$this->listener->handle($event);
191+
$this->assertSame('original', $event->getResponse()->getContent());
192+
}
193+
194+
public function testDoesNotAttemptAuthenticationIfRequestContentTypeIsNotJson()
195+
{
196+
$this->createListener();
197+
$request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": "foo"}');
176198
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
177199
$event->setResponse(new Response('original'));
178200

@@ -183,7 +205,7 @@ public function testDoesNotAttemptAuthenticationIfRequestPathDoesNotMatchCheckPa
183205
public function testAttemptAuthenticationIfRequestPathMatchesCheckPath()
184206
{
185207
$this->createListener(array('check_path' => '/'));
186-
$request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": "foo"}');
208+
$request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": "dunglas", "password": "foo"}');
187209
$event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
188210

189211
$this->listener->handle($event);

0 commit comments

Comments
 (0)