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

Add unlink secondary login flow #15811

Merged
merged 27 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c0ddf22
add unlinkLoginForm
NikkiWines Mar 10, 2023
5213996
update SignInPage to conditionally show unlink form
NikkiWines Mar 10, 2023
58281b8
Merge branch 'main' of github.com:Expensify/App into nikki-unlink-login
NikkiWines Mar 13, 2023
7ba9f6e
Add Session.requestUnlinkValidationLink command
NikkiWines Mar 14, 2023
810b3b1
Add unlinkLogin command
NikkiWines Mar 14, 2023
0d105e6
jsx style
NikkiWines Mar 15, 2023
a7dcb51
Merge branch 'main' of github.com:Expensify/App into nikki-unlink-login
NikkiWines Apr 3, 2023
e686df5
Add UnlinkLogin route and page
NikkiWines Apr 4, 2023
a7dc7c7
clear login from credentials, add some spanish translations, add dot …
NikkiWines Apr 5, 2023
5cc40ba
move styles to prevent padding jump when moving between sign in views
NikkiWines Apr 6, 2023
0b2ff29
update copy
NikkiWines Apr 7, 2023
f35a043
NikkiWines Apr 7, 2023
b175e73
minor style + copy update
NikkiWines Apr 11, 2023
cda9544
Merge branch 'nikki-unlink-login' of github.com:Expensify/App into ni…
NikkiWines Apr 11, 2023
1640222
Merge branch 'main' of https://github.com/Expensify/App into nikki-un…
NikkiWines Apr 14, 2023
1649b94
don't require credentials prop
NikkiWines Apr 15, 2023
51ae018
add default value for prop
NikkiWines Apr 15, 2023
e2f67d0
Merge branch 'main' of https://github.com/Expensify/App into nikki-un…
NikkiWines Apr 19, 2023
722f0d7
remove unneeded dot indicator and failure messages
NikkiWines Apr 20, 2023
fafaf99
remove avatar + login from unlink form and add welcome text
NikkiWines Apr 21, 2023
fb78f5b
re-add dot indicator for non-error messages
NikkiWines Apr 21, 2023
cb88f68
remove unneeded imports
NikkiWines Apr 21, 2023
7325dff
Merge branch 'main' of https://github.com/Expensify/App into nikki-un…
NikkiWines Apr 28, 2023
a850b7c
ensure we have a primaryLogin before showing unlinkLoginForm
NikkiWines Apr 28, 2023
c12f968
Merge branch 'main' of https://github.com/Expensify/App into nikki-un…
NikkiWines May 4, 2023
f2333c6
optimistically remove message from account key when beginning sign in
NikkiWines May 4, 2023
93cf44c
minor style updates
NikkiWines May 5, 2023
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
1 change: 1 addition & 0 deletions src/ROUTES.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export default {
VALIDATE_LOGIN: 'v/:accountID/:validateCode',
GET_ASSISTANCE: 'get-assistance/:taskID',
getGetAssistanceRoute: taskID => `get-assistance/${taskID}`,
UNLINK_LOGIN: 'u/: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.
Expand Down
8 changes: 7 additions & 1 deletion src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,13 @@ export default {
linkHasBeenResent: 'Link has been re-sent',
weSentYouMagicSignInLink: ({login, loginType}) => `I've sent a magic sign-in link to ${login}. Please check your ${loginType} to sign in.`,
resendLink: 'Resend link',
validationCodeFailedMessage: 'It looks like there was an error with your validation link or it has expired.',
},
unlinkLoginForm: {
toValidateLogin: ({primaryLogin, secondaryLogin}) => `To validate ${secondaryLogin}, please resend the magic code from the Account Settings of ${primaryLogin}.`,
noLongerHaveAccess: ({primaryLogin}) => `If you no longer have access to ${primaryLogin}, please unlink your accounts.`,
unlink: 'Unlink',
linkSent: 'Link sent!',
succesfullyUnlinkedLogin: 'Secondary login successfully unlinked!',
},
detailsPage: {
localTime: 'Local time',
Expand Down
8 changes: 7 additions & 1 deletion src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,13 @@ export default {
linkHasBeenResent: 'El enlace se ha reenviado',
weSentYouMagicSignInLink: ({login, loginType}) => `Te he enviado un hiperenlace mágico para iniciar sesión a ${login}. Por favor, revisa tu ${loginType}`,
resendLink: 'Reenviar enlace',
validationCodeFailedMessage: 'Parece que hubo un error con el enlace de validación o ha caducado.',
},
unlinkLoginForm: {
toValidateLogin: ({primaryLogin, secondaryLogin}) => `Para validar ${secondaryLogin}, reenvía el código mágico desde la Configuración de la cuenta de ${primaryLogin}.`,
noLongerHaveAccess: ({primaryLogin}) => `Si ya no tienes acceso a ${primaryLogin} por favor, desvincula las cuentas.`,
unlink: 'Desvincular',
linkSent: '¡Enlace enviado!',
succesfullyUnlinkedLogin: '¡Nombre de usuario secundario desvinculado correctamente!',
},
detailsPage: {
localTime: 'Hora local',
Expand Down
6 changes: 6 additions & 0 deletions src/libs/Navigation/AppNavigator/PublicScreens.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import ValidateLoginPage from '../../../pages/ValidateLoginPage';
import LogInWithShortLivedAuthTokenPage from '../../../pages/LogInWithShortLivedAuthTokenPage';
import SCREENS from '../../../SCREENS';
import defaultScreenOptions from './defaultScreenOptions';
import UnlinkLoginPage from '../../../pages/UnlinkLoginPage';

const RootStack = createStackNavigator();

Expand All @@ -26,6 +27,11 @@ const PublicScreens = () => (
options={defaultScreenOptions}
component={ValidateLoginPage}
/>
<RootStack.Screen
name="UnlinkLogin"
options={defaultScreenOptions}
component={UnlinkLoginPage}
/>
<RootStack.Screen
name="SetPassword"
options={defaultScreenOptions}
Expand Down
1 change: 1 addition & 0 deletions src/libs/Navigation/linkingConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export default {
// Main Routes
SetPassword: ROUTES.SET_PASSWORD_WITH_VALIDATE_CODE,
ValidateLogin: ROUTES.VALIDATE_LOGIN,
UnlinkLogin: ROUTES.UNLINK_LOGIN,
[SCREENS.TRANSITION_FROM_OLD_DOT]: ROUTES.TRANSITION_FROM_OLD_DOT,
Concierge: ROUTES.CONCIERGE,

Expand Down
70 changes: 70 additions & 0 deletions src/libs/actions/Session/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ function beginSignIn(login) {
value: {
...CONST.DEFAULT_ACCOUNT_DATA,
isLoading: true,
message: null,
},
},
];
Expand Down Expand Up @@ -646,6 +647,73 @@ function authenticatePusher(socketID, channelName, callback) {
});
}

/**
* Request a new validation link / magic code to unlink an unvalidated secondary login from a primary login
*/
function requestUnlinkValidationLink() {
const optimisticData = [{
onyxMethod: CONST.ONYX.METHOD.MERGE,
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI this just got removed! And it was replaced with Onyx.METHOD.MERGE, where Onyx is imported from react-native-onyx.

I can spin up a fix for this real quick

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I guess this was found in this bug report: #18526

key: ONYXKEYS.ACCOUNT,
value: {
isLoading: true,
errors: null,
message: null,
},
}];
const successData = [{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.ACCOUNT,
value: {
isLoading: false,
message: Localize.translateLocal('unlinkLoginForm.linkSent'),
Copy link
Member

Choose a reason for hiding this comment

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

Because this will return a static value, it will not translate on runtime when the use changes the language after the message is shown to the user which caused #18827

},
}];
const failureData = [{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.ACCOUNT,
value: {
isLoading: false,
},
}];

API.write('RequestUnlinkValidationLink', {email: credentials.login}, {optimisticData, successData, failureData});
}

function unlinkLogin(accountID, validateCode) {
const optimisticData = [{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.ACCOUNT,
value: {
...CONST.DEFAULT_ACCOUNT_DATA,
isLoading: true,
},
}];
const successData = [{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.ACCOUNT,
value: {
isLoading: false,
message: Localize.translateLocal('unlinkLoginForm.succesfullyUnlinkedLogin'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi ✋ Coming from #22934

We should return the message as the phrase key instead to get the dynamic message based on the language.

Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for highlighting this.

},
},
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.CREDENTIALS,
value: {
login: '',
},
}];
const failureData = [{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.ACCOUNT,
value: {
isLoading: false,
},
}];

API.write('UnlinkLogin', {accountID, validateCode}, {optimisticData, successData, failureData});
}

export {
beginSignIn,
updatePasswordAndSignin,
Expand All @@ -662,6 +730,8 @@ export {
resendLinkWithValidateCode,
resetPassword,
resendResetPassword,
requestUnlinkValidationLink,
unlinkLogin,
clearSignInData,
clearAccountMessages,
authenticatePusher,
Expand Down
34 changes: 34 additions & 0 deletions src/pages/UnlinkLoginPage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import React from 'react';
import lodashGet from 'lodash/get';
import {
propTypes as validateLinkPropTypes,
defaultProps as validateLinkDefaultProps,
} from './ValidateLoginPage/validateLinkPropTypes';
import FullScreenLoadingIndicator from '../components/FullscreenLoadingIndicator';
import Navigation from '../libs/Navigation/Navigation';
import * as Session from '../libs/actions/Session';
import ROUTES from '../ROUTES';

const propTypes = {
/** The accountID and validateCode are passed via the URL */
route: validateLinkPropTypes,
};

const defaultProps = {
route: validateLinkDefaultProps,
};

const UnlinkLoginPage = (props) => {
const accountID = lodashGet(props.route.params, 'accountID', '');
const validateCode = lodashGet(props.route.params, 'validateCode', '');

Session.unlinkLogin(accountID, validateCode);
Navigation.navigate(ROUTES.HOME);
return <FullScreenLoadingIndicator />;
};

UnlinkLoginPage.propTypes = propTypes;
UnlinkLoginPage.defaultProps = defaultProps;
UnlinkLoginPage.displayName = 'UnlinkLoginPage';

export default UnlinkLoginPage;
4 changes: 2 additions & 2 deletions src/pages/signin/LoginForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,10 @@ class LoginForm extends React.Component {
{this.props.account.success}
</Text>
)}
{!_.isEmpty(this.props.closeAccount.success) && (
{!_.isEmpty(this.props.closeAccount.success || this.props.account.message) && (

// DotIndicatorMessage mostly expects onyxData errors, so we need to mock an object so that the messages looks similar to prop.account.errors
<DotIndicatorMessage style={[styles.mv2]} type="success" messages={{0: this.props.closeAccount.success}} />
<DotIndicatorMessage style={[styles.mv2]} type="success" messages={{0: this.props.closeAccount.success || this.props.account.message}} />
)}
{ // We need to unmount the submit button when the component is not visible so that the Enter button
// key handler gets unsubscribed and does not conflict with the Password Form
Expand Down
26 changes: 24 additions & 2 deletions src/pages/signin/SignInPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize
import Performance from '../../libs/Performance';
import * as App from '../../libs/actions/App';
import Permissions from '../../libs/Permissions';
import UnlinkLoginForm from './UnlinkLoginForm';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../components/withWindowDimensions';
import * as Localize from '../../libs/Localize';

Expand All @@ -29,6 +30,9 @@ const propTypes = {

/** Whether the account is validated */
validated: PropTypes.bool,

/** The primaryLogin associated with the account */
primaryLogin: PropTypes.string,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should add a default prop for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, why? The other values don't have a default prop associated with them.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we either make the prop required else we define the default prop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair, but neither errors nor validated have default props here (nor in most other cases from what I can see). I'm happy to add default props for all of them if you'd like, but also to content to leave the default props as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think there was a lint rule that required default props or the prop to be required. Looks like it is not working lately.

}),

/** List of betas available to current user */
Expand Down Expand Up @@ -65,31 +69,46 @@ class SignInPage extends Component {
// - AND a validateCode has not been cached with sign in link
const showLoginForm = !this.props.credentials.login && !this.props.credentials.validateCode;

// Show the unlink form if
// - A login has been entered
// - AND the login is not the primary login
// - AND the login is not validated
const showUnlinkLoginForm = this.props.credentials.login
&& this.props.account.primaryLogin
&& this.props.account.primaryLogin !== this.props.credentials.login
&& !this.props.account.validated;

// Show the old password form if
// - A login has been entered
// - AND an account exists and is validated for this login
// - AND a password hasn't been entered yet
// - AND haven't forgotten password
// - AND the login isn't an unvalidated secondary login
// - AND the user is NOT on the passwordless beta
const showPasswordForm = Boolean(this.props.credentials.login)
&& this.props.account.validated
&& !this.props.credentials.password
&& !this.props.account.forgotPassword
&& !showUnlinkLoginForm
&& !Permissions.canUsePasswordlessLogins(this.props.betas);

// Show the new magic code / validate code form if
// - A login has been entered or a validateCode has been cached from sign in link
// - AND the login isn't an unvalidated secondary login
// - AND the user is on the 'passwordless' beta
const showValidateCodeForm = (this.props.credentials.login
|| this.props.credentials.validateCode)
&& !showUnlinkLoginForm
&& Permissions.canUsePasswordlessLogins(this.props.betas);

// Show the resend validation link form if
// - A login has been entered
// - AND is not validated or password is forgotten
// - AND the login isn't an unvalidated secondary login
// - AND user is not on 'passwordless' beta
const showResendValidationForm = Boolean(this.props.credentials.login)
&& (!this.props.account.validated || this.props.account.forgotPassword)
&& !showUnlinkLoginForm
&& !Permissions.canUsePasswordlessLogins(this.props.betas);

let welcomeHeader = '';
Expand Down Expand Up @@ -121,6 +140,8 @@ class SignInPage extends Component {
welcomeText = this.props.isSmallScreenWidth
? `${this.props.translate('welcomeText.welcomeBack')} ${this.props.translate('welcomeText.enterPassword')}`
: this.props.translate('welcomeText.enterPassword');
} else if (showUnlinkLoginForm) {
welcomeHeader = this.props.isSmallScreenWidth ? this.props.translate('login.hero.header') : this.props.translate('welcomeText.welcomeBack');
} else if (!showResendValidationForm) {
welcomeHeader = this.props.isSmallScreenWidth ? this.props.translate('login.hero.header') : this.props.translate('welcomeText.getStarted');
welcomeText = this.props.isSmallScreenWidth ? this.props.translate('welcomeText.getStarted') : '';
Expand All @@ -131,8 +152,8 @@ class SignInPage extends Component {
<SignInPageLayout
welcomeHeader={welcomeHeader}
welcomeText={welcomeText}
shouldShowWelcomeHeader={(showLoginForm || showPasswordForm || showValidateCodeForm || !showResendValidationForm) || !this.props.isSmallScreenWidth}
shouldShowWelcomeText={showLoginForm || showPasswordForm || showValidateCodeForm || !showResendValidationForm}
shouldShowWelcomeHeader={(showLoginForm || showPasswordForm || showValidateCodeForm || showUnlinkLoginForm) || !this.props.isSmallScreenWidth}
shouldShowWelcomeText={showLoginForm || showPasswordForm || showValidateCodeForm}
>
{/* LoginForm and PasswordForm must use the isVisible prop. This keeps them mounted, but visually hidden
so that password managers can access the values. Conditionally rendering these components will break this feature. */}
Expand All @@ -143,6 +164,7 @@ class SignInPage extends Component {
<PasswordForm isVisible={showPasswordForm} />
)}
{showResendValidationForm && <ResendValidationForm />}
{showUnlinkLoginForm && <UnlinkLoginForm />}
Copy link
Contributor

Choose a reason for hiding this comment

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

This line caused console error as showUnlinkLoginForm can be string value. More details - #19202 (comment)

New checklist item added to prevent such bugs:

  • I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.

</SignInPageLayout>
</SafeAreaView>
);
Expand Down
Loading