Skip to content

Commit

Permalink
feat: redact potentially sensitive headers in the webhook resource
Browse files Browse the repository at this point in the history
  • Loading branch information
bethesque committed Jun 9, 2018
1 parent abccf7a commit 619c7e9
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 6 deletions.
6 changes: 5 additions & 1 deletion lib/pact_broker/api/decorators/webhook_request_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@ class WebhookRequestDecorator < BaseDecorator

property :method
property :url
property :headers, getter: lambda { | _ | headers.empty? ? nil : headers }
property :headers, getter: lambda { | _ | headers.empty? ? nil : self.redacted_headers }
property :body
property :username
property :password, getter: lambda { | _ | display_password }


def redacted_headers
represented.headers
end
end
end
end
Expand Down
14 changes: 11 additions & 3 deletions lib/pact_broker/domain/webhook_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
require 'pact_broker/logging'
require 'pact_broker/messages'
require 'net/http'
require 'pact_broker/webhooks/redact_logs'
require 'pact_broker/webhooks/render'
require 'pact_broker/api/pact_broker_urls'
require 'pact_broker/build_http_options'
Expand All @@ -27,6 +26,7 @@ class WebhookRequest

include PactBroker::Logging
include PactBroker::Messages
HEADERS_TO_REDACT = [/authorization/i, /token/i]

attr_accessor :method, :url, :headers, :body, :username, :password, :uuid

Expand All @@ -52,6 +52,13 @@ def display_password
password.nil? ? nil : "**********"
end

def redacted_headers
headers.each_with_object({}) do | (name, value), new_headers |
redact = HEADERS_TO_REDACT.any?{ | pattern | name =~ pattern }
new_headers[name] = redact ? "**********" : value
end
end

def execute pact, verification, options = {}
logs = StringIO.new
execution_logger = Logger.new(logs)
Expand Down Expand Up @@ -84,8 +91,9 @@ def build_request uri, pact, verification, execution_logger
req = http_request(uri)
execution_logger.info "HTTP/1.1 #{method.upcase} #{url_with_credentials(pact, verification)}"

headers_to_log = redacted_headers
headers.each_pair do | name, value |
execution_logger.info Webhooks::RedactLogs.call("#{name}: #{value}")
execution_logger.info "#{name}: #{headers_to_log[name]}"
req[name] = value
end

Expand Down Expand Up @@ -166,7 +174,7 @@ def log_error e, execution_logger, options
end

def to_s
"#{method.upcase} #{url}, username=#{username}, password=#{display_password}, headers=#{headers}, body=#{body}"
"#{method.upcase} #{url}, username=#{username}, password=#{display_password}, headers=#{redacted_headers}, body=#{body}"
end

def http_request(uri)
Expand Down
7 changes: 7 additions & 0 deletions spec/lib/pact_broker/api/decorators/webhook_decorator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ module Decorators
expect(parsed_json[:request]).to_not have_key :headers
end
end

context 'when the headers contain sensitve information' do
let(:headers) { { 'Authorization' => 'foo' } }
it 'redacts them' do
expect(parsed_json[:request][:headers][:'Authorization']).to eq "**********"
end
end
end

describe 'from_json' do
Expand Down
35 changes: 33 additions & 2 deletions spec/lib/pact_broker/domain/webhook_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ module Domain
let(:username) { nil }
let(:password) { nil }
let(:url) { 'http://example.org/hook' }
let(:headers) { {'Content-Type' => 'text/plain', 'Authorization' => 'foo'} }
let(:body) { 'body' }
let(:logs) { StringIO.new }
let(:execution_logger) { Logger.new(logs) }
Expand All @@ -29,7 +30,7 @@ module Domain
WebhookRequest.new(
method: 'post',
url: url,
headers: {'Content-Type' => 'text/plain', 'Authorization' => 'foo'},
headers: headers,
username: username,
password: password,
body: body)
Expand Down Expand Up @@ -58,6 +59,36 @@ module Domain
end
end

describe "redacted_headers" do
let(:headers) do
{
'Authorization' => 'foo',
'X-authorization' => 'bar',
'token' => 'bar',
'Token' => 'bar',
'X-Auth-Token' => 'bar',
'X-Authorization-Token' => 'bar',
'OK' => 'ok'
}
end

let(:expected_headers) do
{
'Authorization' => '**********',
'X-authorization' => '**********',
'token' => '**********',
'Token' => '**********',
'X-Auth-Token' => '**********',
'X-Authorization-Token' => '**********',
'OK' => 'ok'
}
end

it "redacts sensitive headers" do
expect(subject.redacted_headers).to eq expected_headers
end
end

describe "execute" do
let!(:http_request) do
stub_request(:post, "http://example.org/hook").
Expand Down Expand Up @@ -126,7 +157,7 @@ module Domain
end

it "redacts potentially sensitive headers" do
expect(logs).to include "Authorization: [REDACTED]"
expect(logs).to include "Authorization: **********"
end

it "logs the request body" do
Expand Down

0 comments on commit 619c7e9

Please sign in to comment.