From 22f525b01fdace2f42b457ac807bef1528a19811 Mon Sep 17 00:00:00 2001 From: Iltar van der Berg Date: Mon, 14 Aug 2017 10:19:33 +0200 Subject: [PATCH] [Security] Deprecated not being logged out after user change --- UPGRADE-3.4.md | 4 +++ UPGRADE-4.0.md | 6 ++++ .../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 | 30 ++++++++++++++-- .../Tests/Firewall/ContextListenerTest.php | 34 ++++++++++++++++--- 35 files changed, 161 insertions(+), 21 deletions(-) diff --git a/UPGRADE-3.4.md b/UPGRADE-3.4.md index cdc062008b..44afe9be5d 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/UPGRADE-4.0.md b/UPGRADE-4.0.md index 3aecb70070..2fc980b92f 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/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md index 0c62ce6e2a..b60e2e7e88 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 35db5bd839..e9863145e7 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -252,6 +252,10 @@ class MainConfiguration implements ConfigurationInterface ->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 @@ class MainConfiguration implements ConfigurationInterface 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 21fc056e38..b19f6b1b8d 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -265,14 +265,14 @@ class SecurityExtension extends Extension $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 @@ class SecurityExtension extends Extension $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 fc9b07c4f1..581407fcc0 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 @@ $container->loadFromExtension('security', array( 'logout' => true, 'remember_me' => array('secret' => 'TheSecret'), 'user_checker' => null, + 'logout_on_user_change' => true, ), 'host' => array( 'pattern' => '/test', @@ -80,11 +81,13 @@ $container->loadFromExtension('security', array( '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 ff9d9f6b89..da218fc61c 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 @@ $container->loadFromExtension('security', array( '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 78d461efe3..46a51f91fd 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 @@ $container->loadFromExtension('security', array( '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 d7f1cd6973..072b70b078 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 @@ $container->loadFromExtension('security', array( '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 da54f025d1..567f8a0d4b 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 @@ $container->loadFromExtension('security', array( '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 50ef504ea4..eb34a6a6f6 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 @@ $container->loadFromExtension('security', array( '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 912b9127ef..6ed2d18a36 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 @@ $container->loadFromExtension('security', array( '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 b08d9c60c8..91c475418b 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 @@ $container->loadFromExtension('security', array( ), '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 e0ca4f6ded..a61fde3dc7 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 @@ $container->loadFromExtension('security', array( '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 1916755102..e5049f2033 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 bd87fee4ab..9d37164e8d 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 f596ac5a62..6a05d48e53 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 b1bcd8eae8..f53b91b00e 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 725e85a1d0..75271ad075 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 8a17f6db23..42dc91c997 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 81b8cffd68..051e2a40b3 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 b6ade91a07..583720ea1d 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 e8ed61ef03..a2b57201bf 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 11c329aa8e..b8da52b6e4 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 ec26640540..3385fc3485 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 652f23b5f0..53e2784c4b 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 1916df4c2e..ba5f69ede6 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 60c0bbea55..d8f443c62f 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 4f8db0a09f..a081003a49 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 a521c8c6a8..716cd4cf99 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 a64c4fe410..02e8062c26 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 @@ class MainConfigurationTest extends TestCase 'csrf_token_generator' => 'a_token_generator', 'csrf_token_id' => 'a_token_id', ), + 'logout_on_user_change' => true, ), ), ); @@ -107,6 +109,7 @@ class MainConfigurationTest extends TestCase '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 72ef2e0c3e..1055e4afd4 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php @@ -38,6 +38,7 @@ class SecurityExtensionTest extends TestCase 'form_login' => array( 'check_path' => '/some_area/login_check', ), + 'logout_on_user_change' => true, ), ), )); @@ -61,6 +62,7 @@ class SecurityExtensionTest extends TestCase 'firewalls' => array( 'some_firewall' => array( 'pattern' => '/.*', + 'logout_on_user_change' => true, ), ), )); @@ -88,6 +90,7 @@ class SecurityExtensionTest extends TestCase 'some_firewall' => array( 'pattern' => '/.*', 'http_basic' => array(), + 'logout_on_user_change' => true, ), ), )); @@ -110,6 +113,7 @@ class SecurityExtensionTest extends TestCase 'some_firewall' => array( 'pattern' => '/.*', 'http_basic' => null, + 'logout_on_user_change' => true, ), ), )); @@ -119,6 +123,31 @@ class SecurityExtensionTest extends TestCase $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 292c304fc6..99032dae9d 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 cd26d83ef7..e018f704dd 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php @@ -44,6 +44,7 @@ class ContextListener implements ListenerInterface private $dispatcher; private $registered; private $trustResolver; + private $logoutOnUserChange = false; /** * @param TokenStorageInterface $tokenStorage @@ -68,6 +69,16 @@ class ContextListener implements ListenerInterface $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 +133,14 @@ class ContextListener implements ListenerInterface 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 +183,19 @@ class ContextListener implements ListenerInterface $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()); diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php index 66cf4458bc..d27569fa3a 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 @@ class ContextListenerTest extends TestCase $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 @@ class ContextListenerTest extends TestCase $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 @@ class ContextListenerTest extends TestCase { $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 @@ class ContextListenerTest extends TestCase 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)); } }