Merge pull request #6289 from wallabag/2.5/fix-csrf-user-deletion

Fix CSRF on user deletion
wallabag · Feb 7, 2023 · 268372d · 268372d
2 parents 4e023bd + f1b3d5c
commit 268372d
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 8 deletions.
diff --git a/src/Wallabag/CoreBundle/Controller/ConfigController.php b/src/Wallabag/CoreBundle/Controller/ConfigController.php
@@ -586,14 +586,18 @@ public function resetAction($type)
     /**
      * Delete account for current user.
      *
-     * @Route("/account/delete", name="delete_account")
+     * @Route("/account/delete", name="delete_account", methods={"POST"})
      *
      * @throws AccessDeniedHttpException
      *
      * @return \Symfony\Component\HttpFoundation\RedirectResponse
      */
     public function deleteAccountAction(Request $request)
     {
+        if (!$this->isCsrfTokenValid('delete-account', $request->request->get('token'))) {
+            throw $this->createAccessDeniedException('Bad CSRF token.');
+        }
+
         $enabledUsers = $this->get('wallabag_user.user_repository')
             ->getSumEnabledUsers();
 

diff --git a/src/Wallabag/CoreBundle/Resources/views/themes/material/Config/index.html.twig b/src/Wallabag/CoreBundle/Resources/views/themes/material/Config/index.html.twig
@@ -548,7 +548,7 @@
                             </div>
                         </div>
                     </div>
-                    
+
                     <div id="set7" class="col s12">
                         <div class="row">
                             <h5>{{ 'config.reset.title'|trans }}</h5>
@@ -573,9 +573,11 @@
                             <div class="row">
                                 <h5>{{ 'config.form_user.delete.title'|trans }}</h5>
                                 <p>{{ 'config.form_user.delete.description'|trans }}</p>
-                                <a href="{{ path('delete_account') }}" onclick="return confirm('{{ 'config.form_user.delete.confirm'|trans|escape('js') }}')" class="waves-effect waves-light btn red delete-account">
-                                    {{ 'config.form_user.delete.button'|trans }}
-                                </a>
+                                <form action="{{ path('delete_account') }}" method="post" onsubmit="return confirm('{{ 'config.form_user.delete.confirm'|trans|escape('js') }}')" name="delete-account">
+                                    <input type="hidden" name="token" value="{{ csrf_token('delete-account') }}" />
+
+                                    <button class="waves-effect waves-light btn red" type="submit">{{ 'config.form_user.delete.button'|trans }}</button>
+                                </form>
                             </div>
                         {% endif %}
                     </div>

diff --git a/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php
@@ -794,7 +794,7 @@ public function testDeleteUserButtonVisibility()
         $this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text']));
         $this->assertStringNotContainsString('config.form_user.delete.button', $body[0]);
 
-        $client->request('GET', '/account/delete');
+        $client->request('POST', '/account/delete');
         $this->assertSame(403, $client->getResponse()->getStatusCode());
 
         $user = $em
@@ -860,9 +860,9 @@ public function testDeleteAccount()
 
         $crawler = $client->request('GET', '/config');
 
-        $deleteLink = $crawler->filter('.delete-account')->last()->link();
+        $deleteForm = $crawler->filter('form[name=delete-account]')->form();
 
-        $client->click($deleteLink);
+        $client->submit($deleteForm);
         $this->assertSame(302, $client->getResponse()->getStatusCode());
 
         $em = $client->getContainer()->get('doctrine.orm.entity_manager');