From 118af615405241409d8acd326712511eabc5ead7 Mon Sep 17 00:00:00 2001 From: schmanu Date: Wed, 27 Sep 2023 15:11:45 +0200 Subject: [PATCH 1/9] feat: signer account page to enable MFA, deviceFactor recovery on login" --- package.json | 1 + .../settings/SignerAccountMFA/index.tsx | 56 ++++++ .../SignerAccountMFA/useMFASettings.ts | 32 +++ .../sidebar/SidebarNavigation/config.tsx | 4 + src/config/routes.ts | 1 + .../mpc/__tests__/useMPCWallet.test.ts | 188 ++++++++++++++++++ src/hooks/wallets/mpc/useMPCWallet.ts | 29 +-- src/pages/settings/signer-account.tsx | 37 ++++ 8 files changed, 336 insertions(+), 12 deletions(-) create mode 100644 src/components/settings/SignerAccountMFA/index.tsx create mode 100644 src/components/settings/SignerAccountMFA/useMFASettings.ts create mode 100644 src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts create mode 100644 src/pages/settings/signer-account.tsx diff --git a/package.json b/package.json index ab8f5ec26a..f5f7b6a666 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "@safe-global/safe-react-components": "^2.0.6", "@sentry/react": "^7.28.1", "@sentry/tracing": "^7.28.1", + "@tkey-mpc/common-types": "^8.2.2", "@truffle/hdwallet-provider": "^2.1.4", "@web3-onboard/coinbase": "^2.2.4", "@web3-onboard/core": "^2.21.0", diff --git a/src/components/settings/SignerAccountMFA/index.tsx b/src/components/settings/SignerAccountMFA/index.tsx new file mode 100644 index 0000000000..139b0bee99 --- /dev/null +++ b/src/components/settings/SignerAccountMFA/index.tsx @@ -0,0 +1,56 @@ +import useMPC from '@/hooks/wallets/mpc/useMPC' +import { Box, Button, Typography } from '@mui/material' +import { COREKIT_STATUS } from '@web3auth/mpc-core-kit' +import { getPubKeyPoint } from '@tkey-mpc/common-types' +import useMFASettings from './useMFASettings' +import { BN } from 'bn.js' +import { useState } from 'react' + +const SignerAccountMFA = () => { + const mpcCoreKit = useMPC() + const mfaSettings = useMFASettings() + + const [enablingMFA, setEnablingMFA] = useState(false) + + const enableMFA = async () => { + setEnablingMFA(true) + if (!mpcCoreKit) { + return + } + try { + // First enable MFA in mpcCoreKit + const recoveryFactor = await mpcCoreKit.enableMFA({}) + + // Then remove the recovery factor the mpcCoreKit creates + const recoverKey = new BN(recoveryFactor, 'hex') + const recoverPubKey = getPubKeyPoint(recoverKey) + await mpcCoreKit.deleteFactor(recoverPubKey, recoverKey) + } catch (error) { + console.error(error) + } finally { + setEnablingMFA(false) + } + } + + if (mpcCoreKit?.status !== COREKIT_STATUS.LOGGED_IN) { + return ( + + You are currently not logged in through a social account + + ) + } + + return ( + + {mfaSettings?.mfaEnabled ? ( + MFA is enabled! + ) : ( + + )} + + ) +} + +export default SignerAccountMFA diff --git a/src/components/settings/SignerAccountMFA/useMFASettings.ts b/src/components/settings/SignerAccountMFA/useMFASettings.ts new file mode 100644 index 0000000000..29726087c0 --- /dev/null +++ b/src/components/settings/SignerAccountMFA/useMFASettings.ts @@ -0,0 +1,32 @@ +import useMPC from '@/hooks/wallets/mpc/useMPC' +import { COREKIT_STATUS } from '@web3auth/mpc-core-kit' + +export type MFASettings = { + mfaEnabled: boolean +} | null + +const useMFASettings = () => { + const mpcCoreKit = useMPC() + + if (mpcCoreKit?.status !== COREKIT_STATUS.LOGGED_IN) { + return null + } + + const { shareDescriptions } = mpcCoreKit?.getKeyDetails() + + const hashedShareModuleFactor = Object.entries(shareDescriptions).find(([key, value]) => + value[0]?.includes('hashedShare'), + ) + + if (hashedShareModuleFactor) { + return { + mfaEnabled: false, + } + } + + return { + mfaEnabled: true, + } +} + +export default useMFASettings diff --git a/src/components/sidebar/SidebarNavigation/config.tsx b/src/components/sidebar/SidebarNavigation/config.tsx index dc7df0699c..05eb71effb 100644 --- a/src/components/sidebar/SidebarNavigation/config.tsx +++ b/src/components/sidebar/SidebarNavigation/config.tsx @@ -104,6 +104,10 @@ export const settingsNavItems = [ label: 'Environment variables', href: AppRoutes.settings.environmentVariables, }, + { + label: 'Signer Account', + href: AppRoutes.settings.signerAccount, + }, ] export const generalSettingsNavItems = [ diff --git a/src/config/routes.ts b/src/config/routes.ts index 6bea1cd90d..ad24a9cf41 100644 --- a/src/config/routes.ts +++ b/src/config/routes.ts @@ -27,6 +27,7 @@ export const AppRoutes = { }, settings: { spendingLimits: '/settings/spending-limits', + signerAccount: '/settings/signer-account', setup: '/settings/setup', modules: '/settings/modules', index: '/settings', diff --git a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts new file mode 100644 index 0000000000..ebb0e7fc7a --- /dev/null +++ b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts @@ -0,0 +1,188 @@ +import { act, renderHook, waitFor } from '@/tests/test-utils' +import { MPCWalletState, useMPCWallet } from '../useMPCWallet' +import * as useOnboard from '@/hooks/wallets/useOnboard' +import { type OnboardAPI } from '@web3-onboard/core' +import { COREKIT_STATUS, type UserInfo, type OauthLoginParams, type Web3AuthMPCCoreKit } from '@web3auth/mpc-core-kit' +import * as mpcCoreKit from '@web3auth/mpc-core-kit' +import { setMPCCoreKitInstance } from '../useMPC' +import { ONBOARD_MPC_MODULE_LABEL } from '@/services/mpc/module' +import { ethers } from 'ethers' +import BN from 'bn.js' + +const MOCK_LOGIN_TIME = 1000 +class MockMPCCoreKit { + status: COREKIT_STATUS = COREKIT_STATUS.INITIALIZED + state: { + userInfo: UserInfo | undefined + } = { + userInfo: undefined, + } + + private stateAfterLogin: COREKIT_STATUS + private userInfoAfterLogin: UserInfo | undefined + private expectedFactorKey: BN + constructor(stateAfterLogin: COREKIT_STATUS, userInfoAfterLogin: UserInfo, expectedFactorKey: BN = new BN(-1)) { + this.stateAfterLogin = stateAfterLogin + this.userInfoAfterLogin = userInfoAfterLogin + this.expectedFactorKey = expectedFactorKey + } + + loginWithOauth(params: OauthLoginParams): Promise { + return new Promise((resolve) => { + // Resolve after 1 sec + setTimeout(() => { + this.status = this.stateAfterLogin + this.state.userInfo = this.userInfoAfterLogin + resolve() + }, 1000) + }) + } + + inputFactorKey(factorKey: BN) { + if (factorKey.eq(this.expectedFactorKey)) { + this.status = COREKIT_STATUS.LOGGED_IN + return Promise.resolve() + } else { + Promise.reject() + } + } +} + +describe('useMPCWallet', () => { + beforeAll(() => { + jest.useFakeTimers() + }) + beforeEach(() => { + jest.resetAllMocks() + }) + afterAll(() => { + jest.useRealTimers() + }) + it('should have state NOT_INITIALIZED initially', () => { + const { result } = renderHook(() => useMPCWallet()) + expect(result.current.walletState).toBe(MPCWalletState.NOT_INITIALIZED) + expect(result.current.userInfo.email).toBeUndefined() + }) + + describe('triggerLogin', () => { + it('should throw if Onboard is not initialized', () => { + const { result } = renderHook(() => useMPCWallet()) + expect(result.current.triggerLogin()).rejects.toEqual(new Error('Onboard is not initialized')) + expect(result.current.walletState).toBe(MPCWalletState.NOT_INITIALIZED) + }) + + it('should throw if MPC Core Kit is not initialized', () => { + jest.spyOn(useOnboard, 'default').mockReturnValue({} as unknown as OnboardAPI) + const { result } = renderHook(() => useMPCWallet()) + + expect(result.current.triggerLogin()).rejects.toEqual(new Error('MPC Core Kit is not initialized')) + expect(result.current.walletState).toBe(MPCWalletState.NOT_INITIALIZED) + }) + + it('should handle successful log in for SFA account', async () => { + jest.spyOn(useOnboard, 'default').mockReturnValue({} as unknown as OnboardAPI) + const connectWalletSpy = jest.fn().mockImplementation(() => Promise.resolve()) + jest.spyOn(useOnboard, 'connectWallet').mockImplementation(connectWalletSpy) + setMPCCoreKitInstance( + new MockMPCCoreKit(COREKIT_STATUS.LOGGED_IN, { + email: 'test@test.com', + name: 'Test', + } as unknown as UserInfo) as unknown as Web3AuthMPCCoreKit, + ) + const { result } = renderHook(() => useMPCWallet()) + + act(() => { + result.current.triggerLogin() + }) + + expect(result.current.walletState === MPCWalletState.AUTHENTICATING) + expect(connectWalletSpy).not.toBeCalled() + + jest.advanceTimersByTime(MOCK_LOGIN_TIME) + + await waitFor(() => { + expect(result.current.walletState === MPCWalletState.READY) + expect(connectWalletSpy).toBeCalledWith(expect.anything(), { + autoSelect: { + label: ONBOARD_MPC_MODULE_LABEL, + disableModals: true, + }, + }) + }) + }) + + it('should handle successful log in for MFA account with device share', async () => { + const mockDeviceFactor = ethers.Wallet.createRandom().privateKey.slice(2) + jest.spyOn(useOnboard, 'default').mockReturnValue({} as unknown as OnboardAPI) + const connectWalletSpy = jest.fn().mockImplementation(() => Promise.resolve()) + jest.spyOn(useOnboard, 'connectWallet').mockImplementation(connectWalletSpy) + setMPCCoreKitInstance( + new MockMPCCoreKit( + COREKIT_STATUS.REQUIRED_SHARE, + { + email: 'test@test.com', + name: 'Test', + } as unknown as UserInfo, + new BN(mockDeviceFactor, 'hex'), + ) as unknown as Web3AuthMPCCoreKit, + ) + + jest.spyOn(mpcCoreKit, 'getWebBrowserFactor').mockReturnValue(Promise.resolve(mockDeviceFactor)) + + const { result } = renderHook(() => useMPCWallet()) + + act(() => { + result.current.triggerLogin() + }) + + expect(result.current.walletState === MPCWalletState.AUTHENTICATING) + expect(connectWalletSpy).not.toBeCalled() + + jest.advanceTimersByTime(MOCK_LOGIN_TIME) + + await waitFor(() => { + expect(result.current.walletState === MPCWalletState.READY) + expect(connectWalletSpy).toBeCalledWith(expect.anything(), { + autoSelect: { + label: ONBOARD_MPC_MODULE_LABEL, + disableModals: true, + }, + }) + }) + }) + }) + + describe('resetAccount', () => { + it('should throw if mpcCoreKit is not initialized', () => { + const { result } = renderHook(() => useMPCWallet()) + expect(result.current.resetAccount()).rejects.toEqual( + new Error('MPC Core Kit is not initialized or the user is not logged in'), + ) + }) + it('should reset an account by overwriting the metadata', async () => { + const mockSetMetadata = jest.fn() + const mockMPCCore = { + metadataKey: ethers.Wallet.createRandom().privateKey.slice(2), + state: { + userInfo: undefined, + }, + tKey: { + storageLayer: { + setMetadata: mockSetMetadata, + }, + }, + } + + setMPCCoreKitInstance(mockMPCCore as unknown as Web3AuthMPCCoreKit) + + const { result } = renderHook(() => useMPCWallet()) + + await result.current.resetAccount() + + expect(mockSetMetadata).toHaveBeenCalledWith({ + privKey: new BN(mockMPCCore.metadataKey, 'hex'), + input: { message: 'KEY_NOT_FOUND' }, + }) + }) + }) +}) diff --git a/src/hooks/wallets/mpc/useMPCWallet.ts b/src/hooks/wallets/mpc/useMPCWallet.ts index f0d54044c5..42fd5e097f 100644 --- a/src/hooks/wallets/mpc/useMPCWallet.ts +++ b/src/hooks/wallets/mpc/useMPCWallet.ts @@ -2,18 +2,13 @@ import { useState } from 'react' import useMPC from './useMPC' import BN from 'bn.js' import { GOOGLE_CLIENT_ID, WEB3AUTH_VERIFIER_ID } from '@/config/constants' -import { COREKIT_STATUS } from '@web3auth/mpc-core-kit' +import { COREKIT_STATUS, getWebBrowserFactor } from '@web3auth/mpc-core-kit' import useOnboard, { connectWallet } from '../useOnboard' import { ONBOARD_MPC_MODULE_LABEL } from '@/services/mpc/module' export enum MPCWalletState { NOT_INITIALIZED, AUTHENTICATING, - AUTHENTICATED, - CREATING_SECOND_FACTOR, - RECOVERING_ACCOUNT_PASSWORD, - CREATED_SECOND_FACTOR, - FINALIZING_ACCOUNT, READY, } @@ -38,7 +33,7 @@ export const useMPCWallet = (): MPCWalletHook => { // Resetting your account means clearing all the metadata associated with it from the metadata server // The key details will be deleted from our server and you will not be able to recover your account if (!mpcCoreKit || !mpcCoreKit.metadataKey) { - throw new Error('coreKitInstance is not set or the user is not logged in') + throw new Error('MPC Core Kit is not initialized or the user is not logged in') } // In web3auth an account is reset by overwriting the metadata with KEY_NOT_FOUND @@ -50,13 +45,11 @@ export const useMPCWallet = (): MPCWalletHook => { const triggerLogin = async () => { if (!onboard) { - console.error('Onboard not initialized') - return + throw Error('Onboard is not initialized') } if (!mpcCoreKit) { - console.error('tKey not initialized yet') - return + throw Error('MPC Core Kit is not initialized') } try { setWalletState(MPCWalletState.AUTHENTICATING) @@ -68,6 +61,18 @@ export const useMPCWallet = (): MPCWalletHook => { }, }) + if (mpcCoreKit.status === COREKIT_STATUS.REQUIRED_SHARE) { + // Check if we have a device share stored + const deviceFactor = await getWebBrowserFactor(mpcCoreKit) + if (deviceFactor) { + // Recover from device factor + const deviceFactorKey = new BN(deviceFactor, 'hex') + await mpcCoreKit.inputFactorKey(deviceFactorKey) + } + } + + // TODO: IF still required share, trigger another recovery option (i.e. Security Questions) or throw error as unrecoverable + if (mpcCoreKit.status === COREKIT_STATUS.LOGGED_IN) { connectWallet(onboard, { autoSelect: { @@ -77,7 +82,7 @@ export const useMPCWallet = (): MPCWalletHook => { }).catch((reason) => console.error('Error connecting to MPC module:', reason)) } - setWalletState(MPCWalletState.AUTHENTICATED) + setWalletState(MPCWalletState.READY) } catch (error) { setWalletState(MPCWalletState.NOT_INITIALIZED) console.error(error) diff --git a/src/pages/settings/signer-account.tsx b/src/pages/settings/signer-account.tsx new file mode 100644 index 0000000000..dad68286bd --- /dev/null +++ b/src/pages/settings/signer-account.tsx @@ -0,0 +1,37 @@ +import { Grid, Paper, Typography } from '@mui/material' + +import type { NextPage } from 'next' +import Head from 'next/head' + +import SettingsHeader from '@/components/settings/SettingsHeader' +import SignerAccountMFA from '@/components/settings/SignerAccountMFA' + +const SignerAccountPage: NextPage = () => { + return ( + <> + + {'Safe{Wallet} – Settings – Signer Account'} + + + + +
+ + + + + Multi factor Authentication + + + + + + + + +
+ + ) +} + +export default SignerAccountPage From 4f8b03dcd147137ce9b5620d36f83ea7203af0fe Mon Sep 17 00:00:00 2001 From: schmanu Date: Thu, 28 Sep 2023 10:45:18 +0200 Subject: [PATCH 2/9] fix: review issues --- src/components/settings/SignerAccountMFA/index.tsx | 5 +++-- .../settings/SignerAccountMFA/useMFASettings.ts | 12 ++---------- src/hooks/wallets/mpc/useMPC.ts | 2 +- src/hooks/wallets/mpc/useMPCWallet.ts | 2 +- src/pages/settings/signer-account.tsx | 2 +- 5 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/components/settings/SignerAccountMFA/index.tsx b/src/components/settings/SignerAccountMFA/index.tsx index 139b0bee99..09ae06e276 100644 --- a/src/components/settings/SignerAccountMFA/index.tsx +++ b/src/components/settings/SignerAccountMFA/index.tsx @@ -13,10 +13,11 @@ const SignerAccountMFA = () => { const [enablingMFA, setEnablingMFA] = useState(false) const enableMFA = async () => { - setEnablingMFA(true) if (!mpcCoreKit) { return } + + setEnablingMFA(true) try { // First enable MFA in mpcCoreKit const recoveryFactor = await mpcCoreKit.enableMFA({}) @@ -35,7 +36,7 @@ const SignerAccountMFA = () => { if (mpcCoreKit?.status !== COREKIT_STATUS.LOGGED_IN) { return ( - You are currently not logged in through a social account + You are currently not logged in with a social account ) } diff --git a/src/components/settings/SignerAccountMFA/useMFASettings.ts b/src/components/settings/SignerAccountMFA/useMFASettings.ts index 29726087c0..c8992cc0c2 100644 --- a/src/components/settings/SignerAccountMFA/useMFASettings.ts +++ b/src/components/settings/SignerAccountMFA/useMFASettings.ts @@ -14,18 +14,10 @@ const useMFASettings = () => { const { shareDescriptions } = mpcCoreKit?.getKeyDetails() - const hashedShareModuleFactor = Object.entries(shareDescriptions).find(([key, value]) => - value[0]?.includes('hashedShare'), - ) - - if (hashedShareModuleFactor) { - return { - mfaEnabled: false, - } - } + const isMFAEnabled = !Object.entries(shareDescriptions).some(([key, value]) => value[0]?.includes('hashedShare')) return { - mfaEnabled: true, + mfaEnabled: isMFAEnabled, } } diff --git a/src/hooks/wallets/mpc/useMPC.ts b/src/hooks/wallets/mpc/useMPC.ts index ba110255ea..2e66c036b8 100644 --- a/src/hooks/wallets/mpc/useMPC.ts +++ b/src/hooks/wallets/mpc/useMPC.ts @@ -40,7 +40,7 @@ export const useInitMPC = () => { const web3AuthCoreKit = new Web3AuthMPCCoreKit({ web3AuthClientId: WEB3_AUTH_CLIENT_ID, // Available networks are "sapphire_devnet", "sapphire_mainnet" - web3AuthNetwork: WEB3AUTH_NETWORK.DEVNET, + web3AuthNetwork: WEB3AUTH_NETWORK.MAINNET, baseUrl: `${window.location.origin}/serviceworker`, uxMode: 'popup', enableLogging: true, diff --git a/src/hooks/wallets/mpc/useMPCWallet.ts b/src/hooks/wallets/mpc/useMPCWallet.ts index 42fd5e097f..913f5a534c 100644 --- a/src/hooks/wallets/mpc/useMPCWallet.ts +++ b/src/hooks/wallets/mpc/useMPCWallet.ts @@ -33,7 +33,7 @@ export const useMPCWallet = (): MPCWalletHook => { // Resetting your account means clearing all the metadata associated with it from the metadata server // The key details will be deleted from our server and you will not be able to recover your account if (!mpcCoreKit || !mpcCoreKit.metadataKey) { - throw new Error('MPC Core Kit is not initialized or the user is not logged in') + throw new Error('MPC Core Kit is not initialized or the user is not logged in') } // In web3auth an account is reset by overwriting the metadata with KEY_NOT_FOUND diff --git a/src/pages/settings/signer-account.tsx b/src/pages/settings/signer-account.tsx index dad68286bd..4827c4c8bc 100644 --- a/src/pages/settings/signer-account.tsx +++ b/src/pages/settings/signer-account.tsx @@ -20,7 +20,7 @@ const SignerAccountPage: NextPage = () => { - Multi factor Authentication + Multi-factor Authentication From 211b5dcb93d9a1b1604b33bcd9f38606c80cdeae Mon Sep 17 00:00:00 2001 From: schmanu Date: Fri, 29 Sep 2023 10:50:58 +0200 Subject: [PATCH 3/9] feat: password recovery and device share --- .../common/ConnectWallet/MPCWallet.tsx | 11 +- .../ConnectWallet/MPCWalletProvider.tsx | 2 +- .../common/ConnectWallet/PasswordRecovery.tsx | 64 ++++++++ .../settings/SignerAccountMFA/index.tsx | 143 +++++++++++++++--- .../SignerAccountMFA/useMFASettings.ts | 7 +- .../wallets/mpc/__tests__/useMPC.test.ts | 8 +- .../wallets/mpc/recovery/useDeviceShare.ts | 51 +++++++ .../mpc/recovery/useSecurityQuestions.ts | 58 +++++++ src/hooks/wallets/mpc/useMPC.ts | 4 +- src/hooks/wallets/mpc/useMPCWallet.ts | 74 +++++++-- src/services/mpc/__tests__/module.test.ts | 4 +- src/services/mpc/module.ts | 6 +- 12 files changed, 382 insertions(+), 50 deletions(-) create mode 100644 src/components/common/ConnectWallet/PasswordRecovery.tsx create mode 100644 src/hooks/wallets/mpc/recovery/useDeviceShare.ts create mode 100644 src/hooks/wallets/mpc/recovery/useSecurityQuestions.ts diff --git a/src/components/common/ConnectWallet/MPCWallet.tsx b/src/components/common/ConnectWallet/MPCWallet.tsx index 3799144921..a176751d14 100644 --- a/src/components/common/ConnectWallet/MPCWallet.tsx +++ b/src/components/common/ConnectWallet/MPCWallet.tsx @@ -1,9 +1,14 @@ +import { MPCWalletState } from '@/hooks/wallets/mpc/useMPCWallet' import { Box, Button, CircularProgress } from '@mui/material' import { useContext } from 'react' import { MpcWalletContext } from './MPCWalletProvider' +import { PasswordRecovery } from './PasswordRecovery' export const MPCWallet = () => { - const { loginPending, triggerLogin, resetAccount, userInfo } = useContext(MpcWalletContext) + const { loginPending, triggerLogin, resetAccount, userInfo, walletState, recoverFactorWithPassword } = + useContext(MpcWalletContext) + + console.log(walletState) return ( <> @@ -28,6 +33,10 @@ export const MPCWallet = () => { )} )} + + {walletState === MPCWalletState.MANUAL_RECOVERY && ( + + )} ) } diff --git a/src/components/common/ConnectWallet/MPCWalletProvider.tsx b/src/components/common/ConnectWallet/MPCWalletProvider.tsx index 30e2be7117..30360bdcb2 100644 --- a/src/components/common/ConnectWallet/MPCWalletProvider.tsx +++ b/src/components/common/ConnectWallet/MPCWalletProvider.tsx @@ -6,7 +6,7 @@ type MPCWalletContext = { triggerLogin: () => Promise resetAccount: () => Promise upsertPasswordBackup: (password: string) => Promise - recoverFactorWithPassword: (password: string) => Promise + recoverFactorWithPassword: (password: string, storeDeviceFactor: boolean) => Promise walletState: MPCWalletState userInfo: { email: string | undefined diff --git a/src/components/common/ConnectWallet/PasswordRecovery.tsx b/src/components/common/ConnectWallet/PasswordRecovery.tsx new file mode 100644 index 0000000000..3bc8d5ba5a --- /dev/null +++ b/src/components/common/ConnectWallet/PasswordRecovery.tsx @@ -0,0 +1,64 @@ +import { VisibilityOff, Visibility } from '@mui/icons-material' +import { + DialogContent, + Typography, + TextField, + IconButton, + FormControlLabel, + Checkbox, + Button, + Box, +} from '@mui/material' +import { useState } from 'react' +import ModalDialog from '../ModalDialog' + +export const PasswordRecovery = ({ + recoverFactorWithPassword, +}: { + recoverFactorWithPassword: (password: string, storeDeviceFactor: boolean) => Promise +}) => { + const [showPassword, setShowPassword] = useState(false) + const [recoveryPassword, setRecoveryPassword] = useState('') + const [storeDeviceFactor, setStoreDeviceFactor] = useState(false) + return ( + + + + + This browser is not registered with your Account yet. Please enter your recovery password to restore access + to this account. + + + { + setRecoveryPassword(event.target.value) + }} + InputProps={{ + endAdornment: ( + setShowPassword((prev) => !prev)} + edge="end" + > + {showPassword ? : } + + ), + }} + /> + setStoreDeviceFactor((prev) => !prev)} />} + label="Do not ask again on this device" + /> + + + + + + + ) +} diff --git a/src/components/settings/SignerAccountMFA/index.tsx b/src/components/settings/SignerAccountMFA/index.tsx index 09ae06e276..6518cccdc5 100644 --- a/src/components/settings/SignerAccountMFA/index.tsx +++ b/src/components/settings/SignerAccountMFA/index.tsx @@ -1,17 +1,43 @@ import useMPC from '@/hooks/wallets/mpc/useMPC' -import { Box, Button, Typography } from '@mui/material' +import { Box, Button, Checkbox, FormControlLabel, TextField, Typography } from '@mui/material' import { COREKIT_STATUS } from '@web3auth/mpc-core-kit' import { getPubKeyPoint } from '@tkey-mpc/common-types' -import useMFASettings from './useMFASettings' import { BN } from 'bn.js' -import { useState } from 'react' +import { useEffect, useMemo, useState } from 'react' +import { useSecurityQuestions } from '@/hooks/wallets/mpc/recovery/useSecurityQuestions' +import useMFASettings from './useMFASettings' +import { useForm } from 'react-hook-form' +import { useDeviceShare } from '@/hooks/wallets/mpc/recovery/useDeviceShare' + +type SignerAccountFormData = { + oldPassword: string | undefined + newPassword: string + confirmPassword: string + storeDeviceShare: boolean +} const SignerAccountMFA = () => { const mpcCoreKit = useMPC() - const mfaSettings = useMFASettings() + const mfaSettings = useMFASettings(mpcCoreKit) + const securityQuestions = useSecurityQuestions(mpcCoreKit) + const deviceShareModule = useDeviceShare(mpcCoreKit) + + const formMethods = useForm({ + mode: 'all', + }) + + const { register, formState, watch, setValue, handleSubmit } = formMethods const [enablingMFA, setEnablingMFA] = useState(false) + const isPasswordSet = useMemo(() => securityQuestions.isEnabled(), [securityQuestions]) + + console.log(mpcCoreKit) + + useEffect(() => { + deviceShareModule.isEnabled().then((value) => setValue('storeDeviceShare', value)) + }, [deviceShareModule, setValue]) + const enableMFA = async () => { if (!mpcCoreKit) { return @@ -19,13 +45,35 @@ const SignerAccountMFA = () => { setEnablingMFA(true) try { - // First enable MFA in mpcCoreKit - const recoveryFactor = await mpcCoreKit.enableMFA({}) + 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') + } - // Then remove the recovery factor the mpcCoreKit creates - const recoverKey = new BN(recoveryFactor, 'hex') - const recoverPubKey = getPubKeyPoint(recoverKey) - await mpcCoreKit.deleteFactor(recoverPubKey, recoverKey) + 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 deviceShareModule.isEnabled() + if (hasDeviceShare !== storeDeviceShare) { + if (storeDeviceShare) { + await deviceShareModule.createAndStoreDeviceFactor() + } else { + // Switch to password recovery factor such that we can delete the device factor + await mpcCoreKit.inputFactorKey(new BN(securityQuestionFactor, 'hex')) + + await deviceShareModule.removeDeviceFactor() + } + } } catch (error) { console.error(error) } finally { @@ -41,16 +89,75 @@ const SignerAccountMFA = () => { ) } + const onSubmit = async () => { + console.log('submitting') + await enableMFA() + } + return ( - - {mfaSettings?.mfaEnabled ? ( - MFA is enabled! - ) : ( - - )} - + + ) } diff --git a/src/components/settings/SignerAccountMFA/useMFASettings.ts b/src/components/settings/SignerAccountMFA/useMFASettings.ts index c8992cc0c2..0c56133324 100644 --- a/src/components/settings/SignerAccountMFA/useMFASettings.ts +++ b/src/components/settings/SignerAccountMFA/useMFASettings.ts @@ -1,13 +1,10 @@ -import useMPC from '@/hooks/wallets/mpc/useMPC' -import { COREKIT_STATUS } from '@web3auth/mpc-core-kit' +import { COREKIT_STATUS, type Web3AuthMPCCoreKit } from '@web3auth/mpc-core-kit' export type MFASettings = { mfaEnabled: boolean } | null -const useMFASettings = () => { - const mpcCoreKit = useMPC() - +const useMFASettings = (mpcCoreKit: Web3AuthMPCCoreKit | undefined) => { if (mpcCoreKit?.status !== COREKIT_STATUS.LOGGED_IN) { return null } diff --git a/src/hooks/wallets/mpc/__tests__/useMPC.test.ts b/src/hooks/wallets/mpc/__tests__/useMPC.test.ts index 3f67c0a1e7..edf9dfe9c3 100644 --- a/src/hooks/wallets/mpc/__tests__/useMPC.test.ts +++ b/src/hooks/wallets/mpc/__tests__/useMPC.test.ts @@ -1,6 +1,6 @@ import * as useOnboard from '@/hooks/wallets/useOnboard' import { renderHook, waitFor } from '@/tests/test-utils' -import { getMPCCoreKitInstance, setMPCCoreKitInstance, useInitMPC } from '../useMPC' +import { _getMPCCoreKitInstance, setMPCCoreKitInstance, useInitMPC } from '../useMPC' import * as useChains from '@/hooks/useChains' import { type ChainInfo, RPC_AUTHENTICATION } from '@safe-global/safe-gateway-typescript-sdk' import { hexZeroPad } from 'ethers/lib/utils' @@ -104,7 +104,7 @@ describe('useInitMPC', () => { renderHook(() => useInitMPC()) await waitFor(() => { - expect(getMPCCoreKitInstance()).toBeDefined() + expect(_getMPCCoreKitInstance()).toBeDefined() expect(connectWalletSpy).not.toBeCalled() }) }) @@ -151,7 +151,7 @@ describe('useInitMPC', () => { await waitFor(() => { expect(connectWalletSpy).toBeCalled() - expect(getMPCCoreKitInstance()).toBeDefined() + expect(_getMPCCoreKitInstance()).toBeDefined() }) }) @@ -215,7 +215,7 @@ describe('useInitMPC', () => { await waitFor(() => { expect(mockChainChangedListener).toHaveBeenCalledWith('0x5') - expect(getMPCCoreKitInstance()).toBeDefined() + expect(_getMPCCoreKitInstance()).toBeDefined() expect(connectWalletSpy).not.toBeCalled() }) }) diff --git a/src/hooks/wallets/mpc/recovery/useDeviceShare.ts b/src/hooks/wallets/mpc/recovery/useDeviceShare.ts new file mode 100644 index 0000000000..5ca317a096 --- /dev/null +++ b/src/hooks/wallets/mpc/recovery/useDeviceShare.ts @@ -0,0 +1,51 @@ +import { + BrowserStorage, + getWebBrowserFactor, + storeWebBrowserFactor, + TssShareType, + type Web3AuthMPCCoreKit, +} from '@web3auth/mpc-core-kit' +import BN from 'bn.js' +import { getPubKeyPoint } from '@tkey-mpc/common-types' + +export const useDeviceShare = (mpcCoreKit: Web3AuthMPCCoreKit | undefined) => { + const isEnabled = async () => { + if (!mpcCoreKit || !mpcCoreKit.tKey.metadata) { + return false + } + return !!(await getWebBrowserFactor(mpcCoreKit)) + } + + const createAndStoreDeviceFactor = async () => { + if (!mpcCoreKit) { + return + } + const userAgent = navigator.userAgent + + const deviceFactorKey = new BN( + await mpcCoreKit.createFactor({ shareType: TssShareType.DEVICE, additionalMetadata: { userAgent } }), + 'hex', + ) + await storeWebBrowserFactor(deviceFactorKey, mpcCoreKit) + } + + const removeDeviceFactor = async () => { + if (!mpcCoreKit) { + return + } + const deviceFactor = await getWebBrowserFactor(mpcCoreKit) + const key = new BN(deviceFactor, 'hex') + const pubKey = getPubKeyPoint(key) + const pubKeyX = pubKey.x.toString('hex', 64) + await mpcCoreKit.deleteFactor(pubKey) + const currentStorage = BrowserStorage.getInstance('mpc_corekit_store') + debugger + currentStorage.set(pubKeyX, undefined) + } + + return { + isEnabled, + createAndStoreDeviceFactor, + removeDeviceFactor, + } +} diff --git a/src/hooks/wallets/mpc/recovery/useSecurityQuestions.ts b/src/hooks/wallets/mpc/recovery/useSecurityQuestions.ts new file mode 100644 index 0000000000..9e90b8a10b --- /dev/null +++ b/src/hooks/wallets/mpc/recovery/useSecurityQuestions.ts @@ -0,0 +1,58 @@ +import { TssSecurityQuestion, TssShareType, type Web3AuthMPCCoreKit } from '@web3auth/mpc-core-kit' + +const DEFAULT_SECURITY_QUESTION = 'ENTER PASSWORD' + +const securityQuestions = new TssSecurityQuestion() + +export const useSecurityQuestions = (mpcCoreKit: Web3AuthMPCCoreKit | undefined) => { + const isEnabled = () => { + if (!mpcCoreKit) { + return false + } + try { + const question = securityQuestions.getQuestion(mpcCoreKit) + return !!question + } catch (error) { + console.error(error) + // It errors out if recovery is not setup currently + return false + } + } + + const upsertPassword = async (newPassword: string, oldPassword?: string) => { + if (!mpcCoreKit) { + return + } + if (isEnabled()) { + if (!oldPassword) { + throw Error('To change the password you need to provide the old password') + } + await securityQuestions.changeSecurityQuestion({ + answer: oldPassword, + mpcCoreKit, + newAnswer: newPassword, + newQuestion: DEFAULT_SECURITY_QUESTION, + }) + } else { + await securityQuestions.setSecurityQuestion({ + question: DEFAULT_SECURITY_QUESTION, + answer: newPassword, + mpcCoreKit, + shareType: TssShareType.DEVICE, + }) + } + } + + const recoverWithPassword = async (password: string) => { + if (!mpcCoreKit) { + return + } + return securityQuestions.recoverFactor(mpcCoreKit, password) + } + + return { + isEnabled, + upsertPassword, + recoverWithPassword, + } +} diff --git a/src/hooks/wallets/mpc/useMPC.ts b/src/hooks/wallets/mpc/useMPC.ts index 2e66c036b8..c7f4607c05 100644 --- a/src/hooks/wallets/mpc/useMPC.ts +++ b/src/hooks/wallets/mpc/useMPC.ts @@ -24,7 +24,7 @@ export const useInitMPC = () => { chainNamespace: CHAIN_NAMESPACES.EIP155, rpcTarget: getRpcServiceUrl(chain.rpcUri), displayName: chain.chainName, - blockExplorer: chain.blockExplorerUriTemplate.address, + blockExplorer: new URL(chain.blockExplorerUriTemplate.address).origin, ticker: chain.nativeCurrency.symbol, tickerName: chain.nativeCurrency.name, } @@ -79,7 +79,7 @@ export const useInitMPC = () => { }, [chain, onboard]) } -export const getMPCCoreKitInstance = getStore +export const _getMPCCoreKitInstance = getStore export const setMPCCoreKitInstance = setStore diff --git a/src/hooks/wallets/mpc/useMPCWallet.ts b/src/hooks/wallets/mpc/useMPCWallet.ts index 913f5a534c..d2d0a1e5fd 100644 --- a/src/hooks/wallets/mpc/useMPCWallet.ts +++ b/src/hooks/wallets/mpc/useMPCWallet.ts @@ -5,18 +5,22 @@ import { GOOGLE_CLIENT_ID, WEB3AUTH_VERIFIER_ID } from '@/config/constants' import { COREKIT_STATUS, getWebBrowserFactor } from '@web3auth/mpc-core-kit' import useOnboard, { connectWallet } from '../useOnboard' import { ONBOARD_MPC_MODULE_LABEL } from '@/services/mpc/module' +import { useSecurityQuestions } from './recovery/useSecurityQuestions' +import { useDeviceShare } from './recovery/useDeviceShare' export enum MPCWalletState { NOT_INITIALIZED, AUTHENTICATING, + MANUAL_RECOVERY, READY, } export type MPCWalletHook = { upsertPasswordBackup: (password: string) => Promise - recoverFactorWithPassword: (password: string) => Promise + recoverFactorWithPassword: (password: string, storeDeviceShare: boolean) => Promise walletState: MPCWalletState triggerLogin: () => Promise + isMFAEnabled: () => boolean resetAccount: () => Promise userInfo: { email: string | undefined @@ -27,6 +31,17 @@ export const useMPCWallet = (): MPCWalletHook => { const [walletState, setWalletState] = useState(MPCWalletState.NOT_INITIALIZED) const mpcCoreKit = useMPC() const onboard = useOnboard() + const securityQuestions = useSecurityQuestions(mpcCoreKit) + const deviceShareModule = useDeviceShare(mpcCoreKit) + + 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 @@ -62,37 +77,68 @@ export const useMPCWallet = (): MPCWalletHook => { }) if (mpcCoreKit.status === COREKIT_STATUS.REQUIRED_SHARE) { + console.log('Share required') // Check if we have a device share stored const deviceFactor = await getWebBrowserFactor(mpcCoreKit) if (deviceFactor) { + console.log('Using device factor') // Recover from device factor const deviceFactorKey = new BN(deviceFactor, 'hex') await mpcCoreKit.inputFactorKey(deviceFactorKey) + } else { + console.log('using password') + // Check password recovery + if (securityQuestions.isEnabled()) { + setWalletState(MPCWalletState.MANUAL_RECOVERY) + return + } } } - // TODO: IF still required share, trigger another recovery option (i.e. Security Questions) or throw error as unrecoverable - - if (mpcCoreKit.status === COREKIT_STATUS.LOGGED_IN) { - connectWallet(onboard, { - autoSelect: { - label: ONBOARD_MPC_MODULE_LABEL, - disableModals: true, - }, - }).catch((reason) => console.error('Error connecting to MPC module:', reason)) - } - - setWalletState(MPCWalletState.READY) + finalizeLogin() } catch (error) { setWalletState(MPCWalletState.NOT_INITIALIZED) console.error(error) } } + const finalizeLogin = () => { + if (!mpcCoreKit || !onboard) { + return + } + if (mpcCoreKit.status === COREKIT_STATUS.LOGGED_IN) { + connectWallet(onboard, { + autoSelect: { + label: ONBOARD_MPC_MODULE_LABEL, + disableModals: true, + }, + }).catch((reason) => console.error('Error connecting to MPC module:', reason)) + setWalletState(MPCWalletState.READY) + } + } + + const recoverFactorWithPassword = async (password: string, storeDeviceShare: boolean = false) => { + if (mpcCoreKit && securityQuestions.isEnabled()) { + const factorKeyString = await securityQuestions.recoverWithPassword(password) + if (!factorKeyString) { + throw new Error('The password is invalid') + } + const factorKey = new BN(factorKeyString, 'hex') + await mpcCoreKit.inputFactorKey(factorKey) + + if (storeDeviceShare) { + await deviceShareModule.createAndStoreDeviceFactor() + } + + finalizeLogin() + } + } + return { triggerLogin, walletState, - recoverFactorWithPassword: () => Promise.resolve(), + isMFAEnabled, + recoverFactorWithPassword, resetAccount: criticalResetAccount, upsertPasswordBackup: () => Promise.resolve(), userInfo: { diff --git a/src/services/mpc/__tests__/module.test.ts b/src/services/mpc/__tests__/module.test.ts index 4a510b992f..3a03f09c81 100644 --- a/src/services/mpc/__tests__/module.test.ts +++ b/src/services/mpc/__tests__/module.test.ts @@ -36,7 +36,7 @@ describe('MPC Onboard module', () => { send: mockReadOnlySend, } as any) - jest.spyOn(useMPC, 'getMPCCoreKitInstance').mockImplementation(() => { + jest.spyOn(useMPC, '_getMPCCoreKitInstance').mockImplementation(() => { return { provider: {}, } as any @@ -85,7 +85,7 @@ describe('MPC Onboard module', () => { send: mockReadOnlySend, } as any) - jest.spyOn(useMPC, 'getMPCCoreKitInstance').mockImplementation(() => { + jest.spyOn(useMPC, '_getMPCCoreKitInstance').mockImplementation(() => { return { provider: { request: mockMPCProviderRequest, diff --git a/src/services/mpc/module.ts b/src/services/mpc/module.ts index 180cbddc43..1814f111d0 100644 --- a/src/services/mpc/module.ts +++ b/src/services/mpc/module.ts @@ -1,9 +1,9 @@ -import { getMPCCoreKitInstance } from '@/hooks/wallets/mpc/useMPC' +import { _getMPCCoreKitInstance } from '@/hooks/wallets/mpc/useMPC' import { getWeb3ReadOnly } from '@/hooks/wallets/web3' import { type WalletInit, ProviderRpcError } from '@web3-onboard/common' import { type EIP1193Provider } from '@web3-onboard/core' -const getMPCProvider = () => getMPCCoreKitInstance()?.provider +const getMPCProvider = () => _getMPCCoreKitInstance()?.provider const assertDefined = (mpcProvider: T | undefined) => { if (!mpcProvider) { @@ -78,7 +78,7 @@ function MpcModule(): WalletInit { return web3.removeListener(event, listener) }, disconnect: () => { - getMPCCoreKitInstance()?.logout() + _getMPCCoreKitInstance()?.logout() }, } From 6a1edf01129d9bb751056444684acb2f6ffb9ce4 Mon Sep 17 00:00:00 2001 From: schmanu Date: Wed, 4 Oct 2023 09:43:22 +0200 Subject: [PATCH 4/9] fix: test, refactor, remove console.logs --- .../settings/SignerAccountMFA/index.tsx | 18 +++++++++--------- .../wallets/mpc/__tests__/useMPCWallet.test.ts | 2 +- src/hooks/wallets/mpc/useMPCWallet.ts | 3 --- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/components/settings/SignerAccountMFA/index.tsx b/src/components/settings/SignerAccountMFA/index.tsx index 6518cccdc5..fae01a8191 100644 --- a/src/components/settings/SignerAccountMFA/index.tsx +++ b/src/components/settings/SignerAccountMFA/index.tsx @@ -64,15 +64,15 @@ const SignerAccountMFA = () => { } const hasDeviceShare = await deviceShareModule.isEnabled() - if (hasDeviceShare !== storeDeviceShare) { - if (storeDeviceShare) { - await deviceShareModule.createAndStoreDeviceFactor() - } else { - // Switch to password recovery factor such that we can delete the device factor - await mpcCoreKit.inputFactorKey(new BN(securityQuestionFactor, 'hex')) - - await deviceShareModule.removeDeviceFactor() - } + + if (!hasDeviceShare && storeDeviceShare) { + await deviceShareModule.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 deviceShareModule.removeDeviceFactor() } } catch (error) { console.error(error) diff --git a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts index ebb0e7fc7a..0efea9dec0 100644 --- a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts +++ b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts @@ -156,7 +156,7 @@ describe('useMPCWallet', () => { it('should throw if mpcCoreKit is not initialized', () => { const { result } = renderHook(() => useMPCWallet()) expect(result.current.resetAccount()).rejects.toEqual( - new Error('MPC Core Kit is not initialized or the user is not logged in'), + new Error('MPC Core Kit is not initialized or the user is not logged in'), ) }) it('should reset an account by overwriting the metadata', async () => { diff --git a/src/hooks/wallets/mpc/useMPCWallet.ts b/src/hooks/wallets/mpc/useMPCWallet.ts index d2d0a1e5fd..e4fb5ee68c 100644 --- a/src/hooks/wallets/mpc/useMPCWallet.ts +++ b/src/hooks/wallets/mpc/useMPCWallet.ts @@ -77,16 +77,13 @@ export const useMPCWallet = (): MPCWalletHook => { }) if (mpcCoreKit.status === COREKIT_STATUS.REQUIRED_SHARE) { - console.log('Share required') // Check if we have a device share stored const deviceFactor = await getWebBrowserFactor(mpcCoreKit) if (deviceFactor) { - console.log('Using device factor') // Recover from device factor const deviceFactorKey = new BN(deviceFactor, 'hex') await mpcCoreKit.inputFactorKey(deviceFactorKey) } else { - console.log('using password') // Check password recovery if (securityQuestions.isEnabled()) { setWalletState(MPCWalletState.MANUAL_RECOVERY) From fad177cf49d3ce2ed8c91dc7a9196f452025df37 Mon Sep 17 00:00:00 2001 From: schmanu Date: Wed, 4 Oct 2023 14:23:58 +0200 Subject: [PATCH 5/9] fix: refactor, remove console.logs, new test --- .../common/ConnectWallet/MPCWallet.tsx | 2 - .../settings/SignerAccountMFA/index.tsx | 47 +++---- .../mpc/__tests__/useMPCWallet.test.ts | 116 +++++++++++++++++- .../mpc/recovery/DeviceShareRecovery.ts | 45 +++++++ .../mpc/recovery/SecurityQuestionRecovery.ts | 48 ++++++++ .../wallets/mpc/recovery/useDeviceShare.ts | 51 -------- .../mpc/recovery/useSecurityQuestions.ts | 58 --------- src/hooks/wallets/mpc/useMPCWallet.ts | 23 ++-- 8 files changed, 243 insertions(+), 147 deletions(-) create mode 100644 src/hooks/wallets/mpc/recovery/DeviceShareRecovery.ts create mode 100644 src/hooks/wallets/mpc/recovery/SecurityQuestionRecovery.ts delete mode 100644 src/hooks/wallets/mpc/recovery/useDeviceShare.ts delete mode 100644 src/hooks/wallets/mpc/recovery/useSecurityQuestions.ts diff --git a/src/components/common/ConnectWallet/MPCWallet.tsx b/src/components/common/ConnectWallet/MPCWallet.tsx index a176751d14..86b5e05ca6 100644 --- a/src/components/common/ConnectWallet/MPCWallet.tsx +++ b/src/components/common/ConnectWallet/MPCWallet.tsx @@ -8,8 +8,6 @@ export const MPCWallet = () => { const { loginPending, triggerLogin, resetAccount, userInfo, walletState, recoverFactorWithPassword } = useContext(MpcWalletContext) - console.log(walletState) - return ( <> {userInfo.email ? ( diff --git a/src/components/settings/SignerAccountMFA/index.tsx b/src/components/settings/SignerAccountMFA/index.tsx index fae01a8191..ea3725fea2 100644 --- a/src/components/settings/SignerAccountMFA/index.tsx +++ b/src/components/settings/SignerAccountMFA/index.tsx @@ -4,10 +4,10 @@ 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 { useSecurityQuestions } from '@/hooks/wallets/mpc/recovery/useSecurityQuestions' +import { SecurityQuestionRecovery } from '@/hooks/wallets/mpc/recovery/SecurityQuestionRecovery' import useMFASettings from './useMFASettings' import { useForm } from 'react-hook-form' -import { useDeviceShare } from '@/hooks/wallets/mpc/recovery/useDeviceShare' +import { DeviceShareRecovery } from '@/hooks/wallets/mpc/recovery/DeviceShareRecovery' type SignerAccountFormData = { oldPassword: string | undefined @@ -19,8 +19,6 @@ type SignerAccountFormData = { const SignerAccountMFA = () => { const mpcCoreKit = useMPC() const mfaSettings = useMFASettings(mpcCoreKit) - const securityQuestions = useSecurityQuestions(mpcCoreKit) - const deviceShareModule = useDeviceShare(mpcCoreKit) const formMethods = useForm({ mode: 'all', @@ -30,19 +28,27 @@ const SignerAccountMFA = () => { const [enablingMFA, setEnablingMFA] = useState(false) - const isPasswordSet = useMemo(() => securityQuestions.isEnabled(), [securityQuestions]) - - console.log(mpcCoreKit) + const isPasswordSet = useMemo(() => { + if (!mpcCoreKit) { + return false + } + const securityQuestions = new SecurityQuestionRecovery(mpcCoreKit) + return securityQuestions.isEnabled() + }, [mpcCoreKit]) useEffect(() => { - deviceShareModule.isEnabled().then((value) => setValue('storeDeviceShare', value)) - }, [deviceShareModule, setValue]) + 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() @@ -63,16 +69,16 @@ const SignerAccountMFA = () => { await mpcCoreKit.deleteFactor(recoverPubKey, recoverKey) } - const hasDeviceShare = await deviceShareModule.isEnabled() + const hasDeviceShare = await deviceShareRecovery.isEnabled() if (!hasDeviceShare && storeDeviceShare) { - await deviceShareModule.createAndStoreDeviceFactor() + 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 deviceShareModule.removeDeviceFactor() + await deviceShareRecovery.removeDeviceFactor() } } catch (error) { console.error(error) @@ -90,20 +96,17 @@ const SignerAccountMFA = () => { } const onSubmit = async () => { - console.log('submitting') await enableMFA() } return (
- { - /* TODO: Memoize this*/ securityQuestions.isEnabled() ? ( - You already have a recovery password setup. - ) : ( - You have no password setup. Secure your account now! - ) - } + {isPasswordSet ? ( + You already have a recovery password setup. + ) : ( + You have no password setup. Secure your account now! + )} {isPasswordSet && ( { error={!!formState.errors['oldPassword']} helperText={formState.errors['oldPassword']?.message} {...register('oldPassword', { - required: securityQuestions.isEnabled(), + required: true, })} /> )} diff --git a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts index 0efea9dec0..d9e16fa7e5 100644 --- a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts +++ b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts @@ -2,7 +2,13 @@ import { act, renderHook, waitFor } from '@/tests/test-utils' import { MPCWalletState, useMPCWallet } from '../useMPCWallet' import * as useOnboard from '@/hooks/wallets/useOnboard' import { type OnboardAPI } from '@web3-onboard/core' -import { COREKIT_STATUS, type UserInfo, type OauthLoginParams, type Web3AuthMPCCoreKit } from '@web3auth/mpc-core-kit' +import { + COREKIT_STATUS, + type UserInfo, + type OauthLoginParams, + type Web3AuthMPCCoreKit, + type TssSecurityQuestion, +} from '@web3auth/mpc-core-kit' import * as mpcCoreKit from '@web3auth/mpc-core-kit' import { setMPCCoreKitInstance } from '../useMPC' import { ONBOARD_MPC_MODULE_LABEL } from '@/services/mpc/module' @@ -54,6 +60,7 @@ describe('useMPCWallet', () => { }) beforeEach(() => { jest.resetAllMocks() + setMPCCoreKitInstance(undefined) }) afterAll(() => { jest.useRealTimers() @@ -98,7 +105,9 @@ describe('useMPCWallet', () => { expect(result.current.walletState === MPCWalletState.AUTHENTICATING) expect(connectWalletSpy).not.toBeCalled() - jest.advanceTimersByTime(MOCK_LOGIN_TIME) + act(() => { + jest.advanceTimersByTime(MOCK_LOGIN_TIME) + }) await waitFor(() => { expect(result.current.walletState === MPCWalletState.READY) @@ -137,8 +146,9 @@ describe('useMPCWallet', () => { expect(result.current.walletState === MPCWalletState.AUTHENTICATING) expect(connectWalletSpy).not.toBeCalled() - - jest.advanceTimersByTime(MOCK_LOGIN_TIME) + act(() => { + jest.advanceTimersByTime(MOCK_LOGIN_TIME) + }) await waitFor(() => { expect(result.current.walletState === MPCWalletState.READY) @@ -150,6 +160,42 @@ describe('useMPCWallet', () => { }) }) }) + + it('should require manual share for MFA account without device share', async () => { + jest.spyOn(useOnboard, 'default').mockReturnValue({} as unknown as OnboardAPI) + const connectWalletSpy = jest.fn().mockImplementation(() => Promise.resolve()) + jest.spyOn(useOnboard, 'connectWallet').mockImplementation(connectWalletSpy) + setMPCCoreKitInstance( + new MockMPCCoreKit(COREKIT_STATUS.REQUIRED_SHARE, { + email: 'test@test.com', + name: 'Test', + } as unknown as UserInfo) as unknown as Web3AuthMPCCoreKit, + ) + + // TODO: remove unnecessary cast if mpc core sdk gets updated + jest.spyOn(mpcCoreKit, 'getWebBrowserFactor').mockReturnValue(Promise.resolve(undefined as unknown as string)) + jest.spyOn(mpcCoreKit, 'TssSecurityQuestion').mockReturnValue({ + getQuestion: () => 'SOME RANDOM QUESTION', + } as unknown as TssSecurityQuestion) + + const { result } = renderHook(() => useMPCWallet()) + + act(() => { + result.current.triggerLogin() + }) + + expect(result.current.walletState === MPCWalletState.AUTHENTICATING) + expect(connectWalletSpy).not.toBeCalled() + + act(() => { + jest.advanceTimersByTime(MOCK_LOGIN_TIME) + }) + + await waitFor(() => { + expect(result.current.walletState === MPCWalletState.MANUAL_RECOVERY) + expect(connectWalletSpy).not.toBeCalled() + }) + }) }) describe('resetAccount', () => { @@ -185,4 +231,66 @@ describe('useMPCWallet', () => { }) }) }) + + describe('recoverFactorWithPassword', () => { + it('should throw if mpcCoreKit is not initialized', () => { + const { result } = renderHook(() => useMPCWallet()) + expect(result.current.recoverFactorWithPassword('test', false)).rejects.toEqual( + new Error('MPC Core Kit is not initialized'), + ) + }) + + it('should not recover if wrong password is entered', () => { + setMPCCoreKitInstance({ + state: { + userInfo: undefined, + }, + } as unknown as Web3AuthMPCCoreKit) + const { result } = renderHook(() => useMPCWallet()) + jest.spyOn(mpcCoreKit, 'TssSecurityQuestion').mockReturnValue({ + getQuestion: () => 'SOME RANDOM QUESTION', + recoverFactor: () => { + throw new Error('Invalid answer') + }, + } as unknown as TssSecurityQuestion) + + expect(result.current.recoverFactorWithPassword('test', false)).rejects.toEqual(new Error('Invalid answer')) + }) + + it.only('should input recovered factor if correct password is entered', async () => { + const mockSecurityQuestionFactor = ethers.Wallet.createRandom().privateKey.slice(2) + const connectWalletSpy = jest.fn().mockImplementation(() => Promise.resolve()) + jest.spyOn(useOnboard, 'default').mockReturnValue({} as unknown as OnboardAPI) + jest.spyOn(useOnboard, 'connectWallet').mockImplementation(connectWalletSpy) + + setMPCCoreKitInstance( + new MockMPCCoreKit( + COREKIT_STATUS.REQUIRED_SHARE, + { + email: 'test@test.com', + name: 'Test', + } as unknown as UserInfo, + new BN(mockSecurityQuestionFactor, 'hex'), + ) as unknown as Web3AuthMPCCoreKit, + ) + + const { result } = renderHook(() => useMPCWallet()) + jest.spyOn(mpcCoreKit, 'TssSecurityQuestion').mockReturnValue({ + getQuestion: () => 'SOME RANDOM QUESTION', + recoverFactor: () => Promise.resolve(mockSecurityQuestionFactor), + } as unknown as TssSecurityQuestion) + + act(() => result.current.recoverFactorWithPassword('test', false)) + + await waitFor(() => { + expect(result.current.walletState === MPCWalletState.READY) + expect(connectWalletSpy).toBeCalledWith(expect.anything(), { + autoSelect: { + label: ONBOARD_MPC_MODULE_LABEL, + disableModals: true, + }, + }) + }) + }) + }) }) diff --git a/src/hooks/wallets/mpc/recovery/DeviceShareRecovery.ts b/src/hooks/wallets/mpc/recovery/DeviceShareRecovery.ts new file mode 100644 index 0000000000..afcf78964a --- /dev/null +++ b/src/hooks/wallets/mpc/recovery/DeviceShareRecovery.ts @@ -0,0 +1,45 @@ +import { + BrowserStorage, + getWebBrowserFactor, + storeWebBrowserFactor, + TssShareType, + type Web3AuthMPCCoreKit, +} from '@web3auth/mpc-core-kit' +import BN from 'bn.js' +import { getPubKeyPoint } from '@tkey-mpc/common-types' + +export class DeviceShareRecovery { + private mpcCoreKit: Web3AuthMPCCoreKit + + constructor(mpcCoreKit: Web3AuthMPCCoreKit) { + this.mpcCoreKit = mpcCoreKit + } + + async isEnabled() { + if (!this.mpcCoreKit.tKey.metadata) { + return false + } + return !!(await getWebBrowserFactor(this.mpcCoreKit)) + } + + async createAndStoreDeviceFactor() { + const userAgent = navigator.userAgent + + const deviceFactorKey = new BN( + await this.mpcCoreKit.createFactor({ shareType: TssShareType.DEVICE, additionalMetadata: { userAgent } }), + 'hex', + ) + await storeWebBrowserFactor(deviceFactorKey, this.mpcCoreKit) + } + + async removeDeviceFactor() { + const deviceFactor = await getWebBrowserFactor(this.mpcCoreKit) + const key = new BN(deviceFactor, 'hex') + const pubKey = getPubKeyPoint(key) + 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 new file mode 100644 index 0000000000..3224a07aa1 --- /dev/null +++ b/src/hooks/wallets/mpc/recovery/SecurityQuestionRecovery.ts @@ -0,0 +1,48 @@ +import { TssSecurityQuestion, TssShareType, type Web3AuthMPCCoreKit } from '@web3auth/mpc-core-kit' + +const DEFAULT_SECURITY_QUESTION = 'ENTER PASSWORD' + +export class SecurityQuestionRecovery { + private mpcCoreKit: Web3AuthMPCCoreKit + private securityQuestions = new TssSecurityQuestion() + + constructor(mpcCoreKit: Web3AuthMPCCoreKit) { + this.mpcCoreKit = mpcCoreKit + } + + isEnabled(): boolean { + try { + 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 + } + } + + async upsertPassword(newPassword: string, oldPassword?: string) { + if (this.isEnabled()) { + if (!oldPassword) { + throw Error('To change the password you need to provide the old password') + } + await this.securityQuestions.changeSecurityQuestion({ + answer: oldPassword, + mpcCoreKit: this.mpcCoreKit, + newAnswer: newPassword, + newQuestion: DEFAULT_SECURITY_QUESTION, + }) + } else { + await this.securityQuestions.setSecurityQuestion({ + question: DEFAULT_SECURITY_QUESTION, + answer: newPassword, + mpcCoreKit: this.mpcCoreKit, + shareType: TssShareType.DEVICE, + }) + } + } + + async recoverWithPassword(password: string) { + return this.securityQuestions.recoverFactor(this.mpcCoreKit, password) + } +} diff --git a/src/hooks/wallets/mpc/recovery/useDeviceShare.ts b/src/hooks/wallets/mpc/recovery/useDeviceShare.ts deleted file mode 100644 index 5ca317a096..0000000000 --- a/src/hooks/wallets/mpc/recovery/useDeviceShare.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { - BrowserStorage, - getWebBrowserFactor, - storeWebBrowserFactor, - TssShareType, - type Web3AuthMPCCoreKit, -} from '@web3auth/mpc-core-kit' -import BN from 'bn.js' -import { getPubKeyPoint } from '@tkey-mpc/common-types' - -export const useDeviceShare = (mpcCoreKit: Web3AuthMPCCoreKit | undefined) => { - const isEnabled = async () => { - if (!mpcCoreKit || !mpcCoreKit.tKey.metadata) { - return false - } - return !!(await getWebBrowserFactor(mpcCoreKit)) - } - - const createAndStoreDeviceFactor = async () => { - if (!mpcCoreKit) { - return - } - const userAgent = navigator.userAgent - - const deviceFactorKey = new BN( - await mpcCoreKit.createFactor({ shareType: TssShareType.DEVICE, additionalMetadata: { userAgent } }), - 'hex', - ) - await storeWebBrowserFactor(deviceFactorKey, mpcCoreKit) - } - - const removeDeviceFactor = async () => { - if (!mpcCoreKit) { - return - } - const deviceFactor = await getWebBrowserFactor(mpcCoreKit) - const key = new BN(deviceFactor, 'hex') - const pubKey = getPubKeyPoint(key) - const pubKeyX = pubKey.x.toString('hex', 64) - await mpcCoreKit.deleteFactor(pubKey) - const currentStorage = BrowserStorage.getInstance('mpc_corekit_store') - debugger - currentStorage.set(pubKeyX, undefined) - } - - return { - isEnabled, - createAndStoreDeviceFactor, - removeDeviceFactor, - } -} diff --git a/src/hooks/wallets/mpc/recovery/useSecurityQuestions.ts b/src/hooks/wallets/mpc/recovery/useSecurityQuestions.ts deleted file mode 100644 index 9e90b8a10b..0000000000 --- a/src/hooks/wallets/mpc/recovery/useSecurityQuestions.ts +++ /dev/null @@ -1,58 +0,0 @@ -import { TssSecurityQuestion, TssShareType, type Web3AuthMPCCoreKit } from '@web3auth/mpc-core-kit' - -const DEFAULT_SECURITY_QUESTION = 'ENTER PASSWORD' - -const securityQuestions = new TssSecurityQuestion() - -export const useSecurityQuestions = (mpcCoreKit: Web3AuthMPCCoreKit | undefined) => { - const isEnabled = () => { - if (!mpcCoreKit) { - return false - } - try { - const question = securityQuestions.getQuestion(mpcCoreKit) - return !!question - } catch (error) { - console.error(error) - // It errors out if recovery is not setup currently - return false - } - } - - const upsertPassword = async (newPassword: string, oldPassword?: string) => { - if (!mpcCoreKit) { - return - } - if (isEnabled()) { - if (!oldPassword) { - throw Error('To change the password you need to provide the old password') - } - await securityQuestions.changeSecurityQuestion({ - answer: oldPassword, - mpcCoreKit, - newAnswer: newPassword, - newQuestion: DEFAULT_SECURITY_QUESTION, - }) - } else { - await securityQuestions.setSecurityQuestion({ - question: DEFAULT_SECURITY_QUESTION, - answer: newPassword, - mpcCoreKit, - shareType: TssShareType.DEVICE, - }) - } - } - - const recoverWithPassword = async (password: string) => { - if (!mpcCoreKit) { - return - } - return securityQuestions.recoverFactor(mpcCoreKit, password) - } - - return { - isEnabled, - upsertPassword, - recoverWithPassword, - } -} diff --git a/src/hooks/wallets/mpc/useMPCWallet.ts b/src/hooks/wallets/mpc/useMPCWallet.ts index e4fb5ee68c..319643c818 100644 --- a/src/hooks/wallets/mpc/useMPCWallet.ts +++ b/src/hooks/wallets/mpc/useMPCWallet.ts @@ -5,8 +5,8 @@ import { GOOGLE_CLIENT_ID, WEB3AUTH_VERIFIER_ID } from '@/config/constants' import { COREKIT_STATUS, getWebBrowserFactor } from '@web3auth/mpc-core-kit' import useOnboard, { connectWallet } from '../useOnboard' import { ONBOARD_MPC_MODULE_LABEL } from '@/services/mpc/module' -import { useSecurityQuestions } from './recovery/useSecurityQuestions' -import { useDeviceShare } from './recovery/useDeviceShare' +import { SecurityQuestionRecovery } from './recovery/SecurityQuestionRecovery' +import { DeviceShareRecovery } from './recovery/DeviceShareRecovery' export enum MPCWalletState { NOT_INITIALIZED, @@ -31,14 +31,12 @@ export const useMPCWallet = (): MPCWalletHook => { const [walletState, setWalletState] = useState(MPCWalletState.NOT_INITIALIZED) const mpcCoreKit = useMPC() const onboard = useOnboard() - const securityQuestions = useSecurityQuestions(mpcCoreKit) - const deviceShareModule = useDeviceShare(mpcCoreKit) const isMFAEnabled = () => { if (!mpcCoreKit) { return false } - const { shareDescriptions } = mpcCoreKit?.getKeyDetails() + const { shareDescriptions } = mpcCoreKit.getKeyDetails() return !Object.entries(shareDescriptions).some(([key, value]) => value[0]?.includes('hashedShare')) } @@ -85,6 +83,7 @@ export const useMPCWallet = (): MPCWalletHook => { await mpcCoreKit.inputFactorKey(deviceFactorKey) } else { // Check password recovery + const securityQuestions = new SecurityQuestionRecovery(mpcCoreKit) if (securityQuestions.isEnabled()) { setWalletState(MPCWalletState.MANUAL_RECOVERY) return @@ -115,16 +114,20 @@ export const useMPCWallet = (): MPCWalletHook => { } const recoverFactorWithPassword = async (password: string, storeDeviceShare: boolean = false) => { - if (mpcCoreKit && securityQuestions.isEnabled()) { + if (!mpcCoreKit) { + throw new Error('MPC Core Kit is not initialized') + } + + const securityQuestions = new SecurityQuestionRecovery(mpcCoreKit) + + if (securityQuestions.isEnabled()) { const factorKeyString = await securityQuestions.recoverWithPassword(password) - if (!factorKeyString) { - throw new Error('The password is invalid') - } const factorKey = new BN(factorKeyString, 'hex') await mpcCoreKit.inputFactorKey(factorKey) if (storeDeviceShare) { - await deviceShareModule.createAndStoreDeviceFactor() + const deviceShareRecovery = new DeviceShareRecovery(mpcCoreKit) + await deviceShareRecovery.createAndStoreDeviceFactor() } finalizeLogin() From 14e487261b8ce473252fe6e1c502dc330b5516c6 Mon Sep 17 00:00:00 2001 From: schmanu Date: Wed, 4 Oct 2023 14:27:52 +0200 Subject: [PATCH 6/9] docs: comments in useMPCWallet test --- src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts index d9e16fa7e5..09b7003153 100644 --- a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts +++ b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts @@ -15,7 +15,12 @@ import { ONBOARD_MPC_MODULE_LABEL } from '@/services/mpc/module' import { ethers } from 'ethers' import BN from 'bn.js' +/** time until mock login resolves */ const MOCK_LOGIN_TIME = 1000 + +/** + * Helper class for mocking MPC Core Kit login flow + */ class MockMPCCoreKit { status: COREKIT_STATUS = COREKIT_STATUS.INITIALIZED state: { @@ -27,6 +32,12 @@ class MockMPCCoreKit { private stateAfterLogin: COREKIT_STATUS private userInfoAfterLogin: UserInfo | undefined private expectedFactorKey: BN + /** + * + * @param stateAfterLogin State after loginWithOauth resolves + * @param userInfoAfterLogin User info to set in the state after loginWithOauth resolves + * @param expectedFactorKey For MFA login flow the expected factor key. If inputFactorKey gets called with the expected factor key the state switches to logged in + */ constructor(stateAfterLogin: COREKIT_STATUS, userInfoAfterLogin: UserInfo, expectedFactorKey: BN = new BN(-1)) { this.stateAfterLogin = stateAfterLogin this.userInfoAfterLogin = userInfoAfterLogin From c2fa020eb96f7d2c5064beea3f1fa2589f514d00 Mon Sep 17 00:00:00 2001 From: schmanu Date: Wed, 4 Oct 2023 14:30:18 +0200 Subject: [PATCH 7/9] tests: remove .only --- src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts index 09b7003153..4f7e8fd951 100644 --- a/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts +++ b/src/hooks/wallets/mpc/__tests__/useMPCWallet.test.ts @@ -268,7 +268,7 @@ describe('useMPCWallet', () => { expect(result.current.recoverFactorWithPassword('test', false)).rejects.toEqual(new Error('Invalid answer')) }) - it.only('should input recovered factor if correct password is entered', async () => { + it('should input recovered factor if correct password is entered', async () => { const mockSecurityQuestionFactor = ethers.Wallet.createRandom().privateKey.slice(2) const connectWalletSpy = jest.fn().mockImplementation(() => Promise.resolve()) jest.spyOn(useOnboard, 'default').mockReturnValue({} as unknown as OnboardAPI) From 7359ae78ad109049eef9e23779e34236fa79b314 Mon Sep 17 00:00:00 2001 From: schmanu Date: Wed, 4 Oct 2023 17:41:40 +0200 Subject: [PATCH 8/9] fix: address review issues --- .../common/ConnectWallet/PasswordRecovery.tsx | 2 +- .../create/steps/ConnectWalletStep/index.tsx | 44 ++--- .../SignerAccountMFA/PasswordForm.tsx | 124 ++++++++++++++ .../settings/SignerAccountMFA/helper.ts | 68 ++++++++ .../settings/SignerAccountMFA/index.tsx | 152 +----------------- .../SignerAccountMFA/useMFASettings.ts | 21 --- .../sidebar/SidebarNavigation/config.tsx | 2 +- .../mpc/__tests__/useMPCWallet.test.ts | 12 +- .../mpc/recovery/DeviceShareRecovery.ts | 1 - .../mpc/recovery/SecurityQuestionRecovery.ts | 10 +- src/hooks/wallets/mpc/useMPCWallet.ts | 11 -- src/pages/settings/signer-account.tsx | 2 +- src/services/exceptions/ErrorCodes.ts | 1 + 13 files changed, 228 insertions(+), 222 deletions(-) create mode 100644 src/components/settings/SignerAccountMFA/PasswordForm.tsx create mode 100644 src/components/settings/SignerAccountMFA/helper.ts delete mode 100644 src/components/settings/SignerAccountMFA/useMFASettings.ts 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', From af730b570fb6b58ae62ffed9c58e64115d39f942 Mon Sep 17 00:00:00 2001 From: schmanu Date: Thu, 5 Oct 2023 09:34:14 +0200 Subject: [PATCH 9/9] fix: review follow ups --- src/components/settings/SignerAccountMFA/PasswordForm.tsx | 8 ++++---- src/components/settings/SignerAccountMFA/helper.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/settings/SignerAccountMFA/PasswordForm.tsx b/src/components/settings/SignerAccountMFA/PasswordForm.tsx index 74b3d17616..0f4d905974 100644 --- a/src/components/settings/SignerAccountMFA/PasswordForm.tsx +++ b/src/components/settings/SignerAccountMFA/PasswordForm.tsx @@ -26,10 +26,10 @@ export const PasswordForm = ({ mpcCoreKit }: { mpcCoreKit: Web3AuthMPCCoreKit }) defaultValues: async () => { const isDeviceShareStored = await new DeviceShareRecovery(mpcCoreKit).isEnabled() return { - confirmPassword: '', - oldPassword: undefined, - newPassword: '', - storeDeviceShare: isDeviceShareStored, + [PasswordFieldNames.confirmPassword]: '', + [PasswordFieldNames.oldPassword]: undefined, + [PasswordFieldNames.newPassword]: '', + [PasswordFieldNames.storeDeviceShare]: isDeviceShareStored, } }, }) diff --git a/src/components/settings/SignerAccountMFA/helper.ts b/src/components/settings/SignerAccountMFA/helper.ts index 84595ab857..9984df01cf 100644 --- a/src/components/settings/SignerAccountMFA/helper.ts +++ b/src/components/settings/SignerAccountMFA/helper.ts @@ -11,8 +11,8 @@ export const isMFAEnabled = (mpcCoreKit: Web3AuthMPCCoreKit) => { if (!mpcCoreKit) { return false } - const { shareDescriptions } = mpcCoreKit?.getKeyDetails() - return !Object.entries(shareDescriptions).some((value) => value[0]?.includes('hashedShare')) + const { shareDescriptions } = mpcCoreKit.getKeyDetails() + return !Object.values(shareDescriptions).some((value) => value[0]?.includes('hashedShare')) } export const enableMFA = async (