Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Implement design review feedback, restructure components #2683

Merged
merged 1 commit into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/common/ConnectWallet/PasswordRecovery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
import { useState } from 'react'
import Track from '../Track'
import { FormProvider, useForm } from 'react-hook-form'
import PasswordInput from '@/components/settings/SignerAccountMFA/PasswordInput'
import PasswordInput from '@/components/settings/SecurityLogin/SocialSignerMFA/PasswordInput'

type PasswordFormData = {
password: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Box, Button, DialogContent, DialogTitle, IconButton, TextField, Typogra
import { useContext, useState } from 'react'
import { useForm } from 'react-hook-form'
import { Visibility, VisibilityOff, Close } from '@mui/icons-material'
import css from './styles.module.css'
import css from '@/components/settings/SecurityLogin/SocialSignerExport/styles.module.css'
import ErrorCodes from '@/services/exceptions/ErrorCodes'
import { logError } from '@/services/exceptions'
import ErrorMessage from '@/components/tx/ErrorMessage'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import { Alert, Box, Button, Typography } from '@mui/material'
import { useState } from 'react'
import ExportMPCAccountModal from './ExportMPCAccountModal'
import ExportMPCAccountModal from '@/components/settings/SecurityLogin/SocialSignerExport/ExportMPCAccountModal'

const ExportMPCAccount = () => {
const SocialSignerExport = () => {
const [isModalOpen, setIsModalOpen] = useState(false)

return (
<>
<Box display="flex" flexDirection="column" gap={2} alignItems="flex-start">
<Typography>
Accounts created via Google can be exported and imported to any non-custodial wallet outside of Safe.
Signers created via Google can be exported and imported to any non-custodial wallet outside of Safe.
</Typography>
<Alert severity="warning">
Never disclose your keys or seed phrase to anyone. If someone gains access to them, they have full access over
your signer account.
your social login signer.
</Alert>
<Button color="primary" variant="contained" disabled={isModalOpen} onClick={() => setIsModalOpen(true)}>
Reveal private key
Expand All @@ -24,4 +24,4 @@ const ExportMPCAccount = () => {
)
}

export default ExportMPCAccount
export default SocialSignerExport
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { _getPasswordStrength, PasswordStrength } from '@/components/settings/SignerAccountMFA/PasswordForm'
import { _getPasswordStrength, PasswordStrength } from '@/components/settings/SecurityLogin/SocialSignerMFA/index'

describe('_getPasswordStrength', () => {
it('should return weak if the value has fewer than 9 characters', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,17 @@
import { MPC_WALLET_EVENTS } from '@/services/analytics/events/mpcWallet'
import { useState, useMemo, type ChangeEvent } from 'react'
import { FormProvider, useForm } from 'react-hook-form'
import { enableMFA } from './helper'
import { enableMFA } from '@/components/settings/SecurityLogin/SocialSignerMFA/helper'
import CheckIcon from '@/public/images/common/check-filled.svg'
import LockWarningIcon from '@/public/images/common/lock-warning.svg'
import PasswordInput from '@/components/settings/SignerAccountMFA/PasswordInput'
import css from './styles.module.css'
import PasswordInput from '@/components/settings/SecurityLogin/SocialSignerMFA/PasswordInput'
import css from '@/components/settings/SecurityLogin/SocialSignerMFA/styles.module.css'
import BarChartIcon from '@/public/images/common/bar-chart.svg'
import ShieldIcon from '@/public/images/common/shield.svg'
import ShieldOffIcon from '@/public/images/common/shield-off.svg'
import useMPC from '@/hooks/wallets/mpc/useMPC'
import { useAppDispatch } from '@/store'
import ExpandMoreIcon from '@mui/icons-material/ExpandMore'

enum PasswordFieldNames {
oldPassword = 'oldPassword',
Expand Down Expand Up @@ -76,7 +77,7 @@
},
} as const

export const PasswordForm = () => {
const SocialSignerMFA = () => {

Check warning on line 80 in src/components/settings/SecurityLogin/SocialSignerMFA/index.tsx

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🕹️ Function is not covered

Warning! Not covered function
const dispatch = useAppDispatch()
const mpcCoreKit = useMPC()
const [passwordStrength, setPasswordStrength] = useState<PasswordStrength>(PasswordStrength.weak)
Expand Down Expand Up @@ -110,7 +111,8 @@
setPasswordStrength(PasswordStrength.weak)
}

const passwordsMatch = watch(PasswordFieldNames.newPassword) === watch(PasswordFieldNames.confirmPassword)
const confirmPassword = watch(PasswordFieldNames.confirmPassword)

Check warning on line 114 in src/components/settings/SecurityLogin/SocialSignerMFA/index.tsx

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🧾 Statement is not covered

Warning! Not covered statement
const passwordsMatch = watch(PasswordFieldNames.newPassword) === confirmPassword && confirmPassword !== ''

Check warning on line 115 in src/components/settings/SecurityLogin/SocialSignerMFA/index.tsx

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🧾 Statement is not covered

Warning! Not covered statement

Check warning on line 115 in src/components/settings/SecurityLogin/SocialSignerMFA/index.tsx

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered

Warning! Not covered branch

Check warning on line 115 in src/components/settings/SecurityLogin/SocialSignerMFA/index.tsx

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered

Warning! Not covered branch

const isSubmitDisabled =
!passwordsMatch ||
Expand All @@ -123,11 +125,11 @@
<form onSubmit={handleSubmit(onSubmit)}>
<Box display="flex" flexDirection="column" gap={3} alignItems="baseline">
<Typography>
Protect your account with a password. It will be used to restore access to your Safe in another browser or
on another device.
Protect your social login signer with a password. It will be used to restore access in another browser or on
another device.
</Typography>
<Accordion defaultExpanded={isPasswordSet}>
<AccordionSummary>
<Accordion>
<AccordionSummary expandIcon={<ExpandMoreIcon />}>
<Box display="flex" alignItems="center" gap={1}>
<SvgIcon component={CheckIcon} sx={{ color: isPasswordSet ? 'success.main' : 'border.light' }} />
<Typography fontWeight="bold">Password</Typography>
Expand All @@ -138,7 +140,7 @@
<Grid item container xs={12} md={7} gap={3} p={3}>
{isPasswordSet && (
<>
<Typography>You already have a recovery password setup.</Typography>
<Typography>You already have a password setup.</Typography>
<FormControl fullWidth>
<PasswordInput
name={PasswordFieldNames.oldPassword}
Expand Down Expand Up @@ -178,7 +180,7 @@
{passwordStrengthMap[passwordStrength].label} password
</Typography>
<Typography variant="body2" color="text.secondary" mt={1}>
Include at least 8 or more characters, a number, an uppercase letter and a symbol
Include at least 9 or more characters, a number, an uppercase letter and a symbol
</Typography>
</FormControl>

Expand Down Expand Up @@ -232,8 +234,8 @@
</Typography>
<ol className={css.list}>
<Typography component="li" variant="body2">
You will have to input this password if you need to recover access to Safe in another browser or
on another device.
You will have to input this password if you login with this social login signer in another
browser or on another device.
</Typography>
<Typography component="li" variant="body2">
We suggest to use a password manager or to write the password down on a piece of paper and
Expand All @@ -250,3 +252,5 @@
</FormProvider>
)
}

export default SocialSignerMFA

Check warning on line 256 in src/components/settings/SecurityLogin/SocialSignerMFA/index.tsx

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🧾 Statement is not covered

Warning! Not covered statement
51 changes: 51 additions & 0 deletions src/components/settings/SecurityLogin/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { Grid, Paper, Typography } from '@mui/material'
import SocialSignerMFA from '@/components/settings/SecurityLogin/SocialSignerMFA'
import SocialSignerExport from '@/components/settings/SecurityLogin/SocialSignerExport'
import useWallet from '@/hooks/wallets/useWallet'
import { isSocialLoginWallet } from '@/services/mpc/module'

const SecurityLogin = () => {
const wallet = useWallet()

const isSocialLogin = isSocialLoginWallet(wallet?.label)

if (!isSocialLogin) {
return (
<Paper sx={{ p: 4 }}>
<Typography>You are currently not logged in with a social login signer.</Typography>
</Paper>
)
}

return (
<>
<Paper sx={{ p: 4 }}>
<Grid container spacing={3}>
<Grid item lg={4} xs={12}>
<Typography variant="h4" fontWeight="bold" mb={1}>
Multi-factor Authentication
</Typography>
</Grid>

<Grid item xs>
<SocialSignerMFA />
</Grid>
</Grid>
</Paper>
<Paper sx={{ p: 4, mt: 2 }}>
<Grid container spacing={3}>
<Grid item lg={4} xs={12}>
<Typography variant="h4" fontWeight="bold" mb={1}>
Social login signer export
</Typography>
</Grid>
<Grid item xs>
<SocialSignerExport />
</Grid>
</Grid>
</Paper>
</>
)
}

export default SecurityLogin
23 changes: 0 additions & 23 deletions src/components/settings/SignerAccountMFA/index.tsx

This file was deleted.

8 changes: 4 additions & 4 deletions src/components/sidebar/SidebarNavigation/config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ export const settingsNavItems = [
label: 'Spending limits',
href: AppRoutes.settings.spendingLimits,
},
{
label: 'Security & Login',
href: AppRoutes.settings.securityLogin,
},
{
label: 'Safe Apps',
href: AppRoutes.settings.safeApps.index,
Expand All @@ -108,10 +112,6 @@ export const settingsNavItems = [
label: 'Environment variables',
href: AppRoutes.settings.environmentVariables,
},
{
label: 'Signer account',
href: AppRoutes.settings.signerAccount,
},
]

export const generalSettingsNavItems = [
Expand Down
2 changes: 1 addition & 1 deletion src/components/welcome/WelcomeLogin/WalletLogin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const WalletLogin = ({ onLogin }: { onLogin: () => void }) => {

const isSocialLogin = isSocialLoginWallet(wallet?.label)

if (wallet !== null && isSocialLogin) {
if (wallet !== null && !isSocialLogin) {
return (
<Box sx={{ width: '100%' }}>
<Track {...CREATE_SAFE_EVENTS.CONTINUE_TO_CREATION} label={wallet.label}>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { act, render, waitFor } from '@/tests/test-utils'
import * as useConnectWallet from '@/components/common/ConnectWallet/useConnectWallet'
import * as useWallet from '@/hooks/wallets/useWallet'
import * as mpcModule from '@/services/mpc/module'
import WalletLogin from '../WalletLogin'
import { hexZeroPad } from '@ethersproject/bytes'
import { type EIP1193Provider } from '@web3-onboard/common'
Expand All @@ -21,7 +20,6 @@ describe('WalletLogin', () => {
label: 'MetaMask',
provider: {} as unknown as EIP1193Provider,
})
jest.spyOn(mpcModule, 'isSocialLoginWallet').mockReturnValue(true)
jest.spyOn(useConnectWallet, 'default').mockReturnValue(jest.fn())

const result = render(<WalletLogin onLogin={mockOnLogin} />)
Expand Down
4 changes: 2 additions & 2 deletions src/config/routes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export const AppRoutes = {
'404': '/404',
_offline: '/_offline',
welcome: '/welcome',
terms: '/terms',
privacy: '/privacy',
Expand All @@ -11,6 +10,7 @@ export const AppRoutes = {
cookie: '/cookie',
addressBook: '/address-book',
addOwner: '/addOwner',
_offline: '/_offline',
apps: {
open: '/apps/open',
index: '/apps',
Expand All @@ -27,8 +27,8 @@ export const AppRoutes = {
},
settings: {
spendingLimits: '/settings/spending-limits',
signerAccount: '/settings/signer-account',
setup: '/settings/setup',
securityLogin: '/settings/security-login',
notifications: '/settings/notifications',
modules: '/settings/modules',
index: '/settings',
Expand Down
23 changes: 23 additions & 0 deletions src/pages/settings/security-login.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import type { NextPage } from 'next'
import Head from 'next/head'

import SettingsHeader from '@/components/settings/SettingsHeader'
import SecurityLogin from '@/components/settings/SecurityLogin'

const SecurityLoginPage: NextPage = () => {
return (
<>
<Head>
<title>{'Safe{Wallet} – Settings – Security & Login'}</title>
</Head>

<SettingsHeader />

<main>
<SecurityLogin />
</main>
</>
)
}

export default SecurityLoginPage
50 changes: 0 additions & 50 deletions src/pages/settings/signer-account.tsx

This file was deleted.

Loading