diff --git a/lib/pact_broker/api/decorators/webhook_execution_result_decorator.rb b/lib/pact_broker/api/decorators/webhook_execution_result_decorator.rb index e6ae1c916..4e82b5674 100644 --- a/lib/pact_broker/api/decorators/webhook_execution_result_decorator.rb +++ b/lib/pact_broker/api/decorators/webhook_execution_result_decorator.rb @@ -11,6 +11,32 @@ class ErrorDecorator < BaseDecorator property :backtrace end + class HTTPRequestDecorator < BaseDecorator + property :headers, exec_context: :decorator + property :body, exec_context: :decorator + property :url, exec_context: :decorator + + def headers + headers_hash = represented.to_hash + headers_hash.keys.each_with_object({}) do | name, new_headers_hash| + new_headers_hash[name] = headers_hash[name].join(", ") + end + end + + def body + begin + ::JSON.parse(represented.body) + rescue StandardError => e + represented.body + end + end + + def url + (represented.uri || represented.path).to_s + end + end + + class HTTPResponseDecorator < BaseDecorator property :status, :getter => lambda { |_| code.to_i } property :headers, exec_context: :decorator @@ -33,6 +59,7 @@ def body end property :error, :extend => ErrorDecorator, if: lambda { |context| context[:options][:user_options][:show_response] } + property :request, :extend => HTTPRequestDecorator property :response, :extend => HTTPResponseDecorator, if: lambda { |context| context[:options][:user_options][:show_response] } property :response_hidden_message, as: :message, exec_context: :decorator, if: lambda { |context| !context[:options][:user_options][:show_response] } diff --git a/lib/pact_broker/domain/webhook.rb b/lib/pact_broker/domain/webhook.rb index 6ea046356..2afb898ae 100644 --- a/lib/pact_broker/domain/webhook.rb +++ b/lib/pact_broker/domain/webhook.rb @@ -46,7 +46,7 @@ def execute pact, verification, options end def to_s - "webhook for consumer=#{consumer_name} provider=#{provider_name} uuid=#{uuid} request=#{request}" + "webhook for consumer=#{consumer_name} provider=#{provider_name} uuid=#{uuid}" end def consumer_name diff --git a/lib/pact_broker/domain/webhook_execution_result.rb b/lib/pact_broker/domain/webhook_execution_result.rb index 78280ddb4..ad6cabd39 100644 --- a/lib/pact_broker/domain/webhook_execution_result.rb +++ b/lib/pact_broker/domain/webhook_execution_result.rb @@ -4,7 +4,8 @@ module Domain class WebhookExecutionResult - def initialize response, logs, error = nil + def initialize request, response, logs, error = nil + @request = request @response = response @logs = logs @error = error @@ -14,6 +15,10 @@ def success? !@response.nil? && @response.code.to_i < 300 end + def request + @request + end + def response @response end diff --git a/lib/pact_broker/domain/webhook_request.rb b/lib/pact_broker/domain/webhook_request.rb index 9ac139475..30dad50a2 100644 --- a/lib/pact_broker/domain/webhook_request.rb +++ b/lib/pact_broker/domain/webhook_request.rb @@ -15,12 +15,10 @@ module PactBroker module Domain class WebhookRequestError < StandardError - def initialize message, response super message @response = response end - end class WebhookResponseWithUtf8SafeBody < SimpleDelegator @@ -41,6 +39,18 @@ def unsafe_body? end end + class WebhookRequestWithRedactedHeaders < SimpleDelegator + def to_hash + __getobj__().to_hash.each_with_object({}) do | (key, values), new_hash | + new_hash[key] = redact?(key) ? ["**********"] : values + end + end + + def redact? name + WebhookRequest::HEADERS_TO_REDACT.any?{ | pattern | name =~ pattern } + end + end + class WebhookRequest include PactBroker::Logging @@ -95,7 +105,7 @@ def execute_and_build_result pact, verification, options, logs, execution_logger req = build_request(uri, pact, verification, execution_logger) response = do_request(uri, req) log_response(response, execution_logger, options) - result = WebhookExecutionResult.new(response, logs.string) + result = WebhookExecutionResult.new(WebhookRequestWithRedactedHeaders.new(req), response, logs.string) log_completion_message(options, execution_logger, result.success?) result end @@ -103,7 +113,7 @@ def execute_and_build_result pact, verification, options, logs, execution_logger def handle_error_and_build_result e, options, logs, execution_logger log_error(e, execution_logger, options) log_completion_message(options, execution_logger, false) - WebhookExecutionResult.new(nil, logs.string, e) + WebhookExecutionResult.new(nil, nil, logs.string, e) end def build_request uri, pact, verification, execution_logger @@ -123,11 +133,12 @@ def build_request uri, pact, verification, execution_logger end execution_logger.info(req.body) if req.body + logger.info "Making webhook #{uuid} request #{method.upcase} URI=#{uri} headers=#{headers_to_log} (body in debug logs)" + logger.debug "body=#{req.body}" req end def do_request uri, req - logger.info "Making webhook #{uuid} request #{to_s}" options = PactBroker::BuildHttpOptions.call(uri) response = Net::HTTP.start(uri.hostname, uri.port, :ENV, options) do |http| http.request req @@ -149,8 +160,8 @@ def response_body_hidden_message end def log_response_to_application_logger response - logger.info "Received response for webhook #{uuid} status=#{response.code}" - logger.debug "headers=#{response.to_hash}" + logger.info "Received response for webhook #{uuid} status=#{response.code} (headers and body in debug logs)" + logger.debug "headers=#{response.to_hash} " logger.debug "body=#{response.unsafe_body}" end diff --git a/script/seed.rb b/script/seed.rb index 07a785430..95b0c4921 100755 --- a/script/seed.rb +++ b/script/seed.rb @@ -47,7 +47,7 @@ .create_global_webhook(method: 'GET', url: "http://example.org?consumer=${pactbroker.consumerName}&provider=${pactbroker.providerName}") .create_certificate(path: 'spec/fixtures/certificates/self-signed.badssl.com.pem') .create_consumer("Bethtest") - .create_verification_webhook(method: 'POST', url: "http://localhost:9292", body: webhook_body) + .create_verification_webhook(method: 'POST', url: "http://localhost:9292", body: webhook_body, username: "foo", password: "bar") .create_consumer("Foo") .create_label("microservice") .create_provider("Bar") diff --git a/spec/lib/pact_broker/api/decorators/webhook_execution_result_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/webhook_execution_result_decorator_spec.rb index 205efcda0..5bfb7fda3 100644 --- a/spec/lib/pact_broker/api/decorators/webhook_execution_result_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/webhook_execution_result_decorator_spec.rb @@ -8,9 +8,15 @@ module Decorators describe "to_json" do - let(:webhook_execution_result) { PactBroker::Domain::WebhookExecutionResult.new(response, logs, error)} + let(:webhook_execution_result) { PactBroker::Domain::WebhookExecutionResult.new(request, response, logs, error)} let(:logs) { "logs" } let(:headers) { { "Something" => ["blah", "thing"]} } + let(:request) do + req = Net::HTTP::Get.new("http://example.org?foo=bar") + req['Foo'] = ['bar', 'wiffle'] + req.body = { foo: 'bar' }.to_json + req + end let(:response) { double('http_response', code: '200', body: response_body, to_hash: headers) } let(:response_body) { 'body' } let(:error) { nil } @@ -43,6 +49,32 @@ module Decorators end end + context "when there is a request" do + it "includes the request URL" do + expect(subject[:request][:url]).to eq "http://example.org?foo=bar" + end + + it "includes the request headers" do + expect(subject[:request][:headers][:'foo']).to eq "bar, wiffle" + end + + context "when the request body is JSON" do + it "includes the request body as JSON" do + expect(subject[:request][:body]).to include( foo: 'bar' ) + end + end + + context "when the request body is not json" do + before do + request.body = "" + end + + it "includes the request body as a String" do + expect(subject[:request][:body]).to eq "" + end + end + end + context "when there is a response" do it "includes the response code" do expect(subject[:response][:status]).to eq 200 @@ -51,6 +83,7 @@ module Decorators it "includes the response headers" do expect(subject[:response][:headers]).to eq :'Something' => "blah, thing" end + it "includes the response body" do expect(subject[:response][:body]).to eq response_body end diff --git a/spec/support/test_data_builder.rb b/spec/support/test_data_builder.rb index 6f363b144..d21a6ac8b 100644 --- a/spec/support/test_data_builder.rb +++ b/spec/support/test_data_builder.rb @@ -294,7 +294,7 @@ def create_triggered_webhook params = {} def create_webhook_execution params = {} params.delete(:comment) logs = params[:logs] || "logs" - webhook_execution_result = PactBroker::Domain::WebhookExecutionResult.new(OpenStruct.new(code: "200"), logs, nil) + webhook_execution_result = PactBroker::Domain::WebhookExecutionResult.new(nil, OpenStruct.new(code: "200"), logs, nil) @webhook_execution = PactBroker::Webhooks::Repository.new.create_execution @triggered_webhook, webhook_execution_result created_at = params[:created_at] || @pact.created_at + Rational(1, 86400) set_created_at_if_set created_at, :webhook_executions, {id: @webhook_execution.id}