Introduce infrastructure for obtaining Ad blocking recovery tags from the AdSense API and storing them in the database

[kuasha420]

Feature Description

Historically, Site Kit has always constructed various tags (ie. AdSense and Analytics tags) by interpolating various account and property related information on the hard coded tags available in the source code of the plugin. Due to the undocumented and unpredictable nature nature of the Ad blocking recovery and error protection tag, it has been decided to use the AdSense API to get the tags. See the relevant sections in the design doc.

This issue is about obtaining the tags from the AdSense API and storing them securely in the websites database.

The API was added to the google/apiclient-services in the v0.263.0 release back in August. We are currently using v0.277.* so no library update is needed here as well. 🎉

---

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

Acceptance criteria

• The AdSense Recovery and AdSense Error protection tags should be fetched via the AdSense API and stored in the AdSense settings using distinct option_name values: 'googlesitekit_adsense_recovery-tag-HTML' and 'googlesitekit_adsense_error-protection-HTML' respectively—these values can be unset by default.
  • After they're fetched, the tags should immediately be saved in the AdSense module settings.
  • These tags should be saved base64-encoded so their contents as HTML are not immediately apparent and cannot be changed by another, poorly-coded plugin that modifies HTML data in the database. The values should be returned decoded.
  • Two new methods should be added to the AdSense module: getRecoveryTagHTML and getErrorProtectionHTML that return the decoded contents of these tags, if available. An empty string or null can be returned if not found.
• An action in the datastore of AdSense module should be added to fetch the tags.
  • This action will be used to fetch the tags in the Ad Blocking Recovery screen in Complete Ad Blocking Recovery onboarding screen #6966.

Implementation Brief

• In Google\Site_Kit\Modules\AdSense class:
  • Add GET:ad-blocking-recovery-tag route to get_datapoint_definitions method.
  • In create_data_request method, add a case for the aforementioned route.
    • Make sure that the accountID is provided.
    • Return $service->accounts->getAdBlockingRecoveryTag( self::normalize_account_id( $data['accountID'] ) );
  • In parse_data_response method, add a case for the aforementioned route.
    • Get the adBlockingRecoveryTag and errorProtectionTag from the response using getTag and getErrorProtectionCode methods respectively.
    • Base64 encode both of them and store them in separate options defined in the AC.
    • Return with a simple { success: true } to indicate success as we do not need the tags in the Admin panel.
  • Create two public methods getAdBlockingRecoveryTag and getErrorProtectionTag:
    • Get the appropriate tag from the DB using the options defined in the AC.
    • If they exist/are not empty, base64 decode them.
    • If decoded successfully, return the decoded string.
    • Return null in all other case.
• In modules/adsense datastore:
  • Create an action fetchAdBlockingRecoveryTags using the previously created API and createFetchStore.
  • This action will be used to fetch Ad Blocking Recovery Tags in the first step of ad-blocking-recovery screen in Complete Ad Blocking Recovery onboarding screen #6966.

Test Coverage

• Add test coverage for the above changes and additions.

QA Brief

• Complete AdSense setup.
• Sync the ABD tags on the server using the following console command,
  • await googlesitekit.data.dispatch('modules/adsense').syncAdBlockingRecoveryTags()
• Ensure the response is { "success": true }.
• Check the googlesitekit_adsense_recovery-tag and googlesitekit_adsense_error-protection-tag options from the DATABASE. They should be populated with long base64 string.

Changelog entry

• Add infrastructure for fetching and storing the Ad Blocking Recovery tag.

[tofumatt]

It should be noted that we discussed originally (and outlined in the design doc) that the HTML content would be hashed and the output compared before rendering to ensure it hadn't been modified.

This was largely to prevent accidental modification of the script tag, because any bad actor with database access could change both the source HTML and the hash in order to "trick" Site Kit into outputting arbitrary HTML. But it occurs to me that any change in the length of the string without altering the PHP curried string lengths would also break things.

So I think the hash solution winds up being over-engineered for an unlikely issue that would possibly break before the hash check anyway.

In this case I've omitted it from the ACs. We could always add it later before the feature sees real-world use, but I think it's actually entirely unneeded.

[aaemnnosttv]

This was largely to prevent accidental modification of the script tag, because any bad actor with database access could change both the source HTML and the hash in order to "trick" Site Kit into outputting arbitrary HTML. But it occurs to me that any change in the length of the string without altering the PHP curried string lengths would also break things.

So I think the hash solution winds up being over-engineered for an unlikely issue that would possibly break before the hash check anyway.

In this case I've omitted it from the ACs. We could always add it later before the feature sees real-world use, but I think it's actually entirely unneeded.

Thanks @tofumatt – I think we can probably do without the hash as you're saying, however I do think that we still want to encode the value as discussed to avoid accidental manipulation of the contents as this would break the unserialization of the whole module setting, not just the isolated value of the tag. This is already possible today but very unlikely due to the very limited surface of the keys and values we have. It's also a better experience to avoid the tag becoming broken unintentionally so we don't need to add extra banners, etc to handle a case where "we noticed your tag is broken!" scenario.

[kuasha420]

@tofumatt After some discussion with Evan on this, we'd like to avoid storing the tags in the module settings if possible. Since the tags will only be placed in the front end, we can stop bloating the API responses for AdSense settings with this data.

Also as @aaemnnosttv pointed out, we still want the tags to be encoded, so maybe it should be mentioned in the ACs.

Thank you!

[tofumatt]

Hmmm, fair enough. If we aren't hashing the values, then we can't actually detect that anything is broken, and it does complicate the code paths for a yet-to-be-encountered, edge case caused by a poorly-coded third-party 😆 But fair enough, we can encode them.

Storing them outside serialised settings actually makes sense though, especially as they'll be used a lot on the frontend. I guess we'll want new option values for that 👍🏻

[kuasha420]

@tofumatt Thanks for updating the AC's accordingly. I made a slight tweak to one point to replace encryption with base64-encoding since that's what we agreed on in the call. Everything else LGTM. 👍

[kuasha420]

@tofumatt Hi, I have made a slight change in the AC to not mention the (not yet available) ad blocking recovery onboarding screen. Instead, I added a point about adding an action in the datastore that we can use to fetch the tags. That way, we can start on this, and use the action in the future to actually fetch the tags in the appropriate issue when the on boarding screen will be finalized (#6966). Since you've initially worked on the AC, I'm assigning this to you for a lookover on the AC and IB review at once!

Cheers!

[tofumatt]

IB ✅

[aaemnnosttv]

Noting here that there are some elements of the definition that should have been modified before moving forward that will result in some extra time (and potentially a follow-up issue) to address. See #7134 (review)

[bethanylang]

Thanks, @aaemnnosttv! This is blocking #6963 and #6966, which were both scheduled for this sprint but are only still in IB, anyway. I'm going to move those two to Sprint 104 so that we can focus on completing this issue in Sprint 103.

[bethanylang]

@aaemnnosttv @kuasha420 Actually, will keep #6963 in this sprint as I see there are revisions to take care of there.

[mohitwp]

@wpdarren As discussed on slack I'm getting error when I'm running mentioned code snippet in QAB. Also, verified after setting the status under tester plugin 'ready' as you mentioned. Can you please look into this ?

• Tested on main environment.
• Tested on UA dashboard.

[wpdarren]

@mohitwp I would check the code and QAB with @kuasha420 because I am seeing the same error message. I see the ABD CTA on the dashboard, so everything seems to be set up as expected. I tested this with UA and GA4 dashboard view, and same error occurs. I also made sure that AdSense status for account and site is 'Ready'

[kuasha420]

@mohitwp @wpdarren I've updated the QAB. There was a mistake there as the name of the action changed during CR and I've forgotten to update the QAB accordingly. Sorry!

[mohitwp]

QA Update ✅

• Tested on dev environment.
• Verified that the response is { "success": true } for the code.
• Verified the googlesitekit_adsense_recovery-tag and googlesitekit_adsense_error-protection-tag options from the DATABASE. They populated with long base64 string.
• Verified for both UA and GA4 Dashboard.