8000 [Security] Refactor LogoutListener constructor to take options · symfony/symfony@b1f545b · GitHub
[go: up one dir, main page]
Skip to content

Commit b1f545b

Browse files
jmikolajmikola
committed
[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.
1 parent c48c775 commit b1f545b

File tree

4 files changed

+209
-13
lines changed

4 files changed

+209
-13
lines changed

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,10 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
276276
if (isset($firewall['logout'])) {
277277
$listenerId = 'security.logout_listener.'.$id;
278278
$listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.logout_listener'));
279-
$listener->replaceArgument(2, $firewall['logout']['path']);
280-
$listener->replaceArgument(3, $firewall['logout']['target']);
279+
$listener->replaceArgument(2, array(
280+
'logout_path' => $firewall['logout']['path'],
281+
'target_url' => $firewall['logout']['target'],
282+
));
281283
$listeners[] = new Reference($listenerId);
282284

283285
// add logout success handler

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,7 @@
7878
<service id="security.logout_listener" class="%security.logout_listener.class%" public="false" abstract="true">
7979
<argument type="service" id="security.context" />
8080
<argument type="service" id="security.http_utils" />
81-
<argument /> <!-- Logout Path -->
82-
<argument /> <!-- Target-URL Path -->
81+
<argument /> <!-- Options -->
8382
<argument type="service" id="security.logout.success_handler" on-invalid="null" />
8483
</service>
8584
<service id="security.logout.handler.session" class="%security.logout.handler.session.class%" public="false" />

‎src/Symfony/Component/Security/Http/Firewall/LogoutListener.php‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@
2828
class LogoutListener implements ListenerInterface
2929
{
3030
private $securityContext;
31-
private $logoutPath;
32-
private $targetUrl;
31+
private $options;
3332
private $handlers;
3433
private $successHandler;
3534
private $httpUtils;
@@ -39,16 +38,17 @@ class LogoutListener implements ListenerInterface
3938
*
4039
* @param SecurityContextInterface $securityContext
4140
* @param HttpUtils $httpUtils An HttpUtilsInterface instance
42-
* @param string $logoutPath The path that starts the logout process
43-
* @param string $targetUrl The URL to redirect to after logout
41+
* @param array $options An array of options for the processing of a logout attempt
4442
* @param LogoutSuccessHandlerInterface $successHandler
4543
*/
46-
public function __construct(SecurityContextInterface $securityContext, HttpUtils $httpUtils, $logoutPath, $targetUrl = '/', LogoutSuccessHandlerInterface $successHandler = null)
44+
public function __construct(SecurityContextInterface $securityContext, HttpUtils $httpUtils, array $options = array(), LogoutSuccessHandlerInterface $successHandler = null)
4745
{
4846
$this->securityContext = $securityContext;
4947
$this->httpUtils = $httpUtils;
50-
$this->logoutPath = $logoutPath;
51-
$this->targetUrl = $targetUrl;
48+
$this->options = array_merge(array(
49+
'logout_path' => '/logout',
50+
'target_url' => '/',
51+
), $options);
5252
$this->successHandler = $successHandler;
5353
$this->handlers = array();
5454
}
@@ -83,7 +83,7 @@ public function handle(GetResponseEvent $event)
8383
throw new \RuntimeException('Logout Success Handler did not return a Response.');
8484
}
8585
} else {
86-
$response = $this->httpUtils->createRedirectResponse($request, $this->targetUrl);
86+
$response = $this->httpUtils->createRedirectResponse($request, $this->options['target_url']);
8787
}
8888

8989
// handle multiple logout attempts gracefully
@@ -111,6 +111,6 @@ public function handle(GetResponseEvent $event)
111111
*/
112112
protected function requiresLogout(Request $request)
113113
{
114-
return $this->httpUtils->checkRequestPath($request, $this->logoutPath);
114+
return $this->httpUtils->checkRequestPath($request, $this->options['logout_path']);
115115
}
116116
}

‎tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony framework.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* This source file is subject to the MIT license that is bundled
9+
* with this source code in the file LICENSE.
10+
*/
11+
12+
namespace Symfony\Tests\Component\Security\Http\Firewall;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\HttpFoundation\Response;
16+
use Symfony\Component\Security\Http\Firewall\LogoutListener;
17+
18+
class LogoutListenerTest extends \PHPUnit_Framework_TestCase
19+
{
20+
public function testHandleUnmatchedPath()
21+
{
22+
list($listener, $context, $httpUtils, $options) = $this->getListener();
23+
24+
list($event, $request) = $this->getGetResponseEvent();
25+
26+
$event->expects($this->never())
27+
->method('setResponse');
28+
29+
$httpUtils->expects($this->once())
30+
->method('checkRequestPath')
31+
->with($request, $options['logout_path'])
32+
->will($this->returnValue(false));
33+
34+
$listener->handle($event);
35+
}
36+
37+
public function testHandleMatchedPathWithSuccessHandler()
38+
{
39+
$successHandler = $this->getSuccessHandler();
40+
41+
list($listener, $context, $httpUtils, $options) = $this->getListener($successHandler);
42+
43+
list($event, $request) = $this->getGetResponseEvent();
44+
45+
$httpUtils->expects($this->once())
46+
->method('checkRequestPath')
47+
->with($request, $options['logout_path'])
48+
->will($this->returnValue(true));
49+
50+
$successHandler->expects($this->once())
51+
->method('onLogoutSuccess')
52+
->with($request)
53+
->will($this->returnValue($response = new Response()));
54+
55+
$context->expects($this->once())
56+
->method('getToken')
57+
->will($this->returnValue($token = $this->getToken()));
58+
59+
$handler = $this->getHandler();
60+
$handler->expects($this->once())
61+
->method('logout')
62+
->with($request, $response, $token);
63+
64+
$context->expects($this->once())
65+
->method('setToken')
66+
->with(null);
67+
68+
$event->expects($this->once())
69+
->method('setResponse')
70+
->with($response);
71+
72+
$listener->addHandler($handler);
73+
74+
$listener->handle($event);
75+
}
76+
77+
public function testHandleMatchedPathWithoutSuccessHandler()
78+
{
79+
list($listener, $context, $httpUtils, $options) = $this->getListener();
80+
81+
list($event, $request) = $this->getGetResponseEvent();
82+
83+
$httpUtils->expects($this->once())
84+
->method('checkRequestPath')
85+
->with($request, $options['logout_path'])
86+
->will($this->returnValue(true));
87+
88+
$httpUtils->expects($this->once())
89+
->method('createRedirectResponse')
90+
->with($request, $options['target_url'])
91+
->will($this->returnValue($response = new Response()));
92+
93+
$context->expects($this->once())
94+
->method('getToken')
95+
->will($this->returnValue($token = $this->getToken()));
96+
97+
$handler = $this->getHandler();
98+
$handler->expects($this->once())
99+
->method('logout')
100+
->with($request, $response, $token);
101+
102+
$context->expects($this->once())
103+
->method('setToken')
104+
->with(null);
105+
106+
$event->expects($this->once())
107+
->method('setResponse')
108+
->with($response);
109+
110+
$listener->addHandler($handler);
111+
112+
$listener->handle($event);
113+
}
114+
115+
/**
116+
* @expectedException RuntimeException
117+
*/
118+
public function testSuccessHandlerReturnsNonResponse()
119+
{
120+
$successHandler = $this->getSuccessHandler();
121+
122+
list($listener, $context, $httpUtils, $options) = $this->getListener($successHandler);
123+
124+
list($event, $request) = $this->getGetResponseEvent();
125+
126+
$httpUtils->expects($this->once())
127+
->method('checkRequestPath')
128+
->with($request, $options['logout_path'])
129+
->will($this->returnValue(true));
130+
131+
$successHandler->expects($this->once())
132+
->method('onLogoutSuccess')
133+
->with($request)
134+
->will($this->returnValue(null));
135+
136+
$listener->handle($event);
137+
}
138+
139+
private function getContext()
140+
{
141+
return $this->getMockBuilder('Symfony\Component\Security\Core\SecurityContext')
142+
->disableOriginalConstructor()
143+
->getMock();
144+
}
145+
146+
private function getGetResponseEvent()
147+
{
148+
$event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')
149+
->disableOriginalConstructor()
150+
->getMock();
151+
152+
$event->expects($this->any())
153+
->method('getRequest')
154+
->will($this->returnValue($request = new Request()));
155+
156+
return array($event, $request);
157+
}
158+
159+
private function getHandler()
160+
{
161+
return $this->getMock('Symfony\Component\Security\Http\Logout\LogoutHandlerInterface');
162+
}
163+
164+
private function getHttpUtils()
165+
{
166+
return $this->getMockBuilder('Symfony\Component\Security\Http\HttpUtils')
167+
->disableOriginalConstructor()
168+
->getMock();
169+
}
170+
171+
private function getListener($successHandler = null)
172+
{
173+
$listener = new LogoutListener(
174+
$context = $this->getContext(),
175+
$httpUtils = $this->getHttpUtils(),
176+
$options = array(
177+
'logout_path' => '/logout',
178+
'target_url' => '/',
179+
),
180+
$successHandler
181+
);
182+
183+
return array($listener, $context, $httpUtils, $options);
184+
}
185+
186+
private function getSuccessHandler()
187+
{
188+
return $this->getMock('Symfony\Component\Security\Http\Logout\LogoutSuccessHandlerInterface');
189+
}
190+
191+
private function getToken()
192+
{
193+
return $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
194+
}
195+
}

0 commit comments

Comments
 (0)
0