Site-kit-wp: redirect_uri_mismatch error with TranslatePress plugin

[felixarntz]

Bug Description

With the TranslatePress plugin, with the setting _Use a subdirectory for the default language_. Results in a ‘redirect_uri’ mismatch error. More about the setting:

https://translatepress.com/docs/settings/#default-launguage-subdirectory

Related support topic: https://wordpress.org/support/topic/unable-to-sign-in-redirect_uri-mismatch/

Originally reported in https://github.com/google/site-kit-wp/issues/1116#issuecomment-592195115.

---

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

• The plugin should always use an OAuth redirect URI of {admin_url}/index.php?oauth2callback=1 instead of {site_url}?oauth2callback=1.
• This URI should be passed to the authentication service in the setup via a redirect_uri query parameter.
• No more changes to the plugin are necessary since it already listens for that parameter in WP Admin too.
• The authentication service will need a few changes to match that behavior (especially in regards to backward-compatibility).

Implementation Brief

• Modify OAuth_Client::get_redirect_uri() to use admin_url( 'index.php' ) instead of untrailingslashit( home_url() ).
• Add redirect_uri query parameter to Google_Proxy::get_site_fields().

QA Brief

• This needs to be tested against the staging version of the authentication service.
• The adjustment on the service side will need to go live before this release goes out.

Changelog entry

• Fix bug where plugins modifying the site address during frontend requests would prevent the setup flow from being completed.

[felixarntz]

This _could_ be happening because we handle the ?oauth2callback=1 requests in the frontend, and plugins may alter the home URL differently there from how it is in WP Admin. If that's truly the case, we'll need to have our callback logic elsewhere within WP Admin and inform the authentication service about this new URL.

[adamsilverstein]

I was able to reproduce this issue with the following steps:

• Install and activate the TranslatePress plugin, enabling "Use a subdirectory for the default language" and adding a second language. My homepage now redirects to {site_url}/en
• Activate Site Kit and go thru auth flow.
• You will be returned to /wp-admin with an error:

This could be happening because we handle the ?oauth2callback=1 requests in the frontend, and plugins may alter the home URL differently there from how it is in WP Admin.

That is exactly what is happening. TranslatePress adds a filter on home_url:

https://github.com/phpzio/translatepress/blob/2931ec8e7e0100f954be45bf666e7a35c9400ce6/class-translate-press.php#L284

In the callback, the plugin alters the url for non-admin requests when the 'add-subdirectory-to-default-language' is set:

https://github.com/phpzio/translatepress/blob/2931ec8e7e0100f954be45bf666e7a35c9400ce6/includes/class-url-converter.php#L33-L50

Therefore you wind up with a mismatch between the calculated home url (and thus redirect_uri).

Redirecting to the wp-admin url instead of the home URL could be a good fix here. Is there any technical reason that would not work here, or that the front end URL was chosen instead?

Another possible solution is to store the results of home_url in settings early in the setup process (and reset if the settings are changed); then always use that value for our redirect URLs instead of calling home_url every time. This would ensure the service (proxy) and site urls and redirect uris stay in sync and prevent any filters on home_url from altering the urls we use.

[felixarntz]

@adamsilverstein Thanks for double-checking!

Redirecting to the wp-admin url instead of the home URL could be a good fix here. Is there any technical reason that would not work here, or that the front end URL was chosen instead?

I don't think there is. This was implemented way back, and there should be no difference pointing to a WP Admin URL instead, because the user must be logged in at this point anyway.

Another possible solution is to store the results of home_url in settings early in the setup process (and reset if the settings are changed); then always use that value for our redirect URLs instead of calling home_url every time. This would ensure the service (proxy) and site urls and redirect uris stay in sync and prevent any filters on home_url from altering the urls we use.

That's a reasonable alternative, however I'd say that would be an additional patch. We should add a way to the authentication service to check for the URLs currently registered there, so that we can know as soon as they get out of date. Then there'd be no need to store that in the plugin. I think that would be a separate enhancement though.

[adamsilverstein]

Makes sense. I'll change that detail in the analytics provisioning flow as well.

[felixarntz]

@adamsilverstein Added an IB for your review.

[adamsilverstein]

✅ IB looks good.

[adamsilverstein]

I created a follow up issue to update this in the analytics provisioning flow callback as well: https://github.com/google/site-kit-wp/issues/1362

[tofumatt]

Just need @aaemnnosttv to merge; this is all set from a code review standpoint 👍

[felixarntz]

This should now work against staging - not yet against production.

[cole10up]

Tested

Downloaded and installed TranslatePress as well as the staging proxy plugin.

Activated both and set Use a subdirectory for the default language to 'yes'

Reset SK, and started setup flow.

Confirmed error isn't present.

Passed QA ✅