Skip to content

Commit

Permalink
Default access_decision_manager.strategy option with merge.
Browse files Browse the repository at this point in the history
Security bundle should set default access_decision_manager.strategy in extension instead of configuration.
Otherwise merging configurations will override first set option if the next configurations are empty.
  • Loading branch information
biozshock committed Nov 3, 2021
1 parent 49a0906 commit 18d4a66
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 19 deletions.
18 changes: 0 additions & 18 deletions DependencyInjection/MainConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,6 @@ public function getConfigTreeBuilder()
$rootNode = $tb->getRootNode();

$rootNode
->beforeNormalization()
->ifTrue(function ($v) {
if (!isset($v['access_decision_manager'])) {
return true;
}

if (!isset($v['access_decision_manager']['strategy']) && !isset($v['access_decision_manager']['service'])) {
return true;
}

return false;
})
->then(function ($v) {
$v['access_decision_manager']['strategy'] = AccessDecisionManager::STRATEGY_AFFIRMATIVE;

return $v;
})
->end()
->children()
->scalarNode('access_denied_url')->defaultNull()->example('/foo/error403')->end()
->enumNode('session_fixation_strategy')
Expand Down
3 changes: 2 additions & 1 deletion DependencyInjection/SecurityExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder;
Expand Down Expand Up @@ -130,7 +131,7 @@ public function load(array $configs, ContainerBuilder $container)
} else {
$container
->getDefinition('security.access.decision_manager')
->addArgument($config['access_decision_manager']['strategy'])
->addArgument($config['access_decision_manager']['strategy'] ?? AccessDecisionManager::STRATEGY_AFFIRMATIVE)
->addArgument($config['access_decision_manager']['allow_if_all_abstain'])
->addArgument($config['access_decision_manager']['allow_if_equal_granted_denied']);
}
Expand Down
19 changes: 19 additions & 0 deletions Tests/DependencyInjection/MainConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Bundle\SecurityBundle\DependencyInjection\MainConfiguration;
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
use Symfony\Component\Config\Definition\Processor;
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;

class MainConfigurationTest extends TestCase
{
Expand Down Expand Up @@ -113,4 +114,22 @@ public function testUserCheckers()

$this->assertEquals('app.henk_checker', $processedConfig['firewalls']['stub']['user_checker']);
}

public function testConfigMergeWithAccessDecisionManager()
{
$config = [
'access_decision_manager' => [
'strategy' => AccessDecisionManager::STRATEGY_UNANIMOUS,
],
];
$config = array_merge(static::$minimalConfig, $config);

$config2 = [];

$processor = new Processor();
$configuration = new MainConfiguration([], []);
$processedConfig = $processor->processConfiguration($configuration, [$config, $config2]);

$this->assertSame(AccessDecisionManager::STRATEGY_UNANIMOUS, $processedConfig['access_decision_manager']['strategy']);
}
}

0 comments on commit 18d4a66

Please sign in to comment.