From 97a43f7e660ff244391548965f8c0661961db385 Mon Sep 17 00:00:00 2001 From: Carsten Zimmermann Date: Thu, 6 Feb 2014 21:32:51 +0100 Subject: [PATCH 1/3] Return 403 Forbidden instead of 401 401 Unauthorized suggests that the requests can be retried with appropriate credentials. 403 explicitly states that the request should not be repeated. See #41 --- lib/rack/attack.rb | 2 +- spec/allow2ban_spec.rb | 4 ++-- spec/fail2ban_spec.rb | 8 ++++---- spec/rack_attack_spec.rb | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index c1f9528..0265f4b 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -40,7 +40,7 @@ module Rack::Attack # Set defaults @notifier ||= ActiveSupport::Notifications if defined?(ActiveSupport::Notifications) - @blacklisted_response ||= lambda {|env| [401, {}, ["Unauthorized\n"]] } + @blacklisted_response ||= lambda {|env| [403, {}, ["Unauthorized\n"]] } @throttled_response ||= lambda {|env| retry_after = env['rack.attack.match_data'][:period] rescue nil [429, {'Retry-After' => retry_after.to_s}, ["Retry later\n"]] diff --git a/spec/allow2ban_spec.rb b/spec/allow2ban_spec.rb index 569c213..c6a6836 100644 --- a/spec/allow2ban_spec.rb +++ b/spec/allow2ban_spec.rb @@ -83,7 +83,7 @@ describe 'Rack::Attack.Allow2Ban' do end it 'fails' do - last_response.status.must_equal 401 + last_response.status.must_equal 403 end it 'does not increase fail count' do @@ -103,7 +103,7 @@ describe 'Rack::Attack.Allow2Ban' do end it 'fails' do - last_response.status.must_equal 401 + last_response.status.must_equal 403 end it 'does not increase fail count' do diff --git a/spec/fail2ban_spec.rb b/spec/fail2ban_spec.rb index d35755c..2a4f842 100644 --- a/spec/fail2ban_spec.rb +++ b/spec/fail2ban_spec.rb @@ -24,7 +24,7 @@ describe 'Rack::Attack.Fail2Ban' do describe 'when not at maxretry' do before { get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' } it 'fails' do - last_response.status.must_equal 401 + last_response.status.must_equal 403 end it 'increases fail count' do @@ -46,7 +46,7 @@ describe 'Rack::Attack.Fail2Ban' do end it 'fails' do - last_response.status.must_equal 401 + last_response.status.must_equal 403 end it 'increases fail count' do @@ -83,7 +83,7 @@ describe 'Rack::Attack.Fail2Ban' do end it 'fails' do - last_response.status.must_equal 401 + last_response.status.must_equal 403 end it 'does not increase fail count' do @@ -103,7 +103,7 @@ describe 'Rack::Attack.Fail2Ban' do end it 'fails' do - last_response.status.must_equal 401 + last_response.status.must_equal 403 end it 'does not increase fail count' do diff --git a/spec/rack_attack_spec.rb b/spec/rack_attack_spec.rb index 681edaf..fc13c25 100644 --- a/spec/rack_attack_spec.rb +++ b/spec/rack_attack_spec.rb @@ -15,7 +15,7 @@ describe 'Rack::Attack' do before { get '/', {}, 'REMOTE_ADDR' => @bad_ip } it "should return a blacklist response" do get '/', {}, 'REMOTE_ADDR' => @bad_ip - last_response.status.must_equal 401 + last_response.status.must_equal 403 end it "should tag the env" do last_request.env['rack.attack.matched'].must_equal "ip #{@bad_ip}" From 355a6fbce6a4073cb4f8b0c020b3228e912b5b2d Mon Sep 17 00:00:00 2001 From: Carsten Zimmermann Date: Thu, 6 Feb 2014 21:35:28 +0100 Subject: [PATCH 2/3] Update readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ecf15d3..24f384d 100644 --- a/README.md +++ b/README.md @@ -198,7 +198,7 @@ Customize the response of blacklisted and throttled requests using an object tha ```ruby Rack::Attack.blacklisted_response = lambda do |env| # Using 503 because it may make attacker think that they have successfully - # DOSed the site. Rack::Attack returns 401 for blacklists by default + # DOSed the site. Rack::Attack returns 403 for blacklists by default [ 503, {}, ['Blocked']] end From 1095f852421751e41e64cc87240fd3b3fc38bad6 Mon Sep 17 00:00:00 2001 From: Carsten Zimmermann Date: Thu, 6 Feb 2014 23:29:44 +0100 Subject: [PATCH 3/3] Change response body to 'Forbidden' --- lib/rack/attack.rb | 2 +- spec/rack_attack_spec.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 0265f4b..91baafd 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -40,7 +40,7 @@ module Rack::Attack # Set defaults @notifier ||= ActiveSupport::Notifications if defined?(ActiveSupport::Notifications) - @blacklisted_response ||= lambda {|env| [403, {}, ["Unauthorized\n"]] } + @blacklisted_response ||= lambda {|env| [403, {}, ["Forbidden\n"]] } @throttled_response ||= lambda {|env| retry_after = env['rack.attack.match_data'][:period] rescue nil [429, {'Retry-After' => retry_after.to_s}, ["Retry later\n"]] diff --git a/spec/rack_attack_spec.rb b/spec/rack_attack_spec.rb index fc13c25..89bfc51 100644 --- a/spec/rack_attack_spec.rb +++ b/spec/rack_attack_spec.rb @@ -16,6 +16,7 @@ describe 'Rack::Attack' do it "should return a blacklist response" do get '/', {}, 'REMOTE_ADDR' => @bad_ip last_response.status.must_equal 403 + last_response.body.must_equal "Forbidden\n" end it "should tag the env" do last_request.env['rack.attack.matched'].must_equal "ip #{@bad_ip}"