From fef224d1904da67120fdfe9de3d070f8ba607742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Fri, 28 Jan 2022 18:32:12 +0100 Subject: [PATCH] Enable CSRF in FORM by default --- DependencyInjection/FrameworkExtension.php | 125 +++++++++--------- .../Fixtures/php/form_default_csrf.php | 11 ++ .../Fixtures/xml/form_default_csrf.xml | 13 ++ .../Fixtures/yml/form_default_csrf.yml | 6 + .../FrameworkExtensionTest.php | 12 ++ 5 files changed, 108 insertions(+), 59 deletions(-) create mode 100644 Tests/DependencyInjection/Fixtures/php/form_default_csrf.php create mode 100644 Tests/DependencyInjection/Fixtures/xml/form_default_csrf.xml create mode 100644 Tests/DependencyInjection/Fixtures/yml/form_default_csrf.yml diff --git a/DependencyInjection/FrameworkExtension.php b/DependencyInjection/FrameworkExtension.php index c2d54a704..059c4036f 100644 --- a/DependencyInjection/FrameworkExtension.php +++ b/DependencyInjection/FrameworkExtension.php @@ -311,26 +311,6 @@ public function load(array $configs, ContainerBuilder $container) $this->registerRequestConfiguration($config['request'], $container, $loader); } - if ($this->isConfigEnabled($container, $config['form'])) { - if (!class_exists(Form::class)) { - throw new LogicException('Form support cannot be enabled as the Form component is not installed. Try running "composer require symfony/form".'); - } - - $this->formConfigEnabled = true; - $this->registerFormConfiguration($config, $container, $loader); - - if (ContainerBuilder::willBeAvailable('symfony/validator', Validation::class, ['symfony/framework-bundle', 'symfony/form'])) { - $config['validation']['enabled'] = true; - } else { - $container->setParameter('validator.translation_domain', 'validators'); - - $container->removeDefinition('form.type_extension.form.validator'); - $container->removeDefinition('form.type_guesser.validator'); - } - } else { - $container->removeDefinition('console.command.form_debug'); - } - if ($this->isConfigEnabled($container, $config['assets'])) { if (!class_exists(\Symfony\Component\Asset\Package::class)) { throw new LogicException('Asset support cannot be enabled as the Asset component is not installed. Try running "composer require symfony/asset".'); @@ -339,39 +319,6 @@ public function load(array $configs, ContainerBuilder $container) $this->registerAssetsConfiguration($config['assets'], $container, $loader); } - if ($this->messengerConfigEnabled = $this->isConfigEnabled($container, $config['messenger'])) { - $this->registerMessengerConfiguration($config['messenger'], $container, $loader, $config['validation']); - } else { - $container->removeDefinition('console.command.messenger_consume_messages'); - $container->removeDefinition('console.command.messenger_debug'); - $container->removeDefinition('console.command.messenger_stop_workers'); - $container->removeDefinition('console.command.messenger_setup_transports'); - $container->removeDefinition('console.command.messenger_failed_messages_retry'); - $container->removeDefinition('console.command.messenger_failed_messages_show'); - $container->removeDefinition('console.command.messenger_failed_messages_remove'); - $container->removeDefinition('cache.messenger.restart_workers_signal'); - - if ($container->hasDefinition('messenger.transport.amqp.factory') && !class_exists(AmqpTransportFactory::class)) { - if (class_exists(\Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransportFactory::class)) { - $container->getDefinition('messenger.transport.amqp.factory') - ->setClass(\Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransportFactory::class) - ->addTag('messenger.transport_factory'); - } else { - $container->removeDefinition('messenger.transport.amqp.factory'); - } - } - - if ($container->hasDefinition('messenger.transport.redis.factory') && !class_exists(RedisTransportFactory::class)) { - if (class_exists(\Symfony\Component\Messenger\Transport\RedisExt\RedisTransportFactory::class)) { - $container->getDefinition('messenger.transport.redis.factory') - ->setClass(\Symfony\Component\Messenger\Transport\RedisExt\RedisTransportFactory::class) - ->addTag('messenger.transport_factory'); - } else { - $container->removeDefinition('messenger.transport.redis.factory'); - } - } - } - if ($this->httpClientConfigEnabled = $this->isConfigEnabled($container, $config['http_client'])) { $this->registerHttpClientConfiguration($config['http_client'], $container, $loader, $config['profiler']); } @@ -380,18 +327,12 @@ public function load(array $configs, ContainerBuilder $container) $this->registerMailerConfiguration($config['mailer'], $container, $loader); } - if ($this->notifierConfigEnabled = $this->isConfigEnabled($container, $config['notifier'])) { - $this->registerNotifierConfiguration($config['notifier'], $container, $loader); - } - $propertyInfoEnabled = $this->isConfigEnabled($container, $config['property_info']); - $this->registerValidationConfiguration($config['validation'], $container, $loader, $propertyInfoEnabled); $this->registerHttpCacheConfiguration($config['http_cache'], $container, $config['http_method_override']); $this->registerEsiConfiguration($config['esi'], $container, $loader); $this->registerSsiConfiguration($config['ssi'], $container, $loader); $this->registerFragmentsConfiguration($config['fragments'], $container, $loader); $this->registerTranslatorConfiguration($config['translator'], $container, $loader, $config['default_locale']); - $this->registerProfilerConfiguration($config['profiler'], $container, $loader); $this->registerWorkflowConfiguration($config['workflows'], $container, $loader); $this->registerDebugConfiguration($config['php_errors'], $container, $loader); $this->registerRouterConfiguration($config['router'], $container, $loader, $config['translator']['enabled_locales'] ?? []); @@ -461,6 +402,72 @@ public function load(array $configs, ContainerBuilder $container) } $this->registerSecurityCsrfConfiguration($config['csrf_protection'], $container, $loader); + // form depends on csrf being registered + if ($this->isConfigEnabled($container, $config['form'])) { + if (!class_exists(Form::class)) { + throw new LogicException('Form support cannot be enabled as the Form component is not installed. Try running "composer require symfony/form".'); + } + + $this->formConfigEnabled = true; + $this->registerFormConfiguration($config, $container, $loader); + + if (ContainerBuilder::willBeAvailable('symfony/validator', Validation::class, ['symfony/framework-bundle', 'symfony/form'])) { + $config['validation']['enabled'] = true; + } else { + $container->setParameter('validator.translation_domain', 'validators'); + + $container->removeDefinition('form.type_extension.form.validator'); + $container->removeDefinition('form.type_guesser.validator'); + } + } else { + $container->removeDefinition('console.command.form_debug'); + } + + // validation depends on form, annotations being registered + $this->registerValidationConfiguration($config['validation'], $container, $loader, $propertyInfoEnabled); + + // messenger depends on validation being registered + if ($this->messengerConfigEnabled = $this->isConfigEnabled($container, $config['messenger'])) { + $this->registerMessengerConfiguration($config['messenger'], $container, $loader, $config['validation']); + } else { + $container->removeDefinition('console.command.messenger_consume_messages'); + $container->removeDefinition('console.command.messenger_debug'); + $container->removeDefinition('console.command.messenger_stop_workers'); + $container->removeDefinition('console.command.messenger_setup_transports'); + $container->removeDefinition('console.command.messenger_failed_messages_retry'); + $container->removeDefinition('console.command.messenger_failed_messages_show'); + $container->removeDefinition('console.command.messenger_failed_messages_remove'); + $container->removeDefinition('cache.messenger.restart_workers_signal'); + + if ($container->hasDefinition('messenger.transport.amqp.factory') && !class_exists(AmqpTransportFactory::class)) { + if (class_exists(\Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransportFactory::class)) { + $container->getDefinition('messenger.transport.amqp.factory') + ->setClass(\Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransportFactory::class) + ->addTag('messenger.transport_factory'); + } else { + $container->removeDefinition('messenger.transport.amqp.factory'); + } + } + + if ($container->hasDefinition('messenger.transport.redis.factory') && !class_exists(RedisTransportFactory::class)) { + if (class_exists(\Symfony\Component\Messenger\Transport\RedisExt\RedisTransportFactory::class)) { + $container->getDefinition('messenger.transport.redis.factory') + ->setClass(\Symfony\Component\Messenger\Transport\RedisExt\RedisTransportFactory::class) + ->addTag('messenger.transport_factory'); + } else { + $container->removeDefinition('messenger.transport.redis.factory'); + } + } + } + + // notifier depends on messenger, mailer being registered + if ($this->notifierConfigEnabled = $this->isConfigEnabled($container, $config['notifier'])) { + $this->registerNotifierConfiguration($config['notifier'], $container, $loader); + } + + // profiler depends on form, validation, translation, messenger, mailer, http-client, notifier being registered + $this->registerProfilerConfiguration($config['profiler'], $container, $loader); + $this->addAnnotatedClassesToCompile([ '**\\Controller\\', '**\\Entity\\', diff --git a/Tests/DependencyInjection/Fixtures/php/form_default_csrf.php b/Tests/DependencyInjection/Fixtures/php/form_default_csrf.php new file mode 100644 index 000000000..a57d52338 --- /dev/null +++ b/Tests/DependencyInjection/Fixtures/php/form_default_csrf.php @@ -0,0 +1,11 @@ +loadFromExtension('framework', [ + 'form' => [ + 'legacy_error_messages' => false, + ], + 'session' => [ + 'storage_factory_id' => 'session.storage.factory.native', + 'handler_id' => null, + ], +]); diff --git a/Tests/DependencyInjection/Fixtures/xml/form_default_csrf.xml b/Tests/DependencyInjection/Fixtures/xml/form_default_csrf.xml new file mode 100644 index 000000000..9ed400847 --- /dev/null +++ b/Tests/DependencyInjection/Fixtures/xml/form_default_csrf.xml @@ -0,0 +1,13 @@ + + + + + + + + + diff --git a/Tests/DependencyInjection/Fixtures/yml/form_default_csrf.yml b/Tests/DependencyInjection/Fixtures/yml/form_default_csrf.yml new file mode 100644 index 000000000..5036cc857 --- /dev/null +++ b/Tests/DependencyInjection/Fixtures/yml/form_default_csrf.yml @@ -0,0 +1,6 @@ +framework: + form: + legacy_error_messages: false + session: + storage_factory_id: session.storage.factory.native + handler_id: null diff --git a/Tests/DependencyInjection/FrameworkExtensionTest.php b/Tests/DependencyInjection/FrameworkExtensionTest.php index b13e6a5c3..fd418824e 100644 --- a/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -159,6 +159,18 @@ public function testCsrfProtectionForFormsEnablesCsrfProtectionAutomatically() $this->assertTrue($container->hasDefinition('security.csrf.token_manager')); } + public function testFormsCsrfIsEnabledByDefault() + { + if (class_exists(FullStack::class)) { + $this->markTestSkipped('testing with the FullStack prevents verifying default values'); + } + $container = $this->createContainerFromFile('form_default_csrf'); + + $this->assertTrue($container->hasDefinition('security.csrf.token_manager')); + $this->assertTrue($container->hasParameter('form.type_extension.csrf.enabled')); + $this->assertTrue($container->getParameter('form.type_extension.csrf.enabled')); + } + public function testHttpMethodOverride() { $container = $this->createContainerFromFile('full');