Merge branch 'master' into move_all_deps_to_gemspec

.travis.yml

@@ -11,11 +11,17 @@ before_install:
   - gem install bundler
 
 gemfile:
+  - gemfiles/rails_5_2.gemfile
   - gemfiles/rails_5_1.gemfile
   - gemfiles/rails_5_0.gemfile
   - gemfiles/rails_4_2.gemfile
   - gemfiles/dalli2.gemfile
 
+matrix:
+  allow_failures:
+    - gemfile: gemfiles/rails_5_2.gemfile
+  fast_finish: true
+
 services:
   - redis
   - memcached

Appraisals

@@ -1,3 +1,8 @@
+appraise 'rails_5-2' do
+  gem 'activesupport', '~> 5.2.0.a'
+  gem 'actionpack', '~> 5.2.0.a'
+end
+
 appraise 'rails_5-1' do
   gem 'activesupport', '~> 5.1.0'
   gem 'actionpack', '~> 5.1.0'

README.md

@@ -40,9 +40,9 @@ Or for Rackup files:
 use Rack::Attack
 ```
 
-Add a `rack-attack.rb` file to `config/initializers/`:
+Add a `rack_attack.rb` file to `config/initializers/`:
 ```ruby
-# In config/initializers/rack-attack.rb
+# In config/initializers/rack_attack.rb
 class Rack::Attack
   # your custom configuration...
 end
@@ -237,7 +237,8 @@ Rack::Attack.throttled_response = lambda do |env|
   # NB: you have access to the name and other data about the matched throttle
   #  env['rack.attack.matched'],
   #  env['rack.attack.match_type'],
-  #  env['rack.attack.match_data']
+  #  env['rack.attack.match_data'],
+  #  env['rack.attack.match_discriminator']
 
   # Using 503 because it may make attacker think that they have successfully
   # DOSed the site. Rack::Attack returns 429 for throttling by default

Rakefile

@@ -11,9 +11,13 @@ namespace :test do
   Rake::TestTask.new(:integration) do |t|
     t.pattern = "spec/integration/*_spec.rb"
   end
+
+  Rake::TestTask.new(:acceptance) do |t|
+    t.pattern = "spec/acceptance/*_spec.rb"
+  end
 end
 
 desc 'Run tests'
-task :test => %w[test:units test:integration]
+task :test => %w[test:units test:integration test:acceptance]
 
 task :default => :test

gemfiles/rails_5_2.gemfile

@@ -0,0 +1,14 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "activesupport", "~> 5.2.0.a"
+gem "actionpack", "~> 5.2.0.a"
+
+group :development do
+  gem "pry"
+  gem "guard"
+  gem "guard-minitest"
+end
+
+gemspec path: "../"

rack-attack.gemspec

@@ -33,7 +33,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'dalli'
   s.add_development_dependency 'connection_pool'
   s.add_development_dependency 'memcache-client'
-
+  s.add_development_dependency "timecop"
   s.add_development_dependency 'pry'
   s.add_development_dependency 'guard-minitest'
   # Need to explicitly depend on guard because guard-minitest doesn't declare

spec/acceptance/blocking_spec.rb

@@ -0,0 +1,21 @@
+require_relative "../spec_helper"
+
+describe "#blocklist" do
+  before do
+    Rack::Attack.blocklist("block 1.2.3.4") 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
+end

spec/acceptance/safelisting_spec.rb

@@ -0,0 +1,37 @@
+require_relative "../spec_helper"
+
+describe "#safelist" do
+  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"
+    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
+end

spec/acceptance/throttling_spec.rb

@@ -0,0 +1,30 @@
+require_relative "../spec_helper"
+require "timecop"
+
+describe "#throttle" do
+  it "allows one request per minute by IP" do
+    Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
+
+    Rack::Attack.throttle("by ip", limit: 1, period: 60) do |request|
+      request.ip
+    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
+
+    get "/", {}, "REMOTE_ADDR" => "5.6.7.8"
+
+    assert_equal 200, last_response.status
+
+    Timecop.travel(60) do
+      get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+
+      assert_equal 200, last_response.status
+    end
+  end
+end

spec/rack_attack_request_spec.rb

@@ -14,6 +14,6 @@ def remote_ip
       end
     end
 
-    allow_ok_requests
+    it_allows_ok_requests
   end
 end

spec/rack_attack_spec.rb

@@ -1,7 +1,7 @@
 require_relative 'spec_helper'
 
 describe 'Rack::Attack' do
-  allow_ok_requests
+  it_allows_ok_requests
 
   describe 'normalizing paths' do
     before do
@@ -44,7 +44,7 @@
         last_request.env['rack.attack.match_type'].must_equal :blocklist
       end
 
-      allow_ok_requests
+      it_allows_ok_requests
     end
 
     describe "and safelist" do

spec/rack_attack_throttle_spec.rb

@@ -9,7 +9,7 @@
 
   it('should have a throttle') { Rack::Attack.throttles.key?('ip/sec') }
 
-  allow_ok_requests
+  it_allows_ok_requests
 
   describe 'a single request' do
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
@@ -54,7 +54,7 @@
     Rack::Attack.throttle('ip/sec', :limit => lambda { |req| 1 }, :period => @period) { |req| req.ip }
   end
 
-  allow_ok_requests
+  it_allows_ok_requests
 
   describe 'a single request' do
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
@@ -78,7 +78,7 @@
     Rack::Attack.throttle('ip/sec', :limit => lambda { |req| 1 }, :period => lambda { |req| @period }) { |req| req.ip }
   end
 
-  allow_ok_requests
+  it_allows_ok_requests
 
   describe 'a single request' do
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
@@ -102,7 +102,7 @@
     Rack::Attack.throttle('ip/sec', :limit => 1, :period => @period) { |_| nil }
   end
 
-  allow_ok_requests
+  it_allows_ok_requests
 
   describe 'a single request' do
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }

spec/rack_attack_track_spec.rb

@@ -19,7 +19,7 @@ def self.check
     Rack::Attack.track("everything"){ |req| true }
   end
 
-  allow_ok_requests
+  it_allows_ok_requests
 
   it "should tag the env" do
     get '/'

spec/spec_helper.rb

@@ -23,12 +23,16 @@ class MiniTest::Spec
 
   def app
     Rack::Builder.new {
+      # Use Rack::Lint to test that rack-attack is complying with the rack spec
+      use Rack::Lint
       use Rack::Attack
+      use Rack::Lint
+
       run lambda {|env| [200, {}, ['Hello World']]}
     }.to_app
   end
 
-  def self.allow_ok_requests
+  def self.it_allows_ok_requests
     it "must allow ok requests" do
       get '/', {}, 'REMOTE_ADDR' => '127.0.0.1'
       last_response.status.must_equal 200