Jetpack Connection: Add inline comments when whole input is processed (…

…#37392)

* Jetpack Connection: Add inline comments when whole input is processed

---------

Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>

projects/packages/connection/changelog/update-connection-processing-whole-input-comments

@@ -0,0 +1,4 @@
+Significance: patch
+Type: deprecated
+
+Jetpack Connection Manager: Deprecate `request_params` arg in setup_xmlrpc_handlers method

projects/packages/connection/legacy/class-jetpack-signature.php

@@ -93,9 +93,9 @@ public function sign_current_request( $override = array() ) {
 			// Convert the $_POST to the body, if the body was empty. This is how arrays are hashed
 			// and encoded on the Jetpack side.
 			if ( defined( 'IS_WPCOM' ) && IS_WPCOM ) {
-				// phpcs:ignore WordPress.Security.NonceVerification.Missing
+				// phpcs:ignore WordPress.Security.NonceVerification.Missing -- Used to generate a cryptographic signature of the post data.
 				if ( empty( $body ) && is_array( $_POST ) && $_POST !== array() ) {
-					$body = $_POST; // phpcs:ignore WordPress.Security.NonceVerification.Missing
+					$body = $_POST; // phpcs:ignore WordPress.Security.NonceVerification.Missing -- We need all of $_POST in order to generate a cryptographic signature of the post data.
 				}
 			}
 		} elseif ( isset( $_SERVER['REQUEST_METHOD'] ) && 'PUT' === strtoupper( $_SERVER['REQUEST_METHOD'] ) ) { // phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- This is validating.

projects/packages/connection/src/class-authorize-json-api.php

@@ -40,9 +40,17 @@ class Authorize_Json_Api {
 	 * @return void
 	 */
 	public function verify_json_api_authorization_request( $environment = null ) {
-		$environment = $environment === null
-			? $_REQUEST // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- nonce verification handled later in function.
-			: $environment;
+		if ( null === $environment ) {
+			$request_params_needed_for_auth = array(
+				'data',
+				'jetpack_json_api_original_query',
+				'nonce',
+				'signature',
+				'timestamp',
+				'token',
+			);
+			$environment                    = array_intersect_key( $request_params_needed_for_auth, $_REQUEST ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended,WordPress.Security.ValidatedSanitizedInput.MissingUnslash,WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- nonce verification handled later in function and request data are 1) used to verify a cryptographic signature of the request data and 2) sanitized later in function.
+		}
 
 		if ( ! isset( $environment['token'] ) ) {
 			wp_die( esc_html__( 'You must connect your Jetpack plugin to WordPress.com to use this feature.', 'jetpack-connection' ) );

projects/packages/connection/src/class-manager.php

@@ -111,7 +111,7 @@ public static function configure() {
 		);
 
 		$manager->setup_xmlrpc_handlers(
-			$_GET, // phpcs:ignore WordPress.Security.NonceVerification.Recommended
+			null,
 			$manager->has_connected_owner(),
 			$manager->verify_xml_rpc_signature()
 		);
@@ -161,32 +161,31 @@ public static function configure() {
 	 * Sets up the XMLRPC request handlers.
 	 *
 	 * @since 1.25.0 Deprecate $is_active param.
+	 * @since $$next-version$$ Deprecate $request_params param.
 	 *
-	 * @param array                 $request_params incoming request parameters.
+	 * @param array|null            $deprecated Deprecated. Not used.
 	 * @param bool                  $has_connected_owner Whether the site has a connected owner.
 	 * @param bool                  $is_signed whether the signature check has been successful.
 	 * @param Jetpack_XMLRPC_Server $xmlrpc_server (optional) an instance of the server to use instead of instantiating a new one.
 	 */
 	public function setup_xmlrpc_handlers(
-		$request_params,
+		$deprecated,
 		$has_connected_owner,
 		$is_signed,
 		Jetpack_XMLRPC_Server $xmlrpc_server = null
 	) {
 		add_filter( 'xmlrpc_blog_options', array( $this, 'xmlrpc_options' ), 1000, 2 );
-
-		if (
-			! isset( $request_params['for'] )
-			|| 'jetpack' !== $request_params['for']
-		) {
+		if ( $deprecated !== null ) {
+			_deprecated_argument( __METHOD__, '$$next-version$$' );
+		}
+		// phpcs:ignore WordPress.Security.NonceVerification.Recommended -- We are using the 'for' request param to early return unless it's 'jetpack'.
+		if ( ! isset( $_GET['for'] ) || 'jetpack' !== $_GET['for'] ) {
 			return false;
 		}
 
 		// Alternate XML-RPC, via ?for=jetpack&jetpack=comms.
-		if (
-			isset( $request_params['jetpack'] )
-			&& 'comms' === $request_params['jetpack']
-		) {
+		// phpcs:ignore WordPress.Security.NonceVerification.Recommended -- This just determines whether to handle the request as an XML-RPC request. The actual XML-RPC endpoints do the appropriate nonce checking where applicable. Plus we make sure to clear all cookies via require_jetpack_authentication called later in method.
+		if ( isset( $_GET['jetpack'] ) && 'comms' === $_GET['jetpack'] ) {
 			if ( ! Constants::is_defined( 'XMLRPC_REQUEST' ) ) {
 				// Use the real constant here for WordPress' sake.
 				define( 'XMLRPC_REQUEST', true );
@@ -455,9 +454,9 @@ private function internal_verify_xml_rpc_signature() {
 		}
 
 		$jetpack_signature = new \Jetpack_Signature( $token->secret, (int) \Jetpack_Options::get_option( 'time_diff' ) );
-		// phpcs:disable WordPress.Security.NonceVerification.Missing
+		// phpcs:disable WordPress.Security.NonceVerification.Missing -- Used to verify a cryptographic signature of the post data.
 		if ( isset( $_POST['_jetpack_is_multipart'] ) ) {
-			$post_data   = $_POST;
+			$post_data   = $_POST; // We need all of $_POST in order to verify a cryptographic signature of the post data.
 			$file_hashes = array();
 			foreach ( $post_data as $post_data_key => $post_data_value ) {
 				if ( ! str_starts_with( $post_data_key, '_jetpack_file_hmac_' ) ) {

projects/packages/connection/src/class-webhooks.php

@@ -107,7 +107,7 @@ public function handle_authorize() {
 		}
 		do_action( 'jetpack_client_authorize_processing' );
 
-		$data              = stripslashes_deep( $_GET );
+		$data              = stripslashes_deep( $_GET ); // We need all request data under the context of an authorization request.
 		$data['auth_type'] = 'client';
 		$roles             = new Roles();
 		$role              = $roles->translate_current_user_to_role();

projects/packages/connection/src/sso/class-user-admin.php

@@ -835,7 +835,7 @@ public function render_custom_email_message_form_field( $type ) {
 	 * @return boolean Indicating if the core invitation main should be sent.
 	 */
 	public function should_send_wp_mail_new_user( $send_wp_email ) {
-		if ( ! isset( $_POST['invite_user_wpcom'] ) && isset( $_POST['send_user_notification'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Missing
+		if ( ! isset( $_POST['invite_user_wpcom'] ) && isset( $_POST['send_user_notification'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Missing -- Hooked to 'wp_send_new_user_notification_to_user' to conditionally disable the core invitation email. At this point nonces should be checked already.
 			return $send_wp_email;
 		}
 		return false;

projects/plugins/jetpack/changelog/update-connection-processing-whole-input-comments

@@ -0,0 +1,5 @@
+Significance: patch
+Type: other
+Comment: Jetpack: Update connection related unit tests
+
+

projects/plugins/jetpack/tests/php/general/test_class.jetpack.php

@@ -96,6 +96,7 @@ public function set_up() {
 	 */
 	public function tear_down() {
 		parent::tear_down();
+		unset( $_GET['for'] );
 		Constants::clear_constants();
 		StatusCache::clear();
 	}
@@ -770,11 +771,13 @@ public static function filter_pre_update_option_jetpack_active_modules( $modules
 	private function mocked_setup_xmlrpc_handlers( $request_params, $has_connected_owner, $is_signed, $user = false ) {
 		$GLOBALS['HTTP_RAW_POST_DATA'] = '';
 
+		$_GET['for'] = $request_params['for'];
+
 		Constants::set_constant( 'XMLRPC_REQUEST', true );
 
 		$jetpack       = new MockJetpack();
 		$xmlrpc_server = new MockJetpack_XMLRPC_Server( $user );
-		return $jetpack::connection()->setup_xmlrpc_handlers( $request_params, $has_connected_owner, $is_signed, $xmlrpc_server );
+		return $jetpack::connection()->setup_xmlrpc_handlers( null, $has_connected_owner, $is_signed, $xmlrpc_server );
 	}
 
 	/**