From d6bc89e39f2506538b78da769577f56b48ff9629 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Sat, 12 Oct 2019 18:03:18 +0300 Subject: [PATCH] Move from proxies to well-defined adapters --- .rubocop.yml | 4 + lib/rack/attack.rb | 12 +- lib/rack/attack/base_proxy.rb | 27 ---- lib/rack/attack/cache.rb | 39 ++---- lib/rack/attack/store_adapter.rb | 47 +++++++ .../active_support_redis_store_adapter.rb | 47 +++++++ .../dalli_adapter.rb} | 24 ++-- .../mem_cache_store_adapter.rb} | 13 +- .../redis_adapter.rb} | 28 ++-- .../redis_cache_store_adapter.rb | 57 ++++++++ .../redis_store_adapter.rb} | 12 +- .../active_support_redis_store_proxy.rb | 39 ------ .../store_proxy/redis_cache_store_proxy.rb | 33 ----- rack-attack.gemspec | 1 - spec/acceptance/cache_spec.rb | 55 ++++++++ .../cache_store_config_for_allow2ban_spec.rb | 122 ------------------ .../cache_store_config_for_fail2ban_spec.rb | 117 ----------------- .../cache_store_config_for_throttle_spec.rb | 50 ------- .../cache_store_config_with_rails_spec.rb | 33 ----- .../stores/active_support_dalli_store_spec.rb | 7 +- ...ive_support_mem_cache_store_pooled_spec.rb | 5 +- .../active_support_mem_cache_store_spec.rb | 5 +- .../active_support_memory_store_spec.rb | 5 +- ...e_support_redis_cache_store_pooled_spec.rb | 7 +- .../active_support_redis_cache_store_spec.rb | 7 +- .../stores/active_support_redis_store_spec.rb | 5 +- .../connection_pool_dalli_client_spec.rb | 7 +- spec/acceptance/stores/dalli_client_spec.rb | 7 +- spec/acceptance/stores/redis_spec.rb | 7 +- spec/acceptance/stores/redis_store_spec.rb | 5 +- spec/rack_attack_dalli_proxy_spec.rb | 10 -- spec/rack_attack_spec.rb | 6 +- spec/spec_helper.rb | 2 +- spec/support/dummy_store_implementation.rb | 11 ++ 34 files changed, 325 insertions(+), 531 deletions(-) delete mode 100644 lib/rack/attack/base_proxy.rb create mode 100644 lib/rack/attack/store_adapter.rb create mode 100644 lib/rack/attack/store_adapters/active_support_redis_store_adapter.rb rename lib/rack/attack/{store_proxy/dalli_proxy.rb => store_adapters/dalli_adapter.rb} (78%) rename lib/rack/attack/{store_proxy/mem_cache_store_proxy.rb => store_adapters/mem_cache_store_adapter.rb} (51%) rename lib/rack/attack/{store_proxy/redis_proxy.rb => store_adapters/redis_adapter.rb} (64%) create mode 100644 lib/rack/attack/store_adapters/redis_cache_store_adapter.rb rename lib/rack/attack/{store_proxy/redis_store_proxy.rb => store_adapters/redis_store_adapter.rb} (55%) delete mode 100644 lib/rack/attack/store_proxy/active_support_redis_store_proxy.rb delete mode 100644 lib/rack/attack/store_proxy/redis_cache_store_proxy.rb create mode 100644 spec/acceptance/cache_spec.rb delete mode 100644 spec/acceptance/cache_store_config_for_allow2ban_spec.rb delete mode 100644 spec/acceptance/cache_store_config_for_fail2ban_spec.rb delete mode 100644 spec/acceptance/cache_store_config_for_throttle_spec.rb delete mode 100644 spec/acceptance/cache_store_config_with_rails_spec.rb delete mode 100644 spec/rack_attack_dalli_proxy_spec.rb create mode 100644 spec/support/dummy_store_implementation.rb diff --git a/.rubocop.yml b/.rubocop.yml index 478b1fc1..6cf4e31c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -97,3 +97,7 @@ Style/SpecialGlobalVars: Style/UnneededPercentQ: Enabled: true + +Lint/UnusedMethodArgument: + Exclude: + - "lib/rack/attack/store_adapter.rb" diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index ad668756..e76fcaf4 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -6,12 +6,12 @@ require 'rack/attack/configuration' require 'rack/attack/path_normalizer' require 'rack/attack/request' -require 'rack/attack/store_proxy/dalli_proxy' -require 'rack/attack/store_proxy/mem_cache_store_proxy' -require 'rack/attack/store_proxy/redis_proxy' -require 'rack/attack/store_proxy/redis_store_proxy' -require 'rack/attack/store_proxy/redis_cache_store_proxy' -require 'rack/attack/store_proxy/active_support_redis_store_proxy' +require 'rack/attack/store_adapters/dalli_adapter' +require 'rack/attack/store_adapters/mem_cache_store_adapter' +require 'rack/attack/store_adapters/redis_adapter' +require 'rack/attack/store_adapters/redis_store_adapter' +require 'rack/attack/store_adapters/redis_cache_store_adapter' +require 'rack/attack/store_adapters/active_support_redis_store_adapter' require 'rack/attack/railtie' if defined?(::Rails) diff --git a/lib/rack/attack/base_proxy.rb b/lib/rack/attack/base_proxy.rb deleted file mode 100644 index 3e3c28af..00000000 --- a/lib/rack/attack/base_proxy.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -require 'delegate' - -module Rack - class Attack - class BaseProxy < SimpleDelegator - class << self - def proxies - @@proxies ||= [] - end - - def inherited(klass) - proxies << klass - end - - def lookup(store) - proxies.find { |proxy| proxy.handle?(store) } - end - - def handle?(_store) - raise NotImplementedError - end - end - end - end -end diff --git a/lib/rack/attack/cache.rb b/lib/rack/attack/cache.rb index f27259e1..4d4b777d 100644 --- a/lib/rack/attack/cache.rb +++ b/lib/rack/attack/cache.rb @@ -13,12 +13,16 @@ def initialize attr_reader :store def store=(store) - @store = - if (proxy = BaseProxy.lookup(store)) - proxy.new(store) - else - store - end + raise Rack::Attack::MissingStoreError if store.nil? + + adapter = StoreAdapter.lookup(store) + if adapter + @store = adapter.new(store) + elsif store?(store) + @store = store + else + raise Rack::Attack::MisconfiguredStoreError + end end def count(unprefixed_key, period) @@ -27,9 +31,6 @@ def count(unprefixed_key, period) end def read(unprefixed_key) - enforce_store_presence! - enforce_store_method_presence!(:read) - store.read("#{prefix}:#{unprefixed_key}") end @@ -67,33 +68,19 @@ def key_and_expiry(unprefixed_key, period) end def do_count(key, expires_in) - enforce_store_presence! - enforce_store_method_presence!(:increment) - result = store.increment(key, 1, expires_in: expires_in) # NB: Some stores return nil when incrementing uninitialized values if result.nil? - enforce_store_method_presence!(:write) - store.write(key, 1, expires_in: expires_in) end result || 1 end - def enforce_store_presence! - if store.nil? - raise Rack::Attack::MissingStoreError - end - end + STORE_METHODS = [:read, :write, :increment, :delete].freeze - def enforce_store_method_presence!(method_name) - if !store.respond_to?(method_name) - raise( - Rack::Attack::MisconfiguredStoreError, - "Configured store #{store.class.name} doesn't respond to ##{method_name} method" - ) - end + def store?(object) + STORE_METHODS.all? { |meth| object.respond_to?(meth) } end end end diff --git a/lib/rack/attack/store_adapter.rb b/lib/rack/attack/store_adapter.rb new file mode 100644 index 00000000..551ab632 --- /dev/null +++ b/lib/rack/attack/store_adapter.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Rack + class Attack + class StoreAdapter + class << self + def adapters + @@adapters ||= [] + end + + def inherited(klass) + adapters << klass + end + + def lookup(store) + adapters.find { |adapter| adapter.handle?(store) } + end + + def handle?(store) + raise NotImplementedError + end + end + + attr_reader :store + + def initialize(store) + @store = store + end + + def read(key) + raise NotImplementedError + end + + def write(key, value, options = {}) + raise NotImplementedError + end + + def increment(key, amount, options = {}) + raise NotImplementedError + end + + def delete(key, options = {}) + raise NotImplementedError + end + end + end +end diff --git a/lib/rack/attack/store_adapters/active_support_redis_store_adapter.rb b/lib/rack/attack/store_adapters/active_support_redis_store_adapter.rb new file mode 100644 index 00000000..d409dd52 --- /dev/null +++ b/lib/rack/attack/store_adapters/active_support_redis_store_adapter.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'rack/attack/store_adapter' + +module Rack + class Attack + module StoreAdapters + class ActiveSupportRedisStoreAdapter < StoreAdapter + def self.handle?(store) + defined?(::Redis) && + defined?(::ActiveSupport::Cache::RedisStore) && + store.is_a?(::ActiveSupport::Cache::RedisStore) + end + + def read(key, options = {}) + store.read(key, options.merge!(raw: true)) + end + + def write(key, value, options = {}) + store.write(key, value, options.merge!(raw: true)) + end + + def increment(key, amount = 1, options = {}) + # #increment ignores options[:expires_in]. + # + # So in order to workaround this we use #write (which sets expiration) to initialize + # the counter. After that we continue using the original #increment. + if options[:expires_in] && !read(key) + write(key, amount, options) + + amount + else + store.increment(key, amount, options) + end + end + + def delete(key, options = {}) + store.delete(key, options) + end + + def delete_matched(matcher, options = nil) + store.delete_matched(matcher, options) + end + end + end + end +end diff --git a/lib/rack/attack/store_proxy/dalli_proxy.rb b/lib/rack/attack/store_adapters/dalli_adapter.rb similarity index 78% rename from lib/rack/attack/store_proxy/dalli_proxy.rb rename to lib/rack/attack/store_adapters/dalli_adapter.rb index 48198bb2..a65c6f3c 100644 --- a/lib/rack/attack/store_proxy/dalli_proxy.rb +++ b/lib/rack/attack/store_adapters/dalli_adapter.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -require 'rack/attack/base_proxy' +require 'rack/attack/store_adapter' module Rack class Attack - module StoreProxy - class DalliProxy < BaseProxy + module StoreAdapters + class DalliAdapter < StoreAdapter def self.handle?(store) return false unless defined?(::Dalli) @@ -18,14 +18,14 @@ def self.handle?(store) end end - def initialize(client) - super(client) + def initialize(store) + super stub_with_if_missing end def read(key) rescuing do - with do |client| + store.with do |client| client.get(key) end end @@ -33,7 +33,7 @@ def read(key) def write(key, value, options = {}) rescuing do - with do |client| + store.with do |client| client.set(key, value, options.fetch(:expires_in, 0), raw: true) end end @@ -41,7 +41,7 @@ def write(key, value, options = {}) def increment(key, amount, options = {}) rescuing do - with do |client| + store.with do |client| client.incr(key, amount, options.fetch(:expires_in, 0), amount) end end @@ -49,7 +49,7 @@ def increment(key, amount, options = {}) def delete(key) rescuing do - with do |client| + store.with do |client| client.delete(key) end end @@ -58,10 +58,10 @@ def delete(key) private def stub_with_if_missing - unless __getobj__.respond_to?(:with) - class << self + unless store.respond_to?(:with) + class << store def with - yield __getobj__ + yield store end end end diff --git a/lib/rack/attack/store_proxy/mem_cache_store_proxy.rb b/lib/rack/attack/store_adapters/mem_cache_store_adapter.rb similarity index 51% rename from lib/rack/attack/store_proxy/mem_cache_store_proxy.rb rename to lib/rack/attack/store_adapters/mem_cache_store_adapter.rb index 122037f0..47777448 100644 --- a/lib/rack/attack/store_proxy/mem_cache_store_proxy.rb +++ b/lib/rack/attack/store_adapters/mem_cache_store_adapter.rb @@ -1,19 +1,22 @@ # frozen_string_literal: true -require 'rack/attack/base_proxy' +require 'forwardable' module Rack class Attack - module StoreProxy - class MemCacheStoreProxy < BaseProxy + module StoreAdapters + class MemCacheStoreAdapter < StoreAdapter def self.handle?(store) defined?(::Dalli) && defined?(::ActiveSupport::Cache::MemCacheStore) && store.is_a?(::ActiveSupport::Cache::MemCacheStore) end - def write(name, value, options = {}) - super(name, value, options.merge!(raw: true)) + extend Forwardable + def_delegators :@store, :read, :increment, :delete + + def write(key, value, options = {}) + store.write(key, value, options.merge!(raw: true)) end end end diff --git a/lib/rack/attack/store_proxy/redis_proxy.rb b/lib/rack/attack/store_adapters/redis_adapter.rb similarity index 64% rename from lib/rack/attack/store_proxy/redis_proxy.rb rename to lib/rack/attack/store_adapters/redis_adapter.rb index 3127de63..d9adff6f 100644 --- a/lib/rack/attack/store_proxy/redis_proxy.rb +++ b/lib/rack/attack/store_adapters/redis_adapter.rb @@ -1,17 +1,17 @@ # frozen_string_literal: true -require 'rack/attack/base_proxy' +require 'rack/attack/store_adapter' module Rack class Attack - module StoreProxy - class RedisProxy < BaseProxy - def initialize(*args) + module StoreAdapters + class RedisAdapter < StoreAdapter + def initialize(store) if Gem::Version.new(Redis::VERSION) < Gem::Version.new("3") warn 'RackAttack requires Redis gem >= 3.0.0.' end - super(*args) + super end def self.handle?(store) @@ -19,28 +19,28 @@ def self.handle?(store) end def read(key) - rescuing { get(key) } + rescuing { store.get(key) } end def write(key, value, options = {}) if (expires_in = options[:expires_in]) - rescuing { setex(key, expires_in, value) } + rescuing { store.setex(key, expires_in, value) } else - rescuing { set(key, value) } + rescuing { store.set(key, value) } end end def increment(key, amount, options = {}) rescuing do - pipelined do - incrby(key, amount) - expire(key, options[:expires_in]) if options[:expires_in] + store.pipelined do + store.incrby(key, amount) + store.expire(key, options[:expires_in]) if options[:expires_in] end.first end end def delete(key, _options = {}) - rescuing { del(key) } + rescuing { store.del(key) } end def delete_matched(matcher, _options = nil) @@ -49,8 +49,8 @@ def delete_matched(matcher, _options = nil) rescuing do # Fetch keys in batches using SCAN to avoid blocking the Redis server. loop do - cursor, keys = scan(cursor, match: matcher, count: 1000) - del(*keys) unless keys.empty? + cursor, keys = store.scan(cursor, match: matcher, count: 1000) + store.del(*keys) unless keys.empty? break if cursor == "0" end end diff --git a/lib/rack/attack/store_adapters/redis_cache_store_adapter.rb b/lib/rack/attack/store_adapters/redis_cache_store_adapter.rb new file mode 100644 index 00000000..68d70f86 --- /dev/null +++ b/lib/rack/attack/store_adapters/redis_cache_store_adapter.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'rack/attack/store_adapter' + +module Rack + class Attack + module StoreAdapters + class RedisCacheStoreAdapter < StoreAdapter + def self.handle?(store) + store.class.name == "ActiveSupport::Cache::RedisCacheStore" + end + + def read(*args) + rescuing { store.read(*args) } + end + + def write(key, value, options = {}) + rescuing do + store.write(key, value, options.merge!(raw: true)) + end + end + + def increment(key, amount = 1, options = {}) + # RedisCacheStore#increment ignores options[:expires_in]. + # + # So in order to workaround this we use RedisCacheStore#write (which sets expiration) to initialize + # the counter. After that we continue using the original RedisCacheStore#increment. + rescuing do + if options[:expires_in] && !read(key) + write(key, amount, options) + + amount + else + store.increment(key, amount, options) + end + end + end + + def delete(*args) + rescuing { store.delete(*args) } + end + + def delete_matched(matcher, options = nil) + store.delete_matched(matcher, options) + end + + private + + def rescuing + yield + rescue Redis::BaseConnectionError + nil + end + end + end + end +end diff --git a/lib/rack/attack/store_proxy/redis_store_proxy.rb b/lib/rack/attack/store_adapters/redis_store_adapter.rb similarity index 55% rename from lib/rack/attack/store_proxy/redis_store_proxy.rb rename to lib/rack/attack/store_adapters/redis_store_adapter.rb index 28557bcb..bdce4d2b 100644 --- a/lib/rack/attack/store_proxy/redis_store_proxy.rb +++ b/lib/rack/attack/store_adapters/redis_store_adapter.rb @@ -1,24 +1,24 @@ # frozen_string_literal: true -require 'rack/attack/store_proxy/redis_proxy' +require 'rack/attack/store_adapters/redis_adapter' module Rack class Attack - module StoreProxy - class RedisStoreProxy < RedisProxy + module StoreAdapters + class RedisStoreAdapter < RedisAdapter def self.handle?(store) defined?(::Redis::Store) && store.is_a?(::Redis::Store) end def read(key) - rescuing { get(key, raw: true) } + rescuing { store.get(key, raw: true) } end def write(key, value, options = {}) if (expires_in = options[:expires_in]) - rescuing { setex(key, expires_in, value, raw: true) } + rescuing { store.setex(key, expires_in, value, raw: true) } else - rescuing { set(key, value, raw: true) } + rescuing { store.set(key, value, raw: true) } end end end diff --git a/lib/rack/attack/store_proxy/active_support_redis_store_proxy.rb b/lib/rack/attack/store_proxy/active_support_redis_store_proxy.rb deleted file mode 100644 index d2c0e3b2..00000000 --- a/lib/rack/attack/store_proxy/active_support_redis_store_proxy.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require 'rack/attack/base_proxy' - -module Rack - class Attack - module StoreProxy - class ActiveSupportRedisStoreProxy < BaseProxy - def self.handle?(store) - defined?(::Redis) && - defined?(::ActiveSupport::Cache::RedisStore) && - store.is_a?(::ActiveSupport::Cache::RedisStore) - end - - def increment(name, amount = 1, options = {}) - # #increment ignores options[:expires_in]. - # - # So in order to workaround this we use #write (which sets expiration) to initialize - # the counter. After that we continue using the original #increment. - if options[:expires_in] && !read(name) - write(name, amount, options) - - amount - else - super - end - end - - def read(name, options = {}) - super(name, options.merge!(raw: true)) - end - - def write(name, value, options = {}) - super(name, value, options.merge!(raw: true)) - end - end - end - end -end diff --git a/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb b/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb deleted file mode 100644 index 380215f7..00000000 --- a/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -require 'rack/attack/base_proxy' - -module Rack - class Attack - module StoreProxy - class RedisCacheStoreProxy < BaseProxy - def self.handle?(store) - store.class.name == "ActiveSupport::Cache::RedisCacheStore" - end - - def increment(name, amount = 1, options = {}) - # RedisCacheStore#increment ignores options[:expires_in]. - # - # So in order to workaround this we use RedisCacheStore#write (which sets expiration) to initialize - # the counter. After that we continue using the original RedisCacheStore#increment. - if options[:expires_in] && !read(name) - write(name, amount, options) - - amount - else - super - end - end - - def write(name, value, options = {}) - super(name, value, options.merge!(raw: true)) - end - end - end - end -end diff --git a/rack-attack.gemspec b/rack-attack.gemspec index 65a74444..fa39ca16 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -34,7 +34,6 @@ Gem::Specification.new do |s| s.add_development_dependency 'appraisal', '~> 2.2' s.add_development_dependency "bundler", ">= 1.17", "< 3.0" s.add_development_dependency 'minitest', "~> 5.11" - s.add_development_dependency "minitest-stub-const", "~> 0.6" s.add_development_dependency 'rack-test', "~> 1.0" s.add_development_dependency 'rake', "~> 13.0" s.add_development_dependency "rubocop", "0.75.0" diff --git a/spec/acceptance/cache_spec.rb b/spec/acceptance/cache_spec.rb new file mode 100644 index 00000000..a1303ccd --- /dev/null +++ b/spec/acceptance/cache_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require_relative "../spec_helper" +require_relative "../support/dummy_store_implementation" + +describe "Cache" do + before do + Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| + request.ip + end + end + + it "fails when Rails.cache is not set" do + Rails.cache = nil + assert_raises(Rack::Attack::MissingStoreError) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + end + end + + it "works when Rails.cache is set" do + Rails.cache = ActiveSupport::Cache::MemoryStore.new + 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 + + it "uses store adapter if available" do + store_class = Class.new + adapter_class = Class.new(Rack::Attack::StoreAdapter) do + include DummyStoreImplementation + define_singleton_method(:handle?) do |store| + store.is_a?(store_class) + end + end + + Rack::Attack.cache.store = store_class.new + assert_equal adapter_class, Rack::Attack.cache.store.class + end + + it "uses store if adapter is not available" do + store_class = Class.new { include DummyStoreImplementation } + + Rack::Attack.cache.store = store_class.new + assert_equal store_class, Rack::Attack.cache.store.class + end + + it "raises if store does not implement full required api" do + store_class = Class.new + assert_raises(Rack::Attack::MisconfiguredStoreError) do + Rack::Attack.cache.store = store_class.new + end + end +end diff --git a/spec/acceptance/cache_store_config_for_allow2ban_spec.rb b/spec/acceptance/cache_store_config_for_allow2ban_spec.rb deleted file mode 100644 index ce994993..00000000 --- a/spec/acceptance/cache_store_config_for_allow2ban_spec.rb +++ /dev/null @@ -1,122 +0,0 @@ -# frozen_string_literal: true - -require_relative "../spec_helper" -require "minitest/stub_const" - -describe "Cache store config when using allow2ban" do - before do - 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 - - it "gives semantic error if no store was configured" do - assert_raises(Rack::Attack::MissingStoreError) do - get "/scarce-resource" - end - end - - it "gives semantic error if store is missing #read method" do - raised_exception = nil - - fake_store_class = Class.new do - def write(key, value); end - - def increment(key, count, options = {}); end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/scarce-resource" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #read method", raised_exception.message - end - - it "gives semantic error if store is missing #write method" do - raised_exception = nil - - fake_store_class = Class.new do - def read(key); end - - def increment(key, count, options = {}); end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/scarce-resource" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #write method", raised_exception.message - end - - it "gives semantic error if store is missing #increment method" do - raised_exception = nil - - fake_store_class = Class.new do - def read(key); end - - def write(key, value); end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/scarce-resource" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #increment method", raised_exception.message - end - - it "works with any object that responds to #read, #write and #increment" do - fake_store_class = Class.new do - attr_accessor :backend - - def initialize - @backend = {} - end - - def read(key) - @backend[key] - end - - def write(key, value, _options = {}) - @backend[key] = value - end - - def increment(key, _count, _options = {}) - @backend[key] ||= 0 - @backend[key] += 1 - end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - get "/" - assert_equal 200, last_response.status - - get "/scarce-resource" - assert_equal 200, last_response.status - - get "/scarce-resource" - assert_equal 200, last_response.status - - get "/scarce-resource" - assert_equal 403, last_response.status - - get "/" - assert_equal 403, last_response.status - end - end -end diff --git a/spec/acceptance/cache_store_config_for_fail2ban_spec.rb b/spec/acceptance/cache_store_config_for_fail2ban_spec.rb deleted file mode 100644 index 6f330eee..00000000 --- a/spec/acceptance/cache_store_config_for_fail2ban_spec.rb +++ /dev/null @@ -1,117 +0,0 @@ -# frozen_string_literal: true - -require_relative "../spec_helper" -require "minitest/stub_const" - -describe "Cache store config when using fail2ban" do - before do - 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 - - it "gives semantic error if no store was configured" do - assert_raises(Rack::Attack::MissingStoreError) do - get "/private-place" - end - end - - it "gives semantic error if store is missing #read method" do - raised_exception = nil - - fake_store_class = Class.new do - def write(key, value); end - - def increment(key, count, options = {}); end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/private-place" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #read method", raised_exception.message - end - - it "gives semantic error if store is missing #write method" do - raised_exception = nil - - fake_store_class = Class.new do - def read(key); end - - def increment(key, count, options = {}); end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/private-place" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #write method", raised_exception.message - end - - it "gives semantic error if store is missing #increment method" do - raised_exception = nil - - fake_store_class = Class.new do - def read(key); end - - def write(key, value); end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/private-place" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #increment method", raised_exception.message - end - - it "works with any object that responds to #read, #write and #increment" do - FakeStore = Class.new do - attr_accessor :backend - - def initialize - @backend = {} - end - - def read(key) - @backend[key] - end - - def write(key, value, _options = {}) - @backend[key] = value - end - - def increment(key, _count, _options = {}) - @backend[key] ||= 0 - @backend[key] += 1 - end - end - - Rack::Attack.cache.store = FakeStore.new - - get "/" - assert_equal 200, last_response.status - - get "/private-place" - assert_equal 403, last_response.status - - get "/private-place" - assert_equal 403, last_response.status - - get "/" - assert_equal 403, last_response.status - end -end diff --git a/spec/acceptance/cache_store_config_for_throttle_spec.rb b/spec/acceptance/cache_store_config_for_throttle_spec.rb deleted file mode 100644 index 9be6e59a..00000000 --- a/spec/acceptance/cache_store_config_for_throttle_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -require_relative "../spec_helper" - -describe "Cache store config when throttling without Rails" do - before do - Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request| - request.ip - end - end - - it "gives semantic error if no store was configured" do - assert_raises(Rack::Attack::MissingStoreError) do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - end - end - - it "gives semantic error if incompatible store was configured" do - Rack::Attack.cache.store = Object.new - - assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - end - end - - it "works with any object that responds to #increment" do - basic_store_class = Class.new do - attr_accessor :counts - - def initialize - @counts = {} - end - - def increment(key, _count, _options) - @counts[key] ||= 0 - @counts[key] += 1 - end - end - - Rack::Attack.cache.store = basic_store_class.new - - 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 -end diff --git a/spec/acceptance/cache_store_config_with_rails_spec.rb b/spec/acceptance/cache_store_config_with_rails_spec.rb deleted file mode 100644 index 3d9ac22f..00000000 --- a/spec/acceptance/cache_store_config_with_rails_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -require_relative "../spec_helper" -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 - end - end - - 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 - - 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 - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - - assert_equal 429, last_response.status - end - end -end diff --git a/spec/acceptance/stores/active_support_dalli_store_spec.rb b/spec/acceptance/stores/active_support_dalli_store_spec.rb index 58964434..e681e79a 100644 --- a/spec/acceptance/stores/active_support_dalli_store_spec.rb +++ b/spec/acceptance/stores/active_support_dalli_store_spec.rb @@ -9,13 +9,14 @@ describe "ActiveSupport::Cache::DalliStore as a cache backend" do before do - Rack::Attack.cache.store = ActiveSupport::Cache::DalliStore.new + @store = ActiveSupport::Cache::DalliStore.new + Rack::Attack.cache.store = @store end after do - Rack::Attack.cache.store.clear + @store.clear end - it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.fetch(key) }) + it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.read(key) }) end end diff --git a/spec/acceptance/stores/active_support_mem_cache_store_pooled_spec.rb b/spec/acceptance/stores/active_support_mem_cache_store_pooled_spec.rb index 8344b6e4..6ae8b30c 100644 --- a/spec/acceptance/stores/active_support_mem_cache_store_pooled_spec.rb +++ b/spec/acceptance/stores/active_support_mem_cache_store_pooled_spec.rb @@ -8,11 +8,12 @@ describe "ActiveSupport::Cache::MemCacheStore (pooled) as a cache backend" do before do - Rack::Attack.cache.store = ActiveSupport::Cache::MemCacheStore.new(pool_size: 2) + @store = ActiveSupport::Cache::MemCacheStore.new(pool_size: 2) + Rack::Attack.cache.store = @store end after do - Rack::Attack.cache.store.clear + @store.clear end it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.read(key) }) 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 65abe7d7..e20b2068 100644 --- a/spec/acceptance/stores/active_support_mem_cache_store_spec.rb +++ b/spec/acceptance/stores/active_support_mem_cache_store_spec.rb @@ -8,11 +8,12 @@ describe "ActiveSupport::Cache::MemCacheStore as a cache backend" do before do - Rack::Attack.cache.store = ActiveSupport::Cache::MemCacheStore.new + @store = ActiveSupport::Cache::MemCacheStore.new + Rack::Attack.cache.store = @store end after do - Rack::Attack.cache.store.clear + @store.clear end it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.read(key) }) diff --git a/spec/acceptance/stores/active_support_memory_store_spec.rb b/spec/acceptance/stores/active_support_memory_store_spec.rb index e047b444..ecc62f08 100644 --- a/spec/acceptance/stores/active_support_memory_store_spec.rb +++ b/spec/acceptance/stores/active_support_memory_store_spec.rb @@ -7,11 +7,12 @@ describe "ActiveSupport::Cache::MemoryStore as a cache backend" do before do - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + @store = ActiveSupport::Cache::MemoryStore.new + Rack::Attack.cache.store = @store end after do - Rack::Attack.cache.store.clear + @store.clear end it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.fetch(key) }) diff --git a/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb b/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb index 9c26e8d6..1d32a486 100644 --- a/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb +++ b/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb @@ -14,13 +14,14 @@ describe "ActiveSupport::Cache::RedisCacheStore (pooled) as a cache backend" do before do - Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(pool_size: 2) + @store = ActiveSupport::Cache::RedisCacheStore.new(pool_size: 2) + Rack::Attack.cache.store = @store end after do - Rack::Attack.cache.store.clear + @store.clear end - it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.fetch(key) }) + it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.read(key) }) end end diff --git a/spec/acceptance/stores/active_support_redis_cache_store_spec.rb b/spec/acceptance/stores/active_support_redis_cache_store_spec.rb index f595ec2a..b19458b0 100644 --- a/spec/acceptance/stores/active_support_redis_cache_store_spec.rb +++ b/spec/acceptance/stores/active_support_redis_cache_store_spec.rb @@ -13,13 +13,14 @@ describe "ActiveSupport::Cache::RedisCacheStore as a cache backend" do before do - Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new + @store = ActiveSupport::Cache::RedisCacheStore.new + Rack::Attack.cache.store = @store end after do - Rack::Attack.cache.store.clear + @store.clear end - it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.fetch(key) }) + it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.read(key) }) end end diff --git a/spec/acceptance/stores/active_support_redis_store_spec.rb b/spec/acceptance/stores/active_support_redis_store_spec.rb index 75e4d68d..84f4b220 100644 --- a/spec/acceptance/stores/active_support_redis_store_spec.rb +++ b/spec/acceptance/stores/active_support_redis_store_spec.rb @@ -8,11 +8,12 @@ describe "ActiveSupport::Cache::RedisStore as a cache backend" do before do - Rack::Attack.cache.store = ActiveSupport::Cache::RedisStore.new + @store = ActiveSupport::Cache::RedisStore.new + Rack::Attack.cache.store = @store end after do - Rack::Attack.cache.store.clear + @store.clear end it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.read(key) }) diff --git a/spec/acceptance/stores/connection_pool_dalli_client_spec.rb b/spec/acceptance/stores/connection_pool_dalli_client_spec.rb index d532a29b..886c6436 100644 --- a/spec/acceptance/stores/connection_pool_dalli_client_spec.rb +++ b/spec/acceptance/stores/connection_pool_dalli_client_spec.rb @@ -10,15 +10,16 @@ describe "ConnectionPool with Dalli::Client as a cache backend" do before do - Rack::Attack.cache.store = ConnectionPool.new { Dalli::Client.new } + @store = ConnectionPool.new { Dalli::Client.new } + Rack::Attack.cache.store = @store end after do - Rack::Attack.cache.store.with { |client| client.flush_all } + @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) } } + fetch_from_store: ->(key) { Dalli::Client.new.get(key) } ) end end diff --git a/spec/acceptance/stores/dalli_client_spec.rb b/spec/acceptance/stores/dalli_client_spec.rb index 08038c2f..11a2fc88 100644 --- a/spec/acceptance/stores/dalli_client_spec.rb +++ b/spec/acceptance/stores/dalli_client_spec.rb @@ -9,13 +9,14 @@ describe "Dalli::Client as a cache backend" do before do - Rack::Attack.cache.store = Dalli::Client.new + @client = Dalli::Client.new + Rack::Attack.cache.store = @client end after do - Rack::Attack.cache.store.flush_all + @client.flush_all end - it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.fetch(key) }) + it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.read(key) }) end end diff --git a/spec/acceptance/stores/redis_spec.rb b/spec/acceptance/stores/redis_spec.rb index 4361566c..44b11b13 100644 --- a/spec/acceptance/stores/redis_spec.rb +++ b/spec/acceptance/stores/redis_spec.rb @@ -8,13 +8,14 @@ describe "Plain redis as a cache backend" do before do - Rack::Attack.cache.store = Redis.new + @store = Redis.new + Rack::Attack.cache.store = @store end after do - Rack::Attack.cache.store.flushdb + @store.flushdb end - it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.get(key) }) + it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.read(key) }) end end diff --git a/spec/acceptance/stores/redis_store_spec.rb b/spec/acceptance/stores/redis_store_spec.rb index d7e8e115..12939695 100644 --- a/spec/acceptance/stores/redis_store_spec.rb +++ b/spec/acceptance/stores/redis_store_spec.rb @@ -8,11 +8,12 @@ describe "ActiveSupport::Cache::RedisStore as a cache backend" do before do - Rack::Attack.cache.store = ::Redis::Store.new + @store = ::Redis::Store.new + Rack::Attack.cache.store = @store end after do - Rack::Attack.cache.store.flushdb + @store.flushdb end it_works_for_cache_backed_features(fetch_from_store: ->(key) { Rack::Attack.cache.store.read(key) }) diff --git a/spec/rack_attack_dalli_proxy_spec.rb b/spec/rack_attack_dalli_proxy_spec.rb deleted file mode 100644 index 7eb23f00..00000000 --- a/spec/rack_attack_dalli_proxy_spec.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -require_relative 'spec_helper' - -describe Rack::Attack::StoreProxy::DalliProxy do - it 'should stub Dalli::Client#with on older clients' do - proxy = Rack::Attack::StoreProxy::DalliProxy.new(Class.new) - proxy.with {} # will not raise an error - end -end diff --git a/spec/rack_attack_spec.rb b/spec/rack_attack_spec.rb index d9bab4d7..141e5a7c 100644 --- a/spec/rack_attack_spec.rb +++ b/spec/rack_attack_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative 'spec_helper' +require_relative 'support/dummy_store_implementation' describe 'Rack::Attack' do it_allows_ok_requests @@ -102,7 +103,10 @@ describe 'reset!' do it 'raises an error when is not supported by cache store' do - Rack::Attack.cache.store = Class.new + store_class = Class.new { include DummyStoreImplementation } + + Rack::Attack.cache.store = store_class.new + assert_raises(Rack::Attack::IncompatibleStoreError) do Rack::Attack.reset! end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9649be81..fb5531dd 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -29,7 +29,7 @@ class MiniTest::Spec include Rack::Test::Methods before do - Rails.cache = nil + Rails.cache = ActiveSupport::Cache::MemoryStore.new end after do diff --git a/spec/support/dummy_store_implementation.rb b/spec/support/dummy_store_implementation.rb new file mode 100644 index 00000000..42907f7a --- /dev/null +++ b/spec/support/dummy_store_implementation.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module DummyStoreImplementation + def read(*); end + + def write(*); end + + def increment(*); end + + def delete(*); end +end