Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 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
1 change: 1 addition & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Update - Remove BACS from the unsupported 'change payment method for subscription' page.
* Fix - Fix payment method title display when new payment settings experience is enabled
* Fix - Prevent styles from non-checkout pages affecting the appearance of Stripe element.
* Fix - Prevert further Stripe API calls if API keys are invalid (401 response).

= 9.5.0 - 2025-05-13 =
* Fix - Fixes the listing of payment methods on the classic checkout when the Optimized Checkout is enabled.
Expand Down
4 changes: 2 additions & 2 deletions client/settings/account-details/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,11 @@ const AccountDetails = () => {
{ createInterpolateElement(
isTestModeEnabled
? __(
"Seems like the test API keys we've saved for you are no longer valid. If you recently updated them, use the <strong>Configure Connection</strong> button below to reconnect.",
"We couldn't connect to your account, it seems like the test API keys we've saved for you are no longer valid. Please use the <strong>Configure connection</strong> button below to reconnect.",

Choose a reason for hiding this comment

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

Not urgent in this case. But, will this break internationalization of this string in the patch release?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call.

Copy link
Contributor

Choose a reason for hiding this comment

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

That said, I think the trade-off for correctly communicating the status is probably worth it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, new strings need to be translated in the community translations site: https://translate.wordpress.org/projects/wp-plugins/woocommerce-gateway-stripe/

'woocommerce-gateway-stripe'
)
: __(
"Seems like the live API keys we've saved for you are no longer valid. If you recently updated them, use the <strong>Configure Connection</strong> button below to reconnect.",
"We couldn't connect to your account, it seems like the live API keys we've saved for you are no longer valid. Please use the <strong>Configure connection</strong> button below to reconnect.",
'woocommerce-gateway-stripe'
),
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ const getAccountStatus = ( accountKeys, data, testMode ) => {
},
};

if ( ! hasKeys ) {
if ( ! hasKeys || data?.account === null ) {
return accountStatusMap.disconnected;
}

Expand Down
33 changes: 33 additions & 0 deletions includes/class-wc-stripe-api.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,20 @@ class WC_Stripe_API {
const ENDPOINT = 'https://api.stripe.com/v1/';
const STRIPE_API_VERSION = '2024-06-20';

/**
* The test mode invalid API keys option key.
*
* @var string
*/
const TEST_MODE_INVALID_API_KEYS_OPTION_KEY = 'wc_stripe_test_invalid_api_keys_detected';

/**
* The live mode invalid API keys option key.
*
* @var string
*/
const LIVE_MODE_INVALID_API_KEYS_OPTION_KEY = 'wc_stripe_live_invalid_api_keys_detected';

/**
* Secret API Key.
*
Expand Down Expand Up @@ -231,6 +245,13 @@ public static function request( $request, $api = 'charges', $method = 'POST', $w
* @param string $api
*/
public static function retrieve( $api ) {
// If we have an option flag indicating that the secret key is not valid, we don't attempt the API call and we return an error.
$invalid_api_keys_option_key = WC_Stripe_Mode::is_test() ? self::TEST_MODE_INVALID_API_KEYS_OPTION_KEY : self::LIVE_MODE_INVALID_API_KEYS_OPTION_KEY;
$invalid_api_keys_detected = get_option( $invalid_api_keys_option_key );
if ( $invalid_api_keys_detected ) {
return null; // The UI expects this empty response in case of invalid API keys.
}

WC_Stripe_Logger::log( "{$api}" );

$response = wp_safe_remote_get(
Expand All @@ -242,6 +263,18 @@ public static function retrieve( $api ) {
]
);

// If we get a 401 error, we know the secret key is not valid.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could consider later changing this to check for anything different from 200, to cover more possible issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

My proposed changes in #4327 take that approach when looking at responses. 😁

if ( is_array( $response ) && ! empty( $response['response']['code'] ) && 401 === $response['response']['code'] ) {
// We save a flag in the options to avoid making calls until the secret key gets updated.
update_option( $invalid_api_keys_option_key, true );
update_option( $invalid_api_keys_option_key . '_at', time() );

// We delete the transient for the account data to trigger the not-connected UI in the admin dashboard.
delete_transient( WC_Stripe_Mode::is_test() ? WC_Stripe_Account::TEST_ACCOUNT_OPTION : WC_Stripe_Account::LIVE_ACCOUNT_OPTION );

return null; // The UI expects this empty response in case of invalid API keys.
}

if ( is_wp_error( $response ) || empty( $response['body'] ) ) {
WC_Stripe_Logger::log( 'Error Response: ' . print_r( $response, true ) );
return new WP_Error( 'stripe_error', __( 'There was a problem connecting to the Stripe API endpoint.', 'woocommerce-gateway-stripe' ) );
Expand Down
5 changes: 5 additions & 0 deletions includes/connect/class-wc-stripe-connect.php
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ private function save_stripe_keys( $result, $type = 'connect', $mode = 'live' )
update_option( 'wc_stripe_' . $prefix . 'oauth_failed_attempts', 0 );
update_option( 'wc_stripe_' . $prefix . 'oauth_last_failed_at', '' );

// Clear the invalid API keys transient.
$invalid_api_keys_option_key = $is_test ? WC_Stripe_API::TEST_MODE_INVALID_API_KEYS_OPTION_KEY : WC_Stripe_API::LIVE_MODE_INVALID_API_KEYS_OPTION_KEY;
update_option( $invalid_api_keys_option_key, false );
update_option( $invalid_api_keys_option_key . '_at', time() );

if ( 'app' === $type ) {
// Stripe App OAuth access_tokens expire after 1 hour:
// https://docs.stripe.com/stripe-apps/api-authentication/oauth#refresh-access-token
Expand Down
1 change: 1 addition & 0 deletions readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ If you get stuck, you can ask for help in the [Plugin Forum](https://wordpress.o
* Update - Remove BACS from the unsupported 'change payment method for subscription' page.
* Fix - Fix payment method title display when new payment settings experience is enabled
* Fix - Prevent styles from non-checkout pages affecting the appearance of Stripe element.
* Fix - Prevert further Stripe API calls if API keys are invalid (401 response).


[See changelog for full details across versions](https://raw.githubusercontent.com/woocommerce/woocommerce-gateway-stripe/trunk/changelog.txt).
84 changes: 84 additions & 0 deletions tests/phpunit/test-class-wc-stripe-api.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,88 @@ public function test_set_secret_key_for_mode_with_parameter() {
WC_Stripe_API::set_secret_key_for_mode( 'invalid' );
$this->assertEquals( self::LIVE_SECRET_KEY, WC_Stripe_API::get_secret_key() );
}

/**
* Test WC_Stripe_API::retrieve() when API returns 401 error.
*/
public function test_retrieve_handles_401_error() {
// Mock a 401 API response
add_filter( 'pre_http_request', [ $this, 'mock_401_response' ] );

// Call the retrieve method
$result = WC_Stripe_API::retrieve( 'test_endpoint' );

// Verify the result is null
$this->assertNull( $result );

// Verify the invalid API keys option was set
$this->assertTrue( get_option( WC_Stripe_API::TEST_MODE_INVALID_API_KEYS_OPTION_KEY ) );

// Clean up
remove_filter( 'pre_http_request', [ $this, 'mock_401_response' ] );
delete_option( WC_Stripe_API::TEST_MODE_INVALID_API_KEYS_OPTION_KEY );
}

/**
* Test WC_Stripe_API::retrieve() when API keys are invalid.
*/
public function test_retrieve_returns_null_when_api_keys_are_invalid() {
// Set up the invalid API keys option
update_option( WC_Stripe_API::TEST_MODE_INVALID_API_KEYS_OPTION_KEY, true );

// Call the retrieve method
$result = WC_Stripe_API::retrieve( 'test_endpoint' );

// Verify the result is null
$this->assertNull( $result );

// Clean up
delete_option( WC_Stripe_API::TEST_MODE_INVALID_API_KEYS_OPTION_KEY );
}

/**
* Test WC_Stripe_API::retrieve() when API keys are valid.
*/
public function test_retrieve_makes_api_call_when_api_keys_are_valid() {
// Ensure no invalid API keys option exists
delete_option( WC_Stripe_API::TEST_MODE_INVALID_API_KEYS_OPTION_KEY );

// Mock a successful API response
add_filter( 'pre_http_request', [ $this, 'mock_successful_response' ] );

// Call the retrieve method
$result = WC_Stripe_API::retrieve( 'test_endpoint' );

// Verify the result matches our mock response
$this->assertEquals( 'success', $result );

// Clean up
remove_filter( 'pre_http_request', [ $this, 'mock_successful_response' ] );
}

/**
* Helper method to mock a successful API response.
*/
public function mock_successful_response() {
return [
'response' => [
'code' => 200,
'message' => 'OK',
],
'body' => json_encode( 'success' ),
];
}

/**
* Helper method to mock a 401 API response.
*/
public function mock_401_response() {
return [
'response' => [
'code' => 401,
'message' => 'Unauthorized',
],
'body' => '',
];
}
}
Loading