From ca739946ce66aae2788530bc354f71eb638900ca Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Thu, 25 Jan 2018 10:53:47 -0300 Subject: [PATCH 1/6] Attempt to make it easier to understand that the method is making assertions --- spec/rack_attack_request_spec.rb | 2 +- spec/rack_attack_spec.rb | 4 ++-- spec/rack_attack_throttle_spec.rb | 8 ++++---- spec/rack_attack_track_spec.rb | 2 +- spec/spec_helper.rb | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/rack_attack_request_spec.rb b/spec/rack_attack_request_spec.rb index cc617ae..41dfd82 100644 --- a/spec/rack_attack_request_spec.rb +++ b/spec/rack_attack_request_spec.rb @@ -14,6 +14,6 @@ describe 'Rack::Attack' do end end - allow_ok_requests + it_allows_ok_requests end end diff --git a/spec/rack_attack_spec.rb b/spec/rack_attack_spec.rb index e2232b3..af5963b 100644 --- a/spec/rack_attack_spec.rb +++ b/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 @@ -45,7 +45,7 @@ describe 'Rack::Attack' do last_request.env['rack.attack.match_type'].must_equal :blocklist end - allow_ok_requests + it_allows_ok_requests end describe "and safelist" do diff --git a/spec/rack_attack_throttle_spec.rb b/spec/rack_attack_throttle_spec.rb index 28ab26c..df6235f 100644 --- a/spec/rack_attack_throttle_spec.rb +++ b/spec/rack_attack_throttle_spec.rb @@ -9,7 +9,7 @@ describe 'Rack::Attack.throttle' do 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 @@ describe 'Rack::Attack.throttle with limit as proc' do 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 @@ describe 'Rack::Attack.throttle with period as proc' do 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 @@ describe 'Rack::Attack.throttle with block retuning nil' do 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' } diff --git a/spec/rack_attack_track_spec.rb b/spec/rack_attack_track_spec.rb index 6bd38f3..cb2f2c9 100644 --- a/spec/rack_attack_track_spec.rb +++ b/spec/rack_attack_track_spec.rb @@ -19,7 +19,7 @@ describe 'Rack::Attack.track' do Rack::Attack.track("everything"){ |req| true } end - allow_ok_requests + it_allows_ok_requests it "should tag the env" do get '/' diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b7d535f..ccdda3d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -28,7 +28,7 @@ class MiniTest::Spec }.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 From 980633e1a91780e2ac332542d29427ba9699f821 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Thu, 25 Jan 2018 15:58:18 -0300 Subject: [PATCH 2/6] Adds acceptance-oriented tests --- Rakefile | 6 ++++- rack-attack.gemspec | 1 + spec/acceptance/blocking_spec.rb | 21 ++++++++++++++++ spec/acceptance/safelisting_spec.rb | 37 +++++++++++++++++++++++++++++ spec/acceptance/throttling_spec.rb | 30 +++++++++++++++++++++++ 5 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 spec/acceptance/blocking_spec.rb create mode 100644 spec/acceptance/safelisting_spec.rb create mode 100644 spec/acceptance/throttling_spec.rb diff --git a/Rakefile b/Rakefile index 300ec21..475bece 100644 --- a/Rakefile +++ b/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 diff --git a/rack-attack.gemspec b/rack-attack.gemspec index 1fcdd42..95a833a 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -33,4 +33,5 @@ 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" end diff --git a/spec/acceptance/blocking_spec.rb b/spec/acceptance/blocking_spec.rb new file mode 100644 index 0000000..2e9a562 --- /dev/null +++ b/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 diff --git a/spec/acceptance/safelisting_spec.rb b/spec/acceptance/safelisting_spec.rb new file mode 100644 index 0000000..743d97e --- /dev/null +++ b/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 diff --git a/spec/acceptance/throttling_spec.rb b/spec/acceptance/throttling_spec.rb new file mode 100644 index 0000000..e157f50 --- /dev/null +++ b/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 From f27432df91f77a543430224d5ebdd7330eb8278b Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Tue, 30 Jan 2018 10:08:20 -0300 Subject: [PATCH 3/6] Use Rack::Lint in tests to check any change continues complying with the rack spec --- spec/spec_helper.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b7d535f..3974343 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -23,7 +23,11 @@ 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 From 53095231cc8b87dc5ecc0c437c274e5b9d018d3f Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Thu, 1 Feb 2018 15:57:18 -0300 Subject: [PATCH 4/6] Test against rails 5.2.0.rc to get early feedback without causing TravisCI build failures --- .travis.yml | 6 ++++++ Appraisals | 5 +++++ gemfiles/rails_5_2.gemfile | 14 ++++++++++++++ 3 files changed, 25 insertions(+) create mode 100644 gemfiles/rails_5_2.gemfile diff --git a/.travis.yml b/.travis.yml index 9eb7762..a6a253f 100644 --- a/.travis.yml +++ b/.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 diff --git a/Appraisals b/Appraisals index 56fadfc..296b89c 100644 --- a/Appraisals +++ b/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' diff --git a/gemfiles/rails_5_2.gemfile b/gemfiles/rails_5_2.gemfile new file mode 100644 index 0000000..f1948ff --- /dev/null +++ b/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: "../" From 70a4c1f933229f98a0d20d549abe26159280f0c7 Mon Sep 17 00:00:00 2001 From: jun_tanaka555 Date: Sun, 4 Feb 2018 16:18:54 +0900 Subject: [PATCH 5/6] fix rubocop complains about initializer file name --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b5381e7..61f9cf0 100644 --- a/README.md +++ b/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 From 993d724b64b54c18f8f7bbe3d40b3332457b1813 Mon Sep 17 00:00:00 2001 From: Alex Taylor Date: Mon, 5 Mar 2018 17:21:36 -0800 Subject: [PATCH 6/6] Mention 'match_discriminator' in README.md --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b5381e7..12377b8 100644 --- a/README.md +++ b/README.md @@ -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