diff --git a/lib/pact_broker/api/contracts/webhook_contract.rb b/lib/pact_broker/api/contracts/webhook_contract.rb index 1e94da42c..aa07ffb64 100644 --- a/lib/pact_broker/api/contracts/webhook_contract.rb +++ b/lib/pact_broker/api/contracts/webhook_contract.rb @@ -1,5 +1,6 @@ require 'reform' require 'reform/form' +require 'pact_broker/webhooks/check_host_whitelist' module PactBroker module Api @@ -41,7 +42,8 @@ def self.messages en: { errors: { allowed_webhook_method?: http_method_error_message, - allowed_webhook_scheme?: scheme_error_message + allowed_webhook_scheme?: scheme_error_message, + allowed_webhook_host?: host_error_message } } ) @@ -56,7 +58,11 @@ def self.http_method_error_message end def self.scheme_error_message - "must be #{PactBroker.configuration.webhook_scheme_whitelist.join(", ")}. See /doc/webhooks#whitelist for more information." + "scheme must be #{PactBroker.configuration.webhook_scheme_whitelist.join(", ")}. See /doc/webhooks#whitelist for more information." + end + + def self.host_error_message + "host must be in the whitelist #{PactBroker.configuration.webhook_host_whitelist.join(",")}. See /doc/webhooks#whitelist for more information." end def valid_method?(http_method) @@ -82,10 +88,22 @@ def allowed_webhook_scheme?(url) scheme.downcase == allowed_scheme.downcase end end + + def allowed_webhook_host?(url) + if host_whitelist.any? + PactBroker::Webhooks::CheckHostWhitelist.call(URI(url).host, host_whitelist).any? + else + true + end + end + + def host_whitelist + PactBroker.configuration.webhook_host_whitelist + end end required(:http_method).filled(:valid_method?, :allowed_webhook_method?) - required(:url).filled(:valid_url?, :allowed_webhook_scheme?) + required(:url).filled(:valid_url?, :allowed_webhook_scheme?, :allowed_webhook_host?) end end diff --git a/lib/pact_broker/configuration.rb b/lib/pact_broker/configuration.rb index d3c076b34..e91e40b0c 100644 --- a/lib/pact_broker/configuration.rb +++ b/lib/pact_broker/configuration.rb @@ -30,10 +30,10 @@ class Configuration attr_accessor :validate_database_connection_config, :enable_diagnostic_endpoints, :version_parser, :sha_generator attr_accessor :use_case_sensitive_resource_names, :order_versions_by_date attr_accessor :check_for_potential_duplicate_pacticipant_names - attr_accessor :webhook_http_method_whitelist, :webhook_scheme_whitelist + attr_accessor :webhook_http_method_whitelist, :webhook_scheme_whitelist, :webhook_host_whitelist + attr_accessor :webhook_retry_schedule attr_accessor :semver_formats attr_accessor :enable_public_badge_access, :shields_io_base_url - attr_accessor :webhook_retry_schedule attr_accessor :disable_ssl_verification attr_accessor :base_equality_only_on_content_that_affects_verification_results attr_reader :api_error_reporters @@ -77,6 +77,7 @@ def self.default_configuration config.disable_ssl_verification = false config.webhook_http_method_whitelist = ['POST'] config.webhook_scheme_whitelist = ['https'] + config.webhook_host_whitelist = [] config end diff --git a/lib/pact_broker/webhooks/check_host_whitelist.rb b/lib/pact_broker/webhooks/check_host_whitelist.rb new file mode 100644 index 000000000..1ac0e522c --- /dev/null +++ b/lib/pact_broker/webhooks/check_host_whitelist.rb @@ -0,0 +1,22 @@ +module PactBroker + module Webhooks + class CheckHostWhitelist + + def self.call(host, whitelist = PactBroker.configuration.webhook_host_whitelist) + whitelist.select{ | whitelist_host | match?(host, whitelist_host) } + end + + def self.match?(host, whitelist_host) + if whitelist_host.is_a?(Regexp) + host =~ whitelist_host + else + begin + IPAddr.new(whitelist_host) === IPAddr.new(host) + rescue IPAddr::Error + host == whitelist_host + end + end + end + end + end +end diff --git a/spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb b/spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb index ae3aeeab7..20c02aa82 100644 --- a/spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb +++ b/spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb @@ -21,10 +21,14 @@ def valid_webhook_with describe "errors" do before do PactBroker.configuration.webhook_http_method_whitelist = webhook_http_method_whitelist + PactBroker.configuration.webhook_host_whitelist = webhook_host_whitelist + allow(PactBroker::Webhooks::CheckHostWhitelist).to receive(:call).and_return(whitelist_matches) subject.validate(hash) end let(:webhook_http_method_whitelist) { ['POST'] } + let(:whitelist_matches) { ['foo'] } + let(:webhook_host_whitelist) { [] } context "with valid fields" do it "is empty" do @@ -92,7 +96,7 @@ def valid_webhook_with end it "contains an error for the URL" do - expect(subject.errors[:"request.url"]).to include "must be https. See /doc/webhooks#whitelist for more information." + expect(subject.errors[:"request.url"]).to include "scheme must be https. See /doc/webhooks#whitelist for more information." end end @@ -118,6 +122,28 @@ def valid_webhook_with end end + context "when the host whitelist is empty" do + it "does not attempt to validate the host" do + expect(PactBroker::Webhooks::CheckHostWhitelist).to_not have_received(:call) + end + end + + context "when the host whitelist is populated" do + let(:webhook_host_whitelist) { [/foo/, "bar"] } + + it "validates the host" do + expect(PactBroker::Webhooks::CheckHostWhitelist).to have_received(:call).with("some.url", webhook_host_whitelist) + end + + context "when the host does not match the whitelist" do + let(:whitelist_matches) { [] } + + it "contains an error", pending: "need to work out how to do dynamic messages" do + expect(subject.errors[:"request.url"]).to include "host must be in the whitelist /foo/, \"bar\" . See /doc/webhooks#whitelist for more information." + end + end + end + context "with no URL" do let(:json) do valid_webhook_with do |hash| diff --git a/spec/lib/pact_broker/webhooks/check_host_whitelist_spec.rb b/spec/lib/pact_broker/webhooks/check_host_whitelist_spec.rb new file mode 100644 index 000000000..edd83c411 --- /dev/null +++ b/spec/lib/pact_broker/webhooks/check_host_whitelist_spec.rb @@ -0,0 +1,47 @@ +require 'pact_broker/webhooks/check_host_whitelist' + +module PactBroker + module Webhooks + describe CheckHostWhitelist do + context "when the host is 10.0.0.7" do + let(:host) { "10.0.1.0" } + + it "matches 10.0.0.0/8" do + expect(CheckHostWhitelist.call(host, ["10.0.0.0/8"])).to eq ["10.0.0.0/8"] + end + + it "matches 10.0.1.0" do + expect(CheckHostWhitelist.call(host, [host])).to eq [host] + end + + it "does not match 10.0.0.2" do + expect(CheckHostWhitelist.call(host, ["10.0.0.2"])).to eq [] + end + + it "does not match 10.0.0.0/28" do + expect(CheckHostWhitelist.call(host, ["10.0.0.0/28"])).to eq [] + end + end + + context "when the host is localhost" do + let(:host) { "localhost" } + + it "matches localhost" do + expect(CheckHostWhitelist.call(host, [host])).to eq [host] + end + + it "matches /local.*/" do + expect(CheckHostWhitelist.call(host, [/local*/])).to eq [/local*/] + end + + it "does not match foo" do + expect(CheckHostWhitelist.call(host, ["foo"])).to eq [] + end + + it "does not match /foo.*/" do + expect(CheckHostWhitelist.call(host, [/foo*/])).to eq [] + end + end + end + end +end