diff --git a/Gemfile.lock b/Gemfile.lock index 95eaf8f8e..aafeaacd8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -271,7 +271,7 @@ GEM ruby-progressbar (1.13.0) ruby2_keywords (0.0.5) securerandom (0.4.1) - shopify_api (16.0.0) + shopify_api (16.1.0) activesupport concurrent-ruby hash_diff diff --git a/lib/shopify_app/controller_concerns/payload_verification.rb b/lib/shopify_app/controller_concerns/payload_verification.rb index c14128055..490d70505 100644 --- a/lib/shopify_app/controller_concerns/payload_verification.rb +++ b/lib/shopify_app/controller_concerns/payload_verification.rb @@ -7,7 +7,7 @@ module PayloadVerification private def shopify_hmac - request.headers["HTTP_X_SHOPIFY_HMAC_SHA256"] + shopify_header("hmac-sha256") end def hmac_valid?(data) @@ -21,5 +21,12 @@ def hmac_valid?(data) ) end end + + # Retrieves Shopify headers with fallback to legacy format. + # New headers (shopify-*) take precedence over legacy (X-Shopify-*). + def shopify_header(name) + formatted_name = name.upcase.tr("-", "_") + request.headers["HTTP_SHOPIFY_#{formatted_name}"] || request.headers["HTTP_X_SHOPIFY_#{formatted_name}"] + end end end diff --git a/lib/shopify_app/controller_concerns/webhook_verification.rb b/lib/shopify_app/controller_concerns/webhook_verification.rb index d50b0a783..5e1c61bb0 100644 --- a/lib/shopify_app/controller_concerns/webhook_verification.rb +++ b/lib/shopify_app/controller_concerns/webhook_verification.rb @@ -21,7 +21,7 @@ def verify_request end def shop_domain - request.headers["HTTP_X_SHOPIFY_SHOP_DOMAIN"] + shopify_header("shop-domain") end end end diff --git a/lib/shopify_app/test_helpers/webhook_verification_helper.rb b/lib/shopify_app/test_helpers/webhook_verification_helper.rb index 1a0479c8a..0df759fd2 100644 --- a/lib/shopify_app/test_helpers/webhook_verification_helper.rb +++ b/lib/shopify_app/test_helpers/webhook_verification_helper.rb @@ -3,15 +3,17 @@ module ShopifyApp module TestHelpers module WebhookVerificationHelper - def authorized_webhook_verification_headers!(params = {}) + def authorized_webhook_verification_headers!(params = {}, use_new_headers: false) digest = OpenSSL::Digest.new("sha256") secret = ShopifyApp.configuration.secret valid_hmac = Base64.encode64(OpenSSL::HMAC.digest(digest, secret, params.to_query)).strip - @request.headers["HTTP_X_SHOPIFY_HMAC_SHA256"] = valid_hmac + header_key = use_new_headers ? "HTTP_SHOPIFY_HMAC_SHA256" : "HTTP_X_SHOPIFY_HMAC_SHA256" + @request.headers[header_key] = valid_hmac end - def unauthorized_webhook_verification_headers! - @request.headers["HTTP_X_SHOPIFY_HMAC_SHA256"] = "invalid_hmac" + def unauthorized_webhook_verification_headers!(use_new_headers: false) + header_key = use_new_headers ? "HTTP_SHOPIFY_HMAC_SHA256" : "HTTP_X_SHOPIFY_HMAC_SHA256" + @request.headers[header_key] = "invalid_hmac" end end end diff --git a/test/controllers/extension_verification_controller_test.rb b/test/controllers/extension_verification_controller_test.rb index 3e6c49513..eda50ca7b 100644 --- a/test/controllers/extension_verification_controller_test.rb +++ b/test/controllers/extension_verification_controller_test.rb @@ -29,13 +29,31 @@ class ExtensionVerificationControllerTest < ActionController::TestCase test "responds ok when hmac is correct" do with_application_test_routes do params = { foo: "anything" } - valid_hmac = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" # Valid hmac using the new secret + valid_hmac = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" @request.headers["HTTP_X_SHOPIFY_HMAC_SHA256"] = valid_hmac post "extension_action", params: params assert_response :ok end end + test "responds ok when hmac is correct with new header format" do + with_application_test_routes do + params = { foo: "anything" } + valid_hmac = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" + @request.headers["HTTP_SHOPIFY_HMAC_SHA256"] = valid_hmac + post "extension_action", params: params + assert_response :ok + end + end + + test "return unauthorized when hmac is incorrect with new header format" do + with_application_test_routes do + @request.headers["HTTP_SHOPIFY_HMAC_SHA256"] = "invalid_hmac" + post :extension_action, params: { foo: "anything" } + assert_response :unauthorized + end + end + private def with_application_test_routes diff --git a/test/integration/webhooks_controller_test.rb b/test/integration/webhooks_controller_test.rb index 10fec6c7a..8b209c9c3 100644 --- a/test/integration/webhooks_controller_test.rb +++ b/test/integration/webhooks_controller_test.rb @@ -35,28 +35,49 @@ class WebhooksControllerTest < ActionDispatch::IntegrationTest end end + test "receives webhook with new header format and calls process" do + ShopifyAPI::Webhooks::Registry.stubs(:process).returns(nil) + ShopifyAPI::Webhooks::Registry.expects(:process).once + send_webhook "order_update", { foo: :bar }, use_new_headers: true + assert_response :ok + end + + test "receives webhook with legacy header format and calls process" do + ShopifyAPI::Webhooks::Registry.stubs(:process).returns(nil) + ShopifyAPI::Webhooks::Registry.expects(:process).once + send_webhook "order_update", { foo: :bar }, use_new_headers: false + assert_response :ok + end + private - def send_webhook(name, data) + def send_webhook(name, data, use_new_headers: false) post( shopify_app.webhooks_path(name), params: data, - headers: headers(name), + headers: headers(name, use_new_headers: use_new_headers), ) end - def headers(name) + def headers(name, use_new_headers: false) hmac = OpenSSL::HMAC.digest( OpenSSL::Digest.new("sha256"), "API_SECRET_KEY", "{}", ) - headers = { - "x-shopify-topic" => name, - "x-shopify-hmac-sha256" => Base64.encode64(hmac), - "x-shopify-shop-domain" => "test.myshopify.com", - } - headers + if use_new_headers + { + "shopify-topic" => name, + "shopify-hmac-sha256" => Base64.encode64(hmac), + "shopify-shop-domain" => "test.myshopify.com", + } + else + { + "x-shopify-topic" => name, + "x-shopify-hmac-sha256" => Base64.encode64(hmac), + "x-shopify-shop-domain" => "test.myshopify.com", + } + end end end end diff --git a/test/shopify_app/controller_concerns/webhook_verification_test.rb b/test/shopify_app/controller_concerns/webhook_verification_test.rb index d6de87e20..8ca0d18a6 100644 --- a/test/shopify_app/controller_concerns/webhook_verification_test.rb +++ b/test/shopify_app/controller_concerns/webhook_verification_test.rb @@ -13,6 +13,10 @@ class WebhookVerificationController < ActionController::Base def webhook_action head(:ok) end + + def shop_domain_action + render(plain: shop_domain) + end end class WebhookVerificationTest < ActionController::TestCase @@ -38,7 +42,7 @@ class WebhookVerificationTest < ActionController::TestCase test "authorized requests should be successful" do with_application_test_routes do params = { foo: "anything" } - valid_hmac = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" # Valid hmac using the new secret + valid_hmac = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" @request.headers["HTTP_X_SHOPIFY_HMAC_SHA256"] = valid_hmac post :webhook_action, params: params assert_response :ok @@ -48,7 +52,7 @@ class WebhookVerificationTest < ActionController::TestCase test "authorized request validated with old secret should be successful" do with_application_test_routes do params = { foo: "anything" } - valid_hmac = "XqRcjrSv57VACn6apEO9znyu/wkN1VC7QxdSPwK3Hzs=" # Valid hmac using the old secret + valid_hmac = "XqRcjrSv57VACn6apEO9znyu/wkN1VC7QxdSPwK3Hzs=" @request.headers["HTTP_X_SHOPIFY_HMAC_SHA256"] = valid_hmac post :webhook_action, params: params assert_response :ok @@ -63,12 +67,80 @@ class WebhookVerificationTest < ActionController::TestCase end end + test "authorized request with new header format should be successful" do + with_application_test_routes do + params = { foo: "anything" } + valid_hmac = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" + @request.headers["HTTP_SHOPIFY_HMAC_SHA256"] = valid_hmac + post :webhook_action, params: params + assert_response :ok + end + end + + test "un-verified request with new header format returns unauthorized" do + with_application_test_routes do + @request.headers["HTTP_SHOPIFY_HMAC_SHA256"] = "invalid_hmac" + post :webhook_action, params: { foo: "anything" } + assert_response :unauthorized + end + end + + test "new header format takes precedence when both headers present" do + with_application_test_routes do + params = { foo: "anything" } + valid_hmac = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" + @request.headers["HTTP_SHOPIFY_HMAC_SHA256"] = valid_hmac # New format (valid) + @request.headers["HTTP_X_SHOPIFY_HMAC_SHA256"] = "invalid_hmac" # Legacy format (invalid) + post :webhook_action, params: params + assert_response :ok + end + end + + test "falls back to legacy header when new header not present" do + with_application_test_routes do + params = { foo: "anything" } + valid_hmac = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" + @request.headers["HTTP_X_SHOPIFY_HMAC_SHA256"] = valid_hmac + post :webhook_action, params: params + assert_response :ok + end + end + + test "shop_domain returns value from new header format" do + with_application_test_routes do + @request.headers["HTTP_SHOPIFY_HMAC_SHA256"] = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" + @request.headers["HTTP_SHOPIFY_SHOP_DOMAIN"] = "test-shop.myshopify.com" + post :shop_domain_action, params: { foo: "anything" } + assert_equal "test-shop.myshopify.com", response.body + end + end + + test "shop_domain falls back to legacy header" do + with_application_test_routes do + @request.headers["HTTP_X_SHOPIFY_HMAC_SHA256"] = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" + @request.headers["HTTP_X_SHOPIFY_SHOP_DOMAIN"] = "legacy-shop.myshopify.com" + post :shop_domain_action, params: { foo: "anything" } + assert_equal "legacy-shop.myshopify.com", response.body + end + end + + test "shop_domain prefers new header over legacy header" do + with_application_test_routes do + @request.headers["HTTP_SHOPIFY_HMAC_SHA256"] = "yCGX/RrK4fcuNtr3ztk5tQGsOBjcAzHpGLdMUrbV8yI=" + @request.headers["HTTP_SHOPIFY_SHOP_DOMAIN"] = "new-shop.myshopify.com" + @request.headers["HTTP_X_SHOPIFY_SHOP_DOMAIN"] = "legacy-shop.myshopify.com" + post :shop_domain_action, params: { foo: "anything" } + assert_equal "new-shop.myshopify.com", response.body + end + end + private def with_application_test_routes with_routing do |set| set.draw do post "/webhook_action" => "webhook_verification#webhook_action" + post "/shop_domain_action" => "webhook_verification#shop_domain_action" end yield end