From c48c775018222765ade7d29b153077fc683aa790 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Dec 2011 20:55:45 -0500 Subject: [PATCH 1/7] [SecurityBundle] Add functional test for form login with CSRF token --- .../Controller/LoginController.php | 44 +++++++ .../CsrfFormLoginBundle.php | 18 +++ .../Form/UserLoginFormType.php | 96 ++++++++++++++ .../Resources/config/routing.yml | 30 +++++ .../views/Login/after_login.html.twig | 6 + .../Resources/views/Login/login.html.twig | 12 ++ .../Tests/Functional/CsrfFormLoginTest.php | 121 ++++++++++++++++++ .../Functional/app/CsrfFormLogin/bundles.php | 8 ++ .../Functional/app/CsrfFormLogin/config.yml | 43 +++++++ .../app/CsrfFormLogin/routes_as_path.yml | 13 ++ .../Functional/app/CsrfFormLogin/routing.yml | 2 + 11 files changed, 393 insertions(+) create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Controller/LoginController.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/CsrfFormLoginBundle.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Form/UserLoginFormType.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/config/routing.yml create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/after_login.html.twig create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/login.html.twig create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/bundles.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/config.yml create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/routes_as_path.yml create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/routing.yml diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Controller/LoginController.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Controller/LoginController.php new file mode 100644 index 0000000000..a7482c67ec --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Controller/LoginController.php @@ -0,0 +1,44 @@ + + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\CsrfFormLoginBundle\Controller; + +use Symfony\Component\DependencyInjection\ContainerAware; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Security\Core\SecurityContextInterface; +use Symfony\Component\Security\Core\Exception\AccessDeniedException; + +class LoginController extends ContainerAware +{ + public function loginAction() + { + $form = $this->container->get('form.factory')->create('user_login'); + + return $this->container->get('templating')->renderResponse('CsrfFormLoginBundle:Login:login.html.twig', array( + 'form' => $form->createView(), + )); + } + + public function afterLoginAction() + { + return $this->container->get('templating')->renderResponse('CsrfFormLoginBundle:Login:after_login.html.twig'); + } + + public function loginCheckAction() + { + return new Response('', 400); + } + + public function secureAction() + { + throw new \Exception('Wrapper', 0, new \Exception('Another Wrapper', 0, new AccessDeniedException())); + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/CsrfFormLoginBundle.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/CsrfFormLoginBundle.php new file mode 100644 index 0000000000..322e72effe --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/CsrfFormLoginBundle.php @@ -0,0 +1,18 @@ + + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\CsrfFormLoginBundle; + +use Symfony\Component\HttpKernel\Bundle\Bundle; + +class CsrfFormLoginBundle extends Bundle +{ +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Form/UserLoginFormType.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Form/UserLoginFormType.php new file mode 100644 index 0000000000..c0b7959324 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Form/UserLoginFormType.php @@ -0,0 +1,96 @@ + + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\CsrfFormLoginBundle\Form; + +use Symfony\Component\Form\AbstractType; +use Symfony\Component\Form\FormBuilder; +use Symfony\Component\Form\FormError; +use Symfony\Component\Form\FormEvents; +use Symfony\Component\Form\Event\FilterDataEvent; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\SecurityContextInterface; + +/** + * Form type for use with the Security component's form-based authentication + * listener. + * + * @author Henrik Bjornskov + * @author Jeremy Mikola + */ +class UserLoginFormType extends AbstractType +{ + private $reqeust; + + /** + * @param Request $request A request instance + */ + public function __construct(Request $request) + { + $this->request = $request; + } + + /** + * @see Symfony\Component\Form\AbstractType::buildForm() + */ + public function buildForm(FormBuilder $builder, array $options) + { + $builder + ->add('username', 'text') + ->add('password', 'password') + ->add('_target_path', 'hidden') + ; + + $request = $this->request; + + /* Note: since the Security component's form login listener intercepts + * the POST request, this form will never really be bound to the + * request; however, we can match the expected behavior by checking the + * session for an authentication error and last username. + */ + $builder->addEventListener(FormEvents::SET_DATA, function (FilterDataEvent $event) use ($request) { + if ($request->attributes->has(SecurityContextInterface::AUTHENTICATION_ERROR)) { + $error = $request->attributes->get(SecurityContextInterface::AUTHENTICATION_ERROR); + } else { + $error = $request->getSession()->get(SecurityContextInterface::AUTHENTICATION_ERROR); + } + + if ($error) { + $event->getForm()->addError(new FormError($error->getMessage())); + } + + $event->setData(array_replace((array) $event->getData(), array( + 'username' => $request->getSession()->get(SecurityContextInterface::LAST_USERNAME), + ))); + }); + } + + /** + * @see Symfony\Component\Form\AbstractType::getDefaultOptions() + */ + public function getDefaultOptions(array $options) + { + /* Note: the form's intention must correspond to that for the form login + * listener in order for the CSRF token to validate successfully. + */ + return array( + 'intention' => 'authenticate', + ); + } + + /** + * @see Symfony\Component\Form\FormTypeInterface::getName() + */ + public function getName() + { + return 'user_login'; + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/config/routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/config/routing.yml new file mode 100644 index 0000000000..0c9842bb9a --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/config/routing.yml @@ -0,0 +1,30 @@ +form_login: + pattern: /login + defaults: { _controller: CsrfFormLoginBundle:Login:login } + +form_login_check: + pattern: /login_check + defaults: { _controller: CsrfFormLoginBundle:Login:loginCheck } + +form_login_homepage: + pattern: / + defaults: { _controller: CsrfFormLoginBundle:Login:afterLogin } + +form_login_custom_target_path: + pattern: /foo + defaults: { _controller: CsrfFormLoginBundle:Login:afterLogin } + +form_login_default_target_path: + pattern: /profile + defaults: { _controller: CsrfFormLoginBundle:Login:afterLogin } + +form_login_redirect_to_protected_resource_after_login: + pattern: /protected-resource + defaults: { _controller: CsrfFormLoginBundle:Login:afterLogin } + +form_logout: + pattern: /logout_path + +form_secure_action: + pattern: /secure-but-not-covered-by-access-control + defaults: { _controller: CsrfFormLoginBundle:Login:secure } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/after_login.html.twig b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/after_login.html.twig new file mode 100644 index 0000000000..9972b48ae7 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/after_login.html.twig @@ -0,0 +1,6 @@ +{% extends "::base.html.twig" %} + +{% block body %} + Hello {{ app.user.username }}!

+ You're browsing to path "{{ app.request.pathInfo }}". +{% endblock %} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/login.html.twig b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/login.html.twig new file mode 100644 index 0000000000..36ae0151d0 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/login.html.twig @@ -0,0 +1,12 @@ +{% extends "::base.html.twig" %} + +{% block body %} + +
+ {{ form_widget(form) }} + + {# Note: ensure the submit name does not conflict with the form's name or it may clobber field data #} + +
+ +{% endblock %} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php new file mode 100644 index 0000000000..52b533c591 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php @@ -0,0 +1,121 @@ + + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Symfony\Bundle\SecurityBundle\Tests\Functional; + +/** + * @group functional + */ +class CsrfFormLoginTest extends WebTestCase +{ + /** + * @dataProvider getConfigs + */ + public function testFormLogin($config) + { + $client = $this->createClient(array('test_case' => 'CsrfFormLogin', 'root_config' => $config)); + $client->insulate(); + + $form = $client->request('GET', '/login')->selectButton('login')->form(); + $form['user_login[username]'] = 'johannes'; + $form['user_login[password]'] = 'test'; + $client->submit($form); + + $this->assertRedirect($client->getResponse(), '/profile'); + + $text = $client->followRedirect()->text(); + $this->assertContains('Hello johannes!', $text); + $this->assertContains('You\'re browsing to path "/profile".', $text); + } + + /** + * @dataProvider getConfigs + */ + public function testFormLoginWithInvalidCsrfToken($config) + { + $client = $this->createClient(array('test_case' => 'CsrfFormLogin', 'root_config' => $config)); + $client->insulate(); + + $form = $client->request('GET', '/login')->selectButton('login')->form(); + $form['user_login[_token]'] = ''; + $client->submit($form); + + $this->assertRedirect($client->getResponse(), '/login'); + + $text = $client->followRedirect()->text(); + $this->assertContains('Invalid CSRF token.', $text); + } + + /** + * @dataProvider getConfigs + */ + public function testFormLoginWithCustomTargetPath($config) + { + $client = $this->createClient(array('test_case' => 'CsrfFormLogin', 'root_config' => $config)); + $client->insulate(); + + $form = $client->request('GET', '/login')->selectButton('login')->form(); + $form['user_login[username]'] = 'johannes'; + $form['user_login[password]'] = 'test'; + $form['user_login[_target_path]'] = '/foo'; + $client->submit($form); + + $this->assertRedirect($client->getResponse(), '/foo'); + + $text = $client->followRedirect()->text(); + $this->assertContains('Hello johannes!', $text); + $this->assertContains('You\'re browsing to path "/foo".', $text); + } + + /** + * @dataProvider getConfigs + */ + public function testFormLoginRedirectsToProtectedResourceAfterLogin($config) + { + $client = $this->createClient(array('test_case' => 'CsrfFormLogin', 'root_config' => $config)); + $client->insulate(); + + $client->request('GET', '/protected-resource'); + $this->assertRedirect($client->getResponse(), '/login'); + + $form = $client->followRedirect()->selectButton('login')->form(); + $form['user_login[username]'] = 'johannes'; + $form['user_login[password]'] = 'test'; + $client->submit($form); + $this->assertRedirect($client->getResponse(), '/protected-resource'); + + $text = $client->followRedirect()->text(); + $this->assertContains('Hello johannes!', $text); + $this->assertContains('You\'re browsing to path "/protected-resource".', $text); + } + + public function getConfigs() + { + return array( + array('config.yml'), + array('routes_as_path.yml'), + ); + } + + protected function setUp() + { + parent::setUp(); + + $this->deleteTmpDir('CsrfFormLogin'); + } + + protected function tearDown() + { + parent::tearDown(); + + $this->deleteTmpDir('CsrfFormLogin'); + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/bundles.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/bundles.php new file mode 100644 index 0000000000..cee883f9cb --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/bundles.php @@ -0,0 +1,8 @@ + Date: Thu, 29 Dec 2011 17:56:38 -0500 Subject: [PATCH 2/7] [Security] Refactor LogoutListener constructor to take options This will facilitate adding additional options for CSRF protection. Additionally, a unit test for existing behavior was added. --- .../DependencyInjection/SecurityExtension.php | 6 +- .../Resources/config/security_listeners.xml | 3 +- .../Security/Http/Firewall/LogoutListener.php | 18 +- .../Http/Firewall/LogoutListenerTest.php | 195 ++++++++++++++++++ 4 files changed, 209 insertions(+), 13 deletions(-) create mode 100644 tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index bf13395365..3c3f30f106 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -276,8 +276,10 @@ class SecurityExtension extends Extension if (isset($firewall['logout'])) { $listenerId = 'security.logout_listener.'.$id; $listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.logout_listener')); - $listener->replaceArgument(2, $firewall['logout']['path']); - $listener->replaceArgument(3, $firewall['logout']['target']); + $listener->replaceArgument(2, array( + 'logout_path' => $firewall['logout']['path'], + 'target_url' => $firewall['logout']['target'], + )); $listeners[] = new Reference($listenerId); // add logout success handler diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml index 430e419606..6f088bfe2d 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml @@ -78,8 +78,7 @@ - - + diff --git a/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php b/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php index bb90b6abd3..d79445724b 100644 --- a/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php @@ -28,8 +28,7 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent; class LogoutListener implements ListenerInterface { private $securityContext; - private $logoutPath; - private $targetUrl; + private $options; private $handlers; private $successHandler; private $httpUtils; @@ -39,16 +38,17 @@ class LogoutListener implements ListenerInterface * * @param SecurityContextInterface $securityContext * @param HttpUtils $httpUtils An HttpUtilsInterface instance - * @param string $logoutPath The path that starts the logout process - * @param string $targetUrl The URL to redirect to after logout + * @param array $options An array of options for the processing of a logout attempt * @param LogoutSuccessHandlerInterface $successHandler */ - public function __construct(SecurityContextInterface $securityContext, HttpUtils $httpUtils, $logoutPath, $targetUrl = '/', LogoutSuccessHandlerInterface $successHandler = null) + public function __construct(SecurityContextInterface $securityContext, HttpUtils $httpUtils, array $options = array(), LogoutSuccessHandlerInterface $successHandler = null) { $this->securityContext = $securityContext; $this->httpUtils = $httpUtils; - $this->logoutPath = $logoutPath; - $this->targetUrl = $targetUrl; + $this->options = array_merge(array( + 'logout_path' => '/logout', + 'target_url' => '/', + ), $options); $this->successHandler = $successHandler; $this->handlers = array(); } @@ -83,7 +83,7 @@ class LogoutListener implements ListenerInterface throw new \RuntimeException('Logout Success Handler did not return a Response.'); } } else { - $response = $this->httpUtils->createRedirectResponse($request, $this->targetUrl); + $response = $this->httpUtils->createRedirectResponse($request, $this->options['target_url']); } // handle multiple logout attempts gracefully @@ -111,6 +111,6 @@ class LogoutListener implements ListenerInterface */ protected function requiresLogout(Request $request) { - return $this->httpUtils->checkRequestPath($request, $this->logoutPath); + return $this->httpUtils->checkRequestPath($request, $this->options['logout_path']); } } diff --git a/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php b/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php new file mode 100644 index 0000000000..465ba9e6b8 --- /dev/null +++ b/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php @@ -0,0 +1,195 @@ + + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Symfony\Tests\Component\Security\Http\Firewall; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Security\Http\Firewall\LogoutListener; + +class LogoutListenerTest extends \PHPUnit_Framework_TestCase +{ + public function testHandleUnmatchedPath() + { + list($listener, $context, $httpUtils, $options) = $this->getListener(); + + list($event, $request) = $this->getGetResponseEvent(); + + $event->expects($this->never()) + ->method('setResponse'); + + $httpUtils->expects($this->once()) + ->method('checkRequestPath') + ->with($request, $options['logout_path']) + ->will($this->returnValue(false)); + + $listener->handle($event); + } + + public function testHandleMatchedPathWithSuccessHandler() + { + $successHandler = $this->getSuccessHandler(); + + list($listener, $context, $httpUtils, $options) = $this->getListener($successHandler); + + list($event, $request) = $this->getGetResponseEvent(); + + $httpUtils->expects($this->once()) + ->method('checkRequestPath') + ->with($request, $options['logout_path']) + ->will($this->returnValue(true)); + + $successHandler->expects($this->once()) + ->method('onLogoutSuccess') + ->with($request) + ->will($this->returnValue($response = new Response())); + + $context->expects($this->once()) + ->method('getToken') + ->will($this->returnValue($token = $this->getToken())); + + $handler = $this->getHandler(); + $handler->expects($this->once()) + ->method('logout') + ->with($request, $response, $token); + + $context->expects($this->once()) + ->method('setToken') + ->with(null); + + $event->expects($this->once()) + ->method('setResponse') + ->with($response); + + $listener->addHandler($handler); + + $listener->handle($event); + } + + public function testHandleMatchedPathWithoutSuccessHandler() + { + list($listener, $context, $httpUtils, $options) = $this->getListener(); + + list($event, $request) = $this->getGetResponseEvent(); + + $httpUtils->expects($this->once()) + ->method('checkRequestPath') + ->with($request, $options['logout_path']) + ->will($this->returnValue(true)); + + $httpUtils->expects($this->once()) + ->method('createRedirectResponse') + ->with($request, $options['target_url']) + ->will($this->returnValue($response = new Response())); + + $context->expects($this->once()) + ->method('getToken') + ->will($this->returnValue($token = $this->getToken())); + + $handler = $this->getHandler(); + $handler->expects($this->once()) + ->method('logout') + ->with($request, $response, $token); + + $context->expects($this->once()) + ->method('setToken') + ->with(null); + + $event->expects($this->once()) + ->method('setResponse') + ->with($response); + + $listener->addHandler($handler); + + $listener->handle($event); + } + + /** + * @expectedException RuntimeException + */ + public function testSuccessHandlerReturnsNonResponse() + { + $successHandler = $this->getSuccessHandler(); + + list($listener, $context, $httpUtils, $options) = $this->getListener($successHandler); + + list($event, $request) = $this->getGetResponseEvent(); + + $httpUtils->expects($this->once()) + ->method('checkRequestPath') + ->with($request, $options['logout_path']) + ->will($this->returnValue(true)); + + $successHandler->expects($this->once()) + ->method('onLogoutSuccess') + ->with($request) + ->will($this->returnValue(null)); + + $listener->handle($event); + } + + private function getContext() + { + return $this->getMockBuilder('Symfony\Component\Security\Core\SecurityContext') + ->disableOriginalConstructor() + ->getMock(); + } + + private function getGetResponseEvent() + { + $event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->any()) + ->method('getRequest') + ->will($this->returnValue($request = new Request())); + + return array($event, $request); + } + + private function getHandler() + { + return $this->getMock('Symfony\Component\Security\Http\Logout\LogoutHandlerInterface'); + } + + private function getHttpUtils() + { + return $this->getMockBuilder('Symfony\Component\Security\Http\HttpUtils') + ->disableOriginalConstructor() + ->getMock(); + } + + private function getListener($successHandler = null) + { + $listener = new LogoutListener( + $context = $this->getContext(), + $httpUtils = $this->getHttpUtils(), + $options = array( + 'logout_path' => '/logout', + 'target_url' => '/', + ), + $successHandler + ); + + return array($listener, $context, $httpUtils, $options); + } + + private function getSuccessHandler() + { + return $this->getMock('Symfony\Component\Security\Http\Logout\LogoutSuccessHandlerInterface'); + } + + private function getToken() + { + return $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface'); + } +} From aaaa04003d0a442d5f1adda7b2b8d364aef89257 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Dec 2011 00:08:04 -0500 Subject: [PATCH 3/7] [Security] Allow LogoutListener to validate CSRF tokens This adds several new options to the logout listener, modeled after the form_login listener: * csrf_parameter * intention * csrf_provider The "csrf_parameter" and "intention" have default values if omitted. By default, "csrf_provider" is empty and CSRF validation is disabled in LogoutListener (preserving BC). If a service ID is given for "csrf_provider", CSRF validation will be enabled. Invalid tokens will result in an InvalidCsrfTokenException being thrown before any logout handlers are invoked. --- .../DependencyInjection/MainConfiguration.php | 3 + .../DependencyInjection/SecurityExtension.php | 11 +++- .../Security/Http/Firewall/LogoutListener.php | 39 +++++++++---- .../Http/Firewall/LogoutListenerTest.php | 56 ++++++++++++++++--- 4 files changed, 90 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index 70065d8e17..5f975628ef 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -200,6 +200,9 @@ class MainConfiguration implements ConfigurationInterface ->treatTrueLike(array()) ->canBeUnset() ->children() + ->scalarNode('csrf_parameter')->defaultValue('_csrf_token')->end() + ->scalarNode('csrf_provider')->cannotBeEmpty()->end() + ->scalarNode('intention')->defaultValue('logout')->end() ->scalarNode('path')->defaultValue('/logout')->end() ->scalarNode('target')->defaultValue('/')->end() ->scalarNode('success_handler')->end() diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 3c3f30f106..abdca9a2e9 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -277,8 +277,10 @@ class SecurityExtension extends Extension $listenerId = 'security.logout_listener.'.$id; $listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.logout_listener')); $listener->replaceArgument(2, array( - 'logout_path' => $firewall['logout']['path'], - 'target_url' => $firewall['logout']['target'], + 'csrf_parameter' => $firewall['logout']['csrf_parameter'], + 'intention' => $firewall['logout']['intention'], + 'logout_path' => $firewall['logout']['path'], + 'target_url' => $firewall['logout']['target'], )); $listeners[] = new Reference($listenerId); @@ -287,6 +289,11 @@ class SecurityExtension extends Extension $listener->replaceArgument(4, new Reference($firewall['logout']['success_handler'])); } + // add CSRF provider + if (isset($firewall['logout']['csrf_provider'])) { + $listener->addArgument(new Reference($firewall['logout']['csrf_provider'])); + } + // add session logout handler if (true === $firewall['logout']['invalidate_session'] && false === $firewall['stateless']) { $listener->addMethodCall('addHandler', array(new Reference('security.logout.handler.session'))); diff --git a/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php b/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php index d79445724b..f4d0b2cced 100644 --- a/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php @@ -11,14 +11,15 @@ namespace Symfony\Component\Security\Http\Firewall; -use Symfony\Component\Security\Http\Logout\LogoutSuccessHandlerInterface; - -use Symfony\Component\Security\Http\Logout\LogoutHandlerInterface; -use Symfony\Component\Security\Core\SecurityContextInterface; -use Symfony\Component\Security\Http\HttpUtils; +use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseEvent; +use Symfony\Component\Security\Core\SecurityContextInterface; +use Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException; +use Symfony\Component\Security\Http\HttpUtils; +use Symfony\Component\Security\Http\Logout\LogoutHandlerInterface; +use Symfony\Component\Security\Http\Logout\LogoutSuccessHandlerInterface; /** * LogoutListener logout users. @@ -32,24 +33,29 @@ class LogoutListener implements ListenerInterface private $handlers; private $successHandler; private $httpUtils; + private $csrfProvider; /** * Constructor * * @param SecurityContextInterface $securityContext * @param HttpUtils $httpUtils An HttpUtilsInterface instance - * @param array $options An array of options for the processing of a logout attempt - * @param LogoutSuccessHandlerInterface $successHandler + * @param array $options An array of options to process a logout attempt + * @param LogoutSuccessHandlerInterface $successHandler A LogoutSuccessHandlerInterface instance + * @param CsrfProviderInterface $csrfProvider A CsrfProviderInterface instance */ - public function __construct(SecurityContextInterface $securityContext, HttpUtils $httpUtils, array $options = array(), LogoutSuccessHandlerInterface $successHandler = null) + public function __construct(SecurityContextInterface $securityContext, HttpUtils $httpUtils, array $options = array(), LogoutSuccessHandlerInterface $successHandler = null, CsrfProviderInterface $csrfProvider = null) { $this->securityContext = $securityContext; $this->httpUtils = $httpUtils; $this->options = array_merge(array( - 'logout_path' => '/logout', - 'target_url' => '/', + 'csrf_parameter' => '_csrf_token', + 'intention' => 'logout', + 'logout_path' => '/logout', + 'target_url' => '/', ), $options); $this->successHandler = $successHandler; + $this->csrfProvider = $csrfProvider; $this->handlers = array(); } @@ -66,7 +72,12 @@ class LogoutListener implements ListenerInterface /** * Performs the logout if requested * + * If a CsrfProviderInterface instance is available, it will be used to + * validate the request. + * * @param GetResponseEvent $event A GetResponseEvent instance + * @throws InvalidCsrfTokenException if the CSRF token is invalid + * @throws RuntimeException if the LogoutSuccessHandlerInterface instance does not return a response */ public function handle(GetResponseEvent $event) { @@ -76,6 +87,14 @@ class LogoutListener implements ListenerInterface return; } + if (null !== $this->csrfProvider) { + $csrfToken = $request->get($this->options['csrf_parameter'], null, true); + + if (false === $this->csrfProvider->isCsrfTokenValid($this->options['intention'], $csrfToken)) { + throw new InvalidCsrfTokenException('Invalid CSRF token.'); + } + } + if (null !== $this->successHandler) { $response = $this->successHandler->onLogoutSuccess($request); diff --git a/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php b/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php index 465ba9e6b8..2e275db93b 100644 --- a/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php +++ b/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php @@ -34,19 +34,27 @@ class LogoutListenerTest extends \PHPUnit_Framework_TestCase $listener->handle($event); } - public function testHandleMatchedPathWithSuccessHandler() + public function testHandleMatchedPathWithSuccessHandlerAndCsrfValidation() { $successHandler = $this->getSuccessHandler(); + $csrfProvider = $this->getCsrfProvider(); - list($listener, $context, $httpUtils, $options) = $this->getListener($successHandler); + list($listener, $context, $httpUtils, $options) = $this->getListener($successHandler, $csrfProvider); list($event, $request) = $this->getGetResponseEvent(); + $request->query->set('_csrf_token', $csrfToken = 'token'); + $httpUtils->expects($this->once()) ->method('checkRequestPath') ->with($request, $options['logout_path']) ->will($this->returnValue(true)); + $csrfProvider->expects($this->once()) + ->method('isCsrfTokenValid') + ->with('logout', $csrfToken) + ->will($this->returnValue(true)); + $successHandler->expects($this->once()) ->method('onLogoutSuccess') ->with($request) @@ -74,7 +82,7 @@ class LogoutListenerTest extends \PHPUnit_Framework_TestCase $listener->handle($event); } - public function testHandleMatchedPathWithoutSuccessHandler() + public function testHandleMatchedPathWithoutSuccessHandlerAndCsrfValidation() { list($listener, $context, $httpUtils, $options) = $this->getListener(); @@ -136,6 +144,37 @@ class LogoutListenerTest extends \PHPUnit_Framework_TestCase $listener->handle($event); } + /** + * @expectedException Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException + */ + public function testCsrfValidationFails() + { + $csrfProvider = $this->getCsrfProvider(); + + list($listener, $context, $httpUtils, $options) = $this->getListener(null, $csrfProvider); + + list($event, $request) = $this->getGetResponseEvent(); + + $request->query->set('_csrf_token', $csrfToken = 'token'); + + $httpUtils->expects($this->once()) + ->method('checkRequestPath') + ->with($request, $options['logout_path']) + ->will($this->returnValue(true)); + + $csrfProvider->expects($this->once()) + ->method('isCsrfTokenValid') + ->with('logout', $csrfToken) + ->will($this->returnValue(false)); + + $listener->handle($event); + } + + private function getCsrfProvider() + { + return $this->getMock('Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface'); + } + private function getContext() { return $this->getMockBuilder('Symfony\Component\Security\Core\SecurityContext') @@ -168,16 +207,19 @@ class LogoutListenerTest extends \PHPUnit_Framework_TestCase ->getMock(); } - private function getListener($successHandler = null) + private function getListener($successHandler = null, $csrfProvider = null) { $listener = new LogoutListener( $context = $this->getContext(), $httpUtils = $this->getHttpUtils(), $options = array( - 'logout_path' => '/logout', - 'target_url' => '/', + 'csrf_parameter' => '_csrf_token', + 'intention' => 'logout', + 'logout_path' => '/logout', + 'target_url' => '/', ), - $successHandler + $successHandler, + $csrfProvider ); return array($listener, $context, $httpUtils, $options); From 66722b3d2e478dea671c50f514ad2cb3fad9b2c8 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Dec 2011 21:43:06 -0500 Subject: [PATCH 4/7] [SecurityBundle] Templating helpers to generate logout URL's with CSRF tokens As each firewall is configured, its logout listener (if any) will be registered with the LogoutUrlHelper service. In a template, this helper may be used to generate relative or absolute URL's to a particular firewall's logout path. A CSRF token will be appended to the URL as necessary. The Twig extension composes the helper service to avoid code duplication (see: #2999). --- .../DependencyInjection/SecurityExtension.php | 12 ++ .../Resources/config/templating_php.xml | 7 ++ .../Resources/config/templating_twig.xml | 7 ++ .../Templating/Helper/LogoutUrlHelper.php | 119 ++++++++++++++++++ .../views/Login/after_login.html.twig | 4 +- .../Tests/Functional/CsrfFormLoginTest.php | 15 ++- .../Functional/app/CsrfFormLogin/config.yml | 4 + .../Twig/Extension/LogoutUrlExtension.php | 75 +++++++++++ 8 files changed, 240 insertions(+), 3 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/Templating/Helper/LogoutUrlHelper.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Twig/Extension/LogoutUrlExtension.php diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index abdca9a2e9..41060baf97 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -312,6 +312,18 @@ class SecurityExtension extends Extension foreach ($firewall['logout']['handlers'] as $handlerId) { $listener->addMethodCall('addHandler', array(new Reference($handlerId))); } + + // register with LogoutUrlHelper + $container + ->getDefinition('templating.helper.logout_url') + ->addMethodCall('registerListener', array( + $id, + $firewall['logout']['path'], + $firewall['logout']['intention'], + $firewall['logout']['csrf_parameter'], + isset($firewall['logout']['csrf_provider']) ? new Reference($firewall['logout']['csrf_provider']) : null, + )) + ; } // Authentication listeners diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_php.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_php.xml index f2c2b121c8..83cb5597c1 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_php.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_php.xml @@ -5,10 +5,17 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> + Symfony\Bundle\SecurityBundle\Templating\Helper\LogoutUrlHelper Symfony\Bundle\SecurityBundle\Templating\Helper\SecurityHelper + + + + + + diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_twig.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_twig.xml index 14b0414d06..5271c1434e 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_twig.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_twig.xml @@ -5,9 +5,16 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> + Symfony\Bundle\SecurityBundle\Twig\Extension\LogoutUrlExtension Symfony\Bundle\SecurityBundle\Twig\Extension\SecurityExtension + + + + + + diff --git a/src/Symfony/Bundle/SecurityBundle/Templating/Helper/LogoutUrlHelper.php b/src/Symfony/Bundle/SecurityBundle/Templating/Helper/LogoutUrlHelper.php new file mode 100644 index 0000000000..0d5229c928 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Templating/Helper/LogoutUrlHelper.php @@ -0,0 +1,119 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\Templating\Helper; + +use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; +use Symfony\Component\Templating\Helper\Helper; + +/** + * LogoutUrlHelper provides generator functions for the logout URL. + * + * @author Jeremy Mikola + */ +class LogoutUrlHelper extends Helper +{ + private $listeners; + private $request; + private $router; + + /** + * Constructor. + * + * @param Request $request A request instance + * @param UrlGeneratorInterface $router A Router instance + */ + public function __construct(Request $request, UrlGeneratorInterface $router) + { + $this->request = $request; + $this->router = $router; + $this->listeners = array(); + } + + /** + * Registers a firewall's LogoutListener, allowing its URL to be generated. + * + * @param string $key The firewall key + * @param string $logoutPath The path that starts the logout process + * @param string $intention The intention for CSRF token generation + * @param string $csrfParameter The CSRF token parameter name + * @param CsrfProviderInterface $csrfProvider A CsrfProviderInterface instance + */ + public function registerListener($key, $logoutPath, $intention, $csrfParameter, CsrfProviderInterface $csrfProvider = null) + { + $this->listeners[$key] = array($logoutPath, $intention, $csrfParameter, $csrfProvider); + } + + /** + * Generate the relative logout URL for the firewall. + * + * @param string $key The firewall key + * @return string The relative logout URL + */ + public function getLogoutPath($key) + { + return $this->generateLogoutUrl($key, false); + } + + /** + * Generate the absolute logout URL for the firewall. + * + * @param string $key The firewall key + * @return string The absolute logout URL + */ + public function getLogoutUrl($key) + { + return $this->generateLogoutUrl($key, true); + } + + /** + * Generate the logout URL for the firewall. + * + * @param string $key The firewall key + * @param Boolean $absolute Whether to generate an absolute URL + * @return string The logout URL + * @throws InvalidArgumentException if no LogoutListener is registered for the key + */ + private function generateLogoutUrl($key, $absolute) + { + if (!array_key_exists($key, $this->listeners)) { + throw new \InvalidArgumentException(sprintf('No LogoutListener found for firewall key "%s".', $key)); + } + + list($logoutPath, $intention, $csrfParameter, $csrfProvider) = $this->listeners[$key]; + + $parameters = null !== $csrfProvider ? array($csrfParameter => $csrfProvider->generateCsrfToken($intention)) : array(); + + if ('/' === $logoutPath[0]) { + $url = ($absolute ? $this->request->getUriForPath($logoutPath) : $this->request->getBasePath() . $logoutPath); + + if (!empty($parameters)) { + $url .= '?' . http_build_query($parameters); + } + } else { + $url = $this->router->generate($logoutPath, $parameters, $absolute); + } + + return $url; + } + + /** + * Returns the canonical name of this helper. + * + * @return string The canonical name + */ + public function getName() + { + return 'logout_url'; + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/after_login.html.twig b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/after_login.html.twig index 9972b48ae7..b56c186ecb 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/after_login.html.twig +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/views/Login/after_login.html.twig @@ -2,5 +2,7 @@ {% block body %} Hello {{ app.user.username }}!

- You're browsing to path "{{ app.request.pathInfo }}". + You're browsing to path "{{ app.request.pathInfo }}".

+ Log out. + Log out. {% endblock %} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php index 52b533c591..1440c79425 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php @@ -19,7 +19,7 @@ class CsrfFormLoginTest extends WebTestCase /** * @dataProvider getConfigs */ - public function testFormLogin($config) + public function testFormLoginAndLogoutWithCsrfTokens($config) { $client = $this->createClient(array('test_case' => 'CsrfFormLogin', 'root_config' => $config)); $client->insulate(); @@ -31,9 +31,20 @@ class CsrfFormLoginTest extends WebTestCase $this->assertRedirect($client->getResponse(), '/profile'); - $text = $client->followRedirect()->text(); + $crawler = $client->followRedirect(); + + $text = $crawler->text(); $this->assertContains('Hello johannes!', $text); $this->assertContains('You\'re browsing to path "/profile".', $text); + + $logoutLinks = $crawler->selectLink('Log out')->links(); + $this->assertEquals(2, count($logoutLinks)); + $this->assertContains('_csrf_token=', $logoutLinks[0]->getUri()); + $this->assertSame($logoutLinks[0]->getUri(), $logoutLinks[1]->getUri()); + + $client->click($logoutLinks[0]); + + $this->assertRedirect($client->getResponse(), '/'); } /** diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/config.yml index a4d77c2c3e..1f1fa85d0a 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/config.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/CsrfFormLogin/config.yml @@ -38,6 +38,10 @@ security: csrf_parameter: "user_login[_token]" csrf_provider: form.csrf_provider anonymous: ~ + logout: + path: /logout_path + target: / + csrf_provider: form.csrf_provider access_control: - { path: .*, roles: IS_AUTHENTICATED_FULLY } diff --git a/src/Symfony/Bundle/SecurityBundle/Twig/Extension/LogoutUrlExtension.php b/src/Symfony/Bundle/SecurityBundle/Twig/Extension/LogoutUrlExtension.php new file mode 100644 index 0000000000..bad5e79ee8 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Twig/Extension/LogoutUrlExtension.php @@ -0,0 +1,75 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\Twig\Extension; + +use Symfony\Bundle\SecurityBundle\Templating\Helper\LogoutUrlHelper; + +/** + * LogoutUrlHelper provides generator functions for the logout URL to Twig. + * + * @author Jeremy Mikola + */ +class LogoutUrlExtension extends \Twig_Extension +{ + private $helper; + + /** + * Constructor. + * + * @param LogoutUrlHelper $helper + */ + public function __construct(LogoutUrlHelper $helper) + { + $this->helper = $helper; + } + + /** + * @see Twig_Extension::getFunctions() + */ + public function getFunctions() + { + return array( + 'logout_url' => new \Twig_Function_Method($this, 'getLogoutUrl'), + 'logout_path' => new \Twig_Function_Method($this, 'getLogoutPath'), + ); + } + + /** + * Generate the relative logout URL for the firewall. + * + * @param string $key The firewall key + * @return string The relative logout URL + */ + public function getLogoutPath($key) + { + return $this->helper->getLogoutPath($key); + } + + /** + * Generate the absolute logout URL for the firewall. + * + * @param string $key The firewall key + * @return string The absolute logout URL + */ + public function getLogoutUrl($key) + { + return $this->helper->getLogoutUrl($key); + } + + /** + * @see Twig_ExtensionInterface::getName() + */ + public function getName() + { + return 'logout_url'; + } +} From 4837407527091ec03dbdf15ae47f67e8160b8f6f Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Dec 2011 20:37:35 -0500 Subject: [PATCH 5/7] [SecurityBundle] Fix execution of functional tests with different names Using "securitybundletest" as the default environment for the functional test's kernel causes a PHP fatal error redeclaring the class "appSecuritybundletestDebugProjectContainer" when multiple tests (with unique names) are executed. In lieu of forcing tests to specify their own environment explicitly, we can simply append the test name into the environment. Note: this bug may be related to PHPUnit executing multiple tests within the same process. --- .../Bundle/SecurityBundle/Tests/Functional/WebTestCase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/WebTestCase.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/WebTestCase.php index 29a9dd464b..cb8b817e3e 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/WebTestCase.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/WebTestCase.php @@ -60,7 +60,7 @@ class WebTestCase extends BaseWebTestCase return new $class( $options['test_case'], isset($options['root_config']) ? $options['root_config'] : 'config.yml', - isset($options['environment']) ? $options['environment'] : 'securitybundletest', + isset($options['environment']) ? $options['environment'] : 'securitybundletest' . strtolower($options['test_case']), isset($options['debug']) ? $options['debug'] : true ); } From a96105e3321e1cde5e0e0c7434030310e04c1add Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Sat, 31 Dec 2011 02:58:50 -0500 Subject: [PATCH 6/7] [SecurityBundle] Use assertCount() in tests See: fd174a228b2964ae6f362d2f2b22ee2e353b5803 --- .../SecurityBundle/Tests/Functional/CsrfFormLoginTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php index 1440c79425..65890daf10 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php @@ -38,7 +38,7 @@ class CsrfFormLoginTest extends WebTestCase $this->assertContains('You\'re browsing to path "/profile".', $text); $logoutLinks = $crawler->selectLink('Log out')->links(); - $this->assertEquals(2, count($logoutLinks)); + $this->assertCount(2, $logoutLinks); $this->assertContains('_csrf_token=', $logoutLinks[0]->getUri()); $this->assertSame($logoutLinks[0]->getUri(), $logoutLinks[1]->getUri()); From 49a8654cb8074f648cd13060a2b85ccd2f136c75 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Mon, 6 Feb 2012 14:54:38 -0500 Subject: [PATCH 7/7] [Security] Use LogoutException for invalid CSRF token in LogoutListener On the advice of @schmittjoh, this commit adds a LogoutException class for use by LogoutListener if the CSRF token is invalid. The handling in the Security component's ExceptionListener is modeled after AccessDeniedException, which gets wrapped in an AccessDeniedHttpException in the absence of handler service or error page (I didn't think it was appropriate to re-use those for LogoutException). --- .../Core/Exception/LogoutException.php | 25 +++++++++++++++++++ .../Http/Firewall/ExceptionListener.php | 9 +++++++ .../Security/Http/Firewall/LogoutListener.php | 4 +-- .../Http/Firewall/LogoutListenerTest.php | 2 +- 4 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 src/Symfony/Component/Security/Core/Exception/LogoutException.php diff --git a/src/Symfony/Component/Security/Core/Exception/LogoutException.php b/src/Symfony/Component/Security/Core/Exception/LogoutException.php new file mode 100644 index 0000000000..2bb954fa78 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Exception/LogoutException.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\Component\Security\Core\Exception; + +/** + * LogoutException is thrown when the account cannot be logged out. + * + * @author Jeremy Mikola + */ +class LogoutException extends \RuntimeException +{ + public function __construct($message = 'Logout Exception', \Exception $previous = null) + { + parent::__construct($message, 403, $previous); + } +} diff --git a/src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php b/src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php index 674c648f81..0996ab2e77 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php @@ -20,6 +20,7 @@ use Symfony\Component\Security\Core\Exception\AccountStatusException; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Exception\InsufficientAuthenticationException; +use Symfony\Component\Security\Core\Exception\LogoutException; use Symfony\Component\Security\Http\HttpUtils; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Log\LoggerInterface; @@ -140,6 +141,14 @@ class ExceptionListener return; } } + } elseif ($exception instanceof LogoutException) { + if (null !== $this->logger) { + $this->logger->info(sprintf('Logout exception occurred; wrapping with AccessDeniedHttpException (%s)', $exception->getMessage())); + } + + $event->setException(new AccessDeniedHttpException($exception->getMessage(), $exception)); + + return; } else { return; } diff --git a/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php b/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php index f4d0b2cced..59172dcbf5 100644 --- a/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php @@ -16,7 +16,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\Security\Core\SecurityContextInterface; -use Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException; +use Symfony\Component\Security\Core\Exception\LogoutException; use Symfony\Component\Security\Http\HttpUtils; use Symfony\Component\Security\Http\Logout\LogoutHandlerInterface; use Symfony\Component\Security\Http\Logout\LogoutSuccessHandlerInterface; @@ -91,7 +91,7 @@ class LogoutListener implements ListenerInterface $csrfToken = $request->get($this->options['csrf_parameter'], null, true); if (false === $this->csrfProvider->isCsrfTokenValid($this->options['intention'], $csrfToken)) { - throw new InvalidCsrfTokenException('Invalid CSRF token.'); + throw new LogoutException('Invalid CSRF token.'); } } diff --git a/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php b/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php index 2e275db93b..73c9f1fd6e 100644 --- a/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php +++ b/tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php @@ -145,7 +145,7 @@ class LogoutListenerTest extends \PHPUnit_Framework_TestCase } /** - * @expectedException Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException + * @expectedException Symfony\Component\Security\Core\Exception\LogoutException */ public function testCsrfValidationFails() {