Skip to content

Commit

Permalink
Merge pull request #14390 from brave/fix-password-protect-account-key…
Browse files Browse the repository at this point in the history
…-retrieval

fix: password-protect account private key reveal
  • Loading branch information
josheleonard authored Aug 1, 2022
2 parents 3e6491b + 7cfa50a commit 7fd9cef
Show file tree
Hide file tree
Showing 24 changed files with 417 additions and 98 deletions.
1 change: 1 addition & 0 deletions components/brave_wallet/browser/brave_wallet_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ constexpr char kRampBuyUrl[] =
constexpr char kRampID[] = "8yxja8782as5essk2myz3bmh4az6gpq4nte9n2gf";

constexpr webui::LocalizedString kLocalizedStrings[] = {
{"braveWalletEnterYourPassword", IDS_BRAVE_WALLET_ENTER_YOUR_PASSWORD},
{"braveWallet", IDS_BRAVE_WALLET},
{"braveWalletDefiCategory", IDS_BRAVE_WALLET_DEFI_CATEGORY},
{"braveWalletNftCategory", IDS_BRAVE_WALLET_NFT_CATEGORY},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,4 @@ export const addFilecoinAccount = createAction<AddFilecoinAccountPayloadType>('a
export const getOnRampCurrencies = createAction('getOnRampCurrencies')
export const setOnRampCurrencies = createAction<BraveWallet.OnRampCurrency[]>('setOnRampCurrencies')
export const selectCurrency = createAction<BraveWallet.OnRampCurrency>('selectCurrency')
export const setPasswordAttempts = createAction<number>('setPasswordAttempts')
43 changes: 28 additions & 15 deletions components/brave_wallet_ui/common/async/__mocks__/bridge.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { BraveWallet } from '../../../constants/types'
import { SwapParamsPayloadType } from '../../constants/action_types'
import WalletApiProxy from '../../wallet_api_proxy'
export class MockedWalletApiProxy {
mockQuote = {
Expand Down Expand Up @@ -42,33 +41,41 @@ export class MockedWalletApiProxy {
buyTokenToEthRate: '1'
}

swapService = {
swapService: Partial<InstanceType<typeof BraveWallet.SwapServiceInterface>> = {
getTransactionPayload: async ({
fromAsset,
toAsset,
fromAssetAmount,
toAssetAmount
}: SwapParamsPayloadType): Promise<{ success: boolean, errorResponse: any, response: BraveWallet.SwapResponse }> => ({
buyAmount,
buyToken,
sellAmount,
sellToken
}: BraveWallet.SwapParams): Promise<{
success: boolean
errorResponse: any
response: BraveWallet.SwapResponse
}> => ({
success: true,
errorResponse: {},
response: {
...this.mockQuote,
buyTokenAddress: toAsset.contractAddress,
sellTokenAddress: fromAsset.contractAddress,
buyAmount: toAssetAmount || '',
sellAmount: fromAssetAmount || '',
buyTokenAddress: buyToken,
sellTokenAddress: sellToken,
buyAmount: buyAmount || '',
sellAmount: sellAmount || '',
price: '1'
}
// as BraveWallet.SwapResponse
}),
getPriceQuote: async () => ({
success: true,
errorResponse: {},
errorResponse: null,
response: this.mockTransaction
})
}

ethTxManagerProxy = {
keyringService: Partial<InstanceType<typeof BraveWallet.KeyringServiceInterface>> = {
validatePassword: async (password: string) => ({ result: password === 'password' }),
lock: () => alert('wallet locked')
}

ethTxManagerProxy: Partial<InstanceType<typeof BraveWallet.EthTxManagerProxyInterface>> = {
getGasEstimation1559: async () => {
return {
estimation: {
Expand All @@ -93,6 +100,12 @@ export class MockedWalletApiProxy {
}
}

export default function getAPIProxy (): Partial<WalletApiProxy> {
export function getAPIProxy (): Partial<WalletApiProxy> {
return new MockedWalletApiProxy() as unknown as Partial<WalletApiProxy> & MockedWalletApiProxy
}

export function getMockedAPIProxy (): WalletApiProxy {
return new MockedWalletApiProxy() as unknown as WalletApiProxy
}

export default getAPIProxy
4 changes: 3 additions & 1 deletion components/brave_wallet_ui/common/async/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import WalletApiProxy from '../wallet_api_proxy'
import getWalletPanelApiProxy from '../../panel/wallet_panel_api_proxy'
import getWalletPageApiProxy from '../../page/wallet_page_api_proxy'

export default function getAPIProxy (): WalletApiProxy {
export function getAPIProxy (): WalletApiProxy {
return window.location.hostname === 'wallet-panel.top-chrome'
? getWalletPanelApiProxy() : getWalletPageApiProxy()
}

export default getAPIProxy
10 changes: 10 additions & 0 deletions components/brave_wallet_ui/common/context/api-proxy.context.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright (c) 2022 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

import * as React from 'react'

import WalletApiProxy from '../wallet_api_proxy'

export const ApiProxyContext = React.createContext<WalletApiProxy | undefined>(undefined)
10 changes: 8 additions & 2 deletions components/brave_wallet_ui/common/hooks/swap.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ import { mockPageState } from '../../stories/mock-data/mock-page-state'
import { LibContext } from '../context/lib.context'
import { mockBasicAttentionToken, mockEthToken } from '../../stories/mock-data/mock-asset-options'

jest.useFakeTimers()

const store = createStore(combineReducers({
wallet: createWalletReducer(mockWalletState),
page: createPageReducer(mockPageState)
Expand Down Expand Up @@ -65,6 +63,14 @@ const mockQuote = {
} as BraveWallet.SwapResponse

describe('useSwap hook', () => {
beforeAll(() => {
jest.useFakeTimers()
})
afterAll(() => {
jest.clearAllTimers()
jest.useRealTimers()
})

it('should initialize From and To assets', async () => {
const { result, waitForNextUpdate } = renderHook(() => useSwap(), renderHookOptions)

Expand Down
2 changes: 1 addition & 1 deletion components/brave_wallet_ui/common/hooks/swap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ export default function useSwap ({ fromAsset: fromAssetProp, toAsset: toAssetPro
full
} = payload

const swapParams = {
const swapParams: BraveWallet.SwapParams = {
takerAddress: accountAddress,
sellAmount: fromAssetAmount || '',
buyAmount: toAssetAmount || '',
Expand Down
16 changes: 16 additions & 0 deletions components/brave_wallet_ui/common/hooks/use-api-proxy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) 2022 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

import * as React from 'react'

import { ApiProxyContext } from '../context/api-proxy.context'

export const useApiProxy = () => {
const context = React.useContext(ApiProxyContext)
if (context === undefined) {
throw new Error('useApiProxy must be used within a ApiProxyContext.Provider')
}
return context
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright (c) 2022 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

import * as React from 'react'
import { createStore, combineReducers } from 'redux'
import { Provider } from 'react-redux'
import { act, renderHook } from '@testing-library/react-hooks'

import { createWalletReducer } from '../reducers/wallet_reducer'
import { usePasswordAttempts } from './use-password-attempts'
import { mockWalletState } from '../../stories/mock-data/mock-wallet-state'
import { ApiProxyContext } from '../context/api-proxy.context'
import { getMockedAPIProxy } from '../async/__mocks__/bridge'

const proxy = getMockedAPIProxy()
proxy.keyringService.lock = jest.fn(proxy.keyringService.lock)

const makeStore = () => {
const store = createStore(combineReducers({
wallet: createWalletReducer(mockWalletState)
}))

store.dispatch = jest.fn(store.dispatch)
return store
}

function renderHookOptionsWithCustomStore (store: any) {
return {
wrapper: ({ children }: { children?: React.ReactChildren }) =>
<ApiProxyContext.Provider value={proxy}>
<Provider store={store}>
{children}
</Provider>
</ApiProxyContext.Provider>
}
}

const MAX_ATTEMPTS = 3

describe('useTransactionParser hook', () => {
it('should increment attempts on bad password ', async () => {
const store = makeStore()

const {
result
} = renderHook(() => usePasswordAttempts({
maxAttempts: MAX_ATTEMPTS
}), renderHookOptionsWithCustomStore(store))

expect(result.current.attempts).toEqual(0)

// attempt 1
await act(async () => {
await result.current.attemptPasswordEntry('pass')
})

expect(result.current.attempts).toEqual(1)

// attempt 2
await act(async () => {
await result.current.attemptPasswordEntry('pass')
})

expect(result.current.attempts).toEqual(2)

// attempt 3
await act(async () => {
await result.current.attemptPasswordEntry('pass')
})

// Wallet is now locked
expect(proxy.keyringService.lock).toHaveBeenCalled()

// attempts should be reset since wallet was locked
expect(result.current.attempts).toEqual(0)
})
})
69 changes: 69 additions & 0 deletions components/brave_wallet_ui/common/hooks/use-password-attempts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright (c) 2022 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

import * as React from 'react'
import { useDispatch, useSelector } from 'react-redux'
import { WalletState } from '../../constants/types'
import { WalletActions } from '../actions'
import { useApiProxy } from './use-api-proxy'

interface Options {
maxAttempts: number
}

/**
* Provides a methods to check the user's password,
* and lock the wallet after too many incorrect attempts
*
* Uses the context-injected ApiProxy keyring
* Uses redux to track attempts globally
*/
export const usePasswordAttempts = ({
maxAttempts
}: Options) => {
// custom hooks
const { keyringService } = useApiProxy()

// redux
const dispatch = useDispatch()
const attempts = useSelector(({ wallet }: { wallet: WalletState }) => {
return wallet.passwordAttempts
})

// methods
const attemptPasswordEntry = React.useCallback(async (password: string): Promise<boolean> => {
if (!password) { // require password to view key
return false
}

// entered password must be correct
const {
result: isPasswordValid
} = await keyringService.validatePassword(password)

if (!isPasswordValid) {
const newAttempts = attempts + 1
if (newAttempts >= maxAttempts) {
// lock wallet
keyringService.lock()
dispatch(WalletActions.setPasswordAttempts(0)) // reset attempts now that the wallet is locked
return false
}

// increase attempts count
dispatch(WalletActions.setPasswordAttempts(newAttempts))
return false
}

// correct password entered, reset attempts
dispatch(WalletActions.setPasswordAttempts(0))
return isPasswordValid
}, [keyringService, attempts])

return {
attemptPasswordEntry,
attempts
}
}
10 changes: 9 additions & 1 deletion components/brave_wallet_ui/common/reducers/wallet_reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ const defaultState: WalletState = {
selectedNetworkFilter: AllNetworksOption,
solFeeEstimates: undefined,
onRampCurrencies: [] as BraveWallet.OnRampCurrency[],
selectedCurrency: undefined
selectedCurrency: undefined,
passwordAttempts: 0
}

const getAccountType = (info: AccountInfo) => {
Expand Down Expand Up @@ -554,6 +555,13 @@ export const createWalletReducer = (initialState: WalletState) => {
}
})

reducer.on(WalletActions.setPasswordAttempts, (state: WalletState, payload: number): WalletState => {
return {
...state,
passwordAttempts: payload
}
})

return reducer
}

Expand Down
4 changes: 3 additions & 1 deletion components/brave_wallet_ui/common/wallet_api_proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Store } from './async/types'
import { getBraveKeyring } from './api/hardware_keyrings'
import { BraveWallet } from '../constants/types'

export default class WalletApiProxy {
export class WalletApiProxy {
walletHandler = new BraveWallet.WalletHandlerRemote()
jsonRpcService = new BraveWallet.JsonRpcServiceRemote()
swapService = new BraveWallet.SwapServiceRemote()
Expand Down Expand Up @@ -109,3 +109,5 @@ export default class WalletApiProxy {
this.braveWalletService.addObserver(braveWalletServiceObserverReceiver.$.bindNewPipeAndPassRemote())
}
}

export default WalletApiProxy
2 changes: 1 addition & 1 deletion components/brave_wallet_ui/components/desktop/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import WalletMorePopup from './wallet-more-popup'
import WalletBanner from './wallet-banner'
import PopupModal from './popup-modals'
import { AddAccountModal } from './popup-modals/add-account-modal'
import AccountSettingsModal from './popup-modals/account-settings-modal'
import { AccountSettingsModal } from './popup-modals/account-settings-modal/account-settings-modal'
import EditVisibleAssetsModal from './popup-modals/edit-visible-assets-modal'
import AssetWatchlistItem from './asset-watchlist-item'
import SelectNetworkDropdown from './select-network-dropdown'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
// Copyright (c) 2022 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

import styled from 'styled-components'

import ClipboardIcon from '../../../../assets/svg-icons/clipboard-icon.svg'
import { WalletButton } from '../../../shared/style'

Expand Down Expand Up @@ -48,6 +54,13 @@ export const Input = styled.input`
margin: 0;
}
`
export const InputLabelText = styled.label`
display: block;
margin-bottom: 8px;
color: ${(p) => p.theme.color.text03};
text-align: left;
width: 100%;
`

export const QRCodeWrapper = styled.img`
display: flex;
Expand Down
Loading

0 comments on commit 7fd9cef

Please sign in to comment.