Skip to content

[SecurityBundle] Added csrf_token_generator and csrf_token_id as new #9587

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

Merged
merged 1 commit into from
Nov 23, 2013
Merged
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
3 changes: 3 additions & 0 deletions src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ CHANGELOG
-----

* Added 'host' option to firewall configuration
* Added 'csrf_token_generator' and 'csrf_token_id' options to firewall logout
listener configuration to supercede/alias 'csrf_provider' and 'intention'
respectively
* Moved 'security.secure_random' service configuration to FrameworkBundle

2.3.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,43 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto
->arrayNode('logout')
->treatTrueLike(array())
->canBeUnset()
->beforeNormalization()
->ifTrue(function($v) { return isset($v['csrf_provider']) && isset($v['csrf_token_generator']); })
->thenInvalid("You should define a value for only one of 'csrf_provider' and 'csrf_token_generator' on a security firewall. Use 'csrf_token_generator' as this replaces 'csrf_provider'.")
->end()
->beforeNormalization()
->ifTrue(function($v) { return isset($v['intention']) && isset($v['csrf_token_id']); })
->thenInvalid("You should define a value for only one of 'intention' and 'csrf_token_id' on a security firewall. Use 'csrf_token_id' as this replaces 'intention'.")
->end()
->beforeNormalization()
->ifTrue(function($v) { return isset($v['csrf_provider']); })
->then(function($v) {
$v['csrf_token_generator'] = $v['csrf_provider'];

return $v;
})
->end()
->beforeNormalization()
->ifTrue(function($v) { return isset($v['intention']); })
->then(function($v) {
$v['csrf_token_id'] = $v['intention'];

return $v;
})
->end()
->beforeNormalization()
->always()
->then(function ($v) {
unset($v['csrf_provider']);
unset($v['intention']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move these 2 unset calls in the previous normalization functions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, that would be better. Have submitted PR #9598.


return $v;
})
->end()
->children()
->scalarNode('csrf_parameter')->defaultValue('_csrf_token')->end()
->scalarNode('csrf_provider')->cannotBeEmpty()->end()
->scalarNode('intention')->defaultValue('logout')->end()
->scalarNode('csrf_token_generator')->cannotBeEmpty()->end()
->scalarNode('csrf_token_id')->defaultValue('logout')->end()
->scalarNode('path')->defaultValue('/logout')->end()
->scalarNode('target')->defaultValue('/')->end()
->scalarNode('success_handler')->end()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ protected function createListener($container, $id, $config, $userProvider)
$listenerId = parent::createListener($container, $id, $config, $userProvider);
$listener = $container->getDefinition($listenerId);

if (!isset($config['csrf_provider'])) {
if (!isset($config['csrf_token_generator'])) {
$listener->addArgument(null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
$listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.logout_listener'));
$listener->replaceArgument(3, array(
'csrf_parameter' => $firewall['logout']['csrf_parameter'],
'intention' => $firewall['logout']['intention'],
'intention' => $firewall['logout']['csrf_token_id'],
'logout_path' => $firewall['logout']['path'],
));
$listeners[] = new Reference($listenerId);
Expand All @@ -307,8 +307,8 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
$listener->replaceArgument(2, new Reference($logoutSuccessHandlerId));

// add CSRF provider
if (isset($firewall['logout']['csrf_provider'])) {
$listener->addArgument(new Reference($firewall['logout']['csrf_provider']));
if (isset($firewall['logout']['csrf_token_generator'])) {
$listener->addArgument(new Reference($firewall['logout']['csrf_token_generator']));
}

// add session logout handler
Expand Down Expand Up @@ -336,9 +336,9 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
->addMethodCall('registerListener', array(
$id,
$firewall['logout']['path'],
$firewall['logout']['intention'],
$firewall['logout']['csrf_token_id'],
$firewall['logout']['csrf_parameter'],
isset($firewall['logout']['csrf_provider']) ? new Reference($firewall['logout']['csrf_provider']) : null,
isset($firewall['logout']['csrf_token_generator']) ? new Reference($firewall['logout']['csrf_token_generator']) : null,
))
;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,49 @@ public function testManyConfigForProvider()
$configuration = new MainConfiguration(array(), array());
$config = $processor->processConfiguration($configuration, array($config));
}

public function testCsrfAliases()
{
$config = array(
'firewalls' => array(
'stub' => array(
'logout' => array(
'csrf_provider' => 'a_token_generator',
'intention' => 'a_token_id',
),
),
),
);
$config = array_merge(static::$minimalConfig, $config);

$processor = new Processor();
$configuration = new MainConfiguration(array(), array());
$processedConfig = $processor->processConfiguration($configuration, array($config));
$this->assertTrue(isset($processedConfig['firewalls']['stub']['logout']['csrf_token_generator']));
$this->assertEquals('a_token_generator', $processedConfig['firewalls']['stub']['logout']['csrf_token_generator']);
$this->assertTrue(isset($processedConfig['firewalls']['stub']['logout']['csrf_token_id']));
$this->assertEquals('a_token_id', $processedConfig['firewalls']['stub']['logout']['csrf_token_id']);
}

/**
* @expectedException InvalidArgumentException
*/
public function testCsrfOriginalAndAliasValueCausesException()
{
$config = array(
'firewalls' => array(
'stub' => array(
'logout' => array(
'csrf_token_id' => 'a_token_id',
'intention' => 'old_name',
),
),
),
);
$config = array_merge(static::$minimalConfig, $config);

$processor = new Processor();
$configuration = new MainConfiguration(array(), array());
$processedConfig = $processor->processConfiguration($configuration, array($config));
}
}