From 08bc758c6897852174193a2b636bacde264bceb5 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 2 Feb 2022 16:07:53 +1100 Subject: [PATCH] fix: ensure disable_ssl_verification setting is honoured in webhook --- lib/pact_broker/build_http_options.rb | 14 ++++------ lib/pact_broker/domain/webhook.rb | 2 +- lib/pact_broker/domain/webhook_request.rb | 4 +-- .../webhooks/execution_configuration.rb | 4 +++ .../execution_configuration_creator.rb | 1 + .../webhooks/webhook_request_template.rb | 5 ++-- .../pact_broker/build_http_options_spec.rb | 27 +++++++++---------- spec/lib/pact_broker/domain/webhook_spec.rb | 4 +-- .../webhooks/webhook_request_template_spec.rb | 5 ++-- 9 files changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/pact_broker/build_http_options.rb b/lib/pact_broker/build_http_options.rb index 2be8a076e..9694144b0 100644 --- a/lib/pact_broker/build_http_options.rb +++ b/lib/pact_broker/build_http_options.rb @@ -4,14 +4,14 @@ 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 @@ -19,14 +19,10 @@ def self.call uri 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 diff --git a/lib/pact_broker/domain/webhook.rb b/lib/pact_broker/domain/webhook.rb index f03467fc6..142741df2 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.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 diff --git a/lib/pact_broker/domain/webhook_request.rb b/lib/pact_broker/domain/webhook_request.rb index a55259964..b1e1c3edc 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 + 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. @@ -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 diff --git a/lib/pact_broker/webhooks/execution_configuration.rb b/lib/pact_broker/webhooks/execution_configuration.rb index 2f7519a87..f905d62e2 100644 --- a/lib/pact_broker/webhooks/execution_configuration.rb +++ b/lib/pact_broker/webhooks/execution_configuration.rb @@ -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 diff --git a/lib/pact_broker/webhooks/execution_configuration_creator.rb b/lib/pact_broker/webhooks/execution_configuration_creator.rb index b51ceeffb..25ccb84aa 100644 --- a/lib/pact_broker/webhooks/execution_configuration_creator.rb +++ b/lib/pact_broker/webhooks/execution_configuration_creator.rb @@ -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 diff --git a/lib/pact_broker/webhooks/webhook_request_template.rb b/lib/pact_broker/webhooks/webhook_request_template.rb index 0e34bb207..27f2ed816 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) + def build(template_params, user_agent: nil, disable_ssl_verification: false) attributes = { method: http_method, url: build_url(template_params), @@ -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 diff --git a/spec/lib/pact_broker/build_http_options_spec.rb b/spec/lib/pact_broker/build_http_options_spec.rb index f933c0761..ba7094bf4 100644 --- a/spec/lib/pact_broker/build_http_options_spec.rb +++ b/spec/lib/pact_broker/build_http_options_spec.rb @@ -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}) diff --git a/spec/lib/pact_broker/domain/webhook_spec.rb b/spec/lib/pact_broker/domain/webhook_spec.rb index c9da41bb5..da7203701 100644 --- a/spec/lib/pact_broker/domain/webhook_spec.rb +++ b/spec/lib/pact_broker/domain/webhook_spec.rb @@ -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 } @@ -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 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 61605612e..306e09326 100644 --- a/spec/lib/pact_broker/webhooks/webhook_request_template_spec.rb +++ b/spec/lib/pact_broker/webhooks/webhook_request_template_spec.rb @@ -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 @@ -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 |