Skip to content

Commit

Permalink
feat: allow multiple base URLs to be configured
Browse files Browse the repository at this point in the history
Closes: #392
  • Loading branch information
bethesque committed Feb 25, 2021
1 parent d91662d commit f88c69d
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 8 deletions.
2 changes: 1 addition & 1 deletion config.ru
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ app = PactBroker::App.new do | config |
config.webhook_scheme_whitelist = ['http', 'https']
config.webhook_http_method_whitelist = ['GET', 'POST']
config.webhook_http_code_success = [200, 201, 202, 203, 204, 205, 206]
#config.base_url = ENV['PACT_BROKER_BASE_URL']
config.base_url = ENV['PACT_BROKER_BASE_URL']

database_logger = PactBroker::DB::LogQuietener.new(config.logger)
config.database_connection = Sequel.connect(DATABASE_URL, DB_OPTIONS.merge(logger: database_logger))
Expand Down
10 changes: 9 additions & 1 deletion docker-compose-dev-postgres.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,21 @@ services:
environment:
DATABASE_URL: postgres://postgres:postgres@postgres/postgres
PACT_BROKER_HIDE_PACTFLOW_MESSAGES: 'true'
command: dockerize -wait tcp://postgres:5432 /home/start.sh
PACT_BROKER_BASE_URL: 'http://localhost:9292 http://pact-broker:9292'
command: -wait tcp://postgres:5432 /usr/local/bin/start
entrypoint: dockerize
volumes:
- ./lib:/home/lib
- ./db:/home/db
- ./config.ru:/home/config.ru
- ./tasks:/home/tasks
- ./Rakefile:/home/Rakefile

shell:
image: ruby:2.5.3-alpine
depends_on:
- pact-broker
entrypoint: /bin/sh

volumes:
postgres-volume:
5 changes: 5 additions & 0 deletions lib/pact_broker/api/resources/default_base_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ def identifier_from_path
alias_method :path_info, :identifier_from_path

def base_url
# Have to use something for the base URL here - we can't use an empty string as we can in the UI.
# Can't work out if cache poisoning is a vulnerability for APIs or not.
# Using the request base URI as a fallback if the base_url is not configured may be a vulnerability,
# but the documentation recommends that the
# base_url should be set in the configuration to mitigate this.
request.env["pactbroker.base_url"] || request.base_uri.to_s.chomp('/')
end

Expand Down
2 changes: 1 addition & 1 deletion lib/pact_broker/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ def configure_middleware
@app_builder.use Rack::Static, :urls => ["/favicon.ico"], :root => PactBroker.project_root.join("public/images"), header_rules: [[:all, {'Content-Type' => 'image/x-icon'}]]
@app_builder.use Rack::PactBroker::ConvertFileExtensionToAcceptHeader
# Rack::PactBroker::SetBaseUrl needs to be before the Rack::PactBroker::HalBrowserRedirect
@app_builder.use Rack::PactBroker::SetBaseUrl, configuration.base_url
@app_builder.use Rack::PactBroker::SetBaseUrl, configuration.base_urls

if configuration.use_hal_browser
logger.info "Mounting HAL browser"
Expand Down
4 changes: 4 additions & 0 deletions lib/pact_broker/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,10 @@ def webhook_host_whitelist= webhook_host_whitelist
@webhook_host_whitelist = parse_space_delimited_string_list_property('webhook_host_whitelist', webhook_host_whitelist)
end

def base_urls
base_url ? base_url.split(" ") : []
end

def warning_error_classes
warning_error_class_names.collect do | class_name |
begin
Expand Down
40 changes: 35 additions & 5 deletions lib/rack/pact_broker/set_base_url.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,52 @@
module Rack
module PactBroker
class SetBaseUrl
def initialize app, base_url
X_FORWARDED_PATTERN = /_X_FORWARDED_/.freeze

def initialize app, base_urls
@app = app
@base_url = base_url
@base_urls = base_urls
end

def call env
def call(env)
if env["pactbroker.base_url"]
app.call(env)
else
app.call(env.merge("pactbroker.base_url" => base_url))
app.call(env.merge("pactbroker.base_url" => select_matching_base_url(env)))
end
end

private

attr_reader :app, :base_url
attr_reader :app, :base_urls

def select_matching_base_url(env)
if base_urls.size > 1
return matching_base_url_considering_x_forwarded_headers(env) ||
matching_base_url_not_considering_x_forwarded_headers(env) ||
default_base_url
end
default_base_url
end

def default_base_url
base_urls.first
end

def matching_base_url_considering_x_forwarded_headers(env)
matching_base_url(env)
end

def matching_base_url_not_considering_x_forwarded_headers(env)
matching_base_url(env.reject{ |k, _| k =~ X_FORWARDED_PATTERN} )
end

def matching_base_url(env)
request_base_url = Rack::Request.new(env).base_url
if base_urls.include?(request_base_url)
request_base_url
end
end
end
end
end
86 changes: 86 additions & 0 deletions spec/lib/rack/pact_broker/set_base_url_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
require 'rack/pact_broker/set_base_url'

module Rack
module PactBroker
describe SetBaseUrl do
let(:base_urls) { ["http://pact-broker"] }
let(:rack_env) { {} }
let(:target_app) { double('app', call: [200, {}, []]) }
let(:app) { SetBaseUrl.new(target_app, base_urls) }

subject { get("/", {}, rack_env) }

describe "#call" do
context "when the base_url is already set" do
let(:rack_env) { { "pactbroker.base_url" => "http://foo"} }

it "does not overwrite it" do
expect(target_app).to receive(:call).with(hash_including("pactbroker.base_url" => "http://foo"))
subject
end
end

context "when there is one base URL" do
it "sets that base_url" do
expect(target_app).to receive(:call).with(hash_including("pactbroker.base_url" => "http://pact-broker"))
subject
end
end

context "when there are no base URLs" do
let(:base_urls) { [] }

it "sets the base URL to nil" do
expect(target_app).to receive(:call).with(hash_including("pactbroker.base_url" => nil))
subject
end
end

context "when there are multiple base URLs" do
let(:base_urls) { ["https://foo", "https://pact-broker-external", "http://pact-broker-internal"] }

let(:host) { "pact-broker-internal" }
let(:scheme) { "http" }
let(:forwarded_host) { "pact-broker-external" }
let(:forwarded_scheme) { "https" }
let(:rack_env) do
{
Rack::HTTP_HOST => host,
Rack::RACK_URL_SCHEME => scheme,
"HTTP_X_FORWARDED_HOST" => forwarded_host,
"HTTP_X_FORWARDED_SCHEME" => forwarded_scheme
}
end

context "when the base URL created taking any X-Forwarded headers into account matches one of the base URLs" do
it "uses that base URL" do
expect(target_app).to receive(:call).with(hash_including("pactbroker.base_url" => "https://pact-broker-external"))
subject
end
end

context "when the base URL created NOT taking the X-Forwarded headers into account matches one of the base URLs (potential cache poisoning)" do
let(:forwarded_host) { "pact-broker-external-wrong" }

it "uses that base URL" do
expect(target_app).to receive(:call).with(hash_including("pactbroker.base_url" => "http://pact-broker-internal"))
subject
end
end

context "when neither base URL matches the base URLs (potential cache poisoning)" do
before do
rack_env["HTTP_HOST"] = "silly-buggers-1"
rack_env["HTTP_X_FORWARDED_HOST"] = "silly-buggers-1"
end

it "uses the first base URL" do
expect(target_app).to receive(:call).with(hash_including("pactbroker.base_url" => "https://foo"))
subject
end
end
end
end
end
end
end

0 comments on commit f88c69d

Please sign in to comment.