From 2c7db1906d33e223cc86d1660c960890b27ece0d Mon Sep 17 00:00:00 2001 From: Enej Bajgoric Date: Mon, 8 Jan 2018 10:01:28 -0800 Subject: [PATCH 1/8] Sync send: Fix callables set the user if we do have one --- sync/class.jetpack-sync-sender.php | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/sync/class.jetpack-sync-sender.php b/sync/class.jetpack-sync-sender.php index fa80a3af24f4c..5760387699092 100644 --- a/sync/class.jetpack-sync-sender.php +++ b/sync/class.jetpack-sync-sender.php @@ -25,6 +25,7 @@ class Jetpack_Sync_Sender { private $sync_queue; private $full_sync_queue; private $codec; + private $old_user; // singleton functions private static $instance; @@ -44,11 +45,31 @@ protected function __construct() { } private function init() { + // + add_action( 'jetpack_sync_before_send_queue_sync', array( $this, 'maybe_set_user_from_token' ), 1 ); + add_action( 'jetpack_sync_before_send_queue_sync', array( $this, 'maybe_clear_user_from_token' ), 20 ); foreach ( Jetpack_Sync_Modules::get_modules() as $module ) { $module->init_before_send(); } } + public function maybe_set_user_from_token() { + $jetpack = Jetpack::init(); + if ( $jetpack->verify_xml_rpc_signature() ) { + $verified_user = $jetpack->verify_xml_rpc_signature(); + $old_user = wp_get_current_user(); + $this->old_user = isset( $old_user->ID ) ? $old_user->ID : 0; + $this->old_user = + wp_set_current_user( $verified_user['user_id'] ); + } + } + + public function maybe_clear_user_from_token() { + if ( isset( $this->old_user ) ) { + wp_set_current_user( $this->old_user ); + } + } + public function get_next_sync_time( $queue_name ) { return (double) get_option( self::NEXT_SYNC_TIME_OPTION_NAME . '_' . $queue_name, 0 ); } From bbafbfa28a498967505c00ea2bde0b9853c2dd9f Mon Sep 17 00:00:00 2001 From: Michael Turk Date: Mon, 8 Jan 2018 15:43:50 -0500 Subject: [PATCH 2/8] Fixed formatting --- sync/class.jetpack-sync-sender.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sync/class.jetpack-sync-sender.php b/sync/class.jetpack-sync-sender.php index 5760387699092..981f9ad7df1d2 100644 --- a/sync/class.jetpack-sync-sender.php +++ b/sync/class.jetpack-sync-sender.php @@ -45,7 +45,6 @@ protected function __construct() { } private function init() { - // add_action( 'jetpack_sync_before_send_queue_sync', array( $this, 'maybe_set_user_from_token' ), 1 ); add_action( 'jetpack_sync_before_send_queue_sync', array( $this, 'maybe_clear_user_from_token' ), 20 ); foreach ( Jetpack_Sync_Modules::get_modules() as $module ) { @@ -59,8 +58,7 @@ public function maybe_set_user_from_token() { $verified_user = $jetpack->verify_xml_rpc_signature(); $old_user = wp_get_current_user(); $this->old_user = isset( $old_user->ID ) ? $old_user->ID : 0; - $this->old_user = - wp_set_current_user( $verified_user['user_id'] ); + $this->old_user = wp_set_current_user( $verified_user['user_id'] ); } } From 95f8d0be916df470a7e5fb67c99b9256c94ca147 Mon Sep 17 00:00:00 2001 From: Michael Turk Date: Tue, 9 Jan 2018 17:55:37 -0500 Subject: [PATCH 3/8] Updated to check for XMLRPC_REQUEST before checking for user from token --- sync/class.jetpack-sync-sender.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sync/class.jetpack-sync-sender.php b/sync/class.jetpack-sync-sender.php index 981f9ad7df1d2..f6d8e1d46ec17 100644 --- a/sync/class.jetpack-sync-sender.php +++ b/sync/class.jetpack-sync-sender.php @@ -54,12 +54,16 @@ private function init() { public function maybe_set_user_from_token() { $jetpack = Jetpack::init(); - if ( $jetpack->verify_xml_rpc_signature() ) { + if ( defined( 'XMLRPC_REQUEST' ) && + XMLRPC_REQUEST && + $jetpack->verify_xml_rpc_signature() + ) { $verified_user = $jetpack->verify_xml_rpc_signature(); $old_user = wp_get_current_user(); $this->old_user = isset( $old_user->ID ) ? $old_user->ID : 0; $this->old_user = wp_set_current_user( $verified_user['user_id'] ); } + } public function maybe_clear_user_from_token() { From 331a6b85d8cce84e91bb95a55faa85da13dd88e5 Mon Sep 17 00:00:00 2001 From: Michael Turk Date: Wed, 10 Jan 2018 10:14:33 -0500 Subject: [PATCH 4/8] Updated the check for XMLRPC_REQUEST --- sync/class.jetpack-sync-sender.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sync/class.jetpack-sync-sender.php b/sync/class.jetpack-sync-sender.php index f6d8e1d46ec17..3892ee486bb6f 100644 --- a/sync/class.jetpack-sync-sender.php +++ b/sync/class.jetpack-sync-sender.php @@ -54,8 +54,7 @@ private function init() { public function maybe_set_user_from_token() { $jetpack = Jetpack::init(); - if ( defined( 'XMLRPC_REQUEST' ) && - XMLRPC_REQUEST && + if ( Jetpack_Constants::is_true( 'XMLRPC_REQUEST' ) && $jetpack->verify_xml_rpc_signature() ) { $verified_user = $jetpack->verify_xml_rpc_signature(); From 617130c8e9bd4d5215ad7f8de134041bad6083bb Mon Sep 17 00:00:00 2001 From: Enej Bajgoric Date: Wed, 17 Jan 2018 12:33:05 -0800 Subject: [PATCH 5/8] Add tests --- sync/class.jetpack-sync-sender.php | 12 +-- .../test_class.jetpack-sync-callables.php | 85 +++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/sync/class.jetpack-sync-sender.php b/sync/class.jetpack-sync-sender.php index 3892ee486bb6f..47ed3f0bb2940 100644 --- a/sync/class.jetpack-sync-sender.php +++ b/sync/class.jetpack-sync-sender.php @@ -52,24 +52,24 @@ private function init() { } } - public function maybe_set_user_from_token() { + public function maybe_set_user_from_token( ) { $jetpack = Jetpack::init(); + $verified_user = $jetpack->verify_xml_rpc_signature(); if ( Jetpack_Constants::is_true( 'XMLRPC_REQUEST' ) && - $jetpack->verify_xml_rpc_signature() + ! is_wp_error( $verified_user ) + && $verified_user ) { - $verified_user = $jetpack->verify_xml_rpc_signature(); $old_user = wp_get_current_user(); $this->old_user = isset( $old_user->ID ) ? $old_user->ID : 0; - $this->old_user = wp_set_current_user( $verified_user['user_id'] ); + wp_set_current_user( $verified_user['user_id'] ); } - } public function maybe_clear_user_from_token() { if ( isset( $this->old_user ) ) { wp_set_current_user( $this->old_user ); } - } + } public function get_next_sync_time( $queue_name ) { return (double) get_option( self::NEXT_SYNC_TIME_OPTION_NAME . '_' . $queue_name, 0 ); diff --git a/tests/php/sync/test_class.jetpack-sync-callables.php b/tests/php/sync/test_class.jetpack-sync-callables.php index 4d7d7d6eacf41..92130f6b7c2f4 100644 --- a/tests/php/sync/test_class.jetpack-sync-callables.php +++ b/tests/php/sync/test_class.jetpack-sync-callables.php @@ -15,6 +15,8 @@ class WP_Test_Jetpack_Sync_Functions extends WP_Test_Jetpack_Sync_Base { protected $post; protected $callable_module; + protected static $admin_id; // used in mock_xml_rpc_request + public function setUp() { parent::setUp(); @@ -846,6 +848,89 @@ function test_force_sync_callabled_on_plugin_update() { $this->assertNotEmpty( $synced_value3, 'value is empty!' ); } + + function test_xml_rpc_request_callables_has_() { + $this->server_event_storage->reset(); + $user = wp_get_current_user(); + wp_set_current_user( 0 ); // + $this->mock_authenicated_xml_rpc(); // mock requet + $this->sender->do_sync(); + $events = $this->server_event_storage->get_all_events(); + foreach( $events as $e ) { + var_dump( $e->action ); + } + + $event = $this->server_event_storage->get_most_recent_event( 'jetpack_sync_callable' ); + $this->mock_autehicated_xml_rpc_cleanup( $user->ID ); + $this->assertEquals( $event->user_id, self::$admin_id, ' Callables XMLRPC_Reqeust not equal to event user_id' ); + } + + function mock_authenicated_xml_rpc() { + self::$admin_id = $this->factory->user->create( array( + 'role' => 'administrator', + ) ); + + var_dump( 'mock_reques'); + + add_filter( 'pre_option_jetpack_private_options', array( $this, 'mock_jetpack_private_options' ), 10, 2 ); + $_GET['token'] = 'pretend_this_is_valid:1:' . self::$admin_id; + $_GET['timestamp'] = (string) time(); + $_GET['nonce'] = 'testing123'; + var_dump( $_SERVER['REQUEST_URI'] ); + $_SERVER['REQUEST_URI'] = '/xmlrpc.php'; + $_GET['body'] = 'abc'; + $_GET['body-hash'] = jetpack_sha1_base64( 'abc' ); + $GLOBALS['HTTP_RAW_POST_DATA'] = 'abc'; + $_SERVER['REQUEST_METHOD'] = 'POST'; + + $normalized_request_pieces = array( + $_GET['token'], + $_GET['timestamp'], + $_GET['nonce'], + $_GET['body-hash'], + 'POST', + 'example.org', + '80', + '/xmlrpc.php', + ); + $normalize = join( "\n", $normalized_request_pieces ) . "\n"; + + $_GET['signature'] = base64_encode( hash_hmac( 'sha1', $normalize , 'secret', true ) ); + + // call one of the autheticad endpoints + Jetpack_Constants::set_constant( 'XMLRPC_REQUEST', true ); + $jetpack = Jetpack::init(); + $jetpack->xmlrpc_methods( array() ); + $jetpack->require_jetpack_authentication(); + $jetpack->verify_xml_rpc_signature(); + } + + function mock_autehicated_xml_rpc_cleanup( $user_id ) { + Jetpack_Constants::clear_constants(); + remove_filter( 'pre_option_jetpack_private_options', array( $this, 'mock_jetpack_private_options' ), 10 ); + + unset( $_GET['token'] ); + unset( $_GET['timestamp'] ); + unset( $_GET['nonce'] ); + $_SERVER['REQUEST_URI'] = ''; + unset( $_GET['body'] ); + unset( $_GET['body-hash'] ) ; + unset( $GLOBALS['HTTP_RAW_POST_DATA'] ); + unset( $_SERVER['REQUEST_METHOD'] ); + $jetpack = Jetpack::init(); + $jetpack->reset_saved_auth_state(); + wp_set_current_user( $user_id ); + self::$admin_id = null; + } + + function mock_jetpack_private_options() { + $user_tokens = array(); + $user_tokens[ self::$admin_id ] = 'pretend_this_is_valid.secret.' . self::$admin_id; + return array( + 'user_tokens' => $user_tokens, + ); + } + } function jetpack_foo_is_callable_random() { From ad7f01e14e76c40891be8c7dd8770feac4991c91 Mon Sep 17 00:00:00 2001 From: Enej Bajgoric Date: Wed, 17 Jan 2018 12:37:20 -0800 Subject: [PATCH 6/8] Add an assertion --- .../sync/test_class.jetpack-sync-callables.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/php/sync/test_class.jetpack-sync-callables.php b/tests/php/sync/test_class.jetpack-sync-callables.php index 92130f6b7c2f4..51f8de95eacc3 100644 --- a/tests/php/sync/test_class.jetpack-sync-callables.php +++ b/tests/php/sync/test_class.jetpack-sync-callables.php @@ -813,7 +813,6 @@ function test_taxonomies_objects_do_not_have_meta_box_callback() { foreach ( $check_object_vars as $test ) { $this->assertObjectHasAttribute( $test, $taxonomy, "Taxonomy does not have expected {$test} attribute." ); } - } function test_force_sync_callabled_on_plugin_update() { @@ -849,19 +848,22 @@ function test_force_sync_callabled_on_plugin_update() { } - function test_xml_rpc_request_callables_has_() { + function test_xml_rpc_request_callables_has_actor() { $this->server_event_storage->reset(); $user = wp_get_current_user(); wp_set_current_user( 0 ); // + $this->sender->do_sync(); + $event = $this->server_event_storage->get_most_recent_event( 'jetpack_sync_callable' ); + $this->assertEquals( $event->user_id, 0, ' Callables user_id is null' ); + + $this->resetCallableAndConstantTimeouts(); $this->mock_authenicated_xml_rpc(); // mock requet $this->sender->do_sync(); - $events = $this->server_event_storage->get_all_events(); - foreach( $events as $e ) { - var_dump( $e->action ); - } $event = $this->server_event_storage->get_most_recent_event( 'jetpack_sync_callable' ); + // clean up $this->mock_autehicated_xml_rpc_cleanup( $user->ID ); + $this->assertEquals( $event->user_id, self::$admin_id, ' Callables XMLRPC_Reqeust not equal to event user_id' ); } @@ -870,13 +872,11 @@ function mock_authenicated_xml_rpc() { 'role' => 'administrator', ) ); - var_dump( 'mock_reques'); - add_filter( 'pre_option_jetpack_private_options', array( $this, 'mock_jetpack_private_options' ), 10, 2 ); $_GET['token'] = 'pretend_this_is_valid:1:' . self::$admin_id; $_GET['timestamp'] = (string) time(); $_GET['nonce'] = 'testing123'; - var_dump( $_SERVER['REQUEST_URI'] ); + $_SERVER['REQUEST_URI'] = '/xmlrpc.php'; $_GET['body'] = 'abc'; $_GET['body-hash'] = jetpack_sha1_base64( 'abc' ); From d99c046864b6dd109ff438ba4b2ba3024253808e Mon Sep 17 00:00:00 2001 From: Enej Bajgoric Date: Wed, 17 Jan 2018 19:08:47 -0800 Subject: [PATCH 7/8] Minor fix --- tests/php/sync/test_class.jetpack-sync-callables.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/sync/test_class.jetpack-sync-callables.php b/tests/php/sync/test_class.jetpack-sync-callables.php index 51f8de95eacc3..59f575d10d904 100644 --- a/tests/php/sync/test_class.jetpack-sync-callables.php +++ b/tests/php/sync/test_class.jetpack-sync-callables.php @@ -879,7 +879,7 @@ function mock_authenicated_xml_rpc() { $_SERVER['REQUEST_URI'] = '/xmlrpc.php'; $_GET['body'] = 'abc'; - $_GET['body-hash'] = jetpack_sha1_base64( 'abc' ); + $_GET['body-hash'] = base64_encode( sha1( 'abc', true ) ); $GLOBALS['HTTP_RAW_POST_DATA'] = 'abc'; $_SERVER['REQUEST_METHOD'] = 'POST'; From 8316ba67c7d2bd8c821da72b95fd7813814cf236 Mon Sep 17 00:00:00 2001 From: Michael Turk Date: Thu, 18 Jan 2018 10:07:01 -0500 Subject: [PATCH 8/8] Cleaning up method names and comment --- tests/php/sync/test_class.jetpack-sync-callables.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/php/sync/test_class.jetpack-sync-callables.php b/tests/php/sync/test_class.jetpack-sync-callables.php index 59f575d10d904..a5ce9414171fc 100644 --- a/tests/php/sync/test_class.jetpack-sync-callables.php +++ b/tests/php/sync/test_class.jetpack-sync-callables.php @@ -857,17 +857,17 @@ function test_xml_rpc_request_callables_has_actor() { $this->assertEquals( $event->user_id, 0, ' Callables user_id is null' ); $this->resetCallableAndConstantTimeouts(); - $this->mock_authenicated_xml_rpc(); // mock requet + $this->mock_authenticated_xml_rpc(); // mock requet $this->sender->do_sync(); $event = $this->server_event_storage->get_most_recent_event( 'jetpack_sync_callable' ); - // clean up - $this->mock_autehicated_xml_rpc_cleanup( $user->ID ); + // clean up by unsetting globals, etc. set previously by $this->mock_authenticated_xml_rpc() + $this->mock_authenticated_xml_rpc_cleanup( $user->ID ); $this->assertEquals( $event->user_id, self::$admin_id, ' Callables XMLRPC_Reqeust not equal to event user_id' ); } - function mock_authenicated_xml_rpc() { + function mock_authenticated_xml_rpc() { self::$admin_id = $this->factory->user->create( array( 'role' => 'administrator', ) ); @@ -897,7 +897,7 @@ function mock_authenicated_xml_rpc() { $_GET['signature'] = base64_encode( hash_hmac( 'sha1', $normalize , 'secret', true ) ); - // call one of the autheticad endpoints + // call one of the authenticated endpoints Jetpack_Constants::set_constant( 'XMLRPC_REQUEST', true ); $jetpack = Jetpack::init(); $jetpack->xmlrpc_methods( array() ); @@ -905,7 +905,7 @@ function mock_authenicated_xml_rpc() { $jetpack->verify_xml_rpc_signature(); } - function mock_autehicated_xml_rpc_cleanup( $user_id ) { + function mock_authenticated_xml_rpc_cleanup( $user_id ) { Jetpack_Constants::clear_constants(); remove_filter( 'pre_option_jetpack_private_options', array( $this, 'mock_jetpack_private_options' ), 10 );