From aa06e8328eaac5cbdc7ac02d57a6d3a9f7c63b18 Mon Sep 17 00:00:00 2001
From: Kevin Decherf <kevin@kdecherf.com>
Date: Sat, 30 Sep 2023 00:46:19 +0200
Subject: [PATCH] ConfigController: remove 2fa cancel step

This change annoys me, however this endpoint was anyway problematic:
- it was vulnerable to a CSRF attack, see GHSA-56fm-hfp3-x3w3
- it is useless as we don't really handle a two-steps validation

Still, if you send an incorrect code during the "activation" phase a
flash error will pop up but the 2fa will stay enabled. This need rework
when possible.

Signed-off-by: Kevin Decherf <kevin@kdecherf.com>
---
 .../Controller/ConfigController.php           | 13 ++++++--
 .../Resources/views/Config/otp_app.html.twig  |  3 --
 .../Controller/ConfigControllerTest.php       | 30 -------------------
 3 files changed, 10 insertions(+), 36 deletions(-)

diff --git a/src/Wallabag/CoreBundle/Controller/ConfigController.php b/src/Wallabag/CoreBundle/Controller/ConfigController.php
index db318dddf..cfe992f59 100644
--- a/src/Wallabag/CoreBundle/Controller/ConfigController.php
+++ b/src/Wallabag/CoreBundle/Controller/ConfigController.php
@@ -373,8 +373,10 @@ class ConfigController extends AbstractController
      * Cancelling 2FA using OTP app.
      *
      * @Route("/config/otp/app/cancel", name="config_otp_app_cancel")
+     * 
+     * XXX: commented until we rewrite 2fa with a real two-steps activation
      */
-    public function otpAppCancelAction()
+    /*public function otpAppCancelAction()
     {
         $user = $this->getUser();
         $user->setGoogleAuthenticatorSecret(null);
@@ -383,7 +385,7 @@ class ConfigController extends AbstractController
         $this->userManager->updateUser($user, true);
 
         return $this->redirect($this->generateUrl('config') . '#set3');
-    }
+    }*/
 
     /**
      * Validate OTP code.
@@ -415,7 +417,12 @@ class ConfigController extends AbstractController
             'scheb_two_factor.code_invalid'
         );
 
-        return $this->redirect($this->generateUrl('config_otp_app'));
+        $this->addFlash(
+            'notice',
+            'scheb_two_factor.code_invalid'
+        );
+
+        return $this->redirect($this->generateUrl('config') . '#set3');
     }
 
     /**
diff --git a/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig b/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig
index f042fd500..6d5d402b1 100644
--- a/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig
+++ b/src/Wallabag/CoreBundle/Resources/views/Config/otp_app.html.twig
@@ -50,9 +50,6 @@
                                     </div>
                                 </div>
                                 <div class="card-action">
-                                    <a href="{{ path('config_otp_app_cancel') }}" class="waves-effect waves-light grey btn">
-                                        {{ 'config.otp.app.cancel'|trans }}
-                                    </a>
                                     <button class="btn waves-effect waves-light" type="submit" name="send">
                                         {{ 'config.otp.app.enable'|trans }}
                                         <i class="material-icons right">send</i>
diff --git a/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php
index 46f5c232e..c0d138dfa 100644
--- a/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php
+++ b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php
@@ -1254,36 +1254,6 @@ class ConfigControllerTest extends WallabagCoreTestCase
         $em->flush();
     }
 
-    public function testUserEnable2faGoogleCancel()
-    {
-        $this->logInAs('admin');
-        $client = $this->getTestClient();
-
-        $crawler = $client->request('GET', '/config/otp/app');
-
-        $this->assertSame(200, $client->getResponse()->getStatusCode());
-
-        // restore user
-        $em = $this->getEntityManager();
-        $user = $em
-            ->getRepository(User::class)
-            ->findOneByUsername('admin');
-
-        $this->assertTrue($user->isGoogleTwoFactor());
-        $this->assertGreaterThan(0, $user->getBackupCodes());
-
-        $crawler = $client->request('GET', '/config/otp/app/cancel');
-
-        $this->assertSame(302, $client->getResponse()->getStatusCode());
-
-        $user = $em
-            ->getRepository(User::class)
-            ->findOneByUsername('admin');
-
-        $this->assertFalse($user->isGoogleTwoFactor());
-        $this->assertEmpty($user->getBackupCodes());
-    }
-
     public function testUserDisable2faGoogle()
     {
         $this->logInAs('admin');