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

Sign in, out, or navigate on transition mount #8257

Merged
merged 14 commits into from
Mar 23, 2022
Merged
6 changes: 4 additions & 2 deletions src/libs/actions/Session/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,10 @@ function fetchAccountDetails(login) {
*
* @param {String} authToken
* @param {String} email
* @param {Boolean} shouldProcessImmediately
* @return {Promise}
*/
function createTemporaryLogin(authToken, email) {
function createTemporaryLogin(authToken, email, shouldProcessImmediately = true) {
const autoGeneratedLogin = Str.guid('expensify.cash-');
const autoGeneratedPassword = Str.guid();

Expand All @@ -188,6 +189,7 @@ function createTemporaryLogin(authToken, email) {
partnerUserSecret: autoGeneratedPassword,
shouldRetry: false,
forceNetworkRequest: true,
shouldProcessImmediately,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here - I'm having trouble seeing how this is connected to the transition stuff.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed this code and it doesn't seem to impact anything.

email,
includeEncryptedAuthToken: true,
})
Expand Down Expand Up @@ -268,7 +270,7 @@ function signIn(password, twoFactorAuthCode) {
function signInWithShortLivedToken(accountID, email, shortLivedToken) {
Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true});

createTemporaryLogin(shortLivedToken, email).then((response) => {
createTemporaryLogin(shortLivedToken, email, false).then((response) => {
Onyx.merge(ONYXKEYS.SESSION, {
accountID,
email,
Expand Down
4 changes: 3 additions & 1 deletion src/libs/actions/WelcomeActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ function show({routes, toggleCreateMenu}) {
// If we are rendering the SidebarScreen at the same time as a workspace route that means we've already created a workspace via workspace/new and should not open the global
// create menu right now.
const topRouteName = lodashGet(_.last(routes), 'name', '');
const isDisplayingWorkspaceRoute = topRouteName.toLowerCase().includes('workspace');
const loginWithShortLivedTokenRoute = _.find(routes, route => route.name === 'LogInWithShortLivedToken');
const exitingToWorkspaceRoute = lodashGet(loginWithShortLivedTokenRoute, 'params.exitTo', '') === 'workspace/new';
Copy link
Contributor

Choose a reason for hiding this comment

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

Added this otherwise we would pop open the create menu in some contexts. I guess the top route is not always "workspace" and sometimes still the LoginWithShortLivedToken page.

const isDisplayingWorkspaceRoute = topRouteName.toLowerCase().includes('workspace') || exitingToWorkspaceRoute;

// It's also possible that we already have a workspace policy. In either case we will not toggle the menu but do still want to set the NVP in this case
// since the user does not need to create a workspace.
Expand Down
33 changes: 10 additions & 23 deletions src/pages/LogInWithShortLivedTokenPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import ONYXKEYS from '../ONYXKEYS';
import * as Session from '../libs/actions/Session';
import FullScreenLoadingIndicator from '../components/FullscreenLoadingIndicator';
import Navigation from '../libs/Navigation/Navigation';
import Log from '../libs/Log';

const propTypes = {
/** The parameters needed to authenticate with a short lived token are in the URL */
Expand Down Expand Up @@ -55,38 +56,29 @@ class LogInWithShortLivedTokenPage extends Component {

const isUserSignedIn = this.props.session && this.props.session.authToken;
if (!isUserSignedIn) {
Log.info('[LoginWithShortLivedTokenPage] User not signed in - signing in with short lived token');
Session.signInWithShortLivedToken(accountID, email, shortLivedToken);
return;
}

this.signOutIfNeeded(email);
}

componentDidUpdate() {
if (!lodashGet(this.props, 'session.authToken', null)) {
if (this.signOutIfNeeded(email)) {
return;
roryabraham marked this conversation as resolved.
Show resolved Hide resolved
}

const email = lodashGet(this.props.route.params, 'email', '');
Log.info('[LoginWithShortLivedTokenPage] User is signed in');

// exitTo is URI encoded because it could contain a variable number of slashes (i.e. "workspace/new" vs "workspace/<ID>/card")
const exitTo = decodeURIComponent(lodashGet(this.props.route.params, 'exitTo', ''));

if (this.signOutIfNeeded(email)) {
return;
}

if (exitTo === ROUTES.WORKSPACE_NEW) {
// New workspace creation is handled in AuthScreens, not in its own screen
Log.info('[LoginWithShortLivedTokenPage] exitTo is workspace/new - handling new workspace creation in AuthScreens');
return;
}

// 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 the user cancels out
// of the workspace modal, then they will be routed back to
// /transition/<accountID>/<email>/<authToken>/workspace/<policyID>/card and we don't want that. We want them to go back to `/`
// and by calling dismissModal(), the /transition/... route is removed from history so the user will get taken to `/`
// if they cancel out of the new workspace modal.
// In order to navigate to a modal, we first have to dismiss the current modal. Without dismissing the current modal, if the user cancels out of the workspace modal,
// then they will be routed back to /transition/<accountID>/<email>/<authToken>/workspace/<policyID>/card and we don't want that. We want them to go back to `/`
// and by calling dismissModal(), the /transition/... route is removed from history so the user will get taken to `/` if they cancel out of the new workspace modal.
Log.info('[LoginWithShortLivedTokenPage] Dismissing LoginWithShortLivedTokenPage and navigating to exitTo');
Navigation.dismissModal();
Navigation.navigate(exitTo);
}
Expand All @@ -105,6 +97,7 @@ class LogInWithShortLivedTokenPage extends Component {
return false;
}

Log.info('[LoginWithShortLivedTokenPage] Different user signed in - signing out');
Session.signOutAndRedirectToSignIn();
return true;
}
Expand All @@ -121,10 +114,4 @@ export default withOnyx({
session: {
key: ONYXKEYS.SESSION,
},

// We need to subscribe to the betas so that componentDidUpdate will run,
// causing us to exit to the proper page.
betas: {
key: ONYXKEYS.BETAS,
},
})(LogInWithShortLivedTokenPage);