Make the NewDot login flow work with domain-controlled accounts #5218

Gonals · 2021-09-13T12:53:40Z

@nickmurray47 🐻‍❄️

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/176617#

Tests

Create an email using EmailOnDeck.
Create and validate the account via ./script/clitools.sh generator:account, then run ./script/clitools.sh generator:domain to generate a domain with it as its admin.
Create another email using EmailOnDeck. If you create the account in the same day it should have the same domain.
Add this account to the DEV_EMAILS const in _.config.local.php.
Use this account to sign up for NewDot (don't use any clitools scripts to validate the account). Then, run vssh php ./Web-Expensify/script/notifyall.php.
You should get an email in EmailOnDeck. Use the link in the email to set a password.
- You might have to modify the link format, make sure it's in this format: <baseURL>/setpassword/<accountID>/<validateCode>.
Populate the password fields and complete the form. You should get logged in.

QA Steps

With a fresh email (with no account) in a controlled domain, try to create a new account in newdot.
You should get a magic link in your email. Make sure it points to newdot and not oldDot (this is a different bug currently being fixed in another PR). The format should be new.expensify.com/setpassword/<accountID>/<validateCode>. If it isn't, just change it to that format.
Populate the password fields and submit the form.
You should get logged in.

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Gonals · 2021-09-13T12:55:09Z

Setting @nickmurray47 as reviewer, since he has been assigned to the web PR counterpart

iwiznia

See my comment in web PR

jasperhuangg

Tested with https://github.com/Expensify/Web-Expensify/pull/31898, works well!

nickmurray47

also LGTM and tests well

Gonals · 2021-09-16T09:05:16Z

All yours, @iwiznia!

iwiznia · 2021-09-16T15:35:18Z

src/pages/SetPasswordPage.js

+            accountID,
+            validateCode,
+        }).then((responseValidate) => {
+            if (responseValidate.jsonCode === 200) {


No error message if this fails?

And if the ValidateEmail call fails.. what is the user supposed to do?

Another thought, if the validate code is bad why make them enter a password at all?

Hmmm. Ask for another code, I imagine?

Mainly because we don't know yet 😛. What are you thinking? Calling validate on page load, for example? That would probably work.

Ask for another code, I imagine?

Ahaha asking because when I ran through this flow I just get stuck looking at an error message with no buttons or anything.

Calling validate on page load, for example? That would probably work.

Sure, this is what we do on OldDot.

We can create another issue to nail these two down, if you want. I think validating on load makes sense and would make the flow a bit better, and if validation fails, we can display the error and a button/link to resend the magic link?

I don't think we need them to go live with N6, but they seem like good improvements

src/pages/SetPasswordPage.js

iwiznia · 2021-09-16T15:37:34Z

src/pages/SetPasswordPage.js

+                    password: this.state.password,
+                }).then((responsePassword) => {
+                    if (responsePassword.jsonCode === 200) {
+                        signIn(this.state.password);


Why do we need to call signIn? Is it because validate and changePassword do not set the authToken cookie or what?

Yep. SignIn sets the cookie and redirects us to where we want to go, I believe

I don't think there are any cookies in New Expensify. We mostly need to do this to create the "device login" to make the re-authentication logic work correctly.

Gonals · 2021-09-16T16:10:21Z

Added error messages!

nickmurray47

Re-tested and new error messages look solid!

Gonals · 2021-09-17T07:55:08Z

@iwiznia all yours once again!

OSBotify · 2021-09-17T10:10:04Z

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

flodnv · 2021-09-20T15:59:57Z

src/pages/SetPasswordPage.js

-            lodashGet(this.props.route, 'params.validateCode', ''),
-            lodashGet(this.props.route, 'params.accountID', ''),
-        );
+        API.ValidateEmail({


I am pretty sure this broke the page for when the account is already validated... The API returns 405 Account already validated.

It was already broken anyways, another issue will be created https://expensify.slack.com/archives/C020EPP9B9A/p1632155251298000?thread_ts=1631926252.186300&cid=C020EPP9B9A

OSBotify · 2021-09-20T21:51:59Z

🚀 Deployed to staging by @iwiznia in version: 1.0.99-5 🚀

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

mvtglobally · 2021-09-20T22:56:11Z

@Gonals @marcaaron @flodnv What environments should we verify this PR?
Can you also confirm we should be able to run this PR with Expensifail account?

Gonals · 2021-09-21T10:28:50Z

@Gonals @marcaaron @flodnv What environments should we verify this PR?
Can you also confirm we should be able to run this PR with Expensifail account?

Hey! This should be tested in staging.
I think Expensifail is domain-controlled, so it should work for this just fine :)

OSBotify · 2021-09-22T00:45:55Z

🚀 Deployed to production by @Jag96 in version: 1.1.0-2 🚀

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

Make the NewDot login flow work with domain-controlled accounts

f5d2c16

Gonals requested a review from a team as a code owner September 13, 2021 12:53

Gonals self-assigned this Sep 13, 2021

MelvinBot requested review from iwiznia and removed request for a team September 13, 2021 12:54

Gonals changed the title ~~Make the NewDot login flow work with domain-controlled accounts~~ [HOLD] Make the NewDot login flow work with domain-controlled accounts Sep 13, 2021

Gonals requested review from nickmurray47 and removed request for iwiznia September 13, 2021 12:54

iwiznia requested changes Sep 13, 2021

View reviewed changes

jasperhuangg self-requested a review September 15, 2021 00:30

remove new command and use existing ones

5b00fc0

jasperhuangg changed the title ~~[HOLD] Make the NewDot login flow work with domain-controlled accounts~~ Make the NewDot login flow work with domain-controlled accounts Sep 15, 2021

jasperhuangg previously approved these changes Sep 15, 2021

View reviewed changes

nickmurray47 previously approved these changes Sep 16, 2021

View reviewed changes

iwiznia requested changes Sep 16, 2021

View reviewed changes

add errors

a9bdce2

Gonals dismissed stale reviews from nickmurray47 and jasperhuangg via a9bdce2 September 16, 2021 16:09

lint

fc85568

nickmurray47 approved these changes Sep 16, 2021

View reviewed changes

jasperhuangg approved these changes Sep 17, 2021

View reviewed changes

iwiznia approved these changes Sep 17, 2021

View reviewed changes

iwiznia merged commit a995d4d into main Sep 17, 2021

iwiznia deleted the alberto-magicNew branch September 17, 2021 10:09

flodnv reviewed Sep 20, 2021

View reviewed changes

OSBotify mentioned this pull request Sep 20, 2021

Deploy Checklist: New Expensify 2021-09-20 #5368

Closed

31 tasks

Jag96 mentioned this pull request Sep 21, 2021

Fix forgot password functionality for validated logins #5398

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the NewDot login flow work with domain-controlled accounts #5218

Make the NewDot login flow work with domain-controlled accounts #5218

Gonals commented Sep 13, 2021 •

edited by jasperhuangg

Loading

Gonals commented Sep 13, 2021

iwiznia left a comment

jasperhuangg left a comment

nickmurray47 left a comment

Gonals commented Sep 16, 2021

iwiznia Sep 16, 2021

marcaaron Sep 17, 2021

Gonals Sep 21, 2021 •

edited

Loading

marcaaron Sep 21, 2021

Gonals Sep 21, 2021

iwiznia Sep 16, 2021

Gonals Sep 16, 2021

marcaaron Sep 17, 2021

Gonals commented Sep 16, 2021

nickmurray47 left a comment

Gonals commented Sep 17, 2021

OSBotify commented Sep 17, 2021

flodnv Sep 20, 2021

flodnv Sep 20, 2021

OSBotify commented Sep 20, 2021

mvtglobally commented Sep 20, 2021

Gonals commented Sep 21, 2021

OSBotify commented Sep 22, 2021

Make the NewDot login flow work with domain-controlled accounts #5218

Make the NewDot login flow work with domain-controlled accounts #5218

Conversation

Gonals commented Sep 13, 2021 • edited by jasperhuangg Loading

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Gonals commented Sep 13, 2021

iwiznia left a comment

Choose a reason for hiding this comment

jasperhuangg left a comment

Choose a reason for hiding this comment

nickmurray47 left a comment

Choose a reason for hiding this comment

Gonals commented Sep 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gonals Sep 21, 2021 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gonals commented Sep 16, 2021

nickmurray47 left a comment

Choose a reason for hiding this comment

Gonals commented Sep 17, 2021

OSBotify commented Sep 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OSBotify commented Sep 20, 2021

mvtglobally commented Sep 20, 2021

Gonals commented Sep 21, 2021

OSBotify commented Sep 22, 2021

Gonals commented Sep 13, 2021 •

edited by jasperhuangg

Loading

Gonals Sep 21, 2021 •

edited

Loading