[$375] [HOLD for payment 2024-08-01] [Wave Collect] [Xero] Add 2FA requirement in Authorization flow

[francoisl]

Problem

In order to remain compliant with Xero's third-party app requirements, we need to force workspace admins to enable 2FA before they can use the connection.

Additional internal context

• #whatsnext post
• https://github.com/Expensify/Expensify/issues/397316

Solution

• For new connections:

  • Before starting the OAuth flow, make sure the admin has 2FA enabled. If they don't, show the 2FA enablement page. After they finish setting it up, automatically resume the Xero auth flow.
  • See mockups in this comment below

• For existing connections:

  • If a user signs in and they're admin on a workspace that is connected to Xero but don't have 2FA enabled, show them the 2FA setup steps upon signing in
  • See mockups from this comment

cc @lakchote @zanyrenney

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f129776196b2ac5f
• Upwork Job ID: 1821257266596661328
• Last Price Increase: 2024-08-07

[melvin-bot]

Triggered auto assignment to @sakluger (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[melvin-bot]

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

[melvin-bot]

Triggered auto assignment to Design team member for new feature review - @dannymcclain (NewFeature)

[francoisl]

I'll add a few mockups when I have edit access to Figma again.

[dannymcclain]

@francoisl let me know if you need me to mock anything up or take care of anything in Figma! 🤝

[francoisl]

Thanks. I got an email from Figma saying that Jon gave me edit access but I still can't seem to edit anything.

Basically I was trying to edit the Initial Setup part of the Xero mockups and add the 2FA flow in between. I was thinking of making two variations:

• one where we show the Two-factor authentication flow directly after you click Set up
• one with an intermediary modal with additional context, with text like "To connect to Xero, please enable Two-factor authentication first [Continue]"

[dannymcclain]

Super weird. I see that you were upgraded yesterday, but when I went into the file it still said you were requesting access (I gave it to you). I wonder if that file just hadn't been refreshed or something since Jon upgraded you. Anyways, try it again if you want, or I'm happy to do some jammin'.

[dannymcclain]

Added a little flow here in Figma.

I think it's better to tell people "Hey you need to enable 2fa to use xero" so I like including the intermediary modal.

[francoisl]

That's great, thanks Danny!

Looks like I was wrong about the sign-in flow and <ValidateLoginPage> doesn't do what I expected, so we'll also need to make some changes to make people enable 2FA when they sign in. I'm thinking we add an extra step after entering the magic code. Here's a rough draft - Figma here:

I changed the text description, but we can also make it more generic to accommodate the case when people sign in and they need to enable 2FA because they're on a domain group with that requirement.

[dannymcclain]

Ah I see, yeah that makes sense to me!

[muttmuure]

Looks great to me

[dannymcclain]

Any updates here?

[rushatgabhane]

yo yo yo sorry i lost track of this issue. can you please assign it to me?

[rushatgabhane]

PR raised #44059

[rushatgabhane]

@francoisl everything tests well but we might have some hiccups on how 2FA works.

For the flow: admin signs in and has a policy connected to Xero.

1. Login
2. Start 2FA steps
3. Scan QR code in authenticator
4. Paste the 6 digit code

Expected: the app becomes usable again.
Actual: the API call to enable 2FA completes but the 2FA loader spins infinitely. The app is unusable until we do a re-login.

Screen.Recording.2024-06-20.at.08.mp4

[francoisl]

Hm weird. I'll look into this today.

[francoisl]

As far as I can tell, it's because the authToken returned by TwoFactorAuth_Validate is not merged into Onyx before the next API call to ReconnectApp.

In short, when 2FA is required to be enabled on sign-in like in our case here, the backend will first send an authToken that has very limited permissions - one of the very few API commands it can execute is TwoFactorAuth_Validate. That API command then returns a "regular" authToken that can execute other API commands.

As an example here, after signing in let's say I get this authToken 0322AAEA4169... - this is a two-factor setup authToken with limited permissions.

Once I enter an OTP code to enable 2FA, the client calls TwoFactorAuth_Validate, which returns a new authToken, in this case E6CE5CDF6621...

However, immediately after we see that there is an API call to ReconnectApp, but for some reason it's still using the limited authToken 0322AAEA4169...

This is why the backend returns an error "Two Factor Authentication Required" and the app doesn't load.

I think the reason we call ReconnectApp before updating the authToken in Onyx is because of that OnyxUpdates.saveUpdateInformation() call here, though I can't think of how to fix the issue. I tried adding WRITE_COMMANDS.TWO_FACTOR_AUTH_VALIDATE to const requestsToIgnoreLastUpdateID but that doesn't fully solve the issue because then we don't call ReconnectApp at all.

[c3024]

@sakluger

4. I reviewed the PR 😆

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.11-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [Wave Collect][Xero] Enforce 2FA for xero #45702

If no regressions arise, payment will be issued on 2024-08-01. 🎊

For reference, here are some details about the assignees on this issue:

• @rushatgabhane requires payment through NewDot Manual Requests
• @c3024 requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@rushatgabhane / @c3024] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[rushatgabhane]

@sakluger i think 500 would be great because it was hard to nail down the nitty gritty of this task.

[rushatgabhane]

@c3024 could you please add regression test steps

[c3024]

Regression test proposal

Test 1:

Pre-requisite:

Have an account A that
1. does not have 2FA.
2. is an admin of a control policy P

Steps:

1. Login with the above specified account A
2. Click on Workspaces > Click on the control policy P specified in the pre-requisite > Click on Accounting in the left hand panel
3. Click on Connect Xero
4. Verify that 2FA modal is shown
5. Complete the 2FA setup flow
6. Verify that the Xero integration page opens now

Test 2:

Pre-requisites:

Have an account A that
1. does not have 2FA
2. has a Xero accounting configured policy
3. is an admin of this policy

Steps:

1. Login with the above specified account A
2. Verify that 2FA modal is shown with loading indicator behind
3. Click on Enable Two Factor Authentication button on the modal
4. Verify that the Avatar and Name in the left hand panel of Settings page has loading indicator
5. Click on Two Factor Authentication in the center panel and complete the 2FA setup flow
6. Verify that now the Avatar and Image in the left hand panel of Settings page is fully loaded with details
7. Click on Inbox
8. Verify that LHN and Center Pane load fully

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @rushatgabhane owed $375 via NewDot
• ROLE: @c3024 - $375 offer sent via Upwork (LINK)

BugZero Checklist (@sakluger)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[JmillsExpensify]

Can I get a payment summary on this issue?

[sakluger]

Sorry about that! I missed it because it wasn't automatically moved to Daily.

I chatted with @francoisl via DM to figure out what was going on here. We won't penalize for regressions here because the app was already unusable if you were an admin on a Xero workspace prior to the first PR.

Regarding price, we both thought that $500 was a bit high, but given the trickiness, we'll pay $375 - a bit more than the standard $250.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01f129776196b2ac5f

[melvin-bot]

Current assignees @rushatgabhane and @c3024 are eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Upwork job price has been updated to $375

[sakluger]

@JmillsExpensify the payment summary is ready: #43015 (comment)

[JmillsExpensify]

$375 approved for @rushatgabhane

[sakluger]

I completed the payment to @c3024 via Upwork.