Skip to content

Commit 86286ed

Browse files
authored
Merge pull request #1064 from Kit/revoke-access-token
Revoke and Remove Tokens on Disconnect
2 parents 785f491 + 97169f6 commit 86286ed

6 files changed

Lines changed: 161 additions & 11 deletions

File tree

admin/section/class-convertkit-admin-section-general.php

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,35 @@ private function maybe_disconnect() {
193193
return;
194194
}
195195

196-
// Delete Access Token.
196+
// Get Settings class.
197197
$settings = new ConvertKit_Settings();
198-
$settings->delete_credentials();
198+
199+
// Setup API.
200+
$api = new ConvertKit_API_V4(
201+
CONVERTKIT_OAUTH_CLIENT_ID,
202+
CONVERTKIT_OAUTH_CLIENT_REDIRECT_URI,
203+
$settings->get_access_token(),
204+
$settings->get_refresh_token(),
205+
$settings->debug_enabled(),
206+
'settings'
207+
);
208+
209+
// Check that we're using the Kit WordPress Libraries 2.1.4 or higher.
210+
// If another Kit Plugin is active and out of date, its libraries might
211+
// be loaded that don't have this method.
212+
if ( ! method_exists( $api, 'revoke_tokens' ) ) { // @phpstan-ignore-line Older WordPress Libraries won't have this function.
213+
$this->output_error( __( 'The Kit WordPress Libraries is missing the `revoke_tokens` method. Please update all Kit WordPress Plugins to their latest versions, and click Disconnect again.', 'convertkit' ) );
214+
return;
215+
}
216+
217+
// Revoke Access and Refresh Tokens.
218+
// See convertkit_delete_credentials() method in functions.php, which is called
219+
// by the `convertkit_api_revoke_tokens` action and deletes credentials from the Plugin's settings.
220+
$result = $api->revoke_tokens();
221+
if ( is_wp_error( $result ) ) {
222+
$this->output_error( $result->get_error_message() );
223+
return;
224+
}
199225

200226
// Delete cached resources.
201227
$creator_network = new ConvertKit_Resource_Creator_Network_Recommendations();

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"type": "project",
55
"license": "GPLv3",
66
"require": {
7-
"convertkit/convertkit-wordpress-libraries": "2.1.3"
7+
"convertkit/convertkit-wordpress-libraries": "2.1.5"
88
},
99
"require-dev": {
1010
"php-webdriver/webdriver": "^1.0",

includes/class-convertkit-settings.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,9 +667,14 @@ public function delete_credentials() {
667667

668668
$this->save(
669669
array(
670+
// OAuth.
670671
'access_token' => '',
671672
'refresh_token' => '',
672673
'token_expires' => '',
674+
675+
// API Key.
676+
'api_key' => '',
677+
'api_secret' => '',
673678
)
674679
);
675680

includes/functions.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,29 @@ function convertkit_maybe_update_credentials( $result, $client_id ) {
792792

793793
}
794794

795+
/**
796+
* Deletes the stored access token, refresh token and its expiry from the Plugin settings,
797+
* and clears any existing scheduled WordPress Cron event to refresh the token on expiry,
798+
* when the user revokes the access token.
799+
*
800+
* @since 3.2.4
801+
*
802+
* @param string $client_id OAuth Client ID used for the Access and Refresh Tokens.
803+
*/
804+
function convertkit_delete_credentials( $client_id ) {
805+
806+
// Don't delete these credentials if they're not for this Client ID.
807+
// They're for another Kit Plugin that uses OAuth.
808+
if ( $client_id !== CONVERTKIT_OAUTH_CLIENT_ID ) {
809+
return;
810+
}
811+
812+
// Delete Access and Refresh Tokens.
813+
$settings = new ConvertKit_Settings();
814+
$settings->delete_credentials();
815+
816+
}
817+
795818
/**
796819
* Deletes the stored access token, refresh token and its expiry from the Plugin settings,
797820
* and clears any existing scheduled WordPress Cron event to refresh the token on expiry,
@@ -830,6 +853,9 @@ function convertkit_maybe_delete_credentials( $result, $client_id ) {
830853
add_action( 'convertkit_api_get_access_token', 'convertkit_maybe_update_credentials', 10, 2 );
831854
add_action( 'convertkit_api_refresh_token', 'convertkit_maybe_update_credentials', 10, 2 );
832855

856+
// Delete credentials when the user revokes the access and refresh tokens.
857+
add_action( 'convertkit_api_revoke_tokens', 'convertkit_delete_credentials', 10, 1 );
858+
833859
// Delete credentials if the API class uses a invalid access token.
834860
// This prevents the Plugin making repetitive API requests that will 401.
835861
add_action( 'convertkit_api_access_token_invalid', 'convertkit_maybe_delete_credentials', 10, 2 );

tests/EndToEnd/general/plugin-screens/PluginSettingsGeneralCest.php

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,13 +161,47 @@ public function testValidCredentials(EndToEndTester $I)
161161

162162
// Check that no notice is displayed that the API credentials are invalid.
163163
$I->dontSeeErrorNotice($I, 'Kit: Authorization failed. Please connect your Kit account.');
164+
}
165+
166+
/**
167+
* Test that the credentials and resources are deleted on disconnect.
168+
*
169+
* @since 3.2.4
170+
*
171+
* @param EndToEndTester $I Tester.
172+
*/
173+
public function testCredentialsAndResourcesAreDeletedOnDisconnect(EndToEndTester $I)
174+
{
175+
// Setup Plugin.
176+
$I->setupKitPlugin($I);
177+
$I->setupKitPluginResources($I);
164178

165179
// Go to the Plugin's Settings Screen.
166180
$I->loadKitSettingsGeneralScreen($I);
167181

182+
// Fake the API Key, API Secret, Access and Refresh Tokens; if we revoke the tokens used for tests, future tests will fail.
183+
$I->setupKitPlugin(
184+
$I,
185+
[
186+
'access_token' => 'fakeAccessToken',
187+
'refresh_token' => 'fakeRefreshToken',
188+
'token_expires' => time() + 3600,
189+
'api_key' => 'fakeAPIKey',
190+
'api_secret' => 'fakeAPISecret',
191+
]
192+
);
193+
168194
// Disconnect the Plugin connection to Kit.
169195
$I->click('Disconnect');
170196

197+
// Check credentials are removed from the settings.
198+
$settings = $I->grabOptionFromDatabase('_wp_convertkit_settings');
199+
$I->assertEmpty($settings['access_token']);
200+
$I->assertEmpty($settings['refresh_token']);
201+
$I->assertEmpty($settings['token_expires']);
202+
$I->assertEmpty($settings['api_key']);
203+
$I->assertEmpty($settings['api_secret']);
204+
171205
// Check cached resources are removed from the database on disconnection.
172206
$I->dontSeeOptionInDatabase('convertkit_creator_network_recommendations');
173207
$I->dontSeeOptionInDatabase('convertkit_custom_fields');
@@ -182,14 +216,6 @@ public function testValidCredentials(EndToEndTester $I)
182216
$I->see('Connect');
183217
$I->dontSee('Disconnect');
184218
$I->dontSeeElementInDOM('input#submit');
185-
186-
// Check that the option table no longer contains cached resources.
187-
$I->dontSeeOptionInDatabase('convertkit_creator_network_recommendations');
188-
$I->dontSeeOptionInDatabase('convertkit_forms');
189-
$I->dontSeeOptionInDatabase('convertkit_landing_pages');
190-
$I->dontSeeOptionInDatabase('convertkit_posts');
191-
$I->dontSeeOptionInDatabase('convertkit_products');
192-
$I->dontSeeOptionInDatabase('convertkit_tags');
193219
}
194220

195221
/**

tests/Integration/APITest.php

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,73 @@ public function testCronEventCreatedWhenTokenRefreshed()
176176
$this->assertGreaterThanOrEqual( $nextScheduledTimestamp, time() + 10000 );
177177
}
178178

179+
/**
180+
* Test that the access token and refresh token are deleted from the Plugin's settings
181+
* when the access token is revoked.
182+
*
183+
* @since 3.2.4
184+
*/
185+
public function testCredentialsDeletedAndInvalidWhenRevoked()
186+
{
187+
// Initialize the API without an access token or refresh token.
188+
$api = new \ConvertKit_API_V4(
189+
$_ENV['CONVERTKIT_OAUTH_CLIENT_ID'],
190+
$_ENV['KIT_OAUTH_REDIRECT_URI']
191+
);
192+
193+
// Generate an access token by API key and secret.
194+
$result = $api->get_access_token_by_api_key_and_secret(
195+
$_ENV['CONVERTKIT_API_KEY'],
196+
$_ENV['CONVERTKIT_API_SECRET'],
197+
wp_generate_password( 10, false ) // Random tenant name to produce a token for this request only.
198+
);
199+
200+
// Store the access token in the Plugin's settings.
201+
$settings = new \ConvertKit_Settings();
202+
$settings->save(
203+
array(
204+
'access_token' => $result['oauth']['access_token'],
205+
'refresh_token' => $result['oauth']['refresh_token'],
206+
'token_expires' => $result['oauth']['expires_at'],
207+
)
208+
);
209+
210+
// Initialize the API with the access token and refresh token.
211+
$api = new \ConvertKit_API_V4(
212+
$_ENV['CONVERTKIT_OAUTH_CLIENT_ID'],
213+
$_ENV['KIT_OAUTH_REDIRECT_URI'],
214+
$settings->get_access_token(),
215+
$settings->get_refresh_token()
216+
);
217+
218+
// Confirm the token works when making an authenticated request.
219+
$this->assertNotInstanceOf( 'WP_Error', $api->get_account() );
220+
221+
// Revoke the access and refresh tokens.
222+
$api->revoke_tokens();
223+
224+
// Confirm the access token and refresh token are deleted from the Plugin's settings.
225+
$this->assertEmpty( $settings->get_access_token() );
226+
$this->assertEmpty( $settings->get_refresh_token() );
227+
$this->assertEmpty( $settings->get_token_expiry() );
228+
229+
// Initialize the API with the (now revoked) access token and refresh token.
230+
// revoke_tokens() will have removed the access token and refresh token from the API class, so we need to provide them again
231+
// to test they're revoked.
232+
$api = new \ConvertKit_API_V4(
233+
$_ENV['CONVERTKIT_OAUTH_CLIENT_ID'],
234+
$_ENV['CONVERTKIT_OAUTH_REDIRECT_URI'],
235+
$result['oauth']['access_token'],
236+
$result['oauth']['refresh_token']
237+
);
238+
239+
// Confirm attempting to use the revoked access token no longer works.
240+
$this->assertInstanceOf( 'WP_Error', $api->get_account() );
241+
242+
// Confirm attempting to use the revoked refresh token no longer works.
243+
$this->assertInstanceOf( 'WP_Error', $api->refresh_token() );
244+
}
245+
179246
/**
180247
* Mocks an API response as if the Access Token expired.
181248
*

0 commit comments

Comments
 (0)