feat: allow POST access to the 'pacts for verification' endpoint for …

…the read only user (#22)

* feat: allow SQL log level and warn duration to be set via environment variables and hush the 'table not found' errors that freak people out

* feat: allow POST access to the 'pacts for verification' endpoint for the read only user

* chore: freeze

* chore: downcase sql log level

README.md

@@ -52,6 +52,8 @@ For an sqlite database (only recommended for investigation/spikes, as it will be
 You can additionally set:
 
     * PACT_BROKER_DATABASE_SSLMODE (optional, possible values: 'disable', 'allow', 'prefer', 'require', 'verify-ca', or 'verify-full' to choose how to treat SSL (only respected if using the postgres database adapter). See https://www.postgresql.org/docs/9.1/libpq-ssl.html for more information.)
+    * PACT_BROKER_SQL_LOG_LEVEL (optional, defaults to debug. The level at which to log SQL statements.)
+    * PACT_BROKER_SQL_LOG_WARN_DURATION (optional, defaults to 5 seconds. Log the SQL for queries that take longer than this number of seconds)
 
 3. Test the pact broker environment by executing [script/test.sh][test-script]
 

docker-compose.yml

@@ -30,6 +30,7 @@ services:
       # PACT_BROKER_DATABASE_NAME: postgres
       PACT_BROKER_PORT: "9292"
       PACT_BROKER_LOG_LEVEL: INFO
+      PACT_BROKER_SQL_LOG_LEVEL: DEBUG
 
   # Nginx is not necessary, but demonstrates how
   # one might use a reverse proxy in front of the broker,

pact_broker/basic_auth.rb

@@ -1,32 +1,22 @@
-class BasicAuth
-  PATH_INFO = 'PATH_INFO'.freeze
-  REQUEST_METHOD = 'REQUEST_METHOD'.freeze
-  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
+require_relative 'pact_broker_resource_access_policy'
 
-  def initialize(app, write_user_username, write_user_password, read_user_username, read_user_password, allow_public_read_access, allow_public_access_to_heartbeat)
+class BasicAuth
+  def initialize(app, write_credentials, read_credentials, policy)
     @app = app
-    @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
+    @write_credentials = write_credentials
+    @read_credentials = read_credentials
     @app_with_write_auth = build_app_with_write_auth
-    @app_with_read_auth = build_app_with_read_auth(allow_public_read_access)
+    @app_with_read_auth = build_app_with_read_auth
+    @policy = policy
   end
 
   def call(env)
-    if use_basic_auth? env
-      if read_request?(env)
-        app_with_read_auth.call(env)
-      else
-        app_with_write_auth.call(env)
-      end
-    else
+    if policy.public_access_allowed?(env)
       app.call(env)
+    elsif policy.read_access_allowed?(env)
+      app_with_read_auth.call(env)
+    else
+      app_with_write_auth.call(env)
     end
   end
 
@@ -42,7 +32,7 @@ def read_credentials_match(*credentials)
 
   private
 
-  attr_reader :app, :app_with_read_auth, :app_with_write_auth, :write_credentials, :read_credentials, :allow_public_access_to_heartbeat
+  attr_reader :app, :app_with_read_auth, :app_with_write_auth, :write_credentials, :read_credentials, :policy
 
   def build_app_with_write_auth
     this = self
@@ -51,34 +41,13 @@ def build_app_with_write_auth
     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
+  def build_app_with_read_auth
+    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
 
-  def read_request?(env)
-    READ_METHODS.include?(env[REQUEST_METHOD])
-  end
-
-  def use_basic_auth?(env)
-    !allow_public_access(env)
-  end
-
-  def allow_public_access(env)
-    env[PATH_INFO] =~ PACT_BADGE_PATH || env[PATH_INFO] =~ MATRIX_BADGE_PATH || is_heartbeat_and_public_access_allowed?(env)
-  end
-
-  def is_heartbeat_and_public_access_allowed?(env)
-    allow_public_access_to_heartbeat && env[PATH_INFO] == HEARTBEAT_PATH
-  end
-
   def is_set(string)
     string && string.strip.size > 0
   end

pact_broker/config.ru

@@ -4,6 +4,7 @@ require_relative 'logger'
 require_relative 'basic_auth'
 require_relative 'database_connection'
 require_relative 'docker_configuration'
+require_relative 'pact_broker_resource_access_policy'
 
 dc = PactBroker::DockerConfiguration.new(ENV, PactBroker::Configuration.default_configuration)
 dc.pact_broker_environment_variables.each{ |key, value| $logger.info "#{key}=#{value}"}
@@ -37,14 +38,14 @@ allow_public_read_access = ENV.fetch('PACT_BROKER_ALLOW_PUBLIC_READ', '') == 'tr
 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
+  puts "INFO: Public read access is enabled" if allow_public_read_access
+  policy = PactBrokerResourceAccessPolicy.build(allow_public_read_access, allow_public_access_to_heartbeat)
   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
+        [basic_auth_username, basic_auth_password],
+        [basic_auth_read_only_username, basic_auth_read_only_password],
+        policy
 end
 
 run app

pact_broker/database_connection.rb

@@ -20,8 +20,7 @@ def create_database_connection_from_config(logger, config)
   # when databases are restarted and connections are killed.  This has a performance
   # penalty, so consider increasing this timeout if building a frequently accessed service.
   logger.info "Connecting to database with config: #{config.merge(password: "*****")}"
-  config[:logger] = DatabaseLogger.new(logger)
-  connection = Sequel.connect(config)
+  connection = Sequel.connect(config.merge(logger: DatabaseLogger.new(logger)))
   connection.extension(:connection_validator)
   connection.pool.connection_validation_timeout = -1
   connection

pact_broker/database_logger.rb

@@ -1,7 +1,29 @@
+# This changes the ERROR logs that occur when
+# Sequel doesn't know if a table/view exists or not to DEBUG,
+# so that they don't freak newbies out when they start up the
+# broker for the first time.
+
 require 'delegate'
 
 class DatabaseLogger < SimpleDelegator
-  def info *args
-    __getobj__().debug(*args)
+  def error *args
+    if error_is_about_table_not_existing?(args)
+      __getobj__().debug(*reassure_people_that_this_is_expected(args))
+    else
+      __getobj__().error(*args)
+    end
+  end
+
+  def error_is_about_table_not_existing?(args)
+    args.first.is_a?(String) &&
+      ( args.first.include?("PG::UndefinedTable") ||
+        args.first.include?("no such table") ||
+        args.first.include?("no such view"))
+  end
+
+  def reassure_people_that_this_is_expected(args)
+    message = args.shift
+    message = message + " Don't panic. This happens when Sequel doesn't know if a table/view exists or not."
+    [message] + args
   end
 end

pact_broker/docker_configuration.rb

@@ -32,7 +32,12 @@ def database_configuration
         database_configuration_from_url
       else
         database_configuration_from_parts
-      end.merge(encoding: 'utf8', sslmode: env_or_nil(:database_sslmode)).compact
+      end.merge(
+        encoding: 'utf8',
+        sslmode: env_or_nil(:database_sslmode),
+        sql_log_level: (env_or_nil(:sql_log_level) || 'debug').downcase.to_sym,
+        log_warn_duration: (env_or_nil(:sql_log_warn_duration) || '5').to_f
+      ).compact
     end
 
     def base_equality_only_on_content_that_affects_verification_results

pact_broker/pact_broker_resource_access_policy.rb

@@ -0,0 +1,55 @@
+require 'rack'
+require_relative 'resource_access_rules'
+
+class PactBrokerResourceAccessPolicy
+  READ_METHODS                = %w{GET OPTIONS HEAD}.freeze
+  ALL_METHODS                 = %w{GET POST PUT PATCH DELETE HEAD OPTIONS}.freeze
+  POST                        = 'POST'.freeze
+
+  ALL_PATHS                   = %r{.*}.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              = %r{^/diagnostic/status/heartbeat$}.freeze
+  PACTS_FOR_VERIFICATION_PATH = %r{^/pacts/provider/[^/]+/for-verification$}.freeze
+
+  PUBLIC = 0
+  READ = 1
+  WRITE = 2
+
+  def initialize(resource_access_rules)
+    @resource_access_rules = resource_access_rules
+  end
+
+  def public_access_allowed?(env)
+    resource_access_rules.access_allowed?(env, PUBLIC)
+  end
+
+  def read_access_allowed?(env)
+    resource_access_rules.access_allowed?(env, READ)
+  end
+
+  def self.build(allow_public_read_access, allow_public_access_to_heartbeat)
+    rules = [
+      [WRITE, ALL_METHODS, ALL_PATHS],
+      [READ, READ_METHODS, ALL_PATHS],
+      [READ, [POST], PACTS_FOR_VERIFICATION_PATH],
+      [PUBLIC, READ_METHODS, PACT_BADGE_PATH],
+      [PUBLIC, READ_METHODS, MATRIX_BADGE_PATH]
+    ]
+
+    if allow_public_access_to_heartbeat
+      rules.unshift([PUBLIC, READ_METHODS, HEARTBEAT_PATH])
+    end
+
+    if allow_public_read_access
+      rules.unshift([PUBLIC, READ_METHODS, ALL_PATHS])
+      rules.unshift([PUBLIC, [POST], PACTS_FOR_VERIFICATION_PATH])
+    end
+
+    PactBrokerResourceAccessPolicy.new(ResourceAccessRules.new(rules))
+  end
+
+  private
+
+  attr_reader :resource_access_rules
+end

pact_broker/resource_access_rules.rb

@@ -0,0 +1,34 @@
+require 'rack'
+
+class ResourceAccessRules
+  PATH_INFO = Rack::PATH_INFO
+  REQUEST_METHOD = Rack::REQUEST_METHOD
+
+  def initialize(rules)
+    @rules = rules
+  end
+
+  def access_allowed?(env, level)
+    !!rules.find do | rule_level, allowed_methods, path_pattern |
+      level_allowed?(level, rule_level) &&
+        method_allowed?(env, allowed_methods) &&
+        path_allowed?(env, path_pattern)
+    end
+  end
+
+  private
+
+  attr_reader :rules
+
+  def level_allowed?(level, rule_level)
+    level >= rule_level
+  end
+
+  def path_allowed?(env, pattern)
+    env[PATH_INFO] =~ pattern
+  end
+
+  def method_allowed?(env, allowed_methods)
+    allowed_methods.include?(env[REQUEST_METHOD])
+  end
+end

spec/basic_auth_spec.rb

@@ -7,13 +7,13 @@
 
   let(:protected_app) { ->(env) { [200, {}, []]} }
 
-  let(:app) { BasicAuth.new(protected_app, 'write_username', 'write_password', read_username, read_password, allow_public_read_access, allow_public_access_to_heartbeat) }
+  let(:policy) { PactBrokerResourceAccessPolicy.build(allow_public_read_access, allow_public_access_to_heartbeat) }
+  let(:app) { BasicAuth.new(protected_app, ['write_username', 'write_password'], [read_username, read_password], policy) }
   let(:allow_public_read_access) { false }
   let(:read_username) { 'read_username' }
   let(:read_password) { 'read_password' }
   let(:allow_public_access_to_heartbeat) { true }
 
-
   context "when requesting the heartbeat" do
     let(:path) { "/diagnostic/status/heartbeat" }
 
@@ -166,6 +166,12 @@
       delete "/"
       expect(last_response.status).to eq 401
     end
+
+    it "allows POST to the pacts for verification endpoint" do
+      basic_authorize 'read_username', 'read_password'
+      post "/pacts/provider/Foo/for-verification"
+      expect(last_response.status).to eq 200
+    end
   end
 
   context "with the incorrect username and password for the write user" do