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

Allow e.com to link straight to e.cash and share sessions #3715

Merged
merged 27 commits into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e40fe94
Add new validation routes
tgolen Jun 21, 2021
c6047a2
Use a constant for the screen names
tgolen Jun 21, 2021
27ddb14
Add the route to public screens as well
tgolen Jun 21, 2021
c4738e8
Add a comment for the route
tgolen Jun 21, 2021
90fc9dc
Move variable to a class property
tgolen Jun 21, 2021
9790dfd
Create separate pages for the separate routes
tgolen Jun 21, 2021
d8906d6
WIP on redirecting to new workspace page
tgolen Jun 21, 2021
fbafc61
Get redirection to the new workspace modal working
tgolen Jun 21, 2021
2819a23
Add a user action for getting an authToken
tgolen Jun 21, 2021
a4d43e2
Lint and show proper error message
tgolen Jun 21, 2021
6eac5bb
Rename action method
tgolen Jun 22, 2021
09bded2
Move method to Session action and implement API method
tgolen Jun 22, 2021
a0bb25d
Pass the 2FA code to action method
tgolen Jun 22, 2021
af4cfeb
Remove unneeded data from request
tgolen Jun 24, 2021
d32f02c
Merge branch 'main' into tgolen-validate-redirect
tgolen Jul 1, 2021
3880ba8
Improve comment, redirect in the action file
tgolen Jul 2, 2021
0d5d4f3
Merge branch 'main' into tgolen-validate-redirect
tgolen Jul 2, 2021
f1a55a5
Remove extra call to continue session
tgolen Jul 6, 2021
74b9a1e
Merge branch 'main' into tgolen-validate-redirect
tgolen Jul 6, 2021
243b0f8
Remove extra navigation
tgolen Jul 6, 2021
5054ca4
Fix prop types
tgolen Jul 6, 2021
554b61c
Lower case the translate key
tgolen Jul 6, 2021
504597f
Remove extra space
tgolen Jul 6, 2021
636ad0b
Upper case API method.
tgolen Jul 6, 2021
134564e
Make default screen options into a reusable module
tgolen Jul 6, 2021
a94f7ab
Reword form prompt
tgolen Jul 7, 2021
f9f3c61
Set the loading state to true
tgolen Jul 7, 2021
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
5 changes: 5 additions & 0 deletions src/ROUTES.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ export default {
getReportDetailsRoute: reportID => `r/${reportID}/details`,
VALIDATE_LOGIN: 'v',
VALIDATE_LOGIN_WITH_VALIDATE_CODE: 'v/:accountID/:validateCode',

// This is a special validation URL that will take the user to /workspace/new after validation. This is used
// when linking users from e.com in order to share a session in this app.
VALIDATE_LOGIN_WITH_VALIDATE_CODE_NEW_WORKSPACE: 'v/:accountID/:validateCode/new-workspace',
VALIDATE_LOGIN_WITH_VALIDATE_CODE_2FA_NEW_WORKSPACE: 'v/:accountID/:validateCode/2fa/new-workspace',
ENABLE_PAYMENTS: 'enable-payments',
WORKSPACE_NEW: 'workspace/new',
WORKSPACE: 'workspace',
Expand Down
2 changes: 2 additions & 0 deletions src/SCREENS.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ export default {
HOME: 'Home',
LOADING: 'Loading',
REPORT: 'Report',
VALIDATE_LOGIN_NEW_WORKSPACE: 'ValidateLoginNewWorkspace',
VALIDATE_LOGIN_2FA_NEW_WORKSPACE: 'ValidateLogin2FANewWorkspace',
};
1 change: 1 addition & 0 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ export default {
},
passwordForm: {
pleaseFillOutAllFields: 'Please fill out all fields',
enterATwoFactorAuthenticationCodeToContinue: 'Enter a two factor authentication code to continue',
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: I feel like your reads better than a here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I can change this.

forgot: 'Forgot?',
twoFactorCode: 'Two Factor Code',
requiredWhen2FAEnabled: 'Required when 2FA is enabled',
Expand Down
29 changes: 28 additions & 1 deletion src/libs/API.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@ function Authenticate(parameters) {
'partnerUserSecret',
], parameters, commandName);

// eslint-disable-next-line no-use-before-define
return Network.post(commandName, {
// When authenticating for the first time, we pass useExpensifyLogin as true so we check
// for credentials for the expensify partnerID to let users Authenticate with their expensify user
Expand Down Expand Up @@ -296,6 +295,33 @@ function reauthenticate(command = '') {
});
}

/**
* Calls the command=Authenticate API with an accountID, validateCode, and optional 2FA code. This is used specifically
* for sharing sessions between e.com and this app. It will return an authToken that is used for initiating a session
* in this app. This API call doesn't have any special handling (like retries or special error handling).
*
* @param {Object} parameters
* @param {String} parameters.accountID
* @param {String} parameters.validateCode
* @param {String} [parameters.twoFactorAuthCode]
* @returns {Promise<unknown>}
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB / just curious but why Promise<unknown>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think PHPStorm did that automatically, so I'm not sure why! I'll remove it

*/
function AuthenticateWithAccountID(parameters) {
const commandName = 'Authenticate';

requireParameters([
'accountID',
'validateCode',
], parameters, commandName);

return Network.post(commandName, {
accountID: parameters.accountID,
validateCode: parameters.validateCode,
twoFactorAuthCode: parameters.twoFactorAuthCode,
doNotRetry: true,
});
}

/**
* @param {Object} parameters
* @param {String} parameters.oldPassword
Expand Down Expand Up @@ -994,6 +1020,7 @@ function Inbox_CallUser(parameters) {

export {
Authenticate,
AuthenticateWithAccountID,
BankAccount_Create,
BankAccount_Get,
BankAccount_SetupWithdrawal,
Expand Down
13 changes: 13 additions & 0 deletions src/libs/Navigation/AppNavigator/AuthScreens.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ import {
} from './ModalStackNavigators';
import SCREENS from '../../../SCREENS';
import Timers from '../../Timers';
import ValidateLoginNewWorkspacePage from '../../../pages/ValidateLoginNewWorkspacePage';
import ValidateLogin2FANewWorkspacePage from '../../../pages/ValidateLogin2FANewWorkspacePage';
import WorkspaceSettingsDrawerNavigator from './WorkspaceSettingsDrawerNavigator';
import defaultScreenOptions from './defaultScreenOptions';

Onyx.connect({
key: ONYXKEYS.MY_PERSONAL_DETAILS,
Expand Down Expand Up @@ -253,6 +256,16 @@ class AuthScreens extends React.Component {
}}
component={ValidateLoginPage}
/>
<RootStack.Screen
name={SCREENS.VALIDATE_LOGIN_NEW_WORKSPACE}
options={defaultScreenOptions}
component={ValidateLoginNewWorkspacePage}
/>
<RootStack.Screen
name={SCREENS.VALIDATE_LOGIN_2FA_NEW_WORKSPACE}
options={defaultScreenOptions}
component={ValidateLogin2FANewWorkspacePage}
/>

{/* These are the various modal routes */}
{/* Note: Each modal must have it's own stack navigator since we want to be able to navigate to any
Expand Down
20 changes: 13 additions & 7 deletions src/libs/Navigation/AppNavigator/PublicScreens.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,11 @@ import SetPasswordPage from '../../../pages/SetPasswordPage';
import PublicWorkspaceNewView from '../../../pages/workspace/PublicWorkspaceNewView';
import ValidateLoginPage from '../../../pages/ValidateLoginPage';
import SCREENS from '../../../SCREENS';
import ValidateLoginNewWorkspacePage from '../../../pages/ValidateLoginNewWorkspacePage';
import ValidateLogin2FANewWorkspacePage from '../../../pages/ValidateLogin2FANewWorkspacePage';
import defaultScreenOptions from './defaultScreenOptions';

const RootStack = createStackNavigator();
const defaultScreenOptions = {
cardStyle: {
overflow: 'visible',
},
headerShown: false,
animationTypeForReplace: 'pop',
};

export default () => (
<RootStack.Navigator>
Expand All @@ -36,5 +32,15 @@ export default () => (
name="NewWorkspace"
component={PublicWorkspaceNewView}
/>
<RootStack.Screen
name={SCREENS.VALIDATE_LOGIN_NEW_WORKSPACE}
options={defaultScreenOptions}
component={ValidateLoginNewWorkspacePage}
/>
<RootStack.Screen
name={SCREENS.VALIDATE_LOGIN_2FA_NEW_WORKSPACE}
options={defaultScreenOptions}
component={ValidateLogin2FANewWorkspacePage}
/>
</RootStack.Navigator>
);
9 changes: 9 additions & 0 deletions src/libs/Navigation/AppNavigator/defaultScreenOptions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const defaultScreenOptions = {
cardStyle: {
overflow: 'visible',
},
headerShown: false,
animationTypeForReplace: 'pop',
};

export default defaultScreenOptions;
9 changes: 6 additions & 3 deletions src/libs/Navigation/Navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@ function openDrawer() {

/**
* @private
* @param {Boolean} shouldOpenDrawer
*/
function goBack() {
function goBack(shouldOpenDrawer = true) {
if (!navigationRef.current.canGoBack()) {
console.debug('Unable to go back');
openDrawer();
if (shouldOpenDrawer) {
openDrawer();
}
return;
}

Expand Down Expand Up @@ -102,7 +105,7 @@ function dismissModal(shouldOpenDrawer = false) {
}

// Navigate back to where we were before we launched the modal
goBack();
goBack(shouldOpenDrawer);

if (!normalizedShouldOpenDrawer) {
return;
Expand Down
2 changes: 2 additions & 0 deletions src/libs/Navigation/linkingConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export default {
// Public Routes
SetPassword: ROUTES.SET_PASSWORD_WITH_VALIDATE_CODE,
ValidateLogin: ROUTES.VALIDATE_LOGIN_WITH_VALIDATE_CODE,
[SCREENS.VALIDATE_LOGIN_NEW_WORKSPACE]: ROUTES.VALIDATE_LOGIN_WITH_VALIDATE_CODE_NEW_WORKSPACE,
[SCREENS.VALIDATE_LOGIN_2FA_NEW_WORKSPACE]: ROUTES.VALIDATE_LOGIN_WITH_VALIDATE_CODE_2FA_NEW_WORKSPACE,

// Modal Screens
Settings: {
Expand Down
31 changes: 31 additions & 0 deletions src/libs/actions/Session.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import CONFIG from '../../CONFIG';
import PushNotification from '../Notification/PushNotification';
import Timing from './Timing';
import CONST from '../../CONST';
import Navigation from '../Navigation/Navigation';
import ROUTES from '../../ROUTES';
import {translateLocal} from '../translate';

let credentials = {};
Expand All @@ -20,6 +22,9 @@ Onyx.connect({
* Sets API data in the store when we make a successful "Authenticate"/"CreateLogin" request
*
* @param {Object} data
* @param {String} data.accountID
* @param {String} data.authToken
* @param {String} data.email
*/
function setSuccessfulSignInData(data) {
PushNotification.register(data.accountID);
Expand Down Expand Up @@ -269,7 +274,33 @@ function setPassword(password, validateCode, accountID) {
});
}

/**
* This is used when a user clicks on a link from e.com that goes to this application. We want the user to be able to
* be automatically logged into this app. If the user is not already logged into this app, then this method is called
* in order to retrieve an authToken from e.com and be signed in.
*
* @param {String} accountID
* @param {String} validateCode
* @param {String} [twoFactorAuthCode]
*/
function continueSessionFromECom(accountID, validateCode, twoFactorAuthCode) {
API.AuthenticateWithAccountID({
accountID,
validateCode,
twoFactorAuthCode,
}).then((data) => {
// If something failed, it doesn't really matter what, send the user to the sign in form to log in normally
if (data.jsonCode !== 200) {
Navigation.navigate(ROUTES.HOME);
return;
}

setSuccessfulSignInData(data);
});
}

export {
continueSessionFromECom,
fetchAccountDetails,
setPassword,
signIn,
Expand Down
146 changes: 146 additions & 0 deletions src/pages/ValidateLogin2FANewWorkspacePage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import React, {Component} from 'react';
import {Text, TextInput, View} from 'react-native';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import withLocalize, {withLocalizePropTypes} from '../components/withLocalize';
import validateLinkPropTypes from './validateLinkPropTypes';
import {continueSessionFromECom} from '../libs/actions/Session';
import styles from '../styles/styles';
import ExpensifyCashLogo from '../components/ExpensifyCashLogo';
import variables from '../styles/variables';
import themeColors from '../styles/themes/default';
import CONST from '../CONST';
import Button from '../components/Button';
import compose from '../libs/compose';
import ONYXKEYS from '../ONYXKEYS';
import Navigation from '../libs/Navigation/Navigation';
import ROUTES from '../ROUTES';

const propTypes = {
/* Onyx Props */

/** The data about the current session */
session: PropTypes.shape({
/** The authToken for the current session */
authToken: PropTypes.string,
}),

/** The accountID and validateCode are passed via the URL */
route: validateLinkPropTypes,

...withLocalizePropTypes,
};

const defaultProps = {
route: {
params: {},
},
session: {},
};
class ValidateLogin2FANewWorkspacePage extends Component {
constructor(props) {
super(props);

this.validateAndSubmitForm = this.validateAndSubmitForm.bind(this);

this.state = {
twoFactorAuthCode: '',
formError: false,
loading: false,
};
}

componentDidMount() {
// If the user has an active session already, they need to be redirected straight to the new workspace page
if (this.props.session.authToken) {
// In order to navigate to a modal, we first have to dismiss the current modal. But there is no current
// modal you say? I know, it confuses me too. Without dismissing the current modal, if they user cancels
// out of the new workspace modal, then they will be routed back to
Copy link
Contributor

Choose a reason for hiding this comment

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

But there is no current modal you say? I know, it confuses me too.

NAB, I can see how this could be confusing (because the modal is not visible), but I think maybe it's not that mysterious. Curious if we should replace this commentary with a better explanation and have an idea about what's happening here...

ValidateLogin2FANewWorkspacePage is the modal we are dismissing, it "exists", but the page is returning null. So that's why we need to remove it from the history stack or else when we "goBack()" we will navigate to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm all in favor of updating this comment to make more sense. If the real problem is that ValidateLogin2FANewWorkspacePage is in the history stack, maybe there is a better way for us to remove it from the stack without needing to call dismissModal()?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe there is a better way for us to remove it from the stack without needing to call dismissModal()

I'm having trouble working out a better way. It feels like there are contexts where you'd want the default navigate() method to preserve the underlying modal in the stack e.g. moving from /settings to /settings/payments and contexts where you'll want a different behavior. We could maybe add a parameter to navigate() that forces all other modals to close?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All right, I was able to fix this. Turns out, I was navigating to that modal twice (once in Session, and again from ComponentDidMount (from when the same page was remounted: once in public screens and then again in auth screens).

Once I removed the navigation in the Session action, it all worked like a charm.

// /v/<accountID>/<validateCode>/new-workspace and we don't want that. We want them to go back to `/` and
// by calling dismissModal(), the /v/... route is removed from history so the user will get taken to `/`
// if they cancel out of the new workspace modal.
Navigation.dismissModal();
Navigation.navigate(ROUTES.WORKSPACE_NEW);
}
}

validateAndSubmitForm() {
if (!this.state.twoFactorAuthCode.trim()) {
this.setState({formError: this.props.translate('passwordForm.pleaseFillOutAllFields')});
return;
}

this.setState({
formError: null,
});

const accountID = lodashGet(this.props.route.params, 'accountID', '');
const validateCode = lodashGet(this.props.route.params, 'validateCode', '');
continueSessionFromECom(accountID, validateCode, this.state.twoFactorAuthCode);
}

render() {
// If the user is already logged in, don't need to display anything because they will get redirected to the
// new workspace page in componentDidMount
if (this.props.session.authToken) {
return null;
}

return (
<View style={[styles.signInPageInnerNative]}>
<View style={[styles.signInPageLogoNative]}>
<ExpensifyCashLogo width={variables.componentSizeLarge} height={variables.componentSizeLarge} />
</View>
<View style={[styles.mb6, styles.alignItemsCenter]}>
<Text style={[styles.h1]}>
{this.props.translate('signInPage.expensifyDotCash')}
</Text>
</View>

<View style={[styles.signInPageFormContainer]}>
<View style={[styles.mb4]}>
<Text style={[styles.formLabel]}>
{this.props.translate('passwordForm.enterATwoFactorAuthenticationCodeToContinue')}
</Text>
<TextInput
style={[styles.textInput]}
value={this.state.twoFactorAuthCode}
placeholder={this.props.translate('passwordForm.twoFactorCode')}
placeholderTextColor={themeColors.placeholderText}
onChangeText={text => this.setState({twoFactorAuthCode: text})}
onSubmitEditing={this.validateAndSubmitForm}
keyboardType={CONST.KEYBOARD_TYPE.NUMERIC}
/>
</View>
<View>
<Button
success
style={[styles.mb2]}
text={this.props.translate('common.continue')}
isLoading={this.state.loading}
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: We never set this anywhere but the constructor, meaning the user has no feedback as to when we are actually loading

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll see about cleaning this up.

onPress={this.validateAndSubmitForm}
/>
</View>
{this.state.formError && (
<Text style={[styles.formError]}>
{this.state.formError}
</Text>
)}
</View>
</View>
);
}
}

ValidateLogin2FANewWorkspacePage.propTypes = propTypes;
ValidateLogin2FANewWorkspacePage.defaultProps = defaultProps;

export default compose(
withLocalize,
withOnyx({
session: {
key: ONYXKEYS.SESSION,
},
}),
)(ValidateLogin2FANewWorkspacePage);
Loading