Skip to content
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

Enable double-submit CSRF protection #1337

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nicolas-grekas
Copy link
Member

Q A
License MIT
Doc issue/PR -

Relies on symfony/symfony#58095

Copy link

github-actions bot commented Sep 2, 2024

Thanks for the PR 😍

How to test these changes in your application

  1. Define the SYMFONY_ENDPOINT environment variable:

    # On Unix-like (BSD, Linux and macOS)
    export SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1337/index.json
    # On Windows
    SET SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1337/index.json
  2. Install the package(s) related to this recipe:

    composer req 'symfony/flex:^1.16'
    composer req 'symfony/framework-bundle:^7.2' 'symfony/stimulus-bundle:^2.19' 'symfony/ux-turbo:^2.19'
  3. Don't forget to unset the SYMFONY_ENDPOINT environment variable when done:

    # On Unix-like (BSD, Linux and macOS)
    unset SYMFONY_ENDPOINT
    # On Windows
    SET SYMFONY_ENDPOINT=

Diff between recipe versions

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes.
I'm going keep this comment up to date with any updates of the attached patch.

symfony/framework-bundle

3.3 vs 3.4
diff --git a/symfony/framework-bundle/3.3/config/packages/framework.yaml b/symfony/framework-bundle/3.4/config/packages/framework.yaml
index d2b31bf..f532576 100644
--- a/symfony/framework-bundle/3.3/config/packages/framework.yaml
+++ b/symfony/framework-bundle/3.4/config/packages/framework.yaml
@@ -7,6 +7,7 @@ framework:
     # Remove or comment this section to explicitly disable session support.
     session:
         handler_id: null
+        cookie_samesite: lax
 
     #esi: true
     #fragments: true
3.4 vs 4.2
diff --git a/symfony/framework-bundle/3.4/config/bootstrap.php b/symfony/framework-bundle/4.2/config/bootstrap.php
index 2a47186..55560fb 100644
--- a/symfony/framework-bundle/3.4/config/bootstrap.php
+++ b/symfony/framework-bundle/4.2/config/bootstrap.php
@@ -13,38 +13,8 @@ if (!class_exists(Dotenv::class)) {
 if (is_array($env = @include dirname(__DIR__).'/.env.local.php') && (!isset($env['APP_ENV']) || ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? $env['APP_ENV']) === $env['APP_ENV'])) {
     (new Dotenv(false))->populate($env);
 } else {
-    $path = dirname(__DIR__).'/.env';
-    $dotenv = new Dotenv(false);
-
     // load all the .env files
-    if (method_exists($dotenv, 'loadEnv')) {
-        $dotenv->loadEnv($path);
-    } else {
-        // fallback code in case your Dotenv component is not 4.2 or higher (when loadEnv() was added)
-
-        if (file_exists($path) || !file_exists($p = "$path.dist")) {
-            $dotenv->load($path);
-        } else {
-            $dotenv->load($p);
-        }
-
-        if (null === $env = $_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? null) {
-            $dotenv->populate(array('APP_ENV' => $env = 'dev'));
-        }
-
-        if ('test' !== $env && file_exists($p = "$path.local")) {
-            $dotenv->load($p);
-            $env = $_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? $env;
-        }
-
-        if (file_exists($p = "$path.$env")) {
-            $dotenv->load($p);
-        }
-
-        if (file_exists($p = "$path.$env.local")) {
-            $dotenv->load($p);
-        }
-    }
+    (new Dotenv(false))->loadEnv(dirname(__DIR__).'/.env');
 }
 
 $_SERVER += $_ENV;
diff --git a/symfony/framework-bundle/3.4/config/packages/framework.yaml b/symfony/framework-bundle/4.2/config/packages/framework.yaml
index f532576..cad7f78 100644
--- a/symfony/framework-bundle/3.4/config/packages/framework.yaml
+++ b/symfony/framework-bundle/4.2/config/packages/framework.yaml
@@ -1,3 +1,4 @@
+# see https://symfony.com/doc/current/reference/configuration/framework.html
 framework:
     secret: '%env(APP_SECRET)%'
     #csrf_protection: true
@@ -7,6 +8,7 @@ framework:
     # Remove or comment this section to explicitly disable session support.
     session:
         handler_id: null
+        cookie_secure: auto
         cookie_samesite: lax
 
     #esi: true
diff --git a/symfony/framework-bundle/3.4/config/services.yaml b/symfony/framework-bundle/4.2/config/services.yaml
index 07d653c..99d51bd 100644
--- a/symfony/framework-bundle/3.4/config/services.yaml
+++ b/symfony/framework-bundle/4.2/config/services.yaml
@@ -10,15 +10,12 @@ services:
     _defaults:
         autowire: true      # Automatically injects dependencies in your services.
         autoconfigure: true # Automatically registers your services as commands, event subscribers, etc.
-        public: false       # Allows optimizing the container by removing unused services; this also means
-                            # fetching services directly from the container via $container->get() won't work.
-                            # The best practice is to be explicit about your dependencies anyway.
 
     # makes classes in src/ available to be used as services
     # this creates a service per class whose id is the fully-qualified class name
     App\:
         resource: '../src/*'
-        exclude: '../src/{DependencyInjection,Entity,Migrations,Tests,Kernel.php}'
+        exclude: '../src/{DependencyInjection,Entity,Kernel.php}'
 
     # controllers are imported separately to make sure services can be injected
     # as action arguments even if you don't extend any base controller class
diff --git a/symfony/framework-bundle/3.4/manifest.json b/symfony/framework-bundle/4.2/manifest.json
index aa0150e..101b2aa 100644
--- a/symfony/framework-bundle/3.4/manifest.json
+++ b/symfony/framework-bundle/4.2/manifest.json
@@ -14,13 +14,14 @@
     "env": {
         "APP_ENV": "dev",
         "APP_SECRET": "%generate(secret)%",
-        "#TRUSTED_PROXIES": "127.0.0.1,127.0.0.2",
+        "#TRUSTED_PROXIES": "127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16",
         "#TRUSTED_HOSTS": "'^(localhost|example\\.com)$'"
     },
     "gitignore": [
         "/.env.local",
         "/.env.local.php",
         "/.env.*.local",
+        "/%CONFIG_DIR%/secrets/prod/prod.decrypt.private.php",
         "/%PUBLIC_DIR%/bundles/",
         "/%VAR_DIR%/",
         "/vendor/"
diff --git a/symfony/framework-bundle/3.4/post-install.txt b/symfony/framework-bundle/4.2/post-install.txt
index 944aa06..12f3669 100644
--- a/symfony/framework-bundle/3.4/post-install.txt
+++ b/symfony/framework-bundle/4.2/post-install.txt
@@ -1,7 +1,6 @@
   * Run your application:
     1. Go to the project directory
     2. Create your code repository with the git init command
-    3. Download the Symfony CLI at https://symfony.com/download to install a development web server,
-       or run composer require server --dev for a minimalist one
+    3. Download the Symfony CLI at https://symfony.com/download to install a development web server
 
   * Read the documentation at https://symfony.com/doc
diff --git a/symfony/framework-bundle/3.4/src/Kernel.php b/symfony/framework-bundle/4.2/src/Kernel.php
index 68b7a56..1cd0572 100644
--- a/symfony/framework-bundle/3.4/src/Kernel.php
+++ b/symfony/framework-bundle/4.2/src/Kernel.php
@@ -13,19 +13,9 @@ class Kernel extends BaseKernel
 {
     use MicroKernelTrait;
 
-    const CONFIG_EXTS = '.{php,xml,yaml,yml}';
+    private const CONFIG_EXTS = '.{php,xml,yaml,yml}';
 
-    public function getCacheDir()
-    {
-        return $this->getProjectDir().'/var/cache/'.$this->environment;
-    }
-
-    public function getLogDir()
-    {
-        return $this->getProjectDir().'/var/log';
-    }
-
-    public function registerBundles()
+    public function registerBundles(): iterable
     {
         $contents = require $this->getProjectDir().'/config/bundles.php';
         foreach ($contents as $class => $envs) {
@@ -35,13 +25,16 @@ class Kernel extends BaseKernel
         }
     }
 
-    protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader)
+    public function getProjectDir(): string
+    {
+        return \dirname(__DIR__);
+    }
+
+    protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader): void
     {
         $container->addResource(new FileResource($this->getProjectDir().'/config/bundles.php'));
-        // Feel free to remove the "container.autowiring.strict_mode" parameter
-        // if you are using symfony/dependency-injection 4.0+ as it's the default behavior
-        $container->setParameter('container.autowiring.strict_mode', true);
-        $container->setParameter('container.dumper.inline_class_loader', true);
+        $container->setParameter('container.dumper.inline_class_loader', \PHP_VERSION_ID < 70400 || $this->debug);
+        $container->setParameter('container.dumper.inline_factories', true);
         $confDir = $this->getProjectDir().'/config';
 
         $loader->load($confDir.'/{packages}/*'.self::CONFIG_EXTS, 'glob');
@@ -50,7 +43,7 @@ class Kernel extends BaseKernel
         $loader->load($confDir.'/{services}_'.$this->environment.self::CONFIG_EXTS, 'glob');
     }
 
-    protected function configureRoutes(RouteCollectionBuilder $routes)
+    protected function configureRoutes(RouteCollectionBuilder $routes): void
     {
         $confDir = $this->getProjectDir().'/config';
 
4.2 vs 4.4
diff --git a/symfony/framework-bundle/4.4/config/preload.php b/symfony/framework-bundle/4.4/config/preload.php
new file mode 100644
index 0000000..064bdcd
--- /dev/null
+++ b/symfony/framework-bundle/4.4/config/preload.php
@@ -0,0 +1,9 @@
+<?php
+
+if (file_exists(dirname(__DIR__).'/var/cache/prod/srcApp_KernelProdContainer.preload.php')) {
+    require dirname(__DIR__).'/var/cache/prod/srcApp_KernelProdContainer.preload.php';
+}
+
+if (file_exists(dirname(__DIR__).'/var/cache/prod/App_KernelProdContainer.preload.php')) {
+    require dirname(__DIR__).'/var/cache/prod/App_KernelProdContainer.preload.php';
+}
diff --git a/symfony/framework-bundle/4.4/config/routes/dev/framework.yaml b/symfony/framework-bundle/4.4/config/routes/dev/framework.yaml
new file mode 100644
index 0000000..bcbbf13
--- /dev/null
+++ b/symfony/framework-bundle/4.4/config/routes/dev/framework.yaml
@@ -0,0 +1,3 @@
+_errors:
+    resource: '@FrameworkBundle/Resources/config/routing/errors.xml'
+    prefix: /_error
diff --git a/symfony/framework-bundle/4.2/config/services.yaml b/symfony/framework-bundle/4.4/config/services.yaml
index 99d51bd..9557caa 100644
--- a/symfony/framework-bundle/4.2/config/services.yaml
+++ b/symfony/framework-bundle/4.4/config/services.yaml
@@ -14,13 +14,16 @@ services:
     # makes classes in src/ available to be used as services
     # this creates a service per class whose id is the fully-qualified class name
     App\:
-        resource: '../src/*'
-        exclude: '../src/{DependencyInjection,Entity,Kernel.php}'
+        resource: '../src/'
+        exclude:
+            - '../src/DependencyInjection/'
+            - '../src/Entity/'
+            - '../src/Kernel.php'
 
     # controllers are imported separately to make sure services can be injected
     # as action arguments even if you don't extend any base controller class
     App\Controller\:
-        resource: '../src/Controller'
+        resource: '../src/Controller/'
         tags: ['controller.service_arguments']
 
     # add more service definitions when explicit configuration is needed
diff --git a/symfony/framework-bundle/4.2/public/index.php b/symfony/framework-bundle/4.4/public/index.php
index 929197c..d0b6e02 100644
--- a/symfony/framework-bundle/4.2/public/index.php
+++ b/symfony/framework-bundle/4.4/public/index.php
@@ -1,7 +1,7 @@
 <?php
 
 use App\Kernel;
-use Symfony\Component\Debug\Debug;
+use Symfony\Component\ErrorHandler\Debug;
 use Symfony\Component\HttpFoundation\Request;
 
 require dirname(__DIR__).'/config/bootstrap.php';
4.4 vs 5.1
diff --git a/symfony/framework-bundle/4.4/config/bootstrap.php b/symfony/framework-bundle/4.4/config/bootstrap.php
deleted file mode 100644
index 55560fb..0000000
--- a/symfony/framework-bundle/4.4/config/bootstrap.php
+++ /dev/null
@@ -1,23 +0,0 @@
-<?php
-
-use Symfony\Component\Dotenv\Dotenv;
-
-require dirname(__DIR__).'/vendor/autoload.php';
-
-if (!class_exists(Dotenv::class)) {
-    throw new LogicException('Please run "composer require symfony/dotenv" to load the ".env" files configuring the application.');
-}
-
-// Load cached env vars if the .env.local.php file exists
-// Run "composer dump-env prod" to create it (requires symfony/flex >=1.2)
-if (is_array($env = @include dirname(__DIR__).'/.env.local.php') && (!isset($env['APP_ENV']) || ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? $env['APP_ENV']) === $env['APP_ENV'])) {
-    (new Dotenv(false))->populate($env);
-} else {
-    // load all the .env files
-    (new Dotenv(false))->loadEnv(dirname(__DIR__).'/.env');
-}
-
-$_SERVER += $_ENV;
-$_SERVER['APP_ENV'] = $_ENV['APP_ENV'] = ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? null) ?: 'dev';
-$_SERVER['APP_DEBUG'] = $_SERVER['APP_DEBUG'] ?? $_ENV['APP_DEBUG'] ?? 'prod' !== $_SERVER['APP_ENV'];
-$_SERVER['APP_DEBUG'] = $_ENV['APP_DEBUG'] = (int) $_SERVER['APP_DEBUG'] || filter_var($_SERVER['APP_DEBUG'], FILTER_VALIDATE_BOOLEAN) ? '1' : '0';
diff --git a/symfony/framework-bundle/4.4/config/preload.php b/symfony/framework-bundle/5.1/config/preload.php
index 064bdcd..5ebcdb2 100644
--- a/symfony/framework-bundle/4.4/config/preload.php
+++ b/symfony/framework-bundle/5.1/config/preload.php
@@ -1,9 +1,5 @@
 <?php
 
-if (file_exists(dirname(__DIR__).'/var/cache/prod/srcApp_KernelProdContainer.preload.php')) {
-    require dirname(__DIR__).'/var/cache/prod/srcApp_KernelProdContainer.preload.php';
-}
-
 if (file_exists(dirname(__DIR__).'/var/cache/prod/App_KernelProdContainer.preload.php')) {
     require dirname(__DIR__).'/var/cache/prod/App_KernelProdContainer.preload.php';
 }
diff --git a/symfony/framework-bundle/4.4/public/index.php b/symfony/framework-bundle/5.1/public/index.php
index d0b6e02..097baa3 100644
--- a/symfony/framework-bundle/4.4/public/index.php
+++ b/symfony/framework-bundle/5.1/public/index.php
@@ -1,10 +1,13 @@
 <?php
 
 use App\Kernel;
+use Symfony\Component\Dotenv\Dotenv;
 use Symfony\Component\ErrorHandler\Debug;
 use Symfony\Component\HttpFoundation\Request;
 
-require dirname(__DIR__).'/config/bootstrap.php';
+require dirname(__DIR__).'/vendor/autoload.php';
+
+(new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
 
 if ($_SERVER['APP_DEBUG']) {
     umask(0000);
diff --git a/symfony/framework-bundle/4.4/src/Kernel.php b/symfony/framework-bundle/5.1/src/Kernel.php
index 1cd0572..655e796 100644
--- a/symfony/framework-bundle/4.4/src/Kernel.php
+++ b/symfony/framework-bundle/5.1/src/Kernel.php
@@ -3,52 +3,36 @@
 namespace App;
 
 use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
-use Symfony\Component\Config\Loader\LoaderInterface;
-use Symfony\Component\Config\Resource\FileResource;
-use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
 use Symfony\Component\HttpKernel\Kernel as BaseKernel;
-use Symfony\Component\Routing\RouteCollectionBuilder;
+use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;
 
 class Kernel extends BaseKernel
 {
     use MicroKernelTrait;
 
-    private const CONFIG_EXTS = '.{php,xml,yaml,yml}';
-
-    public function registerBundles(): iterable
+    protected function configureContainer(ContainerConfigurator $container): void
     {
-        $contents = require $this->getProjectDir().'/config/bundles.php';
-        foreach ($contents as $class => $envs) {
-            if ($envs[$this->environment] ?? $envs['all'] ?? false) {
-                yield new $class();
-            }
+        $container->import('../config/{packages}/*.yaml');
+        $container->import('../config/{packages}/'.$this->environment.'/*.yaml');
+
+        if (is_file(\dirname(__DIR__).'/config/services.yaml')) {
+            $container->import('../config/services.yaml');
+            $container->import('../config/{services}_'.$this->environment.'.yaml');
+        } elseif (is_file($path = \dirname(__DIR__).'/config/services.php')) {
+            (require $path)($container->withPath($path), $this);
         }
     }
 
-    public function getProjectDir(): string
+    protected function configureRoutes(RoutingConfigurator $routes): void
     {
-        return \dirname(__DIR__);
-    }
+        $routes->import('../config/{routes}/'.$this->environment.'/*.yaml');
+        $routes->import('../config/{routes}/*.yaml');
 
-    protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader): void
-    {
-        $container->addResource(new FileResource($this->getProjectDir().'/config/bundles.php'));
-        $container->setParameter('container.dumper.inline_class_loader', \PHP_VERSION_ID < 70400 || $this->debug);
-        $container->setParameter('container.dumper.inline_factories', true);
-        $confDir = $this->getProjectDir().'/config';
-
-        $loader->load($confDir.'/{packages}/*'.self::CONFIG_EXTS, 'glob');
-        $loader->load($confDir.'/{packages}/'.$this->environment.'/*'.self::CONFIG_EXTS, 'glob');
-        $loader->load($confDir.'/{services}'.self::CONFIG_EXTS, 'glob');
-        $loader->load($confDir.'/{services}_'.$this->environment.self::CONFIG_EXTS, 'glob');
-    }
-
-    protected function configureRoutes(RouteCollectionBuilder $routes): void
-    {
-        $confDir = $this->getProjectDir().'/config';
-
-        $routes->import($confDir.'/{routes}/'.$this->environment.'/*'.self::CONFIG_EXTS, '/', 'glob');
-        $routes->import($confDir.'/{routes}/*'.self::CONFIG_EXTS, '/', 'glob');
-        $routes->import($confDir.'/{routes}'.self::CONFIG_EXTS, '/', 'glob');
+        if (is_file(\dirname(__DIR__).'/config/routes.yaml')) {
+            $routes->import('../config/routes.yaml');
+        } elseif (is_file($path = \dirname(__DIR__).'/config/routes.php')) {
+            (require $path)($routes->withPath($path), $this);
+        }
     }
 }
5.1 vs 5.2
diff --git a/symfony/framework-bundle/5.1/manifest.json b/symfony/framework-bundle/5.2/manifest.json
index 101b2aa..17fa50a 100644
--- a/symfony/framework-bundle/5.1/manifest.json
+++ b/symfony/framework-bundle/5.2/manifest.json
@@ -13,9 +13,7 @@
     },
     "env": {
         "APP_ENV": "dev",
-        "APP_SECRET": "%generate(secret)%",
-        "#TRUSTED_PROXIES": "127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16",
-        "#TRUSTED_HOSTS": "'^(localhost|example\\.com)$'"
+        "APP_SECRET": "%generate(secret)%"
     },
     "gitignore": [
         "/.env.local",
diff --git a/symfony/framework-bundle/5.1/public/index.php b/symfony/framework-bundle/5.2/public/index.php
index 097baa3..3bcee0b 100644
--- a/symfony/framework-bundle/5.1/public/index.php
+++ b/symfony/framework-bundle/5.2/public/index.php
@@ -15,14 +15,6 @@ if ($_SERVER['APP_DEBUG']) {
     Debug::enable();
 }
 
-if ($trustedProxies = $_SERVER['TRUSTED_PROXIES'] ?? false) {
-    Request::setTrustedProxies(explode(',', $trustedProxies), Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO);
-}
-
-if ($trustedHosts = $_SERVER['TRUSTED_HOSTS'] ?? false) {
-    Request::setTrustedHosts([$trustedHosts]);
-}
-
 $kernel = new Kernel($_SERVER['APP_ENV'], (bool) $_SERVER['APP_DEBUG']);
 $request = Request::createFromGlobals();
 $response = $kernel->handle($request);
5.2 vs 5.3
diff --git a/symfony/framework-bundle/5.2/config/packages/framework.yaml b/symfony/framework-bundle/5.3/config/packages/framework.yaml
index cad7f78..7853e9e 100644
--- a/symfony/framework-bundle/5.2/config/packages/framework.yaml
+++ b/symfony/framework-bundle/5.3/config/packages/framework.yaml
@@ -2,7 +2,7 @@
 framework:
     secret: '%env(APP_SECRET)%'
     #csrf_protection: true
-    #http_method_override: true
+    http_method_override: false
 
     # Enables session support. Note that the session will ONLY be started if you read or write from it.
     # Remove or comment this section to explicitly disable session support.
@@ -10,8 +10,15 @@ framework:
         handler_id: null
         cookie_secure: auto
         cookie_samesite: lax
+        storage_factory_id: session.storage.factory.native
 
     #esi: true
     #fragments: true
     php_errors:
         log: true
+
+when@test:
+    framework:
+        test: true
+        session:
+            storage_factory_id: session.storage.factory.mock_file
diff --git a/symfony/framework-bundle/5.2/config/packages/test/framework.yaml b/symfony/framework-bundle/5.2/config/packages/test/framework.yaml
deleted file mode 100644
index d051c84..0000000
--- a/symfony/framework-bundle/5.2/config/packages/test/framework.yaml
+++ /dev/null
@@ -1,4 +0,0 @@
-framework:
-    test: true
-    session:
-        storage_id: session.storage.mock_file
diff --git a/symfony/framework-bundle/5.2/config/routes/dev/framework.yaml b/symfony/framework-bundle/5.2/config/routes/dev/framework.yaml
deleted file mode 100644
index bcbbf13..0000000
--- a/symfony/framework-bundle/5.2/config/routes/dev/framework.yaml
+++ /dev/null
@@ -1,3 +0,0 @@
-_errors:
-    resource: '@FrameworkBundle/Resources/config/routing/errors.xml'
-    prefix: /_error
diff --git a/symfony/framework-bundle/5.3/config/routes/framework.yaml b/symfony/framework-bundle/5.3/config/routes/framework.yaml
new file mode 100644
index 0000000..0fc74bb
--- /dev/null
+++ b/symfony/framework-bundle/5.3/config/routes/framework.yaml
@@ -0,0 +1,4 @@
+when@dev:
+    _errors:
+        resource: '@FrameworkBundle/Resources/config/routing/errors.xml'
+        prefix: /_error
diff --git a/symfony/framework-bundle/5.2/config/services.yaml b/symfony/framework-bundle/5.3/config/services.yaml
index 9557caa..2d6a76f 100644
--- a/symfony/framework-bundle/5.2/config/services.yaml
+++ b/symfony/framework-bundle/5.3/config/services.yaml
@@ -2,7 +2,7 @@
 # Files in the packages/ subdirectory configure your dependencies.
 
 # Put parameters here that don't need to change on each machine where the app is deployed
-# https://symfony.com/doc/current/best_practices/configuration.html#application-related-configuration
+# https://symfony.com/doc/current/best_practices.html#use-parameters-for-application-configuration
 parameters:
 
 services:
@@ -20,11 +20,5 @@ services:
             - '../src/Entity/'
             - '../src/Kernel.php'
 
-    # controllers are imported separately to make sure services can be injected
-    # as action arguments even if you don't extend any base controller class
-    App\Controller\:
-        resource: '../src/Controller/'
-        tags: ['controller.service_arguments']
-
     # add more service definitions when explicit configuration is needed
     # please note that last definitions always *replace* previous ones
diff --git a/symfony/framework-bundle/5.2/public/index.php b/symfony/framework-bundle/5.3/public/index.php
index 3bcee0b..9982c21 100644
--- a/symfony/framework-bundle/5.2/public/index.php
+++ b/symfony/framework-bundle/5.3/public/index.php
@@ -1,22 +1,9 @@
 <?php
 
 use App\Kernel;
-use Symfony\Component\Dotenv\Dotenv;
-use Symfony\Component\ErrorHandler\Debug;
-use Symfony\Component\HttpFoundation\Request;
 
-require dirname(__DIR__).'/vendor/autoload.php';
+require_once dirname(__DIR__).'/vendor/autoload_runtime.php';
 
-(new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
-
-if ($_SERVER['APP_DEBUG']) {
-    umask(0000);
-
-    Debug::enable();
-}
-
-$kernel = new Kernel($_SERVER['APP_ENV'], (bool) $_SERVER['APP_DEBUG']);
-$request = Request::createFromGlobals();
-$response = $kernel->handle($request);
-$response->send();
-$kernel->terminate($request, $response);
+return function (array $context) {
+    return new Kernel($context['APP_ENV'], (bool) $context['APP_DEBUG']);
+};
diff --git a/symfony/framework-bundle/5.2/src/Kernel.php b/symfony/framework-bundle/5.3/src/Kernel.php
index 655e796..8e96873 100644
--- a/symfony/framework-bundle/5.2/src/Kernel.php
+++ b/symfony/framework-bundle/5.3/src/Kernel.php
@@ -19,8 +19,8 @@ class Kernel extends BaseKernel
         if (is_file(\dirname(__DIR__).'/config/services.yaml')) {
             $container->import('../config/services.yaml');
             $container->import('../config/{services}_'.$this->environment.'.yaml');
-        } elseif (is_file($path = \dirname(__DIR__).'/config/services.php')) {
-            (require $path)($container->withPath($path), $this);
+        } else {
+            $container->import('../config/{services}.php');
         }
     }
 
@@ -31,8 +31,8 @@ class Kernel extends BaseKernel
 
         if (is_file(\dirname(__DIR__).'/config/routes.yaml')) {
             $routes->import('../config/routes.yaml');
-        } elseif (is_file($path = \dirname(__DIR__).'/config/routes.php')) {
-            (require $path)($routes->withPath($path), $this);
+        } else {
+            $routes->import('../config/{routes}.php');
         }
     }
 }
5.3 vs 5.4
diff --git a/symfony/framework-bundle/5.3/src/Kernel.php b/symfony/framework-bundle/5.4/src/Kernel.php
index 8e96873..779cd1f 100644
--- a/symfony/framework-bundle/5.3/src/Kernel.php
+++ b/symfony/framework-bundle/5.4/src/Kernel.php
@@ -3,36 +3,9 @@
 namespace App;
 
 use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
-use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
 use Symfony\Component\HttpKernel\Kernel as BaseKernel;
-use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;
 
 class Kernel extends BaseKernel
 {
     use MicroKernelTrait;
-
-    protected function configureContainer(ContainerConfigurator $container): void
-    {
-        $container->import('../config/{packages}/*.yaml');
-        $container->import('../config/{packages}/'.$this->environment.'/*.yaml');
-
-        if (is_file(\dirname(__DIR__).'/config/services.yaml')) {
-            $container->import('../config/services.yaml');
-            $container->import('../config/{services}_'.$this->environment.'.yaml');
-        } else {
-            $container->import('../config/{services}.php');
-        }
-    }
-
-    protected function configureRoutes(RoutingConfigurator $routes): void
-    {
-        $routes->import('../config/{routes}/'.$this->environment.'/*.yaml');
-        $routes->import('../config/{routes}/*.yaml');
-
-        if (is_file(\dirname(__DIR__).'/config/routes.yaml')) {
-            $routes->import('../config/routes.yaml');
-        } else {
-            $routes->import('../config/{routes}.php');
-        }
-    }
 }
5.4 vs 6.2
diff --git a/symfony/framework-bundle/5.4/config/packages/framework.yaml b/symfony/framework-bundle/6.2/config/packages/framework.yaml
index 7853e9e..6d85c29 100644
--- a/symfony/framework-bundle/5.4/config/packages/framework.yaml
+++ b/symfony/framework-bundle/6.2/config/packages/framework.yaml
@@ -3,6 +3,7 @@ framework:
     secret: '%env(APP_SECRET)%'
     #csrf_protection: true
     http_method_override: false
+    handle_all_throwables: true
 
     # Enables session support. Note that the session will ONLY be started if you read or write from it.
     # Remove or comment this section to explicitly disable session support.
6.2 vs 6.4
diff --git a/symfony/framework-bundle/6.2/config/packages/framework.yaml b/symfony/framework-bundle/6.4/config/packages/framework.yaml
index 6d85c29..28095da 100644
--- a/symfony/framework-bundle/6.2/config/packages/framework.yaml
+++ b/symfony/framework-bundle/6.4/config/packages/framework.yaml
@@ -1,7 +1,7 @@
 # see https://symfony.com/doc/current/reference/configuration/framework.html
 framework:
     secret: '%env(APP_SECRET)%'
-    #csrf_protection: true
+    annotations: false
     http_method_override: false
     handle_all_throwables: true
 
@@ -11,7 +11,6 @@ framework:
         handler_id: null
         cookie_secure: auto
         cookie_samesite: lax
-        storage_factory_id: session.storage.factory.native
 
     #esi: true
     #fragments: true
6.4 vs 7.0
diff --git a/symfony/framework-bundle/6.4/config/packages/framework.yaml b/symfony/framework-bundle/7.0/config/packages/framework.yaml
index 28095da..877eb25 100644
--- a/symfony/framework-bundle/6.4/config/packages/framework.yaml
+++ b/symfony/framework-bundle/7.0/config/packages/framework.yaml
@@ -1,21 +1,13 @@
 # see https://symfony.com/doc/current/reference/configuration/framework.html
 framework:
     secret: '%env(APP_SECRET)%'
-    annotations: false
-    http_method_override: false
-    handle_all_throwables: true
+    #csrf_protection: true
 
-    # Enables session support. Note that the session will ONLY be started if you read or write from it.
-    # Remove or comment this section to explicitly disable session support.
-    session:
-        handler_id: null
-        cookie_secure: auto
-        cookie_samesite: lax
+    # Note that the session will be started ONLY if you read or write from it.
+    session: true
 
     #esi: true
     #fragments: true
-    php_errors:
-        log: true
 
 when@test:
     framework:
7.0 vs 7.2
diff --git a/symfony/framework-bundle/7.0/config/packages/framework.yaml b/symfony/framework-bundle/7.2/config/packages/framework.yaml
index 877eb25..9b68462 100644
--- a/symfony/framework-bundle/7.0/config/packages/framework.yaml
+++ b/symfony/framework-bundle/7.2/config/packages/framework.yaml
@@ -1,7 +1,10 @@
 # see https://symfony.com/doc/current/reference/configuration/framework.html
 framework:
     secret: '%env(APP_SECRET)%'
-    #csrf_protection: true
+
+    form: { csrf_protection: { token_id: submit } }
+    csrf_protection:
+        double_submit_token_ids: [submit, authenticate, logout]
 
     # Note that the session will be started ONLY if you read or write from it.
     session: true
diff --git a/symfony/framework-bundle/7.0/manifest.json b/symfony/framework-bundle/7.2/manifest.json
index 17fa50a..d466ccd 100644
--- a/symfony/framework-bundle/7.0/manifest.json
+++ b/symfony/framework-bundle/7.2/manifest.json
@@ -13,7 +13,7 @@
     },
     "env": {
         "APP_ENV": "dev",
-        "APP_SECRET": "%generate(secret)%"
+        "APP_SECRET": ""
     },
     "gitignore": [
         "/.env.local",

symfony/stimulus-bundle

2.8 vs 2.9
diff --git a/symfony/stimulus-bundle/2.9/assets/bootstrap.js b/symfony/stimulus-bundle/2.9/assets/bootstrap.js
new file mode 100644
index 0000000..2689398
--- /dev/null
+++ b/symfony/stimulus-bundle/2.9/assets/bootstrap.js
@@ -0,0 +1,2 @@
+// register any custom, 3rd party controllers here
+// app.register('some_controller_name', SomeImportedController);
diff --git a/symfony/stimulus-bundle/2.9/assets/controllers/hello_controller.js b/symfony/stimulus-bundle/2.9/assets/controllers/hello_controller.js
new file mode 100644
index 0000000..e847027
--- /dev/null
+++ b/symfony/stimulus-bundle/2.9/assets/controllers/hello_controller.js
@@ -0,0 +1,16 @@
+import { Controller } from '@hotwired/stimulus';
+
+/*
+ * This is an example Stimulus controller!
+ *
+ * Any element with a data-controller="hello" attribute will cause
+ * this controller to be executed. The name "hello" comes from the filename:
+ * hello_controller.js -> "hello"
+ *
+ * Delete this file or adapt it for your use!
+ */
+export default class extends Controller {
+    connect() {
+        this.element.textContent = 'Hello Stimulus! Edit me in assets/controllers/hello_controller.js';
+    }
+}
diff --git a/symfony/stimulus-bundle/2.9/assets/controllers.json b/symfony/stimulus-bundle/2.9/assets/controllers.json
new file mode 100644
index 0000000..a1c6e90
--- /dev/null
+++ b/symfony/stimulus-bundle/2.9/assets/controllers.json
@@ -0,0 +1,4 @@
+{
+    "controllers": [],
+    "entrypoints": []
+}
diff --git a/symfony/stimulus-bundle/2.8/manifest.json b/symfony/stimulus-bundle/2.9/manifest.json
index ff66e87..60e0ddb 100644
--- a/symfony/stimulus-bundle/2.8/manifest.json
+++ b/symfony/stimulus-bundle/2.9/manifest.json
@@ -2,5 +2,46 @@
     "bundles": {
         "Symfony\\UX\\StimulusBundle\\StimulusBundle": ["all"]
     },
-    "aliases": ["stimulus", "stimulus-bundle"]
+    "copy-from-recipe": {
+        "assets/": "assets/"
+    },
+    "aliases": ["stimulus", "stimulus-bundle"],
+    "conflict": {
+        "symfony/webpack-encore-bundle": "<2.0",
+        "symfony/flex": "<1.20.0 || >=2.0.0,<2.3.0"
+    },
+    "add-lines": [
+        {
+            "file": "webpack.config.js",
+            "content": "\n    // enables the Symfony UX Stimulus bridge (used in assets/bootstrap.js)\n    .enableStimulusBridge('./assets/controllers.json')",
+            "position": "after_target",
+            "target": ".splitEntryChunks()"
+        },
+        {
+            "file": "assets/app.js",
+            "content": "import './bootstrap.js';",
+            "position": "top",
+            "warn_if_missing": true
+        },
+        {
+            "file": "assets/bootstrap.js",
+            "content": "import { startStimulusApp } from '@symfony/stimulus-bridge';\n\n// Registers Stimulus controllers from controllers.json and in the controllers/ directory\nexport const app = startStimulusApp(require.context(\n    '@symfony/stimulus-bridge/lazy-controller-loader!./controllers',\n    true,\n    /\\.[jt]sx?$/\n));",
+            "position": "top",
+            "requires": "symfony/webpack-encore-bundle"
+        },
+        {
+            "file": "assets/bootstrap.js",
+            "content": "import { startStimulusApp } from '@symfony/stimulus-bundle';\n\nconst app = startStimulusApp();",
+            "position": "top",
+            "requires": "symfony/asset-mapper"
+        },
+        {
+            "file": "templates/base.html.twig",
+            "content": "            {{ ux_controller_link_tags() }}",
+            "position": "after_target",
+            "target": "{% block stylesheets %}",
+            "warn_if_missing": true,
+            "requires": "symfony/asset-mapper"
+        }
+    ]
 }
2.9 vs 2.13
diff --git a/symfony/stimulus-bundle/2.9/manifest.json b/symfony/stimulus-bundle/2.13/manifest.json
index 60e0ddb..4701215 100644
--- a/symfony/stimulus-bundle/2.9/manifest.json
+++ b/symfony/stimulus-bundle/2.13/manifest.json
@@ -34,14 +34,6 @@
             "content": "import { startStimulusApp } from '@symfony/stimulus-bundle';\n\nconst app = startStimulusApp();",
             "position": "top",
             "requires": "symfony/asset-mapper"
-        },
-        {
-            "file": "templates/base.html.twig",
-            "content": "            {{ ux_controller_link_tags() }}",
-            "position": "after_target",
-            "target": "{% block stylesheets %}",
-            "warn_if_missing": true,
-            "requires": "symfony/asset-mapper"
         }
     ]
 }
2.13 vs 2.19
diff --git a/symfony/stimulus-bundle/2.19/assets/controllers/csrf_protection_controller.js b/symfony/stimulus-bundle/2.19/assets/controllers/csrf_protection_controller.js
new file mode 100644
index 0000000..b4ddf49
--- /dev/null
+++ b/symfony/stimulus-bundle/2.19/assets/controllers/csrf_protection_controller.js
@@ -0,0 +1,53 @@
+document.addEventListener('submit', function (event) {
+    var csrfField = event.target.querySelector('input[data-controller="csrf-protection"]');
+
+    if (!csrfField) {
+        return;
+    }
+
+    var csrfCookie = csrfField.getAttribute('data-csrf-protection-cookie-value');
+    var csrfToken = csrfField.value;
+
+    if (!csrfCookie && /^[-_a-zA-Z0-9]{4,22}$/.test(csrfToken)) {
+        csrfField.setAttribute('data-csrf-protection-cookie-value', csrfCookie = csrfToken);
+        csrfField.value = csrfToken = btoa(String.fromCharCode.apply(null, (window.crypto || window.msCrypto).getRandomValues(new Uint8Array(18))));
+    }
+
+    if (csrfCookie && /^[-_/+a-zA-Z0-9]{24,}$/.test(csrfToken)) {
+        var cookie = csrfCookie + '_' + csrfToken + '=' + csrfCookie + '; path=/; samesite=strict';
+        document.cookie = window.location.protocol === 'https:' ? '__Host-' + cookie + '; secure' : cookie;
+    }
+});
+
+document.addEventListener('turbo:submit-start', function (event) {
+    var csrfField = event.detail.formSubmission.formElement.querySelector('input[data-controller="csrf-protection"]');
+
+    if (!csrfField) {
+        return;
+    }
+
+    var csrfCookie = csrfField.getAttribute('data-csrf-protection-cookie-value');
+
+    if (/^[-_/+a-zA-Z0-9]{24,}$/.test(csrfField.value) && /^[-_a-zA-Z0-9]{4,22}$/.test(csrfCookie)) {
+        event.detail.formSubmission.fetchRequest.headers[csrfCookie] = csrfField.value;
+    }
+});
+
+document.addEventListener('turbo:submit-end', function (event) {
+    var csrfField = event.detail.formSubmission.formElement.querySelector('input[data-controller="csrf-protection"]');
+
+    if (!csrfField) {
+        return;
+    }
+
+    var csrfCookie = csrfField.getAttribute('data-csrf-protection-cookie-value');
+
+    if (/^[-_/+a-zA-Z0-9]{24,}$/.test(csrfField.value) && /^[-_a-zA-Z0-9]{4,22}$/.test(csrfCookie)) {
+        var cookie = csrfCookie + '_' + csrfField.value + '=0; path=/; samesite=strict; max-age=0';
+
+        document.cookie = window.location.protocol === 'https:' ? '__Host-' + cookie + '; secure' : cookie;
+    }
+});
+
+/* stimulusFetch: 'lazy' */
+export default 'csrf-protection-controller';
diff --git a/symfony/stimulus-bundle/2.13/manifest.json b/symfony/stimulus-bundle/2.19/manifest.json
index 4701215..4289495 100644
--- a/symfony/stimulus-bundle/2.13/manifest.json
+++ b/symfony/stimulus-bundle/2.19/manifest.json
@@ -7,6 +7,8 @@
     },
     "aliases": ["stimulus", "stimulus-bundle"],
     "conflict": {
+        "symfony/framework-bundle": "<7.2",
+        "symfony/security-csrf": "<7.2",
         "symfony/webpack-encore-bundle": "<2.0",
         "symfony/flex": "<1.20.0 || >=2.0.0,<2.3.0"
     },

Copy link
Member

@stof stof left a comment

Choose a reason for hiding this comment

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

should we enable this double-submit validation by default ? For sites that rely on stateful authentication anyway, the session-based CSRF token manager has the benefit of being compatible with form submissions done without JS (either because JS is disabled or because it failed to execute)

symfony/stimulus-bundle/2.19/manifest.json Show resolved Hide resolved
@@ -0,0 +1,35 @@
document.addEventListener('submit', function (event) {
Copy link
Member

Choose a reason for hiding this comment

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

IMO, it would make sense to ship this controller in a symfony/ux package instead of a recipe, so that it can be properly maintained (for instance, there is no good way to manage security advisories for code provided by recipes, because the advisories would have to apply to projects that were created based on the recipe, not on the recipe itself, as the recipe is not a dependency of the project).

Then, we could have the recipe of that bundle turning on the double-submit CSRF protection.

Copy link
Member Author

@nicolas-grekas nicolas-grekas Sep 2, 2024

Choose a reason for hiding this comment

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

I'm not sure we should take control of that snippet. By putting it in client's app, we make it easy to tweak. But moving it to vendor dep would require making it parametrized, and this is a path we'd better not take IMHO.
If we have to change the script for security reasons, we can always use another data-contoller attribute by default so that older versions become dead-code.

Copy link
Member

Choose a reason for hiding this comment

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

We're trying to slim down the number of ux packages to only include ones that have a decent amount of both PHP and JS. Our long(ish)-term plan is to create a sort of recipe system on ux.symfony.com where user's could copy paste code (stimulus controllers, twig components, etc) into their apps. Several of our existing ux packages would then be deprecated in favour of a recipe.

Because the snippets are copy/pasted we don't have to worry about breaking apps when we improve the recipes.

That being said, the security situation isn't something I've thought of with regards to this system.

Copy link
Member

Choose a reason for hiding this comment

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

The issue with copy-paste and that the project in which you paste the code has no relation to the place where you copied it, and so security advisory systems cannot copy the advisories they have for the source to all projects in which you pasted the code (not even considering the fact that advisories are about affected versions of each project).

When the code is just glue code, this is fine as this is generally not the source of a security issues (if some particular usages of a library would be insecure, the library itself would have an advisory). But this case is not about glue code but implementing the (security-related) feature itself.

Copy link
Member Author

@nicolas-grekas nicolas-grekas Sep 16, 2024

Choose a reason for hiding this comment

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

Several of our existing ux packages would then be deprecated in favour of a recipe.

That plan is not needed: keep the packages but turn them into symfony-packs instead, that'll provide the best DX.

About putting that snippet for double-submit, I have the same approach: I don't want to make this a vendor package. I prefer putting people in charge.

The security aspect of this is very limited. The server-side is in control of what it accepts.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Sep 2, 2024

should we enable this double-submit validation by default ? For sites that rely on stateful authentication anyway, the session-based CSRF token manager has the benefit of being compatible with form submissions done without JS (either because JS is disabled or because it failed to execute)

We should yes IMHO. Submissions done without JS are also compatible thanks to the Origin check, which is enough to accept the submission. The cookie submission becomes required only if a previous submission successfully used it (which means JS was enabled before). The only thing this would break is support for legacy browsers that don't send neither origin nor have JS enabled. Given we're talking about new projects here, that's acceptable (and it's still possible to reconfigure so everybody is covered, advanced users should do some more work as expected, but the DX is best by default.)

"file": "config/packages/framework.yaml",
"position": "after_target",
"target": " csrf_protection:",
"content": " check_header: true"
Copy link

Choose a reason for hiding this comment

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

Do i understand this wrong or ... will it fail for every form where Turbo is not enabled ?

Because this is a very common scenario

Copy link
Member Author

@nicolas-grekas nicolas-grekas Sep 3, 2024

Choose a reason for hiding this comment

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

you've got it right - new projects start with turbo enabled for every forms

Copy link

Choose a reason for hiding this comment

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

Ok.. bold but we'll see :) And worst case a PR is quickly done to add an option :)

@smnandre
Copy link

⚠️ reminder to not push before 7.2 (i know it's obvious.. but we will release a 2.20 soon so i rather say it ^^)

@@ -2,6 +2,10 @@
framework:
secret: '%env(APP_SECRET)%'

form: { csrf_protection: { token_id: submit } }
csrf_protection:
double_submit_token_ids: [submit, authenticate, logout]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to link to a documentation page here what this means with pros and cons. It's not very self-explaning.

@@ -2,6 +2,10 @@
framework:
secret: '%env(APP_SECRET)%'

form: { csrf_protection: { token_id: submit } }
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes every form use the same token ID from what I understand. But this is in constrast to the recommendation at https://symfony.com/doc/current/security/csrf.html#csrf-protection-in-symfony-forms

// an arbitrary string used to generate the value of the token
// using a different string for each form improves its security
'csrf_token_id' => 'task_item',

If that is not true, it should be changed. What's the point of the token ID then at all? Just a way to distinguish different forms?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the comments. I agree docs on the topic need to be changed after this PR.
(Yes, token id is to differentiate by use case for each tokens, which means by forms most of the time, but this is not required for the CSRF protection to be effective)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants