From 197bec6ab62ec94033f9b8d687e1364b95bc1039 Mon Sep 17 00:00:00 2001 From: Santiago Bartesaghi Date: Tue, 30 Dec 2025 14:21:56 -0300 Subject: [PATCH 1/6] Try removing all dev dependencies --- rack-attack.gemspec | 5 ----- 1 file changed, 5 deletions(-) diff --git a/rack-attack.gemspec b/rack-attack.gemspec index fe2e1566..e2784dd4 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -44,9 +44,4 @@ Gem::Specification.new do |s| if RUBY_ENGINE == "ruby" s.add_development_dependency 'byebug', '~> 11.0' end - - s.add_development_dependency "activesupport" - # Fix activesupport Direct version requirement on connection_pool - # can be removed once https://github.com/rails/rails/issues/56291 is ixed and released - s.add_development_dependency "connection_pool", "~> 2.5" end From 47610c3cee5e1932d6a1ce6ef9012b61d87e16ef Mon Sep 17 00:00:00 2001 From: Santiago Bartesaghi Date: Mon, 5 Jan 2026 12:17:28 -0300 Subject: [PATCH 2/6] Fix tests --- spec/acceptance/allow2ban_spec.rb | 94 +++--- spec/acceptance/blocking_ip_spec.rb | 56 ++-- spec/acceptance/blocking_spec.rb | 106 +++--- spec/acceptance/blocking_subnet_spec.rb | 56 ++-- .../cache_store_config_with_rails_spec.rb | 36 ++- .../customizing_throttled_response_spec.rb | 108 ++++--- spec/acceptance/fail2ban_spec.rb | 160 +++++----- spec/acceptance/safelisting_ip_spec.rb | 76 ++--- spec/acceptance/safelisting_spec.rb | 146 ++++----- spec/acceptance/safelisting_subnet_spec.rb | 68 ++-- .../active_support_mem_cache_store_spec.rb | 2 +- .../active_support_memory_store_spec.rb | 21 +- spec/acceptance/throttling_spec.rb | 230 ++++++------- spec/acceptance/track_spec.rb | 34 +- spec/acceptance/track_throttle_spec.rb | 54 ++-- spec/allow2ban_spec.rb | 180 +++++------ spec/fail2ban_spec.rb | 204 ++++++------ spec/integration/offline_spec.rb | 7 +- spec/rack_attack_instrumentation_spec.rb | 50 +-- spec/rack_attack_throttle_spec.rb | 302 +++++++++--------- spec/rack_attack_track_spec.rb | 66 ++-- spec/spec_helper.rb | 2 +- 22 files changed, 1051 insertions(+), 1007 deletions(-) diff --git a/spec/acceptance/allow2ban_spec.rb b/spec/acceptance/allow2ban_spec.rb index 6de18d07..c22f36d9 100644 --- a/spec/acceptance/allow2ban_spec.rb +++ b/spec/acceptance/allow2ban_spec.rb @@ -3,71 +3,73 @@ require_relative "../spec_helper" require "timecop" -describe "allow2ban" do - before do - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - - Rack::Attack.blocklist("allow2ban pentesters") do |request| - Rack::Attack::Allow2Ban.filter(request.ip, maxretry: 2, findtime: 30, bantime: 60) do - request.path.include?("scarce-resource") +if defined?(::ActiveSupport::Cache::MemoryStore) + describe "allow2ban" do + before do + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + + Rack::Attack.blocklist("allow2ban pentesters") do |request| + Rack::Attack::Allow2Ban.filter(request.ip, maxretry: 2, findtime: 30, bantime: 60) do + request.path.include?("scarce-resource") + end end end - end - - it "returns OK for many requests that doesn't match the filter" do - get "/" - assert_equal 200, last_response.status - get "/" - assert_equal 200, last_response.status - end + it "returns OK for many requests that doesn't match the filter" do + get "/" + assert_equal 200, last_response.status - it "returns OK for first request that matches the filter" do - get "/scarce-resource" - assert_equal 200, last_response.status - end + get "/" + assert_equal 200, last_response.status + end - it "forbids all access after reaching maxretry limit" do - get "/scarce-resource" - assert_equal 200, last_response.status + it "returns OK for first request that matches the filter" do + get "/scarce-resource" + assert_equal 200, last_response.status + end - get "/scarce-resource" - assert_equal 200, last_response.status + it "forbids all access after reaching maxretry limit" do + get "/scarce-resource" + assert_equal 200, last_response.status - get "/scarce-resource" - assert_equal 403, last_response.status + get "/scarce-resource" + assert_equal 200, last_response.status - get "/" - assert_equal 403, last_response.status - end + get "/scarce-resource" + assert_equal 403, last_response.status - it "restores access after bantime elapsed" do - get "/scarce-resource" - assert_equal 200, last_response.status + get "/" + assert_equal 403, last_response.status + end - get "/scarce-resource" - assert_equal 200, last_response.status + it "restores access after bantime elapsed" do + get "/scarce-resource" + assert_equal 200, last_response.status - get "/" - assert_equal 403, last_response.status + get "/scarce-resource" + assert_equal 200, last_response.status - Timecop.travel(60) do get "/" + assert_equal 403, last_response.status - assert_equal 200, last_response.status - end - end + Timecop.travel(60) do + get "/" - it "does not forbid all access if maxrety condition is met but not within the findtime timespan" do - get "/scarce-resource" - assert_equal 200, last_response.status + assert_equal 200, last_response.status + end + end - Timecop.travel(31) do + it "does not forbid all access if maxrety condition is met but not within the findtime timespan" do get "/scarce-resource" assert_equal 200, last_response.status - get "/" - assert_equal 200, last_response.status + Timecop.travel(31) do + get "/scarce-resource" + assert_equal 200, last_response.status + + get "/" + assert_equal 200, last_response.status + end end end end diff --git a/spec/acceptance/blocking_ip_spec.rb b/spec/acceptance/blocking_ip_spec.rb index 4e41f29b..449d4ec1 100644 --- a/spec/acceptance/blocking_ip_spec.rb +++ b/spec/acceptance/blocking_ip_spec.rb @@ -2,44 +2,46 @@ require_relative "../spec_helper" -describe "Blocking an IP" do - let(:notifications) { [] } +if defined?(::ActiveSupport::Notifications) + describe "Blocking an IP" do + let(:notifications) { [] } - before do - Rack::Attack.blocklist_ip("1.2.3.4") - end + before do + Rack::Attack.blocklist_ip("1.2.3.4") + end - it "forbids request if IP matches" do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + it "forbids request if IP matches" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 403, last_response.status - end - - it "succeeds if IP doesn't match" do - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + assert_equal 403, last_response.status + end - assert_equal 200, last_response.status - end + it "succeeds if IP doesn't match" do + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - it "succeeds if IP is missing" do - get "/", {}, "REMOTE_ADDR" => "" + assert_equal 200, last_response.status + end - assert_equal 200, last_response.status - end + it "succeeds if IP is missing" do + get "/", {}, "REMOTE_ADDR" => "" - it "notifies when the request is blocked" do - ActiveSupport::Notifications.subscribe("blocklist.rack_attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) + assert_equal 200, last_response.status end - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + it "notifies when the request is blocked" do + ActiveSupport::Notifications.subscribe("blocklist.rack_attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end - assert notifications.empty? + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + assert notifications.empty? - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] + end end end diff --git a/spec/acceptance/blocking_spec.rb b/spec/acceptance/blocking_spec.rb index 2ea9f67a..e83c3e5d 100644 --- a/spec/acceptance/blocking_spec.rb +++ b/spec/acceptance/blocking_spec.rb @@ -2,80 +2,82 @@ require_relative "../spec_helper" -describe "#blocklist" do - let(:notifications) { [] } - - before do - Rack::Attack.blocklist do |request| - request.ip == "1.2.3.4" +if defined?(::ActiveSupport::Notifications) + describe "#blocklist" do + let(:notifications) { [] } + + before do + Rack::Attack.blocklist do |request| + request.ip == "1.2.3.4" + end end - end - it "forbids request if blocklist condition is true" do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - - assert_equal 403, last_response.status - end + it "forbids request if blocklist condition is true" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - it "succeeds if blocklist condition is false" do - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + assert_equal 403, last_response.status + end - assert_equal 200, last_response.status - end + it "succeeds if blocklist condition is false" do + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - it "notifies when the request is blocked" do - ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) + assert_equal 200, last_response.status end - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + it "notifies when the request is blocked" do + ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end - assert notifications.empty? + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - - assert_equal 1, notifications.size - notification = notifications.pop - assert_nil notification[:request].env["rack.attack.matched"] - assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] - end -end + assert notifications.empty? -describe "#blocklist with name" do - let(:notifications) { [] } + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - before do - Rack::Attack.blocklist("block 1.2.3.4") do |request| - request.ip == "1.2.3.4" + assert_equal 1, notifications.size + notification = notifications.pop + assert_nil notification[:request].env["rack.attack.matched"] + assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] end end - it "forbids request if blocklist condition is true" do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + describe "#blocklist with name" do + let(:notifications) { [] } - assert_equal 403, last_response.status - end + before do + Rack::Attack.blocklist("block 1.2.3.4") do |request| + request.ip == "1.2.3.4" + end + end - it "succeeds if blocklist condition is false" do - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + it "forbids request if blocklist condition is true" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 200, last_response.status - end + assert_equal 403, last_response.status + end + + it "succeeds if blocklist condition is false" do + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - it "notifies when the request is blocked" do - ActiveSupport::Notifications.subscribe("blocklist.rack_attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) + assert_equal 200, last_response.status end - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + it "notifies when the request is blocked" do + ActiveSupport::Notifications.subscribe("blocklist.rack_attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end - assert notifications.empty? + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + assert notifications.empty? - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal "block 1.2.3.4", notification[:request].env["rack.attack.matched"] - assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal "block 1.2.3.4", notification[:request].env["rack.attack.matched"] + assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] + end end end diff --git a/spec/acceptance/blocking_subnet_spec.rb b/spec/acceptance/blocking_subnet_spec.rb index 9fe30598..d2403b04 100644 --- a/spec/acceptance/blocking_subnet_spec.rb +++ b/spec/acceptance/blocking_subnet_spec.rb @@ -2,44 +2,46 @@ require_relative "../spec_helper" -describe "Blocking an IP subnet" do - let(:notifications) { [] } +if defined?(::ActiveSupport::Notifications) + describe "Blocking an IP subnet" do + let(:notifications) { [] } - before do - Rack::Attack.blocklist_ip("1.2.3.4/31") - end + before do + Rack::Attack.blocklist_ip("1.2.3.4/31") + end - it "forbids request if IP is inside the subnet" do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + it "forbids request if IP is inside the subnet" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 403, last_response.status - end - - it "forbids request for another IP in the subnet" do - get "/", {}, "REMOTE_ADDR" => "1.2.3.5" + assert_equal 403, last_response.status + end - assert_equal 403, last_response.status - end + it "forbids request for another IP in the subnet" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.5" - it "succeeds if IP is outside the subnet" do - get "/", {}, "REMOTE_ADDR" => "1.2.3.6" + assert_equal 403, last_response.status + end - assert_equal 200, last_response.status - end + it "succeeds if IP is outside the subnet" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.6" - it "notifies when the request is blocked" do - ActiveSupport::Notifications.subscribe("blocklist.rack_attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) + assert_equal 200, last_response.status end - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + it "notifies when the request is blocked" do + ActiveSupport::Notifications.subscribe("blocklist.rack_attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end - assert notifications.empty? + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + assert notifications.empty? - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] + end end end diff --git a/spec/acceptance/cache_store_config_with_rails_spec.rb b/spec/acceptance/cache_store_config_with_rails_spec.rb index 66bb76a8..23cb1046 100644 --- a/spec/acceptance/cache_store_config_with_rails_spec.rb +++ b/spec/acceptance/cache_store_config_with_rails_spec.rb @@ -4,32 +4,34 @@ require "minitest/stub_const" require "ostruct" -describe "Cache store config with Rails" do - before do - Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| - request.ip +if defined?(::ActiveSupport::Cache::MemoryStore) + describe "Cache store config with Rails" do + before do + Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| + request.ip + end end - end - unless defined?(Rails) - it "fails when Rails.cache is not set" do - Object.stub_const(:Rails, OpenStruct.new(cache: nil)) do - assert_raises(Rack::Attack::MissingStoreError) do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + unless defined?(Rails) + it "fails when Rails.cache is not set" do + Object.stub_const(:Rails, OpenStruct.new(cache: nil)) do + assert_raises(Rack::Attack::MissingStoreError) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + end end end end - end - it "works when Rails.cache is set" do - Object.stub_const(:Rails, OpenStruct.new(cache: ActiveSupport::Cache::MemoryStore.new)) do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + it "works when Rails.cache is set" do + Object.stub_const(:Rails, OpenStruct.new(cache: ActiveSupport::Cache::MemoryStore.new)) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 200, last_response.status + assert_equal 200, last_response.status - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 429, last_response.status + assert_equal 429, last_response.status + end end end end diff --git a/spec/acceptance/customizing_throttled_response_spec.rb b/spec/acceptance/customizing_throttled_response_spec.rb index 0990975e..742d91e8 100644 --- a/spec/acceptance/customizing_throttled_response_spec.rb +++ b/spec/acceptance/customizing_throttled_response_spec.rb @@ -2,81 +2,83 @@ require_relative "../spec_helper" -describe "Customizing throttled response" do - before do - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new +if defined?(::ActiveSupport::Cache::MemoryStore) + describe "Customizing throttled response" do + before do + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| - request.ip + Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| + request.ip + end end - end - it "can be customized" do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + it "can be customized" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 200, last_response.status + assert_equal 200, last_response.status - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 429, last_response.status + assert_equal 429, last_response.status - Rack::Attack.throttled_responder = lambda do |_req| - [503, {}, ["Throttled"]] - end + Rack::Attack.throttled_responder = lambda do |_req| + [503, {}, ["Throttled"]] + end - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 503, last_response.status - assert_equal "Throttled", last_response.body - end + assert_equal 503, last_response.status + assert_equal "Throttled", last_response.body + end - it "exposes match data" do - matched = nil - match_type = nil - match_data = nil - match_discriminator = nil + it "exposes match data" do + matched = nil + match_type = nil + match_data = nil + match_discriminator = nil - Rack::Attack.throttled_responder = lambda do |req| - matched = req.env['rack.attack.matched'] - match_type = req.env['rack.attack.match_type'] - match_data = req.env['rack.attack.match_data'] - match_discriminator = req.env['rack.attack.match_discriminator'] + Rack::Attack.throttled_responder = lambda do |req| + matched = req.env['rack.attack.matched'] + match_type = req.env['rack.attack.match_type'] + match_data = req.env['rack.attack.match_data'] + match_discriminator = req.env['rack.attack.match_discriminator'] - [429, {}, ["Throttled"]] - end + [429, {}, ["Throttled"]] + end - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal "by ip", matched - assert_equal :throttle, match_type - assert_equal 60, match_data[:period] - assert_equal 1, match_data[:limit] - assert_equal 2, match_data[:count] - assert_equal "1.2.3.4", match_discriminator + assert_equal "by ip", matched + assert_equal :throttle, match_type + assert_equal 60, match_data[:period] + assert_equal 1, match_data[:limit] + assert_equal 2, match_data[:count] + assert_equal "1.2.3.4", match_discriminator - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 3, match_data[:count] - end + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + assert_equal 3, match_data[:count] + end - it "supports old style" do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + it "supports old style" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 200, last_response.status + assert_equal 200, last_response.status - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 429, last_response.status + assert_equal 429, last_response.status - silence_warnings do - Rack::Attack.throttled_response = lambda do |_req| - [503, {}, ["Throttled"]] + silence_warnings do + Rack::Attack.throttled_response = lambda do |_req| + [503, {}, ["Throttled"]] + end end - end - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 503, last_response.status - assert_equal "Throttled", last_response.body + assert_equal 503, last_response.status + assert_equal "Throttled", last_response.body + end end end diff --git a/spec/acceptance/fail2ban_spec.rb b/spec/acceptance/fail2ban_spec.rb index 74c01f57..d5d48e57 100644 --- a/spec/acceptance/fail2ban_spec.rb +++ b/spec/acceptance/fail2ban_spec.rb @@ -3,118 +3,120 @@ require_relative "../spec_helper" require "timecop" -describe "fail2ban" do - let(:notifications) { [] } +if defined?(::ActiveSupport::Cache::MemoryStore) + describe "fail2ban" do + let(:notifications) { [] } - before do - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + before do + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - Rack::Attack.blocklist("fail2ban pentesters") do |request| - Rack::Attack::Fail2Ban.filter(request.ip, maxretry: 2, findtime: 30, bantime: 60) do - request.path.include?("private-place") + Rack::Attack.blocklist("fail2ban pentesters") do |request| + Rack::Attack::Fail2Ban.filter(request.ip, maxretry: 2, findtime: 30, bantime: 60) do + request.path.include?("private-place") + end end end - end - - it "returns OK for many requests to non filtered path" do - get "/" - assert_equal 200, last_response.status - get "/" - assert_equal 200, last_response.status - end + it "returns OK for many requests to non filtered path" do + get "/" + assert_equal 200, last_response.status - it "forbids access to private path" do - get "/private-place" - assert_equal 403, last_response.status - end + get "/" + assert_equal 200, last_response.status + end - it "returns OK for non filtered path if yet not reached maxretry limit" do - get "/private-place" - assert_equal 403, last_response.status + it "forbids access to private path" do + get "/private-place" + assert_equal 403, last_response.status + end - get "/" - assert_equal 200, last_response.status - end + it "returns OK for non filtered path if yet not reached maxretry limit" do + get "/private-place" + assert_equal 403, last_response.status - it "forbids all access after reaching maxretry limit" do - get "/private-place" - assert_equal 403, last_response.status + get "/" + assert_equal 200, last_response.status + end - get "/private-place" - assert_equal 403, last_response.status + it "forbids all access after reaching maxretry limit" do + get "/private-place" + assert_equal 403, last_response.status - get "/" - assert_equal 403, last_response.status - end + get "/private-place" + assert_equal 403, last_response.status - it "restores access after bantime elapsed" do - get "/private-place" - assert_equal 403, last_response.status + get "/" + assert_equal 403, last_response.status + end - get "/private-place" - assert_equal 403, last_response.status + it "restores access after bantime elapsed" do + get "/private-place" + assert_equal 403, last_response.status - get "/" - assert_equal 403, last_response.status + get "/private-place" + assert_equal 403, last_response.status - Timecop.travel(60) do get "/" + assert_equal 403, last_response.status - assert_equal 200, last_response.status - end - end + Timecop.travel(60) do + get "/" - it "does not forbid all access if maxrety condition is met but not within the findtime timespan" do - get "/private-place" - assert_equal 403, last_response.status + assert_equal 200, last_response.status + end + end - Timecop.travel(31) do + it "does not forbid all access if maxrety condition is met but not within the findtime timespan" do get "/private-place" assert_equal 403, last_response.status - get "/" - assert_equal 200, last_response.status - end - end + Timecop.travel(31) do + get "/private-place" + assert_equal 403, last_response.status - it "notifies when the request is blocked" do - ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) + get "/" + assert_equal 200, last_response.status + end end - get "/" + it "notifies when the request is blocked" do + ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end - assert_equal 200, last_response.status - assert notifications.empty? + get "/" - get "/private-place" + assert_equal 200, last_response.status + assert notifications.empty? - assert_equal 403, last_response.status - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal 'fail2ban pentesters', notification[:request].env["rack.attack.matched"] - assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] + get "/private-place" - get "/" + assert_equal 403, last_response.status + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal 'fail2ban pentesters', notification[:request].env["rack.attack.matched"] + assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] - assert_equal 200, last_response.status - assert notifications.empty? + get "/" - get "/private-place" + assert_equal 200, last_response.status + assert notifications.empty? - assert_equal 403, last_response.status - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal 'fail2ban pentesters', notification[:request].env["rack.attack.matched"] - assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] + get "/private-place" + + assert_equal 403, last_response.status + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal 'fail2ban pentesters', notification[:request].env["rack.attack.matched"] + assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] - get "/" + get "/" - assert_equal 403, last_response.status - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal 'fail2ban pentesters', notification[:request].env["rack.attack.matched"] - assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] + assert_equal 403, last_response.status + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal 'fail2ban pentesters', notification[:request].env["rack.attack.matched"] + assert_equal :blocklist, notification[:request].env["rack.attack.match_type"] + end end end diff --git a/spec/acceptance/safelisting_ip_spec.rb b/spec/acceptance/safelisting_ip_spec.rb index e04ca6e5..d0efcef8 100644 --- a/spec/acceptance/safelisting_ip_spec.rb +++ b/spec/acceptance/safelisting_ip_spec.rb @@ -2,57 +2,59 @@ require_relative "../spec_helper" -describe "Safelist an IP" do - let(:notifications) { [] } +if defined?(::ActiveSupport::Notifications) + describe "Safelist an IP" do + let(:notifications) { [] } - before do - Rack::Attack.blocklist("admin") do |request| - request.path == "/admin" - end + before do + Rack::Attack.blocklist("admin") do |request| + request.path == "/admin" + end - Rack::Attack.safelist_ip("5.6.7.8") - end + Rack::Attack.safelist_ip("5.6.7.8") + end - it "forbids request if blocklist condition is true and safelist is false" do - get "/admin", {}, "REMOTE_ADDR" => "1.2.3.4" + it "forbids request if blocklist condition is true and safelist is false" do + get "/admin", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 403, last_response.status - end + 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" => "" + 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 + 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" + it "succeeds if blocklist condition is false and safelist is false" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 200, last_response.status - end + assert_equal 200, last_response.status + end - it "succeeds request if blocklist condition is false and safelist is true" do - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + it "succeeds request if blocklist condition is false and safelist is true" do + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - assert_equal 200, last_response.status - end + assert_equal 200, last_response.status + end - it "succeeds request if both blocklist and safelist conditions are true" do - get "/admin", {}, "REMOTE_ADDR" => "5.6.7.8" + it "succeeds request if both blocklist and safelist conditions are true" do + get "/admin", {}, "REMOTE_ADDR" => "5.6.7.8" - assert_equal 200, last_response.status - end - - it "notifies when the request is safe" do - ActiveSupport::Notifications.subscribe("safelist.rack_attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) + assert_equal 200, last_response.status end - get "/admin", {}, "REMOTE_ADDR" => "5.6.7.8" + it "notifies when the request is safe" do + ActiveSupport::Notifications.subscribe("safelist.rack_attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end + + get "/admin", {}, "REMOTE_ADDR" => "5.6.7.8" - assert_equal 200, last_response.status - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal :safelist, notification[:request].env["rack.attack.match_type"] + assert_equal 200, last_response.status + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal :safelist, notification[:request].env["rack.attack.match_type"] + end end end diff --git a/spec/acceptance/safelisting_spec.rb b/spec/acceptance/safelisting_spec.rb index f00fada7..86d5c792 100644 --- a/spec/acceptance/safelisting_spec.rb +++ b/spec/acceptance/safelisting_spec.rb @@ -2,106 +2,108 @@ require_relative "../spec_helper" -describe "#safelist" do - let(:notifications) { [] } - - before do - Rack::Attack.blocklist do |request| - request.ip == "1.2.3.4" +if defined?(::ActiveSupport::Notifications) + describe "#safelist" do + let(:notifications) { [] } + + before do + Rack::Attack.blocklist do |request| + request.ip == "1.2.3.4" + end + + Rack::Attack.safelist do |request| + request.path == "/safe_space" + end end - Rack::Attack.safelist do |request| - request.path == "/safe_space" + it "forbids request if blocklist condition is true and safelist is false" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + + assert_equal 403, last_response.status end - end - it "forbids request if blocklist condition is true and safelist is false" do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + it "succeeds if blocklist condition is false and safelist is false" do + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - assert_equal 403, last_response.status - end + assert_equal 200, last_response.status + end - it "succeeds if blocklist condition is false and safelist is false" do - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + it "succeeds request if blocklist condition is false and safelist is true" do + get "/safe_space", {}, "REMOTE_ADDR" => "5.6.7.8" - assert_equal 200, last_response.status - end + assert_equal 200, last_response.status + end - it "succeeds request if blocklist condition is false and safelist is true" do - get "/safe_space", {}, "REMOTE_ADDR" => "5.6.7.8" + it "succeeds request if both blocklist and safelist conditions are true" do + get "/safe_space", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 200, last_response.status - end + assert_equal 200, last_response.status + end - it "succeeds request if both blocklist and safelist conditions are true" do - get "/safe_space", {}, "REMOTE_ADDR" => "1.2.3.4" + it "notifies when the request is safe" do + ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end - assert_equal 200, last_response.status - end + get "/safe_space", {}, "REMOTE_ADDR" => "1.2.3.4" - it "notifies when the request is safe" do - ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) + assert_equal 200, last_response.status + assert_equal 1, notifications.size + notification = notifications.pop + assert_nil notification[:request].env["rack.attack.matched"] + assert_equal :safelist, notification[:request].env["rack.attack.match_type"] end - - get "/safe_space", {}, "REMOTE_ADDR" => "1.2.3.4" - - assert_equal 200, last_response.status - assert_equal 1, notifications.size - notification = notifications.pop - assert_nil notification[:request].env["rack.attack.matched"] - assert_equal :safelist, notification[:request].env["rack.attack.match_type"] end -end -describe "#safelist with name" do - let(:notifications) { [] } + describe "#safelist with name" do + let(:notifications) { [] } - before do - Rack::Attack.blocklist("block 1.2.3.4") do |request| - request.ip == "1.2.3.4" - end + before do + Rack::Attack.blocklist("block 1.2.3.4") do |request| + request.ip == "1.2.3.4" + end - Rack::Attack.safelist("safe path") do |request| - request.path == "/safe_space" + Rack::Attack.safelist("safe path") do |request| + request.path == "/safe_space" + end end - end - - it "forbids request if blocklist condition is true and safelist is false" do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 403, last_response.status - end + it "forbids request if blocklist condition is true and safelist is false" do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - it "succeeds if blocklist condition is false and safelist is false" do - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + assert_equal 403, last_response.status + end - assert_equal 200, last_response.status - end + it "succeeds if blocklist condition is false and safelist is false" do + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - it "succeeds request if blocklist condition is false and safelist is true" do - get "/safe_space", {}, "REMOTE_ADDR" => "5.6.7.8" + assert_equal 200, last_response.status + end - assert_equal 200, last_response.status - end + it "succeeds request if blocklist condition is false and safelist is true" do + get "/safe_space", {}, "REMOTE_ADDR" => "5.6.7.8" - it "succeeds request if both blocklist and safelist conditions are true" do - get "/safe_space", {}, "REMOTE_ADDR" => "1.2.3.4" + assert_equal 200, last_response.status + end - assert_equal 200, last_response.status - end + it "succeeds request if both blocklist and safelist conditions are true" do + get "/safe_space", {}, "REMOTE_ADDR" => "1.2.3.4" - it "notifies when the request is safe" do - ActiveSupport::Notifications.subscribe("safelist.rack_attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) + assert_equal 200, last_response.status end - get "/safe_space", {}, "REMOTE_ADDR" => "1.2.3.4" + it "notifies when the request is safe" do + ActiveSupport::Notifications.subscribe("safelist.rack_attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end + + get "/safe_space", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 200, last_response.status - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal "safe path", notification[:request].env["rack.attack.matched"] - assert_equal :safelist, notification[:request].env["rack.attack.match_type"] + assert_equal 200, last_response.status + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal "safe path", notification[:request].env["rack.attack.matched"] + assert_equal :safelist, notification[:request].env["rack.attack.match_type"] + end end end diff --git a/spec/acceptance/safelisting_subnet_spec.rb b/spec/acceptance/safelisting_subnet_spec.rb index baeb7e46..6fe58b92 100644 --- a/spec/acceptance/safelisting_subnet_spec.rb +++ b/spec/acceptance/safelisting_subnet_spec.rb @@ -2,51 +2,53 @@ require_relative "../spec_helper" -describe "Safelisting an IP subnet" do - let(:notifications) { [] } +if defined?(::ActiveSupport::Notifications) + describe "Safelisting an IP subnet" do + let(:notifications) { [] } - before do - Rack::Attack.blocklist("admin") do |request| - request.path == "/admin" - end + before do + Rack::Attack.blocklist("admin") do |request| + request.path == "/admin" + end - Rack::Attack.safelist_ip("5.6.0.0/16") - end + Rack::Attack.safelist_ip("5.6.0.0/16") + end - it "forbids request if blocklist condition is true and safelist is false" do - get "/admin", {}, "REMOTE_ADDR" => "5.7.0.0" + it "forbids request if blocklist condition is true and safelist is false" do + get "/admin", {}, "REMOTE_ADDR" => "5.7.0.0" - assert_equal 403, last_response.status - end - - it "succeeds if blocklist condition is false and safelist is false" do - get "/", {}, "REMOTE_ADDR" => "5.7.0.0" + assert_equal 403, last_response.status + end - assert_equal 200, last_response.status - end + it "succeeds if blocklist condition is false and safelist is false" do + get "/", {}, "REMOTE_ADDR" => "5.7.0.0" - it "succeeds request if blocklist condition is false and safelist is true" do - get "/", {}, "REMOTE_ADDR" => "5.6.0.0" + assert_equal 200, last_response.status + end - assert_equal 200, last_response.status - end + it "succeeds request if blocklist condition is false and safelist is true" do + get "/", {}, "REMOTE_ADDR" => "5.6.0.0" - it "succeeds request if both blocklist and safelist conditions are true" do - get "/admin", {}, "REMOTE_ADDR" => "5.6.255.255" + assert_equal 200, last_response.status + end - assert_equal 200, last_response.status - end + it "succeeds request if both blocklist and safelist conditions are true" do + get "/admin", {}, "REMOTE_ADDR" => "5.6.255.255" - it "notifies when the request is safe" do - ActiveSupport::Notifications.subscribe("safelist.rack_attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) + assert_equal 200, last_response.status end - get "/admin", {}, "REMOTE_ADDR" => "5.6.0.0" + it "notifies when the request is safe" do + ActiveSupport::Notifications.subscribe("safelist.rack_attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end + + get "/admin", {}, "REMOTE_ADDR" => "5.6.0.0" - assert_equal 200, last_response.status - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal :safelist, notification[:request].env["rack.attack.match_type"] + assert_equal 200, last_response.status + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal :safelist, notification[:request].env["rack.attack.match_type"] + end end end diff --git a/spec/acceptance/stores/active_support_mem_cache_store_spec.rb b/spec/acceptance/stores/active_support_mem_cache_store_spec.rb index ccfe3db7..f997efc6 100644 --- a/spec/acceptance/stores/active_support_mem_cache_store_spec.rb +++ b/spec/acceptance/stores/active_support_mem_cache_store_spec.rb @@ -2,7 +2,7 @@ require_relative "../../spec_helper" -if defined?(::Dalli) +if defined?(::Dalli) && defined?(::ActiveSupport::Cache::MemCacheStore) require_relative "../../support/cache_store_helper" describe "ActiveSupport::Cache::MemCacheStore as a cache backend" do diff --git a/spec/acceptance/stores/active_support_memory_store_spec.rb b/spec/acceptance/stores/active_support_memory_store_spec.rb index 4ed81e7f..4a219b82 100644 --- a/spec/acceptance/stores/active_support_memory_store_spec.rb +++ b/spec/acceptance/stores/active_support_memory_store_spec.rb @@ -1,16 +1,19 @@ # frozen_string_literal: true require_relative "../../spec_helper" -require_relative "../../support/cache_store_helper" -describe "ActiveSupport::Cache::MemoryStore as a cache backend" do - before do - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - end +if defined?(::ActiveSupport::Cache::MemoryStore) + require_relative "../../support/cache_store_helper" - after do - Rack::Attack.cache.store.clear - end + describe "ActiveSupport::Cache::MemoryStore as a cache backend" do + before do + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + end - it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.fetch(key) }) + after do + Rack::Attack.cache.store.clear + end + + it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.fetch(key) }) + end end diff --git a/spec/acceptance/throttling_spec.rb b/spec/acceptance/throttling_spec.rb index ca81d1c5..524850a1 100644 --- a/spec/acceptance/throttling_spec.rb +++ b/spec/acceptance/throttling_spec.rb @@ -3,125 +3,82 @@ require_relative "../spec_helper" require "timecop" -describe "#throttle" do - let(:notifications) { [] } +if defined?(::ActiveSupport::Cache::MemoryStore) + describe "#throttle" do + let(:notifications) { [] } - before do - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - end - - it "allows one request per minute by IP" do - Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| - request.ip + before do + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new end - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - - assert_equal 200, last_response.status - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - - assert_equal 429, last_response.status - assert_nil last_response.headers["Retry-After"] - assert_equal "Retry later\n", last_response.body - - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - - assert_equal 200, last_response.status + it "allows one request per minute by IP" do + Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| + request.ip + end - Timecop.travel(60) do get "/", {}, "REMOTE_ADDR" => "1.2.3.4" assert_equal 200, last_response.status - end - end - it "returns correct Retry-After header if enabled" do - Rack::Attack.throttled_response_retry_after_header = true + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| - request.ip - end + assert_equal 429, last_response.status + assert_nil last_response.headers["Retry-After"] + assert_equal "Retry later\n", last_response.body + + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - Timecop.freeze(Time.at(0)) do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" assert_equal 200, last_response.status - end - Timecop.freeze(Time.at(25)) do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal "35", last_response.headers["Retry-After"] - end - end + Timecop.travel(60) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - it "supports limit to be dynamic" do - # Could be used to have different rate limits for authorized - # vs general requests - limit_proc = lambda do |request| - if request.env["X-APIKey"] == "private-secret" - 2 - else - 1 + assert_equal 200, last_response.status end end - Rack::Attack.throttle("by ip", limit: limit_proc, period: 60) do |request| - request.ip - end - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 200, last_response.status + it "returns correct Retry-After header if enabled" do + Rack::Attack.throttled_response_retry_after_header = true - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 429, last_response.status - - get "/", {}, "REMOTE_ADDR" => "5.6.7.8", "X-APIKey" => "private-secret" - assert_equal 200, last_response.status - - get "/", {}, "REMOTE_ADDR" => "5.6.7.8", "X-APIKey" => "private-secret" - assert_equal 200, last_response.status + Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| + request.ip + end - get "/", {}, "REMOTE_ADDR" => "5.6.7.8", "X-APIKey" => "private-secret" - assert_equal 429, last_response.status - end + Timecop.freeze(Time.at(0)) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + assert_equal 200, last_response.status + end - it "supports period to be dynamic" do - # Could be used to have different rate limits for authorized - # vs general requests - period_proc = lambda do |request| - if request.env["X-APIKey"] == "private-secret" - 10 - else - 30 + Timecop.freeze(Time.at(25)) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + assert_equal "35", last_response.headers["Retry-After"] end end - Rack::Attack.throttle("by ip", limit: 1, period: period_proc) do |request| - request.ip - end + it "supports limit to be dynamic" do + # Could be used to have different rate limits for authorized + # vs general requests + limit_proc = lambda do |request| + if request.env["X-APIKey"] == "private-secret" + 2 + else + 1 + end + end - # Using Time#at to align to start/end of periods exactly - # to achieve consistenty in different test runs + Rack::Attack.throttle("by ip", limit: limit_proc, period: 60) do |request| + request.ip + end - Timecop.travel(Time.at(0)) do get "/", {}, "REMOTE_ADDR" => "1.2.3.4" assert_equal 200, last_response.status get "/", {}, "REMOTE_ADDR" => "1.2.3.4" assert_equal 429, last_response.status - end - - Timecop.travel(Time.at(10)) do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 429, last_response.status - end - Timecop.travel(Time.at(30)) do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "5.6.7.8", "X-APIKey" => "private-secret" assert_equal 200, last_response.status - end - Timecop.travel(Time.at(0)) do get "/", {}, "REMOTE_ADDR" => "5.6.7.8", "X-APIKey" => "private-secret" assert_equal 200, last_response.status @@ -129,42 +86,87 @@ assert_equal 429, last_response.status end - Timecop.travel(Time.at(10)) do - get "/", {}, "REMOTE_ADDR" => "5.6.7.8", "X-APIKey" => "private-secret" - assert_equal 200, last_response.status - end - end + it "supports period to be dynamic" do + # Could be used to have different rate limits for authorized + # vs general requests + period_proc = lambda do |request| + if request.env["X-APIKey"] == "private-secret" + 10 + else + 30 + end + end - it "notifies when the request is throttled" do - Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| - request.ip - end + Rack::Attack.throttle("by ip", limit: 1, period: period_proc) do |request| + request.ip + end + + # Using Time#at to align to start/end of periods exactly + # to achieve consistenty in different test runs + + Timecop.travel(Time.at(0)) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + assert_equal 200, last_response.status + + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + assert_equal 429, last_response.status + end + + Timecop.travel(Time.at(10)) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + assert_equal 429, last_response.status + end - ActiveSupport::Notifications.subscribe("throttle.rack_attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) + Timecop.travel(Time.at(30)) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + assert_equal 200, last_response.status + end + + Timecop.travel(Time.at(0)) do + get "/", {}, "REMOTE_ADDR" => "5.6.7.8", "X-APIKey" => "private-secret" + assert_equal 200, last_response.status + + get "/", {}, "REMOTE_ADDR" => "5.6.7.8", "X-APIKey" => "private-secret" + assert_equal 429, last_response.status + end + + Timecop.travel(Time.at(10)) do + get "/", {}, "REMOTE_ADDR" => "5.6.7.8", "X-APIKey" => "private-secret" + assert_equal 200, last_response.status + end end - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + it "notifies when the request is throttled" do + Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| + request.ip + end + + ActiveSupport::Notifications.subscribe("throttle.rack_attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end + + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - assert_equal 200, last_response.status - assert notifications.empty? + assert_equal 200, last_response.status + assert notifications.empty? - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 200, last_response.status - assert notifications.empty? + assert_equal 200, last_response.status + assert notifications.empty? - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 429, last_response.status + assert_equal 429, last_response.status - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal "by ip", notification[:request].env["rack.attack.matched"] - assert_equal :throttle, notification[:request].env["rack.attack.match_type"] - assert_equal 60, notification[:request].env["rack.attack.match_data"][:period] - assert_equal 1, notification[:request].env["rack.attack.match_data"][:limit] - assert_equal 2, notification[:request].env["rack.attack.match_data"][:count] - assert_equal "1.2.3.4", notification[:request].env["rack.attack.match_discriminator"] + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal "by ip", notification[:request].env["rack.attack.matched"] + assert_equal :throttle, notification[:request].env["rack.attack.match_type"] + assert_equal 60, notification[:request].env["rack.attack.match_data"][:period] + assert_equal 1, notification[:request].env["rack.attack.match_data"][:limit] + assert_equal 2, notification[:request].env["rack.attack.match_data"][:count] + assert_equal "1.2.3.4", notification[:request].env["rack.attack.match_discriminator"] + end end end diff --git a/spec/acceptance/track_spec.rb b/spec/acceptance/track_spec.rb index 62d1551a..b2549fd1 100644 --- a/spec/acceptance/track_spec.rb +++ b/spec/acceptance/track_spec.rb @@ -2,27 +2,29 @@ require_relative "../spec_helper" -describe "#track" do - let(:notifications) { [] } +if defined?(::ActiveSupport::Notifications) + describe "#track" do + let(:notifications) { [] } - it "notifies when track block returns true" do - Rack::Attack.track("ip 1.2.3.4") do |request| - request.ip == "1.2.3.4" - end + it "notifies when track block returns true" do + Rack::Attack.track("ip 1.2.3.4") do |request| + request.ip == "1.2.3.4" + end - ActiveSupport::Notifications.subscribe("track.rack_attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) - end + ActiveSupport::Notifications.subscribe("track.rack_attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - assert notifications.empty? + assert notifications.empty? - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal "ip 1.2.3.4", notification[:request].env["rack.attack.matched"] - assert_equal :track, notification[:request].env["rack.attack.match_type"] + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal "ip 1.2.3.4", notification[:request].env["rack.attack.matched"] + assert_equal :track, notification[:request].env["rack.attack.match_type"] + end end end diff --git a/spec/acceptance/track_throttle_spec.rb b/spec/acceptance/track_throttle_spec.rb index e6aa553a..5c2d9149 100644 --- a/spec/acceptance/track_throttle_spec.rb +++ b/spec/acceptance/track_throttle_spec.rb @@ -3,47 +3,49 @@ require_relative "../spec_helper" require "timecop" -describe "#track with throttle-ish options" do - let(:notifications) { [] } +if defined?(::ActiveSupport::Cache::MemoryStore) + describe "#track with throttle-ish options" do + let(:notifications) { [] } - it "notifies when throttle goes over the limit without actually throttling requests" do - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + it "notifies when throttle goes over the limit without actually throttling requests" do + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - Rack::Attack.track("by ip", limit: 1, period: 60) do |request| - request.ip - end + Rack::Attack.track("by ip", limit: 1, period: 60) do |request| + request.ip + end - ActiveSupport::Notifications.subscribe("track.rack_attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) - end + ActiveSupport::Notifications.subscribe("track.rack_attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert notifications.empty? + assert notifications.empty? - assert_equal 200, last_response.status + assert_equal 200, last_response.status - get "/", {}, "REMOTE_ADDR" => "5.6.7.8" + get "/", {}, "REMOTE_ADDR" => "5.6.7.8" - assert notifications.empty? + assert notifications.empty? - assert_equal 200, last_response.status + assert_equal 200, last_response.status - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert_equal 1, notifications.size - notification = notifications.pop - assert_equal "by ip", notification[:request].env["rack.attack.matched"] - assert_equal :track, notification[:request].env["rack.attack.match_type"] + assert_equal 1, notifications.size + notification = notifications.pop + assert_equal "by ip", notification[:request].env["rack.attack.matched"] + assert_equal :track, notification[:request].env["rack.attack.match_type"] - assert_equal 200, last_response.status + assert_equal 200, last_response.status - Timecop.travel(60) do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + Timecop.travel(60) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - assert notifications.empty? + assert notifications.empty? - assert_equal 200, last_response.status + assert_equal 200, last_response.status + end end end end diff --git a/spec/allow2ban_spec.rb b/spec/allow2ban_spec.rb index 105e0ad0..8b8f4fcb 100644 --- a/spec/allow2ban_spec.rb +++ b/spec/allow2ban_spec.rb @@ -2,125 +2,127 @@ require_relative 'spec_helper' -describe 'Rack::Attack.Allow2Ban' do - before do - # Use a long findtime; failures due to cache key rotation less likely - @cache = Rack::Attack.cache - @findtime = 60 - @bantime = 60 - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - @f2b_options = { bantime: @bantime, findtime: @findtime, maxretry: 2 } - - Rack::Attack.blocklist('pentest') do |req| - Rack::Attack::Allow2Ban.filter(req.ip, @f2b_options) { req.query_string =~ /OMGHAX/ } - end - end - - describe 'discriminator has not been banned' do - describe 'making ok request' do - it 'succeeds' do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - - _(last_response.status).must_equal 200 +if defined?(::ActiveSupport::Cache::MemoryStore) + describe 'Rack::Attack.Allow2Ban' do + before do + # Use a long findtime; failures due to cache key rotation less likely + @cache = Rack::Attack.cache + @findtime = 60 + @bantime = 60 + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + @f2b_options = { bantime: @bantime, findtime: @findtime, maxretry: 2 } + + Rack::Attack.blocklist('pentest') do |req| + Rack::Attack::Allow2Ban.filter(req.ip, @f2b_options) { req.query_string =~ /OMGHAX/ } end end - describe 'making qualifying request' do - describe 'when not at maxretry' do - before { get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' } - + describe 'discriminator has not been banned' do + describe 'making ok request' do it 'succeeds' do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + _(last_response.status).must_equal 200 end + end - it 'increases fail count' do - key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4" + describe 'making qualifying request' do + describe 'when not at maxretry' do + before { get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' } - _(@cache.store.read(key)).must_equal 1 + it 'succeeds' do + _(last_response.status).must_equal 200 + end + + it 'increases fail count' do + key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4" + + _(@cache.store.read(key)).must_equal 1 + end + + it 'is not banned' do + key = "rack::attack:allow2ban:1.2.3.4" + _(@cache.store.read(key)).must_be_nil + end end - it 'is not banned' do - key = "rack::attack:allow2ban:1.2.3.4" - _(@cache.store.read(key)).must_be_nil + describe 'when at maxretry' do + before do + # maxretry is 2 - so hit with an extra failed request first + get '/?test=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + end + + it 'succeeds' do + _(last_response.status).must_equal 200 + end + + it 'increases fail count' do + key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4" + _(@cache.store.read(key)).must_equal 2 + end + + it 'is banned' do + key = "rack::attack:allow2ban:ban:1.2.3.4" + _(@cache.store.read(key)).must_equal 1 + end end end + end - describe 'when at maxretry' do - before do - # maxretry is 2 - so hit with an extra failed request first - get '/?test=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' - get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' - end + describe 'discriminator has been banned' do + before do + # maxretry is 2 - so hit enough times to get banned + get '/?test=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + end + describe 'making request for other discriminator' do it 'succeeds' do + get '/', {}, 'REMOTE_ADDR' => '2.2.3.4' + _(last_response.status).must_equal 200 end + end - it 'increases fail count' do + describe 'making ok request' do + before do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + end + + it 'fails' do + _(last_response.status).must_equal 403 + end + + it 'does not increase fail count' do key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4" _(@cache.store.read(key)).must_equal 2 end - it 'is banned' do + it 'is still banned' do key = "rack::attack:allow2ban:ban:1.2.3.4" _(@cache.store.read(key)).must_equal 1 end end - end - end - describe 'discriminator has been banned' do - before do - # maxretry is 2 - so hit enough times to get banned - get '/?test=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' - get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' - end - - describe 'making request for other discriminator' do - it 'succeeds' do - get '/', {}, 'REMOTE_ADDR' => '2.2.3.4' - - _(last_response.status).must_equal 200 - end - end - - describe 'making ok request' do - before do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - end - - it 'fails' do - _(last_response.status).must_equal 403 - end - - it 'does not increase fail count' do - key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4" - _(@cache.store.read(key)).must_equal 2 - end - - it 'is still banned' do - key = "rack::attack:allow2ban:ban:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 - end - end - - describe 'making failing request' do - before do - get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' - end + describe 'making failing request' do + before do + get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + end - it 'fails' do - _(last_response.status).must_equal 403 - end + it 'fails' do + _(last_response.status).must_equal 403 + end - it 'does not increase fail count' do - key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4" - _(@cache.store.read(key)).must_equal 2 - end + it 'does not increase fail count' do + key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4" + _(@cache.store.read(key)).must_equal 2 + end - it 'is still banned' do - key = "rack::attack:allow2ban:ban:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 + it 'is still banned' do + key = "rack::attack:allow2ban:ban:1.2.3.4" + _(@cache.store.read(key)).must_equal 1 + end end end end diff --git a/spec/fail2ban_spec.rb b/spec/fail2ban_spec.rb index 6a9d9bcf..009ed8e7 100644 --- a/spec/fail2ban_spec.rb +++ b/spec/fail2ban_spec.rb @@ -2,145 +2,147 @@ require_relative 'spec_helper' -describe 'Rack::Attack.Fail2Ban' do - before do - # Use a long findtime; failures due to cache key rotation less likely - @cache = Rack::Attack.cache - @findtime = 60 - @bantime = 60 - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - @f2b_options = { bantime: @bantime, findtime: @findtime, maxretry: 2 } - - Rack::Attack.blocklist('pentest') do |req| - Rack::Attack::Fail2Ban.filter(req.ip, @f2b_options) { req.query_string =~ /OMGHAX/ } +if defined?(::ActiveSupport::Cache::MemoryStore) + describe 'Rack::Attack.Fail2Ban' do + before do + # Use a long findtime; failures due to cache key rotation less likely + @cache = Rack::Attack.cache + @findtime = 60 + @bantime = 60 + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + @f2b_options = { bantime: @bantime, findtime: @findtime, maxretry: 2 } + + Rack::Attack.blocklist('pentest') do |req| + Rack::Attack::Fail2Ban.filter(req.ip, @f2b_options) { req.query_string =~ /OMGHAX/ } + end end - end - describe 'discriminator has not been banned' do - describe 'making ok request' do - it 'succeeds' do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - _(last_response.status).must_equal 200 + describe 'discriminator has not been banned' do + describe 'making ok request' do + it 'succeeds' do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + _(last_response.status).must_equal 200 + end end - end - describe 'making failing request' do - describe 'when not at maxretry' do - before { get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' } + describe 'making failing request' do + describe 'when not at maxretry' do + before { get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' } - it 'fails' do - _(last_response.status).must_equal 403 + it 'fails' do + _(last_response.status).must_equal 403 + end + + it 'increases fail count' do + key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4" + _(@cache.store.read(key)).must_equal 1 + end + + it 'is not banned' do + key = "rack::attack:fail2ban:1.2.3.4" + _(@cache.store.read(key)).must_be_nil + end end - it 'increases fail count' do - key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 + describe 'when at maxretry' do + before do + # maxretry is 2 - so hit with an extra failed request first + get '/?test=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + end + + it 'fails' do + _(last_response.status).must_equal 403 + end + + it 'increases fail count' do + key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4" + _(@cache.store.read(key)).must_equal 2 + end + + it 'is banned' do + key = "rack::attack:fail2ban:ban:1.2.3.4" + _(@cache.store.read(key)).must_equal 1 + end end - it 'is not banned' do - key = "rack::attack:fail2ban:1.2.3.4" - _(@cache.store.read(key)).must_be_nil + describe 'reset after success' do + before do + get '/?test=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + Rack::Attack::Fail2Ban.reset('1.2.3.4', @f2b_options) + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + end + + it 'succeeds' do + _(last_response.status).must_equal 200 + end + + it 'resets fail count' do + key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4" + assert_nil @cache.store.read(key) + end + + it 'IP is not banned' do + _(Rack::Attack::Fail2Ban.banned?('1.2.3.4')).must_equal false + end end end + end - describe 'when at maxretry' do + describe 'discriminator has been banned' do + before do + # maxretry is 2 - so hit enough times to get banned + get '/?test=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + end + + describe 'making request for other discriminator' do + it 'succeeds' do + get '/', {}, 'REMOTE_ADDR' => '2.2.3.4' + + _(last_response.status).must_equal 200 + end + end + + describe 'making ok request' do before do - # maxretry is 2 - so hit with an extra failed request first - get '/?test=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' - get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' end it 'fails' do _(last_response.status).must_equal 403 end - it 'increases fail count' do + it 'does not increase fail count' do key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4" _(@cache.store.read(key)).must_equal 2 end - it 'is banned' do + it 'is still banned' do key = "rack::attack:fail2ban:ban:1.2.3.4" _(@cache.store.read(key)).must_equal 1 end end - describe 'reset after success' do + describe 'making failing request' do before do - get '/?test=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' - Rack::Attack::Fail2Ban.reset('1.2.3.4', @f2b_options) - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' end - it 'succeeds' do - _(last_response.status).must_equal 200 + it 'fails' do + _(last_response.status).must_equal 403 end - it 'resets fail count' do + it 'does not increase fail count' do key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4" - assert_nil @cache.store.read(key) + _(@cache.store.read(key)).must_equal 2 end - it 'IP is not banned' do - _(Rack::Attack::Fail2Ban.banned?('1.2.3.4')).must_equal false + it 'is still banned' do + key = "rack::attack:fail2ban:ban:1.2.3.4" + _(@cache.store.read(key)).must_equal 1 end end end end - - describe 'discriminator has been banned' do - before do - # maxretry is 2 - so hit enough times to get banned - get '/?test=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' - get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' - end - - describe 'making request for other discriminator' do - it 'succeeds' do - get '/', {}, 'REMOTE_ADDR' => '2.2.3.4' - - _(last_response.status).must_equal 200 - end - end - - describe 'making ok request' do - before do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - end - - it 'fails' do - _(last_response.status).must_equal 403 - end - - it 'does not increase fail count' do - key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4" - _(@cache.store.read(key)).must_equal 2 - end - - it 'is still banned' do - key = "rack::attack:fail2ban:ban:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 - end - end - - describe 'making failing request' do - before do - get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' - end - - it 'fails' do - _(last_response.status).must_equal 403 - end - - it 'does not increase fail count' do - key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4" - _(@cache.store.read(key)).must_equal 2 - end - - it 'is still banned' do - key = "rack::attack:fail2ban:ban:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 - end - end - end end diff --git a/spec/integration/offline_spec.rb b/spec/integration/offline_spec.rb index 85429a42..c480d7e8 100644 --- a/spec/integration/offline_spec.rb +++ b/spec/integration/offline_spec.rb @@ -1,8 +1,13 @@ # frozen_string_literal: true -require 'active_support/cache' require_relative '../spec_helper' +begin + require 'active_support/cache' +rescue LoadError + # ActiveSupport is optional +end + OfflineExamples = Minitest::SharedExamples.new do it 'should write' do @cache.write('cache-test-key', 'foobar', 1) diff --git a/spec/rack_attack_instrumentation_spec.rb b/spec/rack_attack_instrumentation_spec.rb index d2291f77..7bf1c114 100644 --- a/spec/rack_attack_instrumentation_spec.rb +++ b/spec/rack_attack_instrumentation_spec.rb @@ -1,39 +1,41 @@ # frozen_string_literal: true require_relative "spec_helper" -require 'active_support' -require 'active_support/subscriber' -class CustomSubscriber < ActiveSupport::Subscriber - @notification_count = 0 +if defined?(::ActiveSupport::Subscriber) + require 'active_support/subscriber' - class << self - attr_accessor :notification_count - end + class CustomSubscriber < ActiveSupport::Subscriber + @notification_count = 0 - def throttle(_event) - self.class.notification_count += 1 - end -end + class << self + attr_accessor :notification_count + end -describe 'Rack::Attack.instrument' do - before do - @period = 60 # Use a long period; failures due to cache key rotation less likely - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |req| req.ip } + def throttle(_event) + self.class.notification_count += 1 + end end - describe "with throttling" do + describe 'Rack::Attack.instrument' do before do - ActiveSupport::Notifications.stub(:notifier, ActiveSupport::Notifications::Fanout.new) do - CustomSubscriber.attach_to("rack_attack") - 2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' } - end + @period = 60 # Use a long period; failures due to cache key rotation less likely + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |req| req.ip } end - it 'should instrument without error' do - _(last_response.status).must_equal 429 - assert_equal 1, CustomSubscriber.notification_count + describe "with throttling" do + before do + ActiveSupport::Notifications.stub(:notifier, ActiveSupport::Notifications::Fanout.new) do + CustomSubscriber.attach_to("rack_attack") + 2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' } + end + end + + it 'should instrument without error' do + _(last_response.status).must_equal 429 + assert_equal 1, CustomSubscriber.notification_count + end end end end diff --git a/spec/rack_attack_throttle_spec.rb b/spec/rack_attack_throttle_spec.rb index 1bf7f32f..5b07d762 100644 --- a/spec/rack_attack_throttle_spec.rb +++ b/spec/rack_attack_throttle_spec.rb @@ -3,209 +3,211 @@ require_relative 'spec_helper' require_relative 'support/freeze_time_helper' -describe 'Rack::Attack.throttle' do - before do - @period = 60 - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |req| req.ip } - end +if defined?(::ActiveSupport::Cache::MemoryStore) + describe 'Rack::Attack.throttle' do + before do + @period = 60 + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |req| req.ip } + end - it('should have a throttle') { Rack::Attack.throttles.key?('ip/sec') } + it('should have a throttle') { Rack::Attack.throttles.key?('ip/sec') } - it_allows_ok_requests + it_allows_ok_requests - describe 'a single request' do - it 'should set the counter for one request' do - within_same_period do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + describe 'a single request' do + it 'should set the counter for one request' do + within_same_period do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4" - _(Rack::Attack.cache.store.read(key)).must_equal 1 + key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4" + _(Rack::Attack.cache.store.read(key)).must_equal 1 + end end - end - it 'should populate throttle data' do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + it 'should populate throttle data' do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - data = { - count: 1, - limit: 1, - period: @period, - epoch_time: Rack::Attack.cache.last_epoch_time.to_i, - discriminator: "1.2.3.4" - } + data = { + count: 1, + limit: 1, + period: @period, + epoch_time: Rack::Attack.cache.last_epoch_time.to_i, + discriminator: "1.2.3.4" + } - _(last_request.env['rack.attack.throttle_data']['ip/sec']).must_equal data + _(last_request.env['rack.attack.throttle_data']['ip/sec']).must_equal data + end end - end - describe "with 2 requests" do - before do - within_same_period do - 2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' } + describe "with 2 requests" do + before do + within_same_period do + 2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' } + end end - end - it 'should block the last request' do - _(last_response.status).must_equal 429 - end + it 'should block the last request' do + _(last_response.status).must_equal 429 + end - it 'should tag the env' do - _(last_request.env['rack.attack.matched']).must_equal 'ip/sec' - _(last_request.env['rack.attack.match_type']).must_equal :throttle + it 'should tag the env' do + _(last_request.env['rack.attack.matched']).must_equal 'ip/sec' + _(last_request.env['rack.attack.match_type']).must_equal :throttle - _(last_request.env['rack.attack.match_data']).must_equal( - count: 2, - limit: 1, - period: @period, - epoch_time: Rack::Attack.cache.last_epoch_time.to_i, - discriminator: "1.2.3.4" - ) + _(last_request.env['rack.attack.match_data']).must_equal( + count: 2, + limit: 1, + period: @period, + epoch_time: Rack::Attack.cache.last_epoch_time.to_i, + discriminator: "1.2.3.4" + ) - _(last_request.env['rack.attack.match_discriminator']).must_equal('1.2.3.4') + _(last_request.env['rack.attack.match_discriminator']).must_equal('1.2.3.4') + end end end -end -describe 'Rack::Attack.throttle with limit as proc' do - before do - @period = 60 - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - Rack::Attack.throttle('ip/sec', limit: lambda { |_req| 1 }, period: @period) { |req| req.ip } - end + describe 'Rack::Attack.throttle with limit as proc' do + before do + @period = 60 + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + Rack::Attack.throttle('ip/sec', limit: lambda { |_req| 1 }, period: @period) { |req| req.ip } + end - it_allows_ok_requests + it_allows_ok_requests - describe 'a single request' do - it 'should set the counter for one request' do - within_same_period do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + describe 'a single request' do + it 'should set the counter for one request' do + within_same_period do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4" - _(Rack::Attack.cache.store.read(key)).must_equal 1 + key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4" + _(Rack::Attack.cache.store.read(key)).must_equal 1 + end end - end - it 'should populate throttle data' do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - data = { - count: 1, - limit: 1, - period: @period, - epoch_time: Rack::Attack.cache.last_epoch_time.to_i, - discriminator: "1.2.3.4" - } - - _(last_request.env['rack.attack.throttle_data']['ip/sec']).must_equal data + it 'should populate throttle data' do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + data = { + count: 1, + limit: 1, + period: @period, + epoch_time: Rack::Attack.cache.last_epoch_time.to_i, + discriminator: "1.2.3.4" + } + + _(last_request.env['rack.attack.throttle_data']['ip/sec']).must_equal data + end end end -end -describe 'Rack::Attack.throttle with period as proc' do - before do - @period = 60 - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - Rack::Attack.throttle('ip/sec', limit: lambda { |_req| 1 }, period: lambda { |_req| @period }) { |req| req.ip } - end + describe 'Rack::Attack.throttle with period as proc' do + before do + @period = 60 + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + Rack::Attack.throttle('ip/sec', limit: lambda { |_req| 1 }, period: lambda { |_req| @period }) { |req| req.ip } + end - it_allows_ok_requests + it_allows_ok_requests - describe 'a single request' do - it 'should set the counter for one request' do - within_same_period do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + describe 'a single request' do + it 'should set the counter for one request' do + within_same_period do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4" - _(Rack::Attack.cache.store.read(key)).must_equal 1 + key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4" + _(Rack::Attack.cache.store.read(key)).must_equal 1 + end end - end - it 'should populate throttle data' do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + it 'should populate throttle data' do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - data = { - count: 1, - limit: 1, - period: @period, - epoch_time: Rack::Attack.cache.last_epoch_time.to_i, - discriminator: "1.2.3.4" - } + data = { + count: 1, + limit: 1, + period: @period, + epoch_time: Rack::Attack.cache.last_epoch_time.to_i, + discriminator: "1.2.3.4" + } - _(last_request.env['rack.attack.throttle_data']['ip/sec']).must_equal data + _(last_request.env['rack.attack.throttle_data']['ip/sec']).must_equal data + end end end -end -describe 'Rack::Attack.throttle with block returning nil' do - before do - @period = 60 - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |_| nil } - end + describe 'Rack::Attack.throttle with block returning nil' do + before do + @period = 60 + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |_| nil } + end - it_allows_ok_requests + it_allows_ok_requests - describe 'a single request' do - it 'should not set the counter' do - within_same_period do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + describe 'a single request' do + it 'should not set the counter' do + within_same_period do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4" - assert_nil Rack::Attack.cache.store.read(key) + key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4" + assert_nil Rack::Attack.cache.store.read(key) + end end - end - it 'should not populate throttle data' do - get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' - assert_nil last_request.env['rack.attack.throttle_data'] - end - end -end - -describe 'Rack::Attack.throttle with throttle_discriminator_normalizer' do - before do - @period = 60 - @emails = [ - "person@example.com", - "PERSON@example.com ", - " person@example.com\r\n ", - ] - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - Rack::Attack.throttle('logins/email', limit: 4, period: @period) do |req| - if req.path == '/login' && req.post? - req.params['email'] + it 'should not populate throttle data' do + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + assert_nil last_request.env['rack.attack.throttle_data'] end end end - it 'should not differentiate requests when throttle_discriminator_normalizer is enabled' do - within_same_period do - post_logins - key = "rack::attack:#{Time.now.to_i / @period}:logins/email:person@example.com" - _(Rack::Attack.cache.store.read(key)).must_equal 3 + describe 'Rack::Attack.throttle with throttle_discriminator_normalizer' do + before do + @period = 60 + @emails = [ + "person@example.com", + "PERSON@example.com ", + " person@example.com\r\n ", + ] + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + Rack::Attack.throttle('logins/email', limit: 4, period: @period) do |req| + if req.path == '/login' && req.post? + req.params['email'] + end + end end - end - - it 'should differentiate requests when throttle_discriminator_normalizer is disabled' do - begin - prev = Rack::Attack.throttle_discriminator_normalizer - Rack::Attack.throttle_discriminator_normalizer = nil + it 'should not differentiate requests when throttle_discriminator_normalizer is enabled' do within_same_period do post_logins - @emails.each do |email| - key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}" - _(Rack::Attack.cache.store.read(key)).must_equal 1 + key = "rack::attack:#{Time.now.to_i / @period}:logins/email:person@example.com" + _(Rack::Attack.cache.store.read(key)).must_equal 3 + end + end + + it 'should differentiate requests when throttle_discriminator_normalizer is disabled' do + begin + prev = Rack::Attack.throttle_discriminator_normalizer + Rack::Attack.throttle_discriminator_normalizer = nil + + within_same_period do + post_logins + @emails.each do |email| + key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}" + _(Rack::Attack.cache.store.read(key)).must_equal 1 + end end + ensure + Rack::Attack.throttle_discriminator_normalizer = prev end - ensure - Rack::Attack.throttle_discriminator_normalizer = prev end - end - def post_logins - @emails.each do |email| - post '/login', email: email + def post_logins + @emails.each do |email| + post '/login', email: email + end end end end diff --git a/spec/rack_attack_track_spec.rb b/spec/rack_attack_track_spec.rb index 0013c15b..65dd2a3e 100644 --- a/spec/rack_attack_track_spec.rb +++ b/spec/rack_attack_track_spec.rb @@ -2,52 +2,54 @@ require_relative 'spec_helper' -describe 'Rack::Attack.track' do - let(:notifications) { [] } +if defined?(::ActiveSupport::Notifications) + describe 'Rack::Attack.track' do + let(:notifications) { [] } - before do - Rack::Attack.track("everything") { |_req| true } - end + before do + Rack::Attack.track("everything") { |_req| true } + end - it_allows_ok_requests + it_allows_ok_requests - it "should tag the env" do - get '/' + it "should tag the env" do + get '/' - _(last_request.env['rack.attack.matched']).must_equal 'everything' - _(last_request.env['rack.attack.match_type']).must_equal :track - end + _(last_request.env['rack.attack.matched']).must_equal 'everything' + _(last_request.env['rack.attack.match_type']).must_equal :track + end - describe "with a notification subscriber and two tracks" do - before do - # A second track - Rack::Attack.track("homepage") { |req| req.path == "/" } + describe "with a notification subscriber and two tracks" do + before do + # A second track + Rack::Attack.track("homepage") { |req| req.path == "/" } - ActiveSupport::Notifications.subscribe("track.rack_attack") do |_name, _start, _finish, _id, payload| - notifications.push(payload) - end + ActiveSupport::Notifications.subscribe("track.rack_attack") do |_name, _start, _finish, _id, payload| + notifications.push(payload) + end - get "/" - end + get "/" + end - it "should notify twice" do - _(notifications.size).must_equal 2 + it "should notify twice" do + _(notifications.size).must_equal 2 + end end - end - describe "without limit and period options" do - it "should assign the track filter to a Check instance" do - track = Rack::Attack.track("homepage") { |req| req.path == "/" } + describe "without limit and period options" do + it "should assign the track filter to a Check instance" do + track = Rack::Attack.track("homepage") { |req| req.path == "/" } - _(track.filter.class).must_equal Rack::Attack::Check + _(track.filter.class).must_equal Rack::Attack::Check + end end - end - describe "with limit and period options" do - it "should assign the track filter to a Throttle instance" do - track = Rack::Attack.track("homepage", limit: 10, period: 10) { |req| req.path == "/" } + describe "with limit and period options" do + it "should assign the track filter to a Throttle instance" do + track = Rack::Attack.track("homepage", limit: 10, period: 10) { |req| req.path == "/" } - _(track.filter.class).must_equal Rack::Attack::Throttle + _(track.filter.class).must_equal Rack::Attack::Throttle + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 894de6a1..7a929f5b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -6,7 +6,6 @@ require "minitest/autorun" require "minitest/pride" require "rack/test" -require "active_support" require "rack/attack" if RUBY_ENGINE == "ruby" @@ -19,6 +18,7 @@ def safe_require(name) nil end +safe_require "active_support" safe_require "connection_pool" safe_require "dalli" safe_require "rails" From 537579121e48b0779ebcea1c40ce6d1fc310ac9d Mon Sep 17 00:00:00 2001 From: Santiago Bartesaghi Date: Mon, 5 Jan 2026 12:36:51 -0300 Subject: [PATCH 3/6] Fix rubocop --- rack-attack.gemspec | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rack-attack.gemspec b/rack-attack.gemspec index e2784dd4..5549c767 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -34,10 +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", "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 "rubocop", "~> 1.75" + s.add_development_dependency "rubocop-minitest", "~> 0.36" + s.add_development_dependency "rubocop-performance", "~> 1.23" + s.add_development_dependency "rubocop-rake", "~> 0.6" s.add_development_dependency "timecop", "~> 0.9.1" # byebug only works with MRI From 4c67647fbb4893630fd741b4b43d6f13b9f4a1f4 Mon Sep 17 00:00:00 2001 From: Santiago Bartesaghi Date: Mon, 5 Jan 2026 12:46:13 -0300 Subject: [PATCH 4/6] Fix Dalli3 --- spec/acceptance/stores/dalli_client_spec.rb | 24 +++++++++++---------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/spec/acceptance/stores/dalli_client_spec.rb b/spec/acceptance/stores/dalli_client_spec.rb index 2b273740..f213a030 100644 --- a/spec/acceptance/stores/dalli_client_spec.rb +++ b/spec/acceptance/stores/dalli_client_spec.rb @@ -18,17 +18,19 @@ it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.fetch(key) }) end - describe "ConnectionPool with Dalli::Client as a cache backend" do - before do - Rack::Attack.cache.store = ConnectionPool.new { Dalli::Client.new } + if defined?(::ConnectionPool) + describe "ConnectionPool with Dalli::Client as a cache backend" do + before do + Rack::Attack.cache.store = ConnectionPool.new { Dalli::Client.new } + end + + after do + Rack::Attack.cache.store.with { |client| client.flush_all } + end + + it_works_for_cache_backed_features( + fetch_from_store: ->(key) { Rack::Attack.cache.store.with { |client| client.fetch(key) } } + ) end - - after do - Rack::Attack.cache.store.with { |client| client.flush_all } - end - - it_works_for_cache_backed_features( - fetch_from_store: ->(key) { Rack::Attack.cache.store.with { |client| client.fetch(key) } } - ) end end From 12e7fc9e8673abec59b85940226c2e3b76a318c8 Mon Sep 17 00:00:00 2001 From: Santiago Bartesaghi Date: Mon, 5 Jan 2026 12:58:51 -0300 Subject: [PATCH 5/6] Fix ConnectionPool issue --- Appraisals | 15 +++++++++++++++ .../active_support_7_0_redis_cache_store.gemfile | 1 + .../active_support_7_1_redis_cache_store.gemfile | 1 + .../active_support_7_2_redis_cache_store.gemfile | 1 + .../active_support_8_0_redis_cache_store.gemfile | 1 + .../active_support_8_1_redis_cache_store.gemfile | 1 + 6 files changed, 20 insertions(+) diff --git a/Appraisals b/Appraisals index 9ca0281a..6114f5cc 100644 --- a/Appraisals +++ b/Appraisals @@ -42,26 +42,41 @@ end appraise "active_support_8-1_redis_cache_store" do gem "activesupport", "~> 8.1.0" + # Direct version requirement on connection_pool + # can be removed once https://github.com/rails/rails#56291 is fixed and released + gem "connection_pool", "~> 2.5" gem "redis", "~> 5.0" end appraise "active_support_8-0_redis_cache_store" do gem "activesupport", "~> 8.0.0" + # Direct version requirement on connection_pool + # can be removed once https://github.com/rails/rails#56291 is fixed and released + gem "connection_pool", "~> 2.5" gem "redis", "~> 5.0" end appraise "active_support_7-2_redis_cache_store" do gem "activesupport", "~> 7.2.0" + # Direct version requirement on connection_pool + # can be removed once https://github.com/rails/rails#56291 is fixed and released + gem "connection_pool", "~> 2.5" gem "redis", "~> 5.0" end appraise "active_support_7-1_redis_cache_store" do gem "activesupport", "~> 7.1.0" + # Direct version requirement on connection_pool + # can be removed once https://github.com/rails/rails#56291 is fixed and released + gem "connection_pool", "~> 2.5" gem "redis", "~> 5.0" end appraise "active_support_7-0_redis_cache_store" do gem "activesupport", "~> 7.0.0" + # Direct version requirement on connection_pool + # can be removed once https://github.com/rails/rails#56291 is fixed and released + gem "connection_pool", "~> 2.5" gem "redis", "~> 5.0" end diff --git a/gemfiles/active_support_7_0_redis_cache_store.gemfile b/gemfiles/active_support_7_0_redis_cache_store.gemfile index a94cfe88..f4e44cb7 100644 --- a/gemfiles/active_support_7_0_redis_cache_store.gemfile +++ b/gemfiles/active_support_7_0_redis_cache_store.gemfile @@ -3,6 +3,7 @@ source "https://rubygems.org" gem "activesupport", "~> 7.0.0" +gem "connection_pool", "~> 2.5" gem "redis", "~> 5.0" group :maintenance, optional: true do diff --git a/gemfiles/active_support_7_1_redis_cache_store.gemfile b/gemfiles/active_support_7_1_redis_cache_store.gemfile index a0602ba5..f0b0ae68 100644 --- a/gemfiles/active_support_7_1_redis_cache_store.gemfile +++ b/gemfiles/active_support_7_1_redis_cache_store.gemfile @@ -3,6 +3,7 @@ source "https://rubygems.org" gem "activesupport", "~> 7.1.0" +gem "connection_pool", "~> 2.5" gem "redis", "~> 5.0" group :maintenance, optional: true do diff --git a/gemfiles/active_support_7_2_redis_cache_store.gemfile b/gemfiles/active_support_7_2_redis_cache_store.gemfile index 3c433092..f0ec3a90 100644 --- a/gemfiles/active_support_7_2_redis_cache_store.gemfile +++ b/gemfiles/active_support_7_2_redis_cache_store.gemfile @@ -3,6 +3,7 @@ source "https://rubygems.org" gem "activesupport", "~> 7.2.0" +gem "connection_pool", "~> 2.5" gem "redis", "~> 5.0" group :maintenance, optional: true do diff --git a/gemfiles/active_support_8_0_redis_cache_store.gemfile b/gemfiles/active_support_8_0_redis_cache_store.gemfile index b813cb38..e192fda3 100644 --- a/gemfiles/active_support_8_0_redis_cache_store.gemfile +++ b/gemfiles/active_support_8_0_redis_cache_store.gemfile @@ -3,6 +3,7 @@ source "https://rubygems.org" gem "activesupport", "~> 8.0.0" +gem "connection_pool", "~> 2.5" gem "redis", "~> 5.0" group :maintenance, optional: true do diff --git a/gemfiles/active_support_8_1_redis_cache_store.gemfile b/gemfiles/active_support_8_1_redis_cache_store.gemfile index 57e0ff21..1dd14ed8 100644 --- a/gemfiles/active_support_8_1_redis_cache_store.gemfile +++ b/gemfiles/active_support_8_1_redis_cache_store.gemfile @@ -3,6 +3,7 @@ source "https://rubygems.org" gem "activesupport", "~> 8.1.0" +gem "connection_pool", "~> 2.5" gem "redis", "~> 5.0" group :maintenance, optional: true do From 8c257baf6c349697ef49cfe6e8cdf73f40dda469 Mon Sep 17 00:00:00 2001 From: Santiago Bartesaghi Date: Mon, 5 Jan 2026 12:59:00 -0300 Subject: [PATCH 6/6] Fix requires order --- spec/spec_helper.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7a929f5b..de47bcc6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,24 +1,24 @@ # frozen_string_literal: true +def safe_require(name) + require name +rescue LoadError + nil +end + require "bundler/setup" require "logger" require "minitest/autorun" require "minitest/pride" require "rack/test" +safe_require "active_support" require "rack/attack" if RUBY_ENGINE == "ruby" require "byebug" end -def safe_require(name) - require name -rescue LoadError - nil -end - -safe_require "active_support" safe_require "connection_pool" safe_require "dalli" safe_require "rails"