[HOLD for payment 2023-10-16] [$500] Web - Adding extra number to Share Code, it throws error ' Auth GetEmailByAccountID returned an error"

[kbecciv]

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

---

Action Performed:

1. Go to settings -> shareCode -> Copy URL
2. Now paste the URL and add one or two extra numbers in it
   e.g : shareCode profile URL + '1' or shareCode profile URL + '5'
3. Go to message the hidden user
4. Notice that it throws error ' Auth GetEmailByAccountID returned an error"

Expected Result:

Since the proflies are enumerated cronologically, for profile URLs that doesn't exits. ' Hmm,, not there ' or something equivalent must be shown.

Actual Result:

Adding extra number to Share Code, it throws error ' Auth GetEmailByAccountID returned an error"

Workaround:

Unknown

Platforms:

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

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

Version Number: 1.3.69.0
Reproducible in staging?: y
Reproducible in production?: y
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

Screen.Recording.2023-09-11.at.3.17.07.PM.mov

Recording.4448.mp4

Expensify/Expensify Issue URL:
Issue reported by: @ashimsharma10
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694426884818349

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0143309d373d417e8b
• Upwork Job ID: 1702015263918743552
• Last Price Increase: 2023-09-13

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[graylewis]

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

Attempting to access a non-existent user's profile by using the share link throws an error without redirecting to a 404 page.

What is the root cause of that problem?

The code processing the API's response does not have a handler for redirecting to a 404 page.

App/src/pages/ProfilePage.js

Line 110 in 56271f0

The profile data for a given user is requested in a useEffect call on line 103

App/src/pages/ProfilePage.js

Line 103 in 56271f0

useEffect(() => {

This is loaded into state in the openPublicProfilePage method in actions/PersonalDetails.js https://github.com/Expensify/App/blob/56271f0b1a60a2ea17b418b458648bbdf4fde483/src/libs/actions/PersonalDetails.js

and subsequently is available in the props of ProfilePage as props.personalDetails.

There is no check to see if the state update from the action has returned a failed result or a successful result.

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

Since visiting a non-existent user's profile should logically be treated as a 404 error, my solution is to check the value of props.personalDetails, and then use Navigation.navigate(SCREENS.NOT_FOUND) to redirect the user to the 404 page if the result is not a valid PersonalDetails object.

Similar to how the isTaskReport function checks if props.report is a valid report and then exits the modal if the report is not valid in TaskDescriptionPage, I'd redirect to SCREENS.NOT_FOUND (or just exit the page like in the TaskDescriptionPage if that seems more appropriate) if props.personalDetails was not a valid PersonalDetails object. (https://github.com/Expensify/App/blob/56271f0b1a60a2ea17b418b458648bbdf4fde483/src/pages/tasks/TaskDescriptionPage.js)
if (!ReportUtils.isTaskReport(props.report)) { Navigation.isNavigationReady().then(() => { Navigation.dismissModal(props.report.reportID); }); }

What alternative solutions did you explore? (Optional)
Implementing a more strict validation function for Profile IDs

[melvin-bot]

📣 @graylewis! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[graylewis]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01a2b8ad673e3ea542

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[iamuddeshya]

Proposal

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

Opening a non-existing user profile directly through the URL throws error ' Auth GetEmailByAccountID returned an error"

What is the root cause of that problem?

The higher order function applyHTTPSOnyxUpdates returns onyxDataUpdatePromise. This onyxDataUpdatePromise is only checking for a response of 200 if success, otherwise all other response codes are handled as error.

App/src/libs/actions/OnyxUpdates.js

Lines 35 to 41 in c9580da

	if (response.jsonCode === 200 && request.successData) {
	return updateHandler(request.successData);
	}
	if (response.jsonCode !== 200 && request.failureData) {
	return updateHandler(request.failureData);
	}
	return Promise.resolve();

Since as per the current issue, the profile does not exist, hence the error code that is returned is 404 ( See Attached )

Hence, the error returns 404 which is not handled. The message also says "Email not found."

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

We can add another if block in the onyxDataUpdatePromise that checks for the response code 404 and takes user to either a 404 page or shows a better error within the page user currently encountered the error in. [ Screenshot Attached ]

App/src/libs/actions/OnyxUpdates.js

Lines 32 to 42 in c9580da

	return onyxDataUpdatePromise
	.then(() => {
	// Handle the request's success/failure data (client-side data)
	if (response.jsonCode === 200 && request.successData) {
	return updateHandler(request.successData);
	}
	if (response.jsonCode !== 200 && request.failureData) {
	return updateHandler(request.failureData);
	}
	return Promise.resolve();
	})

Attached is the video POC

expensify-bug-fix-1.mov

What alternative solutions did you explore? (Optional)

None

[dukenv0307]

I think this is a BE bug, we should return the avatar as null if the accountID doesn't exist.

[melvin-bot]

Current assignee @burczu is eligible for the Internal assigner, not assigning anyone new.

[iamuddeshya]

I think this is a BE bug, we should return the avatar as null if the accountID doesn't exist.

No, because backend is returning 404 which is fine.

[melvin-bot]

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

[melvin-bot]

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

[maddylewis]

@burczu - will you help me confirm whether or not this recent bug report will be fixed with whichever solution we move forward with for #27381?

[burczu]

@maddylewis I don't think so - in this issue we change the ID that is in the URL address that causes error on the backend side - it can't find this ID in database. In the bug report you've mentioned we add double slash to the URL (URLs with double slashes are in general still correct, that's why the App works correctly with it), and perhaps, we need to add some mechanism to remove it while navigating to other pages.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

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

• Initial fix #28812

If no regressions arise, payment will be issued on 2023-10-16. 🎊

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

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

• @burczu does not require payment (Contractor)
• @graylewis requires payment
• @ashimsharma10 requires payment

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:

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

[madmax330]

I think a better solution for this would to not throw in the back-end when someone tries to access the profile page of an accountID that doesn't exist.

@jasperhuangg and what would the front-end show?

[madmax330]

@graylewis can you look into the deploy blocker and see what was missed?

[graylewis]

@madmax330 yeah definitely, I'll take a look now

[graylewis]

@madmax330 Alright, after looking into it it seems that public profiles are treated the same as a 404 on the backend. When I try to open my account using my share code from a brand new account and message myself, I get the "Auth GetEmailByAccountId returned an error" message from the original report.

As I mention in my PR, the backend doesn't return a 404 for non-existent profiles. This is the primary issue, and my solution is a workaround to throw a 404 appropriately in the meantime. The patch I implemented should work given that a user's display name should be defined for existent profiles and undefined for non-existent profiles (when I looked into it, this seemed to be true). Unfortunately, it seems that the backend isn't returning any info for existent profiles that I don't already have in my contacts.

This is the data returned from the backend for my share link: (note that displayName is undefined)

"value": {
        "1566121632": {
            "accountID": 1566121632,
            "avatar": "https:\/\/d2k5nsl2zxldvw.cloudfront.net\/images\/avatars\/default-avatar_1.png",
            "firstName": "",
            "lastName": "",
            "status": null
        }
}

This is identical to the data received when using a non-existent share code. I'm not sure if this is intended behavior (it feels wrong to me), but either way the fact that "Auth GetEmailByAccountId returned an error" is thrown even for valid share codes seems to indicate a deeper issue.

Reproduction steps:

1. Copy share code from account A
2. Open share code while logged into account B that has never talked to account A
3. Click "message hidden"

EDIT:
The above error is only occurring on production. On staging the same data is returned but no "Auth GetEmailByAccountID returned an error" is thrown when opening the chat. When the chat is opened the user is returned like normal. In my opinion, if the displayName is publicly available information, it should be returned when calling openPublicProfilePage on the API. For some reason the above empty data object is returned instead.
As a side note, on production the avatar returned is default avatar 1, while on staging the correct avatar for the account is returned. I'm not sure why.

[madmax330]

while on staging the correct avatar for the account is returned. I'm not sure why.

Because staging and production run two separate backend versions. So maybe the issue is fixed now?

[maddylewis]

based on the status of this issue, does it make sense for @burczu to go through this checklist? #27381 (comment)

[melvin-bot]

@madmax330, @burczu, @graylewis, @maddylewis Whoops! This issue is 2 days overdue. Let's get this updated quick!

[madmax330]

based on the status of this issue, does it make sense for @burczu to go through this checklist?

No since the changes were reverted I don't think it makes sense

[maddylewis]

Leaving a note here for myself on where we are with this one:

• @ashimsharma10 was paid reporting bonus via Upwork
• @graylewis was paid contributor fee via Upwork

to clarify, are we waiting for @graylewis to share a new proposal for this same issue since the original changes were reverted?

[madmax330]

Since there's no straightforward solution for this. I'm wondering if we should just close this. While I get the issue, I think that if you tamper with the url and the app throws an error I think that's fine, we could handle it better, but not really worth the effort IMO. There's no reason you should be tampering with the url anyways.