Skip to content

Commit c3d7eb9

Browse files
fabpotfabpot
committed
merged branch alcaeus/ticket_8226 (PR #8528)
This PR was merged into the 2.2 branch. Discussion ---------- [Security] fixed issue where x509 authentication clears unrelated tokens | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #8226 | License | MIT | Doc PR | symfony/symfony-docs#2825 | Notes | Replaces PR #8283 TODO: - [x] Feedback on change to make sure security is not affected - [x] Fix other authentication listeners (they suffer the same problem) - [x] Write unit tests for bug and maybe a few listener classes as well This pull request is the summary of the problem mentioned in the ticket above. It only fixes the "disappearing token" problem for one authentication provider, not all. If acceptable, the change needs to be applied to all authentication listeners since they always clear all tokens from the security context. Commits ------- 2317443 [Security] fixed issue where authentication listeners clear unrelated tokens
2 parents 75bf7a1 + 2317443 commit c3d7eb9

7 files changed

+466
-6
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,10 @@ private function onFailure(GetResponseEvent $event, Request $request, Authentica
186186
$this->logger->info(sprintf('Authentication request failed: %s', $failed->getMessage()));
187187
}
188188

189-
$this->securityContext->setToken(null);
189+
$token = $this->securityContext->getToken();
190+
if ($token instanceof UsernamePasswordToken && $this->providerKey === $token->getProviderKey()) {
191+
$this->securityContext->setToken(null);
192+
}
190193

191194
$response = $this->failureHandler->onAuthenticationFailure($request, $failed);
192195

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Psr\Log\LoggerInterface;
2222
use Symfony\Component\HttpFoundation\Request;
2323
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
24+
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
2425

2526
/**
2627
* AbstractPreAuthenticatedListener is the base class for all listener that
@@ -59,7 +60,12 @@ final public function handle(GetResponseEvent $event)
5960
$this->logger->debug(sprintf('Checking secure context token: %s', $this->securityContext->getToken()));
6061
}
6162

62-
list($user, $credentials) = $this->getPreAuthenticatedData($request);
63+
try {
64+
list($user, $credentials) = $this->getPreAuthenticatedData($request);
65+
} catch (BadCredentialsException $exception) {
66+
$this->clearToken();
67+
return;
68+
}
6369

6470
if (null !== $token = $this->securityContext->getToken()) {
6571
if ($token instanceof PreAuthenticatedToken && $this->providerKey == $token->getProviderKey() && $token->isAuthenticated() && $token->getUsername() === $user) {
@@ -84,6 +90,17 @@ final public function handle(GetResponseEvent $event)
8490
$this->dispatcher->dispatch(SecurityEvents::INTERACTIVE_LOGIN, $loginEvent);
8591
}
8692
} catch (AuthenticationException $failed) {
93+
$this->clearToken();
94+
}
95+
}
96+
97+
/**
98+
* Clears a PreAuthenticatedToken for this provider (if present)
99+
*/
100+
protected function clearToken()
101+
{
102+
$token = $this->securityContext->getToken();
103+
if ($token instanceof PreAuthenticatedToken && $this->providerKey === $token->getProviderKey()) {
87104
$this->securityContext->setToken(null);
88105

89106
if (null !== $this->logger) {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ public function handle(GetResponseEvent $event)
7474
$token = $this->authenticationManager->authenticate(new UsernamePasswordToken($username, $request->headers->get('PHP_AUTH_PW'), $this->providerKey));
7575
$this->securityContext->setToken($token);
7676
} catch (AuthenticationException $failed) {
77-
$this->securityContext->setToken(null);
77+
$token = $this->securityContext->getToken();
78+
if ($token instanceof UsernamePasswordToken && $this->providerKey === $token->getProviderKey()) {
79+
$this->securityContext->setToken(null);
80+
}
7881

7982
if (null !== $this->logger) {
8083
$this->logger->info(sprintf('Authentication request failed for user "%s": %s', $username, $failed->getMessage()));

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,10 @@ public function handle(GetResponseEvent $event)
124124

125125
private function fail(GetResponseEvent $event, Request $request, AuthenticationException $authException)
126126
{
127-
$this->securityContext->setToken(null);
127+
$token = $this->securityContext->getToken();
128+
if ($token instanceof UsernamePasswordToken && $this->providerKey === $token->getProviderKey()) {
129+
$this->securityContext->setToken(null);
130+
}
128131

129132
if (null !== $this->logger) {
130133
$this->logger->info($authException);

‎src/Symfony/Component/Security/Tests/Http/Firewall/AbstractPreAuthenticatedListenerTest.php

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Tests\Http\Firewall;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken;
16+
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
17+
use Symfony\Component\Security\Core\Exception\AuthenticationException;
18+
19+
class AbstractPreAuthenticatedListenerTest extends \PHPUnit_Framework_TestCase
20+
{
21+
protected function setUp()
22+
{
23+
if (!class_exists('Symfony\Component\EventDispatcher\EventDispatcher')) {
24+
$this->markTestSkipped('The "EventDispatcher" component is not available');
25+
}
26+
27+
if (!class_exists('Symfony\Component\HttpFoundation\Request')) {
28+
$this->markTestSkipped('The "HttpFoundation" component is not available');
29+
}
30+
31+
if (!class_exists('Symfony\Component\HttpKernel\HttpKernel')) {
32+
$this->markTestSkipped('The "HttpKernel" component is not available');
33+
}
34+
}
35+
36+
public function testHandleWithValidValues()
37+
{
38+
$userCredentials = array('TheUser', 'TheCredentials');
39+
40+
$request = new Request(array(), array(), array(), array(), array(), array());
41+
42+
$token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
43+
44+
$context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
45+
$context
46+
->expects($this->any())
47+
->method('getToken')
48+
->will($this->returnValue(null))
49+
;
50+
$context
51+
->expects($this->once())
52+
->method('setToken')
53+
->with($this->equalTo($token))
54+
;
55+
56+
$authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
57+
$authenticationManager
58+
->expects($this->once())
59+
->method('authenticate')
60+
->with($this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken'))
61+
->will($this->returnValue($token))
62+
;
63+
64+
$listener = $this->getMockForAbstractClass('Symfony\Component\Security\Http\Firewall\AbstractPreAuthenticatedListener', array(
65+
$context,
66+
$authenticationManager,
67+
'TheProviderKey'
68+
));
69+
$listener
70+
->expects($this->once())
71+
->method('getPreAuthenticatedData')
72+
->will($this->returnValue($userCredentials));
73+
74+
$event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
75+
$event
76+
->expects($this->any())
77+
->method('getRequest')
78+
->will($this->returnValue($request))
79+
;
80+
81+
$listener->handle($event);
82+
}
83+
84+
public function testHandleWhenAuthenticationFails()
85+
{
86+
$userCredentials = array('TheUser', 'TheCredentials');
87+
88+
$request = new Request(array(), array(), array(), array(), array(), array());
89+
90+
$context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
91+
$context
92+
->expects($this->any())
93+
->method('getToken')
94+
->will($this->returnValue(null))
95+
;
96+
$context
97+
->expects($this->never())
98+
->method('setToken')
99+
;
100+
101+
$exception = new AuthenticationException('Authentication failed.');
102+
$authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
103+
$authenticationManager
104+
->expects($this->once())
105+
->method('authenticate')
106+
->with($this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken'))
107+
->will($this->throwException($exception))
108+
;
109+
110+
$listener = $this->getMockForAbstractClass('Symfony\Component\Security\Http\Firewall\AbstractPreAuthenticatedListener', array(
111+
$context,
112+
$authenticationManager,
113+
'TheProviderKey'
114+
));
115+
$listener
116+
->expects($this->once())
117+
->method('getPreAuthenticatedData')
118+
->will($this->returnValue($userCredentials));
119+
120+
$event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
121+
$event
122+
->expects($this->any())
123+
->method('getRequest')
124+
->will($this->returnValue($request))
125+
;
126+
127+
$listener->handle($event);
128+
}
129+
130+
public function testHandleWhenAuthenticationFailsWithDifferentToken()
131+
{
132+
$userCredentials = array('TheUser', 'TheCredentials');
133+
134+
$token = new UsernamePasswordToken('TheUsername', 'ThePassword', 'TheProviderKey', array('ROLE_FOO'));
135+
136+
$request = new Request(array(), array(), array(), array(), array(), array());
137+
138+
$context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
139+
$context
140+
->expects($this->any())
141+
->method('getToken')
142+
->will($this->returnValue($token))
143+
;
144+
$context
145+
->expects($this->never())
146+
->method('setToken')
147+
;
148+
149+
$exception = new AuthenticationException('Authentication failed.');
150+
$authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
151+
$authenticationManager
152+
->expects($this->once())
153+
->method('authenticate')
154+
->with($this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken'))
155+
->will($this->throwException($exception))
156+
;
157+
158+
$listener = $this->getMockForAbstractClass('Symfony\Component\Security\Http\Firewall\AbstractPreAuthenticatedListener', array(
159+
$context,
160+
$authenticationManager,
161+
'TheProviderKey'
162+
));
163+
$listener
164+
->expects($this->once())
165+
->method('getPreAuthenticatedData')
166+
->will($this->returnValue($userCredentials));
167+
168+
$event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
169+
$event
170+
->expects($this->any())
171+
->method('getRequest')
172+
->will($this->returnValue($request))
173+
;
174+
175+
$listener->handle($event);
176+
}
177+
178+
public function testHandleWithASimilarAuthenticatedToken()
179+
{
180+
$userCredentials = array('TheUser', 'TheCredentials');
181+
182+
$request = new Request(array(), array(), array(), array(), array(), array());
183+
184+
$token = new PreAuthenticatedToken('TheUser', 'TheCredentials', 'TheProviderKey', array('ROLE_FOO'));
185+
186+
$context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
187+
$context
188+
->expects($this->any())
189+
->method('getToken')
190+
->will($this->returnValue($token))
191+
;
192+
193+
$authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
194+
$authenticationManager
195+
->expects($this->never())
196+
->method('authenticate')
197+
;
198+
199+
$listener = $this->getMockForAbstractClass('Symfony\Component\Security\Http\Firewall\AbstractPreAuthenticatedListener', array(
200+
$context,
201+
$authenticationManager,
202+
'TheProviderKey'
203+
));
204+
$listener
205+
->expects($this->once())
206+
->method('getPreAuthenticatedData')
207+
->will($this->returnValue($userCredentials));
208+
209+
$event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
210+
$event
211+
->expects($this->any())
212+
->method('getRequest')
213+
->will($this->returnValue($request))
214+
;
215+
216+
$listener->handle($event);
217+
}
218+
219+
public function testHandleWithAnInvalidSimilarToken()
220+
{
221+
$userCredentials = array('TheUser', 'TheCredentials');
222+
223+
$request = new Request(array(), array(), array(), array(), array(), array());
224+
225+
$token = new PreAuthenticatedToken('AnotherUser', 'TheCredentials', 'TheProviderKey', array('ROLE_FOO'));
226+
227+
$context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
228+
$context
229+
->expects($this->any())
230+
->method('getToken')
231+
->will($this->returnValue($token))
232+
;
233+
$context
234+
->expects($this->once())
235+
->method('setToken')
236+
->with($this->equalTo(null))
237+
;
238+
239+
$exception = new AuthenticationException('Authentication failed.');
240+
$authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
241+
$authenticationManager
242+
->expects($this->once())
243+
->method('authenticate')
244+
->with($this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken'))
245+
->will($this->throwException($exception))
246+
;
247+
248+
$listener = $this->getMockForAbstractClass('Symfony\Component\Security\Http\Firewall\AbstractPreAuthenticatedListener', array(
249+
$context,
250+
$authenticationManager,
251+
'TheProviderKey'
252+
));
253+
$listener
254+
->expects($this->once())
255+
->method('getPreAuthenticatedData')
256+
->will($this->returnValue($userCredentials));
257+
258+
$event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
259+
$event
260+
->expects($this->any())
261+
->method('getRequest')
262+
->will($this->returnValue($request))
263+
;
264+
265+
$listener->handle($event);
266+
}
267+
}

0 commit comments

Comments
 (0)