Skip to content

Commit

Permalink
fix: ensure webhook_certificates setting is honoured in webhook
Browse files Browse the repository at this point in the history
  • Loading branch information
bethesque committed Feb 2, 2022
1 parent 33b83c6 commit 8a720cd
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 17 deletions.
2 changes: 1 addition & 1 deletion lib/pact_broker/badges/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def do_request(uri)
with_cache uri do
request = Net::HTTP::Get.new(uri)
options = {read_timeout: 3, open_timeout: 1, ssl_timeout: 1, continue_timeout: 1}
options.merge! PactBroker::BuildHttpOptions.call(uri)
options.merge! PactBroker::BuildHttpOptions.call(uri, disable_ssl_verification: PactBroker.configuration.disable_ssl_verification)

Net::HTTP.start(uri.hostname, uri.port, :ENV, options) do |http|
http.request request
Expand Down
8 changes: 2 additions & 6 deletions lib/pact_broker/build_http_options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ module PactBroker
class BuildHttpOptions
extend PactBroker::Services

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

if uri.scheme == "https"
options[:use_ssl] = true
options[:cert_store] = cert_store
options[:cert_store] = cert_store if cert_store
if disable_ssl_verification
options[:verify_mode] = OpenSSL::SSL::VERIFY_NONE
else
Expand All @@ -19,10 +19,6 @@ def self.call uri, disable_ssl_verification: false
end
options
end

def self.cert_store
certificate_service.cert_store
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.slice(:user_agent, :disable_ssl_verification))
webhook_request = request.build(template_params, options.slice(:user_agent, :disable_ssl_verification, :cert_store))
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, :disable_ssl_verification
attr_accessor :method, :url, :headers, :body, :username, :password, :uuid, :user_agent, :disable_ssl_verification, :cert_store

# 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, disable_ssl_verification: disable_ssl_verification).merge(read_timeout: 15, open_timeout: 15)
options = PactBroker::BuildHttpOptions.call(uri, disable_ssl_verification: disable_ssl_verification, cert_store: cert_store).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 @@ -45,6 +45,10 @@ def with_disable_ssl_verification(value)
with_updated_attribute(disable_ssl_verification: value)
end

def with_cert_store(value)
with_updated_attribute(cert_store: value)
end

def webhook_context
self[:webhook_context]
end
Expand Down
4 changes: 4 additions & 0 deletions lib/pact_broker/webhooks/execution_configuration_creator.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
require "pact_broker/configuration"
require "pact_broker/services"
require "pact_broker/webhooks/execution_configuration"

module PactBroker
module Webhooks
class ExecutionConfigurationCreator
extend PactBroker::Services

def self.call(resource)
PactBroker::Webhooks::ExecutionConfiguration.new
.with_show_response(PactBroker.configuration.show_webhook_response?)
.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_cert_store(certificate_service.cert_store)
.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: nil, disable_ssl_verification: false)
def build(template_params, user_agent: nil, disable_ssl_verification: false, cert_store: nil)
attributes = {
method: http_method,
url: build_url(template_params),
Expand All @@ -37,7 +37,8 @@ def build(template_params, user_agent: nil, disable_ssl_verification: false)
uuid: uuid,
body: build_body(template_params),
user_agent: user_agent,
disable_ssl_verification: disable_ssl_verification
disable_ssl_verification: disable_ssl_verification,
cert_store: cert_store
}
PactBroker::Domain::WebhookRequest.new(attributes)
end
Expand Down
6 changes: 5 additions & 1 deletion spec/integration/webhooks/certificate_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "pact_broker/domain/webhook_request"
require "pact_broker/certificates/service"
require "faraday"
# certificates and key generated by script/generate-certificates-for-webooks-certificate-spec.rb

Expand All @@ -18,9 +19,12 @@ def wait_for_server_to_start
let(:webhook_request) do
PactBroker::Domain::WebhookRequest.new(
method: "get",
url: "https://localhost:4444/")
url: "https://localhost:4444/",
cert_store: cert_store)
end

let(:cert_store) { PactBroker::Certificates::Service.cert_store }

subject { webhook_request.execute }

context "without the correct cacert" do
Expand Down
10 changes: 8 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,8 @@ 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", disable_ssl_verification: true} }
let(:options) { { logging_options: logging_options, http_success_codes: [200], user_agent: "user agent", disable_ssl_verification: true, cert_store: cert_store } }
let(:cert_store) { double("cert store") }
let(:pact) { double("pact") }
let(:verification) { double("verification") }
let(:logger) { double("logger").as_null_object }
Expand Down Expand Up @@ -107,7 +108,12 @@ module Domain
end

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

Expand Down
13 changes: 11 additions & 2 deletions spec/lib/pact_broker/webhooks/webhook_request_template_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ module Webhooks
body: built_body,
headers: {"headername" => "headervalueBUILT"},
user_agent: "Pact Broker",
disable_ssl_verification: true
disable_ssl_verification: true,
cert_store: cert_store
}
end

Expand All @@ -37,6 +38,7 @@ module Webhooks
let(:built_url) { "http://example.org/hook?foo=barBUILT" }
let(:body) { { foo: "bar" } }
let(:built_body) { '{"foo":"bar"}BUILT' }
let(:cert_store) { double("cert_store") }

describe "#build" do
before do
Expand All @@ -47,7 +49,14 @@ module Webhooks

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

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

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 8a720cd

Please sign in to comment.