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: password-protect account private key reveal #14390

Merged
merged 6 commits into from
Aug 1, 2022
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
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)
})
})
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