From f7957461a13dd09412b69fb253d8af2d2cede67f Mon Sep 17 00:00:00 2001 From: Christian Beeznest Date: Thu, 10 Apr 2025 01:57:49 -0500 Subject: [PATCH 1/6] User: Add improvements to 2FA - refs #6162 --- assets/vue/composables/auth/login.js | 4 +- .../Controller/AccountController.php | 87 +++++++++++++++---- .../Controller/SecurityController.php | 16 ++-- src/CoreBundle/Form/ChangePasswordType.php | 5 ++ .../views/Account/change_password.html.twig | 11 ++- 5 files changed, 97 insertions(+), 26 deletions(-) diff --git a/assets/vue/composables/auth/login.js b/assets/vue/composables/auth/login.js index 2a132a02516..1955e734d8d 100644 --- a/assets/vue/composables/auth/login.js +++ b/assets/vue/composables/auth/login.js @@ -32,8 +32,8 @@ export function useLogin() { try { const responseData = await securityService.login(payload) - if (responseData.requires2FA) { - return { success: true, requires2FA: true }; + if (responseData.error) { + return { success: false, requires2FA: true, error: responseData.error } } if (route.query.redirect) { diff --git a/src/CoreBundle/Controller/AccountController.php b/src/CoreBundle/Controller/AccountController.php index 4145966f3df..16fc11cfb99 100644 --- a/src/CoreBundle/Controller/AccountController.php +++ b/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,11 +107,12 @@ public function changePassword(Request $request, UserRepository $userRepository, $form->handleRequest($request); $qrCodeBase64 = null; + $showQRCode = false; if ($user->getMfaEnabled() && 'TOTP' === $user->getMfaService() && $user->getMfaSecret()) { $decryptedSecret = $this->decryptTOTPSecret($user->getMfaSecret(), $_ENV['APP_SECRET']); $totp = TOTP::create($decryptedSecret); - $totp->setLabel($user->getEmail()); - + $portalName = $settingsManager->getSetting('platform.institution'); + $totp->setLabel($user->getEmail() . ' (' . $portalName . ')'); $qrCodeResult = Builder::create() ->writer(new PngWriter()) ->data($totp->getProvisioningUri()) @@ -123,6 +124,7 @@ public function changePassword(Request $request, UserRepository $userRepository, ; $qrCodeBase64 = base64_encode($qrCodeResult->getString()); + $showQRCode = true; } if ($form->isSubmitted() && $form->isValid()) { @@ -136,14 +138,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($user->getEmail() . ' (' . $portalName . ')'); $qrCodeResult = Builder::create() ->writer(new PngWriter()) @@ -156,12 +177,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 +235,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 +251,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. */ diff --git a/src/CoreBundle/Controller/SecurityController.php b/src/CoreBundle/Controller/SecurityController.php index 47816e8854c..917775453df 100644 --- a/src/CoreBundle/Controller/SecurityController.php +++ b/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); } } diff --git a/src/CoreBundle/Form/ChangePasswordType.php b/src/CoreBundle/Form/ChangePasswordType.php index a6facbe6872..80845988206 100644 --- a/src/CoreBundle/Form/ChangePasswordType.php +++ b/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, + ]) ; } diff --git a/src/CoreBundle/Resources/views/Account/change_password.html.twig b/src/CoreBundle/Resources/views/Account/change_password.html.twig index 4cda0f2bc19..c9690820b6d 100644 --- a/src/CoreBundle/Resources/views/Account/change_password.html.twig +++ b/src/CoreBundle/Resources/views/Account/change_password.html.twig @@ -55,6 +55,12 @@ {{ form_errors(form.enable2FA) }} +
+ {{ form_label(form.confirm2FACode) }} + {{ form_widget(form.confirm2FACode, {'attr': {'class': 'shadow appearance-none border rounded w-full py-2 px-3 text-gray-700 leading-tight focus:outline-none focus:shadow-outline'}}) }} + {{ form_errors(form.confirm2FACode) }} +
+
-
+
"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" }) - } } diff --git a/assets/vue/composables/auth/login.js b/assets/vue/composables/auth/login.js index 1955e734d8d..7634b28cd34 100644 --- a/assets/vue/composables/auth/login.js +++ b/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.error) { - return { success: false, requires2FA: true, error: responseData.error } + 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, } } diff --git a/src/CoreBundle/Controller/AccountController.php b/src/CoreBundle/Controller/AccountController.php index a52bc10bb45..83e35eacd1b 100644 --- a/src/CoreBundle/Controller/AccountController.php +++ b/src/CoreBundle/Controller/AccountController.php @@ -108,11 +108,13 @@ public function changePassword(Request $request, UserRepository $userRepository, $qrCodeBase64 = null; $showQRCode = false; - if ($user->getMfaEnabled() && 'TOTP' === $user->getMfaService() && $user->getMfaSecret()) { - $decryptedSecret = $this->decryptTOTPSecret($user->getMfaSecret(), $_ENV['APP_SECRET']); - $totp = TOTP::create($decryptedSecret); + $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()); + $totp->setLabel($portalName . ' - ' . $user->getEmail()); + $qrCodeResult = Builder::create() ->writer(new PngWriter()) ->data($totp->getProvisioningUri()) @@ -120,8 +122,7 @@ public function changePassword(Request $request, UserRepository $userRepository, ->errorCorrectionLevel(new ErrorCorrectionLevelHigh()) ->size(300) ->margin(10) - ->build() - ; + ->build(); $qrCodeBase64 = base64_encode($qrCodeResult->getString()); $showQRCode = true; diff --git a/src/CoreBundle/Resources/views/Account/change_password.html.twig b/src/CoreBundle/Resources/views/Account/change_password.html.twig index 3ca2c7d294d..cecc3836f9d 100644 --- a/src/CoreBundle/Resources/views/Account/change_password.html.twig +++ b/src/CoreBundle/Resources/views/Account/change_password.html.twig @@ -23,8 +23,8 @@ {{ form_label(form.currentPassword) }} {{ form_widget(form.currentPassword, {'attr': {'class': 'shadow appearance-none border rounded w-full py-2 px-3 text-gray-700 leading-tight focus:outline-none focus:shadow-outline', 'id': 'change_password_currentPassword'}}) }} - - + + {{ form_errors(form.currentPassword) }}
@@ -32,8 +32,8 @@ {{ form_label(form.newPassword) }} {{ form_widget(form.newPassword, {'attr': {'class': 'shadow appearance-none border rounded w-full py-2 px-3 text-gray-700 leading-tight focus:outline-none focus:shadow-outline', 'id': 'change_password_newPassword'}}) }} - - + +
{{ form_errors(form.newPassword) }} @@ -44,18 +44,18 @@ {{ form_label(form.confirmPassword) }} {{ form_widget(form.confirmPassword, {'attr': {'class': 'shadow appearance-none border rounded w-full py-2 px-3 text-gray-700 leading-tight focus:outline-none focus:shadow-outline', 'id': 'change_password_confirmPassword'}}) }} - - + + {{ form_errors(form.confirmPassword) }}
{{ form_label(form.enable2FA) }} - {{ form_widget(form.enable2FA, {'attr': {'class': 'form-checkbox'}}) }} + {{ form_widget(form.enable2FA, {'attr': {'class': 'form-checkbox', 'id': 'change_password_enable2FA'}}) }} {{ form_errors(form.enable2FA) }}
-
+