Commit 9bb00cd7 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'get-monkey-off-my-rack-attack' into 'master'

Remove Rack Attack monkey patches and bump to version 4.3.0

I finally got these monkey patches into Rack Attack v4.3.0, so GitLab no longer needs them. Hooray!

See: https://github.com/kickstarter/rack-attack/pull/128

See merge request !693
parents d9f4dfe9 3b22cfe6
...@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 7.12.0 (unreleased) v 7.12.0 (unreleased)
- Add web hook support for note events (Stan Hu) - Add web hook support for note events (Stan Hu)
- Disable "New Issue" and "New Merge Request" buttons when features are disabled in project settings (Stan Hu) - Disable "New Issue" and "New Merge Request" buttons when features are disabled in project settings (Stan Hu)
- Remove Rack Attack monkey patches and bump to version 4.3.0 (Stan Hu)
- Allow to configure location of the `.gitlab_shell_secret` file. (Jakub Jirutka) - Allow to configure location of the `.gitlab_shell_secret` file. (Jakub Jirutka)
- Disabled expansion of top/bottom blobs for new file diffs - Disabled expansion of top/bottom blobs for new file diffs
- Update Asciidoctor gem to version 1.5.2. (Jakub Jirutka) - Update Asciidoctor gem to version 1.5.2. (Jakub Jirutka)
......
...@@ -172,7 +172,7 @@ gem "underscore-rails", "~> 1.4.4" ...@@ -172,7 +172,7 @@ gem "underscore-rails", "~> 1.4.4"
gem "sanitize", '~> 2.0' gem "sanitize", '~> 2.0'
# Protect against bruteforcing # Protect against bruteforcing
gem "rack-attack" gem "rack-attack", '~> 4.3.0'
# Ace editor # Ace editor
gem 'ace-rails-ap' gem 'ace-rails-ap'
......
...@@ -421,7 +421,7 @@ GEM ...@@ -421,7 +421,7 @@ GEM
rack (1.5.2) rack (1.5.2)
rack-accept (0.4.5) rack-accept (0.4.5)
rack (>= 0.4) rack (>= 0.4)
rack-attack (4.2.0) rack-attack (4.3.0)
rack rack
rack-cors (0.2.9) rack-cors (0.2.9)
rack-mini-profiler (0.9.0) rack-mini-profiler (0.9.0)
...@@ -764,7 +764,7 @@ DEPENDENCIES ...@@ -764,7 +764,7 @@ DEPENDENCIES
poltergeist (~> 1.5.1) poltergeist (~> 1.5.1)
pry-rails pry-rails
quiet_assets (~> 1.0.1) quiet_assets (~> 1.0.1)
rack-attack rack-attack (~> 4.3.0)
rack-cors rack-cors
rack-mini-profiler rack-mini-profiler
rack-oauth2 (~> 1.0.5) rack-oauth2 (~> 1.0.5)
......
require_relative 'rack_attack_helpers'
require_relative 'shell_env' require_relative 'shell_env'
module Grack module Grack
......
# rack-attack v4.2.0 doesn't yet support clearing of keys.
# Taken from https://github.com/kickstarter/rack-attack/issues/113
class Rack::Attack::Allow2Ban
def self.reset(discriminator, options)
findtime = options[:findtime] or raise ArgumentError, "Must pass findtime option"
cache.reset_count("#{key_prefix}:count:#{discriminator}", findtime)
cache.delete("#{key_prefix}:ban:#{discriminator}")
end
end
class Rack::Attack::Cache
def reset_count(unprefixed_key, period)
epoch_time = Time.now.to_i
# Add 1 to expires_in to avoid timing error: http://git.io/i1PHXA
expires_in = period - (epoch_time % period) + 1
key = "#{(epoch_time / period).to_i}:#{unprefixed_key}"
delete(key)
end
def delete(unprefixed_key)
store.delete("#{prefix}:#{unprefixed_key}")
end
end
class Rack::Attack::StoreProxy::RedisStoreProxy
def delete(key, options={})
self.del(key)
rescue Redis::BaseError
end
end
...@@ -156,7 +156,7 @@ describe Grack::Auth do ...@@ -156,7 +156,7 @@ describe Grack::Auth do
end end
expect(attempt_login(true)).to eq(200) expect(attempt_login(true)).to eq(200)
expect(Rack::Attack::Allow2Ban.send(:banned?, ip)).to eq(nil) expect(Rack::Attack::Allow2Ban.banned?(ip)).to be_falsey
for n in 0..maxretry do for n in 0..maxretry do
expect(attempt_login(false)).to eq(401) expect(attempt_login(false)).to eq(401)
......
require "spec_helper"
describe 'RackAttackHelpers' do
describe 'reset' do
let(:discriminator) { 'test-key'}
let(:maxretry) { 5 }
let(:period) { 1.minute }
let(:options) { { findtime: period, bantime: 60, maxretry: maxretry } }
def do_filter
for i in 1..maxretry - 1 do
status = Rack::Attack::Allow2Ban.filter(discriminator, options) { true }
expect(status).to eq(false)
end
end
def do_reset
Rack::Attack::Allow2Ban.reset(discriminator, options)
end
before do
do_reset
end
after do
do_reset
end
it 'user is not banned after n - 1 retries' do
do_filter
do_reset
do_filter
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment