Skip to content

Commit

Permalink
fix: ensure disable_ssl_verification setting is honoured in webhook
Browse files Browse the repository at this point in the history
  • Loading branch information
bethesque committed Feb 10, 2022
1 parent a6b06f6 commit 08bc758
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 33 deletions.
14 changes: 5 additions & 9 deletions lib/pact_broker/build_http_options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,25 @@ module PactBroker
class BuildHttpOptions
extend PactBroker::Services

def self.call uri
def self.call uri, disable_ssl_verification: false
uri = URI(uri)
options = {}

if uri.scheme == "https"
options[:use_ssl] = true
options[:cert_store] = cert_store
if disable_ssl_verification?
if disable_ssl_verification
options[:verify_mode] = OpenSSL::SSL::VERIFY_NONE
else
options[:verify_mode] = OpenSSL::SSL::VERIFY_PEER
end
end
options
end

def self.disable_ssl_verification?
PactBroker.configuration.disable_ssl_verification
end


def self.cert_store
certificate_service.cert_store
end
end
end
end

2 changes: 1 addition & 1 deletion lib/pact_broker/domain/webhook.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def request_description
def execute pact, verification, event_context, options
logger.info "Executing #{self} event_context=#{event_context}"
template_params = template_parameters(pact, verification, event_context, options)
webhook_request = request.build(template_params, options.fetch(:user_agent))
webhook_request = request.build(template_params, options.slice(:user_agent, :disable_ssl_verification))
http_response, error = execute_request(webhook_request)
success = success?(http_response, options)
http_request = webhook_request.http_request
Expand Down
4 changes: 2 additions & 2 deletions lib/pact_broker/domain/webhook_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class WebhookRequest

HEADERS_TO_REDACT = [/authorization/i, /token/i]

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

# Reform gets confused by the :method method, as :method is a standard
# Ruby method.
Expand Down Expand Up @@ -47,7 +47,7 @@ def redacted_headers
end

def execute
options = PactBroker::BuildHttpOptions.call(uri).merge(read_timeout: 15, open_timeout: 15)
options = PactBroker::BuildHttpOptions.call(uri, disable_ssl_verification: disable_ssl_verification).merge(read_timeout: 15, open_timeout: 15)
req = http_request
Net::HTTP.start(uri.hostname, uri.port, :ENV, options) do |http|
http.request req
Expand Down
4 changes: 4 additions & 0 deletions lib/pact_broker/webhooks/execution_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ def with_user_agent(value)
with_updated_attribute(user_agent: value)
end

def with_disable_ssl_verification(value)
with_updated_attribute(disable_ssl_verification: value)
end

def webhook_context
self[:webhook_context]
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ def self.call(resource)
.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)
.with_disable_ssl_verification(PactBroker.configuration.disable_ssl_verification)
.with_webhook_context(base_url: resource.base_url)
end
end
Expand Down
5 changes: 3 additions & 2 deletions lib/pact_broker/webhooks/webhook_request_template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def initialize attributes = {}
@headers = Rack::Utils::HeaderHash.new(attributes[:headers] || {})
end

def build(template_params, user_agent)
def build(template_params, user_agent: nil, disable_ssl_verification: false)
attributes = {
method: http_method,
url: build_url(template_params),
Expand All @@ -36,7 +36,8 @@ def build(template_params, user_agent)
password: build_string(password, template_params),
uuid: uuid,
body: build_body(template_params),
user_agent: user_agent
user_agent: user_agent,
disable_ssl_verification: disable_ssl_verification
}
PactBroker::Domain::WebhookRequest.new(attributes)
end
Expand Down
27 changes: 12 additions & 15 deletions spec/lib/pact_broker/build_http_options_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,36 @@
module PactBroker
describe BuildHttpOptions do

subject { PactBroker::BuildHttpOptions.call(url) }
subject { PactBroker::BuildHttpOptions.call(url, options) }
let(:options) { {} }

context "default http options" do
before do
PactBroker.configuration.disable_ssl_verification = false
end
let(:options) { { disable_ssl_verification: false } }

describe "when given an insecure URL" do
let(:url) { "http://example.org/insecure" }

it "should provide an empty configuration object" do
expect(subject).to eq({})
end

end

describe "when given a secure URL" do
let(:url) { "https://example.org/secure" }

it "should validate the full certificate chain" do
expect(subject).to include({:use_ssl => true, :verify_mode => 1})
end

end
end

context "disable_ssl_verification is set to true" do
before do
PactBroker.configuration.disable_ssl_verification = true
end

let(:options) { { disable_ssl_verification: true } }

let(:url) { "https://example.org/secure" }

describe "when given a secure URL" do
it "should not validate certificates" do
expect(subject).to include({:use_ssl => true, :verify_mode => 0})
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/pact_broker/domain/webhook_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module Domain
let(:response_code) { "200" }
let(:event_context) { { some: "things" } }
let(:logging_options) { { other: "options" } }
let(:options) { { logging_options: logging_options, http_success_codes: [200], user_agent: "user agent" } }
let(:options) { { logging_options: logging_options, http_success_codes: [200], user_agent: "user agent", disable_ssl_verification: true} }
let(:pact) { double("pact") }
let(:verification) { double("verification") }
let(:logger) { double("logger").as_null_object }
Expand Down Expand Up @@ -107,7 +107,7 @@ module Domain
end

it "builds the request" do
expect(request_template).to receive(:build).with(webhook_template_parameters_hash, "user agent")
expect(request_template).to receive(:build).with(webhook_template_parameters_hash, user_agent: "user agent", disable_ssl_verification: true)
execute
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ module Webhooks
uuid: "1234",
body: built_body,
headers: {"headername" => "headervalueBUILT"},
user_agent: "Pact Broker"
user_agent: "Pact Broker",
disable_ssl_verification: true
}
end

Expand All @@ -46,7 +47,7 @@ module Webhooks

let(:params_hash) { double("params hash") }

subject { WebhookRequestTemplate.new(attributes).build(params_hash, "Pact Broker") }
subject { WebhookRequestTemplate.new(attributes).build(params_hash, user_agent: "Pact Broker", disable_ssl_verification: true) }

it "renders the url template" do
expect(PactBroker::Webhooks::Render).to receive(:call).with(url, params_hash) do | content, pact, verification, &block |
Expand Down

0 comments on commit 08bc758

Please sign in to comment.