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

Redirect internally for 2FA management from bank account step #30746

Merged
merged 13 commits into from
Nov 29, 2023
5 changes: 4 additions & 1 deletion src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@ export default {
getRoute: (contactMethod: string) => `settings/profile/contact-methods/${encodeURIComponent(contactMethod)}/details`,
},
SETTINGS_NEW_CONTACT_METHOD: 'settings/profile/contact-methods/new',
SETTINGS_2FA: 'settings/security/two-factor-auth',
SETTINGS_2FA: {
route: 'settings/security/two-factor-auth',
getRoute: (backTo?: string) => getUrlWithBackToParam('settings/security/two-factor-auth', backTo),
},
SETTINGS_STATUS: 'settings/profile/status',
SETTINGS_STATUS_SET: 'settings/profile/status/set',

Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/linkingConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ export default {
exact: true,
},
Settings_TwoFactorAuth: {
path: ROUTES.SETTINGS_2FA,
path: ROUTES.SETTINGS_2FA.route,
exact: true,
},
Settings_Share_Code: {
Expand Down
8 changes: 4 additions & 4 deletions src/libs/actions/TwoFactorAuthActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* Clear 2FA data if the flow is interrupted without finishing
*/
function clearTwoFactorAuthData() {
Onyx.merge(ONYXKEYS.ACCOUNT, {recoveryCodes: '', twoFactorAuthSecretKey: '', twoFactorAuthStep: '', codesAreCopied: false});
Onyx.merge(ONYXKEYS.ACCOUNT, {recoveryCodes: null, twoFactorAuthSecretKey: null, twoFactorAuthStep: null, codesAreCopied: false});
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: what's this change for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want to remove these data navigating away from 2FA setup. Setting it as null, removes the onyx data.

}
function setTwoFactorAuthStep(twoFactorAuthStep: TwoFactorAuthStep) {
Onyx.merge(ONYXKEYS.ACCOUNT, {twoFactorAuthStep});
Expand All @@ -18,9 +18,9 @@
Onyx.merge(ONYXKEYS.ACCOUNT, {codesAreCopied: true});
}

function quitAndNavigateBackToSettings() {
function quitAndNavigateBack(backTo?: string) {
clearTwoFactorAuthData();
Navigation.goBack(ROUTES.SETTINGS_SECURITY);
Navigation.goBack(backTo ? backTo : ROUTES.SETTINGS_SECURITY);

Check failure on line 23 in src/libs/actions/TwoFactorAuthActions.ts

View workflow job for this annotation

GitHub Actions / lint

Unnecessary use of conditional expression for default assignment
Copy link
Contributor

Choose a reason for hiding this comment

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

To fix TS lint:

Suggested change
Navigation.goBack(backTo ? backTo : ROUTES.SETTINGS_SECURITY);
Navigation.goBack(backTo ? `${backTo}` : ROUTES.SETTINGS_SECURITY);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't using backTo || ROUTES.SETTINGS_SECURITY the right way?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's against our TS rule.

Screenshot 2023-11-27 at 4 42 34 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

@blazejkustra what's the best practice for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

My suggestion would be fine. Otherwise just disable lint for this line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

null coalesce won't work in this case. I think it's fair to use || in this case

}

export {clearTwoFactorAuthData, setTwoFactorAuthStep, quitAndNavigateBackToSettings, setCodesAreCopied};
export {clearTwoFactorAuthData, setTwoFactorAuthStep, quitAndNavigateBack, setCodesAreCopied};
23 changes: 12 additions & 11 deletions src/pages/ReimbursementAccount/Enable2FAPrompt.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,44 @@
import PropTypes from 'prop-types';
import React from 'react';
import {View} from 'react-native';
import * as Expensicons from '@components/Icon/Expensicons';
import * as Illustrations from '@components/Icon/Illustrations';
import Section from '@components/Section';
import Text from '@components/Text';
import withLocalize, {withLocalizePropTypes} from '@components/withLocalize';
import Navigation from '@navigation/Navigation';
import useThemeStyles from '@styles/useThemeStyles';
import * as Link from '@userActions/Link';
import ROUTES from '@src/ROUTES';
import withLocalize, {withLocalizePropTypes} from "@components/withLocalize";

const propTypes = {
...withLocalizePropTypes,

/** policyID of the workspace where user is setting up bank account */
policyID: PropTypes.string.isRequired,
};
function Enable2FAPrompt(props) {

function Enable2FAPrompt({translate, policyID}) {
const styles = useThemeStyles();
const secureYourAccountUrl = encodeURI(
`settings?param={"section":"account","action":"enableTwoFactorAuth","exitTo":"${ROUTES.BANK_ACCOUNT_WITH_STEP_TO_OPEN.getRoute()}","isFromNewDot":"true"}`,
);

return (
<Section
title={props.translate('validationStep.enable2FATitle')}
title={translate('validationStep.enable2FATitle')}
icon={Illustrations.ShieldYellow}
menuItems={[
{
title: props.translate('validationStep.secureYourAccount'),
title: translate('validationStep.secureYourAccount'),
onPress: () => {
Link.openOldDotLink(secureYourAccountUrl);
Navigation.navigate(ROUTES.SETTINGS_2FA.getRoute(ROUTES.BANK_ACCOUNT_WITH_STEP_TO_OPEN.getRoute('', policyID)));
},
icon: Expensicons.Shield,
shouldShowRightIcon: true,
iconRight: Expensicons.NewWindow,
wrapperStyle: [styles.cardMenuItem],
link: () => Link.buildOldDotURL(secureYourAccountUrl),
},
]}
>
<View style={[styles.mv3]}>
<Text>{props.translate('validationStep.enable2FAText')}</Text>
<Text>{translate('validationStep.enable2FAText')}</Text>
</View>
</Section>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ class ReimbursementAccountPage extends React.Component {
<ValidationStep
reimbursementAccount={this.props.reimbursementAccount}
onBackButtonPress={this.goBack}
policyID={policyID}
/>
);
}
Expand Down
9 changes: 6 additions & 3 deletions src/pages/ReimbursementAccount/ValidationStep.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const propTypes = {
/** If user has two-factor authentication enabled */
requiresTwoFactorAuth: PropTypes.bool,
}),

/** policyID of the workspace where user is setting up bank account */
policyID: PropTypes.string.isRequired,
};

const defaultProps = {
Expand Down Expand Up @@ -73,7 +76,7 @@ const filterInput = (amount) => {
return value;
};

function ValidationStep({reimbursementAccount, translate, onBackButtonPress, account}) {
function ValidationStep({reimbursementAccount, translate, onBackButtonPress, account, policyID}) {
const styles = useThemeStyles();
/**
* @param {Object} values - form input values passed by the Form component
Expand Down Expand Up @@ -180,7 +183,7 @@ function ValidationStep({reimbursementAccount, translate, onBackButtonPress, acc
</View>
{!requiresTwoFactorAuth && (
<View style={[styles.mln5, styles.mrn5]}>
<Enable2FAPrompt />
<Enable2FAPrompt policyID={policyID} />
</View>
)}
</Form>
Expand Down Expand Up @@ -211,7 +214,7 @@ function ValidationStep({reimbursementAccount, translate, onBackButtonPress, acc
/>
</Section>
{reimbursementAccount.shouldShowResetModal && <WorkspaceResetBankAccountModal reimbursementAccount={reimbursementAccount} />}
{!requiresTwoFactorAuth && <Enable2FAPrompt />}
{!requiresTwoFactorAuth && <Enable2FAPrompt policyID={policyID} />}
</ScrollView>
)}
</ScreenWrapper>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Security/SecuritySettingsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function SecuritySettingsPage(props) {
{
translationKey: 'twoFactorAuth.headerTitle',
icon: Expensicons.Shield,
action: waitForNavigate(() => Navigation.navigate(ROUTES.SETTINGS_2FA)),
action: waitForNavigate(() => Navigation.navigate(ROUTES.SETTINGS_2FA.getRoute())),
},
{
translationKey: 'closeAccountPage.closeAccount',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import StepWrapperPropTypes from './StepWrapperPropTypes';
function StepWrapper({
title = '',
stepCounter = null,
onBackButtonPress = TwoFactorAuthActions.quitAndNavigateBackToSettings,
onBackButtonPress = TwoFactorAuthActions.quitAndNavigateBack,
MonilBhavsar marked this conversation as resolved.
Show resolved Hide resolved
children = null,
shouldEnableKeyboardAvoidingView = true,
onEntryTransitionEnd,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import * as TwoFactorAuthActions from '@userActions/TwoFactorAuthActions';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';

function CodesStep({account = defaultAccount}) {
function CodesStep({account = defaultAccount, backTo}) {
const theme = useTheme();
const styles = useThemeStyles();
const {translate} = useLocalize();
Expand All @@ -50,6 +50,7 @@ function CodesStep({account = defaultAccount}) {
text: translate('twoFactorAuth.stepCodes'),
total: 3,
}}
onBackButtonPress={() => TwoFactorAuthActions.quitAndNavigateBack(backTo)}
>
<ScrollView contentContainerStyle={styles.flexGrow1}>
<Section
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function DisabledStep() {
<Button
success
text={translate('common.buttonConfirm')}
onPress={TwoFactorAuthActions.quitAndNavigateBackToSettings}
onPress={TwoFactorAuthActions.quitAndNavigateBack}
MonilBhavsar marked this conversation as resolved.
Show resolved Hide resolved
/>
</FixedFooter>
</StepWrapper>
Expand Down
19 changes: 18 additions & 1 deletion src/pages/settings/Security/TwoFactorAuth/Steps/SuccessStep.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
import PropTypes from 'prop-types';
import React from 'react';
import ConfirmationPage from '@components/ConfirmationPage';
import LottieAnimations from '@components/LottieAnimations';
import useLocalize from '@hooks/useLocalize';
import Navigation from '@navigation/Navigation';
import StepWrapper from '@pages/settings/Security/TwoFactorAuth/StepWrapper/StepWrapper';
import useTwoFactorAuthContext from '@pages/settings/Security/TwoFactorAuth/TwoFactorAuthContext/useTwoFactorAuth';
import * as TwoFactorAuthActions from '@userActions/TwoFactorAuthActions';
import CONST from '@src/CONST';

function SuccessStep() {
const propTypes = {
/** The route where user needs to be redirected after setting up 2FA */
backTo: PropTypes.string,
};

const defaultProps = {
backTo: '',
};

function SuccessStep({backTo}) {
const {setStep} = useTwoFactorAuthContext();

const {translate} = useLocalize();
Expand All @@ -29,10 +40,16 @@ function SuccessStep() {
onButtonPress={() => {
TwoFactorAuthActions.clearTwoFactorAuthData();
setStep(CONST.TWO_FACTOR_AUTH_STEPS.ENABLED);
if (backTo) {
Navigation.navigate(backTo);
}
}}
/>
</StepWrapper>
);
}

SuccessStep.propTypes = propTypes;
SuccessStep.defaultProps = defaultProps;

export default SuccessStep;
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import {useRoute} from '@react-navigation/native';
import lodashGet from 'lodash/get';
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import {withOnyx} from 'react-native-onyx';
import useAnimatedStepContext from '@components/AnimatedStep/useAnimatedStepContext';
Expand All @@ -13,6 +15,8 @@ import TwoFactorAuthContext from './TwoFactorAuthContext';
import {defaultAccount, TwoFactorAuthPropTypes} from './TwoFactorAuthPropTypes';

function TwoFactorAuthSteps({account = defaultAccount}) {
const route = useRoute();
const backTo = lodashGet(route.params, 'backTo', '');
const [currentStep, setCurrentStep] = useState(CONST.TWO_FACTOR_AUTH_STEPS.CODES);

const {setAnimationDirection} = useAnimatedStepContext();
Expand Down Expand Up @@ -45,17 +49,17 @@ function TwoFactorAuthSteps({account = defaultAccount}) {
const renderStep = () => {
switch (currentStep) {
case CONST.TWO_FACTOR_AUTH_STEPS.CODES:
return <CodesStep />;
return <CodesStep backTo={backTo} />;
case CONST.TWO_FACTOR_AUTH_STEPS.VERIFY:
return <VerifyStep />;
case CONST.TWO_FACTOR_AUTH_STEPS.SUCCESS:
return <SuccessStep />;
return <SuccessStep backTo={backTo} />;
case CONST.TWO_FACTOR_AUTH_STEPS.ENABLED:
return <EnabledStep />;
case CONST.TWO_FACTOR_AUTH_STEPS.DISABLED:
return <DisabledStep />;
default:
return <CodesStep />;
return <CodesStep backTo={backTo} />;
}
};

Expand Down
Loading