bug #22494 [Security] Fix json_login default success/failure handling (chalasr)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[Security] Fix json_login default success/failure handling

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

This makes the `json_login` listener default configuration stateless oriented by:

- Not using the default (redirect based) failure handler, it returns a 401 (json) response containing the failure reason instead
- Not using the default (redirect based) success handler, just let the original request continue instead (reaching the targeted resource without being redirected).
- Setting `require_previous_session` to `false` by default (I have to set it on `form-login` each time I want it to be stateless)
- Removing the options related to redirections (`default_target_path`, `login_path`, ...) from the listener factory, if one wants redirections then one has to write its own handlers, not the inverse

Commits
-------

9749618ff5 Fix json_login default success/failure handling

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginFactory.php

@@ -28,6 +28,9 @@ class JsonLoginFactory extends AbstractFactory
     {
         $this->addOption('username_path', 'username');
         $this->addOption('password_path', 'password');
+        $this->defaultFailureHandlerOptions = array();
+        $this->defaultSuccessHandlerOptions = array();
+        $this->options['require_previous_session'] = false;
     }
 
     /**
@@ -86,8 +89,8 @@ class JsonLoginFactory extends AbstractFactory
         $listenerId = $this->getListenerId();
         $listener = new ChildDefinition($listenerId);
         $listener->replaceArgument(3, $id);
-        $listener->replaceArgument(4, new Reference($this->createAuthenticationSuccessHandler($container, $id, $config)));
-        $listener->replaceArgument(5, new Reference($this->createAuthenticationFailureHandler($container, $id, $config)));
+        $listener->replaceArgument(4, isset($config['success_handler']) ? new Reference($this->createAuthenticationSuccessHandler($container, $id, $config)) : null);
+        $listener->replaceArgument(5, isset($config['failure_handler']) ? new Reference($this->createAuthenticationFailureHandler($container, $id, $config)) : null);
         $listener->replaceArgument(6, array_intersect_key($config, $this->options));
 
         $listenerId .= '.'.$id;

src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml

@@ -149,8 +149,8 @@
             <argument type="service" id="security.authentication.manager" />
             <argument type="service" id="security.http_utils" />
             <argument /> <!-- Provider-shared Key -->
-            <argument type="service" id="security.authentication.success_handler" />
-            <argument type="service" id="security.authentication.failure_handler" />
+            <argument /> <!-- Failure handler -->
+            <argument /> <!-- Success Handler -->
             <argument type="collection" /> <!-- Options -->
             <argument type="service" id="logger" on-invalid="null" />
             <argument type="service" id="event_dispatcher" on-invalid="null" />

src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Controller/TestController.php

@@ -11,13 +11,16 @@
 
 namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLoginBundle\Controller;
 
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\Security\Core\User\UserInterface;
+
 /**
  * @author Kévin Dunglas <dunglas@gmail.com>
  */
 class TestController
 {
-    public function loginCheckAction()
+    public function loginCheckAction(UserInterface $user)
     {
-        throw new \RuntimeException(sprintf('%s should never be called.', __FUNCTION__));
+        return new JsonResponse(array('message' => sprintf('Welcome @%s!', $user->getUsername())));
     }
 }

src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Security/Http/JsonAuthenticationFailureHandler.php

@@ -0,0 +1,25 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLoginBundle\Security\Http;
+
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
+
+class JsonAuthenticationFailureHandler implements AuthenticationFailureHandlerInterface
+{
+    public function onAuthenticationFailure(Request $request, AuthenticationException $exception)
+    {
+        return new JsonResponse(array('message' => 'Something went wrong'), 500);
+    }
+}

src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Security/Http/JsonAuthenticationSuccessHandler.php

@@ -0,0 +1,25 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLoginBundle\Security\Http;
+
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface;
+
+class JsonAuthenticationSuccessHandler implements AuthenticationSuccessHandlerInterface
+{
+    public function onAuthenticationSuccess(Request $request, TokenInterface $token)
+    {
+        return new JsonResponse(array('message' => sprintf('Good game @%s!', $token->getUsername())));
+    }
+}

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

@@ -11,22 +11,54 @@
 
 namespace Symfony\Bundle\SecurityBundle\Tests\Functional;
 
+use Symfony\Component\HttpFoundation\JsonResponse;
+
 /**
  * @author Kévin Dunglas <dunglas@gmail.com>
  */
 class JsonLoginTest extends WebTestCase
 {
-    public function testJsonLoginSuccess()
+    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"}}');
-        $this->assertEquals('http://localhost/', $client->getResponse()->headers->get('location'));
+        $response = $client->getResponse();
+
+        $this->assertInstanceOf(JsonResponse::class, $response);
+        $this->assertSame(200, $response->getStatusCode());
+        $this->assertSame(array('message' => 'Welcome @dunglas!'), json_decode($response->getContent(), true));
     }
 
-    public function testJsonLoginFailure()
+    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"}}');
-        $this->assertEquals('http://localhost/login', $client->getResponse()->headers->get('location'));
+        $response = $client->getResponse();
+
+        $this->assertInstanceOf(JsonResponse::class, $response);
+        $this->assertSame(401, $response->getStatusCode());
+        $this->assertSame(array('error' => 'Invalid credentials.'), json_decode($response->getContent(), true));
+    }
+
+    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"}}');
+        $response = $client->getResponse();
+
+        $this->assertInstanceOf(JsonResponse::class, $response);
+        $this->assertSame(200, $response->getStatusCode());
+        $this->assertSame(array('message' => 'Good game @dunglas!'), json_decode($response->getContent(), true));
+    }
+
+    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"}}');
+        $response = $client->getResponse();
+
+        $this->assertInstanceOf(JsonResponse::class, $response);
+        $this->assertSame(500, $response->getStatusCode());
+        $this->assertSame(array('message' => 'Something went wrong'), json_decode($response->getContent(), true));
     }
 }

src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLogin/custom_handlers.yml

@@ -0,0 +1,32 @@
+imports:
+    - { resource: ./../config/framework.yml }
+
+security:
+    encoders:
+        Symfony\Component\Security\Core\User\User: plaintext
+
+    providers:
+        in_memory:
+            memory:
+                users:
+                    dunglas: { password: foo, roles: [ROLE_USER] }
+
+    firewalls:
+        main:
+            pattern: ^/
+            anonymous: true
+            json_login:
+               check_path:    /chk
+               username_path: user.login
+               password_path: user.password
+               success_handler: json_login.success_handler
+               failure_handler: json_login.failure_handler
+
+    access_control:
+        - { path: ^/foo, roles: ROLE_USER }
+
+services:
+    json_login.success_handler:
+        class: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLoginBundle\Security\Http\JsonAuthenticationSuccessHandler
+    json_login.failure_handler:
+        class: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLoginBundle\Security\Http\JsonAuthenticationFailureHandler

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

@@ -13,6 +13,7 @@ namespace Symfony\Component\Security\Http\Firewall;
 
 use Psr\Log\LoggerInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
@@ -53,7 +54,7 @@ class UsernamePasswordJsonAuthenticationListener implements ListenerInterface
     private $eventDispatcher;
     private $propertyAccessor;
 
-    public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, HttpUtils $httpUtils, $providerKey, AuthenticationSuccessHandlerInterface $successHandler, AuthenticationFailureHandlerInterface $failureHandler, array $options = array(), LoggerInterface $logger = null, EventDispatcherInterface $eventDispatcher = null, PropertyAccessorInterface $propertyAccessor = null)
+    public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, HttpUtils $httpUtils, $providerKey, AuthenticationSuccessHandlerInterface $successHandler = null, AuthenticationFailureHandlerInterface $failureHandler = null, array $options = array(), LoggerInterface $logger = null, EventDispatcherInterface $eventDispatcher = null, PropertyAccessorInterface $propertyAccessor = null)
     {
         $this->tokenStorage = $tokenStorage;
         $this->authenticationManager = $authenticationManager;
@@ -117,6 +118,10 @@ class UsernamePasswordJsonAuthenticationListener implements ListenerInterface
             $response = $this->onFailure($request, $e);
         }
 
+        if (null === $response) {
+            return;
+        }
+
         $event->setResponse($response);
     }
 
@@ -133,6 +138,10 @@ class UsernamePasswordJsonAuthenticationListener implements ListenerInterface
             $this->eventDispatcher->dispatch(SecurityEvents::INTERACTIVE_LOGIN, $loginEvent);
         }
 
+        if (!$this->successHandler) {
+            return; // let the original request succeeds
+        }
+
         $response = $this->successHandler->onAuthenticationSuccess($request, $token);
 
         if (!$response instanceof Response) {
@@ -153,6 +162,10 @@ class UsernamePasswordJsonAuthenticationListener implements ListenerInterface
             $this->tokenStorage->setToken(null);
         }
 
+        if (!$this->failureHandler) {
+            return new JsonResponse(array('error' => $failed->getMessageKey()), 401);
+        }
+
         $response = $this->failureHandler->onAuthenticationFailure($request, $failed);
 
         if (!$response instanceof Response) {