From 347a121c0bb5eaeadd1741715f6da58621f4b7ec Mon Sep 17 00:00:00 2001 From: sergey Date: Wed, 16 Feb 2022 13:08:58 +0300 Subject: [PATCH] Fix error message when Ledger is locked Do not allow unlocking without the device id --- .../eth_ledger_bridge_keyring.test.ts | 33 ++++++++++++------- .../ledgerjs/eth_ledger_bridge_keyring.ts | 21 ++++++------ .../hardware-wallet-connect/index.tsx | 8 ++--- 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/components/brave_wallet_ui/common/hardware/ledgerjs/eth_ledger_bridge_keyring.test.ts b/components/brave_wallet_ui/common/hardware/ledgerjs/eth_ledger_bridge_keyring.test.ts index b137c0cf0bad..a3bd1a4897e8 100644 --- a/components/brave_wallet_ui/common/hardware/ledgerjs/eth_ledger_bridge_keyring.test.ts +++ b/components/brave_wallet_ui/common/hardware/ledgerjs/eth_ledger_bridge_keyring.test.ts @@ -30,6 +30,13 @@ const createLedgerKeyring = () => { return ledgerHardwareKeyring } +const unlockedLedgerKeyring = () => { + const ledgerHardwareKeyring = new LedgerBridgeKeyring() + ledgerHardwareKeyring.app = new MockApp() + ledgerHardwareKeyring.deviceId = 'device1' + return ledgerHardwareKeyring +} + test('Extracting accounts from device', () => { return expect(createLedgerKeyring().getAccounts(-2, 1, LedgerDerivationPaths.LedgerLive)) .resolves.toStrictEqual({ @@ -85,10 +92,18 @@ test('Check ledger bridge type', () => { return expect(ledgerHardwareKeyring.type()).toStrictEqual(BraveWallet.LEDGER_HARDWARE_VENDOR) }) -test('Check locks for device', () => { +test('Check locks for device app only', () => { + const ledgerHardwareKeyring = new LedgerBridgeKeyring() + expect(ledgerHardwareKeyring.isUnlocked()).toStrictEqual(false) + ledgerHardwareKeyring.app = new MockApp() + expect(ledgerHardwareKeyring.isUnlocked()).toStrictEqual(false) +}) + +test('Check locks for device app and device id', () => { const ledgerHardwareKeyring = new LedgerBridgeKeyring() expect(ledgerHardwareKeyring.isUnlocked()).toStrictEqual(false) ledgerHardwareKeyring.app = new MockApp() + ledgerHardwareKeyring.deviceId = 'test' expect(ledgerHardwareKeyring.isUnlocked()).toStrictEqual(true) }) @@ -102,15 +117,13 @@ test('Extract accounts from locked device', () => { }) test('Extract accounts from unknown device', () => { - const ledgerHardwareKeyring = new LedgerBridgeKeyring() - ledgerHardwareKeyring.app = new MockApp() + const ledgerHardwareKeyring = unlockedLedgerKeyring() return expect(ledgerHardwareKeyring.getAccounts(-2, 1, 'unknown')) .rejects.toThrow() }) test('Sign personal message successfully with padding v<27', () => { - const ledgerHardwareKeyring = new LedgerBridgeKeyring() - ledgerHardwareKeyring.app = new MockApp() + const ledgerHardwareKeyring = unlockedLedgerKeyring() ledgerHardwareKeyring.app.signature = { v: 0, r: 'b68983', s: 'r68983' } return expect(ledgerHardwareKeyring.signPersonalMessage( 'm/44\'/60\'/0\'/0/0', 'message')) @@ -118,8 +131,7 @@ test('Sign personal message successfully with padding v<27', () => { }) test('Sign personal message successfully with padding v>=27', () => { - const ledgerHardwareKeyring = new LedgerBridgeKeyring() - ledgerHardwareKeyring.app = new MockApp() + const ledgerHardwareKeyring = unlockedLedgerKeyring() ledgerHardwareKeyring.app.signature = { v: 28, r: 'b68983', s: 'r68983' } return expect(ledgerHardwareKeyring.signPersonalMessage( 'm/44\'/60\'/0\'/0/0', 'message')) @@ -127,8 +139,7 @@ test('Sign personal message successfully with padding v>=27', () => { }) test('Sign personal message successfully without padding v>=27', () => { - const ledgerHardwareKeyring = new LedgerBridgeKeyring() - ledgerHardwareKeyring.app = new MockApp() + const ledgerHardwareKeyring = unlockedLedgerKeyring() ledgerHardwareKeyring.app.signature = { v: 44, r: 'b68983', s: 'r68983' } return expect(ledgerHardwareKeyring.signPersonalMessage( 'm/44\'/60\'/0\'/0/0', 'message')) @@ -136,8 +147,7 @@ test('Sign personal message successfully without padding v>=27', () => { }) test('Sign personal message successfully without padding v<27', () => { - const ledgerHardwareKeyring = new LedgerBridgeKeyring() - ledgerHardwareKeyring.app = new MockApp() + const ledgerHardwareKeyring = unlockedLedgerKeyring() ledgerHardwareKeyring.app.signature = { v: 17, r: 'b68983', s: 'r68983' } return expect(ledgerHardwareKeyring.signPersonalMessage( 'm/44\'/60\'/0\'/0/0', 'message')) @@ -146,7 +156,6 @@ test('Sign personal message successfully without padding v<27', () => { test('Sign personal message failed', () => { const ledgerHardwareKeyring = createLedgerKeyring() - ledgerHardwareKeyring.app = new MockApp() return expect(ledgerHardwareKeyring.signPersonalMessage( 'm/44\'/60\'/0\'/0/0', 'message')) .resolves.toMatchObject({ success: false }) diff --git a/components/brave_wallet_ui/common/hardware/ledgerjs/eth_ledger_bridge_keyring.ts b/components/brave_wallet_ui/common/hardware/ledgerjs/eth_ledger_bridge_keyring.ts index 2b85b419c465..02285cdf4571 100644 --- a/components/brave_wallet_ui/common/hardware/ledgerjs/eth_ledger_bridge_keyring.ts +++ b/components/brave_wallet_ui/common/hardware/ledgerjs/eth_ledger_bridge_keyring.ts @@ -63,7 +63,7 @@ export default class LedgerBridgeKeyring extends LedgerEthereumKeyring { } isUnlocked = (): boolean => { - return this.app !== undefined + return this.app !== undefined && this.deviceId !== undefined } makeApp = async () => { @@ -71,21 +71,20 @@ export default class LedgerBridgeKeyring extends LedgerEthereumKeyring { } unlock = async (): Promise => { - if (this.app) { + if (this.isUnlocked()) { return { success: true } } - await this.makeApp() if (!this.app) { - return { success: false } + await this.makeApp() + } + if (this.app && !this.deviceId) { + const eth: Eth = this.app + eth.transport.on('disconnect', this.onDisconnected) + const zeroPath = this.getPathForIndex(0, LedgerDerivationPaths.LedgerLive) + const address = (await eth.getAddress(zeroPath)).address + this.deviceId = await hardwareDeviceIdFromAddress(address) } - - const eth: Eth = this.app - eth.transport.on('disconnect', this.onDisconnected) - const zeroPath = this.getPathForIndex(0, LedgerDerivationPaths.LedgerLive) - const address = (await eth.getAddress(zeroPath)).address - this.deviceId = await hardwareDeviceIdFromAddress(address) - return { success: this.isUnlocked() } } diff --git a/components/brave_wallet_ui/components/desktop/popup-modals/add-account-modal/hardware-wallet-connect/index.tsx b/components/brave_wallet_ui/components/desktop/popup-modals/add-account-modal/hardware-wallet-connect/index.tsx index ca00d1fbda42..211aa71fd1e7 100644 --- a/components/brave_wallet_ui/components/desktop/popup-modals/add-account-modal/hardware-wallet-connect/index.tsx +++ b/components/brave_wallet_ui/components/desktop/popup-modals/add-account-modal/hardware-wallet-connect/index.tsx @@ -53,9 +53,9 @@ export default function (props: Props) { LedgerDerivationPaths.LedgerLive ) const [showAccountsList, setShowAccountsList] = React.useState(false) - const getErrorMessage = (error: any) => { + const getErrorMessage = (error: any, accountTypeName: string) => { if (error.statusCode && error.statusCode === 27404) { // Unknown Error - return { error: getLocale('braveWalletConnectHardwareInfo2'), userHint: '' } + return { error: getLocale('braveWalletConnectHardwareInfo2').replace('$1', accountTypeName), userHint: '' } } if (error.statusCode && (error.statusCode === 27904 || error.statusCode === 26368)) { // INCORRECT_LENGTH or INS_NOT_SUPPORTED @@ -81,7 +81,7 @@ export default function (props: Props) { }).then((result) => { setAccounts(result) }).catch((error) => { - setConnectionError(getErrorMessage(error)) + setConnectionError(getErrorMessage(error, selectedAccountType.name)) setShowAccountsList(false) }).finally( () => setIsConnecting(false) @@ -150,7 +150,7 @@ export default function (props: Props) { setAccounts([...accounts, ...result]) setShowAccountsList(true) }).catch((error) => { - setConnectionError(getErrorMessage(error)) + setConnectionError(getErrorMessage(error, selectedAccountType.name)) setShowAccountsList(false) }).finally( () => setIsConnecting(false)