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

Fix magic code expired is shown briefly #49042

Merged
Prev Previous commit
Next Next commit
fix magic code expired is shown briefly
bernhardoj committed Sep 12, 2024

Unverified

This user has not yet uploaded their public signing key.
commit 8462b926d09771b33f3b6050ee204e207620553d
38 changes: 19 additions & 19 deletions src/pages/ValidateLoginPage/index.website.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useEffect, useRef} from 'react';
import React, {useEffect} from 'react';
import {withOnyx} from 'react-native-onyx';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import ExpiredValidateCodeModal from '@components/ValidateCode/ExpiredValidateCodeModal';
@@ -18,24 +18,19 @@ function ValidateLoginPage({
params: {accountID, validateCode, exitTo},
},
session,
autoAuthState: autoAuthStateProp,
}: ValidateLoginPageProps<ValidateLoginPageOnyxProps>) {
const firstRenderRef = useRef(true);
const login = credentials?.login;
const autoAuthState = session?.autoAuthState ?? CONST.AUTO_AUTH_STATE.NOT_STARTED;
const isSignedIn = !!session?.authToken && session?.authTokenType !== CONST.AUTH_TOKEN_TYPES.ANONYMOUS;
// We don't want the previous autoAuthState affects the rendering of the current magic link page, so the autoAuthState prop sets initWithStoredValues as false,
// except if the user is signed in because the page will be remounted when successfully signed in as explained in Session.initAutoAuthState.
const autoAuthState = isSignedIn ? session?.autoAuthState : autoAuthStateProp;
const autoAuthStateWithDefault = autoAuthState ?? CONST.AUTO_AUTH_STATE.NOT_STARTED;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having autoAuthState as undefined initially at first render works fine except when the user is successfully logged in from the magic link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The rest of the changes is just renaming.

const is2FARequired = !!account?.requiresTwoFactorAuth;
const cachedAccountID = credentials?.accountID;
const isUserClickedSignIn = !login && isSignedIn && (autoAuthState === CONST.AUTO_AUTH_STATE.SIGNING_IN || autoAuthState === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN);
const isUserClickedSignIn = !login && isSignedIn && (autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.SIGNING_IN || autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN);
const shouldStartSignInWithValidateCode = !isUserClickedSignIn && !isSignedIn && (!!login || !!exitTo);

// If the magic link was previously expired (FAILED), we want to clear the failed state so the magic code expired won't show briefly.
if (firstRenderRef.current) {
firstRenderRef.current = false;
if (autoAuthState === CONST.AUTO_AUTH_STATE.FAILED) {
Session.clearAutoAuthState();
}
}

useEffect(() => {
if (isUserClickedSignIn) {
// The user clicked the option to sign in the current tab
@@ -44,7 +39,7 @@ function ValidateLoginPage({
});
return;
}
Session.initAutoAuthState(autoAuthState);
Session.initAutoAuthState(autoAuthStateWithDefault);

if (!shouldStartSignInWithValidateCode) {
if (exitTo) {
@@ -59,7 +54,7 @@ function ValidateLoginPage({
// Since on Desktop we don't have multi-tab functionality to handle the login flow,
// we need to `popToTop` the stack after `signInWithValidateCode` in order to
// perform login for both 2FA and non-2FA accounts.
desktopLoginRedirect(autoAuthState, isSignedIn);
desktopLoginRedirect(autoAuthStateWithDefault, isSignedIn);
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, []);

@@ -79,17 +74,17 @@ function ValidateLoginPage({

return (
<>
{autoAuthState === CONST.AUTO_AUTH_STATE.FAILED && <ExpiredValidateCodeModal />}
{autoAuthState === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN && is2FARequired && !isSignedIn && login && <JustSignedInModal is2FARequired />}
{autoAuthState === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN && isSignedIn && !exitTo && login && <JustSignedInModal is2FARequired={false} />}
{autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.FAILED && <ExpiredValidateCodeModal />}
{autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN && is2FARequired && !isSignedIn && login && <JustSignedInModal is2FARequired />}
{autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN && isSignedIn && !exitTo && login && <JustSignedInModal is2FARequired={false} />}
{/* If session.autoAuthState isn't available yet, we use shouldStartSignInWithValidateCode to conditionally render the component instead of local autoAuthState which contains a default value of NOT_STARTED */}
{(!session?.autoAuthState ? !shouldStartSignInWithValidateCode : autoAuthState === CONST.AUTO_AUTH_STATE.NOT_STARTED) && !exitTo && (
{(!autoAuthState ? !shouldStartSignInWithValidateCode : autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.NOT_STARTED) && !exitTo && (
<ValidateCodeModal
accountID={Number(accountID)}
code={validateCode}
/>
)}
{(!session?.autoAuthState ? shouldStartSignInWithValidateCode : autoAuthState === CONST.AUTO_AUTH_STATE.SIGNING_IN) && <FullScreenLoadingIndicator />}
{(!autoAuthState ? shouldStartSignInWithValidateCode : autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.SIGNING_IN) && <FullScreenLoadingIndicator />}
</>
);
}
@@ -100,4 +95,9 @@ export default withOnyx<ValidateLoginPageProps<ValidateLoginPageOnyxProps>, Vali
account: {key: ONYXKEYS.ACCOUNT},
credentials: {key: ONYXKEYS.CREDENTIALS},
session: {key: ONYXKEYS.SESSION},
autoAuthState: {
key: ONYXKEYS.SESSION,
selector: (session) => session?.autoAuthState,
initWithStoredValues: false,
},
})(ValidateLoginPage);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of clearing the autoAuthState on mounted, I connect a new onyx key specifically fo fetch the autoAuthState with initWithStoredValues as false so the value is always undefined at first render.

3 changes: 3 additions & 0 deletions src/pages/ValidateLoginPage/types.ts
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ import type {OnyxEntry} from 'react-native-onyx';
import type {PublicScreensParamList} from '@libs/Navigation/types';
import type SCREENS from '@src/SCREENS';
import type {Account, Credentials, Session} from '@src/types/onyx';
import type {AutoAuthState} from '@src/types/onyx/Session';

type ValidateLoginPageOnyxNativeProps = {
/** Session of currently logged in user */
@@ -15,6 +16,8 @@ type ValidateLoginPageOnyxProps = ValidateLoginPageOnyxNativeProps & {

/** The credentials of the person logging in */
credentials: OnyxEntry<Credentials>;

autoAuthState: OnyxEntry<AutoAuthState>;
};

type ValidateLoginPageProps<OnyxProps> = OnyxProps & StackScreenProps<PublicScreensParamList, typeof SCREENS.VALIDATE_LOGIN>;