From b1f545b677b50f4f09bb746daef6f97f0da57f18 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 29 Dec 2011 17:56:38 -0500 Subject: [PATCH] [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'); + } +}