diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7afda9ae..46d67464 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -27,6 +27,7 @@ jobs: - rack_3 - rack_2 - rack_1 + - rails_7_1 - rails_7_0 - rails_6_1 - rails_6_0 @@ -73,6 +74,10 @@ jobs: ruby: 2.6.10 - gemfile: rails_7_0 ruby: 2.5.8 + - gemfile: rails_7_1 + ruby: 2.6.10 + - gemfile: rails_7_1 + ruby: 2.5.8 env: BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }}.gemfile steps: diff --git a/.rubocop.yml b/.rubocop.yml index 745bb907..865fea87 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,7 @@ require: + - rubocop-minitest - rubocop-performance + - rubocop-rake inherit_mode: merge: @@ -56,7 +58,6 @@ Security: Style/BlockDelimiters: Enabled: true - IgnoredMethods: [] # Workaround rubocop bug: https://github.com/rubocop-hq/rubocop/issues/6179 Style/ClassAndModuleChildren: Enabled: true diff --git a/Appraisals b/Appraisals index 4f222a0f..fe499170 100644 --- a/Appraisals +++ b/Appraisals @@ -21,6 +21,10 @@ appraise "rack_1" do gem "rack-test", ">= 0.6" end +appraise 'rails_7-1' do + gem 'railties', '~> 7.1.0' +end + appraise 'rails_7-0' do gem 'railties', '~> 7.0.0' end diff --git a/gemfiles/rails_7_1.gemfile b/gemfiles/rails_7_1.gemfile new file mode 100644 index 00000000..036d9836 --- /dev/null +++ b/gemfiles/rails_7_1.gemfile @@ -0,0 +1,7 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "railties", "~> 7.1.0" + +gemspec path: "../" diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index b3488ba2..c9094b21 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -17,8 +17,11 @@ module Rack class Attack class Error < StandardError; end + class MisconfiguredStoreError < Error; end + class MissingStoreError < Error; end + class IncompatibleStoreError < Error; end autoload :Check, 'rack/attack/check' diff --git a/lib/rack/attack/base_proxy.rb b/lib/rack/attack/base_proxy.rb index 3e3c28af..f10af3d4 100644 --- a/lib/rack/attack/base_proxy.rb +++ b/lib/rack/attack/base_proxy.rb @@ -11,6 +11,7 @@ def proxies end def inherited(klass) + super proxies << klass end diff --git a/lib/rack/attack/configuration.rb b/lib/rack/attack/configuration.rb index c90fdee6..a4bdc987 100644 --- a/lib/rack/attack/configuration.rb +++ b/lib/rack/attack/configuration.rb @@ -61,11 +61,15 @@ def blocklist(name = nil, &block) end def blocklist_ip(ip_address) - @anonymous_blocklists << Blocklist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } + @anonymous_blocklists << Blocklist.new do |request| + request.ip && !request.ip.empty? && IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) + end end def safelist_ip(ip_address) - @anonymous_safelists << Safelist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } + @anonymous_safelists << Safelist.new do |request| + request.ip && !request.ip.empty? && IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) + end end def throttle(name, options, &block) diff --git a/rack-attack.gemspec b/rack-attack.gemspec index 1cfe2b9f..41cc7a8f 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -34,8 +34,10 @@ Gem::Specification.new do |s| s.add_development_dependency "minitest-stub-const", "~> 0.6" s.add_development_dependency 'rack-test', "~> 2.0" s.add_development_dependency 'rake', "~> 13.0" - s.add_development_dependency "rubocop", "0.89.1" - s.add_development_dependency "rubocop-performance", "~> 1.5.0" + s.add_development_dependency "rubocop", "1.12.1" + s.add_development_dependency "rubocop-minitest", "~> 0.11.1" + s.add_development_dependency "rubocop-performance", "~> 1.10.2" + s.add_development_dependency "rubocop-rake", "~> 0.5.1" s.add_development_dependency "timecop", "~> 0.9.1" # byebug only works with MRI diff --git a/spec/acceptance/blocking_ip_spec.rb b/spec/acceptance/blocking_ip_spec.rb index 102a8fce..4d08042f 100644 --- a/spec/acceptance/blocking_ip_spec.rb +++ b/spec/acceptance/blocking_ip_spec.rb @@ -19,6 +19,12 @@ assert_equal 200, last_response.status end + it "succeeds if IP is missing" do + get "/", {}, "REMOTE_ADDR" => "" + + assert_equal 200, last_response.status + end + it "notifies when the request is blocked" do notified = false notification_type = nil diff --git a/spec/acceptance/cache_store_config_for_fail2ban_spec.rb b/spec/acceptance/cache_store_config_for_fail2ban_spec.rb index 6f330eee..6fd79807 100644 --- a/spec/acceptance/cache_store_config_for_fail2ban_spec.rb +++ b/spec/acceptance/cache_store_config_for_fail2ban_spec.rb @@ -79,7 +79,7 @@ def write(key, value); end end it "works with any object that responds to #read, #write and #increment" do - FakeStore = Class.new do + fake_store_class = Class.new do attr_accessor :backend def initialize @@ -100,7 +100,7 @@ def increment(key, _count, _options = {}) end end - Rack::Attack.cache.store = FakeStore.new + Rack::Attack.cache.store = fake_store_class.new get "/" assert_equal 200, last_response.status diff --git a/spec/acceptance/extending_request_object_spec.rb b/spec/acceptance/extending_request_object_spec.rb index a4ea1a62..5449b90c 100644 --- a/spec/acceptance/extending_request_object_spec.rb +++ b/spec/acceptance/extending_request_object_spec.rb @@ -4,10 +4,8 @@ describe "Extending the request object" do before do - class Rack::Attack::Request - def authorized? - env["APIKey"] == "private-secret" - end + Rack::Attack::Request.define_method :authorized? do + env["APIKey"] == "private-secret" end Rack::Attack.blocklist("unauthorized requests") do |request| @@ -17,9 +15,7 @@ def authorized? # We don't want the extension to leak to other test cases after do - class Rack::Attack::Request - remove_method :authorized? - end + Rack::Attack::Request.undef_method :authorized? end it "forbids request if blocklist condition is true" do diff --git a/spec/acceptance/safelisting_ip_spec.rb b/spec/acceptance/safelisting_ip_spec.rb index c0b8faf5..fd80dc1b 100644 --- a/spec/acceptance/safelisting_ip_spec.rb +++ b/spec/acceptance/safelisting_ip_spec.rb @@ -17,6 +17,12 @@ assert_equal 403, last_response.status end + it "forbids request if blocklist condition is true and safelist is false (missing IP)" do + get "/admin", {}, "REMOTE_ADDR" => "" + + assert_equal 403, last_response.status + end + it "succeeds if blocklist condition is false and safelist is false" do get "/", {}, "REMOTE_ADDR" => "1.2.3.4" diff --git a/spec/rack_attack_request_spec.rb b/spec/rack_attack_request_spec.rb index 8d4d27fc..8f27301a 100644 --- a/spec/rack_attack_request_spec.rb +++ b/spec/rack_attack_request_spec.rb @@ -5,10 +5,8 @@ describe 'Rack::Attack' do describe 'helpers' do before do - class Rack::Attack::Request - def remote_ip - ip - end + Rack::Attack::Request.define_method :remote_ip do + ip end Rack::Attack.safelist('valid IP') do |req| diff --git a/spec/rack_attack_track_spec.rb b/spec/rack_attack_track_spec.rb index 0db66e47..d8c53b8a 100644 --- a/spec/rack_attack_track_spec.rb +++ b/spec/rack_attack_track_spec.rb @@ -3,17 +3,19 @@ require_relative 'spec_helper' describe 'Rack::Attack.track' do - class Counter - def self.incr - @counter += 1 - end + let(:counter_class) do + Class.new do + def self.incr + @counter += 1 + end - def self.reset - @counter = 0 - end + def self.reset + @counter = 0 + end - def self.check - @counter + def self.check + @counter + end end end @@ -32,19 +34,19 @@ def self.check describe "with a notification subscriber and two tracks" do before do - Counter.reset + counter_class.reset # A second track Rack::Attack.track("homepage") { |req| req.path == "/" } ActiveSupport::Notifications.subscribe("track.rack_attack") do |*_args| - Counter.incr + counter_class.incr end get "/" end it "should notify twice" do - _(Counter.check).must_equal 2 + _(counter_class.check).must_equal 2 end end