From 3b6235fbb9c18b823f3965a43031d8bafae527a8 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Wed, 22 Jul 2026 16:54:15 +0300 Subject: [PATCH 1/7] Encode the connected proxy URL setting. Store the setting base64-encoded, so a database search and replace leaves it alone and URL change detection keeps working. The getter decodes on read, and a value an earlier version stored in plain text passes through unchanged. --- .../Authentication/Connected_Proxy_URL.php | 94 ++++++++- .../Connected_Proxy_URLTest.php | 192 +++++++++++++++++- 2 files changed, 275 insertions(+), 11 deletions(-) diff --git a/includes/Core/Authentication/Connected_Proxy_URL.php b/includes/Core/Authentication/Connected_Proxy_URL.php index b811ece498b..805a1d8a9b0 100644 --- a/includes/Core/Authentication/Connected_Proxy_URL.php +++ b/includes/Core/Authentication/Connected_Proxy_URL.php @@ -26,28 +26,110 @@ class Connected_Proxy_URL extends Setting { */ const OPTION = 'googlesitekit_connected_proxy_url'; + /** + * Registers the setting in WordPress. + * + * Decodes the stored value on read, so a caller of the option gets the + * plain-text URL. + * + * @since n.e.x.t + */ + public function register() { + parent::register(); + + add_filter( + 'option_' . static::OPTION, + fn ( $value ) => $this->decode( $value ) + ); + } + /** * Matches provided URL with the current proxy URL in the settings. * * @since 1.17.0 + * @since n.e.x.t Compares against the decoded setting value. * - * @param string $url URL to match against the current one in the settings. + * @param string $site_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(); + public function matches_url( $site_url ) { + return trailingslashit( $site_url ) === $this->get(); + } + + /** + * Gets the connected proxy URL in plain text. + * + * @since n.e.x.t + * + * @return string|bool Connected proxy URL, or FALSE if not set. + */ + public function get() { + return $this->decode( parent::get() ); + } + + /** + * Sets the connected proxy URL, encoding it for storage. + * + * @since n.e.x.t + * + * @param string $value Connected proxy URL, either plain text or encoded. + * @return bool TRUE on success, FALSE on failure. + */ + public function set( $value ) { + return parent::set( $this->encode( $value ) ); } /** * Gets the callback for sanitizing the setting's value before saving. * * @since 1.17.0 + * @since n.e.x.t Encodes the value for storage. * * @return callable A sanitizing function. */ protected function get_sanitize_callback() { - return 'trailingslashit'; + return fn ( $value ) => $this->encode( $value ); + } + + /** + * Encodes the given URL for storage. + * + * Encoding keeps the stored value out of reach of a database search and + * replace. An already encoded value decodes first, so a re-save never + * encodes it twice. + * + * @since n.e.x.t + * + * @param string $value Connected proxy URL, either plain text or encoded. + * @return string Base64-encoded URL with a trailing slash. + */ + private function encode( $value ) { + return base64_encode( trailingslashit( $this->decode( $value ) ) ); + } + + /** + * Decodes the given stored value into a plain-text URL. + * + * Earlier plugin versions stored the value in plain text. A value that + * starts with `http`, or one that fails to decode, passes through + * unchanged. + * + * @since n.e.x.t + * + * @param mixed $value Stored setting value. + * @return mixed Decoded URL, or the given value unchanged. + */ + private function decode( $value ) { + if ( ! is_string( $value ) || '' === $value ) { + return $value; + } + + if ( 0 === strpos( $value, 'http' ) ) { + return $value; + } + + $decoded = base64_decode( $value, true ); + + return false === $decoded ? $value : $decoded; } } diff --git a/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php b/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php index 1d5a423fea3..1efbf27c272 100644 --- a/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php +++ b/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php @@ -2,17 +2,22 @@ /** * 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; use Google\Site_Kit\Context; +use Google\Site_Kit\Core\Authentication\Authentication; use Google\Site_Kit\Core\Authentication\Connected_Proxy_URL; +use Google\Site_Kit\Core\Authentication\Disconnected_Reason; +use Google\Site_Kit\Core\Permissions\Permissions; use Google\Site_Kit\Core\Storage\Options; +use Google\Site_Kit\Core\Storage\User_Options; +use Google\Site_Kit\Tests\Fake_Site_Connection_Trait; use Google\Site_Kit\Tests\Modules\SettingsTestCase; /** @@ -22,6 +27,8 @@ */ class Connected_Proxy_URLTest extends SettingsTestCase { + use Fake_Site_Connection_Trait; + /** * Options object. * @@ -32,6 +39,11 @@ class Connected_Proxy_URLTest extends SettingsTestCase { public function set_up() { parent::set_up(); $this->options = new Options( new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ) ); + + // Drop any sanitize filter the bootstrap plugin instance added, so a + // test that saves the option before it registers the setting stores + // the raw value. + remove_all_filters( 'sanitize_option_' . Connected_Proxy_URL::OPTION ); } public function test_matches_url() { @@ -39,13 +51,164 @@ public function test_matches_url() { $connected_proxy_url->register(); $connected_proxy_url->set( 'https://example.com' ); - $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/' ), 'URL should match when trailing slash is added.' ); + $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/' ), 'URL should match after the setter adds a trailing slash.' ); $connected_proxy_url->set( 'https://example.com/subdirectory' ); - $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/subdirectory/' ), 'URL should match when trailing slash is added to subdirectory.' ); + $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/subdirectory/' ), 'Subdirectory URL should match after the setter adds a trailing slash.' ); $connected_proxy_url->set( 'https://example.com/' ); - $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/' ), 'URL should match when both have trailing slashes.' ); + $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/' ), 'URL should match when both URLs have a trailing slash.' ); + } + + public function test_matches_url__with_legacy_plain_text_value() { + $connected_proxy_url = new Connected_Proxy_URL( $this->options ); + + // Store the option the way versions without encoding saved it. + $this->update_option( 'https://example.com/' ); + + $connected_proxy_url->register(); + + $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com' ), 'URL should match a legacy plain text value.' ); + $this->assertFalse( $connected_proxy_url->matches_url( 'https://other.example.com' ), "A different URL shouldn't match a legacy plain text value." ); + } + + public function test_set__stores_an_encoded_value() { + $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->get_raw_option_value(), + 'Setter should store the URL base64-encoded with a trailing slash.' + ); + } + + public function test_set__avoids_double_encoding() { + $connected_proxy_url = new Connected_Proxy_URL( $this->options ); + $connected_proxy_url->register(); + + $connected_proxy_url->set( base64_encode( 'https://example.com/' ) ); + + $this->assertEquals( + base64_encode( 'https://example.com/' ), + $this->get_raw_option_value(), + 'Setter should not encode an already encoded value a second time.' + ); + $this->assertEquals( + 'https://example.com/', + $connected_proxy_url->get(), + 'Getter should return the plain URL after saving an encoded value.' + ); + } + + public function test_get__decodes_the_stored_value() { + $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(), 'Getter should return the URL in plain text.' ); + } + + public function test_get__returns_a_legacy_plain_text_value() { + $connected_proxy_url = new Connected_Proxy_URL( $this->options ); + + // Store the option the way versions without encoding saved it. + $this->update_option( 'https://example.com/' ); + + $connected_proxy_url->register(); + + $this->assertEquals( 'https://example.com/', $connected_proxy_url->get(), 'Getter should return a legacy plain text value unchanged.' ); + } + + 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(), 'Getter should return FALSE when no value exists.' ); + } + + public function test_get__returns_a_value_that_fails_to_decode_unchanged() { + $connected_proxy_url = new Connected_Proxy_URL( $this->options ); + + $this->update_option( 'not*a*valid*value' ); + + $connected_proxy_url->register(); + + $this->assertEquals( 'not*a*valid*value', $connected_proxy_url->get(), 'Getter should leave a value that fails to decode unchanged.' ); + } + + public function test_sanitize_callback__encodes_a_value_saved_directly() { + $connected_proxy_url = new Connected_Proxy_URL( $this->options ); + $connected_proxy_url->register(); + + // Save the option directly, as an update on the options screen would. + $this->update_option( 'https://example.com' ); + + $this->assertEquals( + base64_encode( 'https://example.com/' ), + $this->get_raw_option_value(), + 'Sanitize callback should encode a value that update_option saves.' + ); + $this->assertEquals( 'https://example.com/', $connected_proxy_url->get(), 'Getter should return the plain URL after a direct save.' ); + } + + public function test_matches_url__stored_value_survives_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 URL. + $stored = $this->get_raw_option_value(); + $rewritten = str_replace( 'old-site.example.com', 'new-site.example.com', $stored ); + + $this->assertSame( $stored, $rewritten, 'Search and replace should not find the URL inside the encoded value.' ); + $this->assertTrue( $connected_proxy_url->matches_url( 'https://old-site.example.com' ), 'Stored URL should still match the original URL after search and replace.' ); + $this->assertFalse( $connected_proxy_url->matches_url( 'https://new-site.example.com' ), "Stored URL shouldn't match the new URL, so Site Kit detects the change." ); + } + + public function test_matches_url__flags_a_url_change_after_a_database_search_and_replace() { + 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 ); + $user_options = new User_Options( $context ); + + $authentication = new Authentication( $context, $this->options, $user_options ); + $authentication->register(); + + // Connect the site with its current URL. + $authentication->get_connected_proxy_url_instance()->set( $context->get_canonical_home_url() ); + + // Emulate proxy credentials and an OAuth access token. + $this->fake_proxy_site_connection(); + $authentication->get_oauth_client()->set_token( array( 'access_token' => 'valid-auth-token' ) ); + + // Grant the administrator the setup capability regardless of authentication state. + add_filter( + 'user_has_cap', + function ( $capabilities ) { + $capabilities[ Permissions::SETUP ] = true; + return $capabilities; + } + ); + + // Emulate a database search and replace that rewrites the site URL: + // it changes the home option but leaves the encoded option untouched. + update_option( 'home', 'https://new-domain.example.com' ); + + do_action( 'admin_init' ); + + $this->assertEquals( + 'connected_url_mismatch', + $user_options->get( Disconnected_Reason::OPTION ), + 'Site Kit should flag the URL change after a database search and replace.' + ); } /** @@ -54,4 +217,23 @@ public function test_matches_url() { protected function get_option_name() { return Connected_Proxy_URL::OPTION; } + + /** + * Gets the raw option value straight from the database. + * + * The `option_` filter decodes the value on read, so this helper reads + * the row the way a search and replace tool sees it. + * + * @return string|null Raw option value, or NULL when the row is missing. + */ + private function get_raw_option_value() { + global $wpdb; + + return $wpdb->get_var( + $wpdb->prepare( + "SELECT option_value FROM {$wpdb->options} WHERE option_name = %s", + Connected_Proxy_URL::OPTION + ) + ); + } } From 2e8c6be6527712e0c5dbc564798740c92878718f Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Wed, 22 Jul 2026 16:54:22 +0300 Subject: [PATCH 2/7] Add the connected proxy URL migration. Re-save the connected proxy URL on admin_init, so a value an earlier version stored in plain text becomes encoded. The migration runs once and then stamps the DB version option. --- includes/Core/Util/Migration_N_E_X_T.php | 98 +++++++++++++ .../Core/Util/Migration_N_E_X_TTest.php | 132 ++++++++++++++++++ 2 files changed, 230 insertions(+) create mode 100644 includes/Core/Util/Migration_N_E_X_T.php create mode 100644 tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php diff --git a/includes/Core/Util/Migration_N_E_X_T.php b/includes/Core/Util/Migration_N_E_X_T.php new file mode 100644 index 00000000000..3fc85f1075c --- /dev/null +++ b/includes/Core/Util/Migration_N_E_X_T.php @@ -0,0 +1,98 @@ +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. + * + * @since n.e.x.t + */ + protected function migrate_connected_proxy_url() { + if ( ! $this->connected_proxy_url->has() ) { + return; + } + + // The getter returns a legacy or an encoded value as plain text, and + // the setter stores it encoded. + $this->connected_proxy_url->set( $this->connected_proxy_url->get() ); + } +} diff --git a/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php b/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php new file mode 100644 index 00000000000..dbb1430653a --- /dev/null +++ b/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php @@ -0,0 +1,132 @@ +context = new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ); + $this->options = new Options( $this->context ); + $this->connected_proxy_url = new Connected_Proxy_URL( $this->options ); + + // Drop the option and any filters the bootstrap plugin instance + // added, so each test reads and writes the raw option value. + delete_option( Connected_Proxy_URL::OPTION ); + remove_all_filters( 'option_' . Connected_Proxy_URL::OPTION ); + remove_all_filters( 'sanitize_option_' . Connected_Proxy_URL::OPTION ); + + $this->delete_db_version(); + } + + public function get_new_migration_instance() { + return new Migration_N_E_X_T( + $this->context, + $this->options + ); + } + + public function test_register() { + $migration = $this->get_new_migration_instance(); + remove_all_actions( 'admin_init' ); + + $migration->register(); + + $this->assertTrue( has_action( 'admin_init' ), 'Migration should register the admin_init action.' ); + } + + public function test_migrate__encodes_a_legacy_plain_text_value() { + $migration = $this->get_new_migration_instance(); + + // Store the option the way versions without encoding saved it. + update_option( Connected_Proxy_URL::OPTION, 'https://example.com/' ); + + $migration->migrate(); + + $this->assertEquals( + base64_encode( 'https://example.com/' ), + get_option( Connected_Proxy_URL::OPTION ), + 'Migration should store the legacy plain text URL encoded.' + ); + $this->assertEquals( + 'https://example.com/', + $this->connected_proxy_url->get(), + 'Getter should still return the plain text URL after the migration.' + ); + } + + public function test_migrate__keeps_an_encoded_value() { + $migration = $this->get_new_migration_instance(); + + $this->connected_proxy_url->set( 'https://example.com' ); + $encoded = get_option( Connected_Proxy_URL::OPTION ); + + $migration->migrate(); + + $this->assertSame( + $encoded, + get_option( Connected_Proxy_URL::OPTION ), + 'Migration should keep an already encoded value unchanged.' + ); + } + + public function test_migrate__skips_when_the_option_is_missing() { + $migration = $this->get_new_migration_instance(); + + $migration->migrate(); + + $this->assertOptionNotExists( Connected_Proxy_URL::OPTION ); + } + + public function test_migrate__sets_db_version() { + $migration = $this->get_new_migration_instance(); + + $migration->migrate(); + + $this->assertEquals( 'n.e.x.t', $this->get_db_version(), "Database version should update to the migration's target version after the migration runs." ); + } + + public function test_migrate__skips_when_the_db_version_is_current() { + $migration = $this->get_new_migration_instance(); + + $this->options->set( Migration_N_E_X_T::DB_VERSION_OPTION, Migration_N_E_X_T::DB_VERSION ); + + // Store the option the way versions without encoding saved it. + update_option( Connected_Proxy_URL::OPTION, 'https://example.com/' ); + + $migration->migrate(); + + $this->assertEquals( + 'https://example.com/', + get_option( Connected_Proxy_URL::OPTION ), + 'Migration should leave the stored value alone when the database version already matches its target.' + ); + } + + protected function get_db_version() { + return $this->options->get( Migration_N_E_X_T::DB_VERSION_OPTION ); + } + + protected function delete_db_version() { + $this->options->delete( Migration_N_E_X_T::DB_VERSION_OPTION ); + } +} From 984a68f6f73fad0b26be08d44f68346a672bb1a4 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Wed, 22 Jul 2026 16:54:27 +0300 Subject: [PATCH 3/7] Register the connected proxy URL migration. Add the migration to the plugin's registration list, so it runs alongside the earlier migrations. --- includes/Plugin.php | 1 + 1 file changed, 1 insertion(+) diff --git a/includes/Plugin.php b/includes/Plugin.php index 4ca0114edb9..3750f5ab81a 100644 --- a/includes/Plugin.php +++ b/includes/Plugin.php @@ -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(); From d57a71a390c1361158c2d0371b228ea5fc185f04 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Wed, 22 Jul 2026 18:31:53 +0300 Subject: [PATCH 4/7] Update comments for clarity in Connected_Proxy_URL and migrations. Rephrase comments to enhance understanding of how legacy values are stored and processed. This improves code readability and maintainability. --- includes/Core/Authentication/Connected_Proxy_URL.php | 6 +++--- includes/Core/Util/Migration_N_E_X_T.php | 9 ++++++++- .../Core/Authentication/Connected_Proxy_URLTest.php | 8 ++++---- .../integration/Core/Util/Migration_N_E_X_TTest.php | 4 ++-- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/includes/Core/Authentication/Connected_Proxy_URL.php b/includes/Core/Authentication/Connected_Proxy_URL.php index 805a1d8a9b0..a2c1fe4f000 100644 --- a/includes/Core/Authentication/Connected_Proxy_URL.php +++ b/includes/Core/Authentication/Connected_Proxy_URL.php @@ -94,9 +94,9 @@ protected function get_sanitize_callback() { /** * Encodes the given URL for storage. * - * Encoding keeps the stored value out of reach of a database search and - * replace. An already encoded value decodes first, so a re-save never - * encodes it twice. + * The encoded value holds no readable URL, so a database search and + * replace leaves it unchanged. An already encoded value decodes first, + * so a re-save never encodes it twice. * * @since n.e.x.t * diff --git a/includes/Core/Util/Migration_N_E_X_T.php b/includes/Core/Util/Migration_N_E_X_T.php index 3fc85f1075c..9bc255f2311 100644 --- a/includes/Core/Util/Migration_N_E_X_T.php +++ b/includes/Core/Util/Migration_N_E_X_T.php @@ -22,7 +22,14 @@ * @ignore */ class Migration_N_E_X_T { - const DB_VERSION = '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'; /** diff --git a/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php b/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php index 1efbf27c272..d19531c0795 100644 --- a/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php +++ b/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php @@ -63,7 +63,7 @@ public function test_matches_url() { public function test_matches_url__with_legacy_plain_text_value() { $connected_proxy_url = new Connected_Proxy_URL( $this->options ); - // Store the option the way versions without encoding saved it. + // Store the option in plain text, the way earlier plugin versions saved it. $this->update_option( 'https://example.com/' ); $connected_proxy_url->register(); @@ -115,7 +115,7 @@ public function test_get__decodes_the_stored_value() { public function test_get__returns_a_legacy_plain_text_value() { $connected_proxy_url = new Connected_Proxy_URL( $this->options ); - // Store the option the way versions without encoding saved it. + // Store the option in plain text, the way earlier plugin versions saved it. $this->update_option( 'https://example.com/' ); $connected_proxy_url->register(); @@ -185,7 +185,7 @@ public function test_matches_url__flags_a_url_change_after_a_database_search_and // Connect the site with its current URL. $authentication->get_connected_proxy_url_instance()->set( $context->get_canonical_home_url() ); - // Emulate proxy credentials and an OAuth access token. + // Simulate proxy credentials and an OAuth access token. $this->fake_proxy_site_connection(); $authentication->get_oauth_client()->set_token( array( 'access_token' => 'valid-auth-token' ) ); @@ -198,7 +198,7 @@ function ( $capabilities ) { } ); - // Emulate a database search and replace that rewrites the site URL: + // Simulate a database search and replace that rewrites the site URL: // it changes the home option but leaves the encoded option untouched. update_option( 'home', 'https://new-domain.example.com' ); diff --git a/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php b/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php index dbb1430653a..bd92af779ce 100644 --- a/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php +++ b/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php @@ -57,7 +57,7 @@ public function test_register() { public function test_migrate__encodes_a_legacy_plain_text_value() { $migration = $this->get_new_migration_instance(); - // Store the option the way versions without encoding saved it. + // Store the option in plain text, the way earlier plugin versions saved it. update_option( Connected_Proxy_URL::OPTION, 'https://example.com/' ); $migration->migrate(); @@ -110,7 +110,7 @@ public function test_migrate__skips_when_the_db_version_is_current() { $this->options->set( Migration_N_E_X_T::DB_VERSION_OPTION, Migration_N_E_X_T::DB_VERSION ); - // Store the option the way versions without encoding saved it. + // Store the option in plain text, the way earlier plugin versions saved it. update_option( Connected_Proxy_URL::OPTION, 'https://example.com/' ); $migration->migrate(); From 7e1de03b4822d2a8d9d5bedaeb0d57f7781a7522 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Wed, 22 Jul 2026 20:16:48 +0300 Subject: [PATCH 5/7] Encode the connected proxy URL without a read filter. --- .../Authentication/Connected_Proxy_URL.php | 21 +------ .../Authentication/AuthenticationTest.php | 2 +- .../Connected_Proxy_URLTest.php | 56 ++++++++----------- .../Core/Util/Migration_N_E_X_TTest.php | 25 ++++----- 4 files changed, 38 insertions(+), 66 deletions(-) diff --git a/includes/Core/Authentication/Connected_Proxy_URL.php b/includes/Core/Authentication/Connected_Proxy_URL.php index a2c1fe4f000..96cb54d13aa 100644 --- a/includes/Core/Authentication/Connected_Proxy_URL.php +++ b/includes/Core/Authentication/Connected_Proxy_URL.php @@ -26,23 +26,6 @@ class Connected_Proxy_URL extends Setting { */ const OPTION = 'googlesitekit_connected_proxy_url'; - /** - * Registers the setting in WordPress. - * - * Decodes the stored value on read, so a caller of the option gets the - * plain-text URL. - * - * @since n.e.x.t - */ - public function register() { - parent::register(); - - add_filter( - 'option_' . static::OPTION, - fn ( $value ) => $this->decode( $value ) - ); - } - /** * Matches provided URL with the current proxy URL in the settings. * @@ -128,8 +111,8 @@ private function decode( $value ) { return $value; } - $decoded = base64_decode( $value, true ); + $decoded_url = base64_decode( $value, true ); - return false === $decoded ? $value : $decoded; + return false === $decoded_url ? $value : $decoded_url; } } diff --git a/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php b/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php index fa5c839fdb6..ea60fb9b92b 100644 --- a/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php +++ b/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php @@ -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 ), 'Setter should store the connected proxy URL base64-encoded from the filtered home_url.' ); // PHPCS: line 692 } public function test_check_connected_proxy_url() { diff --git a/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php b/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php index d19531c0795..015b3b9eba6 100644 --- a/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php +++ b/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php @@ -60,7 +60,7 @@ public function test_matches_url() { $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/' ), 'URL should match when both URLs have a trailing slash.' ); } - public function test_matches_url__with_legacy_plain_text_value() { + public function test_matches_url__with_a_legacy_plain_text_url() { $connected_proxy_url = new Connected_Proxy_URL( $this->options ); // Store the option in plain text, the way earlier plugin versions saved it. @@ -72,7 +72,7 @@ public function test_matches_url__with_legacy_plain_text_value() { $this->assertFalse( $connected_proxy_url->matches_url( 'https://other.example.com' ), "A different URL shouldn't match a legacy plain text value." ); } - public function test_set__stores_an_encoded_value() { + public function test_set__stores_an_encoded_url() { $connected_proxy_url = new Connected_Proxy_URL( $this->options ); $connected_proxy_url->register(); @@ -80,7 +80,7 @@ public function test_set__stores_an_encoded_value() { $this->assertEquals( base64_encode( 'https://example.com/' ), - $this->get_raw_option_value(), + $this->options->get( Connected_Proxy_URL::OPTION ), 'Setter should store the URL base64-encoded with a trailing slash.' ); } @@ -93,7 +93,7 @@ public function test_set__avoids_double_encoding() { $this->assertEquals( base64_encode( 'https://example.com/' ), - $this->get_raw_option_value(), + $this->options->get( Connected_Proxy_URL::OPTION ), 'Setter should not encode an already encoded value a second time.' ); $this->assertEquals( @@ -103,7 +103,7 @@ public function test_set__avoids_double_encoding() { ); } - public function test_get__decodes_the_stored_value() { + public function test_get__decodes_the_stored_url() { $connected_proxy_url = new Connected_Proxy_URL( $this->options ); $connected_proxy_url->register(); @@ -112,7 +112,7 @@ public function test_get__decodes_the_stored_value() { $this->assertEquals( 'https://example.com/', $connected_proxy_url->get(), 'Getter should return the URL in plain text.' ); } - public function test_get__returns_a_legacy_plain_text_value() { + public function test_get__returns_a_legacy_plain_text_url() { $connected_proxy_url = new Connected_Proxy_URL( $this->options ); // Store the option in plain text, the way earlier plugin versions saved it. @@ -140,7 +140,7 @@ public function test_get__returns_a_value_that_fails_to_decode_unchanged() { $this->assertEquals( 'not*a*valid*value', $connected_proxy_url->get(), 'Getter should leave a value that fails to decode unchanged.' ); } - public function test_sanitize_callback__encodes_a_value_saved_directly() { + public function test_sanitize_callback__encodes_a_url_saved_directly() { $connected_proxy_url = new Connected_Proxy_URL( $this->options ); $connected_proxy_url->register(); @@ -149,23 +149,23 @@ public function test_sanitize_callback__encodes_a_value_saved_directly() { $this->assertEquals( base64_encode( 'https://example.com/' ), - $this->get_raw_option_value(), + $this->options->get( Connected_Proxy_URL::OPTION ), 'Sanitize callback should encode a value that update_option saves.' ); $this->assertEquals( 'https://example.com/', $connected_proxy_url->get(), 'Getter should return the plain URL after a direct save.' ); } - public function test_matches_url__stored_value_survives_search_and_replace() { + public function test_matches_url__stored_url_survives_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 URL. - $stored = $this->get_raw_option_value(); - $rewritten = str_replace( 'old-site.example.com', 'new-site.example.com', $stored ); + $stored_connected_url = $this->options->get( Connected_Proxy_URL::OPTION ); + $search_replace_result = str_replace( 'old-site.example.com', 'new-site.example.com', $stored_connected_url ); - $this->assertSame( $stored, $rewritten, 'Search and replace should not find the URL inside the encoded value.' ); + $this->assertSame( $stored_connected_url, $search_replace_result, 'Search and replace should not find the URL inside the encoded value.' ); $this->assertTrue( $connected_proxy_url->matches_url( 'https://old-site.example.com' ), 'Stored URL should still match the original URL after search and replace.' ); $this->assertFalse( $connected_proxy_url->matches_url( 'https://new-site.example.com' ), "Stored URL shouldn't match the new URL, so Site Kit detects the change." ); } @@ -198,10 +198,19 @@ function ( $capabilities ) { } ); - // Simulate a database search and replace that rewrites the site URL: - // it changes the home option but leaves the encoded option untouched. + $connected_host = wp_parse_url( $context->get_canonical_home_url(), PHP_URL_HOST ); + $stored_connected_url = $this->options->get( Connected_Proxy_URL::OPTION ); + + // Simulate a database search and replace: it rewrites the home option, + // and finds no host to rewrite inside the encoded value. update_option( 'home', 'https://new-domain.example.com' ); + $this->assertSame( + $stored_connected_url, + str_replace( $connected_host, 'new-domain.example.com', $stored_connected_url ), + 'Search and replace should find no host to rewrite inside the stored value.' + ); + do_action( 'admin_init' ); $this->assertEquals( @@ -217,23 +226,4 @@ function ( $capabilities ) { protected function get_option_name() { return Connected_Proxy_URL::OPTION; } - - /** - * Gets the raw option value straight from the database. - * - * The `option_` filter decodes the value on read, so this helper reads - * the row the way a search and replace tool sees it. - * - * @return string|null Raw option value, or NULL when the row is missing. - */ - private function get_raw_option_value() { - global $wpdb; - - return $wpdb->get_var( - $wpdb->prepare( - "SELECT option_value FROM {$wpdb->options} WHERE option_name = %s", - Connected_Proxy_URL::OPTION - ) - ); - } } diff --git a/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php b/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php index bd92af779ce..1defa3001bf 100644 --- a/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php +++ b/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php @@ -29,10 +29,9 @@ public function set_up() { $this->options = new Options( $this->context ); $this->connected_proxy_url = new Connected_Proxy_URL( $this->options ); - // Drop the option and any filters the bootstrap plugin instance - // added, so each test reads and writes the raw option value. - delete_option( Connected_Proxy_URL::OPTION ); - remove_all_filters( 'option_' . Connected_Proxy_URL::OPTION ); + // Drop the option and the sanitize filter the bootstrap plugin + // instance added, so each test reads and writes the raw option value. + $this->options->delete( Connected_Proxy_URL::OPTION ); remove_all_filters( 'sanitize_option_' . Connected_Proxy_URL::OPTION ); $this->delete_db_version(); @@ -54,17 +53,17 @@ public function test_register() { $this->assertTrue( has_action( 'admin_init' ), 'Migration should register the admin_init action.' ); } - public function test_migrate__encodes_a_legacy_plain_text_value() { + public function test_migrate__encodes_a_legacy_plain_text_url() { $migration = $this->get_new_migration_instance(); // Store the option in plain text, the way earlier plugin versions saved it. - update_option( Connected_Proxy_URL::OPTION, 'https://example.com/' ); + $this->options->set( Connected_Proxy_URL::OPTION, 'https://example.com/' ); $migration->migrate(); $this->assertEquals( base64_encode( 'https://example.com/' ), - get_option( Connected_Proxy_URL::OPTION ), + $this->options->get( Connected_Proxy_URL::OPTION ), 'Migration should store the legacy plain text URL encoded.' ); $this->assertEquals( @@ -74,17 +73,17 @@ public function test_migrate__encodes_a_legacy_plain_text_value() { ); } - public function test_migrate__keeps_an_encoded_value() { + public function test_migrate__keeps_an_already_encoded_url() { $migration = $this->get_new_migration_instance(); $this->connected_proxy_url->set( 'https://example.com' ); - $encoded = get_option( Connected_Proxy_URL::OPTION ); + $stored_connected_url = $this->options->get( Connected_Proxy_URL::OPTION ); $migration->migrate(); $this->assertSame( - $encoded, - get_option( Connected_Proxy_URL::OPTION ), + $stored_connected_url, + $this->options->get( Connected_Proxy_URL::OPTION ), 'Migration should keep an already encoded value unchanged.' ); } @@ -111,13 +110,13 @@ public function test_migrate__skips_when_the_db_version_is_current() { $this->options->set( Migration_N_E_X_T::DB_VERSION_OPTION, Migration_N_E_X_T::DB_VERSION ); // Store the option in plain text, the way earlier plugin versions saved it. - update_option( Connected_Proxy_URL::OPTION, 'https://example.com/' ); + $this->options->set( Connected_Proxy_URL::OPTION, 'https://example.com/' ); $migration->migrate(); $this->assertEquals( 'https://example.com/', - get_option( Connected_Proxy_URL::OPTION ), + $this->options->get( Connected_Proxy_URL::OPTION ), 'Migration should leave the stored value alone when the database version already matches its target.' ); } From 67a2678417aa393f86f5230f5ed4bf51a3457af6 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Thu, 23 Jul 2026 20:22:02 +0300 Subject: [PATCH 6/7] Address CR feedback. --- .../Core/Authentication/Authentication.php | 4 +- .../Authentication/Connected_Proxy_URL.php | 84 +++------- includes/Core/Util/Migration_N_E_X_T.php | 14 +- .../Authentication/AuthenticationTest.php | 47 +++++- .../Connected_Proxy_URLTest.php | 154 ++---------------- .../Core/Util/Migration_N_E_X_TTest.php | 65 +++++++- 6 files changed, 155 insertions(+), 213 deletions(-) diff --git a/includes/Core/Authentication/Authentication.php b/includes/Core/Authentication/Authentication.php index a10958fc5e7..b7ce536ca6f 100644 --- a/includes/Core/Authentication/Authentication.php +++ b/includes/Core/Authentication/Authentication.php @@ -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' ) ); diff --git a/includes/Core/Authentication/Connected_Proxy_URL.php b/includes/Core/Authentication/Connected_Proxy_URL.php index 96cb54d13aa..fbcc5924391 100644 --- a/includes/Core/Authentication/Connected_Proxy_URL.php +++ b/includes/Core/Authentication/Connected_Proxy_URL.php @@ -30,89 +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 against the decoded setting value. + * @since n.e.x.t Compares the given URL against the decoded stored URL. * - * @param string $site_url URL to match against the current one in the settings. + * @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( $site_url ) { - return trailingslashit( $site_url ) === $this->get(); + public function matches_url( $url ) { + return trailingslashit( $url ) === $this->get(); } /** - * Gets the connected proxy URL in plain text. + * Gets the connected proxy URL, decoded from the form the option holds. * - * @since n.e.x.t - * - * @return string|bool Connected proxy URL, or FALSE if not set. - */ - public function get() { - return $this->decode( parent::get() ); - } - - /** - * Sets the connected proxy URL, encoding it for storage. + * 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 * - * @param string $value Connected proxy URL, either plain text or encoded. - * @return bool TRUE on success, FALSE on failure. + * @return string|bool Connected proxy URL, or FALSE when the option holds + * no value or one that fails to decode. */ - public function set( $value ) { - return parent::set( $this->encode( $value ) ); - } - - /** - * Gets the callback for sanitizing the setting's value before saving. - * - * @since 1.17.0 - * @since n.e.x.t Encodes the value for storage. - * - * @return callable A sanitizing function. - */ - protected function get_sanitize_callback() { - return fn ( $value ) => $this->encode( $value ); - } + public function get() { + $stored_url = parent::get(); - /** - * Encodes the given URL for storage. - * - * The encoded value holds no readable URL, so a database search and - * replace leaves it unchanged. An already encoded value decodes first, - * so a re-save never encodes it twice. - * - * @since n.e.x.t - * - * @param string $value Connected proxy URL, either plain text or encoded. - * @return string Base64-encoded URL with a trailing slash. - */ - private function encode( $value ) { - return base64_encode( trailingslashit( $this->decode( $value ) ) ); + return is_string( $stored_url ) ? base64_decode( $stored_url, true ) : $stored_url; } /** - * Decodes the given stored value into a plain-text URL. + * Sets the connected proxy URL. * - * Earlier plugin versions stored the value in plain text. A value that - * starts with `http`, or one that fails to decode, passes through - * unchanged. + * 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 * - * @param mixed $value Stored setting value. - * @return mixed Decoded URL, or the given value unchanged. + * @param string $value Connected proxy URL. + * @return bool TRUE on success, FALSE on failure. */ - private function decode( $value ) { - if ( ! is_string( $value ) || '' === $value ) { - return $value; - } - - if ( 0 === strpos( $value, 'http' ) ) { - return $value; - } - - $decoded_url = base64_decode( $value, true ); - - return false === $decoded_url ? $value : $decoded_url; + public function set( $value ) { + return parent::set( base64_encode( trailingslashit( $value ) ) ); } } diff --git a/includes/Core/Util/Migration_N_E_X_T.php b/includes/Core/Util/Migration_N_E_X_T.php index 9bc255f2311..a8d73b29ebc 100644 --- a/includes/Core/Util/Migration_N_E_X_T.php +++ b/includes/Core/Util/Migration_N_E_X_T.php @@ -89,17 +89,21 @@ public function migrate() { } /** - * Migrates a plain-text connected proxy URL to the encoded format. + * 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() { - if ( ! $this->connected_proxy_url->has() ) { + $stored_url = $this->options->get( Connected_Proxy_URL::OPTION ); + + if ( ! is_string( $stored_url ) || 0 !== strpos( $stored_url, 'http' ) ) { return; } - // The getter returns a legacy or an encoded value as plain text, and - // the setter stores it encoded. - $this->connected_proxy_url->set( $this->connected_proxy_url->get() ); + $this->connected_proxy_url->set( $stored_url ); } } diff --git a/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php b/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php index ea60fb9b92b..dec8332d237 100644 --- a/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php +++ b/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php @@ -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( base64_encode( 'https://example.com/subsite/' ), $options->get( Connected_Proxy_URL::OPTION ), 'Setter should store the connected proxy URL base64-encoded from the 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() { @@ -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() */ diff --git a/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php b/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php index 015b3b9eba6..29adcf24f58 100644 --- a/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php +++ b/tests/phpunit/integration/Core/Authentication/Connected_Proxy_URLTest.php @@ -11,13 +11,8 @@ namespace Google\Site_Kit\Tests\Core\Authentication; use Google\Site_Kit\Context; -use Google\Site_Kit\Core\Authentication\Authentication; use Google\Site_Kit\Core\Authentication\Connected_Proxy_URL; -use Google\Site_Kit\Core\Authentication\Disconnected_Reason; -use Google\Site_Kit\Core\Permissions\Permissions; use Google\Site_Kit\Core\Storage\Options; -use Google\Site_Kit\Core\Storage\User_Options; -use Google\Site_Kit\Tests\Fake_Site_Connection_Trait; use Google\Site_Kit\Tests\Modules\SettingsTestCase; /** @@ -27,8 +22,6 @@ */ class Connected_Proxy_URLTest extends SettingsTestCase { - use Fake_Site_Connection_Trait; - /** * Options object. * @@ -39,11 +32,6 @@ class Connected_Proxy_URLTest extends SettingsTestCase { public function set_up() { parent::set_up(); $this->options = new Options( new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ) ); - - // Drop any sanitize filter the bootstrap plugin instance added, so a - // test that saves the option before it registers the setting stores - // the raw value. - remove_all_filters( 'sanitize_option_' . Connected_Proxy_URL::OPTION ); } public function test_matches_url() { @@ -51,25 +39,27 @@ public function test_matches_url() { $connected_proxy_url->register(); $connected_proxy_url->set( 'https://example.com' ); - $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/' ), 'URL should match after the setter adds a trailing slash.' ); + $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/' ), 'URL should match when trailing slash is added.' ); $connected_proxy_url->set( 'https://example.com/subdirectory' ); - $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/subdirectory/' ), 'Subdirectory URL should match after the setter adds a trailing slash.' ); + $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/subdirectory/' ), 'URL should match when trailing slash is added to subdirectory.' ); $connected_proxy_url->set( 'https://example.com/' ); - $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/' ), 'URL should match when both URLs have a trailing slash.' ); + $this->assertTrue( $connected_proxy_url->matches_url( 'https://example.com/' ), 'URL should match when both have trailing slashes.' ); } - public function test_matches_url__with_a_legacy_plain_text_url() { + public function test_matches_url__after_a_database_search_and_replace() { $connected_proxy_url = new Connected_Proxy_URL( $this->options ); + $connected_proxy_url->register(); - // Store the option in plain text, the way earlier plugin versions saved it. - $this->update_option( 'https://example.com/' ); + $connected_proxy_url->set( 'https://old-site.example.com' ); - $connected_proxy_url->register(); + // 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->assertTrue( $connected_proxy_url->matches_url( 'https://example.com' ), 'URL should match a legacy plain text value.' ); - $this->assertFalse( $connected_proxy_url->matches_url( 'https://other.example.com' ), "A different URL shouldn't match a legacy plain text value." ); + $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() { @@ -81,25 +71,7 @@ public function test_set__stores_an_encoded_url() { $this->assertEquals( base64_encode( 'https://example.com/' ), $this->options->get( Connected_Proxy_URL::OPTION ), - 'Setter should store the URL base64-encoded with a trailing slash.' - ); - } - - public function test_set__avoids_double_encoding() { - $connected_proxy_url = new Connected_Proxy_URL( $this->options ); - $connected_proxy_url->register(); - - $connected_proxy_url->set( base64_encode( 'https://example.com/' ) ); - - $this->assertEquals( - base64_encode( 'https://example.com/' ), - $this->options->get( Connected_Proxy_URL::OPTION ), - 'Setter should not encode an already encoded value a second time.' - ); - $this->assertEquals( - 'https://example.com/', - $connected_proxy_url->get(), - 'Getter should return the plain URL after saving an encoded value.' + 'The `set()` method should store the URL as a base64-encoded string, with a trailing slash.' ); } @@ -109,115 +81,23 @@ public function test_get__decodes_the_stored_url() { $connected_proxy_url->set( 'https://example.com' ); - $this->assertEquals( 'https://example.com/', $connected_proxy_url->get(), 'Getter should return the URL in plain text.' ); - } - - public function test_get__returns_a_legacy_plain_text_url() { - $connected_proxy_url = new Connected_Proxy_URL( $this->options ); - - // Store the option in plain text, the way earlier plugin versions saved it. - $this->update_option( 'https://example.com/' ); - - $connected_proxy_url->register(); - - $this->assertEquals( 'https://example.com/', $connected_proxy_url->get(), 'Getter should return a legacy plain text value unchanged.' ); + $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(), 'Getter should return FALSE when no value exists.' ); - } - - public function test_get__returns_a_value_that_fails_to_decode_unchanged() { - $connected_proxy_url = new Connected_Proxy_URL( $this->options ); - - $this->update_option( 'not*a*valid*value' ); - - $connected_proxy_url->register(); - - $this->assertEquals( 'not*a*valid*value', $connected_proxy_url->get(), 'Getter should leave a value that fails to decode unchanged.' ); - } - - public function test_sanitize_callback__encodes_a_url_saved_directly() { - $connected_proxy_url = new Connected_Proxy_URL( $this->options ); - $connected_proxy_url->register(); - - // Save the option directly, as an update on the options screen would. - $this->update_option( 'https://example.com' ); - - $this->assertEquals( - base64_encode( 'https://example.com/' ), - $this->options->get( Connected_Proxy_URL::OPTION ), - 'Sanitize callback should encode a value that update_option saves.' - ); - $this->assertEquals( 'https://example.com/', $connected_proxy_url->get(), 'Getter should return the plain URL after a direct save.' ); + $this->assertFalse( $connected_proxy_url->get(), 'The `get()` method should return FALSE when no value exists.' ); } - public function test_matches_url__stored_url_survives_search_and_replace() { + 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(); - $connected_proxy_url->set( 'https://old-site.example.com' ); - - // Simulate a database search and replace of the site URL. - $stored_connected_url = $this->options->get( Connected_Proxy_URL::OPTION ); - $search_replace_result = str_replace( 'old-site.example.com', 'new-site.example.com', $stored_connected_url ); - - $this->assertSame( $stored_connected_url, $search_replace_result, 'Search and replace should not find the URL inside the encoded value.' ); - $this->assertTrue( $connected_proxy_url->matches_url( 'https://old-site.example.com' ), 'Stored URL should still match the original URL after search and replace.' ); - $this->assertFalse( $connected_proxy_url->matches_url( 'https://new-site.example.com' ), "Stored URL shouldn't match the new URL, so Site Kit detects the change." ); - } - - public function test_matches_url__flags_a_url_change_after_a_database_search_and_replace() { - 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 ); - $user_options = new User_Options( $context ); - - $authentication = new Authentication( $context, $this->options, $user_options ); - $authentication->register(); - - // Connect the site with its current URL. - $authentication->get_connected_proxy_url_instance()->set( $context->get_canonical_home_url() ); - - // Simulate proxy credentials and an OAuth access token. - $this->fake_proxy_site_connection(); - $authentication->get_oauth_client()->set_token( array( 'access_token' => 'valid-auth-token' ) ); - - // Grant the administrator the setup capability regardless of authentication state. - add_filter( - 'user_has_cap', - function ( $capabilities ) { - $capabilities[ Permissions::SETUP ] = true; - return $capabilities; - } - ); + $this->options->set( Connected_Proxy_URL::OPTION, 'not*a*valid*value' ); - $connected_host = wp_parse_url( $context->get_canonical_home_url(), PHP_URL_HOST ); - $stored_connected_url = $this->options->get( Connected_Proxy_URL::OPTION ); - - // Simulate a database search and replace: it rewrites the home option, - // and finds no host to rewrite inside the encoded value. - update_option( 'home', 'https://new-domain.example.com' ); - - $this->assertSame( - $stored_connected_url, - str_replace( $connected_host, 'new-domain.example.com', $stored_connected_url ), - 'Search and replace should find no host to rewrite inside the stored value.' - ); - - do_action( 'admin_init' ); - - $this->assertEquals( - 'connected_url_mismatch', - $user_options->get( Disconnected_Reason::OPTION ), - 'Site Kit should flag the URL change after a database search and replace.' - ); + $this->assertFalse( $connected_proxy_url->get(), 'The `get()` method should return FALSE for a stored value that fails to decode.' ); } /** diff --git a/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php b/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php index 1defa3001bf..32920280d14 100644 --- a/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php +++ b/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php @@ -11,13 +11,20 @@ namespace Google\Site_Kit\Tests\Core\Util; use Google\Site_Kit\Context; +use Google\Site_Kit\Core\Authentication\Authentication; use Google\Site_Kit\Core\Authentication\Connected_Proxy_URL; +use Google\Site_Kit\Core\Authentication\Disconnected_Reason; +use Google\Site_Kit\Core\Permissions\Permissions; use Google\Site_Kit\Core\Storage\Options; +use Google\Site_Kit\Core\Storage\User_Options; use Google\Site_Kit\Core\Util\Migration_N_E_X_T; +use Google\Site_Kit\Tests\Fake_Site_Connection_Trait; use Google\Site_Kit\Tests\TestCase; class Migration_N_E_X_TTest extends TestCase { + use Fake_Site_Connection_Trait; + protected Context $context; protected Options $options; protected Connected_Proxy_URL $connected_proxy_url; @@ -29,10 +36,8 @@ public function set_up() { $this->options = new Options( $this->context ); $this->connected_proxy_url = new Connected_Proxy_URL( $this->options ); - // Drop the option and the sanitize filter the bootstrap plugin - // instance added, so each test reads and writes the raw option value. + // Drop the option, so each test starts from the value it stores itself. $this->options->delete( Connected_Proxy_URL::OPTION ); - remove_all_filters( 'sanitize_option_' . Connected_Proxy_URL::OPTION ); $this->delete_db_version(); } @@ -50,7 +55,10 @@ public function test_register() { $migration->register(); - $this->assertTrue( has_action( 'admin_init' ), 'Migration should register the admin_init action.' ); + $this->assertNotFalse( + has_action( 'admin_init', array( $migration, 'migrate' ) ), + 'Migration should register migrate on admin_init.' + ); } public function test_migrate__encodes_a_legacy_plain_text_url() { @@ -69,7 +77,7 @@ public function test_migrate__encodes_a_legacy_plain_text_url() { $this->assertEquals( 'https://example.com/', $this->connected_proxy_url->get(), - 'Getter should still return the plain text URL after the migration.' + 'The `get()` method should still return the plain text URL after the migration.' ); } @@ -88,6 +96,53 @@ public function test_migrate__keeps_an_already_encoded_url() { ); } + public function test_migrate__runs_before_the_connected_proxy_url_check() { + remove_all_actions( 'admin_init' ); + + $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); + wp_set_current_user( $user_id ); + + $user_options = new User_Options( $this->context ); + + // Register in the order the plugin does. The connected proxy URL check + // runs at a later priority, so the migration encodes the stored value + // before the check reads it. + $authentication = new Authentication( $this->context, $this->options, $user_options ); + $authentication->register(); + $this->get_new_migration_instance()->register(); + + // Store the home URL in plain text, the way earlier plugin versions saved it. + $this->options->set( Connected_Proxy_URL::OPTION, $this->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; + } + ); + + do_action( 'admin_init' ); + + $this->assertEquals( + base64_encode( trailingslashit( $this->context->get_canonical_home_url() ) ), + $this->options->get( Connected_Proxy_URL::OPTION ), + 'Migration should encode the plain text URL on admin_init.' + ); + $this->assertFalse( + $user_options->get( Disconnected_Reason::OPTION ), + 'Site Kit should stay connected when the migration encodes the URL the site still runs on.' + ); + } + public function test_migrate__skips_when_the_option_is_missing() { $migration = $this->get_new_migration_instance(); From e35bb46433135b09e9ce816afc0c94df50fe53b4 Mon Sep 17 00:00:00 2001 From: Sherv Elmi Date: Fri, 24 Jul 2026 01:26:33 +0300 Subject: [PATCH 7/7] Fix PHP lint full-stop errors. --- includes/Core/Util/Migration_N_E_X_T.php | 2 +- .../integration/Core/Authentication/AuthenticationTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/Core/Util/Migration_N_E_X_T.php b/includes/Core/Util/Migration_N_E_X_T.php index a8d73b29ebc..13f861b1784 100644 --- a/includes/Core/Util/Migration_N_E_X_T.php +++ b/includes/Core/Util/Migration_N_E_X_T.php @@ -15,7 +15,7 @@ use Google\Site_Kit\Core\Storage\Options; /** - * Class Migration_N_E_X_T + * Class Migration_N_E_X_T. * * @since n.e.x.t * @access private diff --git a/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php b/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php index 6dc80840b2b..7159e863f6f 100644 --- a/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php +++ b/tests/phpunit/integration/Core/Authentication/AuthenticationTest.php @@ -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( 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 + $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() {