diff --git a/plugins/woocommerce/changelog/fix-45912-shipping-options b/plugins/woocommerce/changelog/fix-45912-shipping-options new file mode 100644 index 00000000000..cfcec894ce0 --- /dev/null +++ b/plugins/woocommerce/changelog/fix-45912-shipping-options @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Fixed an issue where orders could be placed when no shipping options were available diff --git a/plugins/woocommerce/src/StoreApi/Utilities/OrderController.php b/plugins/woocommerce/src/StoreApi/Utilities/OrderController.php index d27e0025bd0..b8c0e87b621 100644 --- a/plugins/woocommerce/src/StoreApi/Utilities/OrderController.php +++ b/plugins/woocommerce/src/StoreApi/Utilities/OrderController.php @@ -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; } } } diff --git a/plugins/woocommerce/tests/php/src/Blocks/Helpers/FixtureData.php b/plugins/woocommerce/tests/php/src/Blocks/Helpers/FixtureData.php index 3a2d2bb6156..f1342196129 100644 --- a/plugins/woocommerce/tests/php/src/Blocks/Helpers/FixtureData.php +++ b/plugins/woocommerce/tests/php/src/Blocks/Helpers/FixtureData.php @@ -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. */ diff --git a/plugins/woocommerce/tests/php/src/Blocks/StoreApi/MockSessionHandler.php b/plugins/woocommerce/tests/php/src/Blocks/StoreApi/MockSessionHandler.php new file mode 100644 index 00000000000..916d33e65c7 --- /dev/null +++ b/plugins/woocommerce/tests/php/src/Blocks/StoreApi/MockSessionHandler.php @@ -0,0 +1,328 @@ +_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, + ) + ); + } +} diff --git a/plugins/woocommerce/tests/php/src/Blocks/StoreApi/Routes/Checkout.php b/plugins/woocommerce/tests/php/src/Blocks/StoreApi/Routes/Checkout.php index 59304c6b0f4..e7814c25d42 100644 --- a/plugins/woocommerce/tests/php/src/Blocks/StoreApi/Routes/Checkout.php +++ b/plugins/woocommerce/tests/php/src/Blocks/StoreApi/Routes/Checkout.php @@ -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 ) ); + } } diff --git a/plugins/woocommerce/tests/php/src/Blocks/StoreApi/Utilities/OrderController.php b/plugins/woocommerce/tests/php/src/Blocks/StoreApi/Utilities/OrderController.php index 1e390540c6e..b4dd0742531 100644 --- a/plugins/woocommerce/tests/php/src/Blocks/StoreApi/Utilities/OrderController.php +++ b/plugins/woocommerce/tests/php/src/Blocks/StoreApi/Utilities/OrderController.php @@ -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 );