User: Add improvements to 2FA - refs #6162

assets/vue/components/Login.vue

@@ -28,7 +28,10 @@
         />
       </div>
 
-      <div v-if="requires2FA" class="field">
+      <div
+        v-if="requires2FA"
+        class="field"
+      >
         <InputText
           v-model="totp"
           :placeholder="t('Enter 2FA code')"
@@ -90,18 +93,19 @@ 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()
 
@@ -112,11 +116,5 @@ async function onSubmitLoginForm() {
     totp: requires2FA.value ? totp.value : null,
     _remember_me: remember.value,
   })
-
-  if (response.requires2FA) {
-    requires2FA.value = true
-  } else {
-    router.replace({ name: "Home" })
-  }
 }
 </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,
   }
 }

public/main/auth/inscription.php

@@ -119,9 +119,19 @@
 $htmlHeadXtra[] = api_get_password_checker_js('#username', '#pass1');
 // User is not allowed if Terms and Conditions are disabled and
 // registration is disabled too.
-$isNotAllowedHere = ('false' === api_get_setting('allow_terms_conditions') && 'false' === api_get_setting('allow_registration'));
-if ($isNotAllowedHere) {
-    api_not_allowed(true, get_lang('Sorry, you are trying to access the registration page for this portal, but registration is currently disabled. Please contact the administrator (see contact information in the footer). If you already have an account on this site.'));
+$isCreatingIntroPage = isset($_GET['create_intro_page']);
+$isPlatformAdmin = api_is_platform_admin();
+
+$isNotAllowedHere = (
+    'false' === api_get_setting('allow_terms_conditions') &&
+    'false' === api_get_setting('allow_registration')
+);
+
+if ($isNotAllowedHere && !($isCreatingIntroPage && $isPlatformAdmin)) {
+    api_not_allowed(
+        true,
+        get_lang('Sorry, you are trying to access the registration page for this portal, but registration is currently disabled. Please contact the administrator (see contact information in the footer). If you already have an account on this site.')
+    );
 }
 
 $settingConditions = api_get_setting('profile.show_conditions_to_user', true);
@@ -633,16 +643,6 @@
     $toolName = get_lang('Terms and Conditions');
 }
 
-// Forbidden to self-register
-if ($isNotAllowedHere) {
-    api_not_allowed(
-        true,
-        get_lang(
-            'Sorry, you are trying to access the registration page for this portal, but registration is currently disabled. Please contact the administrator (see contact information in the footer). If you already have an account on this site.'
-        )
-    );
-}
-
 if ('approval' === api_get_setting('allow_registration')) {
     $content .= Display::return_message(get_lang('Your account has to be approved'));
 }
@@ -658,7 +658,7 @@
 // Terms and conditions
 $infoMessage = '';
 if ('true' === api_get_setting('allow_terms_conditions')) {
-    if (!api_is_platform_admin()) {
+    if (!$isPlatformAdmin) {
         if ('true' === api_get_setting('ticket.show_terms_if_profile_completed')) {
             $userId = api_get_user_id();
             if (empty($userId) && isset($termRegistered['user_id'])) {
@@ -1381,7 +1381,7 @@
                 . '</div>' . $content;
         }
 
-        if (isset($_GET['create_intro_page']) && api_is_platform_admin()) {
+        if ($isCreatingIntroPage && $isPlatformAdmin) {
             $user = api_get_user_entity();
 
             if ($introPage) {

src/CoreBundle/Controller/AccountController.php

@@ -92,7 +92,7 @@ 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): Response
     {
         /** @var User $user */
         $user = $this->getUser();
@@ -107,10 +107,13 @@ public function changePassword(Request $request, UserRepository $userRepository,
         $form->handleRequest($request);
 
         $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;
+        $session = $request->getSession();
+        if ($form->get('enable2FA')->getData() && !$user->getMfaSecret() && $session->has('temporary_mfa_secret')) {
+            $secret = $session->get('temporary_mfa_secret');
+            $totp = TOTP::create($secret);
+            $portalName = $settingsManager->getSetting('platform.institution');
+            $totp->setLabel($portalName . ' - ' . $user->getEmail());
 
             $qrCodeResult = Builder::create()
                 ->writer(new PngWriter())
@@ -119,10 +122,10 @@ public function changePassword(Request $request, UserRepository $userRepository,
                 ->errorCorrectionLevel(new ErrorCorrectionLevelHigh())
                 ->size(300)
                 ->margin(10)
-                ->build()
-            ;
+                ->build();
 
             $qrCodeBase64 = base64_encode($qrCodeResult->getString());
+            $showQRCode = true;
         }
 
         if ($form->isSubmitted() && $form->isValid()) {
@@ -136,14 +139,33 @@ public function changePassword(Request $request, UserRepository $userRepository,
                 $confirmPassword = $form->get('confirmPassword')->getData();
                 $enable2FA = $form->get('enable2FA')->getData();
 
+                if ($user->getMfaEnabled()) {
+                    $enteredCode = $form->get('confirm2FACode')->getData();
+                    if (empty($enteredCode) || !$this->isTOTPValid($user, $enteredCode)) {
+                        $form->get('confirm2FACode')->addError(new FormError('The 2FA code is invalid.'));
+                        return $this->render('@ChamiloCore/Account/change_password.html.twig', [
+                            'form' => $form->createView(),
+                            'qrCode' => $qrCodeBase64,
+                            'user' => $user,
+                            'showQRCode' => $qrCodeBase64 !== null || ($form->isSubmitted() && $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);
+                    $session = $request->getSession();
+
+                    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());
 
                     $qrCodeResult = Builder::create()
                         ->writer(new PngWriter())
@@ -156,12 +178,36 @@ public function changePassword(Request $request, UserRepository $userRepository,
                     ;
 
                     $qrCodeBase64 = base64_encode($qrCodeResult->getString());
+                    $enteredCode = $form->get('confirm2FACode')->getData();
+
+                    if (!$enteredCode) {
+                        return $this->render('@ChamiloCore/Account/change_password.html.twig', [
+                            'form' => $form->createView(),
+                            'qrCode' => $qrCodeBase64,
+                            'user' => $user,
+                            'showQRCode' => true,
+                        ]);
+                    }
 
-                    return $this->render('@ChamiloCore/Account/change_password.html.twig', [
-                        'form' => $form->createView(),
-                        'qrCode' => $qrCodeBase64,
-                        'user' => $user,
-                    ]);
+                    if (!$totp->verify($enteredCode)) {
+                        $form->get('confirm2FACode')->addError(new FormError('The code is incorrect.'));
+                        return $this->render('@ChamiloCore/Account/change_password.html.twig', [
+                            'form' => $form->createView(),
+                            'qrCode' => $qrCodeBase64,
+                            'user' => $user,
+                            'showQRCode' => true,
+                        ]);
+                    }
+
+                    $encryptedSecret = $this->encryptTOTPSecret($secret, $_ENV['APP_SECRET']);
+                    $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');
                 }
                 if (!$enable2FA) {
                     $user->setMfaEnabled(false);
@@ -190,6 +236,7 @@ public function changePassword(Request $request, UserRepository $userRepository,
             'form' => $form->createView(),
             'qrCode' => $qrCodeBase64,
             'user' => $user,
+            'showQRCode' => $qrCodeBase64 !== null || ($form->isSubmitted() && $form->get('enable2FA')->getData()),
         ]);
     }
 
@@ -205,6 +252,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/Form/ChangePasswordType.php

@@ -9,6 +9,7 @@
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\CheckboxType;
 use Symfony\Component\Form\Extension\Core\Type\PasswordType;
+use Symfony\Component\Form\Extension\Core\Type\TextType;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 
@@ -38,6 +39,10 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
                 'label' => 'Enable two-factor authentication (2FA)',
                 'required' => false,
             ])
+            ->add('confirm2FACode', TextType::class, [
+                'label' => 'Enter your 2FA code',
+                'required' => false,
+            ])
         ;
     }