Skip to content

[Security] Deprecated not being logged out after user change #23882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions UPGRADE-3.4.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----------

Expand Down
6 changes: 6 additions & 0 deletions UPGRADE-4.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
--------------

Expand All @@ -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
----------

Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,21 @@
'logout' => true,
'remember_me' => array('secret' => 'TheSecret'),
'user_checker' => null,
'logout_on_user_change' => true,
),
'host' => array(
'pattern' => '/test',
'host' => 'foo\\.example\\.org',
'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,
),
),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
),
));
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
'main' => array(
'provider' => 'undefined',
'form_login' => true,
'logout_on_user_change' => true,
),
),
));
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'firewalls' => array(
'main' => array(
'form_login' => array('provider' => 'default'),
'logout_on_user_change' => true,
),
),
));
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'firewalls' => array(
'main' => array(
'form_login' => array('provider' => 'undefined'),
'logout_on_user_change' => true,
),
),
));
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'main' => array(
'form_login' => false,
'http_basic' => null,
'logout_on_user_change' => true,
),
),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
'form_login' => array(
'login_path' => '/login',
),
'logout_on_user_change' => true,
),
),
'role_hierarchy' => array(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
'catch_exceptions' => false,
'token_provider' => 'token_provider_id',
),
'logout_on_user_change' => true,
),
),
));
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@
<remember-me secret="TheSecret"/>
</firewall>

<firewall name="host" pattern="/test" host="foo\.example\.org" methods="GET,POST">
<firewall name="host" pattern="/test" host="foo\.example\.org" methods="GET,POST" logout-on-user-change="true">
<anonymous />
<http-basic />
</firewall>

<firewall name="with_user_checker">
<firewall name="with_user_checker" logout-on-user-change="true">
<anonymous />
<http-basic />
<user-checker>app.user_checker</user-checker>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</sec:providers>

<sec:firewalls>
<sec:firewall name="main" provider="with-dash">
<sec:firewall name="main" provider="with-dash" logout-on-user-change="true">
<sec:form_login />
</sec:firewall>
</sec:firewalls>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</sec:providers>

<sec:firewalls>
<sec:firewall name="main" provider="undefined">
<sec:firewall name="main" provider="undefined" logout-on-user-change="true">
<sec:form_login />
</sec:firewall>
</sec:firewalls>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</sec:providers>

<sec:firewalls>
<sec:firewall name="main">
<sec:firewall name="main" logout-on-user-change="true">
<sec:form_login provider="default" />
</sec:firewall>
</sec:firewalls>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</sec:providers>

<sec:firewalls>
<sec:firewall name="main">
<sec:firewall name="main" logout-on-user-change="true">
<sec:form_login provider="undefined" />
</sec:firewall>
</sec:firewalls>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<sec:config>
<sec:provider name="default" id="foo" />

<sec:firewall name="main" form-login="false">
<sec:firewall name="main" form-login="false" logout-on-user-change="true">
<sec:http-basic />
</sec:firewall>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<config>
<firewall name="main">
<firewall name="main" logout-on-user-change="true">
<form-login login-path="/login" />
</firewall>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<sec:providers>
<sec:default id="foo"/>
</sec:providers>
<sec:firewall name="main">
<sec:firewall name="main" logout-on-user-change="true">
<sec:form-login/>
<sec:remember-me secret="TheSecret" catch-exceptions="false" token-provider="token_provider_id" />
</sec:firewall>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ security:
main:
provider: undefined
form_login: true
logout_on_user_change: true
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ security:
main:
form_login:
provider: default
logout_on_user_change: true
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ security:
main:
form_login:
provider: undefined
logout_on_user_change: true
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ security:
main:
form_login: false
http_basic: ~
logout_on_user_change: true

role_hierarchy:
FOO: [MOO]
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ security:
main:
form_login:
login_path: /login
logout_on_user_change: true

role_hierarchy:
FOO: BAR
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ security:
secret: TheSecret
catch_exceptions: false
token_provider: token_provider_id
logout_on_user_change: true
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class MainConfigurationTest extends TestCase
),
'firewalls' => array(
'stub' => array(),
'logout_on_user_change' => true,
),
);

Expand Down Expand Up @@ -78,6 +79,7 @@ public function testCsrfAliases()
'csrf_token_generator' => 'a_token_generator',
'csrf_token_id' => 'a_token_id',
),
'logout_on_user_change' => true,
),
),
);
Expand Down Expand Up @@ -107,6 +109,7 @@ public function testUserCheckers()
'firewalls' => array(
'stub' => array(
'user_checker' => 'app.henk_checker',
'logout_on_user_change' => true,
),
),
);
Expand Down
Loading