Give clearer error message for misconfigured cache store for allow/fa…

…il2ban

CHANGELOG.md

@@ -3,6 +3,10 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Added
+
+- Throw helpful error message when using `allow2ban` but cache store is misconfigured ([#315](https://github.com/kickstarter/rack-attack/issues/315))
+- Throw helpful error message when using `fail2ban` but cache store is misconfigured ([#315](https://github.com/kickstarter/rack-attack/issues/315))
 
 ## [5.1.0] - 2018-03-10
 

lib/rack/attack/cache.rb

@@ -20,6 +20,9 @@ def count(unprefixed_key, period)
       end
 
       def read(unprefixed_key)
+        enforce_store_presence!
+        enforce_store_method_presence!(:read)
+
         store.read("#{prefix}:#{unprefixed_key}")
       end
 
@@ -46,18 +49,29 @@ 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
-        elsif !store.respond_to?(:increment)
-          raise Rack::Attack::MisconfiguredStoreError, "Store needs to respond to #increment"
-        else
-          result = store.increment(key, 1, :expires_in => expires_in)
-
-          # NB: Some stores return nil when incrementing uninitialized values
-          if result.nil?
-            store.write(key, 1, :expires_in => expires_in)
-          end
-          result || 1
+        end
+      end
+
+      def enforce_store_method_presence!(method_name)
+        if !store.respond_to?(method_name)
+          raise Rack::Attack::MisconfiguredStoreError, "Store needs to respond to ##{method_name}"
         end
       end
     end

spec/acceptance/cache_store_config_for_allow2ban_spec.rb

@@ -9,18 +9,64 @@
     end
   end
 
-  it "gives error if no store was configured" do
-    assert_raises do
-      get "/"
+  it "gives semantic error if no store was configured" do
+    assert_raises(Rack::Attack::MissingStoreError) do
+      get "/scarce-resource"
     end
   end
 
-  it "gives error if incompatible store was configured" do
-    Rack::Attack.cache.store = Object.new
+  it "gives semantic error if store is missing #read method" do
+    basic_store_class = Class.new do
+      def write(key, value)
+      end
+
+      def increment(key, count, options = {})
+      end
+    end
+
+    Rack::Attack.cache.store = basic_store_class.new
+
+    raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
+      get "/scarce-resource"
+    end
+
+    assert_equal "Store needs to respond to #read", raised_exception.message
+  end
+
+  it "gives semantic error if store is missing #write method" do
+    basic_store_class = Class.new do
+      def read(key)
+      end
+
+      def increment(key, count, options = {})
+      end
+    end
+
+    Rack::Attack.cache.store = basic_store_class.new
+
+    raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
+      get "/scarce-resource"
+    end
+
+    assert_equal "Store needs to respond to #write", raised_exception.message
+  end
+
+  it "gives semantic error if store is missing #increment method" do
+    basic_store_class = Class.new do
+      def read(key)
+      end
 
-    assert_raises do
-      get "/"
+      def write(key, value)
+      end
     end
+
+    Rack::Attack.cache.store = basic_store_class.new
+
+    raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
+      get "/scarce-resource"
+    end
+
+    assert_equal "Store needs to respond to #increment", raised_exception.message
   end
 
   it "works with any object that responds to #read, #write and #increment" do

spec/acceptance/cache_store_config_for_fail2ban_spec.rb

@@ -9,18 +9,64 @@
     end
   end
 
-  it "gives error if no store was configured" do
-    assert_raises do
-      get "/"
+  it "gives semantic error if no store was configured" do
+    assert_raises(Rack::Attack::MissingStoreError) do
+      get "/private-place"
     end
   end
 
-  it "gives error if incompatible store was configured" do
-    Rack::Attack.cache.store = Object.new
+  it "gives semantic error if store is missing #read method" do
+    basic_store_class = Class.new do
+      def write(key, value)
+      end
+
+      def increment(key, count, options = {})
+      end
+    end
+
+    Rack::Attack.cache.store = basic_store_class.new
+
+    raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
+      get "/private-place"
+    end
+
+    assert_equal "Store needs to respond to #read", raised_exception.message
+  end
+
+  it "gives semantic error if store is missing #write method" do
+    basic_store_class = Class.new do
+      def read(key)
+      end
+
+      def increment(key, count, options = {})
+      end
+    end
+
+    Rack::Attack.cache.store = basic_store_class.new
+
+    raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
+      get "/private-place"
+    end
+
+    assert_equal "Store needs to respond to #write", raised_exception.message
+  end
+
+  it "gives semantic error if store is missing #increment method" do
+    basic_store_class = Class.new do
+      def read(key)
+      end
 
-    assert_raises do
-      get "/"
+      def write(key, value)
+      end
     end
+
+    Rack::Attack.cache.store = basic_store_class.new
+
+    raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
+      get "/private-place"
+    end
+
+    assert_equal "Store needs to respond to #increment", raised_exception.message
   end
 
   it "works with any object that responds to #read, #write and #increment" do