From 908db1e4222cc3c98b3b9ab39c67a2d2e31e2582 Mon Sep 17 00:00:00 2001 From: Iltar van der Berg Date: Mon, 14 Aug 2017 10:19:33 +0200 Subject: [PATCH 1/3] Deprecated not being logged out after user change --- .../Http/Firewall/ContextListener.php | 34 +++++++++++++++---- .../Tests/Firewall/ContextListenerTest.php | 34 ++++++++++++++++--- 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php index cd26d83ef7291..9d40f17dc4c46 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php @@ -37,13 +37,13 @@ class ContextListener implements ListenerInterface { private $tokenStorage; - private $contextKey; private $sessionKey; private $logger; private $userProviders; private $dispatcher; private $registered; private $trustResolver; + private $logoutOnUserChange = false; /** * @param TokenStorageInterface $tokenStorage @@ -61,13 +61,22 @@ public function __construct(TokenStorageInterface $tokenStorage, $userProviders, $this->tokenStorage = $tokenStorage; $this->userProviders = $userProviders; - $this->contextKey = $contextKey; $this->sessionKey = '_security_'.$contextKey; $this->logger = $logger; $this->dispatcher = $dispatcher; $this->trustResolver = $trustResolver ?: new AuthenticationTrustResolver(AnonymousToken::class, RememberMeToken::class); } + /** + * Enables deauthentication during refreshUser when the user has changed. + * + * @param bool $logoutOnUserChange + */ + public function setLogoutOnUserChange($logoutOnUserChange) + { + $this->logoutOnUserChange = (bool) $logoutOnUserChange; + } + /** * Reads the Security Token from the session. * @@ -122,14 +131,14 @@ public function onKernelResponse(FilterResponseEvent $event) return; } - if (!$event->getRequest()->hasSession()) { + $request = $event->getRequest(); + + if (!$request->hasSession()) { return; } $this->dispatcher->removeListener(KernelEvents::RESPONSE, array($this, 'onKernelResponse')); $this->registered = false; - - $request = $event->getRequest(); $session = $request->getSession(); if ((null === $token = $this->tokenStorage->getToken()) || $this->trustResolver->isAnonymous($token)) { @@ -172,6 +181,19 @@ protected function refreshUser(TokenInterface $token) $refreshedUser = $provider->refreshUser($user); $token->setUser($refreshedUser); + // tokens can be deauthenticated if the user has been changed. + if (!$token->isAuthenticated()) { + if ($this->logoutOnUserChange) { + if (null !== $this->logger) { + $this->logger->debug('Token was deauthenticated after trying to refresh it.', array('username' => $refreshedUser->getUsername(), 'provider' => get_class($provider))); + } + + return null; + } + + @trigger_error('Refreshing a deauthenticated user is deprecated as of 3.4 and will trigger a logout in 4.0.', E_USER_DEPRECATED); + } + if (null !== $this->logger) { $context = array('provider' => get_class($provider), 'username' => $refreshedUser->getUsername()); @@ -198,7 +220,7 @@ protected function refreshUser(TokenInterface $token) } if ($userNotFoundByProvider) { - return; + return null; } throw new \RuntimeException(sprintf('There is no user provider for user "%s".', get_class($user))); diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php index 66cf4458bc4de..d27569fa3a0ba 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php @@ -249,7 +249,11 @@ public function testHandleRemovesTokenIfNoPreviousSessionWasFound() $listener->handle($event); } - public function testTryAllUserProvidersUntilASupportingUserProviderIsFound() + /** + * @group legacy + * @expectedDeprecation Refreshing a deauthenticated user is deprecated as of 3.4 and will trigger a logout in 4.0. + */ + public function testIfTokenIsDeauthenticatedTriggersDeprecations() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); @@ -258,11 +262,29 @@ public function testTryAllUserProvidersUntilASupportingUserProviderIsFound() $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); } + public function testIfTokenIsDeauthenticated() + { + $tokenStorage = new TokenStorage(); + $refreshedUser = new User('foobar', 'baz'); + $this->handleEventWithPreviousSession($tokenStorage, array(new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)), null, true); + + $this->assertNull($tokenStorage->getToken()); + } + + public function testTryAllUserProvidersUntilASupportingUserProviderIsFound() + { + $tokenStorage = new TokenStorage(); + $refreshedUser = new User('foobar', 'baz'); + $this->handleEventWithPreviousSession($tokenStorage, array(new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)), $refreshedUser); + + $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); + } + public function testNextSupportingUserProviderIsTriedIfPreviousSupportingUserProviderDidNotLoadTheUser() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); - $this->handleEventWithPreviousSession($tokenStorage, array(new SupportingUserProvider(), new SupportingUserProvider($refreshedUser))); + $this->handleEventWithPreviousSession($tokenStorage, array(new SupportingUserProvider(), new SupportingUserProvider($refreshedUser)), $refreshedUser); $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); } @@ -287,7 +309,7 @@ public function testAcceptsProvidersAsTraversable() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); - $this->handleEventWithPreviousSession($tokenStorage, new \ArrayObject(array(new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)))); + $this->handleEventWithPreviousSession($tokenStorage, new \ArrayObject(array(new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser))), $refreshedUser); $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); } @@ -320,16 +342,18 @@ protected function runSessionOnKernelResponse($newToken, $original = null) return $session; } - private function handleEventWithPreviousSession(TokenStorageInterface $tokenStorage, $userProviders) + private function handleEventWithPreviousSession(TokenStorageInterface $tokenStorage, $userProviders, UserInterface $user = null, $logoutOnUserChange = false) { + $user = $user ?: new User('foo', 'bar'); $session = new Session(new MockArraySessionStorage()); - $session->set('_security_context_key', serialize(new UsernamePasswordToken(new User('foo', 'bar'), '', 'context_key'))); + $session->set('_security_context_key', serialize(new UsernamePasswordToken($user, '', 'context_key', array('ROLE_USER')))); $request = new Request(); $request->setSession($session); $request->cookies->set('MOCKSESSID', true); $listener = new ContextListener($tokenStorage, $userProviders, 'context_key'); + $listener->setLogoutOnUserChange($logoutOnUserChange); $listener->handle(new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST)); } } From 83cff7c24e9d2198623d9ba0ad5e8b2402885d74 Mon Sep 17 00:00:00 2001 From: Iltar van der Berg Date: Fri, 22 Sep 2017 13:53:22 +0200 Subject: [PATCH 2/3] Added configuration changes for setLogoutOnUserChange --- UPGRADE-3.4.md | 4 +++ .../Bundle/SecurityBundle/CHANGELOG.md | 3 ++ .../DependencyInjection/MainConfiguration.php | 15 ++++++++++ .../DependencyInjection/SecurityExtension.php | 12 ++++++-- .../Fixtures/php/container1.php | 3 ++ .../Fixtures/php/firewall_provider.php | 2 ++ .../php/firewall_undefined_provider.php | 1 + .../Fixtures/php/listener_provider.php | 1 + .../php/listener_undefined_provider.php | 1 + .../Fixtures/php/merge.php | 1 + .../Fixtures/php/merge_import.php | 1 + .../Fixtures/php/no_custom_user_checker.php | 3 +- .../Fixtures/php/remember_me_options.php | 1 + .../Fixtures/xml/container1.xml | 4 +-- .../Fixtures/xml/firewall_provider.xml | 2 +- .../xml/firewall_undefined_provider.xml | 2 +- .../Fixtures/xml/listener_provider.xml | 2 +- .../xml/listener_undefined_provider.xml | 2 +- .../Fixtures/xml/merge.xml | 2 +- .../Fixtures/xml/merge_import.xml | 2 +- .../Fixtures/xml/remember_me_options.xml | 2 +- .../Fixtures/yml/container1.yml | 2 ++ .../Fixtures/yml/firewall_provider.yml | 2 ++ .../yml/firewall_undefined_provider.yml | 1 + .../Fixtures/yml/listener_provider.yml | 1 + .../yml/listener_undefined_provider.yml | 1 + .../Fixtures/yml/merge.yml | 1 + .../Fixtures/yml/merge_import.yml | 1 + .../Fixtures/yml/remember_me_options.yml | 1 + .../MainConfigurationTest.php | 3 ++ .../SecurityExtensionTest.php | 29 +++++++++++++++++++ src/Symfony/Component/Security/CHANGELOG.md | 4 +++ .../Http/Firewall/ContextListener.php | 2 +- 33 files changed, 100 insertions(+), 14 deletions(-) diff --git a/UPGRADE-3.4.md b/UPGRADE-3.4.md index cdc062008bf4b..44afe9be5dc63 100644 --- a/UPGRADE-3.4.md +++ b/UPGRADE-3.4.md @@ -269,6 +269,10 @@ SecurityBundle as first argument. Not passing it is deprecated and will throw a `TypeError` in 4.0. + * Added `logout_on_user_change` to the firewall options. This config item will + trigger a logout when the user has changed. Should be set to true to avoid + deprecations in the configuration. + Translation ----------- diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md index 0c62ce6e2aa9e..b60e2e7e882e0 100644 --- a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md @@ -13,6 +13,9 @@ CHANGELOG * `SetAclCommand::__construct()` now takes an instance of `Symfony\Component\Security\Acl\Model\MutableAclProviderInterfaceConnection` as first argument + * Added `logout_on_user_change` to the firewall options. This config item will + trigger a logout when the user has changed. Should be set to true to avoid + deprecations in the configuration. 3.3.0 ----- diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index 35db5bd83949a..e9863145e798f 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -252,6 +252,10 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto ->scalarNode('provider')->end() ->booleanNode('stateless')->defaultFalse()->end() ->scalarNode('context')->cannotBeEmpty()->end() + ->booleanNode('logout_on_user_change') + ->defaultFalse() + ->info('When true, it will trigger a logout for the user if something has changed. This will be the default behavior as of Syfmony 4.0.') + ->end() ->arrayNode('logout') ->treatTrueLike(array()) ->canBeUnset() @@ -340,6 +344,17 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto return $firewall; }) ->end() + ->validate() + ->ifTrue(function ($v) { + return (isset($v['stateless']) && true === $v['stateless']) || (isset($v['security']) && false === $v['security']); + }) + ->then(function ($v) { + // this option doesn't change behavior when true when stateless, so prevent deprecations + $v['logout_on_user_change'] = true; + + return $v; + }) + ->end() ; } diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 21fc056e382f2..b19f6b1b8d58c 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -265,14 +265,14 @@ private function createFirewalls($config, ContainerBuilder $container) $providerIds = $this->createUserProviders($config, $container); // make the ContextListener aware of the configured user providers - $definition = $container->getDefinition('security.context_listener'); - $arguments = $definition->getArguments(); + $contextListenerDefinition = $container->getDefinition('security.context_listener'); + $arguments = $contextListenerDefinition->getArguments(); $userProviders = array(); foreach ($providerIds as $userProviderId) { $userProviders[] = new Reference($userProviderId); } $arguments[1] = new IteratorArgument($userProviders); - $definition->setArguments($arguments); + $contextListenerDefinition->setArguments($arguments); $customUserChecker = false; @@ -284,6 +284,12 @@ private function createFirewalls($config, ContainerBuilder $container) $customUserChecker = true; } + if (!isset($firewall['logout_on_user_change']) || !$firewall['logout_on_user_change']) { + @trigger_error('Setting logout_on_user_change to false is deprecated as of 3.4 and will always be true in 4.0. Set logout_on_user_change to true in your firewall configuration.', E_USER_DEPRECATED); + } + + $contextListenerDefinition->addMethodCall('setLogoutOnUserChange', array($firewall['logout_on_user_change'])); + $configId = 'security.firewall.map.config.'.$name; list($matcher, $listeners, $exceptionListener) = $this->createFirewall($container, $name, $firewall, $authenticationProviders, $providerIds, $configId); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php index fc9b07c4f18b2..581407fcc05a5 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php @@ -73,6 +73,7 @@ 'logout' => true, 'remember_me' => array('secret' => 'TheSecret'), 'user_checker' => null, + 'logout_on_user_change' => true, ), 'host' => array( 'pattern' => '/test', @@ -80,11 +81,13 @@ 'methods' => array('GET', 'POST'), 'anonymous' => true, 'http_basic' => true, + 'logout_on_user_change' => true, ), 'with_user_checker' => array( 'user_checker' => 'app.user_checker', 'anonymous' => true, 'http_basic' => true, + 'logout_on_user_change' => true, ), ), diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/firewall_provider.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/firewall_provider.php index ff9d9f6b89df6..da218fc61c9cf 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/firewall_provider.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/firewall_provider.php @@ -15,10 +15,12 @@ 'main' => array( 'provider' => 'default', 'form_login' => true, + 'logout_on_user_change' => true, ), 'other' => array( 'provider' => 'with-dash', 'form_login' => true, + 'logout_on_user_change' => true, ), ), )); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/firewall_undefined_provider.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/firewall_undefined_provider.php index 78d461efe38d1..46a51f91fdd22 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/firewall_undefined_provider.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/firewall_undefined_provider.php @@ -12,6 +12,7 @@ 'main' => array( 'provider' => 'undefined', 'form_login' => true, + 'logout_on_user_change' => true, ), ), )); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/listener_provider.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/listener_provider.php index d7f1cd6973f36..072b70b078a58 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/listener_provider.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/listener_provider.php @@ -11,6 +11,7 @@ 'firewalls' => array( 'main' => array( 'form_login' => array('provider' => 'default'), + 'logout_on_user_change' => true, ), ), )); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/listener_undefined_provider.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/listener_undefined_provider.php index da54f025d1a70..567f8a0d4b2d7 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/listener_undefined_provider.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/listener_undefined_provider.php @@ -11,6 +11,7 @@ 'firewalls' => array( 'main' => array( 'form_login' => array('provider' => 'undefined'), + 'logout_on_user_change' => true, ), ), )); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/merge.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/merge.php index 50ef504ea4d43..eb34a6a6f64b6 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/merge.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/merge.php @@ -11,6 +11,7 @@ 'main' => array( 'form_login' => false, 'http_basic' => null, + 'logout_on_user_change' => true, ), ), diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/merge_import.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/merge_import.php index 912b9127ef369..6ed2d18a36709 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/merge_import.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/merge_import.php @@ -6,6 +6,7 @@ 'form_login' => array( 'login_path' => '/login', ), + 'logout_on_user_change' => true, ), ), 'role_hierarchy' => array( diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/no_custom_user_checker.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/no_custom_user_checker.php index b08d9c60c82f8..91c475418b450 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/no_custom_user_checker.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/no_custom_user_checker.php @@ -12,7 +12,8 @@ ), 'firewalls' => array( 'simple' => array('pattern' => '/login', 'security' => false), - 'secure' => array('stateless' => true, + 'secure' => array( + 'stateless' => true, 'http_basic' => true, 'http_digest' => array('secret' => 'TheSecret'), 'form_login' => true, diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/remember_me_options.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/remember_me_options.php index e0ca4f6dedf3e..a61fde3dc7309 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/remember_me_options.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/remember_me_options.php @@ -13,6 +13,7 @@ 'catch_exceptions' => false, 'token_provider' => 'token_provider_id', ), + 'logout_on_user_change' => true, ), ), )); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml index 19167551025e2..e5049f2033e51 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml @@ -60,12 +60,12 @@ - + - + app.user_checker diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/firewall_provider.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/firewall_provider.xml index bd87fee4abae9..9d37164e8d409 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/firewall_provider.xml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/firewall_provider.xml @@ -11,7 +11,7 @@ - + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/firewall_undefined_provider.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/firewall_undefined_provider.xml index f596ac5a6240b..6a05d48e539b9 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/firewall_undefined_provider.xml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/firewall_undefined_provider.xml @@ -11,7 +11,7 @@ - + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/listener_provider.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/listener_provider.xml index b1bcd8eae8155..f53b91b00ef78 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/listener_provider.xml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/listener_provider.xml @@ -11,7 +11,7 @@ - + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/listener_undefined_provider.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/listener_undefined_provider.xml index 725e85a1d0f27..75271ad075f37 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/listener_undefined_provider.xml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/listener_undefined_provider.xml @@ -11,7 +11,7 @@ - + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/merge.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/merge.xml index 8a17f6db23c55..42dc91c9975ef 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/merge.xml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/merge.xml @@ -12,7 +12,7 @@ - + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/merge_import.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/merge_import.xml index 81b8cffd68d9e..051e2a40b3a16 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/merge_import.xml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/merge_import.xml @@ -6,7 +6,7 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/remember_me_options.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/remember_me_options.xml index b6ade91a07970..583720ea1de9b 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/remember_me_options.xml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/remember_me_options.xml @@ -9,7 +9,7 @@ - + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml index e8ed61ef031b9..a2b57201bfbd2 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml @@ -64,11 +64,13 @@ security: methods: [GET,POST] anonymous: true http_basic: true + logout_on_user_change: true with_user_checker: anonymous: ~ http_basic: ~ user_checker: app.user_checker + logout_on_user_change: true role_hierarchy: ROLE_ADMIN: ROLE_USER diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/firewall_provider.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/firewall_provider.yml index 11c329aa8e2fe..b8da52b6e45d3 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/firewall_provider.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/firewall_provider.yml @@ -11,6 +11,8 @@ security: main: provider: default form_login: true + logout_on_user_change: true other: provider: with-dash form_login: true + logout_on_user_change: true diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/firewall_undefined_provider.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/firewall_undefined_provider.yml index ec2664054009c..3385fc3485a0e 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/firewall_undefined_provider.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/firewall_undefined_provider.yml @@ -8,3 +8,4 @@ security: main: provider: undefined form_login: true + logout_on_user_change: true diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/listener_provider.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/listener_provider.yml index 652f23b5f0425..53e2784c4b3a9 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/listener_provider.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/listener_provider.yml @@ -8,3 +8,4 @@ security: main: form_login: provider: default + logout_on_user_change: true diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/listener_undefined_provider.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/listener_undefined_provider.yml index 1916df4c2e7ca..ba5f69ede665d 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/listener_undefined_provider.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/listener_undefined_provider.yml @@ -8,3 +8,4 @@ security: main: form_login: provider: undefined + logout_on_user_change: true diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/merge.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/merge.yml index 60c0bbea558e7..d8f443c62f34e 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/merge.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/merge.yml @@ -9,6 +9,7 @@ security: main: form_login: false http_basic: ~ + logout_on_user_change: true role_hierarchy: FOO: [MOO] diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/merge_import.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/merge_import.yml index 4f8db0a09f7b4..a081003a49578 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/merge_import.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/merge_import.yml @@ -3,6 +3,7 @@ security: main: form_login: login_path: /login + logout_on_user_change: true role_hierarchy: FOO: BAR diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/remember_me_options.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/remember_me_options.yml index a521c8c6a803d..716cd4cf99d14 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/remember_me_options.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/remember_me_options.yml @@ -10,3 +10,4 @@ security: secret: TheSecret catch_exceptions: false token_provider: token_provider_id + logout_on_user_change: true diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php index a64c4fe4101f1..02e8062c269f4 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php @@ -31,6 +31,7 @@ class MainConfigurationTest extends TestCase ), 'firewalls' => array( 'stub' => array(), + 'logout_on_user_change' => true, ), ); @@ -78,6 +79,7 @@ public function testCsrfAliases() 'csrf_token_generator' => 'a_token_generator', 'csrf_token_id' => 'a_token_id', ), + 'logout_on_user_change' => true, ), ), ); @@ -107,6 +109,7 @@ public function testUserCheckers() 'firewalls' => array( 'stub' => array( 'user_checker' => 'app.henk_checker', + 'logout_on_user_change' => true, ), ), ); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php index 72ef2e0c3ed56..1055e4afd40f6 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php @@ -38,6 +38,7 @@ public function testInvalidCheckPath() 'form_login' => array( 'check_path' => '/some_area/login_check', ), + 'logout_on_user_change' => true, ), ), )); @@ -61,6 +62,7 @@ public function testFirewallWithoutAuthenticationListener() 'firewalls' => array( 'some_firewall' => array( 'pattern' => '/.*', + 'logout_on_user_change' => true, ), ), )); @@ -88,6 +90,7 @@ public function testFirewallWithInvalidUserProvider() 'some_firewall' => array( 'pattern' => '/.*', 'http_basic' => array(), + 'logout_on_user_change' => true, ), ), )); @@ -110,6 +113,7 @@ public function testDisableRoleHierarchyVoter() 'some_firewall' => array( 'pattern' => '/.*', 'http_basic' => null, + 'logout_on_user_change' => true, ), ), )); @@ -119,6 +123,31 @@ public function testDisableRoleHierarchyVoter() $this->assertFalse($container->hasDefinition('security.access.role_hierarchy_voter')); } + /** + * @group legacy + * @expectedDeprecation Setting logout_on_user_change to false is deprecated as of 3.4 and will always be true in 4.0. Set logout_on_user_change to true in your firewall configuration. + */ + public function testDeprecationForUserLogout() + { + $container = $this->getRawContainer(); + + $container->loadFromExtension('security', array( + 'providers' => array( + 'default' => array('id' => 'foo'), + ), + + 'firewalls' => array( + 'some_firewall' => array( + 'pattern' => '/.*', + 'http_basic' => null, + 'logout_on_user_change' => false, + ), + ), + )); + + $container->compile(); + } + protected function getRawContainer() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md index 292c304fc68c7..99032dae9da9b 100644 --- a/src/Symfony/Component/Security/CHANGELOG.md +++ b/src/Symfony/Component/Security/CHANGELOG.md @@ -6,6 +6,10 @@ CHANGELOG * Using voters that do not implement the `VoterInterface`is now deprecated in the `AccessDecisionManager` and this functionality will be removed in 4.0. + * Using the `ContextListener` without setting the `logoutOnUserChange` + property will trigger a deprecation when the user has changed. As of 4.0 + the user will always be logged out when the user has changed between + requests. 3.3.0 ----- diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php index 9d40f17dc4c46..d2baf63432584 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php @@ -220,7 +220,7 @@ protected function refreshUser(TokenInterface $token) } if ($userNotFoundByProvider) { - return null; + return; } throw new \RuntimeException(sprintf('There is no user provider for user "%s".', get_class($user))); From 268e922862ee1f1793be6a93f6259adf81b0a283 Mon Sep 17 00:00:00 2001 From: Iltar van der Berg Date: Tue, 26 Sep 2017 08:30:50 +0200 Subject: [PATCH 3/3] Added 4.0 changelog --- UPGRADE-4.0.md | 6 ++++++ .../Component/Security/Http/Firewall/ContextListener.php | 2 ++ 2 files changed, 8 insertions(+) diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index 3aecb70070f0d..2fc980b92fafc 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -642,6 +642,9 @@ Security * Support for defining voters that don't implement the `VoterInterface` has been removed. + * Calling `ContextListener::setLogoutOnUserChange(false)` won't have any + effect anymore. + SecurityBundle -------------- @@ -660,6 +663,9 @@ SecurityBundle `Symfony\Component\Security\Acl\Model\MutableAclProviderInterfaceConnection` as first argument. + * The firewall option `logout_on_user_change` is now always true, which will + trigger a logout if the user changes between requests. + Serializer ---------- diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php index d2baf63432584..e018f704dd659 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php @@ -37,6 +37,7 @@ class ContextListener implements ListenerInterface { private $tokenStorage; + private $contextKey; private $sessionKey; private $logger; private $userProviders; @@ -61,6 +62,7 @@ public function __construct(TokenStorageInterface $tokenStorage, $userProviders, $this->tokenStorage = $tokenStorage; $this->userProviders = $userProviders; + $this->contextKey = $contextKey; $this->sessionKey = '_security_'.$contextKey; $this->logger = $logger; $this->dispatcher = $dispatcher;