Merge pull request #406 from grzuy/optional_name

Make blocklist/safelist name argument optional

lib/rack/attack.rb

@@ -27,26 +27,36 @@ class Rack::Attack
   autoload :Allow2Ban,            'rack/attack/allow2ban'
 
   class << self
-    attr_accessor :notifier, :blocklisted_response, :throttled_response
+    attr_accessor :notifier, :blocklisted_response, :throttled_response, :anonymous_blocklists, :anonymous_safelists
 
-    def safelist(name, &block)
-      self.safelists[name] = Safelist.new(name, block)
+    def safelist(name = nil, &block)
+      safelist = Safelist.new(name, block)
+
+      if name
+        self.safelists[name] = safelist
+      else
+        anonymous_safelists << safelist
+      end
     end
 
-    def blocklist(name, &block)
-      self.blocklists[name] = Blocklist.new(name, block)
+    def blocklist(name = nil, &block)
+      blocklist = Blocklist.new(name, block)
+
+      if name
+        self.blocklists[name] = blocklist
+      else
+        anonymous_blocklists << blocklist
+      end
     end
 
     def blocklist_ip(ip_address)
-      @ip_blocklists ||= []
       ip_blocklist_proc = lambda { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) }
-      @ip_blocklists << Blocklist.new(nil, ip_blocklist_proc)
+      anonymous_blocklists << Blocklist.new(nil, ip_blocklist_proc)
     end
 
     def safelist_ip(ip_address)
-      @ip_safelists ||= []
       ip_safelist_proc = lambda { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) }
-      @ip_safelists << Safelist.new(nil, ip_safelist_proc)
+      anonymous_safelists << Safelist.new(nil, ip_safelist_proc)
     end
 
     def throttle(name, options, &block)
@@ -66,12 +76,12 @@ class Rack::Attack
     def tracks;     @tracks     ||= {}; end
 
     def safelisted?(request)
-      ip_safelists.any? { |safelist| safelist.matched_by?(request) } ||
+      anonymous_safelists.any? { |safelist| safelist.matched_by?(request) } ||
         safelists.any? { |_name, safelist| safelist.matched_by?(request) }
     end
 
     def blocklisted?(request)
-      ip_blocklists.any? { |blocklist| blocklist.matched_by?(request) } ||
+      anonymous_blocklists.any? { |blocklist| blocklist.matched_by?(request) } ||
         blocklists.any? { |_name, blocklist| blocklist.matched_by?(request) }
     end
 
@@ -103,27 +113,19 @@ class Rack::Attack
 
     def clear_configuration
       @safelists, @blocklists, @throttles, @tracks = {}, {}, {}, {}
-      @ip_blocklists = []
-      @ip_safelists = []
+      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
-
-    private
-
-    def ip_blocklists
-      @ip_blocklists ||= []
-    end
-
-    def ip_safelists
-      @ip_safelists ||= []
-    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 { |env|

spec/acceptance/blocking_spec.rb

@@ -3,6 +3,46 @@
 require_relative "../spec_helper"
 
 describe "#blocklist" do
+  before do
+    Rack::Attack.blocklist do |request|
+      request.ip == "1.2.3.4"
+    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 "succeeds if blocklist condition is false" do
+    get "/", {}, "REMOTE_ADDR" => "5.6.7.8"
+
+    assert_equal 200, last_response.status
+  end
+
+  it "notifies when the request is blocked" do
+    notification_matched = nil
+    notification_type = nil
+
+    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+      notification_matched = payload[:request].env["rack.attack.matched"]
+      notification_type = payload[:request].env["rack.attack.match_type"]
+    end
+
+    get "/", {}, "REMOTE_ADDR" => "5.6.7.8"
+
+    assert_nil notification_matched
+    assert_nil notification_type
+
+    get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+
+    assert_nil notification_matched
+    assert_equal :blocklist, notification_type
+  end
+end
+
+describe "#blocklist with name" do
   before do
     Rack::Attack.blocklist("block 1.2.3.4") do |request|
       request.ip == "1.2.3.4"

spec/acceptance/safelisting_spec.rb

@@ -3,6 +3,58 @@
 require_relative "../spec_helper"
 
 describe "#safelist" do
+  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
+
+  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 "succeeds if blocklist condition is false and safelist is false" do
+    get "/", {}, "REMOTE_ADDR" => "5.6.7.8"
+
+    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"
+
+    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"
+
+    assert_equal 200, last_response.status
+  end
+
+  it "notifies when the request is safe" do
+    notification_matched = nil
+    notification_type = nil
+
+    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+      notification_matched = payload[:request].env["rack.attack.matched"]
+      notification_type = payload[:request].env["rack.attack.match_type"]
+    end
+
+    get "/safe_space", {}, "REMOTE_ADDR" => "1.2.3.4"
+
+    assert_equal 200, last_response.status
+    assert_nil notification_matched
+    assert_equal :safelist, notification_type
+  end
+end
+
+describe "#safelist with name" do
   before do
     Rack::Attack.blocklist("block 1.2.3.4") do |request|
       request.ip == "1.2.3.4"