Open sites in the internal browser

[oguzkocer]

Fixes #9625. This PR changes how View Site in the MySiteFragment works by opening the internal browser instead of the external one. However, there are quite a few considerations we should take into account before we make this change. @diegoreymendez @rachelmcr Could you please help with these and/or ping people who should be involved in the decisions for it?

Authentication

• WordPress.com sites without a custom domain works with authentication, so all is good with this one: https://cloudup.com/c3cQIH6v7fi
• WordPress.com sites with custom domain does not work with authentication. This has been a longstanding problem and for showing blog posts with custom domain, we use a hack to replace the custom domain with the unmapped url. (wordpress.com subdomain url) However, even this hack doesn't seem to work for showing the home page of the site, so here is how that looks: https://cloudup.com/cz3m-9tIWmz. Unless I am missing something we'll need to fix this in the backend.
• Jetpack sites does not work with authentication. If we try to authenticate the site with WordPress.com credentials, it wouldn't work and would try to re-direct to WordPress.com. We can't try to authenticate the site with username/password, because we don't have the password, so these would show up unauthenticated like so: https://cloudup.com/c6GgQgPcjlg
• Self-hosted sites would work with authentication since we can use the username/password for it. However, we do disable all the links in this case (more on this later), here is how this one looks: https://cloudup.com/cRwRv60j6PB

External Links

There is a little bit of an inconsistency for whether we allow links in the internal browser. I believe, we disabled them because the users would open their site from the app, end up somewhere that's not part of the app and get confused by it. We should make a decision for which site types should have their links disabled if any and document that in this PR.

Next Steps

For the site types listed below, follow these steps:

1. Decide whether we support authentication and make the necessary changes
2. Decide whether we support links in the internal browser and make the necessary changes
3. Test them
4. Decide whether we should add release notes for this PR.

• WordPress.com site without a subdomain (ex: oguzkocertest.wordpress.com)
• WordPress.com site with a subdomain (ex: example.com)
• Jetpack site
• Atomic site
• Self-hosted site

Update release notes:

• If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

[oguzkocer]

It's unlikely that this PR will make it to the code freeze, but we might still want to keep the current milestone in case we want to target the release branch. Please don't let this PR block the code freeze and remove the milestone entirely if it's a problem. Thanks!

[diegoreymendez]

To clarify, and avoid making assumptions on my end: are the authentication issues specific to the decision of showing the site internally? Or do we have the same issues when loading URLs externally?

For instance: I'd expect we wouldn't be able to authenticate a Jetpack site externally, if we can't do it internally.

Let me know.

[oguzkocer]

To clarify, and avoid making assumptions on my end: are the authentication issues specific to the decision of showing the site internally? Or do we have the same issues when loading URLs externally?

When we open the url in the external browser, it looks like we don't do anything specific with it. So, we are not authenticating at all even when we can. Having said that, I don't think that's too unexpected for the user, if they are already logged in on the external browser, they'll still be logged in and if they are not, they probably wouldn't expect to get logged in. (and logging them in on the external browser might have security implications anyway) However, when it's an internal browser, I feel like it's viewed as part of the app and I wouldn't be surprised if the users are frustrated when features don't work how they expect them to. Hope that clears it up!

[rachelmcr]

Authentication

However, when it's an internal browser, I feel like it's viewed as part of the app and I wouldn't be surprised if the users are frustrated when features don't work how they expect them to.

Agreed. One complaint when authentication doesn't work as expected is that the admin's views are counted in site stats, so it would be good to try to resolve the authentication issues. At least, we should authenticate WordPress.com Simple sites (including sites with a custom domain). We do this on the iOS side, so perhaps we can get inspiration from how it's done there.

I tested and see that iOS also doesn't authenticate Jetpack site views, so I think it's ok if we leave those sites as-is (without authentication), since there isn't a clear solution for that. At least then we'll have matching behavior on both platforms and can revisit it on both platforms at the same time if we want to change things there.

External links

There has been work done on the backend on WordPress.com to reduce the issues around navigating to places that aren't part of the app (i.e. removing links leading to Calypso) so it should be ok to have links enabled there.

I tested on self-hosted sites on iOS and links are enabled there, which means users can navigate to places that aren't part of the app (including wp-admin). We could enable links on self-hosted sites for parity between platforms, and to allow users to navigate around their site in the internal browser, or keep them disabled to avoid issues with that navigation. Either way has its pitfalls for now — I lean slightly toward keeping the behavior consistent across platforms but perhaps a designer can weigh in on that as well.

[oguzkocer]

Thanks for the input @rachelmcr!

Agreed. One complaint when authentication doesn't work as expected is that the admin's views are counted in site stats, so it would be good to try to resolve the authentication issues. At least, we should authenticate WordPress.com Simple sites (including sites with a custom domain). We do this on the iOS side, so perhaps we can get inspiration from how it's done there.

@diegoreymendez Do you mind checking how you are able to show the custom domains authenticated on the iOS side? Since you have recently worked on this code, I hope it'll be very easy for you to figure it out.

I tested on self-hosted sites on iOS and links are enabled there, which means users can navigate to places that aren't part of the app (including wp-admin). We could enable links on self-hosted sites for parity between platforms, and to allow users to navigate around their site in the internal browser, or keep them disabled to avoid issues with that navigation. Either way has its pitfalls for now — I lean slightly toward keeping the behavior consistent across platforms but perhaps a designer can weigh in on that as well.

Sounds good, I can enable the links in this PR and we can update both platforms later if a change is requested.

[diegoreymendez]

@diegoreymendez Do you mind checking how you are able to show the custom domains authenticated on the iOS side? Since you have recently worked on this code, I hope it'll be very easy for you to figure it out.

Yup! I'm wrapping two other issues today, but I'll focus on this one after that - probably tomorrow.

[shiki]

Do you mind checking how you are able to show the custom domains authenticated on the iOS side?

@oguzkocer I verified this with Charles Proxy. When the View Site button is tapped on a post from a site with a custom domain, the iOS app will first do a POST to wordpress.com/wp-login.php:

:method: POST
:scheme: https
:path: /wp-login.php
:authority: wordpress.com
content-type: application/x-www-form-urlencoded
accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
authorization: Bearer **REDACTED**
accept-language: en-us
accept-encoding: br, gzip, deflate
origin: null
referer: https://wordpress.com
content-length: 137
user-agent: Mozilla/5.0 (iPhone; CPU iPhone OS 12_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148 wp-iphone/12.3
cookie: recognized_logins=REDACTED; wordpress_logged_in=USERNAME_REDACTED; _wpndash=REDACTED; tk_ai=REDACTED

log=USERNAME_REDACTED&rememberme=true&redirect_to=https://wordpress.com/?wpios_redirect%3Dhttp://example-custom-domain.com/2019/04/26/post-title/

The first block is the header. The last block is the POST data.

And the response of /wp-login.php is a redirect to the previously given post URL

:status: 302
server: nginx
date: Fri, 26 Apr 2019 22:58:20 GMT
content-type: text/html; charset=UTF-8
location: https://wordpress.com/?wpios_redirect=http://example-custom-domain.com/2019/04/26/post-title/

All of these happen on the Web browser. I hope you can clearly see the redirection in this GIF. 😄

After that, I can confirm that the current WordPress app user is logged in and can comment on the post.

The WebView authentication process is all handled by WebViewAuthenticator.swift.

Please let me know if you have any question!

[nbradbury]

Just a friendly reminder to please make sure to manually merge develop into this branch before you merge this PR, as develop now contains the AndroidX migration. The automatic merge can succeed but still break the build. Better to find out now before it breaks develop. Thanks!

[diegoreymendez]

@malinajirka - I'm not sure who the best person could be to take this over, but I'm suspecting it may be you.

Can I ask you to assess if it's possible to use @shiki 's research to implement what's missing in Android and push this PR? (let's also be aware of what @nbradbury brought up - but you probably know that already since you've been working on it)

Let me know if there's anything.

[malinajirka]

I've looked into the authentication issue on WordPress.com sites with a custom domain. I'm still not sure why it's not working. Here are my findings.

1. We currently use a hack on Android: we use a hack to replace the custom domain with the unmapped URL.. We basically iterate through all the user's sites and we load getUnmappedUrl instead of the original custom domain URL.

2. From what I can tell iOS uses a bit different hack. They send a post request to login https://wordpress.com/?wpios_redirect=TARGET_URL. If the login is successful they get redirected to https://wordpress.com/?wpios_redirect=TARGET_URL. They intercept this redirection on the client and they load "TARGET_URL" instead.

I've tried to quickly implement iOS's approach but it still didn't work on Android. One of the main disadvantages was that cookies were not automatically copied on the redirect to "TARGET_URL". I even tried to copy them manually (I'm not sure if it's safe to do in production) but it didn't work anyway.

I was wondering why Android's approach worked with an external browser, but it doesn't work with the internal browser. Tbh I still don't know why it's not working, but I noticed one vital difference.
When we load the URL in the external browser the login cookie is wordpress_logged_in_[hash]=xyz, but when we load it in the internal browser, the login cookie is wordpress_logged_in=xyz. I've done a quick research and it seems wordpress_logged_in is used with wordpress.com domains and wordpress_logged_in_[hash]=xyz is used with a custom domain. I wanted to ask here first if you have any more info on how these parameters work and if my direction isn't completely wrong so I don't unnecessarily waste time. Thanks

[shiki]

Your analysis is correct, @malinajirka. On iOS, we intercept the redirect in WebViewAuthenticator.interceptRedirect and change the redirection URL. I initially thought that the redirect was from wordpress.com but I was wrong.

Indeed, it is odd that this wouldn't work on Android. I can look more into this tomorrow.

[malinajirka]

I've enabled third-party cookies in internal browser in 6951689. It fixed authentication on sites with custom domains and I believe it shouldn't have any side-effects.

I've also enabled clicking on links when the user clicks on "View Site" button in 1b0dd18 (I tried not to change the current behavior for post/page previews)

• ✅WordPress.com site without a custom domain (ex: oguzkocertest.wordpress.com)
• ✅WordPress.com site with a custom domain (ex: example.com)
• ✅ 🚫Jetpack site
  - tested on jurassic.ninja site with JetPack - works without authentication
• ✅ Atomic site
• ✅ Self-hosted site
  - tested on jurassic.ninja site with not-activated JetPack (added through self-hosted site
  login) works with authentication
  - ⚠️ tested on poopy.life site - loads WP-admin instead of the site (/wp-admin/admin.php?
  page=sandbox&tab=expiration). It does the same thing on iOS.

@shiki I think it's ready for another round. I haven't update the release-notes - please feel free to update them if you want. Thanks 🙇

[shiki]

@malinajirka, great work on this! 🎉 I tested quite a few scenarios and it's looking good. I can confirm that the Calypso UI is no longer visible.

Self-hosted Sites vs Jetpack Connected Sites

I'm not sure if this is something that we can fix. It looks like if a self-hosted site is added manually, the View Site will show the user as logged in. If the self-hosted site is connected via Jetpack, the user is not automatically logged in.

Added using username/pass	Connected using Jetpack

Cookies

As for the cookies that we discussed via Slack, it does not look like we are leaking WPCOM cookies to self-hosted sites.

Added using username/pass	Connected using Jetpack

I would still like to have a second reviewer for this because I'm clearly not an expert. 😅

[malinajirka]

Thanks for the review @shiki !

I'm not sure if this is something that we can fix. It looks like if a self-hosted site is added manually, the View Site will show the user as logged in. If the self-hosted site is connected via Jetpack, the user is not automatically logged in.

If the user adds the site manually, we can use its credentials and authenticate. However, if the self-hosted site is connected via Jetpack, we don't have the credentials and the WPCom authentication doesn't work. Behavior on both Android and iOS should be the same now - we can get back to it when we find out how to authenticate the user against Jetpack sites.

As for the cookies that we discussed via Slack, it does not look like we are leaking WPCOM cookies to self-hosted sites. I would still like to have a second reviewer for this because I'm clearly not an expert.

I also believe it's safe but better safe than sorry:-). Could you please take a look @maxme @daniloercoli. Thanks!

[loremattei]

Hey! I'm moving this to 12.8 since 12.7 has been cut. If this PR has to land 12.7, please feel free to move it back, target the release branch and ping me to build a new beta.

[daniloercoli]

As for the cookies that we discussed via Slack, it does not look like we are leaking WPCOM cookies to self-hosted sites. I would still like to have a second reviewer for this because I'm clearly not an expert.

I got it another review, and doesn't seem we're leaking the OAuth token around.

Although I noticed that we're still using the "open in external app" symbol in the View Site menu item. Maybe we need to remove it now that URLs are opened in-app.

[malinajirka]

Thank you @daniloercoli !

Although I noticed that we're still using the "open in external app" symbol in the View Site menu item. Maybe we need to remove it now that URLs are opened in-app.

Good observation! I removed it at first, but tbh I'm not sure now.

• It's still in the "external" section
• It's still an external website, even though we load it in the app.

I feel like we might want to propagate the information that it's not part of the app - it's a website. Than try to propagate the information that it's loaded in the internal/external browser. Imho most users don't even know/see the difference and I'm not sure why they should care. Wdyt?

cc @SylvesterWilmott You don't need to read the whole discussion, just the last 2 comments. Thanks

[SylvesterWilmott]

I feel like we might want to propagate the information that it's not part of the app - it's a website. Than try to propagate the information that it's loaded in the internal/external browser. Imho most users don't even know/see the difference and I'm not sure why they should care. Wdyt?

The long answer is, it depends. It depends on how our web views look. In Android our other web views look suspiciously like regular views and can be a source of confusion. If for this PR, our web view has the blue app bar, no obvious browser controls then I'd say yes, keep the icon for some clarity.

As a side note though, this work is actually also part of a planned project to improve our previews: paCL20-aw-p2

[malinajirka]

If for this PR, our web view has the blue app bar, no obvious browser controls then I'd say yes, keep the icon for some clarity.

Yes, it looks like part of the app. Imho a regular user won't even notice they are not in the app anymore - there is a loading indicator when the site is loading, but I don't think they'll notice it/realize what it means.

@daniloercoli Could you please approve and merge this PR. It seems we won't be removing the external icon. Thank you for noticing it so we could discuss it and make a decision! I totally missed it.

[daniloercoli]

LGTM!

[SylvesterWilmott]

Yes, it looks like part of the app. Imho a regular user won't even notice they are not in the app anymore - there is a loading indicator when the site is loading, but I don't think they'll notice it/realize what it means.

You're absolutely right. This is not a new problem and hopefully, we can address this in the project i referenced in my previous comment. This PR helps us to establish a baseline against which to compare.