-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Comments
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 ? |
@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 |
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. |
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. |
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. |
Yeah! Will do. |
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. |
I'll work on the figma frames for this ASAP @twisterdotco (I'm going OOO midway through the week!) |
No, all good here. Thanks!
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? |
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. |
Great point! Yes, if they also remove it from authenticator app, they lose access. |
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. |
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 |
Asked in #product: https://expensify.slack.com/archives/C03U7DCU4/p1676984748010739 |
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. |
I would say yes, you should be asked for a magic code which is basically your password at that point. |
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. |
Ahhh hang on, no we updated the mockups for this flow already: https://github.com/Expensify/Expensify/issues/263669#issuecomment-1440083391 I saw the issue closed, but didn't think to put them in the doc, my bad. |
Cool cool, I feel like we should ask for the magic code as in the screenshots, but the Slack thread will be the best |
Asked here: https://expensify.slack.com/archives/C02HWMSMZEC/p1677767921475339. Will update the doc with these mockups when we get a 👍. |
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? |
This is with contributors now: #22335 |
PR is ready: #22335 (comment) |
@MonilBhavsar I think this is waiting on you again: #23390 |
Hey @MonilBhavsar please can you resolve the conflicts on the PR so we can close this one out and get this live? |
@MonilBhavsar can you update this issue with the progress of the PR? I see we are currently blocked on a review. |
Pinged @MonilBhavsar in DM to remind about updating this issue as to progress with #23390 |
PR is in review. Conversation moved back to issue to finalize screens and translations. We're making progress though |
@MonilBhavsar what's the latest now? Can you link the PR please? |
It is active in review. We're close. Latest update was here #23390 (comment) |
Awesome thanks @MonilBhavsar - can you update this with the latest there when you have a moment please? Thanks! |
We've merge the last PR. Should be good to close once it is deployed to production |
This has now been merged and deployed to production: #23390 (comment) Great job @MonilBhavsar 🎉 |
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
The text was updated successfully, but these errors were encountered: