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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions admin/section/class-convertkit-admin-section-general.php
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,35 @@ private function maybe_disconnect() {
return;
}

// Delete Access Token.
// Get Settings class.
$settings = new ConvertKit_Settings();
$settings->delete_credentials();

// Setup API.
$api = new ConvertKit_API_V4(
CONVERTKIT_OAUTH_CLIENT_ID,
CONVERTKIT_OAUTH_CLIENT_REDIRECT_URI,
$settings->get_access_token(),
$settings->get_refresh_token(),
$settings->debug_enabled(),
'settings'
);

// Check that we're using the Kit WordPress Libraries 2.1.4 or higher.
// If another Kit Plugin is active and out of date, its libraries might
// be loaded that don't have this method.
if ( ! method_exists( $api, 'revoke_tokens' ) ) { // @phpstan-ignore-line Older WordPress Libraries won't have this function.
$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' ) );
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nice error message, thank you!

return;
}

// Revoke Access and Refresh Tokens.
// See convertkit_delete_credentials() method in functions.php, which is called
// by the `convertkit_api_revoke_tokens` action and deletes credentials from the Plugin's settings.
$result = $api->revoke_tokens();
if ( is_wp_error( $result ) ) {
$this->output_error( $result->get_error_message() );
return;
}

// Delete cached resources.
$creator_network = new ConvertKit_Resource_Creator_Network_Recommendations();
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"type": "project",
"license": "GPLv3",
"require": {
"convertkit/convertkit-wordpress-libraries": "2.1.3"
"convertkit/convertkit-wordpress-libraries": "dev-add-revoke-token-method"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't know anything about WP so just double-checking we want this and it wasn't accidentally left in from testing

},
"require-dev": {
"php-webdriver/webdriver": "^1.0",
Expand Down
5 changes: 5 additions & 0 deletions includes/class-convertkit-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -667,9 +667,14 @@ public function delete_credentials() {

$this->save(
array(
// OAuth.
'access_token' => '',
'refresh_token' => '',
'token_expires' => '',

// API Key.
'api_key' => '',
'api_secret' => '',
)
);

Expand Down
26 changes: 26 additions & 0 deletions includes/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,29 @@ function convertkit_maybe_update_credentials( $result, $client_id ) {

}

/**
* 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 the user revokes the access token.
*
* @since 3.2.4
*
* @param string $client_id OAuth Client ID used for the Access and Refresh Tokens.
*/
function convertkit_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;
}

// Delete Access and Refresh Tokens.
$settings = new ConvertKit_Settings();
$settings->delete_credentials();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would this delete all the keys such as CONVERTKIT_API_KEY? Are we storing those in the WordPress plugin?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've updated delete_credentials to also remove the API Key and Secret.


}

/**
* 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,
Expand Down Expand Up @@ -830,6 +853,9 @@ function convertkit_maybe_delete_credentials( $result, $client_id ) {
add_action( 'convertkit_api_get_access_token', 'convertkit_maybe_update_credentials', 10, 2 );
add_action( 'convertkit_api_refresh_token', 'convertkit_maybe_update_credentials', 10, 2 );

// Delete credentials when the user revokes the access and refresh tokens.
add_action( 'convertkit_api_revoke_tokens', 'convertkit_delete_credentials', 10, 1 );

// 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', 'convertkit_maybe_delete_credentials', 10, 2 );
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,47 @@ public function testValidCredentials(EndToEndTester $I)

// Check that no notice is displayed that the API credentials are invalid.
$I->dontSeeErrorNotice($I, 'Kit: Authorization failed. Please connect your Kit account.');
}

/**
* Test that the credentials and resources are deleted on disconnect.
*
* @since 3.2.4
*
* @param EndToEndTester $I Tester.
*/
public function testCredentialsAndResourcesAreDeletedOnDisconnect(EndToEndTester $I)
{
// Setup Plugin.
$I->setupKitPlugin($I);
$I->setupKitPluginResources($I);

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

// Fake the API Key, API Secret, Access and Refresh Tokens; if we revoke the tokens used for tests, future tests will fail.
$I->setupKitPlugin(
$I,
[
'access_token' => 'fakeAccessToken',
'refresh_token' => 'fakeRefreshToken',
'token_expires' => time() + 3600,
'api_key' => 'fakeAPIKey',
'api_secret' => 'fakeAPISecret',
]
);

// Disconnect the Plugin connection to Kit.
$I->click('Disconnect');

// Check credentials are removed from the settings.
$settings = $I->grabOptionFromDatabase('_wp_convertkit_settings');
$I->assertEmpty($settings['access_token']);
$I->assertEmpty($settings['refresh_token']);
$I->assertEmpty($settings['token_expires']);
$I->assertEmpty($settings['api_key']);
$I->assertEmpty($settings['api_secret']);

// Check cached resources are removed from the database on disconnection.
$I->dontSeeOptionInDatabase('convertkit_creator_network_recommendations');
$I->dontSeeOptionInDatabase('convertkit_custom_fields');
Expand All @@ -182,14 +216,6 @@ public function testValidCredentials(EndToEndTester $I)
$I->see('Connect');
$I->dontSee('Disconnect');
$I->dontSeeElementInDOM('input#submit');

// Check that the option table no longer contains cached resources.
$I->dontSeeOptionInDatabase('convertkit_creator_network_recommendations');
$I->dontSeeOptionInDatabase('convertkit_forms');
$I->dontSeeOptionInDatabase('convertkit_landing_pages');
$I->dontSeeOptionInDatabase('convertkit_posts');
$I->dontSeeOptionInDatabase('convertkit_products');
$I->dontSeeOptionInDatabase('convertkit_tags');
}

/**
Expand Down
67 changes: 67 additions & 0 deletions tests/Integration/APITest.php
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,73 @@ public function testCronEventCreatedWhenTokenRefreshed()
$this->assertGreaterThanOrEqual( $nextScheduledTimestamp, time() + 10000 );
}

/**
* Test that the access token and refresh token are deleted from the Plugin's settings
* when the access token is revoked.
*
* @since 3.2.4
*/
public function testCredentialsDeletedAndInvalidWhenRevoked()
{
// Initialize the API without an access token or refresh token.
$api = new \ConvertKit_API_V4(
$_ENV['CONVERTKIT_OAUTH_CLIENT_ID'],
$_ENV['KIT_OAUTH_REDIRECT_URI']
);

// Generate an access token by API key and secret.
$result = $api->get_access_token_by_api_key_and_secret(
$_ENV['CONVERTKIT_API_KEY'],
$_ENV['CONVERTKIT_API_SECRET'],
wp_generate_password( 10, false ) // Random tenant name to produce a token for this request only.
);

// Store the access token in the Plugin's settings.
$settings = new \ConvertKit_Settings();
$settings->save(
array(
'access_token' => $result['oauth']['access_token'],
'refresh_token' => $result['oauth']['refresh_token'],
'token_expires' => $result['oauth']['expires_at'],
)
);

// Initialize the API with the access token and 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()
);

// Confirm the token works when making an authenticated request.
$this->assertNotInstanceOf( 'WP_Error', $api->get_account() );

// Revoke the access and refresh tokens.
$api->revoke_tokens();

// Confirm the access token and refresh token are deleted from the Plugin's settings.
$this->assertEmpty( $settings->get_access_token() );
$this->assertEmpty( $settings->get_refresh_token() );
$this->assertEmpty( $settings->get_token_expiry() );

// Initialize the API with the (now revoked) access token and refresh token.
// revoke_tokens() will have removed the access token and refresh token from the API class, so we need to provide them again
// to test they're revoked.
$api = new \ConvertKit_API_V4(
$_ENV['CONVERTKIT_OAUTH_CLIENT_ID'],
$_ENV['CONVERTKIT_OAUTH_REDIRECT_URI'],
$result['oauth']['access_token'],
$result['oauth']['refresh_token']
);

// Confirm attempting to use the revoked access token no longer works.
$this->assertInstanceOf( 'WP_Error', $api->get_account() );

// Confirm attempting to use the revoked refresh token no longer works.
$this->assertInstanceOf( 'WP_Error', $api->refresh_token() );
}

/**
* Mocks an API response as if the Access Token expired.
*
Expand Down
Loading