Passwordless - Web automatic login #14443

cristipaval · 2023-01-20T21:33:55Z

Details

Adds automatic login on web when clicking on the link with the magic code.

Fixed Issues

https://github.com/Expensify/Expensify/issues/249362

Tests

Passwordless beta flag enabled (default in dev environment)

Run the App and sign out if already signed in
Add an email for an existing account with 2fa disabled
Get the link with the magic code and click on it or copy paste it in the same browser, in another tab
Verify that you are successfully logged in

Now that you are signed in, enter that link again in a new tab
Verify that you see a full page with the magic code

In App, go to src/libs/Permissions.js and make the function canUsePasswordlessLogins have return true;
Sign out and make sure you have no email pre-filled in the SignIn page
Enter the email in another device (or other browser, or the same browser but in an incognito window)
Open the link in your original browser used at the step 8
Verify that you see the full page with the magic code and with the option to get straight in with that tab

Tap on just sign in here link and verify that you are signed in in the same tab
Repeat the steps above with an account which has 2fa enabled

Passwordless beta flag disabled

In App, go to src/libs/Permissions.js and make the function canUsePasswordlessLogins have return false;
In Web-E, go to lib/BetaManager.php and make sure $allBetasEnabled is set to false
Run the App and sign out if already signed in
Enter an email which does not belong to an existing account
Verify that you get the email with the magic link
Click the link and verify that you can set the password for the newly created account

Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

Run the App and sign out if already signed in
Add an email for an existing account with 2fa disabled
Get the link with the magic code and click on it or copy paste it in the same browser, in another tab
Verify that you are successfully logged in

Now that you are signed in, enter that link again in a new tab
Verify that you see a full page with the magic code

Repeat the steps above with an account which has 2fa enabled

NOTE: (Steps 8-12 from Tests section can't be QAed before rolling Passwordless to production, because we can't get betas for an accountID from server).

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

Passwordless enabled:

Screen.Recording.2023-02-09.at.16.13.22.mov

Passwordless disabled:

web.np.mov

Mobile Web - Chrome

Passwordless enabled:

Screen.Recording.2023-02-09.at.16.28.26.mov

Passwordless disabled:

android.chrome.np.mov

Mobile Web - Safari

Passwordless enabled:

Screen.Recording.2023-02-09.at.16.19.04.mov

Passwordless disabled:

ios.safari.np.mov

Desktop

N/A

iOS

N/A

Android

N/A

…uto-login

…sFlow' into cristi_passwordless-auto-login

…uto-login

cristipaval · 2023-02-13T20:36:32Z

Please check if all scenarios (without 2fa) are correct.

@aimane-chnaif Yes, they are correct.
I just addressed the other feedback. Please have another look.

aimane-chnaif · 2023-02-13T20:40:28Z

I also think all scenarios are correct except last step of Scenario 2.
After refreshing tab T2, why should it show "code" page back? Isn't it expected to show "you are signed in" page?

cristipaval · 2023-02-13T20:43:57Z

I also think all scenarios are correct except last step of Scenario 2. After refreshing tab T2, why should it show "code" page back? Isn't it expected to show "you are signed in" page?

When you refresh the page, you are in the Scenario 1, with chrome signed in already. So it is correct to see the page as in scenario 1.

NikkiWines

👍

aimane-chnaif · 2023-02-14T14:00:38Z

src/components/DeeplinkWrapper/deeplinkRoutes.js

+
+        // Disable deep linking in desktop App when passwordless is enabled because
+        // we want to open the magic link in its own tab
+        isDisabled: betas => Permissions.canUsePasswordlessLogins(betas),


If passwordless enabled, desktop deep link is disabled completely throughout the app. I think we should disable this only when deep link is magic code link

The magic link looks the same as the link used to validate login , which is going to be removed when Passwordless is fully rolled out in production. For now, we still want validate login to work in production for the users who don't have Passwordless beta enabled.

Or according to new logic, isn't it fine to open desktop app with magic link?

It's fine to open the desktop App when Passwordless beta is not enabled, and a new account is created. The user clicks the link to validate the newly created account and this link looks exactly the same as the magic link. This is a temporary thing, because validate login will be removed when Passwordless is fully rolled out in ptoduction.

The magic link looks the same as the link used to validate login , which is going to be removed when Passwordless is fully rolled out in production. For now, we still want validate login to work in production for the users who don't have Passwordless beta enabled.

i.e. when open user profile link, it should still show open desktop popup. this is current behavior in production

Or according to new logic, isn't it fine to open desktop app with magic link?

Oh, I think I missed your question. Yes, it is desired to NOT have deep linking enabled when there's is the magic code flow, but we want the old flow (validate login) to stay unchanged, with deep linking enabled.

aimane-chnaif · 2023-02-14T14:20:41Z

@cristipaval please check this video. I believe this PR should not disable any other deep link for desktop app.

Or you meant to revert disabling deeplink once passwordless is fully rolled out on production?

deep.link.mov

cristipaval · 2023-02-14T14:34:29Z

@cristipaval please check this video. I believe this PR should not disable any other deep link for desktop app.

Or you meant to revert disabling deeplink once passwordless is fully rolled out on production?

As we discussed 1:1

/v/* path is applicable for magic link and login validation only
we want deep linking disabled for magic link, but we want it enabled for login validation
when passwordless is fully rolled out into production, login validation won't be a thing anymore in the App

aimane-chnaif · 2023-02-14T14:36:19Z

ok, resolved above concern in private discussion with @cristipaval

cristipaval · 2023-02-14T14:39:10Z

ok, resolved above concern in private discussion with @cristipaval

posted here our conclusion for future reference

aimane-chnaif · 2023-02-14T14:45:51Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Passwordless beta disabled:

web-password1.mov

web-password2.mov

Passwordless beta enabled:

web-passwordless1.mov

web-passwordless2.mov

Mobile Web - Chrome

Mobile Web - Safari

msafari.mp4

Desktop

iOS

Android

OSBotify · 2023-02-14T16:39:42Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2023-02-14T17:09:47Z

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	767.680 ms → 817.478 ms (+49.797 ms, +6.5%)
App start runJsBundle	208.323 ms → 229.438 ms (+21.115 ms, +10.1%)
App start regularAppStart	0.017 ms → 0.018 ms (+0.000 ms, +2.5%)
App start nativeLaunch	20.469 ms → 20.469 ms
Open Search Page TTI	598.143 ms → 586.534 ms (-11.609 ms, -1.9%)

Show details

Name	Duration
App start TTI	Baseline Mean: 767.680 ms Stdev: 38.321 ms (5.0%) Runs: 677.2056469999952 685.2870289999992 716.826705000014 734.3776999999536 734.6648379999679 743.4575459999032 744.8783070000354 747.2439500000328 747.4141150000505 750.0931570000248 751.2536310000578 753.4486779998988 756.3340409999946 761.3617340000346 763.6607600001153 765.1504410000052 767.1864960000385 768.3517310000025 768.6686239999253 769.0535899999086 770.9303460000083 779.4999160000589 794.282465999946 805.254212000058 810.1927330000326 810.3968970000278 811.2978300000541 814.4710770000238 815.7051490000449 834.0603459998965 846.073885000078 Current Mean: 817.478 ms Stdev: 41.546 ms (5.1%) Runs: 754.6804480000865 755.5928720000666 757.6606989998836 760.3454430000857 764.099869000027 776.3111300000455 777.5810809999239 779.2087930000853 782.1385210000444 783.819397999905 791.2749199999962 795.3887189999223 796.0118440000806 805.0445709999185 813.1349529998843 815.4920069999062 823.1377620000858 823.3560979999602 826.891753999982 830.2080900000874 831.475834999932 840.535489999922 849.3940119999461 850.7808310000692 852.1124899999704 853.5086050000973 859.9901870000176 865.4214890000876 878.7086910000071 883.2840120000765 886.0888249999844 896.6009490001015
App start runJsBundle	Baseline Mean: 208.323 ms Stdev: 18.843 ms (9.0%) Runs: 161 183 183 184 187 188 193 194 198 198 199 200 202 206 207 207 209 214 215 216 216 216 224 227 230 231 231 232 234 234 239 Current Mean: 229.438 ms Stdev: 21.549 ms (9.4%) Runs: 195 197 199 204 204 206 208 209 210 211 217 217 218 219 220 230 232 234 237 241 241 247 247 248 249 249 251 253 254 256 269 270
App start regularAppStart	Baseline Mean: 0.017 ms Stdev: 0.001 ms (6.2%) Runs: 0.014770999900065362 0.015746999997645617 0.015746999997645617 0.016032000072300434 0.016235999995842576 0.016399000072851777 0.016561000025831163 0.016642000060528517 0.01672299997881055 0.016723000211641192 0.016723999986425042 0.016805000137537718 0.016805000137537718 0.017008000053465366 0.01700900006107986 0.0170499999076128 0.01721199997700751 0.017497000051662326 0.017577999969944358 0.017619000049307942 0.017619000049307942 0.017822999972850084 0.017903999891132116 0.018146999995224178 0.018309999955818057 0.01900200010277331 0.019286999944597483 0.01953100017271936 Current Mean: 0.018 ms Stdev: 0.001 ms (5.2%) Runs: 0.015828999923542142 0.016276000067591667 0.01639799983240664 0.016438999911770225 0.01684599998407066 0.016885999822989106 0.01688600005581975 0.016927000135183334 0.01700900006107986 0.01704900013282895 0.01721199997700751 0.01737500005401671 0.017414999892935157 0.01741599990054965 0.017496000044047832 0.017537000123411417 0.017577999969944358 0.017660000128671527 0.01774100004695356 0.01774100004695356 0.01802499988116324 0.018065999960526824 0.018391000106930733 0.018391000106930733 0.018391999881714582 0.018757999874651432 0.018962000031024218 0.01920600002631545 0.020018999930471182
App start nativeLaunch	Baseline Mean: 20.469 ms Stdev: 2.076 ms (10.1%) Runs: 18 18 18 18 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 20 21 21 21 21 22 22 22 23 23 25 25 26 Current Mean: 20.469 ms Stdev: 2.179 ms (10.6%) Runs: 18 18 18 18 19 19 19 19 19 19 19 19 19 19 19 20 20 20 20 21 21 21 21 21 22 22 22 23 23 25 26 26
Open Search Page TTI	Baseline Mean: 598.143 ms Stdev: 35.047 ms (5.9%) Runs: 557.1005869999062 562.2990729999729 569.8751639998518 571.6988929999061 571.8014330000151 572.0194500000216 573.9050700000953 574.5584720000625 576.0498450000305 576.2487790000159 576.8467200000305 577.0222170001362 577.4628089999314 579.6151129999198 580.1106370000634 582.4245200001169 583.2222089997958 583.3592530000024 586.4379889999982 592.4502769999672 592.5861009999644 593.5468760000076 599.2247719999868 603.3056239997968 604.7459720000625 616.4537359999958 634.7391770000104 635.2511400000658 639.3741459997837 642.7565920001362 667.0253910000902 680.9957690001465 704.2150880000554 Current Mean: 586.534 ms Stdev: 16.666 ms (2.8%) Runs: 552.9497479998972 561.9845779999159 564.4523930000141 566.1731360000558 567.8385830000043 572.2541100000963 573.0568040001672 573.1176760001108 575.0235189998057 575.3395190001465 576.12447099993 576.5612389999442 578.499186999863 580.3756519998424 583.7659909999929 586.2096349999774 587.5041499999352 588.4761560000479 589.7621260001324 590.2895510001108 592.223347999854 593.2574879999738 598.1352950001601 598.171875 599.2054860000499 601.7126060000155 601.7574459998868 605.1125089998823 606.6199139999226 610.7148439998273 617.5524089999963 624.8636479999404

OSBotify · 2023-02-15T18:29:37Z

🚀 Deployed to staging by https://github.com/marcochavezf in version: 1.2.72-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-02-16T21:37:40Z

🚀 Deployed to production by https://github.com/francoisl in version: 1.2.72-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

marcaaron · 2023-03-06T23:54:27Z

src/pages/ValidateLoginPage/index.js

+    /**
+     * @returns {String}
+     */
+    accountID = () => lodashGet(this.props.route.params, 'accountID', '');


Typically, it is recommended to prefix functions that retrieve information with "get", so perhaps we could consider renaming accountID() and validateCode() to getAccountID() and getValidateCode() respectively?

Additionally, I could be out of the loop on our style guide, but I think we have not yet adopted instance class properties.

I understand it is less verbose than having to declare things in the constructor and people are often confused about this in React class components - but can we update to be more consistent with the current styles?

Hey @marcaaron ! Thanks for guiding me on following the style! I am working on a follow up PR which touches this class. It should be ready for review. I am adding your suggestions in the new PR.

rushatgabhane · 2023-06-02T00:25:18Z

src/libs/actions/Session/index.js

+    // This is temporary for now. Server should login with the accountID and validateCode
+    API.write('SigninUser', {
+        validateCode,
+        accountID,


beep boop 🤖 🐧

dropping a friendly note that we missed an edge case - Device language preference does not persist when authenticating with magic link.

Because we didn't write the preferred local to the server.

cristipaval added 10 commits January 19, 2023 22:31

Check if passwordless when validating login.

072c947

Merge remote-tracking branch 'origin/main' into cristi_passwordless-a…

844e210

…uto-login

Create magic code modal component.

7e22cdc

Separate website layout for Magic Code modal.

b7202dc

Add Expensify wordmark in magic code modal.

27880b1

Add illustration in magic code modal

9ab1755

Add title in magic code modal

5de8a45

Add description in magic code modal.

37371af

Add magic code digits in magic code modal.

44bbdd8

Pass magic code to the modal via props.

26330e2

cristipaval changed the title ~~Cristi passwordless auto login~~ Passwordless auto login Jan 20, 2023

cristipaval added 12 commits January 24, 2023 12:19

Work in progress.

ede6534

Merge remote-tracking branch 'origin/main' into cristi_passwordless-a…

546164c

…uto-login

Merge remote-tracking branch 'origin/beaman-addConditionalPasswordles…

622a858

…sFlow' into cristi_passwordless-auto-login

Sign in with account id and magic code when click on link.

a46fb7c

Go passwordless when 'all' betas

912ee45

Temporary solution to auto login.

9a092d6

Merge remote-tracking branch 'origin/beaman-addConditionalPasswordles…

9a1baef

…sFlow' into cristi_passwordless-auto-login

Check if passwordless beta exists in ValidateLogin page

8b46180

Merge remote-tracking branch 'origin/main' into cristi_passwordless-a…

311adbc

…uto-login

Store validateCode in Onyx after authenticating with the link

5ee7105

Move ValidateLoginPage in a folder

a177545

Auto login for web only

963aabe

cristipaval self-assigned this Feb 1, 2023

cristipaval changed the title ~~Passwordless auto login~~ Passwordless - Web automatic login Feb 1, 2023

cristipaval added 5 commits February 1, 2023 23:51

Fix lint errors

e71aa8e

Fix lint errors in MagicCodeModal.js

01089bf

Fix more lint errors.

1c0b7e9

I'm dying, Linter

a7347f2

Please Linter, have mercy on me

5980874

cristipaval dismissed stale reviews from NikkiWines and johnmlee101 via da56773 February 13, 2023 20:35

NikkiWines approved these changes Feb 13, 2023

View reviewed changes

aimane-chnaif reviewed Feb 14, 2023

View reviewed changes

aimane-chnaif approved these changes Feb 14, 2023

View reviewed changes

johnmlee101 approved these changes Feb 14, 2023

View reviewed changes

marcochavezf approved these changes Feb 14, 2023

View reviewed changes

marcochavezf merged commit c61c36f into main Feb 14, 2023

marcochavezf deleted the cristi_passwordless-auto-login branch February 14, 2023 16:35

OSBotify mentioned this pull request Feb 15, 2023

Deploy Checklist: New Expensify 2023-02-15 #15175

Closed

22 tasks

kbecciv mentioned this pull request Feb 15, 2023

[HOLD for payment 2023-03-13] Login - The magic code page is displayed first, followed by the successful login page #15188

Closed

6 tasks

NikkiWines mentioned this pull request Feb 16, 2023

Update secondary login validation flow for passwordless #15157

Closed

54 tasks

mountiny mentioned this pull request Feb 28, 2023

Payment issue for E/App PR#14443 and PR#14532 #15558

Closed

marcaaron reviewed Mar 6, 2023

View reviewed changes

marcaaron mentioned this pull request Mar 6, 2023

Login - Secondary Login validation link redirects to never-ending loading circle #15652

Closed

6 tasks

NikkiWines mentioned this pull request Apr 7, 2023

Clicking on deep links on passwordless crashes the app #17091

Closed

6 tasks

rushatgabhane reviewed Jun 2, 2023

View reviewed changes

rushatgabhane mentioned this pull request Jun 2, 2023

[HOLD for payment 2023-06-02] [$1000] Device language preference does not persist when authenticating with magic link but preference persists when authenticating with magic code #18248

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Passwordless - Web automatic login #14443

Passwordless - Web automatic login #14443

cristipaval commented Jan 20, 2023 •

edited

Loading

cristipaval commented Feb 13, 2023

aimane-chnaif commented Feb 13, 2023

cristipaval commented Feb 13, 2023 •

edited

Loading

NikkiWines left a comment

aimane-chnaif Feb 14, 2023

cristipaval Feb 14, 2023

aimane-chnaif Feb 14, 2023

cristipaval Feb 14, 2023 •

edited

Loading

aimane-chnaif Feb 14, 2023

cristipaval Feb 14, 2023

aimane-chnaif commented Feb 14, 2023 •

edited

Loading

cristipaval commented Feb 14, 2023

aimane-chnaif commented Feb 14, 2023

cristipaval commented Feb 14, 2023

aimane-chnaif commented Feb 14, 2023

OSBotify commented Feb 14, 2023

github-actions bot commented Feb 14, 2023

OSBotify commented Feb 15, 2023

OSBotify commented Feb 16, 2023

marcaaron Mar 6, 2023

cristipaval Mar 7, 2023

rushatgabhane Jun 2, 2023

Passwordless - Web automatic login #14443

Passwordless - Web automatic login #14443

Conversation

cristipaval commented Jan 20, 2023 • edited Loading

Details

Fixed Issues

Tests

Passwordless beta flag enabled (default in dev environment)

Passwordless beta flag disabled

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

cristipaval commented Feb 13, 2023

aimane-chnaif commented Feb 13, 2023

cristipaval commented Feb 13, 2023 • edited Loading

NikkiWines left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cristipaval Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aimane-chnaif commented Feb 14, 2023 • edited Loading

cristipaval commented Feb 14, 2023

aimane-chnaif commented Feb 14, 2023

cristipaval commented Feb 14, 2023

aimane-chnaif commented Feb 14, 2023

Reviewer Checklist

Screenshots/Videos

OSBotify commented Feb 14, 2023

github-actions bot commented Feb 14, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

OSBotify commented Feb 15, 2023

OSBotify commented Feb 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cristipaval commented Jan 20, 2023 •

edited

Loading

cristipaval commented Feb 13, 2023 •

edited

Loading

cristipaval Feb 14, 2023 •

edited

Loading

aimane-chnaif commented Feb 14, 2023 •

edited

Loading