From 52b82613e15a17f7f9318c0fd46492a92bf97e7f Mon Sep 17 00:00:00 2001 From: Carlos Monastyrski Date: Fri, 26 Sep 2025 14:31:06 -0300 Subject: [PATCH 1/5] Improve personal settings 2FA form --- frontend/src/hooks/api/users/queries.tsx | 5 +- .../components/SecuritySection/MFASection.tsx | 507 ++++++++++++++---- 2 files changed, 398 insertions(+), 114 deletions(-) diff --git a/frontend/src/hooks/api/users/queries.tsx b/frontend/src/hooks/api/users/queries.tsx index 2c0125361..22d4f39d8 100644 --- a/frontend/src/hooks/api/users/queries.tsx +++ b/frontend/src/hooks/api/users/queries.tsx @@ -508,7 +508,7 @@ export const useListUserGroupMemberships = (username: string) => { }); }; -export const useGetUserTotpRegistration = () => { +export const useGetUserTotpRegistration = (options?: { enabled?: boolean }) => { return useQuery({ queryKey: userKeys.totpRegistration, queryFn: async () => { @@ -517,7 +517,8 @@ export const useGetUserTotpRegistration = () => { ); return data; - } + }, + enabled: options?.enabled ?? true }); }; diff --git a/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx b/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx index 25275edc2..a23d3e8f9 100644 --- a/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx +++ b/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx @@ -1,6 +1,8 @@ +import { useEffect, useState } from "react"; import { useQueryClient } from "@tanstack/react-query"; +import QRCode from "qrcode"; -import TotpRegistration from "@app/components/mfa/TotpRegistration"; +import { RecoveryCodesDownload } from "@app/components/mfa/RecoveryCodesDownload"; import { createNotification } from "@app/components/notifications"; import { Button, @@ -8,9 +10,9 @@ import { DeleteActionModal, EmailServiceSetupModal, FormControl, + Input, Select, - SelectItem, - Switch + SelectItem } from "@app/components/v2"; import { useToggle } from "@app/hooks"; import { useGetUser, userKeys, useUpdateUserMfa } from "@app/hooks/api"; @@ -18,9 +20,13 @@ import { MfaMethod } from "@app/hooks/api/auth/types"; import { useFetchServerStatus } from "@app/hooks/api/serverDetails"; import { useCreateNewTotpRecoveryCodes, - useDeleteUserTotpConfiguration + useDeleteUserTotpConfiguration, + useVerifyUserTotpRegistration } from "@app/hooks/api/users/mutation"; -import { useGetUserTotpConfiguration } from "@app/hooks/api/users/queries"; +import { + useGetUserTotpConfiguration, + useGetUserTotpRegistration +} from "@app/hooks/api/users/queries"; import { AuthMethod } from "@app/hooks/api/users/types"; import { usePopUp } from "@app/hooks/usePopUp"; @@ -28,24 +34,65 @@ export const MFASection = () => { const { data: user } = useGetUser(); const { mutateAsync } = useUpdateUserMfa(); + const [formData, setFormData] = useState({ + isMfaEnabled: user?.isMfaEnabled || false, + selectedMfaMethod: user?.selectedMfaMethod || MfaMethod.EMAIL + }); + const [isLoading, setIsLoading] = useState(false); + const [totpCode, setTotpCode] = useState(""); + const [qrCodeUrl, setQrCodeUrl] = useState(""); + const [showMobileAuthSetup, setShowMobileAuthSetup] = useState(false); + const { handlePopUpToggle, popUp, handlePopUpOpen, handlePopUpClose } = usePopUp([ "setUpEmail", - "deleteTotpConfig" + "deleteTotpConfig", + "downloadRecoveryCodes" ] as const); const [shouldShowRecoveryCodes, setShouldShowRecoveryCodes] = useToggle(); - const { data: totpConfiguration, isPending: isTotpConfigurationLoading } = - useGetUserTotpConfiguration(); + const { data: totpConfiguration } = useGetUserTotpConfiguration(); + const { data: totpRegistration, isPending: isTotpRegistrationLoading } = + useGetUserTotpRegistration({ + enabled: showMobileAuthSetup + }); const { mutateAsync: deleteTotpConfiguration } = useDeleteUserTotpConfiguration(); const { mutateAsync: createTotpRecoveryCodes } = useCreateNewTotpRecoveryCodes(); + const { mutateAsync: verifyUserTotp } = useVerifyUserTotpRegistration(); const queryClient = useQueryClient(); const { data: serverDetails } = useFetchServerStatus(); + // Update form data when user data changes + useEffect(() => { + if (user) { + setFormData({ + isMfaEnabled: user.isMfaEnabled, + selectedMfaMethod: user.selectedMfaMethod || MfaMethod.EMAIL + }); + } + }, [user]); + + useEffect(() => { + const generateQRCode = async () => { + if (totpRegistration?.otpUrl) { + const url = await QRCode.toDataURL(totpRegistration.otpUrl); + setQrCodeUrl(url); + } + }; + + if (showMobileAuthSetup && totpRegistration?.otpUrl) { + generateQRCode(); + } + }, [totpRegistration, showMobileAuthSetup]); + const handleTotpDeletion = async () => { try { await deleteTotpConfiguration(); + await mutateAsync({ + selectedMfaMethod: MfaMethod.EMAIL + }); + createNotification({ - text: "Successfully deleted mobile authenticator", + text: "Successfully deleted mobile authenticator and switched to email authentication", type: "success" }); @@ -82,30 +129,35 @@ export const MFASection = () => { } }; - const updateSelectedMfa = async (mfaMethod: MfaMethod) => { - try { - if (!user) return; + const handleFormDataChange = (field: string, value: any) => { + setFormData((prev) => ({ + ...prev, + [field]: value + })); - await mutateAsync({ - selectedMfaMethod: mfaMethod - }); - - createNotification({ - text: "Successfully updated selected 2FA method", - type: "success" - }); - } catch (err) { - createNotification({ - text: "Something went wrong while updating selected 2FA method.", - type: "error" - }); - console.error(err); + // Show mobile auth setup when mobile authenticator is selected and we're enabling 2FA + if (field === "selectedMfaMethod" && value === MfaMethod.TOTP && formData.isMfaEnabled) { + setShowMobileAuthSetup(true); + } else if (field === "selectedMfaMethod" && value !== MfaMethod.TOTP) { + setShowMobileAuthSetup(false); + setTotpCode(""); + setShouldShowRecoveryCodes.off(); + if (totpConfiguration?.isVerified) { + deleteTotpConfiguration().catch(console.error); + } + } else if (field === "isMfaEnabled" && value && formData.selectedMfaMethod === MfaMethod.TOTP) { + setShowMobileAuthSetup(true); + } else if (field === "isMfaEnabled" && !value) { + setShowMobileAuthSetup(false); + setTotpCode(""); + setShouldShowRecoveryCodes.off(); } }; - const toggleMfa = async (state: boolean) => { + const handleSaveChanges = async () => { try { if (!user) return; + if (user.authMethods.includes(AuthMethod.LDAP)) { createNotification({ text: "Two-factor authentication is not available for LDAP users.", @@ -114,119 +166,350 @@ export const MFASection = () => { return; } - const newUser = await mutateAsync({ - isMfaEnabled: state - }); + if (!serverDetails?.emailConfigured && formData.isMfaEnabled) { + handlePopUpOpen("setUpEmail"); + return; + } - createNotification({ - text: `${ - newUser.isMfaEnabled - ? "Successfully turned on two-factor authentication." - : "Successfully turned off two-factor authentication." - }`, - type: "success" - }); + setIsLoading(true); + + // If enabling 2FA with mobile authenticator, verify TOTP first + if ( + formData.isMfaEnabled && + formData.selectedMfaMethod === MfaMethod.TOTP && + !totpConfiguration?.isVerified + ) { + if (!totpCode.trim()) { + createNotification({ + text: "Please enter the verification code from your authenticator app", + type: "error" + }); + setIsLoading(false); + return; + } + + try { + await verifyUserTotp({ totp: totpCode }); + + handlePopUpOpen("downloadRecoveryCodes"); + + createNotification({ + text: "Successfully configured mobile authenticator. Please save your recovery codes!", + type: "success" + }); + + await queryClient.invalidateQueries({ queryKey: userKeys.totpConfiguration }); + } catch { + createNotification({ + text: "Failed to verify TOTP code. Please try again.", + type: "error" + }); + setIsLoading(false); + return; + } + } + + // If disabling 2FA and there's a TOTP configuration, delete it + if (!formData.isMfaEnabled && user.isMfaEnabled && totpConfiguration?.isVerified) { + try { + await deleteTotpConfiguration(); + createNotification({ + text: "Mobile authenticator removed", + type: "success" + }); + + // Refresh TOTP configuration + await queryClient.invalidateQueries({ queryKey: userKeys.totpConfiguration }); + } catch { + // Continue with disabling 2FA even if TOTP deletion fails + } + } + + const updates: any = {}; + + // Only update if values have changed + if (formData.isMfaEnabled !== user.isMfaEnabled) { + updates.isMfaEnabled = formData.isMfaEnabled; + } + + if (formData.selectedMfaMethod !== user.selectedMfaMethod) { + updates.selectedMfaMethod = formData.selectedMfaMethod; + } + + if (Object.keys(updates).length > 0) { + await mutateAsync(updates); + + createNotification({ + text: "Successfully updated two-factor authentication settings", + type: "success" + }); + } + + // Reset form state + setShowMobileAuthSetup(false); + setTotpCode(""); + setShouldShowRecoveryCodes.off(); } catch (err) { createNotification({ - text: "Something went wrong while toggling the two-factor authentication.", + text: "Something went wrong while updating two-factor authentication settings.", type: "error" }); console.error(err); + } finally { + setIsLoading(false); } }; + const hasChanges = + user && + (formData.isMfaEnabled !== user.isMfaEnabled || + formData.selectedMfaMethod !== user.selectedMfaMethod); + + const isFormValid = () => { + if (!formData.isMfaEnabled) return true; + if (formData.selectedMfaMethod === MfaMethod.EMAIL) return true; + if (formData.selectedMfaMethod === MfaMethod.TOTP) { + if (totpConfiguration?.isVerified) return true; + return totpCode.trim().length > 0; + } + return false; + }; + return ( <> -
-

Two-factor Authentication

+
{ + e.preventDefault(); + handleSaveChanges(); + }} + className="mb-6 rounded-lg border border-mineshaft-600 bg-mineshaft-900 p-4" + > +

Two-factor Authentication

+ {user && ( - { - if (serverDetails?.emailConfigured) { - toggleMfa(state as boolean); - } else { - handlePopUpOpen("setUpEmail"); - } - }} - > - Enable 2-factor authentication - - )} - {user?.isMfaEnabled && ( - - - - )} -
Mobile Authenticator
- {isTotpConfigurationLoading ? ( - - ) : ( -
- {totpConfiguration?.isVerified ? ( -
-
- - - -
- {shouldShowRecoveryCodes && totpConfiguration.recoveryCodes && ( -
- {totpConfiguration.recoveryCodes.map((code) => ( -
{code}
- ))} -
- )} +
+
+ + + +
+ + {formData.isMfaEnabled && ( +
+ + +
- ) : ( - <> -
- For added security, you can configure a mobile authenticator and set it as your - selected 2FA method. + )} + + {showMobileAuthSetup && !totpConfiguration?.isVerified && ( +
+

+ Setup Mobile Authenticator +

+ +
+

+ Step 1: Scan QR Code +

+

+ Download a two-factor authentication app (Google Authenticator, Authy, etc.) and + scan the QR code below +

+ +
+ {isTotpRegistrationLoading && ( +
+ +
+ )} + {!isTotpRegistrationLoading && qrCodeUrl && ( +
+
+
+ QR Code for mobile authenticator setup +
+
+ {totpRegistration?.otpUrl && ( +
+

+ Can't scan? Enter this code manually:{" "} + + {totpRegistration.otpUrl.split("secret=")[1]?.split("&")[0] || + "Loading..."} + +

+
+ )} +
+ )} +
-
- { - await queryClient.invalidateQueries({ queryKey: userKeys.totpConfiguration }); - }} - /> + +
+

+ Step 2: Enter verification code +

+

+ Enter the 6-digit code from your authenticator app to complete setup +

+ +
+ + { + const value = e.target.value.replace(/\D/g, "").slice(0, 6); + setTotpCode(value); + }} + onPaste={(e) => { + e.preventDefault(); + const pastedData = e.clipboardData + .getData("text") + .replace(/\D/g, "") + .slice(0, 6); + setTotpCode(pastedData); + }} + placeholder="Enter 2FA code" + className="font-mono tracking-wider" + maxLength={6} + /> + +
- +
+ )} + + {hasChanges && ( +
+ + +
+ )} + + {user?.isMfaEnabled && totpConfiguration?.isVerified && ( +
+

+ Mobile Authenticator Management +

+ +
+
+ + + +
+ + {shouldShowRecoveryCodes && ( +
+ {totpConfiguration.recoveryCodes.map((code) => ( + + {code} + + ))} +
+ )} +
+
)}
)} -
+ + handlePopUpToggle("setUpEmail", isOpen)} /> handlePopUpToggle("deleteTotpConfig", isOpen)} deleteKey="confirm" onDeleteApproved={handleTotpDeletion} /> + + handlePopUpClose("downloadRecoveryCodes")} + recoveryCodes={totpRegistration?.recoveryCodes || []} + onDownloadComplete={() => handlePopUpClose("downloadRecoveryCodes")} + /> ); }; From f48a7d313defda2159ad63a075d7d3e4cd3eab65 Mon Sep 17 00:00:00 2001 From: Carlos Monastyrski Date: Fri, 26 Sep 2025 14:40:43 -0300 Subject: [PATCH 2/5] Fix missing await --- .../components/SecuritySection/MFASection.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx b/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx index a23d3e8f9..492e30ac8 100644 --- a/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx +++ b/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx @@ -143,7 +143,7 @@ export const MFASection = () => { setTotpCode(""); setShouldShowRecoveryCodes.off(); if (totpConfiguration?.isVerified) { - deleteTotpConfiguration().catch(console.error); + await deleteTotpConfiguration().catch(console.error); } } else if (field === "isMfaEnabled" && value && formData.selectedMfaMethod === MfaMethod.TOTP) { setShowMobileAuthSetup(true); From 890ab4d80664bf35f0ed45bf96259af7dedc574c Mon Sep 17 00:00:00 2001 From: Carlos Monastyrski Date: Fri, 26 Sep 2025 15:27:14 -0300 Subject: [PATCH 3/5] Fix type error --- .../components/SecuritySection/MFASection.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx b/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx index 492e30ac8..b3bbab779 100644 --- a/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx +++ b/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx @@ -129,7 +129,7 @@ export const MFASection = () => { } }; - const handleFormDataChange = (field: string, value: any) => { + const handleFormDataChange = async (field: string, value: any) => { setFormData((prev) => ({ ...prev, [field]: value From 5beb347fd2a60132815f486ae9dc75e67055091f Mon Sep 17 00:00:00 2001 From: Scott Wilson Date: Fri, 26 Sep 2025 15:22:47 -0700 Subject: [PATCH 4/5] improvement: minor design nits --- .../components/SecuritySection/MFASection.tsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx b/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx index b3bbab779..008271f29 100644 --- a/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx +++ b/frontend/src/pages/user/PersonalSettingsPage/components/SecuritySection/MFASection.tsx @@ -288,10 +288,10 @@ export const MFASection = () => { {user && (
-
+