fix: require PACT_BROKER_ALLOW_PUBLIC_READ to be set to true to enabl…

…e public read access

README.md

@@ -62,6 +62,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.
@@ -90,7 +92,7 @@ Set the environment variable `PACT_BROKER_LOG_LEVEL` to one of `DEBUG`, `INFO`,
 * PACT_BROKER_PORT - the port that the Pact Broker application runs on. Defaults to 9292.
 * PACT_BROKER_DISABLE_SSL_VERIFICATION - `false` by default, may be set to `true`.
 * 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_ORDER_VERSIONS_BY_DATE - `true` by default. Setting this to false is deprecated.
 
 ## General Pact Broker configuration and usage
 

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

pact_broker/config.ru

@@ -3,6 +3,7 @@ require 'pact_broker'
 require_relative 'logger'
 require_relative 'basic_auth'
 require_relative 'database_connection'
+require_relative 'passenger_config'
 require_relative 'docker_configuration'
 
 dc = PactBroker::DockerConfiguration.new(ENV, PactBroker::Configuration.default_configuration)
@@ -28,17 +29,19 @@ 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,
         basic_auth_username,
         basic_auth_password,
         basic_auth_read_only_username,
         basic_auth_read_only_password,
+        allow_public_read_access,
         allow_public_access_to_heartbeat
 end
 

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