Skip to content

Commit

Permalink
bug #25508 [FrameworkBundle] Auto-enable CSRF if the component *+ ses…
Browse files Browse the repository at this point in the history
…sion* are loaded (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] Auto-enable CSRF if the component *+ session* are loaded

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

By binding CSRF and session default state, we provide better DX, but we also provide a way for bundles to enable session on its own: they just need to require "symfony/security-csrf".
Yes, that's a side effect, but I think that's a nice one for 3.4/4.0.
Of course, we might do better in 4.1, but for bug fix only releases, LGTM.

Commits
-------

9e8231f [FrameworkBundle] Automatically enable the CSRF if component *+ session* are loaded
  • Loading branch information
nicolas-grekas committed Jan 16, 2018
2 parents 283e8d3 + 9e8231f commit f3ac9f5
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
Expand Up @@ -21,6 +21,7 @@
use Symfony\Component\Form\Form;
use Symfony\Component\Lock\Lock;
use Symfony\Component\Lock\Store\SemaphoreStore;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Symfony\Component\Serializer\Serializer;
use Symfony\Component\Translation\Translator;
use Symfony\Component\Validator\Validation;
Expand Down Expand Up @@ -142,7 +143,14 @@ private function addCsrfSection(ArrayNodeDefinition $rootNode)
$rootNode
->children()
->arrayNode('csrf_protection')
->canBeEnabled()
->treatFalseLike(array('enabled' => false))
->treatTrueLike(array('enabled' => true))
->treatNullLike(array('enabled' => true))
->addDefaultsIfNotSet()
->children()
// defaults to framework.session.enabled && !class_exists(FullStack::class) && interface_exists(CsrfTokenManagerInterface::class)
->booleanNode('enabled')->defaultNull()->end()
->end()
->end()
->end()
;
Expand Down
Expand Up @@ -17,6 +17,7 @@
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Bundle\FrameworkBundle\Routing\AnnotatedRouteControllerLoader;
use Symfony\Bundle\FullStack;
use Symfony\Component\Cache\Adapter\AbstractAdapter;
use Symfony\Component\Cache\Adapter\AdapterInterface;
use Symfony\Component\Cache\Adapter\ArrayAdapter;
Expand Down Expand Up @@ -65,6 +66,7 @@
use Symfony\Component\Routing\Loader\AnnotationDirectoryLoader;
use Symfony\Component\Routing\Loader\AnnotationFileLoader;
use Symfony\Component\Security\Core\Security;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Symfony\Component\Serializer\Encoder\DecoderInterface;
use Symfony\Component\Serializer\Encoder\EncoderInterface;
use Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory;
Expand Down Expand Up @@ -231,6 +233,11 @@ public function load(array $configs, ContainerBuilder $container)
$this->registerRequestConfiguration($config['request'], $container, $loader);
}

if (null === $config['csrf_protection']['enabled']) {
$config['csrf_protection']['enabled'] = $this->sessionConfigEnabled && !class_exists(FullStack::class) && interface_exists(CsrfTokenManagerInterface::class);
}
$this->registerSecurityCsrfConfiguration($config['csrf_protection'], $container, $loader);

if ($this->isConfigEnabled($container, $config['form'])) {
if (!class_exists('Symfony\Component\Form\Form')) {
throw new LogicException('Form support cannot be enabled as the Form component is not installed.');
Expand All @@ -251,8 +258,6 @@ public function load(array $configs, ContainerBuilder $container)
$container->removeDefinition('console.command.form_debug');
}

$this->registerSecurityCsrfConfiguration($config['csrf_protection'], $container, $loader);

if ($this->isConfigEnabled($container, $config['assets'])) {
if (!class_exists('Symfony\Component\Asset\Package')) {
throw new LogicException('Asset support cannot be enabled as the Asset component is not installed.');
Expand Down

0 comments on commit f3ac9f5

Please sign in to comment.