diff --git a/changelog.txt b/changelog.txt index 61f5d8ea53..d0d2669fe4 100644 --- a/changelog.txt +++ b/changelog.txt @@ -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 - Ensure that we apply rate limits when Stripe returns 429 responses = 9.5.0 - 2025-05-13 = * Fix - Fixes the listing of payment methods on the classic checkout when the Optimized Checkout is enabled. diff --git a/includes/class-wc-stripe-api.php b/includes/class-wc-stripe-api.php index 0ce455bb3d..0d994cf810 100644 --- a/includes/class-wc-stripe-api.php +++ b/includes/class-wc-stripe-api.php @@ -16,6 +16,28 @@ class WC_Stripe_API { const ENDPOINT = 'https://api.stripe.com/v1/'; const STRIPE_API_VERSION = '2024-06-20'; + + /** + * Option key for cases where Stripe rate limits our live API calls. + * + * @var string + */ + public const LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY = 'wc_stripe_live_api_rate_limit'; + + /** + * Option key for cases where Stripe rate limits our test API calls. + * + * @var string + */ + public const TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY = 'wc_stripe_test_api_rate_limit'; + + /** + * Duration we will use to disable Stripe API calls if we have been rate limited. + * + * @var int + */ + public const STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS = 30; + /** * Secret API Key. * @@ -231,6 +253,10 @@ public static function request( $request, $api = 'charges', $method = 'POST', $w * @param string $api */ public static function retrieve( $api ) { + if ( self::is_stripe_api_rate_limited() ) { + return null; + } + WC_Stripe_Logger::log( "{$api}" ); $response = wp_safe_remote_get( @@ -242,6 +268,8 @@ public static function retrieve( $api ) { ] ); + self::check_stripe_api_error_response( $response ); + 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' ) ); @@ -250,6 +278,84 @@ public static function retrieve( $api ) { return json_decode( $response['body'] ); } + /** + * Checks if the Stripe API for the current mode (i.e. test or live) is rate limited. + * + * @return bool True if the Stripe API is rate limited, false otherwise. + */ + public static function is_stripe_api_rate_limited() { + $rate_limit_option_key = WC_Stripe_Mode::is_test() ? self::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : self::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + + $rate_limit_expiration = get_option( $rate_limit_option_key ); + if ( ! $rate_limit_expiration ) { + return false; + } + + $now = time(); + if ( $now > $rate_limit_expiration ) { + delete_option( $rate_limit_option_key ); + return false; + } + + return true; + } + + /** + * Helper function to check error responses from Stripe and ensure we prevent unnecessary API calls, + * primarily in cases where we have been rate limited or we don't have valid keys.. + * + * @param array|WP_Error $response The response from the Stripe API. + * @return void + */ + protected static function check_stripe_api_error_response( $response ) { + // If we don't have an array for $response, return early, as we won't have an HTTP status code. + if ( ! is_array( $response ) ) { + return; + } + + // We specifically want to check $response['response']['code']. If it's not present, return early. + if ( + ! isset( $response['response'] ) + || ! is_array( $response['response'] ) + || ! isset( $response['response']['code'] ) + ) { + return; + } + + $status_code = $response['response']['code']; + + if ( 429 === $status_code ) { + // Stripe has rate limited us, so disable API calls for a period of time. + $is_test_mode = WC_Stripe_Mode::is_test(); + + $timestamp = time(); + $rate_limit_option_key = $is_test_mode ? self::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : self::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + update_option( $rate_limit_option_key, $timestamp + self::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS ); + + $mode = $is_test_mode ? 'test' : 'LIVE'; + $message = "Stripe {$mode} mode API has been rate limited, disabling API calls for " . self::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS . ' seconds.'; + + error_log( 'woocommerce-gateway-stripe: WARNING: ' . $message ); + WC_Stripe_Logger::error( $message ); + + // Store history of rate limits so we can see how often they're occurring. + $history_option_key = $rate_limit_option_key . '_history'; + $history = get_option( $history_option_key, [] ); + if ( ! is_array( $history ) ) { + $history = []; + } + // Keep a maximum of 20 rate limit history entries. + $history = array_slice( $history, -19 ); + $history[] = [ + 'timestamp' => $timestamp, + 'datetime' => gmdate( 'Y-m-d H:i:s', $timestamp ) . ' UTC', + 'duration' => self::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS, + ]; + // Note that we set autoload to false - we don't want this option to be autoloaded by default. + update_option( $history_option_key, $history, false ); + } + } + /** * Send the request to Stripe's API with level 3 data generated * from the order. If the request fails due to an error related diff --git a/readme.txt b/readme.txt index 5e7858b583..647c953008 100644 --- a/readme.txt +++ b/readme.txt @@ -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 - Ensure that we apply rate limits when Stripe returns 429 responses [See changelog for full details across versions](https://raw.githubusercontent.com/woocommerce/woocommerce-gateway-stripe/trunk/changelog.txt). diff --git a/tests/phpunit/test-class-wc-stripe-api.php b/tests/phpunit/test-class-wc-stripe-api.php index 61a64aacf4..05f0953b47 100644 --- a/tests/phpunit/test-class-wc-stripe-api.php +++ b/tests/phpunit/test-class-wc-stripe-api.php @@ -40,6 +40,17 @@ public function set_up() { public function tear_down() { WC_Stripe_Helper::delete_main_stripe_settings(); WC_Stripe_API::set_secret_key( null ); + + WC_Stripe_Logger::$logger = null; + + if ( false !== has_filter( 'pre_http_request', [ $this, 'mock_429_response' ] ) ) { + remove_filter( 'pre_http_request', [ $this, 'mock_429_response' ] ); + } + + if ( false !== has_filter( 'pre_http_request', [ $this, 'throw_exception_on_http_request' ] ) ) { + remove_filter( 'pre_http_request', [ $this, 'throw_exception_on_http_request' ] ); + } + parent::tear_down(); } @@ -94,4 +105,209 @@ 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() ); } + + /** + * Provide test modes for test cases that test both test and live modes. + * + * @return array + */ + public function provide_test_modes() { + return [ + 'live mode' => [ false ], + 'test mode' => [ true ], + ]; + } + + /** + * Test WC_Stripe_API::retrieve() returns early when rate limit is active. + * + * @dataProvider provide_test_modes + * @param bool $is_test_mode Whether the test mode is true or false. + */ + public function test_retrieve_returns_early_when_rate_limit_is_active( $is_test_mode ) { + $settings = WC_Stripe_Helper::get_stripe_settings(); + $settings['testmode'] = $is_test_mode ? 'yes' : 'no'; + $settings['logging'] = 'yes'; + WC_Stripe_Helper::update_main_stripe_settings( $settings ); + + // Add this filter after we update the settings, as that code can trigger HTTP requests. + add_filter( 'pre_http_request', [ $this, 'throw_exception_on_http_request' ] ); + + $mock_logger = $this->createStub( WC_Logger_Interface::class ); + + $mock_logger->expects( $this->never() ) + ->method( 'debug' ); + + $mock_logger->expects( $this->never() ) + ->method( 'error' ); + + $now = time(); + $rate_limit_option_key = $is_test_mode ? WC_Stripe_API::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : WC_Stripe_API::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + update_option( $rate_limit_option_key, $now + 20 ); + + WC_Stripe_Logger::$logger = $mock_logger; + + $result = WC_Stripe_API::retrieve( 'account' ); + + WC_Stripe_Logger::$logger = null; + + $this->assertNull( $result ); + + remove_filter( 'pre_http_request', [ $this, 'throw_exception_on_http_request' ] ); + } + + /** + * Test WC_Stripe_API::is_stripe_api_rate_limited() returns false when no rate limit is active. + * + * @dataProvider provide_test_modes + * @param bool $is_test_mode Whether the test mode is true or false. + */ + public function test_rate_limit_check_returns_false_when_no_rate_limit_is_active( $is_test_mode ) { + $settings = WC_Stripe_Helper::get_stripe_settings(); + $settings['testmode'] = $is_test_mode ? 'yes' : 'no'; + WC_Stripe_Helper::update_main_stripe_settings( $settings ); + + $this->assertFalse( WC_Stripe_API::is_stripe_api_rate_limited() ); + } + + /** + * Test WC_Stripe_API::is_stripe_api_rate_limited() returns false and deletes the option after the rate limit expires. + * + * @dataProvider provide_test_modes + * @param bool $is_test_mode Whether the test mode is true or false. + */ + public function test_rate_limit_check_returns_false_and_deletes_option_after_rate_limit_expires( $is_test_mode ) { + $settings = WC_Stripe_Helper::get_stripe_settings(); + $settings['testmode'] = $is_test_mode ? 'yes' : 'no'; + WC_Stripe_Helper::update_main_stripe_settings( $settings ); + + $rate_limit_option_key = $is_test_mode ? WC_Stripe_API::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : WC_Stripe_API::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + update_option( $rate_limit_option_key, time() - 20 ); + + $this->assertFalse( WC_Stripe_API::is_stripe_api_rate_limited() ); + + $this->assertNull( get_option( $rate_limit_option_key, null ) ); + } + + /** + * Test WC_Stripe_API::retrieve() correctly triggers rate limiting when + * we receive a 429 response from the Stripe API. + * + * @dataProvider provide_test_modes + * @param bool $is_test_mode Whether the test mode is true or false. + */ + public function test_check_stripe_api_429_response_triggers_rate_limit( $is_test_mode ) { + $settings = WC_Stripe_Helper::get_stripe_settings(); + $settings['testmode'] = $is_test_mode ? 'yes' : 'no'; + $settings['logging'] = 'yes'; + WC_Stripe_Helper::update_main_stripe_settings( $settings ); + + $rate_limit_option_key = $is_test_mode ? WC_Stripe_API::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : WC_Stripe_API::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + $history_option_key = $rate_limit_option_key . '_history'; + update_option( $history_option_key, [], false ); + + $mock_logger = $this->createStub( WC_Logger_Interface::class ); + + $mock_logger->expects( $this->exactly( 2 ) ) + ->method( 'debug' ) + ->withConsecutive( + [ $this->get_expected_log_message( 'account' ) ], + [ $this->get_expected_log_prefix( 'Error Response: ' ) ], + ); + + $message_mode = $is_test_mode ? 'test' : 'LIVE'; + $mock_logger->expects( $this->once() ) + ->method( 'error' ) + ->with( + "Stripe $message_mode mode API has been rate limited, disabling API calls for " . + WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS . ' seconds.' + ); + + // Mock 429 responses from the Stripe API. + add_filter( 'pre_http_request', [ $this, 'mock_429_response' ] ); + + WC_Stripe_Logger::$logger = $mock_logger; + + $request_start_time = time(); + $result = WC_Stripe_API::retrieve( 'account' ); + $request_end_time = time(); + + // Unset the mock logger. + WC_Stripe_Logger::$logger = null; + + $this->assertInstanceOf( WP_Error::class, $result ); + $this->assertEquals( 'stripe_error', $result->get_error_code() ); + $this->assertEquals( 'There was a problem connecting to the Stripe API endpoint.', $result->get_error_message() ); + + $rate_limit_option = get_option( $rate_limit_option_key ); + $this->assertIsInt( $rate_limit_option ); + + $runtime_delta = max( $request_end_time - $request_start_time, 1 ); + $this->assertEqualsWithDelta( $request_end_time + WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS, $rate_limit_option, $runtime_delta ); + + $history = get_option( $history_option_key, null ); + $this->assertIsArray( $history ); + $this->assertCount( 1, $history ); + + $history_entry = $history[0]; + $this->assertIsArray( $history_entry ); + $this->assertArrayHasKey( 'timestamp', $history_entry ); + $this->assertArrayHasKey( 'datetime', $history_entry ); + $this->assertArrayHasKey( 'duration', $history_entry ); + + $expected_timestamp = $rate_limit_option - WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS; + $this->assertEquals( $expected_timestamp, $history_entry['timestamp'] ); + $this->assertEquals( gmdate( 'Y-m-d H:i:s', $expected_timestamp ) . ' UTC', $history_entry['datetime'] ); + $this->assertEquals( WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS, $history_entry['duration'] ); + + remove_filter( 'pre_http_request', [ $this, 'mock_429_response' ] ); + } + + /** + * Helper method to get the expected log message. + * + * @param string $message The message we expect to see in the log. + * @return string The expected log message. + */ + protected function get_expected_log_message( $message ) { + $expected_log_message = "\n" . '====Stripe Version: ' . WC_STRIPE_VERSION . '====' . "\n"; + $expected_log_message .= '====Stripe Plugin API Version: ' . WC_Stripe_API::STRIPE_API_VERSION . '====' . "\n"; + $expected_log_message .= '====Start Log====' . "\n" . $message . "\n" . '====End Log====' . "\n\n"; + + return $expected_log_message; + } + + /** + * Helper method to get the expected log message prefix. + * + * @param string $message The message prefix we expect to see in the log. + * @return string The expected log message prefix. + */ + protected function get_expected_log_prefix( $message ) { + $expected_log_prefix = "\n" . '====Stripe Version: ' . WC_STRIPE_VERSION . '====' . "\n"; + $expected_log_prefix .= '====Stripe Plugin API Version: ' . WC_Stripe_API::STRIPE_API_VERSION . '====' . "\n"; + $expected_log_prefix .= '====Start Log====' . "\n" . $message; + + return $this->stringStartsWith( $expected_log_prefix ); + } + + /** + * Helper method to mock an HTTP 429 response from the Stripe API. + */ + public function mock_429_response( $preempt ) { + return [ + 'response' => [ + 'code' => 429, + 'message' => 'Too many requests', + ], + 'body' => '', + ]; + } + + /** + * Helper method to throw an when triggered. + */ + public function throw_exception_on_http_request( $preempt ) { + throw new Exception( 'HTTP request should not be triggered' ); + } }