Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Site ID and site domain is not consistent #2784

Open
mikkamp opened this issue Jan 29, 2025 · 1 comment
Open

Site ID and site domain is not consistent #2784

mikkamp opened this issue Jan 29, 2025 · 1 comment
Assignees
Labels
type: bug The issue is a confirmed bug.

Comments

@mikkamp
Copy link
Contributor

mikkamp commented Jan 29, 2025

Describe the bug:

For the API Pull functionality we handle both the initial granting of the connection, as well as notifications being sent. However the site domain does not remain consistent throughout the whole process.

This is in particular troublesome for sites that do or do not support the www. as prefix for their domain.

When we initially setup the connection we grab the Site URL registered in the local WP database and strip off the https?:// prefix. This happens here: https://github.com/woocommerce/google-listings-and-ads/blob/2.9.7/src/API/Google/Middleware.php#L485

This could potentially be www.domain.com, and is the first entry that is passed to the Google servers, to fetch a redirect URL to use in the auth process. Later on in the process the Google service receives an auth token with the details:

{
    "access_token": "API_TOKEN",
    "blog_id": "blog ID",
    "blog_url": "blog url",
    ...
}

However at that point the blog_url might differ as it could potentially be registered in WPCOM as domain.com. Google relies on the Merchant Center ID to complete the process so doesn't overwrite the original domain we passed when fetching the redirect URL.

Once the connection has been granted we will start sending notifications. A notification is initially sent to WPCOM using the blog_id: https://github.com/woocommerce/google-listings-and-ads/blob/2.9.7/src/API/WP/NotificationsService.php#L81

WPCOM then handles the authentication for the notification, but in the process swaps the blog_id for the matching blog_url, which again in this case could potentially be domain.com. Which can in some cases give a mismatch with the original domain that was sent when requesting the redirect URL.

We've also seen cases where blog_url is registered with something like domain.wordpress.com but the site URL is https://domain.com, that's another discrepancy that would be resolved if we use the blog_url instead of the site_url.

What to resolve

The intention here is to remain consistent with the blog_url throughout the whole process. During the initial step where we fetch the redirect URL we are already connected to WPCOM. So instead of passing the site URL we should find a way to surface the blog_url to the local site. We might need to look through the connection package if there is any tooling available to request this value, either through locally saved data or a request to WPCOM.

Note that resolving this would fix it for anyone setting up a new connection, but wouldn't correct any stored domain names for sites that have already connected.

@mikkamp mikkamp added the type: bug The issue is a confirmed bug. label Jan 29, 2025
@puntope puntope self-assigned this Jan 30, 2025
@mikkamp
Copy link
Contributor Author

mikkamp commented Feb 5, 2025

Update

After discussion, we are now looking at the following approach:

  1. For the initial request to get a redirect URL to: google/google-sdi/v1/credentials/partners/<partner>/merchants/<site_url>/oauth/redirect:generate?merchant_id=<merchant_id> we will include the blog_id &blog_id=<blog_id>. This must then be stored along with the site name so it can be used in requests to the WP.com API. Optionally the blog id can also be stored/confirmed later when the token is received.
  2. For notifications that are sent to shoppingdataintegration.googleapis.com/v1/webhooks/partners/woocommerce/merchants/{merchant}/types/{topic}:webhookHandler we will use the site_url as was used in step 1, to match up the right notification to the site. We will also include the blog_id within the notification body, which can again be used to send requests to the WP.com API and stored along with the site details for previously connected sites that don't have the blog_id stored yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug The issue is a confirmed bug.
Projects
None yet
Development

No branches or pull requests

2 participants