[$500] Chat - App allows to open any invalid email profile details using mention URL

[izarutskaya]

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. Open the app
2. Open any report
3. Write any user mention and send
4. Click on mention and copy the URL
5. Paste URL in compose box and remove or replace parts after 'login=' to make the email invalid eg: https://staging.new.expensify.com/details?login=abcd
6. Send the URL and click on it
7. Observe that instead of displaying 'Hmm its not here', app creates profile for invalid contact user

Expected Result:

App should display 'Hmm its not here' page when we try to open invalid contact profile details using mention URL

Actual Result:

App allows to open any invalid contact profile details on using mention URL structure

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: v1.3.74-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

app.allows.invalid.contact.profile.using.mention.URL.windows.chrome.mp4

Recording.1636.mp4

allows.invalid.email.contact.profile.details.IOS.1.mov

allows.invalid.email.contact.profile.details.IOS.mov

app.allows.invalid.contact.profile.using.mention.URL.android.chrome.mp4

Expensify/Expensify Issue URL:

Issue reported by: @dhanashree-sawant

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695485451642119

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016acedce6ab891123
• Upwork Job ID: 1706398446874087424
• Last Price Increase: 2023-09-25

[melvin-bot]

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

[melvin-bot]

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

[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 @JmillsExpensify (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 - @abdulrahuman5196 (External)

[izarutskaya]

Proposal from @dhanashree-sawant

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

We are solving the problem of app allowing to open profile details of invalid contact using mention result

What is the root cause of that problem?

On DetailsPage we don't have any check to determine whether provided contact is valid or not

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

Add new boolean which will check whether contact is valid contact or not and pass it in shouldShow parameter of FullPageNoFoundView to display 'Hmm its not here' page for invalid contact on DetailsPage
We can do that by adding new boolean here

const isValidContact = Str.isSMSLogin(details.login) || Str.isValidEmail(details.login) ? true : false;

Assign that boolean to FullPageNoFoundView shouldShow parameter here

<FullPageNotFoundView shouldShow={_.isEmpty(login) || !isValidContact}>

What alternative solutions did you explore? (Optional)

[c3024]

Proposal

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

Invalid logins also are displayed in details page when the url is edited

What is the root cause of that problem?

We are not checking if the login parameter from url is a valid email or phone number in DetailsPage at

App/src/pages/DetailsPage.js

Line 130 in 4ea4a55

<FullPageNotFoundView shouldShow={_.isEmpty(login)}>

to show FullPageNotFoundView.

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

We should get the login param from the url and check if it is a valid phone or email by adding

const parsedPhoneNumber = parsePhoneNumber(login);

and modifying this condition to the FullPageNotFoundView in the DetailsPage

<FullPageNotFoundView shouldShow={_.isEmpty(login) || (!parsedPhoneNumber?.possible && !Str.isValidEmail(login))}>

We are using parsePhoneNumber and Str.isValidEmail separately to validate the email or phone in several pages. We might consider making a function to check both.

What alternative solutions did you explore? (Optional)

[dukenv0307]

DetailPage will be deprecated in the feature so I think we should close this now.

[situchan]

DetailPage will be deprecated in the feature so I think we should close this now.

@dukenv0307 can you share link to that discussion?

[dukenv0307]

@situchan https://expensify.slack.com/archives/C01GTK53T8Q/p1689252492397689?thread_ts=1689247696.251109&cid=C01GTK53T8Q

[dhanashree-sawant]

Didn't we decide to not show the page for optimistic users only?

[situchan]

We're close to accountID based mention implementation, which means Details page will be deprecated soon.
So I think we can just do nothing and close this issue for now.

[johncschuster]

Thanks, @situchan! In that case, I'll go ahead and close the issue. If anyone disagrees, feel free to argue your case!