From 2a5c644be47f940e0415bda073948b691ec26990 Mon Sep 17 00:00:00 2001 From: Timur Karimov Date: Tue, 19 Sep 2023 21:36:09 +0200 Subject: [PATCH] Prevent WooPay from modifying non-WooPay Webhooks (#7235) Co-authored-by: Timur Karimov --- .../fix-redundant-webhooks-modifications | 4 + .../woopay/class-woopay-order-status-sync.php | 6 ++ .../test-class-woopay-order-status-sync.php | 76 +++++++++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 changelog/fix-redundant-webhooks-modifications diff --git a/changelog/fix-redundant-webhooks-modifications b/changelog/fix-redundant-webhooks-modifications new file mode 100644 index 00000000000..5c7d87846af --- /dev/null +++ b/changelog/fix-redundant-webhooks-modifications @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Prevent WooPay-related implementation to modify non-WooPay-specific webhooks by changing their data. diff --git a/includes/woopay/class-woopay-order-status-sync.php b/includes/woopay/class-woopay-order-status-sync.php index 06b5fc674de..ae53828060d 100644 --- a/includes/woopay/class-woopay-order-status-sync.php +++ b/includes/woopay/class-woopay-order-status-sync.php @@ -135,6 +135,12 @@ public static function add_topics( $topic_hooks ) { * @param integer $id ID of the webhook. */ public static function create_payload( $payload, $resource, $resource_id, $id ) { + $webhook = wc_get_webhook( $id ); + if ( 0 !== strpos( $webhook->get_delivery_url(), WooPay_Utilities::get_woopay_rest_url( 'merchant-notification' ) ) ) { + // This is not a WooPay webhook, so we don't need to modify the payload. + return $payload; + } + return [ 'blog_id' => \Jetpack_Options::get_option( 'id' ), 'order_id' => $resource_id, diff --git a/tests/unit/woopay/test-class-woopay-order-status-sync.php b/tests/unit/woopay/test-class-woopay-order-status-sync.php index 2d8124af97c..987680d4bc4 100644 --- a/tests/unit/woopay/test-class-woopay-order-status-sync.php +++ b/tests/unit/woopay/test-class-woopay-order-status-sync.php @@ -43,6 +43,69 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { self::$admin_user = $factory->user->create_and_get( [ 'role' => 'administrator' ] ); } + /** + * Tests that WooPay-specific webhooks are modified as expected. + */ + public function test_woopay_specific_webhook_payload_is_updated() { + wp_set_current_user( self::$admin_user->ID ); + $pre_processing_payload = [ + 'status' => 'publish', + 'average_rating' => '0.00', + 'catalog_visibility' => 'visible', + 'categories' => [ + [ + 'id' => 9, + 'name' => 'Clothing', + 'slug' => 'clothing', + ], + ], + ]; + + $woopay_specific_payload = [ + 'blog_id' => false, + 'order_id' => 1, + 'order_status' => 'publish', + ]; + + wp_set_current_user( self::$admin_user->ID ); + // Create the WebHook with WooPay specific delivery URL. + $this->webhook_sync_mock->maybe_create_woopay_order_webhook(); + + $post_processing_payload = $this->webhook_sync_mock->create_payload( $pre_processing_payload, 'product', 1, 1 ); + $this->assertNotEquals( $pre_processing_payload, $post_processing_payload ); + $this->assertEquals( $post_processing_payload, $woopay_specific_payload ); + + $this->webhook_sync_mock->remove_webhook(); + $this->assertEmpty( WooPay_Order_Status_Sync::get_webhook() ); + } + + /** + * Tests that non WooPay webhooks (e.g. Product Updated, Order created) are filtered out and eventually not modified. + */ + public function test_non_woopay_specific_webhook_payload_remains_unaffected() { + wp_set_current_user( self::$admin_user->ID ); + $pre_processing_payload = [ + 'status' => 'publish', + 'average_rating' => '0.00', + 'catalog_visibility' => 'visible', + 'categories' => [ + [ + 'id' => 9, + 'name' => 'Clothing', + 'slug' => 'clothing', + ], + ], + ]; + + $this->create_non_woopay_specific_webhook(); + + $post_processing_payload = $this->webhook_sync_mock->create_payload( $pre_processing_payload, 'product', 1, 2 ); + $this->assertEquals( $pre_processing_payload, $post_processing_payload ); + + $this->webhook_sync_mock->remove_webhook(); + $this->assertEmpty( WooPay_Order_Status_Sync::get_webhook() ); + } + /** * Tests that the webhook is created succesfuly if the logged in user has the capability manage_woocommerce. */ @@ -83,4 +146,17 @@ private function set_is_woopay_eligible( $is_woopay_eligible ) { $this->mock_cache->method( 'get' )->willReturn( [ 'platform_checkout_eligible' => $is_woopay_eligible ] ); } + private function create_non_woopay_specific_webhook() { + $delivery_url_non_specific_for_woopay = 'some-woocommerce-core-webhook-delivery-url'; + + $webhook = new \WC_Webhook(); + $webhook->set_name( 'WCPay woopay order status sync' ); + $webhook->set_user_id( get_current_user_id() ); + $webhook->set_topic( 'order.status_changed' ); + $webhook->set_secret( wp_generate_password( 50, false ) ); + $webhook->set_delivery_url( $delivery_url_non_specific_for_woopay ); + $webhook->set_status( 'active' ); + $webhook->save(); + } + }