8000 bug #27581 Fix bad method call with guard authentication + session mi… · symfony/symfony@2643ec8 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2643ec8

Browse files
author
Robin Chalas
committed
bug #27581 Fix bad method call with guard authentication + session migration (weaverryan)
This PR was squashed before being merged into the 2.8 branch (closes #27581). Discussion ---------- Fix bad method call with guard authentication + session migration | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no (but there needs to be on master) | Tests pass? | yes | Fixed tickets | #27577 | License | MIT | Doc PR | n/a I messed up #27452 :/. Guard is the one class where the session migration is not on the listener, it's on the handler. The tricky part is that there is only ONE handler (unlike listeners where there is 1 listener per firewall). That means that implementing a session migration strategy that avoids stateless firewalls was a bit more tricky: I could only think to inject a map into `GuardAuthenticationHandler`. On the bright side, this also fixes session migration (not happening) when people call the `authenticateUserAndHandleSuccess()` method directly. On master, we'll need to add a deprecation to make the 3rd argument of `authenticateWithToken()` required - it's optional now for BC. We may also need to re-order the constructor args. I DID test this in a real 2.8 project, to make sure that things were properly wired up. Apologies for not doing that for the other PR. Cheers! Commits ------- 2c0ac93 Fix bad method call with guard authentication + session migration
2 parents 5c2b2bb + 2c0ac93 commit 2643ec8

File tree

7 files changed

+64
-8
lines changed

7 files changed

+64
-8
lines changed

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
7777
$listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.authentication.listener.guard'));
7878
$listener->replaceArgument(2, $id);
7979
$listener->replaceArgument(3, $authenticatorReferences);
80-
$listener->addMethodCall('setSessionAuthenticationStrategy', array(new Reference('security.authentication.session_strategy.'.$id)));
8180

8281
// determine the entryPointId to use
8382
$entryPointId = $this->determineEntryPoint($defaultEntryPoint, $config);

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class SecurityExtension extends Extension
3939
private $factories = array();
4040
private $userProviderFactories = array();
4141
private $expressionLanguage;
42+
private $statelessFirewallKeys = array();
4243

4344
public function __construct()
4445
{
@@ -89,6 +90,9 @@ public function load(array $configs, ContainerBuilder $container)
8990
$this->createAuthorization($config, $container);
9091
$this->createRoleHierarchy($config, $container);
9192

93+
$container->getDefinition('security.authentication.guard_handler')
94+
->replaceArgument(2, $this->statelessFirewallKeys);
95+
9296
if ($config['encoders']) {
9397
$this->createEncoders($config['encoders'], $container);
9498
}
@@ -287,6 +291,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
287291
$listeners[] = new Reference($this->createContextListener($container, $contextKey));
288292
$sessionStrategyId = 'security.authentication.session_strategy';
289293
} else {
294+
$this->statelessFirewallKeys[] = $id;
290295
$sessionStrategyId = 'security.authentication.session_strategy_noop';
291296
}
292297
$container->setAlias(new Alias('security.authentication.session_strategy.'.$id, false), $sessionStrategyId);

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
>
1111
<argument type="service" id="security.token_storage" />
1212
<argument type="service" id="event_dispatcher" on-invalid="null" />
13+
<argument /> <!-- stateless firewall keys -->
14+
<call method="setSessionAuthenticationStrategy">
15+
<argument type="service" id="security.authentication.session_strategy" />
16+
</call>
1317
</service>
1418

1519
<!-- See GuardAuthenticationFactory -->

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,33 @@ public function testDisableRoleHierarchyVoter()
119119
$this->assertFalse($container->hasDefinition('security.access.role_hierarchy_voter'));
120120
}
121121

122+
public function testGuardHandlerIsPassedStatelessFirewalls()
123+
{
124+
$container = $this->getRawContainer();
125+
126+
$container->loadFromExtension('security', array(
127+
'providers' => array(
128+
'default' => array('id' => 'foo'),
129+
),
130+
131+
'firewalls' => array(
132+
'some_firewall' => array(
133+
'pattern' => '^/admin',
134+
'http_basic' => null,
135+
),
136+
'stateless_firewall' => array(
137+
'pattern' => '/.*',
138+
'stateless' => true,
139+
'http_basic' => null,
140+
),
141+
),
142+
));
143+
144+
$container->compile();
145+
$definition = $container->getDefinition('security.authentication.guard_handler');
146+
$this->assertSame(array('stateless_firewall'), $definition->getArgument(2));
147+
}
148+
122149
protected function getRawContainer()
123150
{
124151
$container = new ContainerBuilder();

‎src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorIn
117117
}
118118

119119
// sets the token on the token storage, etc
120-
$this->guardHandler->authenticateWithToken($token, $request);
120+
$this->guardHandler->authenticateWithToken($token, $request, $this->providerKey);
121121
} catch (AuthenticationException $e) {
122122
// oh no! Authentication failed!
123123

‎src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,28 @@ class GuardAuthenticatorHandler
3535
private $tokenStorage;
3636
private $dispatcher;
3737
private $sessionStrategy;
38+
private $statelessProviderKeys;
3839

39-
public function __construct(TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher = null)
40+
/**
41+
* @param array $statelessProviderKeys An array of provider/firewall keys that are "stateless" and so do not need the session migrated on success
42+
*/
43+
public function __construct(TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher = null, array $statelessProviderKeys = array())
4044
{
4145
$this->tokenStorage = $tokenStorage;
4246
$this->dispatcher = $eventDispatcher;
47+
$this->statelessProviderKeys = $statelessProviderKeys;
4348
}
4449

4550
/**
4651
* Authenticates the given token in the system.
52+
*
53+
* @param string $providerKey The name of the provider/firewall being used for authentication
4754
*/
48-
public function authenticateWithToken(TokenInterface $token, Request $request)
55+
public function authenticateWithToken(TokenInterface $token, Request $request/*, string $providerKey */)
4956
{
50-
$this->migrateSession($request, $token);
57+
$providerKey = \func_num_args() > 2 ? func_get_arg(2) : null;
58+
59+
$this->migrateSession($request, $token, $providerKey);
5160
$this->tokenStorage->setToken($token);
5261

5362
if (null !== $this->dispatcher) {
@@ -98,7 +107,7 @@ public function authenticateUserAndHandleSuccess(UserInterface $user, Request $r
98107
// create an authenticated token for the User
99108
$token = $authenticator->createAuthenticatedToken($user, $providerKey);
100109
// authenticate this in the system
101-
$this->authenticateWithToken($token, $request);
110+
$this->authenticateWithToken($token, $request, $providerKey);
102111

103112
// return the success metric
104113
return $this->handleAuthenticationSuccess($token, $request, $authenticator, $providerKey);
@@ -140,9 +149,9 @@ public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyIn
140149
$this->sessionStrategy = $sessionStrategy;
141150
}
142151

143-
private function migrateSession(Request $request, TokenInterface $token)
152+
private function migrateSession(Request $request, TokenInterface $token, $providerKey)
144153
{
145-
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
154+
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession() || \in_array($providerKey, $this->statelessProviderKeys, true)) {
146155
return;
147156
}
148157

‎src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,18 @@ public function testSessionStrategyIsCalled()
143143
$handler->authenticateWithToken($this->token, $this->request);
144144
}
145145

146+
public function testSessionStrategyIsNotCalledWhenStateless()
147+
{
148+
$this->configurePreviousSession();
149+
150+
$this->sessionStrategy->expects($this->never())
151+
->method('onAuthentication');
152+
153+
$handler = new GuardAuthenticatorHandler($this->tokenStorage, $this->dispatcher, array('some_provider_key'));
154+
$handler->setSessionAuthenticationStrategy($this->sessionStrategy);
155+
$handler->authenticateWithToken($this->token, $this->request, 'some_provider_key');
156+
}
157+
146158
protected function setUp()
147159
{
148160
$this->tokenStorage = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface')->getMock();

0 commit comments

Comments
 (0)
0