Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion includes/Core/Authentication/Authentication.php
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,9 @@ public function register() {
add_filter( 'googlesitekit_setup_data', $this->get_method_proxy( 'inline_js_setup_data' ) );

add_action( 'admin_init', $this->get_method_proxy( 'handle_oauth' ) );
add_action( 'admin_init', $this->get_method_proxy( 'check_connected_proxy_url' ) );
// Run after the connected proxy URL migration (default priority) so it
// encodes the stored value before this check compares it to the site URL.
add_action( 'admin_init', $this->get_method_proxy( 'check_connected_proxy_url' ), 20 );

add_action( 'admin_action_' . self::ACTION_CONNECT, $this->get_method_proxy( 'handle_connect' ) );
add_action( 'admin_action_' . self::ACTION_DISCONNECT, $this->get_method_proxy( 'handle_disconnect' ) );
Expand Down
37 changes: 29 additions & 8 deletions includes/Core/Authentication/Connected_Proxy_URL.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,45 @@ class Connected_Proxy_URL extends Setting {
* Matches provided URL with the current proxy URL in the settings.
*
* @since 1.17.0
* @since n.e.x.t Compares the given URL against the decoded stored URL.
*
* @param string $url URL to match against the current one in the settings.
* @return bool TRUE if URL matches the current one, otherwise FALSE.
*/
public function matches_url( $url ) {
$sanitize = $this->get_sanitize_callback();
$normalized = $sanitize( $url );
return $normalized === $this->get();
return trailingslashit( $url ) === $this->get();
}

/**
* Gets the callback for sanitizing the setting's value before saving.
* Gets the connected proxy URL, decoded from the form the option holds.
*
* @since 1.17.0
* A stored value that fails to decode reads as no value at all, so a
* corrupted option sends the site back through the connection flow rather
* than through a comparison against an unreadable URL.
*
* @since n.e.x.t
*
* @return string|bool Connected proxy URL, or FALSE when the option holds
* no value or one that fails to decode.
*/
public function get() {
$stored_url = parent::get();

return is_string( $stored_url ) ? base64_decode( $stored_url, true ) : $stored_url;
}

/**
* Sets the connected proxy URL.
*
* We encode the URL to prevent database-wide search-and-replace tasks
* from changing the URL used to connect to the Site Kit Proxy service.
*
* @since n.e.x.t
*
* @return callable A sanitizing function.
* @param string $value Connected proxy URL.
* @return bool TRUE on success, FALSE on failure.
*/
protected function get_sanitize_callback() {
return 'trailingslashit';
public function set( $value ) {
return parent::set( base64_encode( trailingslashit( $value ) ) );
}
}
109 changes: 109 additions & 0 deletions includes/Core/Util/Migration_N_E_X_T.php

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we already have this migration, why do we need to account for both encoded and unencoded values in the Connected_Proxy_URL file? 🤔

Seems we can assume they're always encoded, no?

Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
<?php
/**
* Migration for the connected proxy URL setting.
*
* @package Google\Site_Kit\Core\Util
* @copyright 2026 Google LLC
* @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0
* @link https://sitekit.withgoogle.com
*/

namespace Google\Site_Kit\Core\Util;

use Google\Site_Kit\Context;
use Google\Site_Kit\Core\Authentication\Connected_Proxy_URL;
use Google\Site_Kit\Core\Storage\Options;

/**
* Class Migration_N_E_X_T.
*
* @since n.e.x.t
* @access private
* @ignore
*/
class Migration_N_E_X_T {
/**
* Target DB version.
*/
const DB_VERSION = 'n.e.x.t';

/**
* DB version option name.
*/
const DB_VERSION_OPTION = 'googlesitekit_db_version';

/**
* Options instance.
*
* @since n.e.x.t
* @var Options
*/
protected Options $options;

/**
* Connected_Proxy_URL instance.
*
* @since n.e.x.t
* @var Connected_Proxy_URL
*/
protected Connected_Proxy_URL $connected_proxy_url;

/**
* Constructor.
*
* @since n.e.x.t
*
* @param Context $context Plugin context instance.
* @param Options $options Optional. Options instance.
*/
public function __construct(
Context $context,
?Options $options = null
) {
$this->options = $options ?: new Options( $context );
$this->connected_proxy_url = new Connected_Proxy_URL( $this->options );
}

/**
* Registers hooks.
*
* @since n.e.x.t
*/
public function register() {
add_action( 'admin_init', array( $this, 'migrate' ) );
}

/**
* Migrates the DB.
*
* @since n.e.x.t
*/
public function migrate() {
$db_version = $this->options->get( self::DB_VERSION_OPTION );

if ( ! $db_version || version_compare( $db_version, self::DB_VERSION, '<' ) ) {
$this->migrate_connected_proxy_url();

$this->options->set( self::DB_VERSION_OPTION, self::DB_VERSION );
}
}

/**
* Migrates a plain text connected proxy URL to the encoded format.
*
* Earlier plugin versions stored the URL in plain text. A stored value that
* still starts with its scheme goes back through Connected_Proxy_URL::set(),
* which stores it encoded.
*
* @since n.e.x.t
*/
protected function migrate_connected_proxy_url() {
$stored_url = $this->options->get( Connected_Proxy_URL::OPTION );

if ( ! is_string( $stored_url ) || 0 !== strpos( $stored_url, 'http' ) ) {
return;
}

$this->connected_proxy_url->set( $stored_url );
}
}
1 change: 1 addition & 0 deletions includes/Plugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ function () use ( $options, $activation_flag ) {
( new Core\Util\Migration_1_150_0( $this->context, $options ) )->register();
( new Core\Util\Migration_1_163_0( $this->context, $options ) )->register();
( new Core\Util\Migration_1_177_0( $this->context, $options ) )->register();
( new Core\Util\Migration_N_E_X_T( $this->context, $options ) )->register();
( new Core\Dashboard_Sharing\Dashboard_Sharing( $this->context ) )->register();
( new Core\Key_Metrics\Key_Metrics( $this->context, $user_options, $options ) )->register();
( new Core\Prompts\Prompts( $this->context, $user_options ) )->register();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ public function test_set_connected_proxy_url() {
do_action( 'googlesitekit_authorize_user', array(), array(), array() );
remove_filter( 'home_url', $home_url_hook );

$this->assertEquals( 'https://example.com/subsite/', $options->get( Connected_Proxy_URL::OPTION ), 'Connected proxy URL should be set to filtered home_url.' ); // PHPCS: line 692.
$this->assertEquals( base64_encode( 'https://example.com/subsite/' ), $options->get( Connected_Proxy_URL::OPTION ), 'The `set()` method should store the connected proxy URL base64-encoded from the filtered home_url.' ); // PHPCS: line 692.
}

public function test_check_connected_proxy_url() {
Expand Down Expand Up @@ -735,6 +735,51 @@ function ( $caps ) {
);
}

public function test_check_connected_proxy_url__after_the_home_url_changes() {
remove_all_actions( 'admin_init' );

$user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) );
wp_set_current_user( $user_id );

$context = new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE );
$options = new Options( $context );
$user_options = new User_Options( $context );

$authentication = new Authentication( $context, $options, $user_options );
$authentication->register();

// Connect the site with the home URL it runs on.
$authentication->get_connected_proxy_url_instance()->set( $context->get_canonical_home_url() );

// Emulate credentials.
$this->fake_proxy_site_connection();

// Emulate OAuth access token.
$authentication->get_oauth_client()->set_token( array( 'access_token' => 'valid-auth-token' ) );

// Grant the administrator the Permissions::SETUP capability regardless
// of authentication.
add_filter(
'user_has_cap',
function ( $caps ) {
$caps[ Permissions::SETUP ] = true;
return $caps;
}
);

// Move the site to another domain, as a search and replace over the
// database does.
update_option( 'home', 'https://new-domain.example.com' );

do_action( 'admin_init' );

$this->assertEquals(
Disconnected_Reason::REASON_CONNECTED_URL_MISMATCH,
$user_options->get( Disconnected_Reason::OPTION ),
'Site Kit should flag the URL change once the home URL moves to another domain.'
);
}

/**
* Test handle_proxy_permissions().
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
/**
* Class Google\Site_Kit\Core\Authentication\Connected_Proxy_URLTest
*
* @package Google\Site_Kit
* @package Google\Site_Kit\Tests\Core\Authentication
* @copyright 2021 Google LLC
* @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0
* @link https://sitekit.withgoogle.com
* */
*/

namespace Google\Site_Kit\Tests\Core\Authentication;

Expand Down Expand Up @@ -48,6 +48,58 @@ public function test_matches_url() {
$this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/' ), 'URL should match when both have trailing slashes.' );
}

public function test_matches_url__after_a_database_search_and_replace() {
$connected_proxy_url = new Connected_Proxy_URL( $this->options );
$connected_proxy_url->register();

$connected_proxy_url->set( 'https://old-site.example.com' );

// Simulate a database search and replace of the site domain.
$stored_url = $this->options->get( Connected_Proxy_URL::OPTION );
$rewritten_url = str_replace( 'old-site.example.com', 'new-site.example.com', $stored_url );

$this->assertSame( $stored_url, $rewritten_url, 'A search and replace should find no domain to rewrite inside the stored value.' );
$this->assertTrue( $connected_proxy_url->matches_url( 'https://old-site.example.com' ), 'The stored URL should still match the connected URL after a search and replace.' );
}

public function test_set__stores_an_encoded_url() {
$connected_proxy_url = new Connected_Proxy_URL( $this->options );
$connected_proxy_url->register();

$connected_proxy_url->set( 'https://example.com' );

$this->assertEquals(
base64_encode( 'https://example.com/' ),
$this->options->get( Connected_Proxy_URL::OPTION ),
'The `set()` method should store the URL as a base64-encoded string, with a trailing slash.'
);
}

public function test_get__decodes_the_stored_url() {
$connected_proxy_url = new Connected_Proxy_URL( $this->options );
$connected_proxy_url->register();

$connected_proxy_url->set( 'https://example.com' );

$this->assertEquals( 'https://example.com/', $connected_proxy_url->get(), 'The `get()` method should return the URL in plain text.' );
}

public function test_get__returns_false_when_not_set() {
$connected_proxy_url = new Connected_Proxy_URL( $this->options );
$connected_proxy_url->register();

$this->assertFalse( $connected_proxy_url->get(), 'The `get()` method should return FALSE when no value exists.' );
}

public function test_get__returns_false_for_a_value_that_fails_to_decode() {
$connected_proxy_url = new Connected_Proxy_URL( $this->options );
$connected_proxy_url->register();

$this->options->set( Connected_Proxy_URL::OPTION, 'not*a*valid*value' );

$this->assertFalse( $connected_proxy_url->get(), 'The `get()` method should return FALSE for a stored value that fails to decode.' );
}

/**
* @inheritDoc
*/
Expand Down
Loading