Merge pull request #29542 from woocommerce/fix/23682.1

Update UID only for WooCommerce cookies

includes/class-wc-session-handler.php

@@ -75,7 +75,7 @@ class WC_Session_Handler extends WC_Session {
 		add_action( 'wp_logout', array( $this, 'destroy_session' ) );
 
 		if ( ! is_user_logged_in() ) {
-			add_filter( 'nonce_user_logged_out', array( $this, 'nonce_user_logged_out' ) );
+			add_filter( 'nonce_user_logged_out', array( $this, 'maybe_update_nonce_user_logged_out' ), 10, 2 );
 		}
 	}
 
@@ -187,6 +187,25 @@ class WC_Session_Handler extends WC_Session {
 		return $customer_id;
 	}
 
+	/**
+	 * 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;
+	}
+
 	/**
 	 * Get the session cookie, if set. Otherwise return false.
 	 *
@@ -288,13 +307,33 @@ class WC_Session_Handler extends WC_Session {
 	/**
 	 * When a user is logged out, ensure they have a unique nonce by using the customer/session ID.
 	 *
+	 * @deprecated 5.3.0
 	 * @param int $uid User ID.
-	 * @return string
+	 * @return int|string
 	 */
 	public function nonce_user_logged_out( $uid ) {
+		wc_deprecated_function( 'WC_Session_Handler::nonce_user_logged_out', '5.3', 'WC_Session_Handler::maybe_update_nonce_user_logged_out' );
+
 		return $this->has_session() && $this->_customer_id ? $this->_customer_id : $uid;
 	}
 
+	/**
+	 * When a user is logged out, ensure they have a unique nonce to manage cart and more using the customer/session ID.
+	 * This filter runs everything `wp_verify_nonce()` and `wp_create_nonce()` gets called.
+	 *
+	 * @since 5.3.0
+	 * @param int    $uid    User ID.
+	 * @param string $action The nonce action.
+	 * @return int|string
+	 */
+	public function maybe_update_nonce_user_logged_out( $uid, $action ) {
+		if ( Automattic\WooCommerce\Utilities\StringUtil::starts_with( $action, 'woocommerce' ) ) {
+			return $this->has_session() && $this->_customer_id ? $this->_customer_id : $uid;
+		}
+
+		return $uid;
+	}
+
 	/**
 	 * Cleanup session data from the database and clear caches.
 	 */

tests/legacy/unit-tests/session/class-wc-tests-session-handler.php

@@ -10,6 +10,9 @@
  */
 class WC_Tests_Session_Handler extends WC_Unit_Test_Case {
 
+	/**
+	 * Setup.
+	 */
 	public function setUp() {
 		parent::setUp();
 
@@ -17,6 +20,9 @@ class WC_Tests_Session_Handler extends WC_Unit_Test_Case {
 		$this->create_session();
 	}
 
+	/**
+	 * @testdox Test that save data should insert new row.
+	 */
 	public function test_save_data_should_insert_new_row() {
 		$current_session_data = $this->get_session_from_db( $this->session_key );
 		// delete session to make sure a new row is created in the DB.
@@ -35,6 +41,9 @@ class WC_Tests_Session_Handler extends WC_Unit_Test_Case {
 		$this->assertEquals( array( 'cart' => 'new cart' ), wp_cache_get( $this->cache_prefix . $this->session_key, WC_SESSION_CACHE_GROUP ) );
 	}
 
+	/**
+	 * @testdox Test that save data should replace existing row.
+	 */
 	public function test_save_data_should_replace_existing_row() {
 		$current_session_data = $this->get_session_from_db( $this->session_key );
 
@@ -49,23 +58,35 @@ class WC_Tests_Session_Handler extends WC_Unit_Test_Case {
 		$this->assertTrue( is_numeric( $updated_session_data->session_expiry ) );
 	}
 
+	/**
+	 * @testdox Test that get_setting() should use cache.
+	 */
 	public function test_get_session_should_use_cache() {
 		$session = $this->handler->get_session( $this->session_key );
 		$this->assertEquals( array( 'cart' => 'fake cart' ), $session );
 	}
 
+	/**
+	 * @testdox Test that get_setting() shouldn't use cache.
+	 */
 	public function test_get_session_should_not_use_cache() {
 		wp_cache_delete( $this->cache_prefix . $this->session_key, WC_SESSION_CACHE_GROUP );
 		$session = $this->handler->get_session( $this->session_key );
 		$this->assertEquals( array( 'cart' => 'fake cart' ), $session );
 	}
 
+	/**
+	 * @testdox Test that get_setting() should return default value.
+	 */
 	public function test_get_session_should_return_default_value() {
 		$default_session = array( 'session' => 'default' );
 		$session         = $this->handler->get_session( 'non-existent key', $default_session );
 		$this->assertEquals( $default_session, $session );
 	}
 
+	/**
+	 * @testdox Test delete_session().
+	 */
 	public function test_delete_session() {
 		global $wpdb;
 
@@ -82,6 +103,9 @@ class WC_Tests_Session_Handler extends WC_Unit_Test_Case {
 		$this->assertNull( $session_id );
 	}
 
+	/**
+	 * @testdox Test update_session_timestamp().
+	 */
 	public function test_update_session_timestamp() {
 		global $wpdb;
 
@@ -98,6 +122,14 @@ class WC_Tests_Session_Handler extends WC_Unit_Test_Case {
 		$this->assertEquals( $timestamp, $session_expiry );
 	}
 
+	/**
+	 * @testdox Test that nonce of user logged out is only changed by WooCommerce.
+	 */
+	public function test_maybe_update_nonce_user_logged_out() {
+		$this->assertEquals( 1, $this->handler->maybe_update_nonce_user_logged_out( 1, 'wp_rest' ) );
+		$this->assertEquals( $this->handler->get_customer_unique_id(), $this->handler->maybe_update_nonce_user_logged_out( 1, 'woocommerce-something' ) );
+	}
+
 	/**
 	 * Helper function to create a WC session and save it to the DB.
 	 */
@@ -113,7 +145,7 @@ class WC_Tests_Session_Handler extends WC_Unit_Test_Case {
 	/**
 	 * Helper function to get session data from DB.
 	 *
-	 * @param string $session_key
+	 * @param string $session_key Session key.
 	 * @return stdClass
 	 */
 	protected function get_session_from_db( $session_key ) {