[HOLD for payment 2024-02-14] [$500] Login - After opening public room and killing app user stuck on login page & back icon shown

[lanitochka17]

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: 1.4.31-4
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/4228315
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Navigate to a public room via deeplink, e.g. https://staging.new.expensify.com/r/5408450846930023
2. Kill the app
3. Open the app again
4. Log in with any email address

Expected Result:

The back icon is no longer displayed and user is able to log in

Actual Result:

The back icon is displayed and it is not functioning. When user is trying to log in, user is stuck on the sign page with the "Incorrect magic code" message. After restarting the app, user is logged in

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6353889_1706134250494.video_2024-01-24_17-10-20.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01434d068cd2fd6a96
• Upwork Job ID: 1750328972969820160
• Last Price Increase: 2024-01-25
• Automatic offers:
• jjcoffee | Reviewer | 28131302
• bernhardoj | Contributor | 28131303

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp.
CC @quinthar

[bernhardoj]

Proposal

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

As anonymous user, closing and reopening the app will show a sign in page/modal, but the back button isn't functional.

What is the root cause of that problem?

When we open the app, openReportFromDeepLink will always be called, but with a null url, because we didn't do any deeplink. If we open it as an anonymous user, it will check whether the URL can be accessed by the user or not.

App/src/libs/actions/Report.ts

Lines 2068 to 2071 in 0d0b0a8

	if (Session.isAnonymousUser() && !Session.canAccessRouteByAnonymousUser(route)) {
	Session.signOutAndRedirectToSignIn(true);
	return;
	}

App/src/libs/actions/Session/index.ts

Lines 114 to 115 in 0d0b0a8

	if (shouldReplaceCurrentScreen) {
	Navigation.navigate(ROUTES.SIGN_IN_MODAL, CONST.NAVIGATION.TYPE.UP);

Because it's null, canAccessRouteByAnonymousUser will return false and will be redirected to the sign in page. Notice that we pass true to the param which will replace the current screen with the sign-in screen. So, the LHN is replaced with the sign-in page making it impossible to go back.

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

If the url (route) is empty, don't redirect to sign in page

if (route && Session.isAnonymousUser() && !Session.canAccessRouteByAnonymousUser(route)) {

UPDATE:
btw, I notice a similar issue with a different step. You won't be able to go back after pressing the "Sign in" button in the LHN header and that's because the press event object is passed to the function which will evaluate to true replacing the LHN.

App/src/pages/home/sidebar/SignInButton.js

Line 19 in 0d0b0a8

onPress={Session.signOutAndRedirectToSignIn}

we should update it like this () => Session.signOutAndRedirectToSignIn() to every occurrences

[dukenv0307]

Proposal

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

• Login - After opening public room and killing app user stuck on login page & back icon shown

What is the root cause of that problem?

• Generally, the RCA is that, we are considering the undefined route as the route that cannot be accessed by anonymous user. The detail is below.

• When we open the public room, Session.isAnonymousUser(): true

• Then kill app, reopen app, the below logic is called:
  

  App/src/libs/actions/Report.ts

  Lines 2068 to 2071 in 0d0b0a8

  	if (Session.isAnonymousUser() && !Session.canAccessRouteByAnonymousUser(route)) {
  	Session.signOutAndRedirectToSignIn(true);
  	return;
  	}

• Session.canAccessRouteByAnonymousUser(route) return false because in this case, route param is empty, that is not in routesCanAccessByAnonymousUser

• Because the signOutAndRedirectToSignIn is called with shouldReplaceCurrentScreen param is true, so it will replace the current screen (Home screen). Now, the screen stack just contains the RightModalNavigator, which leads to the back button and the "sign in" button is not functioning

• Note: The shouldReplaceCurrentScreen is to fix the bug:

the issue where you're able to go back from the sign in modal to the "protected route" (after being forced navigate to the sign in modal when trying to accessing the "protected route" directly. (see comment)

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

• We can update the routesCanAccessByAnonymousUser to:

    const routesCanAccessByAnonymousUser = [..., ROUTES.HOME];

because anonymous user can access the Home route.

• Then update:
  

  App/src/libs/actions/Session/index.ts

  Line 922 in 0d0b0a8

  const canAccessRouteByAnonymousUser = (route: string) => {

  
  to:

const canAccessRouteByAnonymousUser = (route: string = ROUTES.HOME) => {

• Also, we need to update routesCanAccessByAnonymousUser because it is outdated. There are many routes that can be accessed by anonymous user but we missed before, for example, desktop-signin-redirect, a/:accountID/avatar, r/:reportID/avatar, ...

What alternative solutions did you explore? (Optional)

• NA

[dukenv0307]

Update proposal`s solution

[jjcoffee]

Reviewing tomorrow!

[jjcoffee]

Happy to go with @bernhardoj's proposal here. It seems pretty clear that this is just a missed case from #29248 (see here), where they weren't expecting to have to handle a null route.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[dukenv0307]

@jjcoffee Do you have any feedback about my proposal?

[melvin-bot]

📣 @jjcoffee 🎉 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]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[bernhardoj]

PR is ready

cc: @jjcoffee

[melvin-bot]

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

[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.4.37-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix can't go back after opening a public room as anonymous user and reopen the app #35453

If no regressions arise, payment will be issued on 2024-02-14. 🎊

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

• @jjcoffee requires payment automatic offer (Reviewer)
• @bernhardoj requires payment automatic offer (Contributor)

[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:

• [@jjcoffee] The PR that introduced the bug has been identified. Link to the PR:
• [@jjcoffee] 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:
• [@jjcoffee] 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:
• [@jjcoffee] Determine if we should create a regression test for this bug.
• [@jjcoffee] 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.
• [@lschurr] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[lschurr]

Hi @jjcoffee - Could you work through the checklist for this one?

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: Fix/28925: Anonymous user can edit profile #29248
• 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: ce65315#r138568449
• 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: N/A - just a missed test case
• Determine if we should create a regression test for this bug. Yes
• 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.

Regression Test Proposal

1. Log out
2. Open a public room as anonymous user (deep link)
3. Close and reopen the app or on web go directly to staging.new.expensify.com
4. Verify the LHN is shown with the public room visible
5. Verify the sign in button works

Do we agree 👍 or 👎

[lschurr]

Payment summary:

• Contributor: @bernhardoj $500 (paid in Upwork)
• Contributor+: @jjcoffee $500 (paid in Upwork)

[lschurr]

Submitted regression test request. Closing this one out :)