Skip to content

Commit

Permalink
fix: race condition on feature-flag during startup (#629)
Browse files Browse the repository at this point in the history
* fix: initialize biometry from asyncstorage instead of redux
  • Loading branch information
r4mmer authored Nov 25, 2024
1 parent 5efc75b commit f791e6d
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 81 deletions.
5 changes: 0 additions & 5 deletions src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,6 @@ export const setUseWalletService = (data) => ({
payload: data,
});

export const setUseSafeBiometryMode = (data) => ({
type: types.SET_USE_SAFE_BIOMETRY_MODE,
payload: data,
});

export const setUniqueDeviceId = (uniqueId) => ({
type: types.SET_UNIQUE_DEVICE_ID,
payload: uniqueId,
Expand Down
11 changes: 0 additions & 11 deletions src/reducers/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,6 @@ const initialState = {
address: null,
error: null,
},

safeBiometryEnabled: false,
};

export const reducer = (state = initialState, action) => {
Expand Down Expand Up @@ -736,8 +734,6 @@ export const reducer = (state = initialState, action) => {
return onNewNanoContractTransactionRetry(state);
case types.REOWN_NEW_NANOCONTRACT_RETRY_DISMISS:
return onNewNanoContractTransactionRetryDismiss(state);
case types.SET_USE_SAFE_BIOMETRY_MODE:
return onSetUseSafeBiometryMode(state, action);
default:
return state;
}
Expand Down Expand Up @@ -932,22 +928,15 @@ const onSetUseWalletService = (state, action) => ({
useWalletService: action.payload,
});

const onSetUseSafeBiometryMode = (state, action) => ({
...state,
safeBiometryEnabled: action.payload,
});

const onResetWalletSuccess = (state) => {
const oldUnleashClient = state.unleashClient;
const oldFeatureTogglesInitialized = state.featureTogglesInitialized;
const oldFeatureToggles = state.featureToggles;
const oldSafeBiometryEnabled = state.safeBiometryEnabled;
return {
...initialState,
unleashClient: oldUnleashClient,
featureTogglesInitialized: oldFeatureTogglesInitialized,
featureToggles: oldFeatureToggles,
safeBiometryEnabled: oldSafeBiometryEnabled,
};
};

Expand Down
6 changes: 3 additions & 3 deletions src/sagas/featureToggle.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,17 @@ import {
setUnleashClient,
setFeatureToggles,
featureToggleInitialized,
setUseSafeBiometryMode,
} from '../actions';
import {
UNLEASH_URL,
UNLEASH_CLIENT_KEY,
UNLEASH_POLLING_INTERVAL,
STAGE,
FEATURE_TOGGLE_DEFAULTS,
SAFE_BIOMETRY_MODE_FEATURE_TOGGLE,
} from '../constants';
import { disableFeaturesIfNeeded } from './helpers';
import { logger } from '../logger';
import { STORE, FEATURE_TOGGLES_LAST_KNOWN_VALUES_KEY } from '../store';

const MAX_RETRIES = 5;

Expand Down Expand Up @@ -89,6 +88,7 @@ export function* handleToggleUpdate() {

const toggles = unleashClient.getToggles();
const featureToggles = disableFeaturesIfNeeded(networkSettings, mapFeatureToggles(toggles));
STORE.setItem(FEATURE_TOGGLES_LAST_KNOWN_VALUES_KEY, featureToggles);

yield put(setFeatureToggles(featureToggles));
yield put({ type: types.FEATURE_TOGGLE_UPDATED });
Expand Down Expand Up @@ -126,9 +126,9 @@ export function* monitorFeatureFlags(currentRetry = 0) {
// At this point, unleashClient.fetchToggles() already fetched the toggles
// (this will throw if it hasn't)
const featureToggles = mapFeatureToggles(unleashClient.getToggles());
STORE.setItem(FEATURE_TOGGLES_LAST_KNOWN_VALUES_KEY, featureToggles);

yield put(setFeatureToggles(featureToggles));
yield put(setUseSafeBiometryMode(featureToggles[SAFE_BIOMETRY_MODE_FEATURE_TOGGLE]));
yield put(featureToggleInitialized());
} catch (e) {
log.error(e);
Expand Down
25 changes: 0 additions & 25 deletions src/sagas/wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import {
DEFAULT_TOKEN,
WALLET_SERVICE_FEATURE_TOGGLE,
PUSH_NOTIFICATION_FEATURE_TOGGLE,
SAFE_BIOMETRY_MODE_FEATURE_TOGGLE,
networkSettingsKeyMap,
} from '../constants';
import { STORE } from '../store';
Expand Down Expand Up @@ -71,8 +70,6 @@ import {
selectAddressAddressesFailure,
firstAddressFailure,
firstAddressSuccess,
setUseSafeBiometryMode,
lockScreen,
firstAddressRequest,
} from '../actions';
import { fetchTokenData } from './tokens';
Expand All @@ -90,7 +87,6 @@ import {
getAllAddresses,
getFirstAddress,
setKeychainPin,
isBiometryEnabled,
} from '../utils';
import { logger } from '../logger';

Expand Down Expand Up @@ -438,11 +434,6 @@ export function* onPushNotificationDisabled() {
yield put(setAvailablePushNotification(false));
}

export function* onSafeBiometryToggleChanged() {
log.debug('Safe biometry mode feature toggle changed state, locking wallet.');
yield put(lockScreen());
}

/**
* This saga will wait for feature toggle updates and react when a toggle state
* transition is done
Expand All @@ -457,22 +448,6 @@ export function* featureToggleUpdateListener() {
const oldPushNotificationToggle = yield select((state) => state.pushNotification.available);
const newPushNotificationToggle = yield call(isPushNotificationEnabled);

const oldSafeBiometryEnabled = yield select(({ safeBiometryEnabled }) => safeBiometryEnabled);
const newSafeBiometryEnabled = yield call(
checkForFeatureFlag,
SAFE_BIOMETRY_MODE_FEATURE_TOGGLE,
);

if (oldSafeBiometryEnabled !== newSafeBiometryEnabled) {
// Safe biometry feature changed, need to update the state.
yield put(setUseSafeBiometryMode(newSafeBiometryEnabled));
if (isBiometryEnabled()) {
// Since biometry is enabled, a migration is required, this means the wallet
// needs to restart and the migration will fix the next time it opens.
yield call(onSafeBiometryToggleChanged);
}
}

// WalletService is currently ON and the featureToggle is now OFF
if (!newWalletServiceToggle && oldWalletServiceToggle) {
yield call(onWalletServiceDisabled);
Expand Down
18 changes: 9 additions & 9 deletions src/screens/PinScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ import {
resetOnLockScreen,
onExceptionCaptured,
} from '../actions';
import { PIN_SIZE, SAFE_BIOMETRY_MODE_FEATURE_TOGGLE } from '../constants';
import { PIN_SIZE } from '../constants';
import { COLORS } from '../styles/themes';
import { STORE } from '../store';
import { SAFE_BIOMETRY_FEATURE_FLAG_KEY, STORE } from '../store';
import baseStyle from '../styles/init';
import Spinner from '../components/Spinner';
import FeedbackModal from '../components/FeedbackModal';
Expand All @@ -43,7 +43,6 @@ const log = logger('PIN_SCREEN');
const mapStateToProps = (state) => ({
loadHistoryActive: state.loadHistoryStatus.active,
wallet: state.wallet,
safeBiometryEnabled: state.featureToggles[SAFE_BIOMETRY_MODE_FEATURE_TOGGLE],
});

const mapDispatchToProps = (dispatch) => ({
Expand Down Expand Up @@ -82,6 +81,7 @@ class PinScreen extends React.Component {
this.biometryText = props.route.params.biometryText ?? this.biometryText;
this.biometryLoadingText = props.route.params.biometryLoadingText ?? '';
}
this.useSafeBiometryFeature = STORE.getItem(SAFE_BIOMETRY_FEATURE_FLAG_KEY);
this.biometryEnabled = isBiometryEnabled();

this.focusEvent = null;
Expand Down Expand Up @@ -152,14 +152,14 @@ class PinScreen extends React.Component {
// method an change redux state. No need to execute callback or go back on navigation
try {
await STORE.handleDataMigration(pin);
const newPin = await biometricsMigration(pin, this.props.safeBiometryEnabled);
const actualPin = await biometricsMigration(pin);
if (!this.props.wallet) {
// We have already made sure we have an available accessData
// The handleDataMigration method ensures we have already migrated if necessary
// This means the wallet is loaded and the access data is ready to be used.

const words = await STORE.getWalletWords(newPin);
this.props.startWalletRequested({ words, pin: newPin });
const words = await STORE.getWalletWords(actualPin);
this.props.startWalletRequested({ words, pin: actualPin });
}
this.props.unlockScreen();
} catch (e) {
Expand Down Expand Up @@ -298,12 +298,12 @@ class PinScreen extends React.Component {
);

const renderButton = () => {
if ((!this.state.biometryFailed) && this.props.safeBiometryEnabled && this.biometryEnabled) {
if ((!this.state.biometryFailed) && this.useSafeBiometryFeature && this.biometryEnabled) {
// Biometry has not failed, so we should not show a cancellation button.
return null;
}
const biometryFailed = this.state.biometryFailed
&& this.props.safeBiometryEnabled
&& this.useSafeBiometryFeature
&& this.biometryEnabled;
let title;
let onPress;
Expand Down Expand Up @@ -372,7 +372,7 @@ class PinScreen extends React.Component {
);

const renderBody = () => {
if (this.props.safeBiometryEnabled && this.biometryEnabled) {
if (this.useSafeBiometryFeature && this.biometryEnabled) {
// Safe biometry mode is enabled, we should not render the pin input.
return safeBiometryMessage();
}
Expand Down
34 changes: 18 additions & 16 deletions src/screens/Security.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@ import {
import { HathorList, ListItem, ListMenu } from '../components/HathorList';
import { lockScreen, onExceptionCaptured } from '../actions';
import { COLORS } from '../styles/themes';
import { STORE } from '../store';
import { SAFE_BIOMETRY_MODE_FEATURE_TOGGLE } from '../constants';
import { SAFE_BIOMETRY_FEATURE_FLAG_KEY, STORE } from '../store';

const mapStateToProps = (state) => ({
wallet: state.wallet,
safeBiometryEnabled: state.featureToggles[SAFE_BIOMETRY_MODE_FEATURE_TOGGLE],
});

const mapDispatchToProps = (dispatch) => ({
Expand Down Expand Up @@ -67,16 +65,17 @@ export class Security extends React.Component {
};
}

onBiometrySwitchChange = (value) => {
this.setState({ biometryEnabled: value });
setBiometryEnabled(value);
}

onSafeBiometrySwitchChange = (value) => {
if (value) {
this.onSafeBiometryEnabled();
onBiometrySwitchChange(value) {
const useSafeBiometryFeature = STORE.getItem(SAFE_BIOMETRY_FEATURE_FLAG_KEY);
if (useSafeBiometryFeature) {
if (value) {
this.onSafeBiometryEnabled();
} else {
this.onSafeBiometryDisabled();
}
} else {
this.onSafeBiometryDisabled();
this.setState({ biometryEnabled: value });
setBiometryEnabled(value);
}
}

Expand Down Expand Up @@ -147,13 +146,18 @@ export class Security extends React.Component {
}

onLockWallet = () => {
// After the screen is unlocked the Home screen will be shown
this.props.navigation.navigate('Home');
this.props.lockScreen();
}

render() {
const switchDisabled = !this.supportedBiometry;
const biometryText = (switchDisabled ? t`No biometry supported` : t`Use ${this.supportedBiometry}`);
const safeBiometryActive = this.state.biometryEnabled && this.props.safeBiometryEnabled;

const useSafeBiometryFeature = STORE.getItem(SAFE_BIOMETRY_FEATURE_FLAG_KEY);
const safeBiometryActive = this.state.biometryEnabled && useSafeBiometryFeature;

return (
<View style={{ flex: 1, backgroundColor: COLORS.lowContrastDetail }}>
<HathorHeader
Expand All @@ -168,9 +172,7 @@ export class Security extends React.Component {
titleStyle={!switchDisabled ? { color: COLORS.textColor } : null}
text={(
<Switch
onValueChange={this.props.safeBiometryEnabled
? this.onSafeBiometrySwitchChange
: this.onBiometrySwitchChange}
onValueChange={this.onBiometrySwitchChange.bind(this)}
value={this.state.biometryEnabled}
disabled={switchDisabled}
/>
Expand Down
9 changes: 9 additions & 0 deletions src/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,18 @@ export const REGISTERED_TOKENS_KEY = 'asyncstorage:registeredTokens';
export const STORE_VERSION_KEY = 'asyncstorage:version';
export const REGISTERED_NANO_CONTRACTS_KEY = 'asyncstorage:registeredNanoContracts';
export const PIN_BACKUP_KEY = 'asyncstorage:pinBackup';
// Wheather old biometry mode is active (by the user request)
export const IS_OLD_BIOMETRY_ENABLED_KEY = 'mobile:isBiometryEnabled';
// Wheather safe biometry is active (by the user request)
export const IS_BIOMETRY_ENABLED_KEY = 'mobile:isSafeBiometryEnabled';
// Which type of biometry the device can handle.
export const SUPPORTED_BIOMETRY_KEY = 'mobile:supportedBiometry';
// This key determines under which biometry mode the wallet is operating on.
// The value here only changes during a migration (when the user unlocks the wallet)
export const SAFE_BIOMETRY_FEATURE_FLAG_KEY = 'asyncstorage:featureFlagSafeBiometryMode';
// These are the last known values of the unleash feature toggles
// These are updated on every call to unleash
export const FEATURE_TOGGLES_LAST_KNOWN_VALUES_KEY = 'asyncstorage:featureTogglesLastKnownValues';

export const walletKeys = [
ACCESS_DATA_KEY,
Expand Down
38 changes: 26 additions & 12 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import { Linking, Platform, Text } from 'react-native';
import { getStatusBarHeight } from 'react-native-status-bar-height';
import moment from 'moment';
import baseStyle from './styles/init';
import { KEYCHAIN_USER, NETWORK_MAINNET, NANO_CONTRACT_FEATURE_TOGGLE } from './constants';
import { STORE, IS_BIOMETRY_ENABLED_KEY, IS_OLD_BIOMETRY_ENABLED_KEY, SUPPORTED_BIOMETRY_KEY } from './store';
import { KEYCHAIN_USER, NETWORK_MAINNET, NANO_CONTRACT_FEATURE_TOGGLE, SAFE_BIOMETRY_MODE_FEATURE_TOGGLE } from './constants';
import { STORE, IS_BIOMETRY_ENABLED_KEY, IS_OLD_BIOMETRY_ENABLED_KEY, SUPPORTED_BIOMETRY_KEY, SAFE_BIOMETRY_FEATURE_FLAG_KEY, FEATURE_TOGGLES_LAST_KNOWN_VALUES_KEY } from './store';
import { TxHistory } from './models';
import { COLORS, STYLE } from './styles/themes';
import { logger } from './logger';
Expand Down Expand Up @@ -94,17 +94,28 @@ export const getTokenLabel = (token) => `${token.name} (${token.symbol})`;
/**
* Migrate the biometry configuration state if needed.
*
* @param {string} currentPassword
* @param {bool} safeBiometryEnabled
* @param {string} currentPassword - The password returned from the system keychain.
* @return {Promise<string>} The actual pin/password for the application.
*/
export async function biometricsMigration(currentPassword, safeBiometryEnabled) {
const oldBiometry = STORE.getItem(IS_OLD_BIOMETRY_ENABLED_KEY);
const safeBiometry = STORE.getItem(IS_BIOMETRY_ENABLED_KEY);
export async function biometricsMigration(currentPassword) {
const storeSafeBiometryFeature = !!STORE.getItem(SAFE_BIOMETRY_FEATURE_FLAG_KEY);
const unleashToggles = STORE.getItem(FEATURE_TOGGLES_LAST_KNOWN_VALUES_KEY) ?? {};
const unleashSafeBiometryFeature = !!unleashToggles[SAFE_BIOMETRY_MODE_FEATURE_TOGGLE];

if (storeSafeBiometryFeature === unleashSafeBiometryFeature) {
// No migration is required since the store and unleash flags are the same
return currentPassword;
}

STORE.setItem(SAFE_BIOMETRY_FEATURE_FLAG_KEY, unleashSafeBiometryFeature);
// The safe biometry feature flag has changed, we need to check if a migration is required.

if (safeBiometryEnabled) {
// Safe biometry mode, need to migrate if old biometry is enabled.
if (oldBiometry) {
if (unleashSafeBiometryFeature) {
// Unleash flag is enabling safe biometry
// if we have the old mode active a migration is required.
// else we can ignore migration since the user is not using biometry.
const oldBiometryActive = STORE.getItem(IS_OLD_BIOMETRY_ENABLED_KEY);
if (oldBiometryActive) {
// currentPassword is the pin, we need to generate a new random password
// and encrypt the pin.
const password = generateRandomPassword();
Expand All @@ -116,9 +127,12 @@ export async function biometricsMigration(currentPassword, safeBiometryEnabled)
return password;
}
} else {
// Old biometry mode, need to migrate if safe biometry is enabled.
// Unleash flag is disabling safe biometry
// if we have safe mode active a migration is required.
// else we can ignore migration since the use is not using biometry.
const safeBiometryActive = STORE.getItem(IS_BIOMETRY_ENABLED_KEY);
// eslint-disable-next-line no-lonely-if
if (safeBiometry) {
if (safeBiometryActive) {
// currentPassword is the random password, we need to decrypt the pin and
// toggle the old biometry key
const pin = STORE.disableSafeBiometry(currentPassword);
Expand Down

0 comments on commit f791e6d

Please sign in to comment.