diff --git a/packages/extension-ui/src/Popup/Derive/DerivationPath.tsx b/packages/extension-ui/src/Popup/Derive/DerivationPath.tsx index c1d223ff..4838d56b 100644 --- a/packages/extension-ui/src/Popup/Derive/DerivationPath.tsx +++ b/packages/extension-ui/src/Popup/Derive/DerivationPath.tsx @@ -18,9 +18,10 @@ interface Props extends ThemeProps{ onChange: (suri: string) => void; parentAddress: string; parentPassword: string; + withSoftPath: boolean; } -function DerivationPath ({ className, defaultPath, isError, onChange }: Props): React.ReactElement { +function DerivationPath ({ className, defaultPath, isError, onChange, withSoftPath }: Props): React.ReactElement { const { t } = useTranslation(); const [path, setPath] = useState(defaultPath); const [isDisabled, setIsDisabled] = useState(true); @@ -46,7 +47,10 @@ function DerivationPath ({ className, defaultPath, isError, onChange }: Props): : t('Derivation Path') } onChange={_onChange} - placeholder={t('//hard/soft')} + placeholder={withSoftPath + ? t('//hard/soft') + : t('//hard') + } value={path} /> diff --git a/packages/extension-ui/src/Popup/Derive/Derive.test.tsx b/packages/extension-ui/src/Popup/Derive/Derive.test.tsx index a434ce1e..14e25787 100644 --- a/packages/extension-ui/src/Popup/Derive/Derive.test.tsx +++ b/packages/extension-ui/src/Popup/Derive/Derive.test.tsx @@ -33,19 +33,19 @@ const accounts = [ { address: '5D2TPhGEy2FhznvzaNYW9AkuMBbg3cyRemnPsBvBY4ZhkZXA', name: 'BB', parentAddress: '5GYmFzQCuC5u3tQNiMZNbFGakrz3Jq31NmMg4D2QAkSoQ2g5', type: 'sr25519' }, { address: '5GhGENSJBWQZ8d8mARKgqEkiAxiW3hHeznQDW2iG4XzNieb6', isExternal: true, name: 'C', type: 'sr25519' }, { address: '0xd5D81CD4236a43F48A983fc5B895975c511f634D', name: 'Ethereum', type: 'ethereum' }, - { address: '5EeaoDj4VDk8V6yQngKBaCD5MpJUCHrhYjVhBjgMHXoYon1s', isExternal: false, name: 'D', type: 'sr25519' }, - { address: '5HRKYp5anSNGtqC7cq9ftiaq4y8Mk7uHk7keaXUrQwZqDWLJ', name: 'DD', parentAddress: '5EeaoDj4VDk8V6yQngKBaCD5MpJUCHrhYjVhBjgMHXoYon1s', type: 'sr25519' } + { address: '5EeaoDj4VDk8V6yQngKBaCD5MpJUCHrhYjVhBjgMHXoYon1s', isExternal: false, name: 'D', type: 'ed25519' }, + { address: '5HRKYp5anSNGtqC7cq9ftiaq4y8Mk7uHk7keaXUrQwZqDWLJ', name: 'DD', parentAddress: '5EeaoDj4VDk8V6yQngKBaCD5MpJUCHrhYjVhBjgMHXoYon1s', type: 'ed25519' } ] as AccountJson[]; describe('Derive', () => { - const mountComponent = async (locked = false): Promise<{ + const mountComponent = async (locked = false, account = 1): Promise<{ wrapper: ReactWrapper; onActionStub: jest.Mock; }> => { const onActionStub = jest.fn(); const wrapper = mount( - + { return { address: derivedAddress, suri: defaultDerivation } as ResponseDeriveValidate; }); - it('Button is disabled and password field visible', () => { + it('Button is disabled and password field visible, path field is hidden', () => { const button = wrapper.find('[data-button-action="create derived account"] button'); expect(button.exists()).toBe(true); expect(button.prop('disabled')).toBe(true); + expect(wrapper.find('.pathInput').exists()).toBe(false); }); it('Password field is visible and not in error state', () => { @@ -154,6 +155,30 @@ describe('Derive', () => { expect(wrapper.find('.warning-message')).toHaveLength(0); }); + it('Derivation path gets visible, is set and locked', async () => { + await type(wrapper.find('input[type="password"]'), 'wrong_pass'); + + expect(wrapper.find('.pathInput.locked input').prop('disabled')).toBe(true); + expect(wrapper.find('.pathInput.locked input').prop('value')).toBe('//1'); + }); + + it('Derivation path can be unlocked', async () => { + await type(wrapper.find('input[type="password"]'), 'wrong_pass'); + wrapper.find('FontAwesomeIcon.lockIcon').simulate('click'); + await act(flushAllPromises); + wrapper.update(); + + expect(wrapper.find('.pathInput').exists()).toBe(true); + expect(wrapper.find('.pathInput input').prop('disabled')).toBe(false); + }); + + it('Derivation path placeholder contains //hard/soft', async () => { + await type(wrapper.find('input[type="password"]'), parentPassword); + const pathInput = wrapper.find('[data-input-suri] input'); + + expect(pathInput.first().prop('placeholder')).toEqual('//hard/soft'); + }); + it('An error is visible and the button is disabled when suri is incorrect', async () => { await type(wrapper.find('input[type="password"]'), parentPassword); await type(wrapper.find('[data-input-suri] input'), '//'); @@ -165,7 +190,29 @@ describe('Derive', () => { expect(button.prop('disabled')).toBe(true); expect(wrapper.find('.warning-message')).toHaveLength(1); - expect(wrapper.find('.warning-message').first().text()).toEqual('Incorrect derivation path'); + expect(wrapper.find('.warning-message').first().text()).toEqual('Invalid derivation path'); + }); + + it('An error is visible and the button is disabled when suri contains `///`', async () => { + await type(wrapper.find('input[type="password"]'), parentPassword); + await type(wrapper.find('[data-input-suri] input'), '///'); + + const button = wrapper.find('[data-button-action="create derived account"] button'); + + expect(button.prop('disabled')).toBe(true); + expect(wrapper.find('.warning-message')).toHaveLength(1); + // eslint-disable-next-line quotes + expect(wrapper.find('.warning-message').first().text()).toEqual("`///password` not supported for derivation"); + }); + + it('No error is shown when suri contains soft derivation `/` with sr25519', async () => { + await type(wrapper.find('input[type="password"]'), parentPassword); + await type(wrapper.find('[data-input-suri] input'), '//somehard/soft'); + + const button = wrapper.find('[data-button-action="create derived account"] button'); + + expect(button.prop('disabled')).toBe(false); + expect(wrapper.find('.warning-message')).toHaveLength(0); }); it('The error disappears and "Create derived account" is enabled when typing a new suri', async () => { @@ -228,8 +275,6 @@ describe('Derive', () => { await act(flushAllPromises); wrapper.update(); await enterName(newAccount.name).then(password(newAccount.password)).then(repeat(newAccount.password)); - await act(flushAllPromises); - wrapper.update(); wrapper.find('[data-button-action="add new root"] button').simulate('click'); await act(flushAllPromises); wrapper.update(); @@ -239,4 +284,33 @@ describe('Derive', () => { }); }); }); + + describe('Ed25519 Parent', () => { + beforeEach(async () => { + const mountedComponent = await mountComponent(false, 5); + + wrapper = mountedComponent.wrapper; + onActionStub = mountedComponent.onActionStub; + await type(wrapper.find('input[type="password"]'), parentPassword); + }); + + it('Derivation path placeholder only contains //hard', () => { + const pathInput = wrapper.find('[data-input-suri] input'); + + expect(pathInput.first().prop('placeholder')).toEqual('//hard'); + }); + + it('An error is shown when suri contains soft derivation `/` with ed25519', async () => { + const pathInput = wrapper.find('[data-input-suri] input'); + + await type(pathInput, '//somehard/soft'); + + const button = wrapper.find('[data-button-action="create derived account"] button'); + + expect(button.prop('disabled')).toBe(true); + expect(wrapper.find('[data-input-suri]').first().prop('isError')).toBe(true); + expect(wrapper.find('.warning-message')).toHaveLength(1); + expect(wrapper.find('.warning-message').first().text()).toEqual('Soft derivation is only allowed for sr25519 accounts'); + }); + }); }); diff --git a/packages/extension-ui/src/Popup/Derive/SelectParent.tsx b/packages/extension-ui/src/Popup/Derive/SelectParent.tsx index 6ef92761..a198a221 100644 --- a/packages/extension-ui/src/Popup/Derive/SelectParent.tsx +++ b/packages/extension-ui/src/Popup/Derive/SelectParent.tsx @@ -4,7 +4,6 @@ import React, { useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react'; import { canDerive } from '@polkadot/extension-base/utils'; -import { assert } from '@polkadot/util'; import { AccountContext, ActionContext, Address, ButtonArea, InputWithLabel, Label, NextStepButton, VerticalSpace, Warning } from '../../components'; import useTranslation from '../../hooks/useTranslation'; @@ -21,6 +20,9 @@ interface Props { onDerivationConfirmed: (derivation: { account: { address: string; suri: string }; parentPassword: string }) => void; } +// match any single slash +const singleSlashRegex = /([^/]|^)\/([^/]|$)/; + export default function SelectParent ({ className, isLocked, onDerivationConfirmed, parentAddress, parentGenesis }: Props): React.ReactElement { const { t } = useTranslation(); const onAction = useContext(ActionContext); @@ -30,7 +32,24 @@ export default function SelectParent ({ className, isLocked, onDerivationConfirm const [suriPath, setSuriPath] = useState(defaultPath); const [parentPassword, setParentPassword] = useState(''); const [isProperParentPassword, setIsProperParentPassword] = useState(false); + const [pathError, setPathError] = useState(''); const passwordInputRef = useRef(null); + const allowSoftDerivation = useMemo(() => { + const parent = accounts.find(({ address }) => address === parentAddress); + + return parent?.type === 'sr25519'; + }, [accounts, parentAddress]); + + useEffect(() => { + // forbid the use of password since Keyring ignores it + if (suriPath?.includes('///')) { + setPathError(t('`///password` not supported for derivation')); + } + + if (!allowSoftDerivation && suriPath && singleSlashRegex.test(suriPath)) { + setPathError(t('Soft derivation is only allowed for sr25519 accounts')); + } + }, [allowSoftDerivation, suriPath, t]); const allAddresses = useMemo( () => hierarchy @@ -48,6 +67,14 @@ export default function SelectParent ({ className, isLocked, onDerivationConfirm [] ); + const _onSuriPathChange = useCallback( + (path: string): void => { + setSuriPath(path); + setPathError(''); + }, + [] + ); + const _onParentChange = useCallback( (address: string) => onAction(`/account/derive/${address}`), [onAction] @@ -64,11 +91,11 @@ export default function SelectParent ({ className, isLocked, onDerivationConfirm try { const account = await validateDerivationPath(parentAddress, suriPath, parentPassword); - assert(account, 'Unable to derive'); onDerivationConfirmed({ account, parentPassword }); } catch (error) { setIsBusy(false); - setSuriPath(null); + setPathError(t('Invalid derivation path')); + console.error(error); } } else { setIsBusy(false); @@ -76,7 +103,7 @@ export default function SelectParent ({ className, isLocked, onDerivationConfirm } } }, - [parentAddress, parentPassword, onDerivationConfirmed, suriPath] + [parentAddress, parentPassword, onDerivationConfirmed, suriPath, t] ); useEffect(() => { @@ -89,71 +116,70 @@ export default function SelectParent ({ className, isLocked, onDerivationConfirm return ( <>
-
- {isLocked - ? ( -
- ) - : ( - - ) - } -
- ('enter the password for the account you want to derive from')} - onChange={_onParentPasswordEnter} - type='password' - value={parentPassword} + {isLocked + ? ( +
- {!!parentPassword && !isProperParentPassword && ( + ) + : ( + + ) + } +
+ ('enter the password for the account you want to derive from')} + onChange={_onParentPasswordEnter} + type='password' + value={parentPassword} + /> + {!!parentPassword && !isProperParentPassword && ( + + {t('Wrong password')} + + )} +
+ {isProperParentPassword && ( + <> + + {(!!pathError) && ( - {t('Wrong password')} + {pathError} )} -
- {isProperParentPassword && ( - <> - - {!suriPath && ( - - {t('Incorrect derivation path')} - - )} - - )} -
+ + )}
{t('Create a derived account')}