User: Add improvements to 2FA - refs #6162

assets/vue/components/Login.vue

@@ -85,7 +85,6 @@
 
 <script setup>
 import { computed, ref } from "vue"
-import { useRouter } from "vue-router"
 import Button from "primevue/button"
 import InputText from "primevue/inputtext"
 import Password from "primevue/password"
@@ -94,45 +93,28 @@ import { useI18n } from "vue-i18n"
 import { useLogin } from "../composables/auth/login"
 import LoginOAuth2Buttons from "./login/LoginOAuth2Buttons.vue"
 import { usePlatformConfig } from "../store/platformConfig"
+import { useRouter } from "vue-router"
 
 const { t } = useI18n()
-
 const router = useRouter()
-
 const platformConfigStore = usePlatformConfig()
 const allowRegistration = computed(() => "false" !== platformConfigStore.getSetting("registration.allow_registration"))
 
-const { redirectNotAuthenticated, performLogin, isLoading } = useLogin()
+const { redirectNotAuthenticated, performLogin, isLoading, requires2FA } = useLogin()
 
 const login = ref("")
 const password = ref("")
 const totp = ref("")
 const remember = ref(false)
-const requires2FA = ref(false)
 
 redirectNotAuthenticated()
 
 async function onSubmitLoginForm() {
-  try {
-    const response = await performLogin({
-      login: login.value,
-      password: password.value,
-      totp: requires2FA.value ? totp.value : null,
-      _remember_me: remember.value,
-    })
-
-    if (!response) {
-      console.warn("[Login] No response from performLogin.")
-      return
-    }
-
-    if (response.requires2FA) {
-      requires2FA.value = true
-    } else {
-      await router.replace({ name: "Home" })
-    }
-  } catch (error) {
-    console.error("[Login] performLogin failed:", error)
-  }
+  const response = await performLogin({
+    login: login.value,
+    password: password.value,
+    totp: requires2FA.value ? totp.value : null,
+    _remember_me: remember.value,
+  })
 }
 </script>

assets/vue/composables/auth/login.js

@@ -25,44 +25,46 @@ export function useLogin() {
   const { showSuccessNotification, showErrorNotification } = useNotification()
 
   const isLoading = ref(false)
+  const requires2FA = ref(false)
 
   async function performLogin(payload) {
     isLoading.value = true
+    requires2FA.value = false
 
     try {
       const responseData = await securityService.login(payload)
 
-      if (responseData.requires2FA) {
-        return { success: true, requires2FA: true };
+      if (responseData.requires2FA && !payload.totp) {
+        requires2FA.value = true
+        return { success: false, requires2FA: true }
       }
 
-      if (route.query.redirect) {
-        // Check if 'redirect' is an absolute URL
-        if (isValidHttpUrl(route.query.redirect.toString())) {
-          // If it's an absolute URL, redirect directly
-          window.location.href = route.query.redirect.toString()
-
-          return
-        }
-      } else if (responseData.load_terms) {
-        window.location.href = responseData.redirect
-
-        return
+      if (responseData.error) {
+        showErrorNotification(responseData.error)
+        return { success: false, error: responseData.error }
       }
 
       securityStore.user = responseData
-
       await platformConfigurationStore.initialize()
 
       if (route.query.redirect) {
-        // If 'redirect' is a relative path, use 'router.push' to navigate
-        await router.replace({ path: route.query.redirect.toString() })
+        const redirect = route.query.redirect.toString()
+        if (isValidHttpUrl(redirect)) {
+          window.location.href = redirect
+        } else {
+          await router.replace({ path: redirect })
+        }
+      } else if (responseData.load_terms) {
+        window.location.href = responseData.redirect
       } else {
         await router.replace({ name: "Home" })
       }
+
+      return { success: true }
     } catch (error) {
       const errorMessage = error.response?.data?.error || "An error occurred during login."
       showErrorNotification(errorMessage)
+      return { success: false, error: errorMessage }
     } finally {
       isLoading.value = false
     }
@@ -84,5 +86,6 @@ export function useLogin() {
     isLoading,
     performLogin,
     redirectNotAuthenticated,
+    requires2FA,
   }
 }

src/CoreBundle/Controller/AccountController.php

@@ -20,13 +20,12 @@
 use Endroid\QrCode\Writer\PngWriter;
 use OTPHP\TOTP;
 use Security;
-use Symfony\Component\Form\FormError;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
 use Symfony\Component\Routing\Attribute\Route;
 use Symfony\Component\Security\Core\User\UserInterface;
-use Symfony\Component\Security\Csrf\CsrfToken;
 use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
@@ -92,104 +91,109 @@ public function edit(Request $request, UserRepository $userRepository, Illustrat
     }
 
     #[Route('/change-password', name: 'chamilo_core_account_change_password', methods: ['GET', 'POST'])]
-    public function changePassword(Request $request, UserRepository $userRepository, CsrfTokenManagerInterface $csrfTokenManager): Response
-    {
+    public function changePassword(
+        Request $request,
+        UserRepository $userRepository,
+        CsrfTokenManagerInterface $csrfTokenManager,
+        SettingsManager $settingsManager,
+        UserPasswordHasherInterface $passwordHasher
+    ): Response {
         /** @var User $user */
         $user = $this->getUser();
 
+        // Ensure user is authenticated and has proper interface
         if (!\is_object($user) || !$user instanceof UserInterface) {
             throw $this->createAccessDeniedException('This user does not have access to this section');
         }
 
+        // Build the form and inject user-related options
         $form = $this->createForm(ChangePasswordType::class, [
             'enable2FA' => $user->getMfaEnabled(),
+        ], [
+            'user' => $user,
+            'portal_name' => $settingsManager->getSetting('platform.institution'),
+            'password_hasher' => $passwordHasher,
         ]);
-        $form->handleRequest($request);
 
+        $form->handleRequest($request);
+        $session = $request->getSession();
         $qrCodeBase64 = null;
-        if ($user->getMfaEnabled() && 'TOTP' === $user->getMfaService() && $user->getMfaSecret()) {
-            $decryptedSecret = $this->decryptTOTPSecret($user->getMfaSecret(), $_ENV['APP_SECRET']);
-            $totp = TOTP::create($decryptedSecret);
-            $totp->setLabel($user->getEmail());
+        $showQRCode = false;
+
+        // Generate TOTP secret and QR code for 2FA activation
+        if ($form->get('enable2FA')->getData() && !$user->getMfaSecret()) {
+            if (!$session->has('temporary_mfa_secret')) {
+                $totp = TOTP::create();
+                $secret = $totp->getSecret();
+                $session->set('temporary_mfa_secret', $secret);
+            } else {
+                $secret = $session->get('temporary_mfa_secret');
+            }
+
+            $totp = TOTP::create($secret);
+            $portalName = $settingsManager->getSetting('platform.institution');
+            $totp->setLabel($portalName . ' - ' . $user->getEmail());
 
+            // Build QR code image
             $qrCodeResult = Builder::create()
                 ->writer(new PngWriter())
                 ->data($totp->getProvisioningUri())
                 ->encoding(new Encoding('UTF-8'))
                 ->errorCorrectionLevel(new ErrorCorrectionLevelHigh())
                 ->size(300)
                 ->margin(10)
-                ->build()
-            ;
+                ->build();
 
             $qrCodeBase64 = base64_encode($qrCodeResult->getString());
+            $showQRCode = true;
         }
 
+        // Handle form submission
         if ($form->isSubmitted() && $form->isValid()) {
-            $submittedToken = $request->request->get('_token');
+            $newPassword = $form->get('newPassword')->getData();
+            $enable2FA = $form->get('enable2FA')->getData();
 
-            if (!$csrfTokenManager->isTokenValid(new CsrfToken('change_password', $submittedToken))) {
-                $form->addError(new FormError($this->translator->trans('CSRF token is invalid. Please try again.')));
-            } else {
-                $currentPassword = $form->get('currentPassword')->getData();
-                $newPassword = $form->get('newPassword')->getData();
-                $confirmPassword = $form->get('confirmPassword')->getData();
-                $enable2FA = $form->get('enable2FA')->getData();
-
-                if ($enable2FA && !$user->getMfaSecret()) {
-                    $totp = TOTP::create();
-                    $totp->setLabel($user->getEmail());
-                    $encryptedSecret = $this->encryptTOTPSecret($totp->getSecret(), $_ENV['APP_SECRET']);
-                    $user->setMfaSecret($encryptedSecret);
-                    $user->setMfaEnabled(true);
-                    $user->setMfaService('TOTP');
-                    $userRepository->updateUser($user);
-
-                    $qrCodeResult = Builder::create()
-                        ->writer(new PngWriter())
-                        ->data($totp->getProvisioningUri())
-                        ->encoding(new Encoding('UTF-8'))
-                        ->errorCorrectionLevel(new ErrorCorrectionLevelHigh())
-                        ->size(300)
-                        ->margin(10)
-                        ->build()
-                    ;
-
-                    $qrCodeBase64 = base64_encode($qrCodeResult->getString());
-
-                    return $this->render('@ChamiloCore/Account/change_password.html.twig', [
-                        'form' => $form->createView(),
-                        'qrCode' => $qrCodeBase64,
-                        'user' => $user,
-                    ]);
-                }
-                if (!$enable2FA) {
-                    $user->setMfaEnabled(false);
-                    $user->setMfaSecret(null);
-                    $userRepository->updateUser($user);
-                    $this->addFlash('success', '2FA disabled successfully.');
-                }
+            // Enable 2FA and store encrypted secret
+            if ($enable2FA && !$user->getMfaSecret() && $session->has('temporary_mfa_secret')) {
+                $secret = $session->get('temporary_mfa_secret');
+                $encryptedSecret = $this->encryptTOTPSecret($secret, $_ENV['APP_SECRET']);
 
-                if ($newPassword || $confirmPassword || $currentPassword) {
-                    if (!$userRepository->isPasswordValid($user, $currentPassword)) {
-                        $form->get('currentPassword')->addError(new FormError($this->translator->trans('The current password is incorrect')));
-                    } elseif ($newPassword !== $confirmPassword) {
-                        $form->get('confirmPassword')->addError(new FormError($this->translator->trans('Passwords do not match')));
-                    } else {
-                        $user->setPlainPassword($newPassword);
-                        $userRepository->updateUser($user);
-                        $this->addFlash('success', 'Password updated successfully');
-                    }
-                }
+                $user->setMfaSecret($encryptedSecret);
+                $user->setMfaEnabled(true);
+                $user->setMfaService('TOTP');
+
+                $userRepository->updateUser($user);
+                $session->remove('temporary_mfa_secret');
 
+                $this->addFlash('success', '2FA activated successfully.');
+                return $this->redirectToRoute('chamilo_core_account_home');
+            }
+
+            // Disable 2FA if it was previously enabled
+            if (!$enable2FA && $user->getMfaEnabled()) {
+                $user->setMfaEnabled(false);
+                $user->setMfaSecret(null);
+
+                $userRepository->updateUser($user);
+                $this->addFlash('success', '2FA disabled successfully.');
+                return $this->redirectToRoute('chamilo_core_account_home');
+            }
+
+            // Update password if provided
+            if (!empty($newPassword)) {
+                $user->setPlainPassword($newPassword);
+                $userRepository->updateUser($user);
+                $this->addFlash('success', 'Password updated successfully.');
                 return $this->redirectToRoute('chamilo_core_account_home');
             }
         }
 
+        // Render form with optional QR code for 2FA
         return $this->render('@ChamiloCore/Account/change_password.html.twig', [
             'form' => $form->createView(),
             'qrCode' => $qrCodeBase64,
             'user' => $user,
+            'showQRCode' => $showQRCode,
         ]);
     }
 
@@ -205,6 +209,17 @@ private function encryptTOTPSecret(string $secret, string $encryptionKey): strin
         return base64_encode($iv.'::'.$encryptedSecret);
     }
 
+    /**
+     * Validates the provided TOTP code for the given user.
+     */
+    private function isTOTPValid(User $user, string $totpCode): bool
+    {
+        $decryptedSecret = $this->decryptTOTPSecret($user->getMfaSecret(), $_ENV['APP_SECRET']);
+        $totp = TOTP::create($decryptedSecret);
+
+        return $totp->verify($totpCode);
+    }
+
     /**
      * Decrypts the TOTP secret using AES-256-CBC decryption.
      */

src/CoreBundle/Controller/SecurityController.php

@@ -66,19 +66,21 @@ public function loginJson(Request $request, EntityManager $entityManager, Settin
         }
 
         if ($user->getMfaEnabled()) {
-            $totpCode = null;
             $data = json_decode($request->getContent(), true);
-            if (isset($data['totp'])) {
-                $totpCode = $data['totp'];
+            $totpCode = $data['totp'] ?? null;
+
+            if (null === $totpCode) {
+                $tokenStorage->setToken(null);
+                $request->getSession()->invalidate();
+
+                return $this->json(['requires2FA' => true], 200);
             }
 
-            if (null === $totpCode || !$this->isTOTPValid($user, $totpCode)) {
+            if (!$this->isTOTPValid($user, $totpCode)) {
                 $tokenStorage->setToken(null);
                 $request->getSession()->invalidate();
 
-                return $this->json([
-                    'requires2FA' => true,
-                ], 200);
+                return $this->json(['error' => 'Invalid 2FA code.'], 401);
             }
         }
 

src/CoreBundle/Controller/SocialController.php

@@ -553,11 +553,10 @@ private function getExtraFieldBlock(
                     );
                     if (!empty($extraFieldOptions)) {
                         $optionTexts = array_map(
-                            fn (ExtraFieldOptions $option) => $option['display_text'],
+                            fn (ExtraFieldOptions $option) => $option->getDisplayText(),
                             $extraFieldOptions
                         );
                         $fieldValue = implode(', ', $optionTexts);
-                        $fieldValue = $extraFieldOptions->getDisplayText();
                     }
 
                     break;

src/CoreBundle/Entity/ExtraFieldOptions.php

@@ -95,10 +95,7 @@ public function setValue(string $value): self
         return $this;
     }
 
-    /**
-     * @return string
-     */
-    public function getDisplayText()
+    public function getDisplayText(): ?string
     {
         return $this->displayText;
     }