From 8a720cdb6afd13e917c2624e1ef87eff71a6d228 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 2 Feb 2022 20:56:52 +1100 Subject: [PATCH] fix: ensure webhook_certificates setting is honoured in webhook --- lib/pact_broker/badges/service.rb | 2 +- lib/pact_broker/build_http_options.rb | 8 ++------ lib/pact_broker/domain/webhook.rb | 2 +- lib/pact_broker/domain/webhook_request.rb | 4 ++-- lib/pact_broker/webhooks/execution_configuration.rb | 4 ++++ .../webhooks/execution_configuration_creator.rb | 4 ++++ .../webhooks/webhook_request_template.rb | 5 +++-- spec/integration/webhooks/certificate_spec.rb | 6 +++++- spec/lib/pact_broker/domain/webhook_spec.rb | 10 ++++++++-- .../webhooks/webhook_request_template_spec.rb | 13 +++++++++++-- 10 files changed, 41 insertions(+), 17 deletions(-) diff --git a/lib/pact_broker/badges/service.rb b/lib/pact_broker/badges/service.rb index 2737a41fe..5a79a5e22 100644 --- a/lib/pact_broker/badges/service.rb +++ b/lib/pact_broker/badges/service.rb @@ -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 diff --git a/lib/pact_broker/build_http_options.rb b/lib/pact_broker/build_http_options.rb index 9694144b0..3d1a7cb38 100644 --- a/lib/pact_broker/build_http_options.rb +++ b/lib/pact_broker/build_http_options.rb @@ -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 @@ -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 diff --git a/lib/pact_broker/domain/webhook.rb b/lib/pact_broker/domain/webhook.rb index 142741df2..4c99b5d97 100644 --- a/lib/pact_broker/domain/webhook.rb +++ b/lib/pact_broker/domain/webhook.rb @@ -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 diff --git a/lib/pact_broker/domain/webhook_request.rb b/lib/pact_broker/domain/webhook_request.rb index b1e1c3edc..c127a3d82 100644 --- a/lib/pact_broker/domain/webhook_request.rb +++ b/lib/pact_broker/domain/webhook_request.rb @@ -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. @@ -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 diff --git a/lib/pact_broker/webhooks/execution_configuration.rb b/lib/pact_broker/webhooks/execution_configuration.rb index f905d62e2..599c4b243 100644 --- a/lib/pact_broker/webhooks/execution_configuration.rb +++ b/lib/pact_broker/webhooks/execution_configuration.rb @@ -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 diff --git a/lib/pact_broker/webhooks/execution_configuration_creator.rb b/lib/pact_broker/webhooks/execution_configuration_creator.rb index 25ccb84aa..f05c165a9 100644 --- a/lib/pact_broker/webhooks/execution_configuration_creator.rb +++ b/lib/pact_broker/webhooks/execution_configuration_creator.rb @@ -1,9 +1,12 @@ 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?) @@ -11,6 +14,7 @@ def self.call(resource) .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 diff --git a/lib/pact_broker/webhooks/webhook_request_template.rb b/lib/pact_broker/webhooks/webhook_request_template.rb index 27f2ed816..b29c76895 100644 --- a/lib/pact_broker/webhooks/webhook_request_template.rb +++ b/lib/pact_broker/webhooks/webhook_request_template.rb @@ -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), @@ -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 diff --git a/spec/integration/webhooks/certificate_spec.rb b/spec/integration/webhooks/certificate_spec.rb index 9cfd0734b..2af727a96 100644 --- a/spec/integration/webhooks/certificate_spec.rb +++ b/spec/integration/webhooks/certificate_spec.rb @@ -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 @@ -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 diff --git a/spec/lib/pact_broker/domain/webhook_spec.rb b/spec/lib/pact_broker/domain/webhook_spec.rb index da7203701..42a7da10f 100644 --- a/spec/lib/pact_broker/domain/webhook_spec.rb +++ b/spec/lib/pact_broker/domain/webhook_spec.rb @@ -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 } @@ -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 diff --git a/spec/lib/pact_broker/webhooks/webhook_request_template_spec.rb b/spec/lib/pact_broker/webhooks/webhook_request_template_spec.rb index 306e09326..ef38302d5 100644 --- a/spec/lib/pact_broker/webhooks/webhook_request_template_spec.rb +++ b/spec/lib/pact_broker/webhooks/webhook_request_template_spec.rb @@ -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 @@ -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 @@ -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 |