Mark Google API requests from PHP client as user-specific

[felixarntz]

Almost all Google APIs generally support a request query parameter called quotaUser, which allows passing a string that uniquely identifies a user of the application. This parameter is recommended to be used when issuing Google API requests from a server backend, as otherwise it may be impossible for the API to distinguish where the traffic is coming from and which rate limits it applies to.

The Site Kit authentication service makes use of this parameter because otherwise all requests would be associated with the same user which obviously would result in rate limits being exceeded at that scale. We didn't implement the same behavior in the plugin originally, however there could still be a case where it helps, e.g. if a site itself has too many users.

---

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

Acceptance criteria

• The plugin should (in PHP) pass the quotaUser query parameter or the X-Goog-Quota-User HTTP header to every Google API request with a string that uniquely identifies this user (e.g. some combination of site URL and WordPress user ID). The string doesn't need to be immutable (e.g. if the site URL changes, that's okay), it's just important that it doesn't conflict with another Site Kit user's identifier.
• This should be handled centrally, not individually in every API request (potentially via a request middleware or some extended class).

Implementation Brief

• Write a function to get the quotaUser query string param, by editing the following file /includes/Core/Authentication/Clients/OAuth_Client.php and adding the function: private function get_quota_user()
• The function will return a unique identifier for a user on a given site:
  return get_home_url() . '-' . get_current_user_id();
• Next we need to hook into every http request and add a header to our request. As we're using Guzzle 5 library, we can't make use of middlewares, so instead we use the event emitter to hook into requests - event emitter documentation https://docs.guzzlephp.org/en/5.3/http-messages.html?highlight=emitter#event-emitter
• Adding request headers
• We do that by adding to the code here:

  site-kit-wp/includes/Core/Authentication/Clients/OAuth_Client.php

  Line 237 in 8c341d3

  $http_client->setDefaultOption( 'headers/User-Agent', Google_Proxy::get_application_name() );

$http_client->getEmitter()->on( 'before', function ( BeforeEvent $before ) {
        $before->getRequest()->addHeader('X-Goog-Quota-User, $this->get_quota_user() );
} );

Google Header Reference

For future reference Guzzle 7 middlewares https://docs.guzzlephp.org/en/stable/handlers-and-middleware.html which will come in handy if we ever update to guzzle 6/7. Keep in mind that Guzzle 7 requires php 7.2.5 ( https://docs.guzzlephp.org/en/stable/overview.html#requirements ) whereas guzzle 6 only requires php 5.5 ( https://github.com/guzzle/guzzle/tree/6.5 ), also here's how to upgrade to v6 guzzle to middlewares: https://github.com/guzzle/guzzle/blob/master/UPGRADING.md#migrating-to-middleware

Test Coverage

• In /tests/phpunit/integration/Core/Authentication/Clients/OAuth_ClientTest.php
  Add a test_get_quota_user()

Visual Regression Changes

N/A

QA Brief

• Ensure our tests run.
• You can see what request url is being hit by making a temporary change to our third party dependencies:
• third-party/guzzlehttp/guzzle/src/Client.php in public function send() just before return $trans->response; add this:
• error_log( var_export( $request->getHeaders(), true ) ); enable your debugging, and open a page that makes requests to google apis /wp-admin/admin.php?page=googlesitekit-dashboard check your debug.log and ensure that the requested urls all contain the quotaUser= param.

Changelog entry

• Enhance Google API client with user-specific quota token to differentiate quota usage between users.

[tofumatt]

Looks like we always want to set a query param, even for POST requests. I wasn't sure about this because some APIs like it and others don't (they'll ignore query params when using post data). But from some Google API docs it seems it's fine to use a query param all the time.

Just wanting to make sure as the ACs don't explicitly mention it, but the param should ALWAYS be a query param/"GET" param.

That said, it looks like an HTTP Header (X-Goog-Quota-User, see: https://cloud.google.com/apis/docs/system-parameters) is also supported. Not a big deal, but using the HTTP header instead of a query param would allow us to better debug the query params of requests (using proxy apps like Charles) by not polluting the query params' data with this debugging/logging field.

@felixarntz This would slightly change the ACs, but how would you feel about using the HTTP header instead?

Otherwise IB looks good, though explicitly mentioning how the param should be used or linking to the above docs would be good because without it there's not much context to how this param works. I guess this comment can serve as that 🙂

[felixarntz]

@ivankruchkoff @tofumatt Using the HTTP header lgtm, although I don't have a strong opinion on either. I've updated the ACs to mention both approaches, i.e. being more agnostic about this. The IB here is ✅ from my end, but I'll defer to you whether to go with the HTTP header approach (if so, please make the changes accordingly).

[tofumatt]

@ivankruchkoff Is it anymore of a pain to send this data via an HTTP header instead of a query param? If it's more work to do so with Guzzle (I don't know the API well) then we don't have to bother as the IB looks fine, but if it's possible to use the HTTP Header instead of the query param I think it'd be nicer. I guess to me, the query params are more related to the specific request endpoint, but the headers are usually more "broad" requests metadata (eg access tokens, user info, etc.)

If it's straightforward to use the HTTP Header instead can you update the IB? But if it's not straightforward just assign this back to me and let me know—either way after that this is good-to-go 🙂

[ivankruchkoff]

@tofumatt updated to use a header. I was initially thinking to add both, query param and header, but maybe that gets handled differently, so I added an html comment in the IB for the query string approach.

[tofumatt]

Awesome, thanks for checking into that, IB looks good 🙂

IB ✅

[eugene-manuilov]

@aaemnnosttv @felixarntz do either of you mind reviewing my PR #3737 for this ticket? I found a way to add that header more easily without creating custom hooks and intercepting all requests. Hope it looks good to you.

[asvinb]

From debug.log:

23-Jul-2021 09:12:29 UTC] array (
  'Host' => 
  array (
    0 => 'analyticsreporting.googleapis.com',
  ),
  'content-type' => 
  array (
    0 => 'application/json',
  ),
  'X-Php-Expected-Class' => 
  array (
    0 => 'Google\\Site_Kit_Dependencies\\Google\\Service\\AnalyticsReporting\\GetReportsResponse',
  ),
  'X-Goog-Quota-User' => 
  array (
    0 => 'http://1@sitekit.10uplabs.com',
  ),

X-Goog-Quota-User header found.

QA: ✅