diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #25272 [SecurityBundle] fix setLogoutOnUserChange calls for context listeners (dmaicher)

Browse Source 
This PR was squashed before being merged into the 3.4 branch (closes #25272).

Discussion
----------

[SecurityBundle] fix setLogoutOnUserChange calls for context listeners

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #25267
| License       | MIT
| Doc PR        | -

As pointed out in https://github.com/symfony/symfony/issues/25267 the `setLogoutOnUserChange` method calls were added to the parent definition `security.context_listener` instead of the concrete child definitions `security.context_listener.*`.

ping @iltar @chalasr

Commits
-------

4eff146 [SecurityBundle] fix setLogoutOnUserChange calls for context listeners
This commit is contained in:
Robin Chalas 2017-12-04 21:03:56 +01:00
commit 0152527af3
3 changed files with 54 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php
View File

@ -340,17 +340,6 @@ 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()
;
}

21
src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
View File

@ -43,6 +43,7 @@ class SecurityExtension extends Extension
private $factories = array();
private $userProviderFactories = array();
private $expressionLanguage;
private $logoutOnUserChangeByContextKey = array();
public function __construct()
{
@ -276,12 +277,6 @@ class SecurityExtension extends Extension
$customUserChecker = true;
}
if (!isset($firewall['logout_on_user_change']) || !$firewall['logout_on_user_change']) {
@trigger_error(sprintf('Not setting "logout_on_user_change" to true on firewall "%s" is deprecated as of 3.4, it will always be true in 4.0.', $name), 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);
@ -370,7 +365,16 @@ class SecurityExtension extends Extension
$contextKey = $firewall['context'];
}
$listeners[] = new Reference($this->createContextListener($container, $contextKey));
if (!$logoutOnUserChange = $firewall['logout_on_user_change']) {
@trigger_error(sprintf('Not setting "logout_on_user_change" to true on firewall "%s" is deprecated as of 3.4, it will always be true in 4.0.', $id), E_USER_DEPRECATED);
}
if (isset($this->logoutOnUserChangeByContextKey[$contextKey]) && $this->logoutOnUserChangeByContextKey[$contextKey][1] !== $logoutOnUserChange) {
throw new InvalidConfigurationException(sprintf('Firewalls "%s" and "%s" need to have the same value for option "logout_on_user_change" as they are sharing the context "%s"', $this->logoutOnUserChangeByContextKey[$contextKey][0], $id, $contextKey));
}
$this->logoutOnUserChangeByContextKey[$contextKey] = array($id, $logoutOnUserChange);
$listeners[] = new Reference($this->createContextListener($container, $contextKey, $logoutOnUserChange));
}
$config->replaceArgument(6, $contextKey);
@ -481,7 +485,7 @@ class SecurityExtension extends Extension
return array($matcher, $listeners, $exceptionListener);
}
private function createContextListener($container, $contextKey)
private function createContextListener($container, $contextKey, $logoutUserOnChange)
{
if (isset($this->contextListeners[$contextKey])) {
return $this->contextListeners[$contextKey];
@ -490,6 +494,7 @@ class SecurityExtension extends Extension
$listenerId = 'security.context_listener.'.count($this->contextListeners);
$listener = $container->setDefinition($listenerId, new ChildDefinition('security.context_listener'));
$listener->replaceArgument(2, $contextKey);
$listener->addMethodCall('setLogoutOnUserChange', array($logoutUserOnChange));
return $this->contextListeners[$contextKey] = $listenerId;
}

43
src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php
View File

@ -127,7 +127,7 @@ class SecurityExtensionTest extends TestCase
* @group legacy
* @expectedDeprecation Not setting "logout_on_user_change" to true on firewall "some_firewall" is deprecated as of 3.4, it will always be true in 4.0.
*/
public function testDeprecationForUserLogout()
public function testConfiguresLogoutOnUserChangeForContextListenersCorrectly()
{
$container = $this->getRawContainer();
@ -135,13 +135,52 @@ class SecurityExtensionTest extends TestCase
'providers' => array(
'default' => array('id' => 'foo'),
),
'firewalls' => array(
'some_firewall' => array(
'pattern' => '/.*',
'http_basic' => null,
'logout_on_user_change' => false,
),
'some_other_firewall' => array(
'pattern' => '/.*',
'http_basic' => null,
'logout_on_user_change' => true,
),
),
));
$container->compile();
$this->assertEquals(array(array('setLogoutOnUserChange', array(false))), $container->getDefinition('security.context_listener.0')->getMethodCalls());
$this->assertEquals(array(array('setLogoutOnUserChange', array(true))), $container->getDefinition('security.context_listener.1')->getMethodCalls());
}
/**
* @group legacy
* @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
* @expectedExceptionMessage Firewalls "some_firewall" and "some_other_firewall" need to have the same value for option "logout_on_user_change" as they are sharing the context "my_context"
*/
public function testThrowsIfLogoutOnUserChangeDifferentForSharedContext()
{
$container = $this->getRawContainer();
$container->loadFromExtension('security', array(
'providers' => array(
'default' => array('id' => 'foo'),
),
'firewalls' => array(
'some_firewall' => array(
'pattern' => '/.*',
'http_basic' => null,
'context' => 'my_context',
'logout_on_user_change' => false,
),
'some_other_firewall' => array(
'pattern' => '/.*',
'http_basic' => null,
'context' => 'my_context',
'logout_on_user_change' => true,
),
),
));