minor #20407 [SecurityBundle] FirewallConfig's user_checker should be mandatory (chalasr)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[SecurityBundle] FirewallConfig's user_checker should be mandatory

| Q             | A
| ------------- | ---
| Branch?       | master
| Tests pass?   | yes
| Fixed tickets | https://github.com/symfony/symfony/pull/20404#issuecomment-258395500
| License       | MIT

Commits
-------

6754af2 [SecurityBundle] FirewallConfig's user_checker should be mandatory
This commit is contained in:
Fabien Potencier 2016-11-04 07:59:30 -07:00
commit fc557db841
7 changed files with 20 additions and 19 deletions

View File

@ -280,14 +280,15 @@ class SecurityExtension extends Extension
} }
$config->replaceArgument(1, (string) $matcher); $config->replaceArgument(1, (string) $matcher);
$config->replaceArgument(2, $firewall['security']); $config->replaceArgument(2, $firewall['user_checker']);
$config->replaceArgument(3, $firewall['security']);
// Security disabled? // Security disabled?
if (false === $firewall['security']) { if (false === $firewall['security']) {
return array($matcher, array(), null); return array($matcher, array(), null);
} }
$config->replaceArgument(3, $firewall['stateless']); $config->replaceArgument(4, $firewall['stateless']);
// Provider id (take the first registered provider if none defined) // Provider id (take the first registered provider if none defined)
if (isset($firewall['provider'])) { if (isset($firewall['provider'])) {
@ -296,7 +297,7 @@ class SecurityExtension extends Extension
$defaultProvider = reset($providerIds); $defaultProvider = reset($providerIds);
} }
$config->replaceArgument(4, $defaultProvider); $config->replaceArgument(5, $defaultProvider);
// Register listeners // Register listeners
$listeners = array(); $listeners = array();
@ -312,7 +313,7 @@ class SecurityExtension extends Extension
$contextKey = $firewall['context']; $contextKey = $firewall['context'];
} }
$config->replaceArgument(5, $contextKey); $config->replaceArgument(6, $contextKey);
$listeners[] = new Reference($this->createContextListener($container, $contextKey)); $listeners[] = new Reference($this->createContextListener($container, $contextKey));
} }
@ -382,7 +383,7 @@ class SecurityExtension extends Extension
// Authentication listeners // Authentication listeners
list($authListeners, $defaultEntryPoint) = $this->createAuthenticationListeners($container, $id, $firewall, $authenticationProviders, $defaultProvider, $configuredEntryPoint); list($authListeners, $defaultEntryPoint) = $this->createAuthenticationListeners($container, $id, $firewall, $authenticationProviders, $defaultProvider, $configuredEntryPoint);
$config->replaceArgument(6, $configuredEntryPoint ?: $defaultEntryPoint); $config->replaceArgument(7, $configuredEntryPoint ?: $defaultEntryPoint);
$listeners = array_merge($listeners, $authListeners); $listeners = array_merge($listeners, $authListeners);
@ -399,14 +400,13 @@ class SecurityExtension extends Extension
$exceptionListener = new Reference($this->createExceptionListener($container, $firewall, $id, $configuredEntryPoint ?: $defaultEntryPoint, $firewall['stateless'])); $exceptionListener = new Reference($this->createExceptionListener($container, $firewall, $id, $configuredEntryPoint ?: $defaultEntryPoint, $firewall['stateless']));
if (isset($firewall['access_denied_handler'])) { if (isset($firewall['access_denied_handler'])) {
$config->replaceArgument(7, $firewall['access_denied_handler']); $config->replaceArgument(8, $firewall['access_denied_handler']);
} }
if (isset($firewall['access_denied_url'])) { if (isset($firewall['access_denied_url'])) {
$config->replaceArgument(8, $firewall['access_denied_url']); $config->replaceArgument(9, $firewall['access_denied_url']);
} }
$container->setAlias(new Alias('security.user_checker.'.$id, false), $firewall['user_checker']); $container->setAlias(new Alias('security.user_checker.'.$id, false), $firewall['user_checker']);
$config->replaceArgument(9, $firewall['user_checker']);
foreach ($this->factories as $position) { foreach ($this->factories as $position) {
foreach ($position as $factory) { foreach ($position as $factory) {

View File

@ -117,6 +117,7 @@
<service id="security.firewall.config" class="Symfony\Bundle\SecurityBundle\Security\FirewallConfig" abstract="true" public="false"> <service id="security.firewall.config" class="Symfony\Bundle\SecurityBundle\Security\FirewallConfig" abstract="true" public="false">
<argument /> <!-- name --> <argument /> <!-- name -->
<argument /> <!-- request_matcher --> <argument /> <!-- request_matcher -->
<argument /> <!-- user_checker -->
<argument /> <!-- security enabled --> <argument /> <!-- security enabled -->
<argument /> <!-- stateless --> <argument /> <!-- stateless -->
<argument /> <!-- provider --> <argument /> <!-- provider -->
@ -124,7 +125,6 @@
<argument /> <!-- entry_point --> <argument /> <!-- entry_point -->
<argument /> <!-- access_denied_handler --> <argument /> <!-- access_denied_handler -->
<argument /> <!-- access_denied_url --> <argument /> <!-- access_denied_url -->
<argument /> <!-- user_checker -->
<argument type="collection" /> <!-- listeners --> <argument type="collection" /> <!-- listeners -->
</service> </service>

View File

@ -18,6 +18,7 @@ final class FirewallConfig
{ {
private $name; private $name;
private $requestMatcher; private $requestMatcher;
private $userChecker;
private $securityEnabled; private $securityEnabled;
private $stateless; private $stateless;
private $provider; private $provider;
@ -25,12 +26,12 @@ final class FirewallConfig
private $entryPoint; private $entryPoint;
private $accessDeniedHandler; private $accessDeniedHandler;
private $accessDeniedUrl; private $accessDeniedUrl;
private $userChecker;
private $listeners; private $listeners;
/** /**
* @param string $name * @param string $name
* @param string $requestMatcher * @param string $requestMatcher
* @param string $userChecker
* @param bool $securityEnabled * @param bool $securityEnabled
* @param bool $stateless * @param bool $stateless
* @param string|null $provider * @param string|null $provider
@ -38,13 +39,13 @@ final class FirewallConfig
* @param string|null $entryPoint * @param string|null $entryPoint
* @param string|null $accessDeniedHandler * @param string|null $accessDeniedHandler
* @param string|null $accessDeniedUrl * @param string|null $accessDeniedUrl
* @param string|null $userChecker
* @param string[] $listeners * @param string[] $listeners
*/ */
public function __construct($name, $requestMatcher, $securityEnabled = true, $stateless = false, $provider = null, $context = null, $entryPoint = null, $accessDeniedHandler = null, $accessDeniedUrl = null, $userChecker = null, $listeners = array()) public function __construct($name, $requestMatcher, $userChecker, $securityEnabled = true, $stateless = false, $provider = null, $context = null, $entryPoint = null, $accessDeniedHandler = null, $accessDeniedUrl = null, $listeners = array())
{ {
$this->name = $name; $this->name = $name;
$this->requestMatcher = $requestMatcher; $this->requestMatcher = $requestMatcher;
$this->userChecker = $userChecker;
$this->securityEnabled = $securityEnabled; $this->securityEnabled = $securityEnabled;
$this->stateless = $stateless; $this->stateless = $stateless;
$this->provider = $provider; $this->provider = $provider;
@ -52,7 +53,6 @@ final class FirewallConfig
$this->entryPoint = $entryPoint; $this->entryPoint = $entryPoint;
$this->accessDeniedHandler = $accessDeniedHandler; $this->accessDeniedHandler = $accessDeniedHandler;
$this->accessDeniedUrl = $accessDeniedUrl; $this->accessDeniedUrl = $accessDeniedUrl;
$this->userChecker = $userChecker;
$this->listeners = $listeners; $this->listeners = $listeners;
} }

View File

@ -78,7 +78,7 @@ class SecurityDataCollectorTest extends \PHPUnit_Framework_TestCase
public function testGetFirewall() public function testGetFirewall()
{ {
$firewallConfig = new FirewallConfig('dummy', 'security.request_matcher.dummy'); $firewallConfig = new FirewallConfig('dummy', 'security.request_matcher.dummy', 'security.user_checker.dummy');
$request = $this->getRequest(); $request = $this->getRequest();
$firewallMap = $this $firewallMap = $this

View File

@ -79,16 +79,17 @@ abstract class CompleteConfigurationTest extends \PHPUnit_Framework_TestCase
array( array(
'simple', 'simple',
'security.request_matcher.707b20193d4cb9f2718114abcbebb32af48f948484fc166a03482f49bf14f25e271f72c7', 'security.request_matcher.707b20193d4cb9f2718114abcbebb32af48f948484fc166a03482f49bf14f25e271f72c7',
'security.user_checker',
false, false,
), ),
array( array(
'secure', 'secure',
'', '',
'security.user_checker',
true, true,
true, true,
'security.user.provider.concrete.default', 'security.user.provider.concrete.default',
'security.authentication.form_entry_point.secure', 'security.authentication.form_entry_point.secure',
'security.user_checker',
array( array(
'logout', 'logout',
'switch_user', 'switch_user',
@ -104,12 +105,12 @@ abstract class CompleteConfigurationTest extends \PHPUnit_Framework_TestCase
array( array(
'host', 'host',
'security.request_matcher.dda8b565689ad8509623ee68fb2c639cd81cd4cb339d60edbaf7d67d30e6aa09bd8c63c3', 'security.request_matcher.dda8b565689ad8509623ee68fb2c639cd81cd4cb339d60edbaf7d67d30e6aa09bd8c63c3',
'security.user_checker',
true, true,
false, false,
'security.user.provider.concrete.default', 'security.user.provider.concrete.default',
'host', 'host',
'security.authentication.basic_entry_point.host', 'security.authentication.basic_entry_point.host',
'security.user_checker',
array( array(
'http_basic', 'http_basic',
'anonymous', 'anonymous',
@ -118,12 +119,12 @@ abstract class CompleteConfigurationTest extends \PHPUnit_Framework_TestCase
array( array(
'with_user_checker', 'with_user_checker',
'', '',
'app.user_checker',
true, true,
false, false,
'security.user.provider.concrete.default', 'security.user.provider.concrete.default',
'with_user_checker', 'with_user_checker',
'security.authentication.basic_entry_point.with_user_checker', 'security.authentication.basic_entry_point.with_user_checker',
'app.user_checker',
array( array(
'http_basic', 'http_basic',
'anonymous', 'anonymous',

View File

@ -33,6 +33,7 @@ class FirewallConfigTest extends \PHPUnit_Framework_TestCase
$config = new FirewallConfig( $config = new FirewallConfig(
'foo_firewall', 'foo_firewall',
$options['request_matcher'], $options['request_matcher'],
$options['user_checker'],
$options['security'], $options['security'],
$options['stateless'], $options['stateless'],
$options['provider'], $options['provider'],
@ -40,7 +41,6 @@ class FirewallConfigTest extends \PHPUnit_Framework_TestCase
$options['entry_point'], $options['entry_point'],
$options['access_denied_handler'], $options['access_denied_handler'],
$options['access_denied_url'], $options['access_denied_url'],
$options['user_checker'],
$listeners $listeners
); );

View File

@ -20,7 +20,7 @@ class FirewallContextTest extends \PHPUnit_Framework_TestCase
{ {
public function testGetters() public function testGetters()
{ {
$config = new FirewallConfig('main', 'request_matcher'); $config = new FirewallConfig('main', 'request_matcher', 'user_checker');
$exceptionListener = $this $exceptionListener = $this
->getMockBuilder(ExceptionListener::class) ->getMockBuilder(ExceptionListener::class)