diff --git a/src/components/common/ConnectWallet/PasswordRecovery.tsx b/src/components/common/ConnectWallet/PasswordRecovery.tsx index 3bc8d5ba5a..17c8fb6060 100644 --- a/src/components/common/ConnectWallet/PasswordRecovery.tsx +++ b/src/components/common/ConnectWallet/PasswordRecovery.tsx @@ -26,7 +26,7 @@ export const PasswordRecovery = ({ This browser is not registered with your Account yet. Please enter your recovery password to restore access - to this account. + to this Account. ) => { const [pendingSafe] = usePendingSafe() const wallet = useWallet() - const chain = useCurrentChain() - const isSupported = isPairingSupported(chain?.disabledWallets) const handleConnect = useConnectWallet() const [, setSubmitted] = useState(false) useSyncSafeCreationStep(setStep) @@ -37,33 +31,21 @@ const ConnectWalletStep = ({ onSubmit, setStep }: StepRenderProps - - - - - + + + + - + - - or - + + or + - - - - {isSupported && ( - - - - Connect to {'Safe{Wallet}'} mobile - - - - )} - + + ) diff --git a/src/components/settings/SignerAccountMFA/PasswordForm.tsx b/src/components/settings/SignerAccountMFA/PasswordForm.tsx new file mode 100644 index 0000000000..74b3d17616 --- /dev/null +++ b/src/components/settings/SignerAccountMFA/PasswordForm.tsx @@ -0,0 +1,124 @@ +import { DeviceShareRecovery } from '@/hooks/wallets/mpc/recovery/DeviceShareRecovery' +import { SecurityQuestionRecovery } from '@/hooks/wallets/mpc/recovery/SecurityQuestionRecovery' +import { Typography, TextField, FormControlLabel, Checkbox, Button, Box } from '@mui/material' +import { type Web3AuthMPCCoreKit } from '@web3auth/mpc-core-kit' +import { useState, useMemo } from 'react' +import { Controller, useForm } from 'react-hook-form' +import { enableMFA } from './helper' + +enum PasswordFieldNames { + oldPassword = 'oldPassword', + newPassword = 'newPassword', + confirmPassword = 'confirmPassword', + storeDeviceShare = 'storeDeviceShare', +} + +type PasswordFormData = { + [PasswordFieldNames.oldPassword]: string | undefined + [PasswordFieldNames.newPassword]: string + [PasswordFieldNames.confirmPassword]: string + [PasswordFieldNames.storeDeviceShare]: boolean +} + +export const PasswordForm = ({ mpcCoreKit }: { mpcCoreKit: Web3AuthMPCCoreKit }) => { + const formMethods = useForm({ + mode: 'all', + defaultValues: async () => { + const isDeviceShareStored = await new DeviceShareRecovery(mpcCoreKit).isEnabled() + return { + confirmPassword: '', + oldPassword: undefined, + newPassword: '', + storeDeviceShare: isDeviceShareStored, + } + }, + }) + + const { register, formState, getValues, control, handleSubmit } = formMethods + + const [enablingMFA, setEnablingMFA] = useState(false) + + const isPasswordSet = useMemo(() => { + const securityQuestions = new SecurityQuestionRecovery(mpcCoreKit) + return securityQuestions.isEnabled() + }, [mpcCoreKit]) + + const onSubmit = async (data: PasswordFormData) => { + setEnablingMFA(true) + try { + await enableMFA(mpcCoreKit, data) + } finally { + setEnablingMFA(false) + } + } + + return ( +
+ + {isPasswordSet ? ( + You already have a recovery password setup. + ) : ( + You have no password setup. We suggest adding one to secure your Account. + )} + {isPasswordSet && ( + + )} + + { + const currentNewPW = getValues(PasswordFieldNames.newPassword) + if (value !== currentNewPW) { + return 'Passwords do not match' + } + }, + })} + /> + + ( + } + label="Do not ask for second factor on this device" + /> + )} + /> + + + +
+ ) +} diff --git a/src/components/settings/SignerAccountMFA/helper.ts b/src/components/settings/SignerAccountMFA/helper.ts new file mode 100644 index 0000000000..84595ab857 --- /dev/null +++ b/src/components/settings/SignerAccountMFA/helper.ts @@ -0,0 +1,68 @@ +import { DeviceShareRecovery } from '@/hooks/wallets/mpc/recovery/DeviceShareRecovery' +import { SecurityQuestionRecovery } from '@/hooks/wallets/mpc/recovery/SecurityQuestionRecovery' +import { logError } from '@/services/exceptions' +import ErrorCodes from '@/services/exceptions/ErrorCodes' +import { asError } from '@/services/exceptions/utils' +import { getPubKeyPoint } from '@tkey-mpc/common-types' +import { type Web3AuthMPCCoreKit } from '@web3auth/mpc-core-kit' +import BN from 'bn.js' + +export const isMFAEnabled = (mpcCoreKit: Web3AuthMPCCoreKit) => { + if (!mpcCoreKit) { + return false + } + const { shareDescriptions } = mpcCoreKit?.getKeyDetails() + return !Object.entries(shareDescriptions).some((value) => value[0]?.includes('hashedShare')) +} + +export const enableMFA = async ( + mpcCoreKit: Web3AuthMPCCoreKit, + { + newPassword, + oldPassword, + storeDeviceShare, + }: { + newPassword: string + oldPassword: string | undefined + storeDeviceShare: boolean + }, +) => { + if (!mpcCoreKit) { + return + } + const securityQuestions = new SecurityQuestionRecovery(mpcCoreKit) + const deviceShareRecovery = new DeviceShareRecovery(mpcCoreKit) + try { + // 1. setup device factor with password recovery + await securityQuestions.upsertPassword(newPassword, oldPassword) + const securityQuestionFactor = await securityQuestions.recoverWithPassword(newPassword) + if (!securityQuestionFactor) { + throw Error('Could not recover using the new password recovery') + } + + if (!isMFAEnabled(mpcCoreKit)) { + // 2. enable MFA in mpcCoreKit + const recoveryFactor = await mpcCoreKit.enableMFA({}) + + // 3. remove the recovery factor the mpcCoreKit creates + const recoverKey = new BN(recoveryFactor, 'hex') + const recoverPubKey = getPubKeyPoint(recoverKey) + await mpcCoreKit.deleteFactor(recoverPubKey, recoverKey) + } + + const hasDeviceShare = await deviceShareRecovery.isEnabled() + + if (!hasDeviceShare && storeDeviceShare) { + await deviceShareRecovery.createAndStoreDeviceFactor() + } + + if (hasDeviceShare && !storeDeviceShare) { + // Switch to password recovery factor such that we can delete the device factor + await mpcCoreKit.inputFactorKey(new BN(securityQuestionFactor, 'hex')) + await deviceShareRecovery.removeDeviceFactor() + } + } catch (e) { + const error = asError(e) + logError(ErrorCodes._304, error.message) + } +} diff --git a/src/components/settings/SignerAccountMFA/index.tsx b/src/components/settings/SignerAccountMFA/index.tsx index ea3725fea2..b3918f7c65 100644 --- a/src/components/settings/SignerAccountMFA/index.tsx +++ b/src/components/settings/SignerAccountMFA/index.tsx @@ -1,91 +1,11 @@ import useMPC from '@/hooks/wallets/mpc/useMPC' -import { Box, Button, Checkbox, FormControlLabel, TextField, Typography } from '@mui/material' +import { Box, Typography } from '@mui/material' import { COREKIT_STATUS } from '@web3auth/mpc-core-kit' -import { getPubKeyPoint } from '@tkey-mpc/common-types' -import { BN } from 'bn.js' -import { useEffect, useMemo, useState } from 'react' -import { SecurityQuestionRecovery } from '@/hooks/wallets/mpc/recovery/SecurityQuestionRecovery' -import useMFASettings from './useMFASettings' -import { useForm } from 'react-hook-form' -import { DeviceShareRecovery } from '@/hooks/wallets/mpc/recovery/DeviceShareRecovery' -type SignerAccountFormData = { - oldPassword: string | undefined - newPassword: string - confirmPassword: string - storeDeviceShare: boolean -} +import { PasswordForm } from './PasswordForm' const SignerAccountMFA = () => { const mpcCoreKit = useMPC() - const mfaSettings = useMFASettings(mpcCoreKit) - - const formMethods = useForm({ - mode: 'all', - }) - - const { register, formState, watch, setValue, handleSubmit } = formMethods - - const [enablingMFA, setEnablingMFA] = useState(false) - - const isPasswordSet = useMemo(() => { - if (!mpcCoreKit) { - return false - } - const securityQuestions = new SecurityQuestionRecovery(mpcCoreKit) - return securityQuestions.isEnabled() - }, [mpcCoreKit]) - - useEffect(() => { - if (!mpcCoreKit) { - return - } - new DeviceShareRecovery(mpcCoreKit).isEnabled().then((value) => setValue('storeDeviceShare', value)) - }, [mpcCoreKit, setValue]) - - const enableMFA = async () => { - if (!mpcCoreKit) { - return - } - const securityQuestions = new SecurityQuestionRecovery(mpcCoreKit) - const deviceShareRecovery = new DeviceShareRecovery(mpcCoreKit) - setEnablingMFA(true) - try { - const { newPassword, oldPassword, storeDeviceShare } = formMethods.getValues() - // 1. setup device factor with password recovery - await securityQuestions.upsertPassword(newPassword, oldPassword) - const securityQuestionFactor = await securityQuestions.recoverWithPassword(newPassword) - if (!securityQuestionFactor) { - throw Error('Could not recover using the new password recovery') - } - - if (!mfaSettings?.mfaEnabled) { - // 2. enable MFA in mpcCoreKit - const recoveryFactor = await mpcCoreKit.enableMFA({}) - - // 3. remove the recovery factor the mpcCoreKit creates - const recoverKey = new BN(recoveryFactor, 'hex') - const recoverPubKey = getPubKeyPoint(recoverKey) - await mpcCoreKit.deleteFactor(recoverPubKey, recoverKey) - } - - const hasDeviceShare = await deviceShareRecovery.isEnabled() - - if (!hasDeviceShare && storeDeviceShare) { - await deviceShareRecovery.createAndStoreDeviceFactor() - } - - if (hasDeviceShare && !storeDeviceShare) { - // Switch to password recovery factor such that we can delete the device factor - await mpcCoreKit.inputFactorKey(new BN(securityQuestionFactor, 'hex')) - await deviceShareRecovery.removeDeviceFactor() - } - } catch (error) { - console.error(error) - } finally { - setEnablingMFA(false) - } - } if (mpcCoreKit?.status !== COREKIT_STATUS.LOGGED_IN) { return ( @@ -95,73 +15,7 @@ const SignerAccountMFA = () => { ) } - const onSubmit = async () => { - await enableMFA() - } - - return ( -
- - {isPasswordSet ? ( - You already have a recovery password setup. - ) : ( - You have no password setup. Secure your account now! - )} - {isPasswordSet && ( - - )} - - { - const currentNewPW = watch('newPassword') - if (value !== currentNewPW) { - return 'Passwords do not match' - } - }, - })} - /> - - } - label="Do not ask for second factor on this device" - /> - - - -
- ) + return } export default SignerAccountMFA diff --git a/src/components/settings/SignerAccountMFA/useMFASettings.ts b/src/components/settings/SignerAccountMFA/useMFASettings.ts deleted file mode 100644 index 0c56133324..0000000000 --- a/src/components/settings/SignerAccountMFA/useMFASettings.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { COREKIT_STATUS, type Web3AuthMPCCoreKit } from '@web3auth/mpc-core-kit' - -export type MFASettings = { - mfaEnabled: boolean -} | null - -const useMFASettings = (mpcCoreKit: Web3AuthMPCCoreKit | undefined) => { - if (mpcCoreKit?.status !== COREKIT_STATUS.LOGGED_IN) { - return null - } - - const { shareDescriptions } = mpcCoreKit?.getKeyDetails() - - const isMFAEnabled = !Object.entries(shareDescriptions).some(([key, value]) => value[0]?.includes('hashedShare')) - - return { - mfaEnabled: isMFAEnabled, - } -} - -export default useMFASettings diff --git a/src/components/sidebar/SidebarNavigation/config.tsx b/src/components/sidebar/SidebarNavigation/config.tsx index 05eb71effb..b3c940981c 100644 --- a/src/components/sidebar/SidebarNavigation/config.tsx +++ b/src/components/sidebar/SidebarNavigation/config.tsx @@ -105,7 +105,7 @@ export const settingsNavItems = [ href: AppRoutes.settings.environmentVariables, }, { - label: 'Signer Account', + label: 'Signer account', href: AppRoutes.settings.signerAccount, }, ] diff --git a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts index 4f7e8fd951..c6af1f84dd 100644 --- a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts +++ b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts @@ -51,7 +51,7 @@ class MockMPCCoreKit { this.status = this.stateAfterLogin this.state.userInfo = this.userInfoAfterLogin resolve() - }, 1000) + }, MOCK_LOGIN_TIME) }) } @@ -113,13 +113,16 @@ describe('useMPCWallet', () => { result.current.triggerLogin() }) + // While the login resolves we are in Authenticating state expect(result.current.walletState === MPCWalletState.AUTHENTICATING) expect(connectWalletSpy).not.toBeCalled() + // Resolve mock login act(() => { jest.advanceTimersByTime(MOCK_LOGIN_TIME) }) + // We should be logged in and onboard should get connected await waitFor(() => { expect(result.current.walletState === MPCWalletState.READY) expect(connectWalletSpy).toBeCalledWith(expect.anything(), { @@ -155,12 +158,16 @@ describe('useMPCWallet', () => { result.current.triggerLogin() }) + // While the login resolves we are in Authenticating state expect(result.current.walletState === MPCWalletState.AUTHENTICATING) expect(connectWalletSpy).not.toBeCalled() + + // Resolve mock login act(() => { jest.advanceTimersByTime(MOCK_LOGIN_TIME) }) + // We should be logged in and onboard should get connected await waitFor(() => { expect(result.current.walletState === MPCWalletState.READY) expect(connectWalletSpy).toBeCalledWith(expect.anything(), { @@ -195,13 +202,16 @@ describe('useMPCWallet', () => { result.current.triggerLogin() }) + // While the login resolves we are in Authenticating state expect(result.current.walletState === MPCWalletState.AUTHENTICATING) expect(connectWalletSpy).not.toBeCalled() + // Resolve mock login act(() => { jest.advanceTimersByTime(MOCK_LOGIN_TIME) }) + // A missing second factor should result in manual recovery state await waitFor(() => { expect(result.current.walletState === MPCWalletState.MANUAL_RECOVERY) expect(connectWalletSpy).not.toBeCalled() diff --git a/src/hooks/wallets/mpc/recovery/DeviceShareRecovery.ts b/src/hooks/wallets/mpc/recovery/DeviceShareRecovery.ts index afcf78964a..8a080ec626 100644 --- a/src/hooks/wallets/mpc/recovery/DeviceShareRecovery.ts +++ b/src/hooks/wallets/mpc/recovery/DeviceShareRecovery.ts @@ -39,7 +39,6 @@ export class DeviceShareRecovery { const pubKeyX = pubKey.x.toString('hex', 64) await this.mpcCoreKit.deleteFactor(pubKey) const currentStorage = BrowserStorage.getInstance('mpc_corekit_store') - debugger currentStorage.set(pubKeyX, undefined) } } diff --git a/src/hooks/wallets/mpc/recovery/SecurityQuestionRecovery.ts b/src/hooks/wallets/mpc/recovery/SecurityQuestionRecovery.ts index 3224a07aa1..0d707cb29f 100644 --- a/src/hooks/wallets/mpc/recovery/SecurityQuestionRecovery.ts +++ b/src/hooks/wallets/mpc/recovery/SecurityQuestionRecovery.ts @@ -1,8 +1,9 @@ import { TssSecurityQuestion, TssShareType, type Web3AuthMPCCoreKit } from '@web3auth/mpc-core-kit' -const DEFAULT_SECURITY_QUESTION = 'ENTER PASSWORD' - export class SecurityQuestionRecovery { + /** This is only used internally in the metadata store of tKey. Not in the UI */ + private static readonly DEFAULT_SECURITY_QUESTION = 'ENTER PASSWORD' + private mpcCoreKit: Web3AuthMPCCoreKit private securityQuestions = new TssSecurityQuestion() @@ -15,7 +16,6 @@ export class SecurityQuestionRecovery { const question = this.securityQuestions.getQuestion(this.mpcCoreKit) return !!question } catch (error) { - console.error(error) // It errors out if recovery is not setup currently return false } @@ -30,11 +30,11 @@ export class SecurityQuestionRecovery { answer: oldPassword, mpcCoreKit: this.mpcCoreKit, newAnswer: newPassword, - newQuestion: DEFAULT_SECURITY_QUESTION, + newQuestion: SecurityQuestionRecovery.DEFAULT_SECURITY_QUESTION, }) } else { await this.securityQuestions.setSecurityQuestion({ - question: DEFAULT_SECURITY_QUESTION, + question: SecurityQuestionRecovery.DEFAULT_SECURITY_QUESTION, answer: newPassword, mpcCoreKit: this.mpcCoreKit, shareType: TssShareType.DEVICE, diff --git a/src/hooks/wallets/mpc/useMPCWallet.ts b/src/hooks/wallets/mpc/useMPCWallet.ts index 319643c818..9fc69b4d06 100644 --- a/src/hooks/wallets/mpc/useMPCWallet.ts +++ b/src/hooks/wallets/mpc/useMPCWallet.ts @@ -20,7 +20,6 @@ export type MPCWalletHook = { recoverFactorWithPassword: (password: string, storeDeviceShare: boolean) => Promise walletState: MPCWalletState triggerLogin: () => Promise - isMFAEnabled: () => boolean resetAccount: () => Promise userInfo: { email: string | undefined @@ -32,15 +31,6 @@ export const useMPCWallet = (): MPCWalletHook => { const mpcCoreKit = useMPC() const onboard = useOnboard() - const isMFAEnabled = () => { - if (!mpcCoreKit) { - return false - } - const { shareDescriptions } = mpcCoreKit.getKeyDetails() - - return !Object.entries(shareDescriptions).some(([key, value]) => value[0]?.includes('hashedShare')) - } - const criticalResetAccount = async (): Promise => { // This is a critical function that should only be used for testing purposes // Resetting your account means clearing all the metadata associated with it from the metadata server @@ -137,7 +127,6 @@ export const useMPCWallet = (): MPCWalletHook => { return { triggerLogin, walletState, - isMFAEnabled, recoverFactorWithPassword, resetAccount: criticalResetAccount, upsertPasswordBackup: () => Promise.resolve(), diff --git a/src/pages/settings/signer-account.tsx b/src/pages/settings/signer-account.tsx index 4827c4c8bc..3c0f36b91b 100644 --- a/src/pages/settings/signer-account.tsx +++ b/src/pages/settings/signer-account.tsx @@ -10,7 +10,7 @@ const SignerAccountPage: NextPage = () => { return ( <> - {'Safe{Wallet} – Settings – Signer Account'} + {'Safe{Wallet} – Settings – Signer account'} diff --git a/src/services/exceptions/ErrorCodes.ts b/src/services/exceptions/ErrorCodes.ts index 22fe117513..f9a6986dc7 100644 --- a/src/services/exceptions/ErrorCodes.ts +++ b/src/services/exceptions/ErrorCodes.ts @@ -16,6 +16,7 @@ enum ErrorCodes { _302 = '302: Error connecting to the wallet', _303 = '303: Error creating pairing session', + _304 = '304: Error enabling MFA', _600 = '600: Error fetching Safe info', _601 = '601: Error fetching balances',