minor #22477 [Security] add Request type json check in json_login (ls…

…mith77) 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 #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
symfony · Apr 29, 2017 · 35608f5 · 35608f5
2 parents 5863e4b + 045a36b
commit 35608f5
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 14 deletions.
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginTest.php
@@ -21,7 +21,7 @@ class JsonLoginTest extends WebTestCase
     public function testDefaultJsonLoginSuccess()
     {
         $client = $this->createClient(array('test_case' => 'JsonLogin', 'root_config' => 'config.yml'));
-        $client->request('POST', '/chk', array(), array(), array(), '{"user": {"login": "dunglas", "password": "foo"}}');
+        $client->request('POST', '/chk', array(), array(), array('CONTENT_TYPE' => 'application/json'), '{"user": {"login": "dunglas", "password": "foo"}}');
         $response = $client->getResponse();
 
         $this->assertInstanceOf(JsonResponse::class, $response);
@@ -32,7 +32,7 @@ public function testDefaultJsonLoginSuccess()
     public function testDefaultJsonLoginFailure()
     {
         $client = $this->createClient(array('test_case' => 'JsonLogin', 'root_config' => 'config.yml'));
-        $client->request('POST', '/chk', array(), array(), array(), '{"user": {"login": "dunglas", "password": "bad"}}');
+        $client->request('POST', '/chk', array(), array(), array('CONTENT_TYPE' => 'application/json'), '{"user": {"login": "dunglas", "password": "bad"}}');
         $response = $client->getResponse();
 
         $this->assertInstanceOf(JsonResponse::class, $response);
@@ -43,7 +43,7 @@ public function testDefaultJsonLoginFailure()
     public function testCustomJsonLoginSuccess()
     {
         $client = $this->createClient(array('test_case' => 'JsonLogin', 'root_config' => 'custom_handlers.yml'));
-        $client->request('POST', '/chk', array(), array(), array(), '{"user": {"login": "dunglas", "password": "foo"}}');
+        $client->request('POST', '/chk', array(), array(), array('CONTENT_TYPE' => 'application/json'), '{"user": {"login": "dunglas", "password": "foo"}}');
         $response = $client->getResponse();
 
         $this->assertInstanceOf(JsonResponse::class, $response);
@@ -54,7 +54,7 @@ public function testCustomJsonLoginSuccess()
     public function testCustomJsonLoginFailure()
     {
         $client = $this->createClient(array('test_case' => 'JsonLogin', 'root_config' => 'custom_handlers.yml'));
-        $client->request('POST', '/chk', array(), array(), array(), '{"user": {"login": "dunglas", "password": "bad"}}');
+        $client->request('POST', '/chk', array(), array(), array('CONTENT_TYPE' => 'application/json'), '{"user": {"login": "dunglas", "password": "bad"}}');
         $response = $client->getResponse();
 
         $this->assertInstanceOf(JsonResponse::class, $response);

diff --git a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php
@@ -75,6 +75,11 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM
     public function handle(GetResponseEvent $event)
     {
         $request = $event->getRequest();
+        if (false === strpos($request->getRequestFormat(), 'json')
+            && false === strpos($request->getContentType(), 'json')
+        ) {
+            return;
+        }
 
         if (isset($this->options['check_path']) && !$this->httpUtils->checkRequestPath($request, $this->options['check_path'])) {
             return;

diff --git a/...Component/Security/Http/Tests/Firewall/UsernamePasswordJsonAuthenticationListenerTest.php b/...Component/Security/Http/Tests/Firewall/UsernamePasswordJsonAuthenticationListenerTest.php
@@ -63,10 +63,21 @@ private function createListener(array $options = array(), $success = true, $matc
         $this->listener = new UsernamePasswordJsonAuthenticationListener($tokenStorage, $authenticationManager, $httpUtils, 'providerKey', $authenticationSuccessHandler, $authenticationFailureHandler, $options);
     }
 
-    public function testHandleSuccess()
+    public function testHandleSuccessIfRequestContentTypeIsJson()
+    {
+        $this->createListener();
+        $request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": "dunglas", "password": "foo"}');
+        $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
+
+        $this->listener->handle($event);
+        $this->assertEquals('ok', $event->getResponse()->getContent());
+    }
+
+    public function testSuccessIfRequestFormatIsJsonLD()
     {
         $this->createListener();
         $request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": "foo"}');
+        $request->setRequestFormat('json-ld');
         $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
 
         $this->listener->handle($event);
@@ -76,7 +87,7 @@ public function testHandleSuccess()
     public function testHandleFailure()
     {
         $this->createListener(array(), false);
-        $request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": "foo"}');
+        $request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": "dunglas", "password": "foo"}');
         $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
 
         $this->listener->handle($event);
@@ -86,7 +97,7 @@ public function testHandleFailure()
     public function testUsePath()
     {
         $this->createListener(array('username_path' => 'user.login', 'password_path' => 'user.pwd'));
-        $request = new Request(array(), array(), array(), array(), array(), array(), '{"user": {"login": "dunglas", "pwd": "foo"}}');
+        $request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"user": {"login": "dunglas", "pwd": "foo"}}');
         $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
 
         $this->listener->handle($event);
@@ -113,7 +124,7 @@ public function testAttemptAuthenticationNoJson()
     public function testAttemptAuthenticationNoUsername()
     {
         $this->createListener();
-        $request = new Request(array(), array(), array(), array(), array(), array(), '{"usr": "dunglas", "password": "foo"}');
+        $request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"usr": "dunglas", "password": "foo"}');
         $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
 
         $this->listener->handle($event);
@@ -126,7 +137,7 @@ public function testAttemptAuthenticationNoUsername()
     public function testAttemptAuthenticationNoPassword()
     {
         $this->createListener();
-        $request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "pass": "foo"}');
+        $request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": "dunglas", "pass": "foo"}');
         $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
 
         $this->listener->handle($event);
@@ -139,7 +150,7 @@ public function testAttemptAuthenticationNoPassword()
     public function testAttemptAuthenticationUsernameNotAString()
     {
         $this->createListener();
-        $request = new Request(array(), array(), array(), array(), array(), array(), '{"username": 1, "password": "foo"}');
+        $request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": 1, "password": "foo"}');
         $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
 
         $this->listener->handle($event);
@@ -152,7 +163,7 @@ public function testAttemptAuthenticationUsernameNotAString()
     public function testAttemptAuthenticationPasswordNotAString()
     {
         $this->createListener();
-        $request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": 1}');
+        $request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": "dunglas", "password": 1}');
         $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
 
         $this->listener->handle($event);
@@ -162,7 +173,7 @@ public function testAttemptAuthenticationUsernameTooLong()
     {
         $this->createListener();
         $username = str_repeat('x', Security::MAX_USERNAME_LENGTH + 1);
-        $request = new Request(array(), array(), array(), array(), array(), array(), sprintf('{"username": "%s", "password": 1}', $username));
+        $request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), sprintf('{"username": "%s", "password": 1}', $username));
         $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
 
         $this->listener->handle($event);
@@ -172,7 +183,18 @@ public function testAttemptAuthenticationUsernameTooLong()
     public function testDoesNotAttemptAuthenticationIfRequestPathDoesNotMatchCheckPath()
     {
         $this->createListener(array('check_path' => '/'), true, false);
-        $request = new Request();
+        $request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'));
+        $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
+        $event->setResponse(new Response('original'));
+
+        $this->listener->handle($event);
+        $this->assertSame('original', $event->getResponse()->getContent());
+    }
+
+    public function testDoesNotAttemptAuthenticationIfRequestContentTypeIsNotJson()
+    {
+        $this->createListener();
+        $request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": "foo"}');
         $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
         $event->setResponse(new Response('original'));
 
@@ -183,7 +205,7 @@ public function testDoesNotAttemptAuthenticationIfRequestPathDoesNotMatchCheckPa
     public function testAttemptAuthenticationIfRequestPathMatchesCheckPath()
     {
         $this->createListener(array('check_path' => '/'));
-        $request = new Request(array(), array(), array(), array(), array(), array(), '{"username": "dunglas", "password": "foo"}');
+        $request = new Request(array(), array(), array(), array(), array(), array('HTTP_CONTENT_TYPE' => 'application/json'), '{"username": "dunglas", "password": "foo"}');
         $event = new GetResponseEvent($this->getMockBuilder(KernelInterface::class)->getMock(), $request, KernelInterface::MASTER_REQUEST);
 
         $this->listener->handle($event);