diff --git a/lib/stripe/authorize_response_patcher.rb b/lib/stripe/authorize_response_patcher.rb index 4128bf6577..5586718949 100644 --- a/lib/stripe/authorize_response_patcher.rb +++ b/lib/stripe/authorize_response_patcher.rb @@ -27,8 +27,9 @@ module Stripe next_action_type = next_action["type"] return unless %w(authorize_with_url redirect_to_url).include?(next_action_type) - url = next_action[next_action_type]["url"] - url if url.match(%r{https?://\S+}) && url.include?("stripe.com") + url = URI(next_action[next_action_type]["url"]) + # Check the URL is from a stripe subdomain + url.to_s if url.is_a?(URI::HTTPS) && url.host.match?(/\.stripe.com\Z/) end # This field is used because the Spree code recognizes and stores it diff --git a/spec/lib/stripe/authorize_response_patcher_spec.rb b/spec/lib/stripe/authorize_response_patcher_spec.rb index c99b2da340..3eb2f2e626 100644 --- a/spec/lib/stripe/authorize_response_patcher_spec.rb +++ b/spec/lib/stripe/authorize_response_patcher_spec.rb @@ -2,30 +2,49 @@ require 'spec_helper' -module Stripe - RSpec.describe AuthorizeResponsePatcher do - describe "#call!" do - let(:patcher) { Stripe::AuthorizeResponsePatcher.new(response) } - let(:params) { {} } - let(:response) { ActiveMerchant::Billing::Response.new(true, "Transaction approved", params) } +RSpec.describe Stripe::AuthorizeResponsePatcher do + describe "#call!" do + subject(:patcher) { Stripe::AuthorizeResponsePatcher.new(response) } + let(:params) { {} } + let(:response) { ActiveMerchant::Billing::Response.new(true, "Transaction approved", params) } - context "when url not found in response" do - it "does nothing" do - new_response = patcher.call! - expect(new_response).to eq response - end + context "when url not found in response" do + it "does nothing" do + new_response = patcher.call! + expect(new_response).to eq response + end + end + + context "when url is found in response" do + let(:params) { + { + "status" => "requires_source_action", + "next_source_action" => { + "type" => "authorize_with_url", + "authorize_with_url" => { "url" => "https://www.stripe.com/authorize" } + } + } + } + + it "patches response.cvv_result.message with the url in the response" do + new_response = patcher.call! + expect(new_response.cvv_result['message']).to eq "https://www.stripe.com/authorize" end - context "when url is found in response" do + context "with invalid url containing 'stripe.com'" do let(:params) { - { "status" => "requires_source_action", - "next_source_action" => { "type" => "authorize_with_url", - "authorize_with_url" => { "url" => "https://www.stripe.com/authorize" } } } + { + "status" => "requires_source_action", + "next_source_action" => { + "type" => "authorize_with_url", + "authorize_with_url" => { "url" => "https://www.evil-stripe.com.malicious.org/authorize" } + } + } } - it "patches response.cvv_result.message with the url in the response" do + it "patches response.cvv_result.message with nil" do new_response = patcher.call! - expect(new_response.cvv_result['message']).to eq "https://www.stripe.com/authorize" + expect(new_response.cvv_result['message']).to be_nil end end end