Site-kit-wp: Support PageSpeed Insights transition to using OAuth

[felixarntz]

Addressed by PR #607

---

The PageSpeed Insights API is moving away from using API keys, which comes in quite handy for us actually. This issue is about making the necessary changes ahead of time so we can switch as soon as we need to. Depending on when exactly the new version of the API goes live, we may need to do that in a "hot fix" release.

Here's the necessary changes:

• Request openid scope (_not_ prefixed with www.googleapis.com...). This can be part of the OAuth_Client class itself since the scope is not only associated with PSI.
  
  
  
  • Because it's not prefixed, we should change the scope validation in the OAuth_Client class, so that everything prefixed passes (as it does already), but also these three specific scopes without prefix.
  
  
• Make PageSpeed_Insights module class use the regular OAuth client instead of a custom client using an API key.
• Remove API key step required to setup PSI module, and make it a "setup-less" module (except for the OAuth scope being required which should already be the case).
• Remove API key infrastructure, including PHP class, the field being exposed in settings, and all consuming code.

The API itself will not change, so it won't move from a v5 to v6 or anything.

---

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

Acceptance criteria

• Connection to PageSpeed Insights API should continue to work after it switches to requiring OAuth2.
  Note: This can only be tested after it goes live. Implementation should happen based on the above guidance, in advance (without merging though).

Implementation Brief

• Update OAuth_Client to include openid in the default required scopes
• Update PageSpeed_Insights module
  
  
  
  • Use OAuth_Client in all places instead of API_Key_Client
  
  
• Remove classes/methods and related tests
  
  
  
  • \Google\Site_Kit\Core\Authentication\API_Key
  
  
  • \Google\Site_Kit\Core\Authentication\Clients\API_Key_Client
  
  
  • \Google\Site_Kit\Core\Authentication\Authentication::get_api_key_client
  
  
  • core/site/data/apikey Rest route
  
  
  • <PageSpeedInsightsSetup> and references to it
    
    
    
    
    
    • googlesitekit.PageSpeedInsightsModuleSetupWizard
    
    
    
  
  
• Update auth scope handling to allow for a whitelist of scopes that do not match https://www.googleapis.com/auth/* - should cover profile, email, openid (only the latter is actually used by this issue)
  
  
  
  • Module_With_Scopes_ContractTests
  
  
  • \Google\Site_Kit\Core\Authentication\Clients\OAuth_Client::authorize_user
  
  

Changelog entry

• Migrate PageSpeed Insights module to rely on OAuth with openid scope instead of an API key, and remove overall API key support.

[felixarntz]

@tofumatt

• Update PageSpeed_Insights module
  
  
  
  • Use Module_With_Scopes trait
    
    
    
    
    
    • Requires only openid scope
    
    
    
  
  

Note that the scope should be handled in OAuth_Client, not the module, as mentioned above, because it is a generic scope. If it is part of the module, we unnecessarily require an extra OAuth redirect for the user.

[aaemnnosttv]

Note that the scope should be handled in OAuth_Client, not the module, as mentioned above, because it is a generic scope.

My mistake. IB updated!

[felixarntz]

IB ✅

Made one minor change: I replaced

currently only openid but could be more later perhaps

with

should cover profile, email, openid (only the latter is actually used by this issue)

[aaemnnosttv]

@felixarntz assigning this one to you for now and moving to blocked. It requires a bit more detail regarding how the new module activation flow will look and work. See #607

[felixarntz]

@aaemnnosttv I replied in https://github.com/google/site-kit-wp/pull/607#issuecomment-539026499

There shouldn't be any setup step, going to the dashboard is fine.

[felixarntz]

Thanks @lilybonney for changing the labels accordingly.

Let's see if we can merge this for the beta.1.0.8 milestone or not. I haven't heard any update regarding PSI transition, but @aaemnnosttv you reported it apparently _already_ works with just OAuth (and openid scope). Let's keep this a priority for this week, but we need to throughly test whether it works before merging. I'll also follow up on our end.

[aaemnnosttv]

@felixarntz giving this another few tests locally, I can confirm that it works as-is 👍

Now that PageSpeed Insights no longer requires any additional scopes of its own, should it be an always on module? It is a bit time-consuming to load the first time but that's just the nature of the statistics that it has to collect. I don't see a reason why it shouldn't be enabled as part of the core services. What do you think?

[felixarntz]

Thanks @aaemnnosttv! Let's move forward with this then. I'll also test it again today or tomorrow.

Regarding it being a default module, I wouldn't say so. We shouldn't change that just because it's easy to set up. There's no reason why e.g. PSI would be enabled by default when Analytics isn't.

A point for having it active by default would be to expose people to these performance stats because they matter, while probably many people don't know what PSI is. I suppose a service like Analytics will see a much higher activation count, so we might want to boost PSI's exposure. I will discuss that in a bit with @marrrmarrr.

[aaemnnosttv]

I just noticed a bug with this: when activating via _Settings > Connect More Services_ it still goes to the old reauth screen with an empty widget body, instead of redirecting to the dashboard with success message as it does correctly when clicking on the activation CTA.

Activating via _Settings > Connect More Services_

Activating via activation CTA on dashboard

I will work on a PR to fix this now.

[adamsilverstein]

I QAed this on the feature/auth-proxy branch. Overall QA passed and I am moving this to "Acceptance". I did notice one bug that deserves a follow up Issue: I am unable to deactivate PSI.

Testing Procedure

• Build a zip from the feature/auth-proxy branch and upload to a new https://jurassic.ninja/ site.
• activate the module from the CTA on the dashboard
• Note everything worked well, data began flowing
• Delete plugin & start over, this time activating from the Settings screen.

Note: I am unable to deactivate the PSI module - it is behaving the the (required) Search console module which cannot be deactivated. I should be able to deactivate PSI, however on the settings screen there is no "Edit" link that would normally enable deactivating:

I will open an issue to cover this.

[adamsilverstein]

I created this follow up issue to address the "unable to deactivate" issue noted in my QA above: https://github.com/google/site-kit-wp/issues/682