[$250] "Terms of Service" and "Privacy Policy" is briefly shown before magic code is requested

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.63-3 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

1. Navigate to https://staging.new.expensify.com/
2. Log in with a new Gmail account
3. Navigate to Settings - About - "Terms of Service" or "Privacy"

Expected Result:

I should be logged into OD, "Terms of Service" and "Privacy Policy" should be visible without asking for magic code. Or the magic code modal should be visible without showing the "Terms of Service" and "Privacy Policy"

Actual Result:

"Terms of Service" and "Privacy Policy" is briefly shown before magic code is requested

Workaround:

Unknown

Platforms:

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

• [ ] Android: Standalone
• [x] 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/68e966bd-a62a-4396-8cd3-f9128c5e69f6

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021858637388498137276
• Upwork Job ID: 1858637388498137276
• Last Price Increase: 2024-11-25

Issue Owner

Current Issue Owner: @nkdengineer

[melvin-bot[bot]]

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

[Shahidullah-Muffakir]

Proposal

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

Magic code is requested if user click on "Terms of Service" or "Privacy Policy" in About page

What is the root cause of that problem?

this is causing the issue: https://github.com/Expensify/App/blob/99bf5541f9d49da20952bd7f2592b7e773a6073b/src/pages/settings/AboutPage/AboutPage.tsx#L169 and this: https://github.com/Expensify/App/blob/99bf5541f9d49da20952bd7f2592b7e773a6073b/src/pages/settings/AboutPage/AboutPage.tsx#L176

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

I think we can simply change it to the following, because the Terms of Service and Privacy Policy pages are publicly available user can simply open it by the URL: https://www.expensify.com/terms

change those two lines to: onPress={() => Link.openExternalLink(CONST.TERMS_URL)}and: onPress={() => Link.openExternalLink(CONST.PRIVACY_URL)}

[nkdengineer]

Edited by proposal-police: This proposal was edited at 2024-11-18 08:26:59 UTC.

Proposal

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

"Terms of Service" and "Privacy Policy" is briefly shown before magic code is requested

What is the root cause of that problem?

By default, openLink function is called when we click on a TextLink

https://github.com/Expensify/App/blob/d5e7a5abde0dfc52312a15feba776f3ef2f0be49/src/components/TextLink.tsx#L43

In this function, openOldDotLink is called if the href is an internalExpensifyPath. Then, we will open an OD link with authenToken, but maybe it doesn't work as expected in OD now. And a magic code page is opened with another account.

https://github.com/Expensify/App/blob/d5e7a5abde0dfc52312a15feba776f3ef2f0be49/src/libs/actions/Link.ts#L187-L190

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

I think the search teams and privacy can be a public URL so we shouldn't call with openOldDotLink. To fix for all places that we're using term and privacy url, we can create a new const OLD_DOT_PUBLIC_URLS which will contain all public old dot URL

const OLD_DOT_PUBLIC_URLS: string[] = [CONST.TERMS_URL, CONST.PRIVACY_URL];

then here we will not call openOldDotLink if it is a public URL. So it can simply open a new tab with the href here

const isPublicOldDotURL = OLD_DOT_PUBLIC_URLS.includes(href);
if (internalExpensifyPath && !isAttachment && !isPublicOldDotURL) {
    openOldDotLink(internalExpensifyPath);
    return;
}

https://github.com/Expensify/App/blob/d5e7a5abde0dfc52312a15feba776f3ef2f0be49/src/libs/actions/Link.ts#L187-L190

What alternative solutions did you explore? (Optional)

We can remove these lines if we always want to open old dot link without token.

https://github.com/Expensify/App/blob/d5e7a5abde0dfc52312a15feba776f3ef2f0be49/src/libs/actions/Link.ts#L187-L190

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[MitchExpensify]

Not overdue, Melvin

[ikevin127]

This issue should be Internal for two reasons:

1. External contributors don't have access to the OD / BE repos which is needed to debug this and find the root cause.
2. The issue's root cause seems to be OD / BE related since the ND / FE logic which handles the openOldDotLink works as expected, meaning it generates the shortLivedAuthToken and passes the data to the link correctly which if OD / BE would work as expected -> would automatically login the user and display the "Terms of Service" and "Privacy Policy" pages right away without displaying any additional login step hence redirecting away from the target pages.

Seems like both proposals are suggesting simply navigating to the "Terms of Service" and "Privacy Policy" pages as they are public, removing the auto-login logic. This can be an alternative until the OD / BE issue is fixed but ultimately I think that solution is a workaround which defeats the purpose of the existing auto-login logic (which is the root cause of our issue and should be fixed because it doesn't work as expected).

[rayane-d]

@MitchExpensify I agree with @ikevin127 that this should be internal. If we're looking for a temporary / workaround fix, @Shahidullah-Muffakir's proposal looks good to me. Let’s get an internal engineer’s opinion.

[nkdengineer]

@rayane-djouah

• I think you should add C+ review comment to assign internal to take a look.

• term URL and privacy URL are used in many places so I think we should fix them for all cases. What do you think about my proposal if we're looking for a temporary fix

[melvin-bot[bot]]

@MitchExpensify, @rayane-djouah Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[MitchExpensify]

I think you should add C+ review comment to assign internal to take a look.

Let's do this to get an internal engineer's opinion!

[rayane-d]

:ribbon::eyes::ribbon:

[melvin-bot[bot]]

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

[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? 💸

[MitchExpensify]

Hi @marcochavezf , can you help us move along here as internal assignee?

[marcochavezf]

Sure, I will check it out today

[melvin-bot[bot]]

@marcochavezf, @MitchExpensify, @rayane-djouah Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@marcochavezf @MitchExpensify @rayane-djouah this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[MitchExpensify]

Have you had a chance to check this out @marcochavezf ?

[marcochavezf]

This is an edge case for unvalidated accounts trying to log in from NewDot to OldDot. Given the ROI of this flow and the potential security risk, I think it's acceptable to open the public link for now or close it out

[nkdengineer]

I think it's acceptable to open the public link for now or close it out

term URL and privacy URL are used in many places so I think we should fix them for all cases. What do you think about my https://github.com/Expensify/App/issues/52666#issuecomment-2482242233 if we're looking for a temporary fix

@rayane-djouah Please help to review the proposal again.

[rayane-d]

Both proposals offer valid solutions to the issue, but @nkdengineer's proposal is particularly compelling as it reliably addresses all instances where these URLs are used. Therefore, I support moving forward with their approach.

:ribbon::eyes::ribbon: C+ reviewed

[melvin-bot[bot]]

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

[Shahidullah-Muffakir]

@rayane-djouah @MitchExpensify @marcochavezf, Nothing has changed in terms of the expected behavior or the content of the other proposal. Since my solution https://github.com/Expensify/App/issues/52666#issuecomment-2480828277 was already selected initially https://github.com/Expensify/App/issues/52666#issuecomment-2489613805, I find it unfair to switch to another without a solid justification.

I strongly believe that my solution is more effective because:

1. It’s simple and straightforward
2. It matches how similar problems are already solved in other parts of the app using openExternalLink

Could we revisit this decision?

[melvin-bot[bot]]

@marcochavezf, @MitchExpensify, @rayane-djouah Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[nkdengineer]

@marcochavezf Please help to check this comment.

[rayane-d]

1. @Shahidullah-Muffakir was the first to identify the root cause but didn't provide a detailed explanation. He suggested a fix for the URLs on the about page only using onPress={() => Link.openExternalLink(CONST.TERMS_URL)}. We need to check all similar URL usages to apply this fix consistently, and there's a risk that the bug could come back if links are added in other places using href in the future.

2. @nkdengineer offered a comprehensive proposal that fixes the bug across all URL usages without needing to review each one. By improving the Link.openLink function, we can ensure the bug won't reappear.

While both proposals are good, I lean towards @nkdengineer's solution because it covers more ground and prevents the issue from coming back. However, I'll let @marcochavezf make the final call on how we should move forward with this.

[marcochavezf]

I agree with @rayane-djouah analysis in the last comment. @Shahidullah-Muffakir @nkdengineer onward and upward 🚀

[melvin-bot[bot]]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot[bot]]

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

• https://github.com/Expensify/App/pull/53741

If no regressions arise, payment will be issued on 2024-12-30. :confetti_ball:

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

• @rayane-djouah requires payment (Needs manual offer from BZ)
• @nkdengineer requires payment (Needs manual offer from BZ)