Secondary admin authentication takes ownership of Search Console

[eclarke1]

Bug Description

Bug bash issue: https://app.asana.com/0/1202258919887896/1202445809380588 please refer to Asana issue for background

When a user authenticates with SK for the first time, we set the SC property ID if it is provided in the token response.

site-kit-wp/includes/Modules/Search_Console.php

Lines 78 to 84 in 75735ed

	// If the response includes the Search Console property, set that.
	if ( ! empty( $token_response['search_console_property'] ) ) {
	$this->get_settings()->merge(
	array( 'propertyID' => $token_response['search_console_property'] )
	);
	return;
	}

If the module was shared, this would violate one of the core principles of dashboard sharing which is that users need to be informed about data shared via their account.

It's not likely that the property ID would change on the service side after registration but it is technically possible.

---

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

Acceptance criteria

• When a Search Console property is returned in the proxy token response, it should only be set in the corresponding Search Console option under the following situations:
  • Either there is no setting value for this yet.
  • Or the current user is the owner of the Search Console module.
  • Or there is no owner ID set at all (probably not realistically possible, but technically could happen).
• In other words, any connecting administrator that is not the owner of Search Console (unless there is no owner set at all) should not affect this setting through this logic even if their Search Console property is different from the current one.

Implementation Brief

• Update the condition on line 79 of the Search_Console.php snippet listed in the Bug Description to include tests for the following:
  • The propertyID setting is empty, OR
  • The ownerID setting is set to the current user ID, OR
  • The ownerID setting is returned as 0.
• $this->get_property_id() can be used to retrieve the propertyID setting.
• $this->get_owner_id() can be used to retrieve the ownerID setting.
• get_current_user_id() can be used to retrieve the current user ID.

Test Coverage

• Add some PHPUnit tests to cover the above.

QA Brief

QA

• Setup SiteKit and connect Search Console using one admin.
• Create another admin user.
• Login as the second admin user and ensure SiteKit can still be connected as before.

QA: Eng

• Follow the above steps. But check the googlesitekit_search-console_settings option in the database before and after authenticating SiteKit with the second admin. Verify that the propertyID and ownerID do not change when SiteKit is connected with a second admin.

  10updocker wp option get googlesitekit_search-console_settings
  

Changelog entry

• Fix bug where a secondary admin would always take ownership of Search Console when connecting Site Kit.

[aaemnnosttv]

IB ✅

[wpdarren]

QA Update: ✅

• Setup SiteKit and connect Search Console using one admin. I was able to login as the second admin user and SiteKit was connected as before. I tested this on the initial SK dashboard splash page and also on the CTA on the view only menu on shared dashboard. No issues, could connect Site Kit.

I realise this is QA:Eng but I am able to check the database. So completed this test too.

• Checked the googlesitekit_search-console_settings option in the database before and after authenticating SiteKit with the second admin. Confirmed that the propertyID and ownerID do not change.