diff --git a/.rubocop.yml b/.rubocop.yml index b96f7f3..28f8502 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,3 +1,6 @@ +require: + - rubocop-performance + inherit_mode: merge: - Exclude @@ -26,6 +29,9 @@ Naming: Exclude: - "lib/rack/attack/path_normalizer.rb" +Metrics/LineLength: + Max: 120 + Performance: Enabled: true @@ -34,13 +40,25 @@ Security: Style/BlockDelimiters: Enabled: true + IgnoredMethods: [] # Workaround rubocop bug: https://github.com/rubocop-hq/rubocop/issues/6179 Style/BracesAroundHashParameters: Enabled: true +Style/ClassAndModuleChildren: + Enabled: true + Exclude: + - "spec/**/*" + +Style/ConditionalAssignment: + Enabled: true + Style/Encoding: Enabled: true +Style/ExpandPathArguments: + Enabled: true + Style/EmptyMethod: Enabled: true @@ -53,6 +71,9 @@ Style/HashSyntax: Style/OptionalArguments: Enabled: true +Style/ParallelAssignment: + Enabled: true + Style/RaiseArgs: Enabled: true @@ -70,3 +91,9 @@ Style/Semicolon: Style/SingleLineMethods: Enabled: true + +Style/SpecialGlobalVars: + Enabled: true + +Style/UnneededPercentQ: + Enabled: true diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 8c62e7d..f4297a5 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -6,159 +6,164 @@ require 'rack/attack/path_normalizer' require 'rack/attack/request' require "ipaddr" -class Rack::Attack - class MisconfiguredStoreError < StandardError; end - class MissingStoreError < StandardError; end +module Rack + class Attack + class MisconfiguredStoreError < StandardError; end + class MissingStoreError < StandardError; end - autoload :Cache, 'rack/attack/cache' - autoload :Check, 'rack/attack/check' - autoload :Throttle, 'rack/attack/throttle' - autoload :Safelist, 'rack/attack/safelist' - autoload :Blocklist, 'rack/attack/blocklist' - autoload :Track, 'rack/attack/track' - autoload :StoreProxy, 'rack/attack/store_proxy' - autoload :DalliProxy, 'rack/attack/store_proxy/dalli_proxy' - autoload :MemCacheStoreProxy, 'rack/attack/store_proxy/mem_cache_store_proxy' - autoload :RedisProxy, 'rack/attack/store_proxy/redis_proxy' - autoload :RedisStoreProxy, 'rack/attack/store_proxy/redis_store_proxy' - autoload :RedisCacheStoreProxy, 'rack/attack/store_proxy/redis_cache_store_proxy' - autoload :ActiveSupportRedisStoreProxy, 'rack/attack/store_proxy/active_support_redis_store_proxy' - autoload :Fail2Ban, 'rack/attack/fail2ban' - autoload :Allow2Ban, 'rack/attack/allow2ban' + autoload :Cache, 'rack/attack/cache' + autoload :Check, 'rack/attack/check' + autoload :Throttle, 'rack/attack/throttle' + autoload :Safelist, 'rack/attack/safelist' + autoload :Blocklist, 'rack/attack/blocklist' + autoload :Track, 'rack/attack/track' + autoload :StoreProxy, 'rack/attack/store_proxy' + autoload :DalliProxy, 'rack/attack/store_proxy/dalli_proxy' + autoload :MemCacheStoreProxy, 'rack/attack/store_proxy/mem_cache_store_proxy' + autoload :RedisProxy, 'rack/attack/store_proxy/redis_proxy' + autoload :RedisStoreProxy, 'rack/attack/store_proxy/redis_store_proxy' + autoload :RedisCacheStoreProxy, 'rack/attack/store_proxy/redis_cache_store_proxy' + autoload :ActiveSupportRedisStoreProxy, 'rack/attack/store_proxy/active_support_redis_store_proxy' + autoload :Fail2Ban, 'rack/attack/fail2ban' + autoload :Allow2Ban, 'rack/attack/allow2ban' - class << self - attr_accessor :notifier, :blocklisted_response, :throttled_response, :anonymous_blocklists, :anonymous_safelists + class << self + attr_accessor :notifier, :blocklisted_response, :throttled_response, :anonymous_blocklists, :anonymous_safelists - def safelist(name = nil, &block) - safelist = Safelist.new(name, &block) + def safelist(name = nil, &block) + safelist = Safelist.new(name, &block) - if name - safelists[name] = safelist + if name + safelists[name] = safelist + else + anonymous_safelists << safelist + end + end + + def blocklist(name = nil, &block) + blocklist = Blocklist.new(name, &block) + + if name + blocklists[name] = blocklist + else + anonymous_blocklists << blocklist + end + end + + def blocklist_ip(ip_address) + anonymous_blocklists << Blocklist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } + end + + def safelist_ip(ip_address) + anonymous_safelists << Safelist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } + end + + def throttle(name, options, &block) + throttles[name] = Throttle.new(name, options, &block) + end + + def track(name, options = {}, &block) + tracks[name] = Track.new(name, options, &block) + end + + def safelists + @safelists ||= {} + end + + def blocklists + @blocklists ||= {} + end + + def throttles + @throttles ||= {} + end + + def tracks + @tracks ||= {} + end + + def safelisted?(request) + anonymous_safelists.any? { |safelist| safelist.matched_by?(request) } || + safelists.any? { |_name, safelist| safelist.matched_by?(request) } + end + + def blocklisted?(request) + anonymous_blocklists.any? { |blocklist| blocklist.matched_by?(request) } || + blocklists.any? { |_name, blocklist| blocklist.matched_by?(request) } + end + + def throttled?(request) + throttles.any? do |_name, throttle| + throttle.matched_by?(request) + end + end + + def tracked?(request) + tracks.each_value do |track| + track.matched_by?(request) + end + end + + def instrument(request) + if notifier + event_type = request.env["rack.attack.match_type"] + notifier.instrument("#{event_type}.rack_attack", request: request) + + # Deprecated: Keeping just for backwards compatibility + notifier.instrument("rack.attack", request: request) + end + end + + def cache + @cache ||= Cache.new + end + + def clear_configuration + @safelists = {} + @blocklists = {} + @throttles = {} + @tracks = {} + self.anonymous_blocklists = [] + self.anonymous_safelists = [] + end + + def clear! + warn "[DEPRECATION] Rack::Attack.clear! is deprecated. Please use Rack::Attack.clear_configuration instead" + clear_configuration + end + end + + # Set defaults + @anonymous_blocklists = [] + @anonymous_safelists = [] + @notifier = ActiveSupport::Notifications if defined?(ActiveSupport::Notifications) + @blocklisted_response = lambda { |_env| [403, { 'Content-Type' => 'text/plain' }, ["Forbidden\n"]] } + @throttled_response = lambda do |env| + retry_after = (env['rack.attack.match_data'] || {})[:period] + [429, { 'Content-Type' => 'text/plain', 'Retry-After' => retry_after.to_s }, ["Retry later\n"]] + end + + def initialize(app) + @app = app + end + + def call(env) + env['PATH_INFO'] = PathNormalizer.normalize_path(env['PATH_INFO']) + request = Rack::Attack::Request.new(env) + + if safelisted?(request) + @app.call(env) + elsif blocklisted?(request) + self.class.blocklisted_response.call(env) + elsif throttled?(request) + self.class.throttled_response.call(env) else - anonymous_safelists << safelist + tracked?(request) + @app.call(env) end end - def blocklist(name = nil, &block) - blocklist = Blocklist.new(name, &block) - - if name - blocklists[name] = blocklist - else - anonymous_blocklists << blocklist - end - end - - def blocklist_ip(ip_address) - anonymous_blocklists << Blocklist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } - end - - def safelist_ip(ip_address) - anonymous_safelists << Safelist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } - end - - def throttle(name, options, &block) - throttles[name] = Throttle.new(name, options, &block) - end - - def track(name, options = {}, &block) - tracks[name] = Track.new(name, options, &block) - end - - def safelists - @safelists ||= {} - end - - def blocklists - @blocklists ||= {} - end - - def throttles - @throttles ||= {} - end - - def tracks - @tracks ||= {} - end - - def safelisted?(request) - anonymous_safelists.any? { |safelist| safelist.matched_by?(request) } || - safelists.any? { |_name, safelist| safelist.matched_by?(request) } - end - - def blocklisted?(request) - anonymous_blocklists.any? { |blocklist| blocklist.matched_by?(request) } || - blocklists.any? { |_name, blocklist| blocklist.matched_by?(request) } - end - - def throttled?(request) - throttles.any? do |_name, throttle| - throttle.matched_by?(request) - end - end - - def tracked?(request) - tracks.each_value do |track| - track.matched_by?(request) - end - end - - def instrument(request) - if notifier - event_type = request.env["rack.attack.match_type"] - notifier.instrument("#{event_type}.rack_attack", request: request) - - # Deprecated: Keeping just for backwards compatibility - notifier.instrument("rack.attack", request: request) - end - end - - def cache - @cache ||= Cache.new - end - - def clear_configuration - @safelists, @blocklists, @throttles, @tracks = {}, {}, {}, {} - self.anonymous_blocklists = [] - self.anonymous_safelists = [] - end - - def clear! - warn "[DEPRECATION] Rack::Attack.clear! is deprecated. Please use Rack::Attack.clear_configuration instead" - clear_configuration - end + extend Forwardable + def_delegators self, :safelisted?, :blocklisted?, :throttled?, :tracked? end - - # Set defaults - @anonymous_blocklists = [] - @anonymous_safelists = [] - @notifier = ActiveSupport::Notifications if defined?(ActiveSupport::Notifications) - @blocklisted_response = lambda { |_env| [403, { 'Content-Type' => 'text/plain' }, ["Forbidden\n"]] } - @throttled_response = lambda { |env| - retry_after = (env['rack.attack.match_data'] || {})[:period] - [429, { 'Content-Type' => 'text/plain', 'Retry-After' => retry_after.to_s }, ["Retry later\n"]] - } - - def initialize(app) - @app = app - end - - def call(env) - env['PATH_INFO'] = PathNormalizer.normalize_path(env['PATH_INFO']) - request = Rack::Attack::Request.new(env) - - if safelisted?(request) - @app.call(env) - elsif blocklisted?(request) - self.class.blocklisted_response.call(env) - elsif throttled?(request) - self.class.throttled_response.call(env) - else - tracked?(request) - @app.call(env) - end - end - - extend Forwardable - def_delegators self, :safelisted?, :blocklisted?, :throttled?, :tracked? end diff --git a/lib/rack/attack/cache.rb b/lib/rack/attack/cache.rb index 53f09f1..cfa2efa 100644 --- a/lib/rack/attack/cache.rb +++ b/lib/rack/attack/cache.rb @@ -73,7 +73,10 @@ module Rack 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" + raise( + Rack::Attack::MisconfiguredStoreError, + "Configured store #{store.class.name} doesn't respond to ##{method_name} method" + ) end end end diff --git a/lib/rack/attack/check.rb b/lib/rack/attack/check.rb index ef4ad78..4c985eb 100644 --- a/lib/rack/attack/check.rb +++ b/lib/rack/attack/check.rb @@ -5,7 +5,8 @@ module Rack class Check attr_reader :name, :block, :type def initialize(name, options = {}, &block) - @name, @block = name, block + @name = name + @block = block @type = options.fetch(:type, nil) end diff --git a/lib/rack/attack/path_normalizer.rb b/lib/rack/attack/path_normalizer.rb index 635f9f2..110ff6f 100644 --- a/lib/rack/attack/path_normalizer.rb +++ b/lib/rack/attack/path_normalizer.rb @@ -1,24 +1,26 @@ # frozen_string_literal: true -class Rack::Attack - # When using Rack::Attack with a Rails app, developers expect the request path - # to be normalized. In particular, trailing slashes are stripped. - # (See https://git.io/v0rrR for implementation.) - # - # Look for an ActionDispatch utility class that Rails folks would expect - # to normalize request paths. If unavailable, use a fallback class that - # doesn't normalize the path (as a non-Rails rack app developer expects). +module Rack + class Attack + # When using Rack::Attack with a Rails app, developers expect the request path + # to be normalized. In particular, trailing slashes are stripped. + # (See https://git.io/v0rrR for implementation.) + # + # Look for an ActionDispatch utility class that Rails folks would expect + # to normalize request paths. If unavailable, use a fallback class that + # doesn't normalize the path (as a non-Rails rack app developer expects). - module FallbackPathNormalizer - def self.normalize_path(path) - path + module FallbackPathNormalizer + def self.normalize_path(path) + path + end end - end - PathNormalizer = if defined?(::ActionDispatch::Journey::Router::Utils) - # For Rails apps - ::ActionDispatch::Journey::Router::Utils - else - FallbackPathNormalizer - end + PathNormalizer = if defined?(::ActionDispatch::Journey::Router::Utils) + # For Rails apps + ::ActionDispatch::Journey::Router::Utils + else + FallbackPathNormalizer + 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 index a18d081..68f0326 100644 --- a/lib/rack/attack/store_proxy/active_support_redis_store_proxy.rb +++ b/lib/rack/attack/store_proxy/active_support_redis_store_proxy.rb @@ -7,7 +7,9 @@ module Rack module StoreProxy class ActiveSupportRedisStoreProxy < SimpleDelegator def self.handle?(store) - defined?(::Redis) && defined?(::ActiveSupport::Cache::RedisStore) && store.is_a?(::ActiveSupport::Cache::RedisStore) + defined?(::Redis) && + defined?(::ActiveSupport::Cache::RedisStore) && + store.is_a?(::ActiveSupport::Cache::RedisStore) end def increment(name, amount = 1, options = {}) diff --git a/lib/rack/attack/store_proxy/mem_cache_store_proxy.rb b/lib/rack/attack/store_proxy/mem_cache_store_proxy.rb index eebfed7..f8c036c 100644 --- a/lib/rack/attack/store_proxy/mem_cache_store_proxy.rb +++ b/lib/rack/attack/store_proxy/mem_cache_store_proxy.rb @@ -7,7 +7,9 @@ module Rack module StoreProxy class MemCacheStoreProxy < SimpleDelegator def self.handle?(store) - defined?(::Dalli) && defined?(::ActiveSupport::Cache::MemCacheStore) && store.is_a?(::ActiveSupport::Cache::MemCacheStore) + defined?(::Dalli) && + defined?(::ActiveSupport::Cache::MemCacheStore) && + store.is_a?(::ActiveSupport::Cache::MemCacheStore) end def write(name, value, options = {}) diff --git a/lib/rack/attack/throttle.rb b/lib/rack/attack/throttle.rb index 672140a..c222325 100644 --- a/lib/rack/attack/throttle.rb +++ b/lib/rack/attack/throttle.rb @@ -7,11 +7,12 @@ module Rack attr_reader :name, :limit, :period, :block, :type def initialize(name, options, &block) - @name, @block = name, block + @name = name + @block = block MANDATORY_OPTIONS.each do |opt| raise ArgumentError, "Must pass #{opt.inspect} option" unless options[opt] end - @limit = options[:limit] + @limit = options[:limit] @period = options[:period].respond_to?(:call) ? options[:period] : options[:period].to_i @type = options.fetch(:type, :throttle) end diff --git a/lib/rack/attack/track.rb b/lib/rack/attack/track.rb index dc7afb8..2b3424c 100644 --- a/lib/rack/attack/track.rb +++ b/lib/rack/attack/track.rb @@ -8,11 +8,12 @@ module Rack def initialize(name, options = {}, &block) options[:type] = :track - if options[:limit] && options[:period] - @filter = Throttle.new(name, options, &block) - else - @filter = Check.new(name, options, &block) - end + @filter = + if options[:limit] && options[:period] + Throttle.new(name, options, &block) + else + Check.new(name, options, &block) + end end def matched_by?(request) diff --git a/rack-attack.gemspec b/rack-attack.gemspec index 7312881..96aa458 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -1,7 +1,7 @@ # frozen_string_literal: true -lib = File.expand_path('../lib/', __FILE__) -$:.unshift lib unless $:.include?(lib) +lib = File.expand_path('lib', __dir__) +$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'rack/attack/version' @@ -18,7 +18,7 @@ Gem::Specification.new do |s| s.homepage = 'https://github.com/kickstarter/rack-attack' s.rdoc_options = ["--charset=UTF-8"] s.require_paths = ["lib"] - s.summary = %q{Block & throttle abusive requests} + s.summary = 'Block & throttle abusive requests' s.test_files = Dir.glob("spec/**/*") s.metadata = { @@ -37,7 +37,8 @@ Gem::Specification.new do |s| s.add_development_dependency "minitest-stub-const", "~> 0.6" s.add_development_dependency 'rack-test', "~> 1.0" s.add_development_dependency 'rake', "~> 12.3" - s.add_development_dependency "rubocop", "0.67.2" + s.add_development_dependency "rubocop", "0.74.0" + s.add_development_dependency "rubocop-performance", "~> 1.4.1" s.add_development_dependency "timecop", "~> 0.9.1" # byebug only works with MRI 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 c477380..8344b6e 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 @@ -15,8 +15,6 @@ if defined?(::ConnectionPool) && defined?(::Dalli) Rack::Attack.cache.store.clear end - it_works_for_cache_backed_features(fetch_from_store: ->(key) { - Rack::Attack.cache.store.read(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_pooled_spec.rb b/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb index 3bfc2cf..9c26e8d 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 @@ -2,7 +2,13 @@ require_relative "../../spec_helper" -if defined?(::ConnectionPool) && defined?(::Redis) && Gem::Version.new(::Redis::VERSION) >= Gem::Version.new("4") && defined?(::ActiveSupport::Cache::RedisCacheStore) +should_run = + defined?(::ConnectionPool) && + defined?(::Redis) && + Gem::Version.new(::Redis::VERSION) >= Gem::Version.new("4") && + defined?(::ActiveSupport::Cache::RedisCacheStore) + +if should_run require_relative "../../support/cache_store_helper" require "timecop" 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 565ac29..f595ec2 100644 --- a/spec/acceptance/stores/active_support_redis_cache_store_spec.rb +++ b/spec/acceptance/stores/active_support_redis_cache_store_spec.rb @@ -2,7 +2,12 @@ require_relative "../../spec_helper" -if defined?(::Redis) && Gem::Version.new(::Redis::VERSION) >= Gem::Version.new("4") && defined?(::ActiveSupport::Cache::RedisCacheStore) +should_run = + defined?(::Redis) && + Gem::Version.new(::Redis::VERSION) >= Gem::Version.new("4") && + defined?(::ActiveSupport::Cache::RedisCacheStore) + +if should_run require_relative "../../support/cache_store_helper" require "timecop" diff --git a/spec/acceptance/stores/connection_pool_dalli_client_spec.rb b/spec/acceptance/stores/connection_pool_dalli_client_spec.rb index 5689fbd..d532a29 100644 --- a/spec/acceptance/stores/connection_pool_dalli_client_spec.rb +++ b/spec/acceptance/stores/connection_pool_dalli_client_spec.rb @@ -17,8 +17,8 @@ if defined?(::Dalli) && defined?(::ConnectionPool) 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) } - }) + it_works_for_cache_backed_features( + fetch_from_store: ->(key) { Rack::Attack.cache.store.with { |client| client.fetch(key) } } + ) end end diff --git a/spec/rack_attack_spec.rb b/spec/rack_attack_spec.rb index d99e89d..39a0928 100644 --- a/spec/rack_attack_spec.rb +++ b/spec/rack_attack_spec.rb @@ -22,9 +22,9 @@ describe 'Rack::Attack' do Rack::Attack.blocklist("ip #{@bad_ip}") { |req| req.ip == @bad_ip } end - it('has a blocklist') { + it 'has a blocklist' do Rack::Attack.blocklists.key?("ip #{@bad_ip}").must_equal true - } + end describe "a bad request" do before { get '/', {}, 'REMOTE_ADDR' => @bad_ip }