[HOLD for payment 2023-05-29]OldDot deeplink to Concierge does not keep user signed in using Firefox

[mountiny]

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

---

related to https://github.com/Expensify/Expensify/issues/276016

Action Performed:

Break down in numbered steps

1. Create account in OldDot (expensify.com)
2. Click the Concierge chat bubble which should redirect you to NewDot Concierge tab (update the url to point to your localhost with the token)

Expected Result:

Describe what you think should've happened

1. The old dot should show on the new tab and you should be signed in

Actual Result:

Describe what actually happened

1. In firefox, they are not signed in in staging it seems but this works in chrome

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Given that the affected user only uses Google SSO and they cannot use google chrome for their internal reasosn they do not have a workaround

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Firefox
• MacOS / Desktop

Version Number:
Reproducible in staging?:
Reproducible in production?:
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
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @twisterdotcom (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[mountiny]

Asking callstack to investigate

[twisterdotcom]

This is weird. LMK if I can help. I won't add any External labels for now.

[mountiny]

They will get someone tomorrow https://expensify.slack.com/archives/C03UK30EA1Z/p1681849120955559

Seems like in Firefox for some reason the app thinks its offline when loaded for a first time which then leads to the deeplink not working. Its weird because this works locally in firefox and also it works in other browsers just fine

[thiagobrez]

Hi! I'm Thiago from Callstack - expert contributor group - and I would like to take a look at this issue

[mountiny]

Thanks Thiago, he will look into this on Monday

[mountiny]

Feel free to drop your investigation here or also in Slack for faster communication @thiagobrez this one might be tricky to resolve

[thiagobrez]

Re-posting here the investigation results I posted on Slack, to keep this updated:

On Firefox in production (only production) the app quickly becomes offline at first, before going online, which might be breaking the deeplink, or at least making debugging harder.

Screen.Recording.2023-04-25.at.10.48.38.mov

This can be stated by looking at the NetInfo state changes. isInternetReachable starts null, goes to false, then finally to true.

NetInfo is configured to test reachability against Expensify's API. We can see that in the network tab, it tries to send a HEAD request twice but fails, which causes isInternetReachable to go false. The error is NS_BINDING_ABORTED, which is a Firefox-specific error indicating that the request has been aborted by the browser (didn't even start, 0 bytes transferred), usually by user actions that interrupt the request (such as clicking a button that redirects), or automatic redirects and timeouts.

This only happens on Firefox in production, not on Chrome or Firefox in development. The difference between Firefox production and development is that in development the HEAD request points to the proxy localhost:8080/api.

This situation is happening even if you do not come from the OldDot concierge. If you just load new.expensify.com, you will see in the Network tab the first 3 requests all get aborted (even the first one which is not related), and then it starts working. Seems like Firefox is aborting everything in the first few seconds? My best guess would be that the automatic redirect between the long URL with the authToken and the final URL /r/<reportId> is causing these first requests to be aborted.

We can try delaying the redirect for a few seconds and check if those requests will succeed. We can also try playing with the default NetInfo timeouts, or with Content-Security Policies in the client. But for that I would need a way to run the app in production mode, so I can reproduce it locally. Is there any way I can do that? I tried npm run build and serving the static files through an http server, but then I get CORS issues, which makes sense since that is not through the proxy.

[mountiny]

Thank you very much for such thorough investigation, I am not sure whats the best way you could test this locallly, but if you create a PR with the changes I can make an internal release to build it https://expensify.slack.com/archives/C01GTK53T8Q/p1682444061299849 started a thread to discuss this in case anyone knows better way to debug this

[thiagobrez]

@mountiny Draft PR with first attempt: #18021

[mountiny]

Made the build, thanks!

[thiagobrez]

Update:

On Firefox,signInWithShortLivedAuthToken is fired and even succeeds, but for some reason, the onyx store is not updated to contain authToken, so the user never reaches the authenticated pages

[mountiny]

I would forbid people from using Firefox..

What are the next steps here, any ideas how this might be happening? Thank you!

[thiagobrez]

@mountiny Definitely, it's not logging in because isAuthenticated on Expensify.js never goes true. This variable changes when props.session changes (coming from Onyx).

In LogInWithShortLivedAuthTokenPage, Session.signInWithShortLivedAuthToken is called and succeeds. But I need to understand better how and why Onyx is not being updated with the result of this request. To be confirmed, but I think we're assigning someone else on our end to look into this with fresh eyes while I'm working in the 2-FA implementation

[mountiny]

Makes sense, thank you!

[twisterdotcom]

Does this mean you can't use new.expensify.com if you use Firefox? Mozilla are literally a customer of ours.

[thiagobrez]

Hey @twisterdotcom, this issue only happens when being redirected from OldDot to NewDot via the Concierge chat bubble. You can still log in manually using Firefox and it should work as usual

[bionanek]

Hi! I'm Jakub from Callstack - expert contributor group - and I would like to take over this issue as Thiago is currently working on a different issue

[mountiny]

Correct this seems to be issue with the way how our deeplink from old dot to new dot works and the fact they only use Google SSO to log in and they cant do that now on New Dot.

[twisterdotcom]

Okay okay, this is very edgy then. I go back on my caution here.

[melvin-bot]

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

[melvin-bot]

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

• fix: fix firefox redirection from old dot #18631

If no regressions arise, payment will be issued on 2023-05-25. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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

• [@mountiny] The PR that introduced the bug has been identified. Link to the PR:
• [@mountiny] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mountiny] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@ntdiary / @thiagobrez / @koko57] Determine if we should create a regression test for this bug.
• [@ntdiary / @thiagobrez / @koko57] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

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

• fix: fix firefox redirection from old dot #18631

If no regressions arise, payment will be issued on 2023-05-29. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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

• [@mountiny] The PR that introduced the bug has been identified. Link to the PR:
• [@mountiny] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mountiny] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@ntdiary / @thiagobrez / @koko57] Determine if we should create a regression test for this bug.
• [@ntdiary / @thiagobrez / @koko57] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[ntdiary]

@mountiny, TBH, I don't know what to base on to determine whether we should create a regression test for this bug. 😂

[mountiny]

I dont think we need a platform specific test for this to be done each regression suite

[twisterdotcom]

Okay, I marked the regression test issues off. There are two payments here? [HOLD for payment 2023-05-29] [HOLD for payment 2023-05-25]

@mountiny do we need to pay out @ntdiary and @koko57 or is everybody from Callstack here?

[mountiny]

@ntdiary $1000 for testing, review and help with the proposal. @koko57 is from Callstack :)

[twisterdotcom]

Okay @ntdiary - could you DM me (ted@expensify.com) on new.expensify.com or post here your Upwork profile details for payment please?

[ntdiary]

@twisterdotcom, this is my profile link: 🙂
https://www.upwork.com/freelancers/ntdiary

[twisterdotcom]

https://www.upwork.com/nx/wm/offer/24734360

[mountiny]

@ntdiary has this been paid out, can we close the issue?

[ntdiary]

@ntdiary has this been paid out, can we close the issue?

@mountiny, not yet, last week when I saw and accepted the offer, team may have been off work (we have a time difference).
Maybe this can be paid out today. : )

[mountiny]

ok there was a bank holiday here in UK so lots of people were off cc @twisterdotcom

[twisterdotcom]

Paid!