Fix rack:attack flaky tests and test end of throttle period (#23799)

This commit is contained in:
Claire 2023-02-22 10:28:52 +01:00 committed by GitHub
parent 4ff44be134
commit 3ed1b9ebb6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 23 additions and 4 deletions

View File

@ -10,6 +10,17 @@ describe Rack::Attack do
end end
shared_examples 'throttled endpoint' do shared_examples 'throttled endpoint' do
before do
# Rack::Attack periods are not rolling, so avoid flaky tests by setting the time in a way
# to avoid crossing period boundaries.
# The code Rack::Attack uses to set periods is the following:
# https://github.com/rack/rack-attack/blob/v6.6.1/lib/rack/attack/cache.rb#L64-L66
# So we want to minimize `Time.now.to_i % period`
travel_to Time.zone.at((Time.now.to_i / period.seconds).to_i * period.seconds)
end
context 'when the number of requests is lower than the limit' do context 'when the number of requests is lower than the limit' do
it 'does not change the request status' do it 'does not change the request status' do
limit.times do limit.times do
@ -20,11 +31,16 @@ describe Rack::Attack do
end end
context 'when the number of requests is higher than the limit' do context 'when the number of requests is higher than the limit' do
it 'returns http too many requests' do it 'returns http too many requests after limit and returns to normal status after period' do
(limit * 2).times do |i| (limit * 2).times do |i|
request.call request.call
expect(last_response.status).to eq(429) if i > limit expect(last_response.status).to eq(429) if i > limit
end end
travel period
request.call
expect(last_response.status).to_not eq(429)
end end
end end
end end
@ -33,7 +49,8 @@ describe Rack::Attack do
describe 'throttle excessive sign-up requests by IP address' do describe 'throttle excessive sign-up requests by IP address' do
context 'through the website' do context 'through the website' do
let(:limit) { 25 } let(:limit) { 25 }
let(:period) { 5.minutes }
let(:request) { -> { post path, {}, 'REMOTE_ADDR' => remote_ip } } let(:request) { -> { post path, {}, 'REMOTE_ADDR' => remote_ip } }
context 'for exact path' do context 'for exact path' do
@ -50,7 +67,8 @@ describe Rack::Attack do
end end
context 'through the API' do context 'through the API' do
let(:limit) { 5 } let(:limit) { 5 }
let(:period) { 30.minutes }
let(:request) { -> { post path, {}, 'REMOTE_ADDR' => remote_ip } } let(:request) { -> { post path, {}, 'REMOTE_ADDR' => remote_ip } }
context 'for exact path' do context 'for exact path' do
@ -71,7 +89,8 @@ describe Rack::Attack do
end end
describe 'throttle excessive sign-in requests by IP address' do describe 'throttle excessive sign-in requests by IP address' do
let(:limit) { 25 } let(:limit) { 25 }
let(:period) { 5.minutes }
let(:request) { -> { post path, {}, 'REMOTE_ADDR' => remote_ip } } let(:request) { -> { post path, {}, 'REMOTE_ADDR' => remote_ip } }
context 'for exact path' do context 'for exact path' do