[$250] Bug: Old dot link without ‘www.’ prefix ( expensify.com/inbox) does not auto signin to old dot reported by @sobitneupane

[kavimuru]

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. Login to new dot expensify.
2. Send old dot link without ‘www.’ prefix (example: expensify.com/inbox)
3. Press the link.

Expected Result:

You are automatically signed in to old dot.

Actual Result:

You are prompted to enter password.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.2.14 Reproducible in staging?: y Reproducible in production?: y Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/195652418-0de43661-52fa-4eab-9290-7553df8d2252.mov

Expensify/Expensify Issue URL: Issue reported by: @sobitneupane Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1665568331616469

View all open jobs on GitHub

https://user-images.githubusercontent.com/43996225/195652444-18c7006d-2fae-4355-8b01-6c5ab9894113.mp4

[melvin-bot[bot]]

Triggered auto assignment to @conorpendergrast (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[melvin-bot[bot]]

@conorpendergrast Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

Triggered auto assignment to @sakluger (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[conorpendergrast]

Didn't get to this before I went OoO, reassigning!

[sakluger]

Reproduced in prod in an incognito window.

[melvin-bot[bot]]

Triggered auto assignment to @MonilBhavsar (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[melvin-bot[bot]]

@MonilBhavsar Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot[bot]]

@MonilBhavsar 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[MonilBhavsar]

Can be worked externally

[tienifr]

Proposal

Problem

In https://github.com/Expensify/App/blob/d6baba0be2da93beee72d8f21a6ee83e297fdd1a/src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.js#L28-L48 we check attrHref.startsWith(CONFIG.EXPENSIFY.EXPENSIFY_URL) to detect it's expensify url or not, but EXPENSIFY_URL = https://www.expensify.com/

Solution

In https://github.com/Expensify/App/blob/d6baba0be2da93beee72d8f21a6ee83e297fdd1a/src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.js We'll remove www. in attrHref and expensifyUrl before we check attrHref.startsWith(CONFIG.EXPENSIFY.EXPENSIFY_URL).

+    const attrHrefWithoutW3 = attrHref.startsWith('www.')? attrHref.replace('www.',''):attrHref;
+    const expensifyUrlWithoutW3 = CONFIG.EXPENSIFY.EXPENSIFY_URL.replace('www.','');
+    const conciergeUrlWithoutW3 = CONFIG.EXPENSIFY.CONCIERGE_URL.replace('www.','');

    const internalNewExpensifyPath = (attrHref.startsWith(CONST.NEW_EXPENSIFY_URL) && attrHref.replace(CONST.NEW_EXPENSIFY_URL, ''))
        || (attrHref.startsWith(CONST.STAGING_NEW_EXPENSIFY_URL) && attrHref.replace(CONST.STAGING_NEW_EXPENSIFY_URL, ''));
-    const internalExpensifyPath = attrHref.startsWith(CONFIG.EXPENSIFY.EXPENSIFY_URL)
-                                    && !attrHref.startsWith(CONFIG.EXPENSIFY.CONCIERGE_URL)
-                                    && attrHref.replace(CONFIG.EXPENSIFY.EXPENSIFY_URL, '');
+    const internalExpensifyPath = attrHrefWithoutW3.startsWith(expensifyUrlWithoutW3)
+                                    && !attrHrefWithoutW3.startsWith(conciergeUrlWithoutW3)
+                                   && attrHrefWithoutW3.replace(expensifyUrlWithoutW3, '');

https://user-images.githubusercontent.com/113963320/198528098-8234dfc1-f9b4-4c9a-a3e9-fd64741e0295.mp4

[parasharrajat]

Thanks for the proposal @tienifr Good catch. Let's improve your solution. It would be great to create a common function that matches any two domains. I think simple rules will be.

1. Create a function that takes two params A and B with type url.
2. Exclude the parts after domain segment from URLs.
3. Return true when domains A and B are exact matches.
4. Return true when domains A and B and their subdomains are a match. (if no subdomain is passed use www or if www subdomain is passed, ignore it) 3 Ignore protocol part https or http.
5. Return false for all other cases.

[tienifr]

Update Proposal

@parasharrajat I updated my solution in here: https://github.com/tienifr/App/pull/18. Pls help to check, thanks.

[parasharrajat]

Ok, Thanks. Is URL supported on all platforms? Native, web, desktop.

[tienifr]

@conorpendergrast Thanks for your review. I realized that new URL not work on native, so I changed to use url libs. Pls help check: https://github.com/tienifr/App/pull/18

[parasharrajat]

@MonilBhavsar Are we fine with package url? https://github.com/Expensify/App/issues/11810#issuecomment-1298209880

[MonilBhavsar]

Hmm, This looks good. But I'm not sure about the process of using new package. Let's post this in channel and get thoughts from the team?

[melvin-bot[bot]]

@CortneyOfstad, @parasharrajat, @MonilBhavsar Whoops! This issue is 2 days overdue. Let's get this updated quick!

[MonilBhavsar]

@parasharrajat let's discuss this in slack about adding and using a new package. Do you mind opening a conversation with the context you have?

[parasharrajat]

Asked in https://expensify.slack.com/archives/C01GTK53T8Q/p1667833896505429. Please chime in.

[parasharrajat]

@tienifr Is it possible to use regExp or something to get rid of url package from your solution?

The discussion is leading towards this.

[tienifr]

@parasharrajat I've updated my PR to use regExp: https://github.com/tienifr/App/pull/18. Pls help check again. Thanks

Here is the sample result of getLocation function:

[parasharrajat]

const reURLInformation = new RegExp([
    '^(https?:)//', // protocol
    '(([^:/?#]*)(?::([0-9]+))?)', // host (hostname and port)
    '(/{0,1}[^?#]*)', // pathname
    '(\\?[^#]*|)', // search
    '(#.*|)$', // hash
].join(''));

I am not sure how good these are you will have to improve them during tests. try with different possible URLs. e.g https://github.com/Expensify/App/issues/7853

Also, the purpose of creating this function is to replace all usages of manual URL matching. Both internalNewExpensifyPath and internalExpensifyPath should be replaced with this.

Also, a better name for the function would be getURLObject. We can drop the unnecessary parts e.g. hash search as they are not needed for this.

Rest, @tienifr 's proposal looks good to me.

cc: @MonilBhavsar

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

[tienifr]

@parasharrajat Thanks for your review. I've updated my sample PR: https://github.com/tienifr/App/pull/18