Skip to content

Commit

Permalink
Revert "fix: temporarily redact webhook response body from UI for sec…
Browse files Browse the repository at this point in the history
…urity purposes"

This reverts commit becf20c.
  • Loading branch information
bethesque committed Jun 7, 2018
1 parent d525527 commit 6b0df51
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,12 @@ def body
represented.body
end
end
end

property :message, exec_context: :decorator
# property :error, :extend => ErrorDecorator
# property :response, :extend => HTTPResponseDecorator

def message
"Webhook response has been redacted temporarily for security purposes. Please see the Pact Broker application logs for the response body."
end

property :error, :extend => ErrorDecorator
property :response, :extend => HTTPResponseDecorator

link :webhook do | options |
{
href: webhook_url(options.fetch(:webhook).uuid, options.fetch(:base_url))
Expand Down
23 changes: 11 additions & 12 deletions lib/pact_broker/domain/webhook_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,18 @@ def log_response response, execution_logger
execution_logger.info(" ")
logger.info "Received response for webhook #{uuid} status=#{response.code}"
execution_logger.info "HTTP/#{response.http_version} #{response.code} #{response.message}"
#response.each_header do | header |
# execution_logger.info "#{header.split("-").collect(&:capitalize).join('-')}: #{response[header]}"
#end
response.each_header do | header |
execution_logger.info "#{header.split("-").collect(&:capitalize).join('-')}: #{response[header]}"
end
logger.debug "body=#{response.body}"
# safe_body = nil
# if response.body
# safe_body = response.body.encode('UTF-8', 'binary', invalid: :replace, undef: :replace, replace: '')
# if response.body != safe_body
# execution_logger.debug "Note that invalid UTF-8 byte sequences were removed from response body before saving the logs"
# end
# end
#execution_logger.info safe_body
execution_logger.info "Webhook response has been redacted temporarily for security purposes. Please see the Pact Broker application logs for the response body."
safe_body = nil
if response.body
safe_body = response.body.encode('UTF-8', 'binary', invalid: :replace, undef: :replace, replace: '')
if response.body != safe_body
execution_logger.debug "Note that invalid UTF-8 byte sequences were removed from response body before saving the logs"
end
end
execution_logger.info safe_body
end

def log_completion_message options, execution_logger, success
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,7 @@ module Decorators
expect(subject[:_links][:webhook][:href]).to eq 'http://example.org/webhooks/some-uuid'
end

it "includes a message about the response being redacted" do
expect(subject[:message]).to match /redacted/
end

context "when there is an error", pending: "temporarily disabled" do
context "when there is an error" do
let(:error) { double('error', message: 'message', backtrace: ['blah','blah']) }

it "includes the message" do
Expand All @@ -46,7 +42,7 @@ module Decorators
end
end

context "when there is a response", pending: "temporarily disabled" do
context "when there is a response" do
it "includes the response code" do
expect(subject[:response][:status]).to eq 200
end
Expand Down
15 changes: 6 additions & 9 deletions spec/lib/pact_broker/domain/webhook_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,9 @@ module Domain
before do
allow(PactBroker::Api::PactBrokerUrls).to receive(:pact_url).and_return('http://example.org/pact-url')
allow(PactBroker.configuration).to receive(:base_url).and_return('http://example.org')
allow(PactBroker.logger).to receive(:info).and_call_original
allow(PactBroker.logger).to receive(:debug).and_call_original
allow(PactBroker.logger).to receive(:warn).and_call_original
allow(PactBroker::Webhooks::Render).to receive(:call) do | content, pact, verification, &block |
content
end
end

let(:username) { nil }
let(:password) { nil }
Expand Down Expand Up @@ -143,12 +139,12 @@ module Domain
expect(logs).to include "HTTP/1.0 200"
end

it "does not log the response headers" do
expect(logs).to_not include "Content-Type: text/foo, blah"
it "logs the response headers" do
expect(logs).to include "Content-Type: text/foo, blah"
end

it "does not log the response body" do
expect(logs).to_not include "respbod"
it "logs the response body" do
expect(logs).to include "respbod"
end

context "when the response code is a success" do
Expand Down Expand Up @@ -249,6 +245,7 @@ module Domain
end

context "when the request is not successful" do

let!(:http_request) do
stub_request(:post, "http://example.org/hook").
with(:headers => {'Content-Type'=>'text/plain'}, :body => 'body').
Expand All @@ -264,7 +261,7 @@ module Domain
end
end

context "when the response body contains a non UTF-8 character", pending: "execution logs disabled temporarily for security purposes" do
context "when the response body contains a non UTF-8 character" do
let!(:http_request) do
stub_request(:post, "http://example.org/hook").
to_return(:status => 200, :body => "This has some \xC2 invalid chars")
Expand Down

0 comments on commit 6b0df51

Please sign in to comment.