[$250] Xero – Can’t enable 2FA after relogin if Xero connection is active

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: v9.0.25-13 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4899502 Email or phone of affected tester (no customers): ponikarchuks+128824@gmail.com Issue reported by: Applause Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Log in
3. Create a WS
4. Connect Xero to the WS
5. Navigate to account settings and disable 2FA
6. Log out of the account and log back in
7. Verify that 2FA modal is shown
8. Click on Enable Two Factor Authentication
9. Click on Two Factor Authentication in the center panel and complete the 2FA setup flow

Expected Result:

Able to complete the 2FA setup flow

Actual Result:

“Hold up! We need you to validate your account first. To do so, sign back in with a magic code or verify your account here.” message appears

Workaround:

Unknown

Platforms:

• [x] Android: Native
• [x] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/ea9bbfcf-41d2-4a9f-873d-2f5b024d3bc9

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0199165fd987545c87
• Upwork Job ID: 1831077661756285595
• Last Price Increase: 2024-09-10
• Automatic offers:
• dukenv0307 | Reviewer | 103989578

Issue Owner

Current Issue Owner: @dukenv0307

[melvin-bot[bot]]

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[IuliiaHerets]

@johncschuster FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Validate account page shows not found after relogin.

What is the root cause of that problem?

From the video, we can see that we open the 2FA page while the app is still loading (and it looks like it loads infinitely on the video). We can replicate this by using "Clear cache and restart".

When the app is still loading, we don't have the user information yet, so the validated condition is false at this point and a message to validate the account will be shown. https://github.com/Expensify/App/blob/fbc672053a37a380c2322eb01b484a7118ea0b47/src/pages/settings/Security/TwoFactorAuth/Steps/CodesStep.tsx#L41 https://github.com/Expensify/App/blob/fbc672053a37a380c2322eb01b484a7118ea0b47/src/pages/settings/Security/TwoFactorAuth/Steps/CodesStep.tsx#L133

I believe it's the first issue since we don't know yet whether the user is already validated or not, but we still show the message asking the user to validate their account.

Next, when the user presses the "verify your account here", it will navigate to the contact method details page passing the user email from the loginList object. https://github.com/Expensify/App/blob/fbc672053a37a380c2322eb01b484a7118ea0b47/src/components/ValidateAccountMessage.tsx#L43-L45

However, loginList is also not available yet while the app is loading. So, the value is undefined, thus a not found contact method page is shown.

What changes do you think we should make in order to solve the problem?

To address the first issue, we can show a loading screen when the user data is not available yet.

!user ? (<View><FullScreenLoadingIndicator /></View> : ...

For the loginList issue, there are several options. First, we can also show the loading screen if it's empty, so the code becomes:

(!user || !loginList) ? (<View><FullScreenLoadingIndicator /></View> : ...

Or, we can hide the "verify your account here" text if loginList is not available. https://github.com/Expensify/App/blob/fbc672053a37a380c2322eb01b484a7118ea0b47/src/components/ValidateAccountMessage.tsx#L40-L49

{loginList && (
    {translate('bankAccount.validateAccountError.phrase3')}
    <TextLink ... />
)}

Or, we can take the login from session.email (or credentials.login or account.primaryLogin)

[melvin-bot[bot]]

@johncschuster Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0199165fd987545c87

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @dukenv0307 (External)

[daledah]

Proposal

Please re-state the problem that we are trying to solve in this issue.

“Hold up! We need you to validate your account first. To do so, sign back in with a magic code or verify your account here.” message appears

What is the root cause of that problem?

2FA is required if using Xero.

When Xero connection is active then users disable 2FA, they can't login again because the it requires 2FA

What changes do you think we should make in order to solve the problem?

We shouldn't allow users to disable 2FA in case Xero is active. To do that we can

1. Disable Disable 2FA button
2. Leave the message for users to indicate that they can't disable that button, If they still want to disable 2FA, they need to disconnect Xero connection first (we can add the link the Xero connection)

What alternative solutions did you explore? (Optional)

[dukenv0307]

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

Triggered auto assignment to @Beamanator, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[dukenv0307]

@Beamanator I need to confirm the expectation here

I see users using Xero need to enable 2FA first

1. Can they disable 2FA after connecting Xero successfully?
2. If yes, Is it BE bug since it returns the error for that user?

IMO, we shouldn't allow users to disable 2FA if there's at least one Xero connection.

[dukenv0307]

@Beamanator Can you check my concern above ^? Thanks

[Beamanator]

Yo yo that's definitely a good question @dukenv0307 - I'm not sure so I'll have to bring this to slack, but do you know if connecting to other integrations does or does not require 2FA?

[Beamanator]

Asking in slack here

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[dukenv0307]

@Beamanator Is it BE bug, or should we disable it on FE side?

[melvin-bot[bot]]

@Beamanator @johncschuster @dukenv0307 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[Beamanator]

Sorry for the delay here 🙈

IMO, we shouldn't allow users to disable 2FA if there's at least one Xero connection.

It was agreed in slack that it would be simplest to prevent users from disabling 2FA if there's at least one Xero connection 👍

[Beamanator]

I think that answers all the questions needed, right @dukenv0307 ?

[dukenv0307]

Thanks @Beamanator, Let's go with @daledah's proposal since he suggested to disable 2FA if there's at least one Xero connection.

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

Current assignee @Beamanator is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot[bot]]

@Beamanator, @johncschuster, @dukenv0307 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[melvin-bot[bot]]

📣 @daledah You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

[daledah]

@dukenv0307 Do we need to add a RBR message to inform user to disconnect Xero first before disabling 2FA? If we do, can you help me to ask for the translations? I don't have access to Expensify Slack channel right now.

[dukenv0307]

@Beamanator What do you think about this comment? IMO, we should add the message

[dukenv0307]

@Beamanator Can you help check the concern above? Thanks

[Beamanator]

Sorry for the delay - IMO no we don't need that RBR / warning message, but I think it would be great to get another opinion - @johncschuster should we bring in design team for that or bring this discussion to slack / maybe #product?

[dukenv0307]

@johncschuster do you have another idea on that?

[dukenv0307]

@Beamanator @johncschuster friendly bump

[francoisl]

Chiming in here (coming from https://expensify.slack.com/archives/C05LX9D6E07/p1727177826122279), I think the root issue will still persist if you invite a new admin to a Xero workspace and they don't have 2FA enabled.

The selected proposal is still valuable, but we should also look into fixing the root issue where you can't even enable 2FA when you sign in.

[daledah]

@francoisl How do we know the new admin doesn't have 2FA enabled? Should we disable inviting that user from FE side?

[francoisl]

How do we know the new admin doesn't have 2FA enabled?

We don't 😄 You can even invite someone who doesn't have an Expensify account yet.

Should we disable inviting that user from FE side?

My first instinct would be to say no. We'd most likely need to make some backend changes to the invite API command and return an error from there. In addition, there's also the case where 2FA can be required at the domain level via OldDot, and in that case anyone who signs in to NewDot should theoretically be prompted to enable 2FA (even if there they're not admin on a Xero workspace)

So anyway, I still think the best next step is to fix the bug where you can't enable 2FA on sign-in, when you're prompted to enable it.

[trjExpensify]

So anyway, I still think the best next step is to fix the bug where you can't enable 2FA on sign-in, when you're prompted to enable it.

Totally agree. Can we prioritise that? This has just been observed in a FS by @danielrvidal here.(Internal ref).

[dukenv0307]

@daledah so we should put this PR on hold cc @Beamanator

[Beamanator]

Interesting so yeah the current PR doesn't seem to deal with "the bug where you can't enable 2FA on sign-in, when you're prompted to enable it." right? @dukenv0307 we never got a proposal for fixing that specifically, right?

[trjExpensify]

Doesn't this proposal address it? https://github.com/Expensify/App/issues/48208#issuecomment-2316658593

@bernhardoj it would be good to include a video if you can, so we're all on the same page.

[dukenv0307]

the current PR doesn't seem to deal with "the bug where you can't enable 2FA on sign-in, when you're prompted to enable it." right?

@Beamanator Yes, but I don't think we can do it on FE side

After login, SignInUser API returns

That makes the modal shows

https://github.com/Expensify/App/blob/7dde09e01e77ba55a79d043847b7ff86c5c1d1fd/src/Expensify.tsx#L103-L108

And isUserValidated is false -> we can't perform enable 2FA

[dukenv0307]

Should BE return user?.validated as true?

[bernhardoj]

Doesn't this proposal address it? https://github.com/Expensify/App/issues/48208#issuecomment-2316658593

Actually, my proposal is addressing another thing 😅.

As discussed, we can't enable the 2FA because the user is detected as an unvalidated user (user?.validated is undefined).

In a normal login flow (without Xero), user.validated data will be available from the OpenApp response. However, OpenApp throws an error if we login to an account with Xero connected without 2FA enabled.

From this discussion, I think we can consider using account.validated (which is available from the sign-in API response) instead of user.validated to check whether the user is validated or not.

[trjExpensify]

Are you saying we should change every instance of this to account?.validated to solve the problem?

[trjExpensify]

Kinda' adjacent, but we have a bunch of places where user?.validated exists in the code and I still don't understand why we have both user?.validated and account?.validated in Onyx for what seems like to me the exact same thing?

[bernhardoj]

To solve this issue, we just need to change it on the CodesStep page.

I still don't understand why we have both user?.validated and account?.validated in Onyx for what seems like to me the exact same thing?

Me neither.

[Beamanator]

I definitely like the idea of trying to update our code to ONLY use account.validated OR user.validated 👍 I don't thinkkk there's any diff, but would have to look into that in a separate issue.

So it sounds like we have a few potential solutions:

1. On the CodesStep page we start looking at account?.validated instead of user?.validated
2. We send the user?.validated data in the SigninUser response

Right? If those are the two possible solutions, I'd vote we go with 1️⃣ for now (can be worked externally) so I'll start the discussion about "why both account.validated AND user.validated, and can we combine them"

Thoughts?

[trjExpensify]

Yep, works for me!

I'll start the discussion about "why both account.validated AND user.validated, and can we combine them"

Please do, it's rife for confusion and using the wrong one. 😂

[Beamanator]

Groovy 👍 so @dukenv0307 i think we might be able to go with @bernhardoj 's proposal to fix 1️⃣ on the front end above, right? Meanwhile i just posted about this in slack here

[dukenv0307]

@bernhardoj Can you create the PR fix?

[bernhardoj]

PR is up

cc: @dukenv0307