From 9749618ff5606bfa1fc4860c36b009ea4c101b98 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Thu, 20 Apr 2017 23:05:53 +0200 Subject: [PATCH] Fix json_login default success/failure handling --- .../Security/Factory/JsonLoginFactory.php | 7 +++- .../Resources/config/security_listeners.xml | 4 +- .../Controller/TestController.php | 7 +++- .../Http/JsonAuthenticationFailureHandler.php | 25 ++++++++++++ .../Http/JsonAuthenticationSuccessHandler.php | 25 ++++++++++++ .../Tests/Functional/JsonLoginTest.php | 40 +++++++++++++++++-- .../app/JsonLogin/custom_handlers.yml | 32 +++++++++++++++ ...namePasswordJsonAuthenticationListener.php | 15 ++++++- 8 files changed, 144 insertions(+), 11 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Security/Http/JsonAuthenticationFailureHandler.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Security/Http/JsonAuthenticationSuccessHandler.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLogin/custom_handlers.yml diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginFactory.php index f7da4aa4a6..c69cab8949 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginFactory.php +++ b/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; diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml index 1eb7687a5d..45ebde81f1 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml @@ -149,8 +149,8 @@ - - + + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Controller/TestController.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Controller/TestController.php index ae32244364..3effab9b59 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Controller/TestController.php +++ b/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 */ 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()))); } } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Security/Http/JsonAuthenticationFailureHandler.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Security/Http/JsonAuthenticationFailureHandler.php new file mode 100644 index 0000000000..fbd482ddff --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Security/Http/JsonAuthenticationFailureHandler.php @@ -0,0 +1,25 @@ + + * + * 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); + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Security/Http/JsonAuthenticationSuccessHandler.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Security/Http/JsonAuthenticationSuccessHandler.php new file mode 100644 index 0000000000..0e65bbb56b --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLoginBundle/Security/Http/JsonAuthenticationSuccessHandler.php @@ -0,0 +1,25 @@ + + * + * 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()))); + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginTest.php index 5d13a2d649..fbee1d98aa 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginTest.php +++ b/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 */ 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)); } } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLogin/custom_handlers.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLogin/custom_handlers.yml new file mode 100644 index 0000000000..e15e203c62 --- /dev/null +++ b/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 diff --git a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php index 2cfd4d9d23..c7a61d3a02 100644 --- a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php +++ b/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) {