fix: ensure that the templating in the webhook body uses the correct base URL for the broker

Author: bethesque

lib/pact_broker/app.rb

@@ -14,6 +14,7 @@
 require 'rack/pact_broker/configurable_make_it_later'
 require 'rack/pact_broker/no_auth'
 require 'rack/pact_broker/convert_404_to_hal'
+require 'rack/pact_broker/reset_thread_data'
 require 'sucker_punch'
 
 module PactBroker
@@ -122,6 +123,7 @@ def configure_middleware
         @app_builder.use Rack::Protection, except: [:path_traversal, :remote_token, :session_hijacking, :http_origin]
       end
       @app_builder.use Rack::PactBroker::InvalidUriProtection
+      @app_builder.use Rack::PactBroker::ResetThreadData
       @app_builder.use Rack::PactBroker::StoreBaseURL
       @app_builder.use Rack::PactBroker::AddPactBrokerVersionHeader
       @app_builder.use Rack::Static, :urls => ["/stylesheets", "/css", "/fonts", "/js", "/javascripts", "/images"], :root => PactBroker.project_root.join("public")

lib/pact_broker/domain/webhook.rb

@@ -42,7 +42,7 @@ def request_description
 
       def execute pact, verification, options
         logger.info "Executing #{self}"
-        request.build(pact: pact, verification: verification).execute(options)
+        request.build(pact: pact, verification: verification, base_url: options[:base_url]).execute(options)
       end
 
       def to_s

lib/pact_broker/ui/view_models/index_item.rb

@@ -68,11 +68,11 @@ def any_webhooks?
         end
 
         def pact_versions_url
-          PactBroker::Api::PactBrokerUrls.pact_versions_url(consumer_name, provider_name, PactBroker.configuration.base_url)
+          PactBroker::Api::PactBrokerUrls.pact_versions_url(consumer_name, provider_name)
         end
 
         def integration_url
-          PactBroker::Api::PactBrokerUrls.integration_url(consumer_name, provider_name, PactBroker.configuration.base_url)
+          PactBroker::Api::PactBrokerUrls.integration_url(consumer_name, provider_name)
         end
 
         def webhook_label

lib/pact_broker/webhooks/job.rb

@@ -25,7 +25,7 @@ def perform_with_connection(data)
         @triggered_webhook = PactBroker::Webhooks::TriggeredWebhook.find(id: data[:triggered_webhook].id)
         @error_count = data[:error_count] || 0
         begin
-          webhook_execution_result = PactBroker::Webhooks::Service.execute_triggered_webhook_now triggered_webhook, execution_options
+          webhook_execution_result = PactBroker::Webhooks::Service.execute_triggered_webhook_now triggered_webhook, execution_options(data)
           if webhook_execution_result.success?
             handle_success
           else
@@ -36,10 +36,11 @@ def perform_with_connection(data)
         end
       end
 
-      def execution_options
+      def execution_options(data)
         {
           success_log_message: "Successfully executed webhook",
-          failure_log_message: failure_log_message
+          failure_log_message: failure_log_message,
+          base_url: data.fetch(:base_url)
         }
       end
 

lib/pact_broker/webhooks/render.rb

@@ -4,12 +4,11 @@ class Render
 
       TEMPLATE_PARAMETER_REGEXP = /\$\{pactbroker\.[^\}]+\}/
 
-      def self.call(template, pact, trigger_verification = nil, &escaper)
-        base_url = PactBroker.configuration.base_url
+      def self.call(template, pact, trigger_verification, base_url, &escaper)
         verification = trigger_verification || (pact && pact.latest_verification)
         params = {
           '${pactbroker.pactUrl}' => pact ? PactBroker::Api::PactBrokerUrls.pact_url(base_url, pact) : "",
-          '${pactbroker.verificationResultUrl}' => verification_url(verification),
+          '${pactbroker.verificationResultUrl}' => verification_url(verification, base_url),
           '${pactbroker.consumerVersionNumber}' => pact ? pact.consumer_version_number : "",
           '${pactbroker.providerVersionNumber}' => verification ? verification.provider_version_number : "",
           '${pactbroker.providerVersionTags}' => provider_version_tags(verification),
@@ -40,9 +39,9 @@ def self.github_verification_status verification
         end
       end
 
-      def self.verification_url verification
+      def self.verification_url verification, base_url
         if verification
-          PactBroker::Api::PactBrokerUrls.verification_url(verification, PactBroker.configuration.base_url)
+          PactBroker::Api::PactBrokerUrls.verification_url(verification, base_url)
         else
           ""
         end

lib/pact_broker/webhooks/service.rb

@@ -64,7 +64,7 @@ def self.find_all
       end
 
       def self.test_execution webhook
-        options = { failure_log_message: "Webhook execution failed", show_response: PactBroker.configuration.show_webhook_response?}
+        options = { failure_log_message: "Webhook execution failed", show_response: PactBroker.configuration.show_webhook_response?, base_url: base_url}
         verification = nil
         if webhook.trigger_on_provider_verification_published?
           verification = verification_service.search_for_latest(webhook.consumer_name, webhook.provider_name) || PactBroker::Verifications::PlaceholderVerification.new
@@ -126,7 +126,8 @@ def self.run_later webhooks, pact, verification, event_name
             logger.info "Scheduling job for #{webhook.description} with uuid #{webhook.uuid}"
             job_data = {
               triggered_webhook: triggered_webhook,
-              database_connector: job_database_connector
+              database_connector: job_database_connector,
+              base_url: base_url
             }
             # Delay slightly to make sure the request transaction has finished before we execute the webhook
             Job.perform_in(5, job_data)
@@ -140,6 +141,10 @@ def self.job_database_connector
         Thread.current[:pact_broker_thread_data].database_connector
       end
 
+      def self.base_url
+        Thread.current[:pact_broker_thread_data].base_url
+      end
+
       def self.find_latest_triggered_webhooks_for_pact pact
         webhook_repository.find_latest_triggered_webhooks_for_pact pact
       end

lib/pact_broker/webhooks/webhook_request_template.rb

@@ -30,22 +30,23 @@ def initialize attributes = {}
       def build(context)
         attributes = {
           method: http_method,
-          url: build_url(context[:pact], context[:verification]),
+          url: build_url(context[:pact], context[:verification], context[:base_url]),
           headers: headers,
           username: username,
           password: password,
           uuid: uuid,
-          body: build_body(context[:pact], context[:verification])
+          body: build_body(context[:pact], context[:verification], context[:base_url])
         }
         PactBroker::Domain::WebhookRequest.new(attributes)
       end
 
-      def build_url(pact, verification)
-        URI(PactBroker::Webhooks::Render.call(url, pact, verification){ | value | CGI::escape(value) if !value.nil? } ).to_s
+      def build_url(pact, verification, broker_base_url)
+        URI(PactBroker::Webhooks::Render.call(url, pact, verification, broker_base_url){ | value | CGI::escape(value) if !value.nil? } ).to_s
       end
 
-      def build_body(pact, verification)
-        PactBroker::Webhooks::Render.call(String === body ? body : body.to_json, pact, verification)
+      def build_body(pact, verification, broker_base_url)
+        body_string = String === body ? body : body.to_json
+        PactBroker::Webhooks::Render.call(body_string, pact, verification, broker_base_url)
       end
 
       def description

lib/rack/pact_broker/reset_thread_data.rb

@@ -0,0 +1,22 @@
+require 'ostruct'
+
+module Rack
+  module PactBroker
+    class ResetThreadData
+      def initialize app
+        @app = app
+      end
+
+      def call env
+        data = OpenStruct.new
+        Thread.current[:pact_broker_thread_data] ||= data
+        response = @app.call(env)
+        # only delete it if it's the same object that we set
+        if data.equal?(Thread.current[:pact_broker_thread_data])
+          Thread.current[:pact_broker_thread_data] = nil
+        end
+        response
+      end
+    end
+  end
+end

lib/rack/pact_broker/store_base_url.rb

@@ -6,7 +6,7 @@ def initialize app
       end
 
       def call(env)
-        ENV['PACT_BROKER_BASE_URL'] ||= ::Rack::Request.new(env).base_url
+        Thread.current[:pact_broker_thread_data].base_url ||= (ENV['PACT_BROKER_BASE_URL'] || ::Rack::Request.new(env).base_url)
         @app.call(env)
       end
     end

spec/features/execute_webhook_spec.rb

@@ -7,23 +7,19 @@
   let(:td) { TestDataBuilder.new }
 
   before do
-    ENV['PACT_BROKER_BASE_URL'] = 'http://example.org'
+    Thread.current[:pact_broker_thread_data] = OpenStruct.new(base_url: 'http://broker')
     td.create_pact_with_hierarchy("Foo", "1", "Bar")
       .create_webhook(method: 'POST', body: '${pactbroker.pactUrl}')
   end
 
-  after do
-    ENV.delete('PACT_BROKER_BASE_URL')
-  end
-
   let(:path) { "/webhooks/#{td.webhook.uuid}/execute" }
   let(:response_body) { JSON.parse(last_response.body, symbolize_names: true)}
 
   subject { post path; last_response }
 
   context "when the execution is successful" do
     let!(:request) do
-      stub_request(:post, /http/).with(body: 'http://example.org/pacts/provider/Bar/consumer/Foo/version/1').to_return(:status => 200, body: response_body)
+      stub_request(:post, /http/).with(body: 'http://broker/pacts/provider/Bar/consumer/Foo/version/1').to_return(:status => 200, body: response_body)
     end
 
     let(:response_body) { "webhook-response-body" }

spec/lib/pact_broker/domain/webhook_spec.rb

@@ -8,7 +8,8 @@ module Domain
       let(:request_template) { instance_double(PactBroker::Webhooks::WebhookRequestTemplate, build: request)}
       let(:request) { instance_double(PactBroker::Domain::WebhookRequest, execute: result) }
       let(:result) { double('result') }
-      let(:options) { double('options') }
+      let(:options) { { base_url: base_url } }
+      let(:base_url) { "http://broker" }
       let(:pact) { double('pact') }
       let(:verification) { double('verification') }
       let(:logger) { double('logger').as_null_object }
@@ -54,7 +55,7 @@ module Domain
         let(:execute) { subject.execute pact, verification, options }
 
         it "builds the request" do
-          expect(request_template).to receive(:build).with(pact: pact, verification: verification)
+          expect(request_template).to receive(:build).with(pact: pact, verification: verification, base_url: base_url)
           execute
         end
 

spec/lib/pact_broker/webhooks/job_spec.rb

@@ -11,13 +11,14 @@ module Webhooks
         allow(Job).to receive(:logger).and_return(logger)
       end
 
+      let(:base_url) { "http://broker" }
       let(:triggered_webhook) { instance_double("PactBroker::Webhooks::TriggeredWebhook", webhook_uuid: '1234', id: 1) }
       let(:result) { instance_double("PactBroker::Domain::WebhookExecutionResult", success?: success)}
       let(:success) { true }
       let(:logger) { double('logger').as_null_object }
       let(:database_connector) { ->(&block) { block.call } }
 
-      subject { Job.new.perform(triggered_webhook: triggered_webhook, database_connector: database_connector) }
+      subject { Job.new.perform(triggered_webhook: triggered_webhook, database_connector: database_connector, base_url: base_url) }
 
       it "reloads the TriggeredWebhook object to make sure it has a fresh copy" do
         expect(PactBroker::Webhooks::TriggeredWebhook).to receive(:find).with(id: 1)
@@ -44,7 +45,7 @@ module Webhooks
         end
 
         it "reschedules the job in 10 seconds" do
-          expect(Job).to receive(:perform_in).with(10, {triggered_webhook: triggered_webhook, error_count: 1, database_connector: database_connector})
+          expect(Job).to receive(:perform_in).with(10, {triggered_webhook: triggered_webhook, error_count: 1, database_connector: database_connector, base_url: base_url})
           subject
         end
 
@@ -59,15 +60,16 @@ module Webhooks
         let(:success) { false }
 
         it "reschedules the job in 10 seconds" do
-          expect(Job).to receive(:perform_in).with(10, {triggered_webhook: triggered_webhook, error_count: 1, database_connector: database_connector})
+          expect(Job).to receive(:perform_in).with(10, {triggered_webhook: triggered_webhook, error_count: 1, database_connector: database_connector, base_url: base_url})
           subject
         end
 
         it "executes the job with an log message indicating that the webhook will be retried" do
           expect(PactBroker::Webhooks::Service).to receive(:execute_triggered_webhook_now)
             .with(triggered_webhook, {
               failure_log_message: "Retrying webhook in 10 seconds",
-              success_log_message: "Successfully executed webhook"
+              success_log_message: "Successfully executed webhook",
+              base_url: base_url
           })
           subject
         end
@@ -84,10 +86,10 @@ module Webhooks
           allow(PactBroker::Webhooks::Service).to receive(:execute_triggered_webhook_now).and_raise("an error")
         end
 
-        subject { Job.new.perform(triggered_webhook: triggered_webhook, error_count: 1, database_connector: database_connector) }
+        subject { Job.new.perform(triggered_webhook: triggered_webhook, error_count: 1, database_connector: database_connector, base_url: base_url) }
 
         it "reschedules the job in 60 seconds" do
-          expect(Job).to receive(:perform_in).with(60, {triggered_webhook: triggered_webhook, error_count: 2, database_connector: database_connector})
+          expect(Job).to receive(:perform_in).with(60, {triggered_webhook: triggered_webhook, error_count: 2, database_connector: database_connector, base_url: base_url})
           subject
         end
 
@@ -101,13 +103,14 @@ module Webhooks
       context "when the job is not successful for the last time" do
         let(:success) { false }
 
-        subject { Job.new.perform(triggered_webhook: triggered_webhook, error_count: 6, database_connector: database_connector) }
+        subject { Job.new.perform(triggered_webhook: triggered_webhook, error_count: 6, database_connector: database_connector, base_url: base_url) }
 
         it "executes the job with an log message indicating that the webhook has failed" do
           expect(PactBroker::Webhooks::Service).to receive(:execute_triggered_webhook_now)
             .with(triggered_webhook, {
               failure_log_message: "Webhook execution failed after 7 attempts",
-              success_log_message: "Successfully executed webhook"
+              success_log_message: "Successfully executed webhook",
+              base_url: base_url
           })
           subject
         end

spec/lib/pact_broker/webhooks/render_spec.rb

@@ -15,6 +15,8 @@ module Webhooks
           end
         end
 
+        let(:base_url) { "http://broker" }
+
         let(:pact) do
           double("pact",
             consumer_version: consumer_version,
@@ -93,7 +95,7 @@ module Webhooks
         let(:nil_pact) { nil }
         let(:nil_verification) { nil }
 
-        subject { Render.call(template, pact, verification) }
+        subject { Render.call(template, pact, verification, base_url) }
 
         TEST_CASES = [
           ["${pactbroker.pactUrl}", "http://foo", :pact, :verification],
@@ -122,15 +124,15 @@ module Webhooks
             it "replaces #{template} with #{expected_output.inspect}" do
               the_pact = send(pact_var_name)
               the_verification = send(verification_var_name)
-              output = Render.call(template, the_pact, the_verification)
+              output = Render.call(template, the_pact, the_verification, base_url)
               expect(output).to eq expected_output
             end
           end
         end
 
         context "with an escaper" do
           subject do
-            Render.call(template, pact, verification) do | value |
+            Render.call(template, pact, verification, base_url) do | value |
               CGI.escape(value)
             end
           end
@@ -145,13 +147,14 @@ module Webhooks
       describe "#call with placeholder domain objects" do
         let(:placeholder_pact) { PactBroker::Pacts::PlaceholderPact.new }
         let(:placeholder_verification) { PactBroker::Verifications::PlaceholderVerification.new }
+        let(:base_url) { "http://broker" }
 
         it "does not blow up with a placeholder pact" do
-          Render.call("", placeholder_pact)
+          Render.call("", placeholder_pact, nil, base_url)
         end
 
         it "does not blow up with a placeholder verification" do
-          Render.call("", placeholder_pact, placeholder_verification)
+          Render.call("", placeholder_pact, placeholder_verification, base_url)
         end
       end
     end

spec/lib/pact_broker/webhooks/service_spec.rb

@@ -105,14 +105,16 @@ module Webhooks
         let(:options) do
           {
             failure_log_message: "Webhook execution failed",
-            show_response: 'foo'
+            show_response: 'foo',
+            base_url: 'http://broker'
           }
         end
 
         before do
           allow(PactBroker::Pacts::Service).to receive(:search_for_latest_pact).and_return(pact)
           allow(PactBroker::Verifications::Service).to receive(:search_for_latest).and_return(verification)
           allow(PactBroker.configuration).to receive(:show_webhook_response?).and_return('foo')
+          allow(Service).to receive(:base_url).and_return("http://broker")
         end
 
         subject { Service.test_execution(webhook) }