From b2c81135256dacfe96a596eccff740e881fa2839 Mon Sep 17 00:00:00 2001 From: Claudio Sanches Date: Tue, 30 Mar 2021 13:10:21 -0300 Subject: [PATCH 1/7] Change UID only for WooCommerce cookies --- includes/class-wc-session-handler.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/includes/class-wc-session-handler.php b/includes/class-wc-session-handler.php index cacda9e59a2..163b78fec4a 100644 --- a/includes/class-wc-session-handler.php +++ b/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, 'nonce_user_logged_out' ), 10, 2 ); } } @@ -288,11 +288,16 @@ 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. * - * @param int $uid User ID. - * @return string + * @param int $uid User ID. + * @param string $action The nonce action. + * @return int|string */ - public function nonce_user_logged_out( $uid ) { - return $this->has_session() && $this->_customer_id ? $this->_customer_id : $uid; + public function 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; } /** From 65b024a96d2330298aa4e2f758041dba9039e86f Mon Sep 17 00:00:00 2001 From: Claudio Sanches Date: Tue, 30 Mar 2021 14:11:24 -0300 Subject: [PATCH 2/7] Moved to a new function --- includes/class-wc-session-handler.php | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/includes/class-wc-session-handler.php b/includes/class-wc-session-handler.php index 163b78fec4a..1ba0095d3d0 100644 --- a/includes/class-wc-session-handler.php +++ b/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' ), 10, 2 ); + add_filter( 'nonce_user_logged_out', array( $this, 'maybe_update_nonce_user_logged_out' ), 10, 2 ); } } @@ -288,11 +288,24 @@ 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 int|string + */ + public function nonce_user_logged_out( $uid ) { + 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 nonce_user_logged_out( $uid, $action ) { + 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; } From 2da3a3745077cfad6a486d2d9220336e1130e83c Mon Sep 17 00:00:00 2001 From: Claudio Sanches Date: Tue, 30 Mar 2021 14:11:33 -0300 Subject: [PATCH 3/7] Added unit test --- includes/class-wc-session-handler.php | 19 +++++++++++ .../class-wc-tests-session-handler.php | 34 ++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/includes/class-wc-session-handler.php b/includes/class-wc-session-handler.php index 1ba0095d3d0..e486d3666ff 100644 --- a/includes/class-wc-session-handler.php +++ b/includes/class-wc-session-handler.php @@ -187,6 +187,25 @@ class WC_Session_Handler extends WC_Session { return $customer_id; } + /** + * Get session unique ID for quests 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. * diff --git a/tests/legacy/unit-tests/session/class-wc-tests-session-handler.php b/tests/legacy/unit-tests/session/class-wc-tests-session-handler.php index 4149f90fbb6..7af97b30165 100644 --- a/tests/legacy/unit-tests/session/class-wc-tests-session-handler.php +++ b/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 ) { From f913dc09dda51a4be2beb410cb77c3f84e7cab29 Mon Sep 17 00:00:00 2001 From: Claudio Sanches Date: Tue, 30 Mar 2021 14:53:37 -0300 Subject: [PATCH 4/7] Improve get_customer_unique_id() --- includes/class-wc-session-handler.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/includes/class-wc-session-handler.php b/includes/class-wc-session-handler.php index e486d3666ff..4113e4147ce 100644 --- a/includes/class-wc-session-handler.php +++ b/includes/class-wc-session-handler.php @@ -195,11 +195,9 @@ class WC_Session_Handler extends WC_Session { * @return string */ public function get_customer_unique_id() { - $customer_id = ''; + $customer_id = $this->has_session() && $this->_customer_id ? $this->_customer_id : ''; - if ( $this->has_session() && $this->_customer_id ) { - $customer_id = $this->_customer_id; - } elseif ( is_user_logged_in() ) { + if ( is_user_logged_in() ) { $customer_id = (string) get_current_user_id(); } From 076248aca00f776398da642c740c950cb4fc832b Mon Sep 17 00:00:00 2001 From: Claudio Sanches Date: Tue, 30 Mar 2021 18:17:12 -0300 Subject: [PATCH 5/7] Fixed logic of get_customer_unique_id --- includes/class-wc-session-handler.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/includes/class-wc-session-handler.php b/includes/class-wc-session-handler.php index 4113e4147ce..e486d3666ff 100644 --- a/includes/class-wc-session-handler.php +++ b/includes/class-wc-session-handler.php @@ -195,9 +195,11 @@ class WC_Session_Handler extends WC_Session { * @return string */ public function get_customer_unique_id() { - $customer_id = $this->has_session() && $this->_customer_id ? $this->_customer_id : ''; + $customer_id = ''; - if ( is_user_logged_in() ) { + if ( $this->has_session() && $this->_customer_id ) { + $customer_id = $this->_customer_id; + } elseif ( is_user_logged_in() ) { $customer_id = (string) get_current_user_id(); } From 5ebab07677625e3919ce663075062a36c822d21d Mon Sep 17 00:00:00 2001 From: Claudio Sanches Date: Wed, 31 Mar 2021 19:25:28 -0300 Subject: [PATCH 6/7] Fixed typo --- includes/class-wc-session-handler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/class-wc-session-handler.php b/includes/class-wc-session-handler.php index e486d3666ff..13771e21f31 100644 --- a/includes/class-wc-session-handler.php +++ b/includes/class-wc-session-handler.php @@ -188,7 +188,7 @@ class WC_Session_Handler extends WC_Session { } /** - * Get session unique ID for quests if session is initialized or user ID if logged in. + * 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 From 6540b804b7e8530c10744a7a0950f9cc3d675be7 Mon Sep 17 00:00:00 2001 From: Claudio Sanches Date: Wed, 31 Mar 2021 19:29:26 -0300 Subject: [PATCH 7/7] Added deprecated notice to nonce_user_logged_out --- includes/class-wc-session-handler.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/includes/class-wc-session-handler.php b/includes/class-wc-session-handler.php index 13771e21f31..0c6dda12484 100644 --- a/includes/class-wc-session-handler.php +++ b/includes/class-wc-session-handler.php @@ -312,6 +312,8 @@ class WC_Session_Handler extends WC_Session { * @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; }