bug #48341 [SecurityBundle] Fix logout.csrf_token_generator default value (MatTheCat)

fabpot · fabpot · commit 25026b39f05a · 2022-11-26T17:42:59.000+01:00
This PR was merged into the 6.2 branch. Discussion ---------- [SecurityBundle] Fix `logout.csrf_token_generator` default value | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #48339 | License | MIT | Doc PR | N/A The token **manager** service ID configuration node is called <code>csrf_token_**generator**</code>. As such it has been wrongly assumed in #46580 `security.csrf.token_generator` was a good default value, whereas `security.csrf.token_manager` should be used (this is reflected by [the documentation](https://symfony.com/doc/current/reference/configuration/security.html#csrf-token-generator)). `csrf_token_generator` should ideally be deprecated and renamed `csrf_token_manager`. Commits ------- df539e2 [SecurityBundle] Fix `logout.csrf_token_generator` default value
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php
@@ -222,7 +222,7 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto
                         if (isset($v['csrf_token_generator'])) {
                             $v['enable_csrf'] = true;
                         } elseif ($v['enable_csrf']) {
-                            $v['csrf_token_generator'] = 'security.csrf.token_generator';
+                            $v['csrf_token_generator'] = 'security.csrf.token_manager';
                         }
 
                         return $v;
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php
@@ -122,7 +122,7 @@ public function testLogoutCsrf()
 
         $assertions = [
             'custom_token_generator' => [true, 'a_token_generator'],
-            'default_token_generator' => [true, 'security.csrf.token_generator'],
+            'default_token_generator' => [true, 'security.csrf.token_manager'],
             'disabled_csrf' => [false, null],
             'empty' => [false, null],
         ];
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php
@@ -69,6 +69,19 @@ public function testCookieClearingOnLogout()
         $this->assertNull($cookieJar->get('flavor'));
     }
 
+    public function testEnabledCsrf()
+    {
+        $client = $this->createClient(['test_case' => 'Logout', 'root_config' => 'config_csrf_enabled.yml']);
+
+        $cookieJar = $client->getCookieJar();
+        $cookieJar->set(new Cookie('flavor', 'chocolate', strtotime('+1 day'), null, 'somedomain'));
+
+        $client->request('POST', '/login', ['_username' => 'johannes', '_password' => 'test']);
+        $client->request('GET', '/logout');
+
+        $this->assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN);
+    }
+
     private function callInRequestContext(KernelBrowser $client, callable $callable): void
     {
         /** @var EventDispatcherInterface $eventDispatcher */
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/Logout/config_csrf_enabled.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/Logout/config_csrf_enabled.yml
@@ -0,0 +1,25 @@
+imports:
+- { resource: ./../config/framework.yml }
+
+security:
+    password_hashers:
+        Symfony\Component\Security\Core\User\InMemoryUser: plaintext
+
+    providers:
+        in_memory:
+            memory:
+                users:
+                    johannes: { password: test, roles: [ROLE_USER] }
+
+    firewalls:
+        default:
+            form_login:
+                check_path: login
+                remember_me: true
+                require_previous_session: false
+            logout:
+                enable_csrf: true
+
+    access_control:
+        - { path: ^/login$, roles: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: .*, roles: IS_AUTHENTICATED_FULLY }

Original file line number	Diff line number	Diff line change
`@@ -222,7 +222,7 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto`
`222`	`222`	`if (isset($v['csrf_token_generator'])) {`
`223`	`223`	`$v['enable_csrf'] = true;`
`224`	`224`	`} elseif ($v['enable_csrf']) {`
`225`		`- $v['csrf_token_generator'] = 'security.csrf.token_generator';`
	`225`	`+ $v['csrf_token_generator'] = 'security.csrf.token_manager';`
`226`	`226`	`}`
`227`	`227`
`228`	`228`	`return $v;`