Skip to content
This repository was archived by the owner on May 31, 2024. It is now read-only.

Commit f20bfb2

Browse files
fabpotfabpot
committed
bug #27452 Avoid migration on stateless firewalls (weaverryan)
This PR was squashed before being merged into the 2.8 branch (closes #27452). Discussion ---------- Avoid migration on stateless firewalls | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | Related to #27395 | License | MIT | Doc PR | symfony/symfony-docs#9860 This is a proof-of-concept. Once we agree / are happy, I need to add this to all of the other authentication mechanisms that recently got the session migration code & add tests. Basically, this avoids migrating the session if the firewall is stateless. There were 2 options to do this: A) Make the `SessionAuthenticationStrategy` aware of all stateless firewalls. **This is the current approach** or B) Make each individual authentication listener aware whether or not *its* firewall is stateless. Commits ------- cca73bb564 Avoid migration on stateless firewalls
2 parents 59cc034 + f036be7 commit f20bfb2

6 files changed

+123
-37
lines changed

‎Guard/GuardAuthenticatorHandler.php

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\Security\Core\User\UserInterface;
2121
use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
2222
use Symfony\Component\Security\Http\SecurityEvents;
23+
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;
2324

2425
/**
2526
* A utility class that does much of the *work* during the guard authentication process.
@@ -32,8 +33,8 @@
3233
class GuardAuthenticatorHandler
3334
{
3435
private $tokenStorage;
35-
3636
private $dispatcher;
37+
private $sessionStrategy;
3738

3839
public function __construct(TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher = null)
3940
{
@@ -46,7 +47,7 @@ public function __construct(TokenStorageInterface $tokenStorage, EventDispatcher
4647
*/
4748
public function authenticateWithToken(TokenInterface $token, Request $request)
4849
{
49-
$this->migrateSession($request);
50+
$this->migrateSession($request, $token);
5051
$this->tokenStorage->setToken($token);
5152

5253
if (null !== $this->dispatcher) {
@@ -129,15 +130,22 @@ public function handleAuthenticationFailure(AuthenticationException $authenticat
129130
));
130131
}
131132

132-
private function migrateSession(Request $request)
133+
/**
134+
* Call this method if your authentication token is stored to a session.
135+
*
136+
* @final since version 2.8
137+
*/
138+
public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyInterface $sessionStrategy)
139+
{
140+
$this->sessionStrategy = $sessionStrategy;
141+
}
142+
143+
private function migrateSession(Request $request, TokenInterface $token)
133144
{
134-
if (!$request->hasSession() || !$request->hasPreviousSession()) {
145+
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
135146
return;
136147
}
137148

138-
// Destroying the old session is broken in php 5.4.0 - 5.4.10
139-
// See https://bugs.php.net/63379
140-
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
141-
$request->getSession()->migrate($destroy);
149+
$this->sessionStrategy->onAuthentication($request, $token);
142150
}
143151
}

‎Guard/Tests/GuardAuthenticatorHandlerTest.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class GuardAuthenticatorHandlerTest extends TestCase
2525
private $dispatcher;
2626
private $token;
2727
private $request;
28+
private $sessionStrategy;
2829
private $guardAuthenticator;
2930

3031
public function testAuthenticateWithToken()
@@ -117,12 +118,38 @@ public function getTokenClearingTests()
117118
return $tests;
118119
}
119120

121+
public function testNoFailureIfSessionStrategyNotPassed()
122+
{
123+
$this->configurePreviousSession();
124+
125+
$this->tokenStorage->expects($this->once())
126+
->method('setToken')
127+
->with($this->token);
128+
129+
$handler = new GuardAuthenticatorHandler($this->tokenStorage, $this->dispatcher);
130+
$handler->authenticateWithToken($this->token, $this->request);
131+
}
132+
133+
public function testSessionStrategyIsCalled()
134+
{
135+
$this->configurePreviousSession();
136+
137+
$this->sessionStrategy->expects($this->once())
138+
->method('onAuthentication')
139+
->with($this->request, $this->token);
140+
141+
$handler = new GuardAuthenticatorHandler($this->tokenStorage, $this->dispatcher);
142+
$handler->setSessionAuthenticationStrategy($this->sessionStrategy);
143+
$handler->authenticateWithToken($this->token, $this->request);
144+
}
145+
120146
protected function setUp()
121147
{
122148
$this->tokenStorage = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface')->getMock();
123149
$this->dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock();
124150
$this->token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
125151
$this->request = new Request(array(), array(), array(), array(), array(), array());
152+
$this->sessionStrategy = $this->getMockBuilder('Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface')->getMock();
126153
$this->guardAuthenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock();
127154
}
128155

@@ -134,4 +161,14 @@ protected function tearDown()
134161
$this->request = null;
135162
$this->guardAuthenticator = null;
136163
}
164+
165+
private function configurePreviousSession()
166+
{
167+
$session = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock();
168+
$session->expects($this->any())
169+
->method('getName')
170+
->willReturn('test_session_name');
171+
$this->request->setSession($session);
172+
$this->request->cookies->set('test_session_name', 'session_cookie_val');
173+
}
137174
}

‎Http/Firewall/AbstractPreAuthenticatedListener.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
1515
use Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken;
1616
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
17+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1718
use Symfony\Component\Security\Core\Exception\AuthenticationException;
1819
use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
1920
use Symfony\Component\Security\Http\SecurityEvents;
@@ -22,6 +23,7 @@
2223
use Symfony\Component\HttpFoundation\Request;
2324
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
2425
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
26+
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;
2527

2628
/**
2729
* AbstractPreAuthenticatedListener is the base class for all listener that
@@ -37,6 +39,7 @@ abstract class AbstractPreAuthenticatedListener implements ListenerInterface
3739
private $authenticationManager;
3840
private $providerKey;
3941
private $dispatcher;
42+
private $sessionStrategy;
4043

4144
public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, $providerKey, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null)
4245
{
@@ -83,7 +86,7 @@ final public function handle(GetResponseEvent $event)
8386
$this->logger->info('Pre-authentication successful.', array('token' => (string) $token));
8487
}
8588

86-
$this->migrateSession($request);
89+
$this->migrateSession($request, $token);
8790

8891
$this->tokenStorage->setToken($token);
8992

@@ -96,6 +99,16 @@ final public function handle(GetResponseEvent $event)
9699
}
97100
}
98101

102+
/**
103+
* Call this method if your authentication token is stored to a session.
104+
*
105+
* @final since version 2.8
106+
*/
107+
public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyInterface $sessionStrategy)
108+
{
109+
$this->sessionStrategy = $sessionStrategy;
110+
}
111+
99112
/**
100113
* Clears a PreAuthenticatedToken for this provider (if present).
101114
*/
@@ -118,15 +131,12 @@ private function clearToken(AuthenticationException $exception)
118131
*/
119132
abstract protected function getPreAuthenticatedData(Request $request);
120133

121-
private function migrateSession(Request $request)
134+
private function migrateSession(Request $request, TokenInterface $token)
122135
{
123-
if (!$request->hasSession() || !$request->hasPreviousSession()) {
136+
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
124137
return;
125138
}
126139

127-
// Destroying the old session is broken in php 5.4.0 - 5.4.10
128-
// See https://bugs.php.net/63379
129-
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
130-
$request->getSession()->migrate($destroy);
140+
$this->sessionStrategy->onAuthentication($request, $token);
131141
}
132142
}

‎Http/Firewall/BasicAuthenticationListener.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@
1414
use Symfony\Component\HttpFoundation\Request;
1515
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
1616
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
17+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1718
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
1819
use Psr\Log\LoggerInterface;
1920
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
2021
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2122
use Symfony\Component\Security\Core\Exception\AuthenticationException;
23+
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;
2224

2325
/**
2426
* BasicAuthenticationListener implements Basic HTTP authentication.
@@ -33,6 +35,7 @@ class BasicAuthenticationListener implements ListenerInterface
3335
private $authenticationEntryPoint;
3436
private $logger;
3537
private $ignoreFailure;
38+
private $sessionStrategy;
3639

3740
public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, $providerKey, AuthenticationEntryPointInterface $authenticationEntryPoint, LoggerInterface $logger = null)
3841
{
@@ -72,7 +75,7 @@ public function handle(GetResponseEvent $event)
7275
try {
7376
$token = $this->authenticationManager->authenticate(new UsernamePasswordToken($username, $request->headers->get('PHP_AUTH_PW'), $this->providerKey));
7477

75-
$this->migrateSession($request);
78+
$this->migrateSession($request, $token);
7679

7780
$this->tokenStorage->setToken($token);
7881
} catch (AuthenticationException $e) {
@@ -93,15 +96,22 @@ public function handle(GetResponseEvent $event)
9396
}
9497
}
9598

96-
private function migrateSession(Request $request)
99+
/**
100+
* Call this method if your authentication token is stored to a session.
101+
*
102+
* @final since version 2.8
103+
*/
104+
public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyInterface $sessionStrategy)
105+
{
106+
$this->sessionStrategy = $sessionStrategy;
107+
}
108+
109+
private function migrateSession(Request $request, TokenInterface $token)
97110
{
98-
if (!$request->hasSession() || !$request->hasPreviousSession()) {
111+
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
99112
return;
100113
}
101114

102-
// Destroying the old session is broken in php 5.4.0 - 5.4.10
103-
// See https://bugs.php.net/63379
104-
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
105-
$request->getSession()->migrate($destroy);
115+
$this->sessionStrategy->onAuthentication($request, $token);
106116
}
107117
}

‎Http/Firewall/DigestAuthenticationListener.php

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Security\Http\Firewall;
1313

14+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1415
use Symfony\Component\Security\Core\User\UserProviderInterface;
1516
use Symfony\Component\Security\Http\EntryPoint\DigestAuthenticationEntryPoint;
1617
use Psr\Log\LoggerInterface;
@@ -23,6 +24,7 @@
2324
use Symfony\Component\Security\Core\Exception\NonceExpiredException;
2425
use Symfony\Component\HttpFoundation\Request;
2526
use Symfony\Component\Security\Core\Exception\AuthenticationException;
27+
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;
2628

2729
/**
2830
* DigestAuthenticationListener implements Digest HTTP authentication.
@@ -36,6 +38,7 @@ class DigestAuthenticationListener implements ListenerInterface
3638
private $providerKey;
3739
private $authenticationEntryPoint;
3840
private $logger;
41+
private $sessionStrategy;
3942

4043
public function __construct(TokenStorageInterface $tokenStorage, UserProviderInterface $provider, $providerKey, DigestAuthenticationEntryPoint $authenticationEntryPoint, LoggerInterface $logger = null)
4144
{
@@ -117,9 +120,20 @@ public function handle(GetResponseEvent $event)
117120
$this->logger->info('Digest authentication successful.', array('username' => $digestAuth->getUsername(), 'received' => $digestAuth->getResponse()));
118121
}
119122

120-
$this->migrateSession($request);
123+
$token = new UsernamePasswordToken($user, $user->getPassword(), $this->providerKey);
124+
$this->migrateSession($request, $token);
121125

122-
$this->tokenStorage->setToken(new UsernamePasswordToken($user, $user->getPassword(), $this->providerKey));
126+
$this->tokenStorage->setToken($token);
127+
}
128+
129+
/**
130+
* Call this method if your authentication token is stored to a session.
131+
*
132+
* @final since version 2.8
133+
*/
134+
public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyInterface $sessionStrategy)
135+
{
136+
$this->sessionStrategy = $sessionStrategy;
123137
}
124138

125139
private function fail(GetResponseEvent $event, Request $request, AuthenticationException $authException)
@@ -136,16 +150,13 @@ private function fail(GetResponseEvent $event, Request $request, AuthenticationE
136150
$event->setResponse($this->authenticationEntryPoint->start($request, $authException));
137151
}
138152

139-
private function migrateSession(Request $request)
153+
private function migrateSession(Request $request, TokenInterface $token)
140154
{
141-
if (!$request->hasSession() || !$request->hasPreviousSession()) {
155+
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
142156
return;
143157
}
144158

145-
// Destroying the old session is broken in php 5.4.0 - 5.4.10
146-
// See https://bugs.php.net/63379
147-
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
148-
$request->getSession()->migrate($destroy);
159+
$this->sessionStrategy->onAuthentication($request, $token);
149160
}
150161
}
151162

‎Http/Firewall/SimplePreAuthenticationListener.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919
use Symfony\Component\Security\Core\Authentication\SimplePreAuthenticatorInterface;
2020
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
2121
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
22+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2223
use Symfony\Component\Security\Core\Exception\AuthenticationException;
2324
use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
2425
use Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface;
2526
use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
2627
use Symfony\Component\Security\Http\SecurityEvents;
2728
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
29+
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;
2830

2931
/**
3032
* SimplePreAuthenticationListener implements simple proxying to an authenticator.
@@ -39,6 +41,7 @@ class SimplePreAuthenticationListener implements ListenerInterface
3941
private $simpleAuthenticator;
4042
private $logger;
4143
private $dispatcher;
44+
private $sessionStrategy;
4245

4346
/**
4447
* @param TokenStorageInterface $tokenStorage A TokenStorageInterface instance
@@ -62,6 +65,16 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM
6265
$this->dispatcher = $dispatcher;
6366
}
6467

68+
/**
69+
* Call this method if your authentication token is stored to a session.
70+
*
71+
* @final since version 2.8
72+
*/
73+
public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyInterface $sessionStrategy)
74+
{
75+
$this->sessionStrategy = $sessionStrategy;
76+
}
77+
6578
/**
6679
* Handles basic authentication.
6780
*/
@@ -87,7 +100,7 @@ public function handle(GetResponseEvent $event)
87100

88101
$token = $this->authenticationManager->authenticate($token);
89102

90-
$this->migrateSession($request);
103+
$this->migrateSession($request, $token);
91104

92105
$this->tokenStorage->setToken($token);
93106

@@ -124,15 +137,12 @@ public function handle(GetResponseEvent $event)
124137
}
125138
}
126139

127-
private function migrateSession(Request $request)
140+
private function migrateSession(Request $request, TokenInterface $token)
128141
{
129-
if (!$request->hasSession() || !$request->hasPreviousSession()) {
142+
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
130143
return;
131144
}
132145

133-
// Destroying the old session is broken in php 5.4.0 - 5.4.10
134-
// See https://bugs.php.net/63379
135-
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
136-
$request->getSession()->migrate($destroy);
146+
$this->sessionStrategy->onAuthentication($request, $token);
137147
}
138148
}

0 commit comments

Comments
 (0)