feat(webhooks): allow auth headers to be logged for debugging purposes (

#575)
pact-foundation · Oct 7, 2022 · 102b193 · 102b193
1 parent 757f030
commit 102b193
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 29 deletions.
diff --git a/lib/pact_broker/config/runtime_configuration.rb b/lib/pact_broker/config/runtime_configuration.rb
@@ -52,6 +52,7 @@ class RuntimeConfiguration < Anyway::Config
         webhook_http_code_success: [200, 201, 202, 203, 204, 205, 206],
         webhook_scheme_whitelist: ["https"],
         webhook_host_whitelist: [],
+        webhook_redact_sensitive_data: true,
         disable_ssl_verification: false,
         user_agent: "Pact Broker v#{PactBroker::VERSION}"
       )

diff --git a/lib/pact_broker/config/runtime_configuration_logging_methods.rb b/lib/pact_broker/config/runtime_configuration_logging_methods.rb
@@ -31,6 +31,9 @@ def log_configuration(logger)
               source: source_info.dig(key, :source) || {:type=>:defaults}
             }
           end.sort_by { |key, _| key }.each { |key, value| log_config_inner(key, value, logger) }
+          if self.webhook_redact_sensitive_data == false
+            logger.warn("WARNING!!! webhook_redact_sensitive_data is set to false. This will allow authentication information to be included in the webhook logs. This should only be used for debugging purposes. Do not run the application permanently in production with this value.")
+          end
         end
 
         def log_config_inner(key, value, logger)

diff --git a/lib/pact_broker/test/http_test_data_builder.rb b/lib/pact_broker/test/http_test_data_builder.rb
@@ -265,7 +265,9 @@ def create_webhook_for_event(uuid: nil, url: "https://postman-echo.com/post", bo
           "request" => {
             "method" => "POST",
             "url" => url,
-            "body" => body || default_body
+            "body" => body || default_body,
+            "username" => "user",
+            "password" => "pass"
           }
         }.compact
         path = "webhooks/#{uuid}"
@@ -286,32 +288,9 @@ def create_global_webhook_for_contract_requiring_verification_published(uuid: ni
       def create_global_webhook_for_anything_published(uuid: nil, url: "https://postman-echo.com/post")
         puts "Creating global webhook for contract changed event with uuid #{uuid}"
         uuid ||= SecureRandom.uuid
-        request_body = {
-          "description" => "A webhook for all consumers and providers",
-          "events" => [{
-            "name" => "contract_published"
-          },{
-            "name" => "provider_verification_published"
-          }],
-          "request" => {
-            "method" => "POST",
-            "url" => url,
-            "headers" => { "Content-Type" => "application/json"},
-            "body" => {
-              "eventName" => "${pactbroker.eventName}",
-              "consumerVersionNumber" => "${pactbroker.consumerVersionNumber}",
-              "consumerVersionTags" => "${pactbroker.consumerVersionTags}",
-              "consumerVersionBranch" => "${pactbroker.consumerVersionBranch}",
-              "githubVerificationStatus" => "${pactbroker.githubVerificationStatus}",
-              "providerVersionNumber" => "${pactbroker.providerVersionNumber}",
-              "providerVersionTags" => "${pactbroker.providerVersionTags}",
-              "providerVersionBranch" => "${pactbroker.providerVersionBranch}",
-              "canIMerge" => "${pactbroker.providerMainBranchGithubVerificationStatus}"
-            }
-          }
-        }
+
         path = "webhooks/#{uuid}"
-        client.put(path, request_body.to_json).tap { |response| check_for_error(response) }
+        client.put(path, webhook_body_with_all_parameters(url).to_json).tap { |response| check_for_error(response) }
         separate
         self
       end
@@ -438,6 +417,35 @@ def check_for_error(response)
           puts response.body
         end
       end
+
+      def webhook_body_with_all_parameters(url)
+        {
+          "description" => "A webhook for all consumers and providers",
+          "events" => [{
+            "name" => "contract_published"
+          },{
+            "name" => "provider_verification_published"
+          }],
+          "request" => {
+            "method" => "POST",
+            "url" => url,
+            "headers" => { "Content-Type" => "application/json"},
+            "username" => "user",
+            "password" => "pass",
+            "body" => {
+              "eventName" => "${pactbroker.eventName}",
+              "consumerVersionNumber" => "${pactbroker.consumerVersionNumber}",
+              "consumerVersionTags" => "${pactbroker.consumerVersionTags}",
+              "consumerVersionBranch" => "${pactbroker.consumerVersionBranch}",
+              "githubVerificationStatus" => "${pactbroker.githubVerificationStatus}",
+              "providerVersionNumber" => "${pactbroker.providerVersionNumber}",
+              "providerVersionTags" => "${pactbroker.providerVersionTags}",
+              "providerVersionBranch" => "${pactbroker.providerVersionBranch}",
+              "canIMerge" => "${pactbroker.providerMainBranchGithubVerificationStatus}"
+            }
+          }
+        }
+      end
     end
   end
 end
diff --git a/lib/pact_broker/webhooks/execution_configuration.rb b/lib/pact_broker/webhooks/execution_configuration.rb
@@ -25,6 +25,10 @@ def with_failure_log_message(value)
         with_updated_attribute(logging_options: { failure_log_message: value })
       end
 
+      def with_redact_sensitive_data(value)
+        with_updated_attribute(logging_options: { redact_sensitive_data: value })
+      end
+
       def with_retry_schedule(value)
         with_updated_attribute(retry_schedule: value)
       end

diff --git a/lib/pact_broker/webhooks/execution_configuration_creator.rb b/lib/pact_broker/webhooks/execution_configuration_creator.rb
@@ -10,6 +10,7 @@ class ExecutionConfigurationCreator
       def self.call(resource)
         PactBroker::Webhooks::ExecutionConfiguration.new
           .with_show_response(PactBroker.configuration.show_webhook_response?)
+          .with_redact_sensitive_data(PactBroker.configuration.webhook_redact_sensitive_data)
           .with_retry_schedule(PactBroker.configuration.webhook_retry_schedule)
           .with_http_success_codes(PactBroker.configuration.webhook_http_code_success)
           .with_user_agent(PactBroker.configuration.user_agent)

diff --git a/lib/pact_broker/webhooks/webhook_request_logger.rb b/lib/pact_broker/webhooks/webhook_request_logger.rb
@@ -51,7 +51,7 @@ def log_webhook_context(webhook_context)
       end
 
       def log_request(webhook_request)
-        http_request = HttpRequestWithRedactedHeaders.new(webhook_request.http_request)
+        http_request = options[:redact_sensitive_data] ? HttpRequestWithRedactedHeaders.new(webhook_request.http_request) : webhook_request.http_request
         logger.info "Making webhook #{webhook_request.uuid} request #{http_request.method.upcase} URI=#{webhook_request.url} (headers and body in debug logs)"
         logger.debug "Webhook #{webhook_request.uuid} request headers=#{http_request.to_hash}"
         logger.debug "Webhook #{webhook_request.uuid} request body=#{http_request.body}"

diff --git a/script/data/webhook.rb b/script/data/webhook.rb
@@ -12,6 +12,7 @@
     .delete_pacticipant("bar-provider")
     .create_pacticipant("foo-consumer")
     .create_pacticipant("bar-provider")
+    .create_version(pacticipant: "bar-provider", branch: "main", version: "1")
     .publish_pact_the_old_way(consumer: "foo-consumer", consumer_version: "1", provider: "bar-provider", content_id: "111", tag: "main")
     .get_pacts_for_verification(
       provider: "bar-provider",

diff --git a/spec/lib/pact_broker/config/runtime_configuration_documentation_spec.rb b/spec/lib/pact_broker/config/runtime_configuration_documentation_spec.rb
@@ -18,7 +18,8 @@ module Config
         :semver_formats,
         :seed_example_data,
         :use_first_tag_as_branch_time_limit,
-        :validate_database_connection_config
+        :validate_database_connection_config,
+        :webhook_redact_sensitive_data
       ]
 
       (ATTRIBUTES - DELIBERATELY_UNDOCUMENTED_ATTRIBUTES).each do | attribute_name |

diff --git a/spec/lib/pact_broker/webhooks/webhook_request_logger_spec.rb b/spec/lib/pact_broker/webhooks/webhook_request_logger_spec.rb
@@ -16,7 +16,8 @@ module Webhooks
 
       let(:logger) { double("logger").as_null_object }
       let(:uuid) { "uuid" }
-      let(:options) { { failure_log_message: "oops", show_response: show_response, http_code_success: [200] } }
+      let(:options) { { failure_log_message: "oops", show_response: show_response, http_code_success: [200], redact_sensitive_data: redact_sensitive_data } }
+      let(:redact_sensitive_data) { true }
       let(:show_response) { true }
       let(:username) { nil }
       let(:password) { nil }
@@ -144,6 +145,14 @@ module Webhooks
           it "logs the Authorization header with a starred value" do
             expect(logs).to include "authorization: **********"
           end
+
+          context "when redact_sensitive_data is false" do
+            let(:redact_sensitive_data) { false }
+
+            it "logs the real value of the Authorization header" do
+              expect(logs).to include "authorization: foo"
+            end
+          end
         end
 
         context "when the response body contains a non UTF-8 character" do