From 9199aa2000485d54bfc8bc7e78a18713cbc1f232 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Mon, 10 Nov 2025 16:12:31 +0800 Subject: [PATCH 1/8] Settings: Delete Invalid Access Tokens --- composer.json | 2 +- includes/class-convertkit-settings.php | 34 +++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 28283fd3f..c0941c953 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,7 @@ "type": "project", "license": "GPLv3", "require": { - "convertkit/convertkit-wordpress-libraries": "2.1.0" + "convertkit/convertkit-wordpress-libraries": "dev-add-hooks-on-request-error" }, "require-dev": { "php-webdriver/webdriver": "^1.0", diff --git a/includes/class-convertkit-settings.php b/includes/class-convertkit-settings.php index ae83d09dd..dc7a72320 100644 --- a/includes/class-convertkit-settings.php +++ b/includes/class-convertkit-settings.php @@ -49,6 +49,11 @@ public function __construct() { add_action( 'convertkit_api_get_access_token', array( $this, 'update_credentials' ), 10, 2 ); add_action( 'convertkit_api_refresh_token', array( $this, 'update_credentials' ), 10, 2 ); + // Delete credentials if the API class fails to refresh the token or uses a invalid access token. + // This prevents the Plugin making repetitive API requests that will 401. + add_action( 'convertkit_api_request_error_access_token_invalid', array( $this, 'maybe_delete_credentials' ), 10, 1 ); + add_action( 'convertkit_api_request_error_refresh_token_failed', array( $this, 'maybe_delete_credentials' ), 10, 1 ); + } /** @@ -646,7 +651,31 @@ public function update_credentials( $result, $client_id ) { } /** - * Deletes any existing access token, refresh token and its expiry from the Plugin settings. + * Deletes the stored access token, refresh token and its expiry from the Plugin settings, + * and clears any existing scheduled WordPress Cron event to refresh the token on expiry, + * when either: + * - The access token is invalid + * - The access token expired, and refreshing failed + * + * @since 3.1.0 + * + * @param string $client_id OAuth Client ID used for the Access and Refresh Tokens. + */ + public function maybe_delete_credentials( $client_id ) { + + // Don't delete these credentials if they're not for this Client ID. + // They're for another Kit Plugin that uses OAuth. + if ( $client_id !== CONVERTKIT_OAUTH_CLIENT_ID ) { + return; + } + + $this->delete_credentials(); + + } + + /** + * Deletes any existing access token, refresh token and its expiry from the Plugin settings, + * and clears any existing scheduled WordPress Cron event to refresh the token on expiry. * * @since 2.5.0 */ @@ -660,6 +689,9 @@ public function delete_credentials() { ) ); + // Clear any existing scheduled WordPress Cron event. + wp_clear_scheduled_hook( 'convertkit_refresh_token' ); + } /** From 1d479ace7a65fbcbd26dea50425676145d21e5b5 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Mon, 10 Nov 2025 17:46:16 +0800 Subject: [PATCH 2/8] Update hooks --- includes/class-convertkit-settings.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/includes/class-convertkit-settings.php b/includes/class-convertkit-settings.php index dc7a72320..0ae3978d5 100644 --- a/includes/class-convertkit-settings.php +++ b/includes/class-convertkit-settings.php @@ -51,8 +51,8 @@ public function __construct() { // Delete credentials if the API class fails to refresh the token or uses a invalid access token. // This prevents the Plugin making repetitive API requests that will 401. - add_action( 'convertkit_api_request_error_access_token_invalid', array( $this, 'maybe_delete_credentials' ), 10, 1 ); - add_action( 'convertkit_api_request_error_refresh_token_failed', array( $this, 'maybe_delete_credentials' ), 10, 1 ); + add_action( 'convertkit_api_access_token_invalid', array( $this, 'maybe_delete_credentials' ), 10, 2 ); + add_action( 'convertkit_api_refresh_token_error', array( $this, 'maybe_delete_credentials' ), 10, 2 ); } @@ -659,9 +659,10 @@ public function update_credentials( $result, $client_id ) { * * @since 3.1.0 * - * @param string $client_id OAuth Client ID used for the Access and Refresh Tokens. + * @param WP_Error $result Error result. + * @param string $client_id OAuth Client ID used for the Access and Refresh Tokens. */ - public function maybe_delete_credentials( $client_id ) { + public function maybe_delete_credentials( $result, $client_id ) { // Don't delete these credentials if they're not for this Client ID. // They're for another Kit Plugin that uses OAuth. From 0fefe4a8f9b910fb4ea3b3c761c35a14f44143e0 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Mon, 10 Nov 2025 17:48:04 +0800 Subject: [PATCH 3/8] Added integration tests --- tests/Integration/APITest.php | 92 +++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tests/Integration/APITest.php b/tests/Integration/APITest.php index 11137c893..b74cf7fd6 100644 --- a/tests/Integration/APITest.php +++ b/tests/Integration/APITest.php @@ -82,12 +82,104 @@ public function testAccessTokenRefreshedAndSavedWhenExpired() // Run request, which will trigger the above filters as if the token expired and refreshes automatically. $result = $this->api->get_account(); + // Confirm no WP_Error is returned. + $this->assertNotInstanceOf( 'WP_Error', $result ); + $this->assertIsArray( $result ); + // Confirm "new" tokens now exist in the Plugin's settings, which confirms the `convertkit_api_refresh_token` hook was called when // the tokens were refreshed. $this->assertEquals( $settings->get_access_token(), $_ENV['CONVERTKIT_OAUTH_ACCESS_TOKEN'] ); $this->assertEquals( $settings->get_refresh_token(), $_ENV['CONVERTKIT_OAUTH_REFRESH_TOKEN'] ); } + /** + * Test that the Access Token, Refresh Token and Token Expiry are deleted from the Plugin's settings + * when the Refresh Token used fails to refresh the Access Token. + * + * @since 3.1.0 + */ + public function testAccessTokenDeletedWhenRefreshFails() + { + // Save an invalid access token and refresh token in the Plugin's settings. + $settings = new \ConvertKit_Settings(); + $settings->save( + array( + 'access_token' => $_ENV['CONVERTKIT_OAUTH_ACCESS_TOKEN'], + 'refresh_token' => 'invalidRefreshToken', + 'token_expires' => time() + 10000, + ) + ); + + // Confirm the tokens saved. + $this->assertEquals( $settings->get_access_token(), $_ENV['CONVERTKIT_OAUTH_ACCESS_TOKEN'] ); + $this->assertEquals( $settings->get_refresh_token(), 'invalidRefreshToken' ); + + // Initialize the API using the invalid refresh token. + $api = new \ConvertKit_API_V4( + $_ENV['CONVERTKIT_OAUTH_CLIENT_ID'], + $_ENV['KIT_OAUTH_REDIRECT_URI'], + $settings->get_access_token(), + $settings->get_refresh_token() + ); + + // Run request to refresh the token. + $result = $api->refresh_token(); + + // Confirm a WP_Error is returned. + $this->assertInstanceOf( 'WP_Error', $result ); + $this->assertEquals( $result->get_error_code(), 'convertkit_api_error' ); + $this->assertEquals( $result->get_error_message(), 'The provided authorization grant is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client.' ); + + // Confirm tokens removed from the Plugin's settings, which confirms the `convertkit_api_refresh_token_error` hook was called when the tokens were deleted. + $this->assertEmpty( $settings->get_access_token() ); + $this->assertEmpty( $settings->get_refresh_token() ); + $this->assertEmpty( $settings->get_token_expiry() ); + } + + /** + * Test that the Access Token, Refresh Token and Token Expiry are deleted from the Plugin's settings + * when the Access Token used is invalid. + * + * @since 3.1.0 + */ + public function testAccessTokenDeletedWhenInvalid() + { + // Save an invalid access token and refresh token in the Plugin's settings. + $settings = new \ConvertKit_Settings(); + $settings->save( + array( + 'access_token' => 'invalidAccessToken', + 'refresh_token' => $_ENV['CONVERTKIT_OAUTH_REFRESH_TOKEN'], + 'token_expires' => time() + 10000, + ) + ); + + // Confirm the tokens saved. + $this->assertEquals( $settings->get_access_token(), 'invalidAccessToken' ); + $this->assertEquals( $settings->get_refresh_token(), $_ENV['CONVERTKIT_OAUTH_REFRESH_TOKEN'] ); + + // Initialize the API using the invalid refresh token. + $api = new \ConvertKit_API_V4( + $_ENV['CONVERTKIT_OAUTH_CLIENT_ID'], + $_ENV['KIT_OAUTH_REDIRECT_URI'], + $settings->get_access_token(), + $settings->get_refresh_token() + ); + + // Run request. + $result = $api->get_account(); + + // Confirm a WP_Error is returned. + $this->assertInstanceOf( 'WP_Error', $result ); + $this->assertEquals( $result->get_error_code(), 'convertkit_api_error' ); + $this->assertEquals( $result->get_error_message(), 'The access token is invalid' ); + + // Confirm tokens removed from the Plugin's settings, which confirms the `convertkit_api_access_token_invalid` hook was called when the tokens were deleted. + $this->assertEmpty( $settings->get_access_token() ); + $this->assertEmpty( $settings->get_refresh_token() ); + $this->assertEmpty( $settings->get_token_expiry() ); + } + /** * Test that a WordPress Cron event is created when an access token is obtained. * From 5cc94bec1e363c69d7b8dcca70789ba0f166a765 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Mon, 10 Nov 2025 20:37:19 +0800 Subject: [PATCH 4/8] Notices: Persist authorization notice based on token success/error hooks --- ...class-convertkit-admin-section-general.php | 40 ++++++------------- includes/class-convertkit-settings.php | 7 ++++ 2 files changed, 19 insertions(+), 28 deletions(-) diff --git a/admin/section/class-convertkit-admin-section-general.php b/admin/section/class-convertkit-admin-section-general.php index 837ac1b90..bfb383099 100644 --- a/admin/section/class-convertkit-admin-section-general.php +++ b/admin/section/class-convertkit-admin-section-general.php @@ -133,7 +133,17 @@ private function check_credentials() { // Bail if no access and refresh token exist. if ( ! $this->settings->has_access_and_refresh_token() ) { - return; + // Redirect to General screen, which will now show the ConvertKit_Admin_Section_OAuth screen, because + // the Plugin has no access token. + wp_safe_redirect( + add_query_arg( + array( + 'page' => $this->settings_key, + ), + 'options-general.php' + ) + ); + exit(); } // Initialize the API. @@ -152,37 +162,11 @@ private function check_credentials() { // If the request succeeded, no need to perform further actions. if ( ! is_wp_error( $this->account ) ) { - // Remove any existing persistent notice. - WP_ConvertKit()->get_class( 'admin_notices' )->delete( 'authorization_failed' ); - return; } - // Depending on the error code, maybe persist a notice in the WordPress Administration until the user + // Depending on the error code, display an error notice in the settings screen until the user // fixes the problem. - switch ( $this->account->get_error_data( $this->account->get_error_code() ) ) { - case 401: - // Access token either expired or was revoked in ConvertKit. - // Remove from settings. - $this->settings->delete_credentials(); - - // Display a site wide notice. - WP_ConvertKit()->get_class( 'admin_notices' )->add( 'authorization_failed' ); - - // Redirect to General screen, which will now show the ConvertKit_Admin_Section_OAuth screen, because - // the Plugin has no access token. - wp_safe_redirect( - add_query_arg( - array( - 'page' => $this->settings_key, - ), - 'options-general.php' - ) - ); - exit(); - } - - // Output a non-401 error now. $this->output_error( $this->account->get_error_message() ); } diff --git a/includes/class-convertkit-settings.php b/includes/class-convertkit-settings.php index 0ae3978d5..dd45760ba 100644 --- a/includes/class-convertkit-settings.php +++ b/includes/class-convertkit-settings.php @@ -634,6 +634,9 @@ public function update_credentials( $result, $client_id ) { return; } + // Remove any existing persistent notice. + WP_ConvertKit()->get_class( 'admin_notices' )->delete( 'authorization_failed' ); + $this->save( array( 'access_token' => $result['access_token'], @@ -670,6 +673,10 @@ public function maybe_delete_credentials( $result, $client_id ) { return; } + // Persist an error notice in the WordPress Administration until the user fixes the problem. + WP_ConvertKit()->get_class( 'admin_notices' )->add( 'authorization_failed' ); + + // Delete the credentials from the Plugin settings. $this->delete_credentials(); } From aafda834943b4de5fd8111a0b9d9bc5d5f0943e9 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Mon, 10 Nov 2025 21:11:57 +0800 Subject: [PATCH 5/8] Tests: Reset persistent notices between tests --- tests/Support/Helper/KitPlugin.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Support/Helper/KitPlugin.php b/tests/Support/Helper/KitPlugin.php index 62ef8efce..72f8f25ef 100644 --- a/tests/Support/Helper/KitPlugin.php +++ b/tests/Support/Helper/KitPlugin.php @@ -531,6 +531,9 @@ public function resetKitPlugin($I) $I->dontHaveOptionInDatabase('convertkit_tags'); $I->dontHaveOptionInDatabase('convertkit_tags_last_queried'); + // Persistent notices. + $I->dontHaveOptionInDatabase('convertkit-admin-notices'); + // Review Request. $I->dontHaveOptionInDatabase('convertkit-review-request'); $I->dontHaveOptionInDatabase('convertkit-review-dismissed'); From 357425db0e24668477a9cde21b3463fc7a6dabbb Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Tue, 11 Nov 2025 14:40:39 +0800 Subject: [PATCH 6/8] Move Admin_Notices class to global This ensures it can be accessed when a non-admin request refreshes or deletes an access token. --- {admin => includes}/class-convertkit-admin-notices.php | 0 includes/class-wp-convertkit.php | 2 +- wp-convertkit.php | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename {admin => includes}/class-convertkit-admin-notices.php (100%) diff --git a/admin/class-convertkit-admin-notices.php b/includes/class-convertkit-admin-notices.php similarity index 100% rename from admin/class-convertkit-admin-notices.php rename to includes/class-convertkit-admin-notices.php diff --git a/includes/class-wp-convertkit.php b/includes/class-wp-convertkit.php index 135757263..840aa306e 100644 --- a/includes/class-wp-convertkit.php +++ b/includes/class-wp-convertkit.php @@ -83,7 +83,6 @@ private function initialize_admin() { $this->classes['admin_cache_plugins'] = new ConvertKit_Admin_Cache_Plugins(); $this->classes['admin_category'] = new ConvertKit_Admin_Category(); $this->classes['admin_landing_page'] = new ConvertKit_Admin_Landing_Page(); - $this->classes['admin_notices'] = new ConvertKit_Admin_Notices(); $this->classes['admin_post'] = new ConvertKit_Admin_Post(); $this->classes['admin_quick_edit'] = new ConvertKit_Admin_Quick_Edit(); $this->classes['admin_refresh_resources'] = new ConvertKit_Admin_Refresh_Resources(); @@ -178,6 +177,7 @@ private function initialize_frontend() { */ private function initialize_global() { + $this->classes['admin_notices'] = new ConvertKit_Admin_Notices(); $this->classes['ajax'] = new ConvertKit_AJAX(); $this->classes['blocks_convertkit_broadcasts'] = new ConvertKit_Block_Broadcasts(); $this->classes['blocks_convertkit_content'] = new ConvertKit_Block_Content(); diff --git a/wp-convertkit.php b/wp-convertkit.php index 3be2fb71f..e72dd385d 100644 --- a/wp-convertkit.php +++ b/wp-convertkit.php @@ -52,6 +52,7 @@ require_once CONVERTKIT_PLUGIN_PATH . '/includes/cron-functions.php'; require_once CONVERTKIT_PLUGIN_PATH . '/includes/functions.php'; require_once CONVERTKIT_PLUGIN_PATH . '/includes/class-wp-convertkit.php'; +require_once CONVERTKIT_PLUGIN_PATH . '/includes/class-convertkit-admin-notices.php'; require_once CONVERTKIT_PLUGIN_PATH . '/includes/class-convertkit-ajax.php'; require_once CONVERTKIT_PLUGIN_PATH . '/includes/class-convertkit-broadcasts-exporter.php'; require_once CONVERTKIT_PLUGIN_PATH . '/includes/class-convertkit-broadcasts-importer.php'; @@ -108,7 +109,6 @@ require_once CONVERTKIT_PLUGIN_PATH . '/admin/class-convertkit-admin-cache-plugins.php'; require_once CONVERTKIT_PLUGIN_PATH . '/admin/class-convertkit-admin-category.php'; require_once CONVERTKIT_PLUGIN_PATH . '/admin/class-convertkit-admin-landing-page.php'; -require_once CONVERTKIT_PLUGIN_PATH . '/admin/class-convertkit-admin-notices.php'; require_once CONVERTKIT_PLUGIN_PATH . '/admin/class-convertkit-admin-post.php'; require_once CONVERTKIT_PLUGIN_PATH . '/admin/class-convertkit-admin-refresh-resources.php'; require_once CONVERTKIT_PLUGIN_PATH . '/admin/class-convertkit-admin-restrict-content.php'; From df2f6c3fe42792bf6c47ac6c59efbb984ec647b4 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Thu, 13 Nov 2025 11:09:50 +0800 Subject: [PATCH 7/8] Remove `convertkit_api_refresh_token_error` hook --- includes/class-convertkit-settings.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/includes/class-convertkit-settings.php b/includes/class-convertkit-settings.php index dd45760ba..d74a5e47c 100644 --- a/includes/class-convertkit-settings.php +++ b/includes/class-convertkit-settings.php @@ -49,10 +49,9 @@ public function __construct() { add_action( 'convertkit_api_get_access_token', array( $this, 'update_credentials' ), 10, 2 ); add_action( 'convertkit_api_refresh_token', array( $this, 'update_credentials' ), 10, 2 ); - // Delete credentials if the API class fails to refresh the token or uses a invalid access token. + // Delete credentials if the API class uses a invalid access token. // This prevents the Plugin making repetitive API requests that will 401. add_action( 'convertkit_api_access_token_invalid', array( $this, 'maybe_delete_credentials' ), 10, 2 ); - add_action( 'convertkit_api_refresh_token_error', array( $this, 'maybe_delete_credentials' ), 10, 2 ); } From 4a183a3d2f3bacf8ae2db3695b4482df54928ff1 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Thu, 13 Nov 2025 11:31:28 +0800 Subject: [PATCH 8/8] Remove redundant test --- tests/Integration/APITest.php | 44 ----------------------------------- 1 file changed, 44 deletions(-) diff --git a/tests/Integration/APITest.php b/tests/Integration/APITest.php index b74cf7fd6..c601522d1 100644 --- a/tests/Integration/APITest.php +++ b/tests/Integration/APITest.php @@ -92,50 +92,6 @@ public function testAccessTokenRefreshedAndSavedWhenExpired() $this->assertEquals( $settings->get_refresh_token(), $_ENV['CONVERTKIT_OAUTH_REFRESH_TOKEN'] ); } - /** - * Test that the Access Token, Refresh Token and Token Expiry are deleted from the Plugin's settings - * when the Refresh Token used fails to refresh the Access Token. - * - * @since 3.1.0 - */ - public function testAccessTokenDeletedWhenRefreshFails() - { - // Save an invalid access token and refresh token in the Plugin's settings. - $settings = new \ConvertKit_Settings(); - $settings->save( - array( - 'access_token' => $_ENV['CONVERTKIT_OAUTH_ACCESS_TOKEN'], - 'refresh_token' => 'invalidRefreshToken', - 'token_expires' => time() + 10000, - ) - ); - - // Confirm the tokens saved. - $this->assertEquals( $settings->get_access_token(), $_ENV['CONVERTKIT_OAUTH_ACCESS_TOKEN'] ); - $this->assertEquals( $settings->get_refresh_token(), 'invalidRefreshToken' ); - - // Initialize the API using the invalid refresh token. - $api = new \ConvertKit_API_V4( - $_ENV['CONVERTKIT_OAUTH_CLIENT_ID'], - $_ENV['KIT_OAUTH_REDIRECT_URI'], - $settings->get_access_token(), - $settings->get_refresh_token() - ); - - // Run request to refresh the token. - $result = $api->refresh_token(); - - // Confirm a WP_Error is returned. - $this->assertInstanceOf( 'WP_Error', $result ); - $this->assertEquals( $result->get_error_code(), 'convertkit_api_error' ); - $this->assertEquals( $result->get_error_message(), 'The provided authorization grant is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client.' ); - - // Confirm tokens removed from the Plugin's settings, which confirms the `convertkit_api_refresh_token_error` hook was called when the tokens were deleted. - $this->assertEmpty( $settings->get_access_token() ); - $this->assertEmpty( $settings->get_refresh_token() ); - $this->assertEmpty( $settings->get_token_expiry() ); - } - /** * Test that the Access Token, Refresh Token and Token Expiry are deleted from the Plugin's settings * when the Access Token used is invalid.