From 3c29ac8d7efcb35565405765b1bb79d541aee7de Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 13 Nov 2019 20:08:41 +0200 Subject: [PATCH 1/9] lazy-load profile data by moving to get --- includes/Core/Authentication/Profile.php | 25 +++++++++++------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/includes/Core/Authentication/Profile.php b/includes/Core/Authentication/Profile.php index e88386c16cb..cc4d7ab9fad 100644 --- a/includes/Core/Authentication/Profile.php +++ b/includes/Core/Authentication/Profile.php @@ -54,19 +54,6 @@ final class Profile { public function __construct( User_Options $user_options, OAuth_Client $auth_client ) { $this->user_options = $user_options; $this->auth_client = $auth_client; - - // Ensure we have fresh profile data. - $profile_data = $this->get(); - $timestamp = isset( $profile_data['timestamp'] ) ? (int) $profile_data['timestamp'] : 0; - $currenttime = time(); - - // If the stored profile data is missing, or older than a week, re-fetch it. - if ( ! $profile_data || ( ( $currenttime - $timestamp ) > ( 7 * DAY_IN_SECONDS ) ) ) { - $profile_data = $this->retrieve_google_profile_from_api(); - } - if ( 0 !== $profile_data['timestamp'] ) { - $this->set( $profile_data ); - } } /** @@ -77,7 +64,17 @@ public function __construct( User_Options $user_options, OAuth_Client $auth_clie * @return array|bool Value set for the profile, or false if not set. */ public function get() { - return $this->user_options->get( self::OPTION ); + // Ensure we have fresh profile data. + $profile_data = $this->user_options->get( self::OPTION ); + $profile_time = isset( $profile_data['timestamp'] ) ? (int) $profile_data['timestamp'] : 0; + $current_time = current_time( 'timestamp' ); + + // If the stored profile data is missing, or older than a week, re-fetch it. + if ( ! $profile_data || ( $current_time - $profile_time ) > WEEK_IN_SECONDS ) { + $profile_data = $this->retrieve_google_profile_from_api(); + } + + return $profile_data; } /** From ec3388d81a8e7ad925aa6c94c1c96776231b46f8 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 13 Nov 2019 20:09:43 +0200 Subject: [PATCH 2/9] update profile fetch to save data on success --- includes/Core/Authentication/Profile.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/includes/Core/Authentication/Profile.php b/includes/Core/Authentication/Profile.php index cc4d7ab9fad..8d08467abb9 100644 --- a/includes/Core/Authentication/Profile.php +++ b/includes/Core/Authentication/Profile.php @@ -126,14 +126,14 @@ private function retrieve_google_profile_from_api() { $people_service = new Google_Service_PeopleService( $client ); $profile = $people_service->people->get( 'people/me', array( 'personFields' => 'emailAddresses,photos' ) ); - if ( isset( $profile['emailAddresses'][0]['value'] ) && isset( $profile['photos'][0]['url'] ) ) { - - // Success - we have the profile data from the People API. + if ( isset( $profile['emailAddresses'][0]['value'], $profile['photos'][0]['url'] ) ) { $profile_data = array( 'email' => $profile['emailAddresses'][0]['value'], 'photo' => $profile['photos'][0]['url'], - 'timestamp' => time(), + 'timestamp' => current_time( 'timestamp' ), ); + + $this->set( $profile_data ); } } catch ( \Exception $e ) { return $profile_data; From 4376a5e641875d47c50909b6f5abcdfb36bd57d2 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 13 Nov 2019 20:11:02 +0200 Subject: [PATCH 3/9] remove fallback profile data profile should always be from authenticated Google user --- includes/Core/Authentication/Profile.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/includes/Core/Authentication/Profile.php b/includes/Core/Authentication/Profile.php index 8d08467abb9..91e74a9af44 100644 --- a/includes/Core/Authentication/Profile.php +++ b/includes/Core/Authentication/Profile.php @@ -112,13 +112,7 @@ public function has() { */ private function retrieve_google_profile_from_api() { - // Fall back to the user's WordPress profile information. - $current_user = wp_get_current_user(); - $profile_data = array( - 'email' => $current_user->user_email, - 'photo' => get_avatar_url( $current_user->user_email ), - 'timestamp' => 0, // Don't cache WP user data. - ); + $profile_data = false; // Retrieve and store the user's Google profile data. try { From fbe6b4465975f5849693c27d01d5c267d88e1d35 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 13 Nov 2019 20:14:39 +0200 Subject: [PATCH 4/9] remove old non-existent import --- includes/Core/Authentication/Profile.php | 1 - 1 file changed, 1 deletion(-) diff --git a/includes/Core/Authentication/Profile.php b/includes/Core/Authentication/Profile.php index 91e74a9af44..8add8611e1e 100644 --- a/includes/Core/Authentication/Profile.php +++ b/includes/Core/Authentication/Profile.php @@ -10,7 +10,6 @@ namespace Google\Site_Kit\Core\Authentication; -use Google\Site_Kit\Helpers; use Google\Site_Kit\Core\Storage\User_Options; use Google\Site_Kit\Core\Authentication\Clients\OAuth_Client; use Google\Site_Kit_Dependencies\Google_Service_PeopleService; From 63e313ae55e990f3f81aa447dd4b7e36e789154b Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 13 Nov 2019 20:15:12 +0200 Subject: [PATCH 5/9] update ProfileTest for changes to get --- .../Core/Authentication/ProfileTest.php | 50 +++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/integration/Core/Authentication/ProfileTest.php b/tests/phpunit/integration/Core/Authentication/ProfileTest.php index 873bd90e493..ee23e973cf5 100644 --- a/tests/phpunit/integration/Core/Authentication/ProfileTest.php +++ b/tests/phpunit/integration/Core/Authentication/ProfileTest.php @@ -14,7 +14,10 @@ use Google\Site_Kit\Core\Authentication\Clients\OAuth_Client; use Google\Site_Kit\Core\Authentication\Profile; use Google\Site_Kit\Core\Storage\User_Options; +use Google\Site_Kit\Tests\FakeHttpClient; use Google\Site_Kit\Tests\TestCase; +use Google\Site_Kit_Dependencies\GuzzleHttp\Message\Response; +use Google\Site_Kit_Dependencies\GuzzleHttp\Stream\Stream; /** * @group Authentication @@ -28,12 +31,51 @@ public function test_get() { $client = new OAuth_Client( new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ) ); $profile = new Profile( $user_options, $client ); + // Profile data is always an array with email, photo, and a timestamp otherwise false. $this->assertFalse( $profile->get() ); - // get() is a simple wrapper for fetching the option value. - $user_options->set( Profile::OPTION, 'test-profile' ); + $valid_profile_data = array( + 'email' => 'user@foo.com', + 'photo' => 'https://example.com/me.jpg', + 'timestamp' => current_time( 'timestamp' ) - DAY_IN_SECONDS, + ); + $user_options->set( Profile::OPTION, $valid_profile_data ); + + $this->assertEquals( $valid_profile_data, $profile->get() ); + + // If there is no data, or the timestamp is older than 1 week it attempts to fetch new data. + $stale_profile_data = $valid_profile_data; + $stale_profile_data['timestamp'] = current_time( 'timestamp' ) - WEEK_IN_SECONDS - MINUTE_IN_SECONDS; + update_user_option( $user_id, Profile::OPTION, $stale_profile_data ); + + // Stub the response to return fresh profile data from the API. + $fake_http = new FakeHttpClient(); + $fake_http->set_request_handler( function () { + return new Response( + 200, + array(), + Stream::factory(json_encode(array( + // ['emailAddresses'][0]['value'] + 'emailAddresses' => array( + array( 'value' => 'fresh@foo.com' ), + ), + // ['photos'][0]['url'] + 'photos' => array( + array( 'url' => 'https://example.com/fresh.jpg' ), + ), + ))) + ); + }); + $client->get_client()->setHttpClient( $fake_http ); - $this->assertEquals( 'test-profile', $profile->get() ); + $fresh_profile_data = $profile->get(); + + $this->assertEquals( 'fresh@foo.com', $fresh_profile_data['email'] ); + $this->assertEquals( 'https://example.com/fresh.jpg', $fresh_profile_data['photo'] ); + $this->assertGreaterThan( + $valid_profile_data['timestamp'], + $fresh_profile_data['timestamp'] + ); } public function test_has() { @@ -58,6 +100,8 @@ public function test_has() { $user_options->set( Profile::OPTION, array( 'email' => '', 'photo' => 'test-photo.jpg' ) ); $this->assertFalse( $profile->has() ); $user_options->set( Profile::OPTION, array( 'email' => 'user@example.com', 'photo' => 'test-photo.jpg' ) ); + $this->assertFalse( $profile->has() ); + $user_options->set( Profile::OPTION, array( 'email' => 'user@example.com', 'photo' => 'test-photo.jpg', 'timestamp' => current_time( 'timestamp' ) ) ); $this->assertTrue( $profile->has() ); } From bd0526dab2e0291502fc96d5a7892bc13f74b112 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 13 Nov 2019 20:29:51 +0200 Subject: [PATCH 6/9] don't bail out in revoke_token unnecessarily --- includes/Core/Authentication/Clients/OAuth_Client.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/includes/Core/Authentication/Clients/OAuth_Client.php b/includes/Core/Authentication/Clients/OAuth_Client.php index 4e6b8c27651..944212442bf 100644 --- a/includes/Core/Authentication/Clients/OAuth_Client.php +++ b/includes/Core/Authentication/Clients/OAuth_Client.php @@ -287,13 +287,8 @@ public function refresh_token() { * @since 1.0.0 */ public function revoke_token() { - // Stop if google_client not initialized yet. - if ( ! $this->google_client instanceof Google_Client ) { - return; - } - try { - $this->google_client->revokeToken(); + $this->get_client()->revokeToken(); } catch ( \Exception $e ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement // No special handling, we just need to make sure this goes through. } From 0414e7414eae00c564c3c56e8e373f3f2dda1440 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Thu, 14 Nov 2019 13:44:14 +0200 Subject: [PATCH 7/9] update error in refresh_token test --- .../Core/Authentication/Clients/OAuth_ClientTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/phpunit/integration/Core/Authentication/Clients/OAuth_ClientTest.php b/tests/phpunit/integration/Core/Authentication/Clients/OAuth_ClientTest.php index 26c71368fd6..5091cebeee1 100644 --- a/tests/phpunit/integration/Core/Authentication/Clients/OAuth_ClientTest.php +++ b/tests/phpunit/integration/Core/Authentication/Clients/OAuth_ClientTest.php @@ -49,8 +49,7 @@ public function test_refresh_token() { $client->refresh_token(); - // Google client must be initialized first - $this->assertEquals( 'refresh_token_not_exist', get_user_option( OAuth_Client::OPTION_ERROR_CODE, $user_id ) ); + $this->assertEquals( 'access_token_not_received', get_user_option( OAuth_Client::OPTION_ERROR_CODE, $user_id ) ); $client->get_client()->setHttpClient( new FakeHttpClient() ); $client->refresh_token(); From 487602175b5e5cddb249170ca8b94738ec86c29e Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 14 Nov 2019 22:18:42 +0530 Subject: [PATCH 8/9] Bump plugin version to 1.0.4. --- google-site-kit.php | 4 ++-- readme.txt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/google-site-kit.php b/google-site-kit.php index 7fb82289ade..ff49bab415e 100644 --- a/google-site-kit.php +++ b/google-site-kit.php @@ -11,7 +11,7 @@ * Plugin Name: Site Kit by Google * Plugin URI: https://sitekit.withgoogle.com * Description: Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web. - * Version: 1.0.3 + * Version: 1.0.4 * Author: Google * Author URI: https://opensource.google.com * License: Apache License 2.0 @@ -24,7 +24,7 @@ } // Define most essential constants. -define( 'GOOGLESITEKIT_VERSION', '1.0.3' ); +define( 'GOOGLESITEKIT_VERSION', '1.0.4' ); define( 'GOOGLESITEKIT_PLUGIN_MAIN_FILE', __FILE__ ); /** diff --git a/readme.txt b/readme.txt index 77aeb7815b8..c0583daf9af 100644 --- a/readme.txt +++ b/readme.txt @@ -4,7 +4,7 @@ Contributors: google Requires at least: 4.7 Tested up to: 5.3 Requires PHP: 5.4 -Stable tag: 1.0.3 +Stable tag: 1.0.4 License: Apache License 2.0 License URI: https://www.apache.org/licenses/LICENSE-2.0 Tags: google, search-console, analytics, adsense, pagespeed-insights, optimize, tag-manager, site-kit From af294103b4ec79c4ef82ad3f3ddc0f3eeef900e7 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Thu, 14 Nov 2019 18:47:02 +0200 Subject: [PATCH 9/9] set default userData from current logged in user --- includes/Core/Authentication/Authentication.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/includes/Core/Authentication/Authentication.php b/includes/Core/Authentication/Authentication.php index 4af01cb2615..876a6706551 100644 --- a/includes/Core/Authentication/Authentication.php +++ b/includes/Core/Authentication/Authentication.php @@ -466,7 +466,11 @@ private function refresh_auth_token_on_login() { */ private function inline_js_admin_data( $data ) { if ( ! isset( $data['userData'] ) ) { - $data['userData'] = array(); + $current_user = wp_get_current_user(); + $data['userData'] = array( + 'email' => $current_user->user_email, + 'picture' => get_avatar_url( $current_user->user_email ), + ); } $profile_data = $this->profile->get(); if ( $profile_data ) {