From 10efddb3933bc586ff5ca6c3edbe9c191784d4f2 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 5 Sep 2017 08:51:23 +1000 Subject: [PATCH] feat(webhook status): redact authorization headers in webhook logs --- lib/pact_broker/domain/webhook_request.rb | 8 ++- lib/pact_broker/webhooks/redact_logs.rb | 10 ++++ .../domain/webhook_request_spec.rb | 9 ++-- .../pact_broker/webhooks/redact_logs_spec.rb | 49 +++++++++++++++++++ spec/spec_helper.rb | 14 +++--- 5 files changed, 74 insertions(+), 16 deletions(-) create mode 100644 lib/pact_broker/webhooks/redact_logs.rb create mode 100644 spec/lib/pact_broker/webhooks/redact_logs_spec.rb diff --git a/lib/pact_broker/domain/webhook_request.rb b/lib/pact_broker/domain/webhook_request.rb index 2f5936052..b88f8815e 100644 --- a/lib/pact_broker/domain/webhook_request.rb +++ b/lib/pact_broker/domain/webhook_request.rb @@ -3,6 +3,7 @@ require 'pact_broker/logging' require 'pact_broker/messages' require 'net/http' +require 'pact_broker/webhooks/redact_logs' module PactBroker @@ -56,7 +57,7 @@ def execute execution_logger.info "HTTP/1.1 #{method.upcase} #{url_with_credentials}" headers.each_pair do | name, value | - execution_logger.info "#{name}: #{value}" + execution_logger.info Webhooks::RedactLogs.call("#{name}: #{value}") req[name] = value end @@ -91,9 +92,8 @@ def execute rescue StandardError => e logger.error "Error executing webhook #{uuid} #{e.class.name} - #{e.message}" - execution_logger.error "Error executing webhook #{uuid} #{e.class.name} - #{e.message}" logger.error e.backtrace.join("\n") - execution_logger.error e.backtrace.join("\n") + execution_logger.error "Error executing webhook #{uuid} #{e.class.name} - #{e.message}" WebhookExecutionResult.new(nil, logs.string, e) end end @@ -118,7 +118,5 @@ def url_with_credentials u end end - end - end diff --git a/lib/pact_broker/webhooks/redact_logs.rb b/lib/pact_broker/webhooks/redact_logs.rb new file mode 100644 index 000000000..a13158a17 --- /dev/null +++ b/lib/pact_broker/webhooks/redact_logs.rb @@ -0,0 +1,10 @@ +module PactBroker + module Webhooks + class RedactLogs + def self.call logs + logs.gsub(/(Authorization: )(.*)/i,'\1[REDACTED]') + .gsub(/(Token: )(.*)/i,'\1[REDACTED]') + end + end + end +end diff --git a/spec/lib/pact_broker/domain/webhook_request_spec.rb b/spec/lib/pact_broker/domain/webhook_request_spec.rb index 5c8f8f2c1..c78bd7018 100644 --- a/spec/lib/pact_broker/domain/webhook_request_spec.rb +++ b/spec/lib/pact_broker/domain/webhook_request_spec.rb @@ -19,7 +19,7 @@ module Domain WebhookRequest.new( method: 'post', url: url, - headers: {'Content-Type' => 'text/plain'}, + headers: {'Content-Type' => 'text/plain', 'Authorization' => 'foo'}, username: username, password: password, body: body) @@ -73,9 +73,6 @@ module Domain end describe "execution logs" do - before do - - end let(:logs) { subject.execute.logs } @@ -87,6 +84,10 @@ module Domain expect(logs).to include "Content-Type: text/plain" end + it "redacts potentially sensitive headers" do + expect(logs).to include "Authorization: [REDACTED]" + end + it "logs the request body" do expect(logs).to include body end diff --git a/spec/lib/pact_broker/webhooks/redact_logs_spec.rb b/spec/lib/pact_broker/webhooks/redact_logs_spec.rb new file mode 100644 index 000000000..307d26bf1 --- /dev/null +++ b/spec/lib/pact_broker/webhooks/redact_logs_spec.rb @@ -0,0 +1,49 @@ +require 'pact_broker/webhooks/redact_logs' + +module PactBroker + module Webhooks + describe RedactLogs do + describe ".call" do + let(:string) do + "Authorization: foo\nX-Thing: bar" + end + + let(:x_auth_string) do + "X-Authorization: bar foo\nX-Thing: bar" + end + + let(:x_auth_token) do + "X-Auth-Token: bar foo\nX-Thing: bar" + end + + let(:x_authorization_token) do + "X-Authorization-Token: bar foo\nX-Thing: bar" + end + + let(:string_lower) do + "authorization: foo\nX-Thing: bar" + end + + it "hides the value of the Authorization header" do + expect(RedactLogs.call(string)).to eq "Authorization: [REDACTED]\nX-Thing: bar" + end + + it "hides the value of the X-Authorization header" do + expect(RedactLogs.call(x_auth_string)).to eq "X-Authorization: [REDACTED]\nX-Thing: bar" + end + + it "hides the value of the X-Auth-Token header" do + expect(RedactLogs.call(x_auth_token)).to eq "X-Auth-Token: [REDACTED]\nX-Thing: bar" + end + + it "hides the value of the X-Authorization-Token header" do + expect(RedactLogs.call(x_authorization_token)).to eq "X-Authorization-Token: [REDACTED]\nX-Thing: bar" + end + + it "hides the value of the authorization header" do + expect(RedactLogs.call(string_lower)).to eq "authorization: [REDACTED]\nX-Thing: bar" + end + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e00ba16d7..46d967d71 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,10 +5,15 @@ RACK_ENV = 'test' $: << File.expand_path("../../", __FILE__) -require 'rack/test' + require 'db' -require 'pact_broker/api' require 'tasks/database' +require 'pact_broker/db' +raise "Wrong environment!!! Don't run this script!! ENV['RACK_ENV'] is #{ENV['RACK_ENV']} and RACK_ENV is #{RACK_ENV}" if ENV['RACK_ENV'] != 'test' || RACK_ENV != 'test' +PactBroker::DB.connection = PactBroker::Database.database = DB::PACT_BROKER_DB + +require 'rack/test' +require 'pact_broker/api' require 'rspec/its' Dir.glob("./spec/support/**/*.rb") { |file| require file } @@ -16,11 +21,6 @@ I18n.config.enforce_available_locales = false RSpec.configure do | config | - config.before :suite do - raise "Wrong environment!!! Don't run this script!! ENV['RACK_ENV'] is #{ENV['RACK_ENV']} and RACK_ENV is #{RACK_ENV}" if ENV['RACK_ENV'] != 'test' || RACK_ENV != 'test' - PactBroker::DB.connection = PactBroker::Database.database = DB::PACT_BROKER_DB - end - config.before :each do PactBroker.reset_configuration end