From 2c0ac93e259c061a9e0b59003424ae28f0ec33e9 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Mon, 11 Jun 2018 18:23:43 -0400 Subject: [PATCH] Fix bad method call with guard authentication + session migration --- .../Factory/GuardAuthenticationFactory.php | 1 - .../DependencyInjection/SecurityExtension.php | 5 ++++ .../SecurityBundle/Resources/config/guard.xml | 4 +++ .../SecurityExtensionTest.php | 27 +++++++++++++++++++ .../Firewall/GuardAuthenticationListener.php | 2 +- .../Guard/GuardAuthenticatorHandler.php | 21 ++++++++++----- .../Tests/GuardAuthenticatorHandlerTest.php | 12 +++++++++ 7 files changed, 64 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php index bd49cbc932..533560d6d9 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php @@ -77,7 +77,6 @@ class GuardAuthenticationFactory implements SecurityFactoryInterface $listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.authentication.listener.guard')); $listener->replaceArgument(2, $id); $listener->replaceArgument(3, $authenticatorReferences); - $listener->addMethodCall('setSessionAuthenticationStrategy', array(new Reference('security.authentication.session_strategy.'.$id))); // determine the entryPointId to use $entryPointId = $this->determineEntryPoint($defaultEntryPoint, $config); diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 5138eff367..0b671d9691 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -39,6 +39,7 @@ class SecurityExtension extends Extension private $factories = array(); private $userProviderFactories = array(); private $expressionLanguage; + private $statelessFirewallKeys = array(); public function __construct() { @@ -89,6 +90,9 @@ class SecurityExtension extends Extension $this->createAuthorization($config, $container); $this->createRoleHierarchy($config, $container); + $container->getDefinition('security.authentication.guard_handler') + ->replaceArgument(2, $this->statelessFirewallKeys); + if ($config['encoders']) { $this->createEncoders($config['encoders'], $container); } @@ -287,6 +291,7 @@ class SecurityExtension extends Extension $listeners[] = new Reference($this->createContextListener($container, $contextKey)); $sessionStrategyId = 'security.authentication.session_strategy'; } else { + $this->statelessFirewallKeys[] = $id; $sessionStrategyId = 'security.authentication.session_strategy_noop'; } $container->setAlias(new Alias('security.authentication.session_strategy.'.$id, false), $sessionStrategyId); diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml index 80318c243a..a4e57be0b0 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml @@ -10,6 +10,10 @@ > + + + + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php index 72ef2e0c3e..6c6b26c48f 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php @@ -119,6 +119,33 @@ class SecurityExtensionTest extends TestCase $this->assertFalse($container->hasDefinition('security.access.role_hierarchy_voter')); } + public function testGuardHandlerIsPassedStatelessFirewalls() + { + $container = $this->getRawContainer(); + + $container->loadFromExtension('security', array( + 'providers' => array( + 'default' => array('id' => 'foo'), + ), + + 'firewalls' => array( + 'some_firewall' => array( + 'pattern' => '^/admin', + 'http_basic' => null, + ), + 'stateless_firewall' => array( + 'pattern' => '/.*', + 'stateless' => true, + 'http_basic' => null, + ), + ), + )); + + $container->compile(); + $definition = $container->getDefinition('security.authentication.guard_handler'); + $this->assertSame(array('stateless_firewall'), $definition->getArgument(2)); + } + protected function getRawContainer() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php index ed0a36e9dc..8184406fc6 100644 --- a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php +++ b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php @@ -117,7 +117,7 @@ class GuardAuthenticationListener implements ListenerInterface } // sets the token on the token storage, etc - $this->guardHandler->authenticateWithToken($token, $request); + $this->guardHandler->authenticateWithToken($token, $request, $this->providerKey); } catch (AuthenticationException $e) { // oh no! Authentication failed! diff --git a/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php b/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php index 7b340a2601..d715c80582 100644 --- a/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php +++ b/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php @@ -35,19 +35,28 @@ class GuardAuthenticatorHandler private $tokenStorage; private $dispatcher; private $sessionStrategy; + private $statelessProviderKeys; - public function __construct(TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher = null) + /** + * @param array $statelessProviderKeys An array of provider/firewall keys that are "stateless" and so do not need the session migrated on success + */ + public function __construct(TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher = null, array $statelessProviderKeys = array()) { $this->tokenStorage = $tokenStorage; $this->dispatcher = $eventDispatcher; + $this->statelessProviderKeys = $statelessProviderKeys; } /** * Authenticates the given token in the system. + * + * @param string $providerKey The name of the provider/firewall being used for authentication */ - public function authenticateWithToken(TokenInterface $token, Request $request) + public function authenticateWithToken(TokenInterface $token, Request $request/*, string $providerKey */) { - $this->migrateSession($request, $token); + $providerKey = \func_num_args() > 2 ? func_get_arg(2) : null; + + $this->migrateSession($request, $token, $providerKey); $this->tokenStorage->setToken($token); if (null !== $this->dispatcher) { @@ -98,7 +107,7 @@ class GuardAuthenticatorHandler // create an authenticated token for the User $token = $authenticator->createAuthenticatedToken($user, $providerKey); // authenticate this in the system - $this->authenticateWithToken($token, $request); + $this->authenticateWithToken($token, $request, $providerKey); // return the success metric return $this->handleAuthenticationSuccess($token, $request, $authenticator, $providerKey); @@ -140,9 +149,9 @@ class GuardAuthenticatorHandler $this->sessionStrategy = $sessionStrategy; } - private function migrateSession(Request $request, TokenInterface $token) + private function migrateSession(Request $request, TokenInterface $token, $providerKey) { - if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) { + if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession() || \in_array($providerKey, $this->statelessProviderKeys, true)) { return; } diff --git a/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php b/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php index 49ce6548ac..f7a8de7aff 100644 --- a/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php @@ -143,6 +143,18 @@ class GuardAuthenticatorHandlerTest extends TestCase $handler->authenticateWithToken($this->token, $this->request); } + public function testSessionStrategyIsNotCalledWhenStateless() + { + $this->configurePreviousSession(); + + $this->sessionStrategy->expects($this->never()) + ->method('onAuthentication'); + + $handler = new GuardAuthenticatorHandler($this->tokenStorage, $this->dispatcher, array('some_provider_key')); + $handler->setSessionAuthenticationStrategy($this->sessionStrategy); + $handler->authenticateWithToken($this->token, $this->request, 'some_provider_key'); + } + protected function setUp() { $this->tokenStorage = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface')->getMock();