Prevent Stripe API calls after detecting the API keys are not valid (401 response from API) #4323

diegocurbelo · 2025-05-15T19:13:25Z

Fixes #4270
Relates to STRIPE-445

Changes proposed in this Pull Request:

This PR implements a mechanism to prevent unnecessary Stripe API calls when the API keys are invalid (401 response).

Added an option to track invalid API keys state
Added automatic clearing of invalid API keys state when new keys are saved
Updated the notice messages, and the state in the connection status check to show the Account as disconnected when API keys are invalid

This helps reduce unnecessary API calls and improves error notices/UI for merchants with invalid API credentials:

Testing instructions

Navigate to WP-Admin > WooCommerce > Settings > Payments, and select the Stripe option from the list
Go to the Settings tab and make sure the account is correctly connected:
Replace the API keys with an invalid value:
wp option patch update woocommerce_stripe_settings test_secret_key 'sk_test_INVALID'
Navigate to the WP-Admin dashboard and check that Stripe notice is shwon:
Navigate to WP-Admin > WooCommerce > Settings > Payments, and select the Stripe option from the list
Go to the Settings tab and make sure the account status has the connection error message:
Click Configure connection and make sure the Account status is Disconnected
Click the Create or connect a test account button, a re-connect your account
Check that after returning to the WP-Admin dasboard, the top notice is no longere present, and the acount status section has been restored:

Covered with tests (or have a good reason not to test in description ☝️)
Tested on mobile (or does not apply)

Changelog entry

This Pull Request does not require a changelog entry. (Comment required below)

Changelog Entry Comment

Comment

Post merge

Added testing instructions to the Release Testing Instructions wiki page (or does not apply)
Added the needs docs label (or does not apply)
Included this PR in the Release Thread scope (or does not apply)

daledupreez

I think this is the general approach should work well, with a concern about triggering the code after one 401 response.

More broadly, I think we also need to clear the transient when we save/update the Stripe keys, which feels like it ought to occur in WC_Stripe_Connect->save_stripe_keys() and whatever code we use to disconnect/clear keys.

includes/class-wc-stripe-api.php

Co-authored-by: daledupreez <[email protected]>

…e save similar data for onboarding and webhooks stats

includes/class-wc-stripe-api.php

… modal)

ebinnion · 2025-05-16T18:58:07Z

client/settings/account-details/index.js

@@ -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.",


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

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

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

daledupreez

Overall, this is looking good and tests as described.

I do think we need to fix the typos in the changelogs, and I had one minor comment about possibly making the path to identify the response code more defensive.

includes/class-wc-stripe-api.php

changelog.txt

readme.txt

daledupreez · 2025-05-16T19:18:10Z

client/settings/account-details/index.js

@@ -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.",


Co-authored-by: daledupreez <[email protected]>

wjrosa · 2025-05-16T19:26:27Z

includes/class-wc-stripe-api.php

@@ -242,6 +263,18 @@ public static function retrieve( $api ) {
 			]
 		);

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


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

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

daledupreez

Thanks for the cleanup, @diegocurbelo! 🚀

…401 response from API) (#4323) * Add transient to prevent api calls after getting a 401 response * Update includes/class-wc-stripe-api.php Co-authored-by: daledupreez <[email protected]> * Update error response to empty json * Remove invalid transient when saving new keys * Refactored to use get_option/update_option for concistency with how we save similar data for onboarding and webhooks stats * Update code comments * Update account not connected notification message * Show account as disconnected if the account data is not valid * Refactor 401 response to be null (needed for the Configure connection modal) * Clear account cache after detecting a 401 response * Add tests * Simplify option keys in test * Fix success response mock * Add changelo entry * Apply suggestions from code review Co-authored-by: daledupreez <[email protected]> --------- Co-authored-by: daledupreez <[email protected]>

Add transient to prevent api calls after getting a 401 response

10fc910

daledupreez reviewed May 15, 2025

View reviewed changes

includes/class-wc-stripe-api.php Outdated Show resolved Hide resolved

includes/class-wc-stripe-api.php Outdated Show resolved Hide resolved

includes/class-wc-stripe-api.php Outdated Show resolved Hide resolved

includes/class-wc-stripe-api.php Outdated Show resolved Hide resolved

diegocurbelo and others added 6 commits May 15, 2025 15:51

Update includes/class-wc-stripe-api.php

1063336

Co-authored-by: daledupreez <[email protected]>

Update error response to empty json

5a11e7c

Remove invalid transient when saving new keys

4f674ab

Refactored to use get_option/update_option for concistency with how w…

7ef8df3

…e save similar data for onboarding and webhooks stats

Update code comments

f90e45e

Update account not connected notification message

1c7c8f5

daledupreez mentioned this pull request May 16, 2025

Enforce plugin rate limit when Stripe returns 429 errors #4327

Open

5 tasks

daledupreez reviewed May 16, 2025

View reviewed changes

includes/class-wc-stripe-api.php Outdated Show resolved Hide resolved

includes/class-wc-stripe-api.php Outdated Show resolved Hide resolved

diegocurbelo added 3 commits May 16, 2025 12:28

Show account as disconnected if the account data is not valid

47b42d2

Refactor 401 response to be null (needed for the Configure connection…

24ea6ea

… modal)

Merge branch 'develop' into fix/handle-stripe-api-401-errors

9740ff3

diegocurbelo marked this pull request as ready for review May 16, 2025 16:26

diegocurbelo requested review from a team and annemirasol and removed request for a team May 16, 2025 16:26

diegocurbelo added 5 commits May 16, 2025 13:28

Clear account cache after detecting a 401 response

9666dcf

Add tests

cd43fc9

Simplify option keys in test

4d288fc

Fix success response mock

52048d9

Add changelo entry

13e6401

ebinnion reviewed May 16, 2025

View reviewed changes

daledupreez reviewed May 16, 2025

View reviewed changes

Apply suggestions from code review

ff5d7eb

Co-authored-by: daledupreez <[email protected]>

wjrosa reviewed May 16, 2025

View reviewed changes

daledupreez approved these changes May 16, 2025

View reviewed changes

wjrosa merged commit 962163c into develop May 17, 2025
49 of 51 checks passed

wjrosa deleted the fix/handle-stripe-api-401-errors branch May 17, 2025 14:41

daledupreez added this to the 9.5.1 milestone Jun 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Prevent Stripe API calls after detecting the API keys are not valid (401 response from API) #4323

Prevent Stripe API calls after detecting the API keys are not valid (401 response from API) #4323

Uh oh!

diegocurbelo commented May 15, 2025 •

edited

Loading

Uh oh!

daledupreez left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ebinnion May 16, 2025

Uh oh!

daledupreez May 16, 2025

Uh oh!

daledupreez May 16, 2025

Uh oh!

diegocurbelo May 16, 2025

Uh oh!

daledupreez left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daledupreez May 16, 2025

Uh oh!

wjrosa May 16, 2025

Uh oh!

daledupreez May 16, 2025

Uh oh!

daledupreez left a comment

Uh oh!

Uh oh!

Uh oh!

Prevent Stripe API calls after detecting the API keys are not valid (401 response from API) #4323

Prevent Stripe API calls after detecting the API keys are not valid (401 response from API) #4323

Uh oh!

Conversation

diegocurbelo commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

Testing instructions

Changelog entry

Comment

Uh oh!

daledupreez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ebinnion May 16, 2025

Choose a reason for hiding this comment

Uh oh!

daledupreez May 16, 2025

Choose a reason for hiding this comment

Uh oh!

daledupreez May 16, 2025

Choose a reason for hiding this comment

Uh oh!

diegocurbelo May 16, 2025

Choose a reason for hiding this comment

Uh oh!

daledupreez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daledupreez May 16, 2025

Choose a reason for hiding this comment

Uh oh!

wjrosa May 16, 2025

Choose a reason for hiding this comment

Uh oh!

daledupreez May 16, 2025

Choose a reason for hiding this comment

Uh oh!

daledupreez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

diegocurbelo commented May 15, 2025 •

edited

Loading