[$250] User is able to disable 2FA when connected to Xero accounting

[m-natarajan]

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: 9.0.64-4 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @allgandalf Slack conversation (hyperlinked to channel name): ts_external_expensify_bugs

Action Performed:

1. Connect to Xero
2. Disable two-factor authentication in settings

Expected Result:

We should block this with a RHP stating that we cannot disable two-factor auth while connected to xero

Actual Result:

User is able to disable two-factor authentication and on each login it just asks the user to re-enable it

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/a45269eb-8c61-4e30-83f5-5e589b882baf

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021859734183969700022
• Upwork Job ID: 1859734183969700022
• Last Price Increase: 2024-11-21

Issue Owner

Current Issue Owner: @

[melvin-bot[bot]]

Triggered auto assignment to @RachCHopkins (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.

[allgandalf]

@Expensify/design Can you please let us know the expected result here? I wrote that expected result thinking it was the ideal one but more ideas would be appreciated here.

[dannymcclain]

I think when the user taps Disable two-factor authentication we should open one of our confirmation modals letting them know they can't do that because of their Xero connection. Something like this but with help from @jamesdeanexpensify on the copy.

[Shahidullah-Muffakir]

Edited by proposal-police: This proposal was edited at 2024-11-20 21:13:19 UTC.

Proposal

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

User is able to disable 2FA if connected to xero

What is the root cause of that problem?

we are adding the disableTwoFactorAuth to menu items, without checking for Xero or other connections: https://github.com/Expensify/App/blob/2d2555b173b47c0bce0b1c16b275f1f98639664c/src/pages/settings/Security/TwoFactorAuth/Steps/EnabledStep.tsx#L29

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

I think we can simply don't add this item if user has a connection (if we want to hide it for other connections as well) or Xero connection:

+ const hasAccountingConnection = !isEmptyObject(policy?.connections);
or
+ const hasXeroConnections = allPolicies?.some(policy => policy?.connections?.[CONST.POLICY.CONNECTIONS.NAME.XERO]);

 menuItems={
+    !hasAccountingConnection ? [
        {
            title: translate('twoFactorAuth.disableTwoFactorAuth'),
            onPress: () => {
                setStep(CONST.TWO_FACTOR_AUTH_STEPS.GETCODE);
            },
            icon: Expensicons.Close,
            iconFill: theme.danger,
            wrapperStyle: [styles.cardMenuItem],
        },
    ] : []}

What alternative solutions did you explore? (Optional)

[FitseTLT]

Proposal

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

User is able to disable 2FA when connected to Xero accounting

What is the root cause of that problem?

We are not disabling the disabling of 2FA while there is xero accounting connection https://github.com/Expensify/App/blob/2d2555b173b47c0bce0b1c16b275f1f98639664c/src/pages/settings/Security/TwoFactorAuth/Steps/EnabledStep.tsx#L27-L39

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

We should first check for xero connection by checking if there is a single policy that the current user owns/admins where policy.connections.xero exists

function hasXeroConnections() {
    return !!allPolicies?.some((policy) => isPolicyOwner(policy, getCurrentUserAccountID()) && !!policy?.connections?.[CONST.POLICY.CONNECTIONS.NAME.XERO]);
}

we can also use isPolicyAdmin instead of isPolicyOwner according to the requirements

then we can show cannot disable 2FA ConfirmModal (some what similar to our FocusModeNotification with the respective copys and buttons with callbacks) In here when they press the disable button in EnabledStep https://github.com/Expensify/App/blob/2d2555b173b47c0bce0b1c16b275f1f98639664c/src/pages/settings/Security/TwoFactorAuth/Steps/EnabledStep.tsx#L33-L34

we will set the modal visibility and return if PolicyUtils.hasXeroConnections() is true

when pressing on the Got it button on the modal we can either dismiss only the confirm modal or we can additionally also dismissModal the 2FA RHP too.

or optionally we can hide/disable the disable 2FA button and inform users with some message on the bottom of the RHP by avoiding the menu item or passing disable prop respectively when hasXeroConnections

What alternative solutions did you explore? (Optional)

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-11-20 21:42:19 UTC.

Proposal


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

User is able to disable 2FA when connected to Xero accounting

What is the root cause of that problem?

• We do not check if the user has any workspace connected to Xero before calling going to next step. https://github.com/Expensify/App/blob/f5fd9526a7ecbf5c1171766bae45570d1356f712/src/pages/settings/Security/TwoFactorAuth/Steps/EnabledStep.tsx#L32-L34

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


• First we need to create a function hasWorkspaceWithXeroConnected which will check if the user has a policy with xero or not.

  function hasWorkspaceWithXeroConnected(currentUserLogin: string | undefined): boolean {
  const activePolicies = getActiveAdminWorkspaces(allPolicies, currentUserLogin);
  const accountingIntegrations = Object.values(CONST.POLICY.CONNECTIONS.NAME);
  // isPolicyAdmin(policy, currentUserLogin) check is optional.
  return activePolicies.some((policy) => {
      const connectedIntegration = getConnectedIntegration(policy, accountingIntegrations);
      return shouldShowPolicy(policy, NetworkStore.isOffline(), currentUserLogin) && isPolicyAdmin(policy, currentUserLogin) && connectedIntegration === CONST.POLICY.CONNECTIONS.NAME.XERO;
  });
  }

• Then in EnabledStep, we will add ConfirmModal.

          const [isXeroModalVisible, setIsXeroModalVisible] = useState(false);
          const [session] = useOnyx(ONYXKEYS.SESSION);
          // We need to add translations for the text.
          <ConfirmModal
              title="Xero requires 2FA"
              onConfirm={() => {
                  setIsXeroModalVisible(false);
              }}
              isVisible={isXeroModalVisible}
              prompt="Your Xero integration requires two-factor authentication. You cannot disable 2FA with an active Xero integration."
              confirmText={translate('common.buttonConfirm')}
              shouldShowCancelButton={false}
          />

• Then we should modify onPress callback

                  menuItems={[
                      {
                          title: translate('twoFactorAuth.disableTwoFactorAuth'),
                          onPress: () => {
                              if (hasWorkspaceWithXeroConnected(session?.email)) {
                                  setIsXeroModalVisible(true);
                                  return;
                              }
                              setStep(CONST.TWO_FACTOR_AUTH_STEPS.GETCODE);
                          },
                          icon: Expensicons.Close,
                          iconFill: theme.danger,
                          wrapperStyle: [styles.cardMenuItem],
                      },
                  ]}

• If we want to navigate when Settings > Workspaces from alert modal, then we can use confirm and cancel button both.

What alternative solutions did you explore? (Optional)

Result

https://github.com/user-attachments/assets/fb029e6e-e053-4f3d-aab4-2e8f239674e8

[allgandalf]

@RachCHopkins if we decide to make it external, can you assign me as C+ here please, I have context from slack and some design suggestions from here

[shawnborton]

Danny's solution makes total sense to me. We might consider linking to Settings > Workspaces from the alert modal copy, but I don't feel strongly about that.

[jamesdeanexpensify]

A little late here, but what about for the copy, something like:

Cannot disable 2FA Two-factor authentication (2FA) is required for your Xero connection and cannot be disabled.

Normally I would give them next steps if they really want to disable 2FA, but generally it's bad practice and I also wouldn't want them to disconnect their accounting integration. So I kinda feel....ok?...not saying anything else here.

[shawnborton]

Works for me!

[RachCHopkins]

Definitely able to repro this.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Current assignee @allgandalf is eligible for the External assigner, not assigning anyone new.