diff --git a/README.md b/README.md index fbc1d49..3b3b36c 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,8 @@ To enable basic auth, run your container with: * `PACT_BROKER_BASIC_AUTH_READ_ONLY_USERNAME` * `PACT_BROKER_BASIC_AUTH_READ_ONLY_PASSWORD` +If you want to allow public read access (but still require credentials for writing), then omit setting the READ_ONLY credentials and set `PACT_BROKER_ALLOW_PUBLIC_READ=true`. + Developers should use the read only credentials on their local machines, and the CI should use the read/write credentials. This will ensure that pacts and verification results are only published from your CI. Note that the [verification status badges][badges] are not protected by basic auth, so that you may embed them in README markdown. @@ -77,8 +79,8 @@ Set the environment variable `PACT_BROKER_LOG_LEVEL` to one of `DEBUG`, `INFO`, ## Other environment variables * PACT_BROKER_BASE_EQUALITY_ONLY_ON_CONTENT_THAT_AFFECTS_VERIFICATION_RESULTS - `true` by default, may be set to `false`. -* PACT_BROKER_ORDER_VERSIONS_BY_DATE - `true` by default, may be set to `false`. -* PACT_BROKER_DISABLE_SSL_VERIFICATION - `false` by default, may be set to `true`. +* PACT_BROKER_ORDER_VERSIONS_BY_DATE - `true` by default. Setting this to false is deprecated. +* PACT_BROKER_DISABLE_SSL_VERIFICATION - `false` by default, may be set to `true`. Disables SSL verification for webhook endpoints. ## General Pact Broker configuration and usage diff --git a/pact_broker/basic_auth.rb b/pact_broker/basic_auth.rb index 0e2f8e3..8c409be 100644 --- a/pact_broker/basic_auth.rb +++ b/pact_broker/basic_auth.rb @@ -4,47 +4,67 @@ class BasicAuth GET = 'GET'.freeze OPTIONS = 'OPTIONS'.freeze HEAD = 'HEAD'.freeze + READ_METHODS = [GET, OPTIONS, HEAD].freeze PACT_BADGE_PATH = %r{^/pacts/provider/[^/]+/consumer/.*/badge(?:\.[A-Za-z]+)?$}.freeze MATRIX_BADGE_PATH = %r{^/matrix/provider/[^/]+/latest/[^/]+/consumer/[^/]+/latest/[^/]+/badge(?:\.[A-Za-z]+)?$}.freeze HEARTBEAT_PATH = "/diagnostic/status/heartbeat".freeze - def initialize(app, write_user_username, write_user_password, read_user_username, read_user_password, allow_public_access_to_heartbeat) + def initialize(app, write_user_username, write_user_password, read_user_username, read_user_password, allow_public_read_access, allow_public_access_to_heartbeat) @app = app - @write_user_username = write_user_username - @write_user_password = write_user_password - @read_user_username = read_user_username - @read_user_password = read_user_password + @write_credentials = [write_user_username, write_user_password] + @read_credentials = [read_user_username, read_user_password] @allow_public_access_to_heartbeat = allow_public_access_to_heartbeat - - @app_with_write_auth = Rack::Auth::Basic.new(app, "Restricted area") do |username, password| - username == @write_user_username && password == @write_user_password - end - - @app_with_read_auth = if read_user_username && read_user_username.size > 0 - Rack::Auth::Basic.new(app, "Restricted area") do |username, password| - (username == @write_user_username && password == @write_user_password) || - (username == @read_user_username && password == @read_user_password) - end - else - puts "WARN: Public read access is enabled as no PACT_BROKER_BASIC_AUTH_READ_ONLY_USERNAME has been set" - app - end + @app_with_write_auth = build_app_with_write_auth + @app_with_read_auth = build_app_with_read_auth(allow_public_read_access) end def call(env) if use_basic_auth? env if read_request?(env) - @app_with_read_auth.call(env) + app_with_read_auth.call(env) else - @app_with_write_auth.call(env) + app_with_write_auth.call(env) end else - @app.call(env) + app.call(env) + end + end + + protected + + def write_credentials_match(*credentials) + credentials == write_credentials + end + + def read_credentials_match(*credentials) + is_set(read_credentials[0]) && credentials == read_credentials + end + + private + + attr_reader :app, :app_with_read_auth, :app_with_write_auth, :write_credentials, :read_credentials, :allow_public_access_to_heartbeat + + def build_app_with_write_auth + this = self + Rack::Auth::Basic.new(app, "Restricted area") do |username, password| + this.write_credentials_match(username, password) + end + end + + def build_app_with_read_auth(allow_public_read_access) + if allow_public_read_access + puts "INFO: Public read access is enabled" + app + else + this = self + Rack::Auth::Basic.new(app, "Restricted area") do |username, password| + this.write_credentials_match(username, password) || this.read_credentials_match(username, password) + end end end def read_request?(env) - env.fetch(REQUEST_METHOD) == GET || env.fetch(REQUEST_METHOD) == OPTIONS || env.fetch(REQUEST_METHOD) == HEAD + READ_METHODS.include?(env[REQUEST_METHOD]) end def use_basic_auth?(env) @@ -56,6 +76,10 @@ def allow_public_access(env) end def is_heartbeat_and_public_access_allowed?(env) - @allow_public_access_to_heartbeat && env[PATH_INFO] == HEARTBEAT_PATH + allow_public_access_to_heartbeat && env[PATH_INFO] == HEARTBEAT_PATH + end + + def is_set(string) + string && string.strip.size > 0 end end diff --git a/pact_broker/config.ru b/pact_broker/config.ru index 31db4d5..b5ab1d3 100644 --- a/pact_broker/config.ru +++ b/pact_broker/config.ru @@ -29,10 +29,11 @@ end basic_auth_username = ENV.fetch('PACT_BROKER_BASIC_AUTH_USERNAME','') basic_auth_password = ENV.fetch('PACT_BROKER_BASIC_AUTH_PASSWORD', '') -basic_auth_read_only_username = ENV.fetch('PACT_BROKER_BASIC_AUTH_READ_ONLY_USERNAME','') -basic_auth_read_only_password = ENV.fetch('PACT_BROKER_BASIC_AUTH_READ_ONLY_PASSWORD', '') -use_basic_auth = basic_auth_username != '' && basic_auth_password != '' +basic_auth_read_only_username = ENV['PACT_BROKER_BASIC_AUTH_READ_ONLY_USERNAME'] +basic_auth_read_only_password = ENV['PACT_BROKER_BASIC_AUTH_READ_ONLY_PASSWORD'] +allow_public_read_access = ENV.fetch('PACT_BROKER_ALLOW_PUBLIC_READ', '') == 'true' allow_public_access_to_heartbeat = ENV.fetch('PACT_BROKER_PUBLIC_HEARTBEAT', '') == 'true' +use_basic_auth = basic_auth_username != '' && basic_auth_password != '' if use_basic_auth use BasicAuth, @@ -40,6 +41,7 @@ if use_basic_auth basic_auth_password, basic_auth_read_only_username, basic_auth_read_only_password, + allow_public_read_access, allow_public_access_to_heartbeat end diff --git a/spec/basic_auth_spec.rb b/spec/basic_auth_spec.rb index 58fa17d..85ce95f 100644 --- a/spec/basic_auth_spec.rb +++ b/spec/basic_auth_spec.rb @@ -7,7 +7,8 @@ let(:protected_app) { ->(env) { [200, {}, []]} } - let(:app) { BasicAuth.new(protected_app, 'write_username', 'write_password', read_username, read_password, allow_public_access_to_heartbeat) } + let(:app) { BasicAuth.new(protected_app, 'write_username', 'write_password', read_username, read_password, allow_public_read_access, allow_public_access_to_heartbeat) } + let(:allow_public_read_access) { false } let(:read_username) { 'read_username' } let(:read_password) { 'read_password' } let(:allow_public_access_to_heartbeat) { true } @@ -107,9 +108,17 @@ end end - context "with the incorrect username and password for the write user" do + context "with the incorrect username for the write user" do it "does not allow POST" do - basic_authorize 'foo', 'password' + basic_authorize 'wrong_username', 'write_password' + post "/" + expect(last_response.status).to eq 401 + end + end + + context "with the incorrect password for the write user" do + it "does not allow POST" do + basic_authorize 'write_username', 'wrong_password' post "/" expect(last_response.status).to eq 401 end @@ -189,21 +198,79 @@ allow($stdout).to receive(:puts) end - let(:read_username) { '' } - let(:read_password) { '' } + let(:read_username) { nil } + let(:read_password) { nil } - context "with no credentials" do - it "allows a GET" do - get "/" - expect(last_response.status).to eq 200 + context "when allow_public_read_access is false" do + context "with no credentials" do + it "does not allow a GET" do + get "/" + expect(last_response.status).to eq 401 + end + end + + context "with empty credentials" do + it "does not allow a GET" do + basic_authorize('', '') + get "/" + expect(last_response.status).to eq 401 + end + end + + context "with incorrect credentials" do + it "does not allow a GET" do + basic_authorize("foo", "bar") + get "/" + expect(last_response.status).to eq 401 + end end end - context "with incorrect credentials" do - it "allows a GET" do - basic_authorize "foo", "bar" - get "/" - expect(last_response.status).to eq 200 + context "when allow_public_read_access is true" do + let(:allow_public_read_access) { true } + + context "with no credentials" do + it "allows a GET" do + get "/" + expect(last_response.status).to eq 200 + end + + it "does not allow POST" do + post "/" + expect(last_response.status).to eq 401 + end + end + + context "with empty credentials" do + before do + basic_authorize("", "") + end + + it "allows a GET" do + get "/" + expect(last_response.status).to eq 200 + end + + it "does not allow POST" do + post "/" + expect(last_response.status).to eq 401 + end + end + + context "with incorrect credentials" do + before do + basic_authorize("foo", "bar") + end + + it "allows a GET" do + get "/" + expect(last_response.status).to eq 200 + end + + it "does not allow POST" do + post "/" + expect(last_response.status).to eq 401 + end end end end