From 3ed1b9ebb60a50ea1cb6a229ab25d4e7d6b848c8 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 22 Feb 2023 10:28:52 +0100 Subject: [PATCH] Fix rack:attack flaky tests and test end of throttle period (#23799) --- spec/config/initializers/rack_attack_spec.rb | 27 +++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/spec/config/initializers/rack_attack_spec.rb b/spec/config/initializers/rack_attack_spec.rb index 2cfe90b1a59..cc931b21b3b 100644 --- a/spec/config/initializers/rack_attack_spec.rb +++ b/spec/config/initializers/rack_attack_spec.rb @@ -10,6 +10,17 @@ describe Rack::Attack do end 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 it 'does not change the request status' do limit.times do @@ -20,11 +31,16 @@ describe Rack::Attack do end 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| request.call expect(last_response.status).to eq(429) if i > limit end + + travel period + + request.call + expect(last_response.status).to_not eq(429) end end end @@ -33,7 +49,8 @@ describe Rack::Attack do describe 'throttle excessive sign-up requests by IP address' do context 'through the website' do - let(:limit) { 25 } + let(:limit) { 25 } + let(:period) { 5.minutes } let(:request) { -> { post path, {}, 'REMOTE_ADDR' => remote_ip } } context 'for exact path' do @@ -50,7 +67,8 @@ describe Rack::Attack do end context 'through the API' do - let(:limit) { 5 } + let(:limit) { 5 } + let(:period) { 30.minutes } let(:request) { -> { post path, {}, 'REMOTE_ADDR' => remote_ip } } context 'for exact path' do @@ -71,7 +89,8 @@ describe Rack::Attack do end 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 } } context 'for exact path' do