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

feat: improving loginAttempt storage #1067

Merged
2 changes: 1 addition & 1 deletion packages/legacy/core/App/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export const testIdPrefix = 'com.ariesbifold:id/'

export enum LocalStorageKeys {
Onboarding = 'OnboardingState',
LoginAttempts = 'LoginAttempts',
// FIXME: Once hooks are updated this should no longer be necessary
RevokedCredentials = 'RevokedCredentials',
RevokedCredentialsMessageDismissed = 'RevokedCredentialsMessageDismissed',
Expand All @@ -24,6 +23,7 @@ export enum LocalStorageKeys {
export enum KeychainServices {
Salt = 'secret.wallet.salt',
Key = 'secret.wallet.key',
LoginAttempt = 'wallet.loginAttempt',
}

export enum EventTypes {
Expand Down
3 changes: 2 additions & 1 deletion packages/legacy/core/App/contexts/reducers/store.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import AsyncStorage from '@react-native-async-storage/async-storage'

import { LocalStorageKeys } from '../../constants'
import { storeLoginAttempt } from '../../services/keychain'
import {
Preferences as PreferencesState,
Tours as ToursState,
Expand Down Expand Up @@ -415,7 +416,7 @@ export const reducer = <S extends State>(state: S, action: ReducerAction<Dispatc
...state,
loginAttempt,
}
AsyncStorage.setItem(LocalStorageKeys.LoginAttempts, JSON.stringify(newState.loginAttempt))
storeLoginAttempt(loginAttempt)
return newState
}
case LockoutDispatchAction.LOCKOUT_UPDATED: {
Expand Down
2 changes: 2 additions & 0 deletions packages/legacy/core/App/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import Preface from './screens/Preface'
import Splash from './screens/Splash'
import Terms from './screens/Terms'
import UseBiometry from './screens/UseBiometry'
import { loadLoginAttempt } from './services/keychain'
import * as types from './types'

export { LocalStorageKeys } from './constants'
Expand Down Expand Up @@ -144,4 +145,5 @@ export {
components,
contexts,
Text,
loadLoginAttempt,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wadeking98 Is this okay?

}
6 changes: 3 additions & 3 deletions packages/legacy/core/App/screens/Splash.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { useConfiguration } from '../contexts/configuration'
import { DispatchAction } from '../contexts/reducers/store'
import { useStore } from '../contexts/store'
import { useTheme } from '../contexts/theme'
import { loadLoginAttempt } from '../services/keychain'
import { BifoldError } from '../types/error'
import { Screens, Stacks } from '../types/navigators'
import {
Expand Down Expand Up @@ -104,9 +105,8 @@ const Splash: React.FC = () => {
})

const loadAuthAttempts = async (): Promise<LoginAttemptState | undefined> => {
const attemptsData = await AsyncStorage.getItem(LocalStorageKeys.LoginAttempts)
if (attemptsData) {
const attempts = JSON.parse(attemptsData) as LoginAttemptState
const attempts = await loadLoginAttempt()
if (attempts) {
dispatch({
type: DispatchAction.ATTEMPT_UPDATED,
payload: [attempts],
Expand Down
23 changes: 22 additions & 1 deletion packages/legacy/core/App/services/keychain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@

import { walletId, KeychainServices } from '../constants'
import { WalletSecret } from '../types/security'
import { LoginAttempt } from '../types/state'
import { hashPIN } from '../utils/crypto'

const keyFauxUserName = 'WalletFauxPINUserName'
const saltFauxUserName = 'WalletFauxSaltUserName'

const loginAttemptFauxUserName = 'WalletFauxLoginAttemptUserName'
export interface WalletSalt {
id: string
salt: string
Expand Down Expand Up @@ -72,6 +73,13 @@
return typeof result === 'boolean' ? false : true
}

export const storeLoginAttempt = async (loginAttempt: LoginAttempt): Promise<boolean> => {
const opts = optionsForKeychainAccess(KeychainServices.LoginAttempt, false)
const loginAttemptAsString = JSON.stringify(loginAttempt)
const result = await Keychain.setGenericPassword(loginAttemptFauxUserName, loginAttemptAsString, opts)
return typeof result !== 'boolean'

Check warning on line 80 in packages/legacy/core/App/services/keychain.ts

View check run for this annotation

Codecov / codecov/patch

packages/legacy/core/App/services/keychain.ts#L77-L80

Added lines #L77 - L80 were not covered by tests
}

export const storeWalletSecret = async (secret: WalletSecret, useBiometrics = false): Promise<boolean> => {
let keyResult = false
if (secret.key) {
Expand All @@ -95,6 +103,19 @@
return JSON.parse(result.password) as WalletSalt
}

export const loadLoginAttempt = async (): Promise<LoginAttempt | undefined> => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you export this method from the bifold index?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bryce-mcmath I didn't understand. We don't have this method in the index

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you go into packages/legacy/core/App/index.ts and import it and export it from there then other wallets can access it, like BC Wallet. Other than that it looks good

const opts: Keychain.Options = {

Check warning on line 107 in packages/legacy/core/App/services/keychain.ts

View check run for this annotation

Codecov / codecov/patch

packages/legacy/core/App/services/keychain.ts#L107

Added line #L107 was not covered by tests
service: KeychainServices.LoginAttempt,
}

const result = await Keychain.getGenericPassword(opts)

Check warning on line 111 in packages/legacy/core/App/services/keychain.ts

View check run for this annotation

Codecov / codecov/patch

packages/legacy/core/App/services/keychain.ts#L111

Added line #L111 was not covered by tests
if (!result) {
return

Check warning on line 113 in packages/legacy/core/App/services/keychain.ts

View check run for this annotation

Codecov / codecov/patch

packages/legacy/core/App/services/keychain.ts#L113

Added line #L113 was not covered by tests
}

return JSON.parse(result.password) as LoginAttempt

Check warning on line 116 in packages/legacy/core/App/services/keychain.ts

View check run for this annotation

Codecov / codecov/patch

packages/legacy/core/App/services/keychain.ts#L116

Added line #L116 was not covered by tests
}

export const loadWalletKey = async (title?: string, description?: string): Promise<WalletKey | undefined> => {
let opts: Keychain.Options = {
service: KeychainServices.Key,
Expand Down
24 changes: 15 additions & 9 deletions packages/legacy/core/__tests__/screens/Splash.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Splash from '../../App/screens/Splash'
import AsyncStorage from '../../__mocks__/@react-native-async-storage/async-storage'
import authContext from '../contexts/auth'
import configurationContext from '../contexts/configuration'
import { loadLoginAttempt } from '../../App/services/keychain'

jest.mock('@hyperledger/aries-askar-react-native', () => ({}))
jest.mock('@hyperledger/anoncreds-react-native', () => ({}))
Expand All @@ -24,12 +25,15 @@ jest.mock('@react-navigation/core', () => {
}),
}
})
jest.mock('../../App/services/keychain', () => ({
loadLoginAttempt: jest.fn(),
}))

describe('Splash Screen', () => {
beforeAll(()=>{
beforeAll(() => {
jest.useFakeTimers()
})
afterAll(()=>{
afterAll(() => {
jest.useRealTimers()
})
test('Renders default correctly', async () => {
Expand All @@ -40,18 +44,18 @@ describe('Splash Screen', () => {
</AuthContext.Provider>
</ConfigurationContext.Provider>
)
await act(()=>{ jest.runAllTimers() })
await act(() => {
jest.runAllTimers()
})
expect(tree).toMatchSnapshot()
})

test('Starts onboarding correctly', async () => {
// @ts-ignore-next-line
loadLoginAttempt.mockReturnValue({ servedPenalty: true, loginAttempts: 0 })

AsyncStorage.getItem = jest.fn().mockImplementation((key: string) => {
switch (key) {
case LocalStorageKeys.LoginAttempts:
return JSON.stringify({
servedPenalty: true,
loginAttempts: 0,
})
case LocalStorageKeys.Preferences:
return JSON.stringify({
useBiometry: false,
Expand Down Expand Up @@ -101,7 +105,9 @@ describe('Splash Screen', () => {
</StoreProvider>
)
})
await act(()=>{ jest.runAllTimers() })
await act(() => {
jest.runAllTimers()
})
expect(mockedDispatch).toHaveBeenCalled()
})
})
Loading