Add accountID to the set password link

[jasperhuangg]

Details

The SetPassword API command requires an email to be provided, along with a new password and a validateCode. For the SetPasswordPage, we were previously assuming that the email would be read from Onyx storage.

However, that may not be the case if we send the user a set password link via a notification, as the user may not have completed the first part of the sign-in flow (which saves their email into Onyx storage). If this is the case, the API call to SetPassword will fail since the email sent over will be undefined.

In lieu of the changes in https://github.com/Expensify/Web-Expensify/pull/30466/ and https://github.com/Expensify/Auth/pull/5402, I'm adding an email parameter to the set password route to ensure we're always sending over and reading an email when the user navigates from this page.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/156786
$ https://github.com/Expensify/Expensify/issues/157873

Tests (local dev)

1. On Web-Expensify, check out jasper-ecashValidatePasswordEmail; on Auth, check out and make jasper-resendValidateCodeSetCashPassword.
2. Make sure you're following the steps with an unvalidated Expensify email. Run ./script/sql.sh then:

• Get your accountID:

SELECT validated from accounts WHERE email = "[your email]@expensify.com";

• Make sure your account is invalidated with this command:

UPDATE logins SET validatedDate = "" WHERE accountID = [your accountID];

• Make sure you've got the most up to date partnerName for Expensify.cash:

INSERT OR IGNORE INTO partners ( created, partnerID, partnerName, partnerPassword, partnerKey ) VALUES ( '2021-03-18 00:00:00', 84, 'chat-expensify-com', 'DD1431EB9E11F0071EC05C24E9CB2FD595B1ED75', 'asdf,fdsa');

3. Hit expensify.com.dev/api?command=ResendValidateCode&referer=https://expensify.cash/&accountID=[your accountID] to queue up a job that will send yourself a set password email.

• Use SELECT * from notifications; and check to make sure there's a SetCashPassword notification with your accountID queued up in the last row.

4. Then run vssh and php /vagrant/Web-Expensify/script/notifyall.php to send it out.

• to troubleshoot bedrock jobs, use the dev Bedrock Job Manager

5. Check your email for an email with the subject "Your magic sign in link for Expensify.cash". Click on the link.
6. Modify the first part of the URL with whatever localhost:[port] your local version of cash is running on (e.g. localhost:8081)

• After this, the URL should look something like this: localhost:8081/setpassword/[accountID]/[validateCode]

7. Navigate to that new URL and set a new password (it's better to do this in an incognito window).
8. Sign out of e.cash and try to sign in to that email with the new password. Verify that you can sign in.

QA Test Steps

1. Navigate to the sign in page.
2. Create a new account with an email address you have access to (I recommend using EmailOnDeck)
3. Verify that an email (like the screenshot) was sent to that email address.
4. Click "Resend Link". Verify that another email was sent to that email address.
5. In the newer email, click here. Verify that it takes you to https://expensify.cash/setpassword/:accountID/:validateCode
6. Set a password, it should log you in after you click "Set Password"
7. Sign out. Verify that you can sign back in with the same email address and password you just set.

• NOTE: this can also be tested on iOS/Android

Tested On

• Web
• Mobile Web
• Desktop
• iOS
• Android

Screenshots

Web

Mobile Web

[shawnborton]

For your mobile web screenshot, is there a way we can give the whole page a background color so that we don't have the strange background color block below the form content?

[jasperhuangg]

@shawnborton For sure! I made a new issue since it's not entirely related to what I'm fixing here. I've included more details in the issue. Look out for a PR soon!

[marcaaron]

QUESTION FOR REVIEWER: Is this testable on Desktop/iOS/Android?

Short answer: No.
Longer answer: Once we have "deep links" set up for iOS and Android we can theoretically drop you straight into the app on your device and everything should work the same (see Deep Linking in react-nav docs). There is an issue here to address this. As for Desktop, I think it just wouldn't be very valuable to implement right now, but maybe in the future.

[marcaaron]

Just have a couple questions so far.

[AndrewGable]

I'm curious if this issue is related: https://github.com/Expensify/Expensify/issues/157873 to this PR?

[marcaaron]

Hmm not sure if related.. the error you are seeing there seems to suggest that we have not set the login in credentials.

https://github.com/Expensify/Expensify.cash/blob/ec2ffc28f0d988cb73daad44c037534ba676c287/src/libs/actions/Session.js#L216

[jasperhuangg]

I left a comment on your review that I thought would be better for you to respond to first before reviewing

Just so I'm on the same page are you talking about a comment on my review in this PR or elsewhere that you are waiting for a response? Either way, if i haven't responded to it can you link me which comment you're waiting for a response from 🙂

Also yeah no worries about the back and forth. As long as there is daily movement its all cool.

Looks like you were able to get to my comment anyways! Thanks :) @chiragsalian

[chiragsalian]

LGTM, all yours @marcaaron

[marcaaron]

Is this still on HOLD ?

[chiragsalian]

@marcaaron, Yes because https://github.com/Expensify/Web-Expensify/pull/30466 isn't live yet.

[marcaaron]

Changes LGTM. Leaving this unmerged as I'm not sure when it would be deployed.

[tgolen]

I'm a little confused about the title of this PR. Isn't this adding an accountID to the set password link? The title says it's adding email to the link.

[marcaaron]

@jasperhuangg let's add some QA test steps to the description in addition to the local dev steps?

[marcaaron]

This LGTM. I tested on iOS and also Android since I noticed we did not test on those platforms. We should let QA know that they can test iOS and Android by clicking on the links with the app installed (at least this should work in theory).

[jasperhuangg]

This LGTM. I tested on iOS and also Android since I noticed we did not test on those platforms. We should let QA know that they can test iOS and Android by clicking on the links with the app installed (at least this should work in theory).

Thanks a lot! Will add a note in the testing instructions 👍

Seems like we've resolved comments and need to merge this to fix the broken set password flow.