Prevent orders being placed when no shipping options are available (#46026)

* Prevent orders being placed with invalid shipping options

* Add changelog

* Add shipping_disable_flat_rate fixture function

* Test checking out with no valid shipping methods selected

* Update tests to add a default shipping method

* Update test_checkout_invalid_shipping_method to disable method

* If shipping methods is null, return an array with an empty string inside

* Replace WC session in tests that rely on setting cookies

* Add MockSessionHandler to handle test cases using cookies

* Add docblock comment

* Expect shipping validation to fail if chosen methods are null

* Add shipping method before testing validate_selected_shipping_methods

* Update MockSessionHandler to handle caching

* Show error when test fails

* Default the chosen shipping methods to an empty array if not set

* Split checks for needs_shipping and valid shipping apart

* Remove unnecessary session set and total calculation

* Fix lint errors

* Init session in each test

* Reimplement required methods (those that are private or use cookies)

* Update phpcs ignore comment to be inline

* Prevent error when accessing unset variable in mock cache

* Fix lint error
This commit is contained in:
Thomas Roberts 2024-04-02 10:03:14 +01:00 committed by GitHub
parent c0d0596574
commit 68c6ec6100
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 479 additions and 13 deletions

View File

@ -0,0 +1,4 @@
Significance: patch
Type: fix
Fixed an issue where orders could be placed when no shipping options were available

View File

@ -168,7 +168,7 @@ class OrderController {
*/
public function validate_order_before_payment( \WC_Order $order ) {
$needs_shipping = wc()->cart->needs_shipping();
$chosen_shipping_methods = wc()->session->get( 'chosen_shipping_methods' );
$chosen_shipping_methods = wc()->session->get( 'chosen_shipping_methods', [] );
$this->validate_coupons( $order );
$this->validate_email( $order );
@ -549,18 +549,30 @@ class OrderController {
* @param array $chosen_shipping_methods Array of shipping methods.
*/
public function validate_selected_shipping_methods( $needs_shipping, $chosen_shipping_methods = array() ) {
if ( ! $needs_shipping || ! is_array( $chosen_shipping_methods ) ) {
if ( ! $needs_shipping ) {
return;
}
$exception = new RouteException(
'woocommerce_rest_invalid_shipping_option',
__( 'Sorry, this order requires a shipping option.', 'woocommerce' ),
400,
array()
);
if ( ! is_array( $chosen_shipping_methods ) || empty( $chosen_shipping_methods ) ) {
throw $exception;
}
$valid_methods = array_keys( WC()->shipping()->get_shipping_methods() );
foreach ( $chosen_shipping_methods as $chosen_shipping_method ) {
if ( false === $chosen_shipping_method ) {
throw new RouteException(
'woocommerce_rest_invalid_shipping_option',
__( 'Sorry, this order requires a shipping option.', 'woocommerce' ),
400,
array()
);
if (
false === $chosen_shipping_method ||
! is_string( $chosen_shipping_method ) ||
! in_array( current( explode( ':', $chosen_shipping_method ) ), $valid_methods, true )
) {
throw $exception;
}
}
}

View File

@ -321,6 +321,26 @@ class FixtureData {
WC()->shipping()->load_shipping_methods();
}
/**
* Disables the flat rate method.
*
* @param float $cost Optional. Cost of flat rate method.
*/
public function shipping_disable_flat_rate( $cost = 10 ) {
$flat_rate_settings = array(
'enabled' => 'no',
'title' => 'Flat rate',
'availability' => 'all',
'countries' => '',
'tax_status' => 'taxable',
'cost' => $cost,
);
update_option( 'woocommerce_flat_rate_settings', $flat_rate_settings );
update_option( 'woocommerce_flat_rate', array() );
\WC_Cache_Helper::get_transient_version( 'shipping', true );
WC()->shipping()->load_shipping_methods();
}
/**
* Enable bacs payment method.
*/

View File

@ -0,0 +1,328 @@
<?php
namespace Automattic\WooCommerce\Tests\Blocks\StoreApi;
use WC_Customer;
use WC_Session_Handler;
use Automattic\Jetpack\Constants;
defined( 'ABSPATH' ) || exit;
/**
* Mock Session handler class. Required because PHPUnit doesn't allow cookies to be set in the response as it runs tests
* in a single context.
*/
class MockSessionHandler extends WC_Session_Handler {
/**
* @var array Mock cache for unit tests.
*/
protected $mock_cache = array();
/**
* Override the set function to ensure the data is added to the cache. Without this the data is not saved and
* changing users doesn't work.
*
* @param string $key Key to set.
* @param mixed $value Value to set.
*/
public function set( $key, $value ) {
$this->_data[ $key ] = $value;
$this->_dirty = true;
$this->mock_cache[ $this->_customer_id ] = $this->_data;
}
/**
* Init hooks and session data.
*
* @since 3.3.0
*/
public function init() {
$this->init_session_cookie();
$this->set_customer_session_cookie( true );
add_action( 'woocommerce_set_cart_cookies', array( $this, 'set_customer_session_cookie' ), 10 );
add_action( 'wp', array( $this, 'maybe_set_customer_session_cookie' ), 99 );
add_action( 'shutdown', array( $this, 'save_data' ), 20 );
add_action( 'wp_logout', array( $this, 'destroy_session' ) );
if ( ! is_user_logged_in() ) {
add_filter( 'nonce_user_logged_out', array( $this, 'maybe_update_nonce_user_logged_out' ), 10, 2 );
}
}
/**
* Setup cookie and customer ID.
*
* @since 3.6.0
*/
public function init_session_cookie() {
$cookie = $this->get_session_cookie();
if ( $cookie ) {
// Customer ID will be an MD5 hash id this is a guest session.
$this->_customer_id = $cookie[0];
$this->_session_expiration = $cookie[1];
$this->_session_expiring = $cookie[2];
$this->_has_cookie = true;
$this->_data = $this->get_session_data();
if ( ! $this->is_session_cookie_valid() ) {
$this->destroy_session();
$this->set_session_expiration();
}
// If the user logs in, update session.
if ( is_user_logged_in() && strval( get_current_user_id() ) !== $this->_customer_id ) {
$guest_session_id = $this->_customer_id;
$this->_customer_id = strval( get_current_user_id() );
$this->_dirty = true;
$this->save_data( $guest_session_id );
$this->set_customer_session_cookie( true );
/**
* Fires after a customer has logged in, and their guest session id has been
* deleted with its data migrated to a customer id.
*
* This hook gives extensions the chance to connect the old session id to the
* customer id, if the key is being used externally.
*
* @since 8.8.0
*
* @param string $guest_session_id The former session ID, as generated by `::generate_customer_id()`.
* @param int $customer_id The Customer ID that the former session was converted to.
*/
do_action( 'woocommerce_guest_session_to_user_id', $guest_session_id, $this->_customer_id );
}
// Update session if its close to expiring.
if ( time() > $this->_session_expiring ) {
$this->set_session_expiration();
$this->update_session_timestamp( $this->_customer_id, $this->_session_expiration );
}
} else {
$this->set_session_expiration();
$this->_customer_id = $this->generate_customer_id();
$this->_data = $this->get_session_data();
}
}
/**
* Checks if session cookie is expired, or belongs to a logged out user.
*
* @return bool Whether session cookie is valid.
*/
private function is_session_cookie_valid() {
// If session is expired, session cookie is invalid.
if ( time() > $this->_session_expiration ) {
return false;
}
// If user has logged out, session cookie is invalid.
if ( ! is_user_logged_in() && ! $this->is_customer_guest( $this->_customer_id ) ) {
return false;
}
// Session from a different user is not valid. (Although from a guest user will be valid).
if ( is_user_logged_in() && ! $this->is_customer_guest( $this->_customer_id ) && strval( get_current_user_id() ) !== $this->_customer_id ) {
return false;
}
return true;
}
/**
* Hooks into the wp action to maybe set the session cookie if the user is on a certain page e.g. a checkout endpoint.
*
* Certain gateways may rely on sessions and this ensures a session is present even if the customer does not have a
* cart.
*/
public function maybe_set_customer_session_cookie() {
if ( is_wc_endpoint_url( 'order-pay' ) ) {
$this->set_customer_session_cookie( true );
}
}
/**
* Sets the session cookie on-demand (usually after adding an item to the cart).
*
* Since the cookie name (as of 2.1) is prepended with wp, cache systems like batcache will not cache pages when set.
*
* Warning: Cookies will only be set if this is called before the headers are sent.
*
* @param bool $set Should the session cookie be set.
*/
public function set_customer_session_cookie( $set ) {
if ( $set ) {
$to_hash = $this->_customer_id . '|' . $this->_session_expiration;
$cookie_hash = hash_hmac( 'md5', $to_hash, wp_hash( $to_hash ) );
$cookie_value = $this->_customer_id . '||' . $this->_session_expiration . '||' . $this->_session_expiring . '||' . $cookie_hash;
$this->_has_cookie = true;
if ( ! isset( $_COOKIE[ $this->_cookie ] ) || $_COOKIE[ $this->_cookie ] !== $cookie_value ) {
$_COOKIE[ $this->_cookie ] = $cookie_value;
}
}
}
/**
* Checks if this is an auto-generated customer ID.
*
* @param string|int $customer_id Customer ID to check.
*
* @return bool Whether customer ID is randomly generated.
*/
private function is_customer_guest( $customer_id ) {
$customer_id = strval( $customer_id );
if ( empty( $customer_id ) ) {
return true;
}
if ( 't_' === substr( $customer_id, 0, 2 ) ) {
return true;
}
/**
* Legacy checks. This is to handle sessions that were created from a previous release.
* Maybe we can get rid of them after a few releases.
*/
// Almost all random $customer_ids will have some letters in it, while all actual ids will be integers.
if ( strval( (int) $customer_id ) !== $customer_id ) {
return true;
}
// Performance hack to potentially save a DB query, when same user as $customer_id is logged in.
if ( is_user_logged_in() && strval( get_current_user_id() ) === $customer_id ) {
return false;
} else {
$customer = new WC_Customer( $customer_id );
if ( 0 === $customer->get_id() ) {
return true;
}
}
return false;
}
/**
* Get session unique ID for requests if session is initialized or user ID if logged in.
* Introduced to help with unit tests.
*
* @since 5.3.0
* @return string
*/
public function get_customer_unique_id() {
$customer_id = '';
if ( $this->has_session() && $this->_customer_id ) {
$customer_id = $this->_customer_id;
} elseif ( is_user_logged_in() ) {
$customer_id = (string) get_current_user_id();
}
return $customer_id;
}
/**
* Save data and delete guest session.
*
* @param int $old_session_key session ID before user logs in.
*/
public function save_data( $old_session_key = 0 ) {
// Dirty if something changed - prevents saving nothing new.
if ( $this->_dirty && $this->has_session() ) {
global $wpdb;
$wpdb->query(
$wpdb->prepare(
// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
"INSERT INTO $this->_table (`session_key`, `session_value`, `session_expiry`) VALUES (%s, %s, %d)
ON DUPLICATE KEY UPDATE `session_value` = VALUES(`session_value`), `session_expiry` = VALUES(`session_expiry`)",
$this->_customer_id,
maybe_serialize( $this->_data ),
$this->_session_expiration
)
);
$this->mock_cache[ $this->_customer_id ] = $this->_data;
$this->_dirty = false;
// phpcs:ignore Universal.Operators.StrictComparisons.LooseNotEqual -- Loose comparison is in parent class so we need to keep it here.
if ( get_current_user_id() != $old_session_key && ! is_object( get_user_by( 'id', $old_session_key ) ) ) {
$this->delete_session( $old_session_key );
}
}
}
/**
* Forget all session data without destroying it.
* Overriden in this mock because the parent method sets a cookie using wc_setcookie which doesn't work with PHPUnit.
*/
public function forget_session() {
unset( $_COOKIE[ $this->_cookie ] );
if ( ! is_admin() ) {
include_once WC_ABSPATH . 'includes/wc-cart-functions.php';
wc_empty_cart();
}
$this->_data = array();
$this->_dirty = false;
$this->_customer_id = $this->generate_customer_id();
}
/**
* Returns the session.
*
* @param string $customer_id Customer ID.
* @param mixed $default Default session value.
* @return string|array
*/
public function get_session( $customer_id, $default = false ) { // phpcs:ignore Universal.NamingConventions.NoReservedKeywordParameterNames.defaultFound
global $wpdb;
if ( Constants::is_defined( 'WP_SETUP_CONFIG' ) ) {
return false;
}
// Try to get it from the cache, it will return false if not present or if object cache not in use.
$value = empty( $this->mock_cache[ $customer_id ] ) ? false : $this->mock_cache[ $customer_id ];
if ( false === $value ) {
$value = $wpdb->get_var( $wpdb->prepare( "SELECT session_value FROM $this->_table WHERE session_key = %s", $customer_id ) ); // @codingStandardsIgnoreLine.
if ( is_null( $value ) ) {
$value = $default;
}
$cache_duration = $this->_session_expiration - time();
if ( 0 < $cache_duration ) {
$this->mock_cache[ $customer_id ] = $value;
}
}
return maybe_unserialize( $value );
}
/**
* Delete the session from the cache and database.
*
* @param int $customer_id Customer ID.
*/
public function delete_session( $customer_id ) {
global $wpdb;
unset( $this->mock_cache[ $customer_id ] );
$wpdb->delete(
$this->_table,
array(
'session_key' => $customer_id,
)
);
}
}

View File

@ -15,6 +15,7 @@ use Automattic\WooCommerce\Tests\Blocks\Helpers\FixtureData;
use Automattic\WooCommerce\StoreApi\Routes\V1\Checkout as CheckoutRoute;
use Automattic\WooCommerce\StoreApi\SchemaController;
use Automattic\WooCommerce\Tests\Blocks\StoreApi\MockSessionHandler;
use Mockery\Adapter\Phpunit\MockeryTestCase;
/**
@ -65,7 +66,13 @@ class Checkout extends MockeryTestCase {
register_rest_route( $route->get_namespace(), $route->get_path(), $route->get_args(), true );
$fixtures = new FixtureData();
$fixtures->shipping_add_flat_rate();
// Add a flat rate to the default zone.
$flat_rate = WC()->shipping()->get_shipping_methods()['flat_rate'];
$default_zone = \WC_Shipping_Zones::get_zone( 0 );
$default_zone->add_shipping_method( $flat_rate->id );
$default_zone->save();
$fixtures->payments_enable_bacs();
$this->products = array(
$fixtures->get_simple_product(
@ -95,6 +102,13 @@ class Checkout extends MockeryTestCase {
*/
protected function tearDown(): void {
parent::tearDown();
$default_zone = \WC_Shipping_Zones::get_zone( 0 );
$shipping_methods = $default_zone->get_shipping_methods();
foreach ( $shipping_methods as $method ) {
$default_zone->delete_shipping_method( $method->instance_id );
}
$default_zone->save();
global $wp_rest_server;
$wp_rest_server = null;
}
@ -310,6 +324,12 @@ class Checkout extends MockeryTestCase {
* Check that accounts are created on request.
*/
public function test_checkout_create_account() {
// We need to replace the WC_Session with a mock because this test relies on cookies being set which
// is not easy with PHPUnit. This is a simpler approach.
$old_session = WC()->session;
WC()->session = new MockSessionHandler();
WC()->session->init();
update_option( 'woocommerce_enable_guest_checkout', 'yes' );
update_option( 'woocommerce_enable_signup_and_login_from_checkout', 'yes' );
@ -356,17 +376,26 @@ class Checkout extends MockeryTestCase {
$status = $response->get_status();
$data = $response->get_data();
$this->assertEquals( $status, 200 );
$this->assertEquals( $status, 200, print_r( $data, true ) );
$this->assertTrue( $data['customer_id'] > 0 );
$customer = get_user_by( 'id', $data['customer_id'] );
$this->assertEquals( $customer->user_email, 'testaccount@test.com' );
// Return WC_Session to original state.
WC()->session = $old_session;
}
/**
* Test account creation options.
*/
public function test_checkout_do_not_create_account() {
// We need to replace the WC_Session with a mock because this test relies on cookies being set which
// is not easy with PHPUnit. This is a simpler approach.
$old_session = WC()->session;
WC()->session = new MockSessionHandler();
WC()->session->init();
update_option( 'woocommerce_enable_guest_checkout', 'yes' );
update_option( 'woocommerce_enable_signup_and_login_from_checkout', 'yes' );
@ -415,12 +444,21 @@ class Checkout extends MockeryTestCase {
$this->assertEquals( $status, 200 );
$this->assertEquals( $data['customer_id'], 0 );
// Return WC_Session to original state.
WC()->session = $old_session;
}
/**
* Test account creation options.
*/
public function test_checkout_force_create_account() {
// We need to replace the WC_Session with a mock because this test relies on cookies being set which
// is not easy with PHPUnit. This is a simpler approach.
$old_session = WC()->session;
WC()->session = new MockSessionHandler();
WC()->session->init();
update_option( 'woocommerce_enable_guest_checkout', 'no' );
update_option( 'woocommerce_enable_signup_and_login_from_checkout', 'yes' );
@ -471,6 +509,9 @@ class Checkout extends MockeryTestCase {
$customer = get_user_by( 'id', $data['customer_id'] );
$this->assertEquals( $customer->user_email, 'testaccount@test.com' );
// Return WC_Session to original state.
WC()->session = $old_session;
}
/**
@ -518,4 +559,56 @@ class Checkout extends MockeryTestCase {
$this->assertEquals( 400, $status, print_r( $data, true ) );
}
/**
* Test checkout without valid shipping methods.
*/
public function test_checkout_invalid_shipping_method() {
global $wpdb;
$shipping_methods = \WC_Shipping_Zones::get_zone( 0 )->get_shipping_methods();
foreach ( $shipping_methods as $shipping_method ) {
$wpdb->update( "{$wpdb->prefix}woocommerce_shipping_zone_methods", array( 'is_enabled' => '0' ), array( 'instance_id' => absint( $shipping_method->instance_id ) ) );
}
$request = new \WP_REST_Request( 'POST', '/wc/store/v1/checkout' );
$request->set_header( 'Nonce', wp_create_nonce( 'wc_store_api' ) );
$request->set_body_params(
array(
'billing_address' => (object) array(
'first_name' => 'test',
'last_name' => 'test',
'company' => '',
'address_1' => 'test',
'address_2' => '',
'city' => 'test',
'state' => '',
'postcode' => 'cb241ab',
'country' => 'GB',
'phone' => '',
'email' => 'testaccount@test.com',
),
'shipping_address' => (object) array(
'first_name' => 'test',
'last_name' => 'test',
'company' => '',
'address_1' => 'test',
'address_2' => '',
'city' => 'test',
'state' => '',
'postcode' => 'cb241ab',
'country' => 'GB',
'phone' => '',
),
'payment_method' => 'bacs',
)
);
$response = rest_get_server()->dispatch( $request );
$status = $response->get_status();
$data = $response->get_data();
$this->assertEquals( 400, $status, print_r( $data, true ) );
$this->assertEquals( 'woocommerce_rest_invalid_shipping_option', $data['code'], print_r( $data, true ) );
$this->assertEquals( 'Sorry, this order requires a shipping option.', $data['message'], print_r( $data, true ) );
}
}

View File

@ -21,18 +21,27 @@ class OrderControllerTests extends TestCase {
$this->expectException( RouteException::class );
$class->validate_selected_shipping_methods( true, array( false ) );
$class->validate_selected_shipping_methods( true, null );
}
/**
* test_validate_selected_shipping_methods.
*/
public function test_validate_selected_shipping_methods() {
// Add a flat rate to the default zone.
$flat_rate = WC()->shipping()->get_shipping_methods()['flat_rate'];
$default_zone = \WC_Shipping_Zones::get_zone( 0 );
$default_zone->add_shipping_method( $flat_rate->id );
$default_zone->save();
$class = new OrderController();
$registered_methods = \WC_Shipping_Zones::get_zone( 0 )->get_shipping_methods();
$valid_method = array_shift( $registered_methods );
// By running this method we assert that it doesn't error because if it does this test will fail.
$class->validate_selected_shipping_methods( true, array( 'free-shipping' ) );
$class->validate_selected_shipping_methods( true, array( $valid_method->id . ':' . $valid_method->instance_id ) );
$class->validate_selected_shipping_methods( false, array( 'free-shipping' ) );
$class->validate_selected_shipping_methods( true, null );
// The above methods throw Exception on error, but this is classed as a risky test because there are no
// assertions. Assert true to work around this warning.
$this->assertTrue( true );