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 2FA to NewDot Settings #15215

Closed
3 of 5 tasks
JmillsExpensify opened this issue Feb 16, 2023 · 72 comments
Closed
3 of 5 tasks

Add 2FA to NewDot Settings #15215

JmillsExpensify opened this issue Feb 16, 2023 · 72 comments
Assignees
Labels
NewFeature Something to build that is a new item. Weekly KSv2

Comments

@JmillsExpensify
Copy link

JmillsExpensify commented Feb 16, 2023

Design Doc to review: https://docs.google.com/document/d/11C65_BCS6D_V08JKX_mftI5ocWTYcazfblunrh9QGek/edit#heading=h.olra4emh77tp

Problem

For Newsletter 7, we're adding new financial flows for the very first time. This includes the Expensify Card and related virtual card transactions. It also includes bank account and related reimbursement flows. Aside from #passwordless, 2FA provides strong protections for /App users that aren't currently available.

Solution

Given that we're close to closing out the refactor of personal settings on /App, now is a good time to circle back and add 2FA to NewDot. In fact, let's launch this in conjunction with newsletter 7.

Design doc https://docs.google.com/document/d/11C65_BCS6D_V08JKX_mftI5ocWTYcazfblunrh9QGek/edit#heading=h.ns12dr1f35rx


Implementation issues:

cc @Beamanator @twisterdotcom @zanyrenney

@JmillsExpensify JmillsExpensify added Weekly KSv2 NewFeature Something to build that is a new item. labels Feb 16, 2023
@JmillsExpensify JmillsExpensify self-assigned this Feb 16, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Feb 16, 2023
@Beamanator
Copy link
Contributor

Makes sense to me! I believe @MonilBhavsar has been taking over the detailed part of the 2FA doc 👍 Anything you need from me here, @MonilBhavsar ?

@twisterdotcom
Copy link
Contributor

@zanyrenney let's see if we can update the Figma files this week in here for Monil: https://docs.google.com/document/d/11C65_BCS6D_V08JKX_mftI5ocWTYcazfblunrh9QGek/edit#heading=h.ns12dr1f35rx

@JmillsExpensify
Copy link
Author

Oh sweet! Do we have a p/s in #whatsnext for this one? If not, we should start there for sure. This issue was created to make sure we don't forget about this for N7.

@twisterdotcom
Copy link
Contributor

Do we need one? We have a reviewed doc for including this as part of N7 already here: https://docs.google.com/document/d/11C65_BCS6D_V08JKX_mftI5ocWTYcazfblunrh9QGek/edit#. All we need to do is update the doc to include new screenshots because the whole flow is online only.

@JmillsExpensify
Copy link
Author

Oh wow! Then yeah, I would update the screenshots in the HL, complete the detailed, and then resend the updated doc in the original email chain where it was first shared. I didn't realize this existed, must have missed it.

@twisterdotcom
Copy link
Contributor

Yeah! Will do.

@zanyrenney
Copy link
Contributor

Wow flashback! This doc slipped my mind entirely! I agree with not running this through WN again given we just took this out of the other original Account Settings doc.

@zanyrenney
Copy link
Contributor

I'll work on the figma frames for this ASAP @twisterdotco (I'm going OOO midway through the week!)

@MonilBhavsar
Copy link
Contributor

Makes sense to me! I believe @MonilBhavsar has been taking over the detailed part of the 2FA doc 👍 Anything you need from me here, @MonilBhavsar ?

No, all good here. Thanks!

@zanyrenney let's see if we can update the Figma files this week in here for Monil: https://docs.google.com/document/d/11C65_BCS6D_V08JKX_mftI5ocWTYcazfblunrh9QGek/edit#heading=h.ns12dr1f35rx

Thanks! I'll start working in parallel to update API's, get rid of one growl and account for offline mode.

Regarding offline mode, I think setting up 2FA should be pattern C. i.e. full blocking view. Surprisingly google authenticator app works offline. But to enable 2FA, user needs to enter 2FA code and as per our offline design, if we queue the API call, then I think that temporary code will be expired and the queued request is going to fail. So we can't really be optimistic in this case.

And, disabling 2FA should be pattern A, i.e. optimistic without feedback. We'll be optimistic that 2FA is disabled and redirect user to main security page.

UX Pattern doc for reference

Should we bring this to slack to get more eyes?

@twisterdotcom
Copy link
Contributor

disabling 2FA should be pattern A, i.e. optimistic without feedback. We'll be optimistic that 2FA is disabled and redirect user to main security page.

I think we should then, as I always also thought this would be C. If you disable it, then remove it from your device, you have basically lost access to Expensify, so I think you need confirmation that it's removed.

@MonilBhavsar
Copy link
Contributor

Great point! Yes, if they also remove it from authenticator app, they lose access.
Pretty edge case, that the deactivation request fails or meanwhile they tried authenticating on other device and it asks for 2FA code. But we should consider it and go with pattern C 👍

@JmillsExpensify
Copy link
Author

JmillsExpensify commented Feb 20, 2023

I think I agree that to ensure no one gets into a weird state, we should use Pattern D for enable/disable. That said, I would also make sure we all agree by posting something in #product as well.

@zanyrenney
Copy link
Contributor

I've made the "product side" changes to the flows in figma (e.g. removing passwords, pattern C for offline failure) but I have whipped up a design issue to get the branding updated as I am shortly going OOO and don't want this to be blocked on me. Issue is here - https://github.com/Expensify/Expensify/issues/263669

@twisterdotcom
Copy link
Contributor

@MonilBhavsar
Copy link
Contributor

A quick question - While disabling 2FA do we need to ask for magic code? Asking as right now it is not required in old dot.

https://docs.google.com/document/d/11C65_BCS6D_V08JKX_mftI5ocWTYcazfblunrh9QGek/edit#bookmark=id.f83o0e9dqioo

@mountiny
Copy link
Contributor

mountiny commented Mar 1, 2023

I would say yes, you should be asked for a magic code which is basically your password at that point.

@MonilBhavsar
Copy link
Contributor

MonilBhavsar commented Mar 1, 2023

I agree it is a good practice, but during #passwordless, it was decided to remove the prompt for these actions and only notify the user when such action is performed.

I think we should take this to slack and discuss if we want to prompt for a magic code in this flow.

@twisterdotcom
Copy link
Contributor

Ahhh hang on, no we updated the mockups for this flow already: https://github.com/Expensify/Expensify/issues/263669#issuecomment-1440083391

image

I saw the issue closed, but didn't think to put them in the doc, my bad.

@mountiny
Copy link
Contributor

mountiny commented Mar 1, 2023

Cool cool, I feel like we should ask for the magic code as in the screenshots, but the Slack thread will be the best

@twisterdotcom
Copy link
Contributor

Asked here: https://expensify.slack.com/archives/C02HWMSMZEC/p1677767921475339.

Will update the doc with these mockups when we get a 👍.

@zanyrenney
Copy link
Contributor

zanyrenney commented Mar 8, 2023

Hey both @twisterdotcom @MonilBhavsar , what's the situation with the doc? I can't see the new frames from Shawn have been added in and looks like we have an agreement from that thread 6 days ago now so we should be able to progress, right? Is there something I can help with to move this along?

@twisterdotcom
Copy link
Contributor

This is with contributors now: #22335

@melvin-bot melvin-bot bot removed the Overdue label Jul 14, 2023
@melvin-bot melvin-bot bot added the Overdue label Jul 24, 2023
@zanyrenney
Copy link
Contributor

PR is ready: #22335 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Jul 24, 2023
@melvin-bot melvin-bot bot added the Overdue label Aug 1, 2023
@twisterdotcom
Copy link
Contributor

@MonilBhavsar I think this is waiting on you again: #23390

@melvin-bot melvin-bot bot removed the Overdue label Aug 2, 2023
@melvin-bot melvin-bot bot added the Overdue label Aug 10, 2023
@zanyrenney
Copy link
Contributor

Hey @MonilBhavsar please can you resolve the conflicts on the PR so we can close this one out and get this live?

@melvin-bot melvin-bot bot removed the Overdue label Aug 11, 2023
@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Aug 21, 2023
@zanyrenney
Copy link
Contributor

@MonilBhavsar can you update this issue with the progress of the PR? I see we are currently blocked on a review.

@melvin-bot melvin-bot bot added the Overdue label Aug 29, 2023
@zanyrenney
Copy link
Contributor

Pinged @MonilBhavsar in DM to remind about updating this issue as to progress with #23390

@melvin-bot melvin-bot bot removed the Overdue label Aug 29, 2023
@MonilBhavsar
Copy link
Contributor

PR is in review. Conversation moved back to issue to finalize screens and translations. We're making progress though

@melvin-bot melvin-bot bot added the Overdue label Sep 11, 2023
@zanyrenney
Copy link
Contributor

@MonilBhavsar what's the latest now? Can you link the PR please?

@melvin-bot melvin-bot bot removed the Overdue label Sep 11, 2023
@MonilBhavsar
Copy link
Contributor

It is active in review. We're close. Latest update was here #23390 (comment)

@melvin-bot melvin-bot bot added the Overdue label Sep 19, 2023
@zanyrenney
Copy link
Contributor

Awesome thanks @MonilBhavsar - can you update this with the latest there when you have a moment please? Thanks!

@melvin-bot melvin-bot bot removed the Overdue label Sep 19, 2023
@MonilBhavsar
Copy link
Contributor

We've merge the last PR. Should be good to close once it is deployed to production

@melvin-bot melvin-bot bot added the Overdue label Sep 29, 2023
@zanyrenney
Copy link
Contributor

This has now been merged and deployed to production: #23390 (comment)

Great job @MonilBhavsar 🎉

@melvin-bot melvin-bot bot removed the Overdue label Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NewFeature Something to build that is a new item. Weekly KSv2
Projects
None yet
Development

No branches or pull requests