Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: freeze time in more specs #643

Merged
merged 4 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion spec/acceptance/stores/active_support_dalli_store_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
if should_run
require_relative "../../support/cache_store_helper"
require "active_support/cache/dalli_store"
require "timecop"

describe "ActiveSupport::Cache::DalliStore as a cache backend" do
before do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

if defined?(::ConnectionPool) && defined?(::Dalli)
require_relative "../../support/cache_store_helper"
require "timecop"

describe "ActiveSupport::Cache::MemCacheStore (pooled) as a cache backend" do
before do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

if defined?(::Dalli)
require_relative "../../support/cache_store_helper"
require "timecop"

describe "ActiveSupport::Cache::MemCacheStore as a cache backend" do
before do
Expand Down
2 changes: 0 additions & 2 deletions spec/acceptance/stores/active_support_memory_store_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
require_relative "../../spec_helper"
require_relative "../../support/cache_store_helper"

require "timecop"

describe "ActiveSupport::Cache::MemoryStore as a cache backend" do
before do
Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

if should_run
require_relative "../../support/cache_store_helper"
require "timecop"

describe "ActiveSupport::Cache::RedisCacheStore (pooled) as a cache backend" do
before do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

if should_run
require_relative "../../support/cache_store_helper"
require "timecop"

describe "ActiveSupport::Cache::RedisCacheStore as a cache backend" do
before do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
require_relative "../../support/cache_store_helper"
require "connection_pool"
require "dalli"
require "timecop"

describe "ConnectionPool with Dalli::Client as a cache backend" do
before do
Expand Down
1 change: 0 additions & 1 deletion spec/acceptance/stores/dalli_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
if defined?(::Dalli)
require_relative "../../support/cache_store_helper"
require "dalli"
require "timecop"

describe "Dalli::Client as a cache backend" do
before do
Expand Down
1 change: 0 additions & 1 deletion spec/acceptance/stores/redis_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

if defined?(::Redis)
require_relative "../../support/cache_store_helper"
require "timecop"

describe "Plain redis as a cache backend" do
before do
Expand Down
2 changes: 0 additions & 2 deletions spec/acceptance/stores/redis_store_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
require_relative "../../support/cache_store_helper"

if defined?(::Redis::Store)
require "timecop"

describe "Redis::Store as a cache backend" do
before do
Rack::Attack.cache.store = ::Redis::Store.new
Expand Down
16 changes: 8 additions & 8 deletions spec/rack_attack_throttle_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

require_relative 'spec_helper'
require 'timecop'
require_relative 'support/freeze_time_helper'

describe 'Rack::Attack.throttle' do
before do
Expand All @@ -16,7 +16,7 @@

describe 'a single request' do
it 'should set the counter for one request' do
Timecop.freeze do
within_same_period do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'

key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
Expand All @@ -41,7 +41,7 @@

describe "with 2 requests" do
before do
Timecop.freeze do
within_same_period do
2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
end
end
Expand Down Expand Up @@ -78,7 +78,7 @@

describe 'a single request' do
it 'should set the counter for one request' do
Timecop.freeze do
within_same_period do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'

key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
Expand Down Expand Up @@ -112,7 +112,7 @@

describe 'a single request' do
it 'should set the counter for one request' do
Timecop.freeze do
within_same_period do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'

key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
Expand Down Expand Up @@ -147,7 +147,7 @@

describe 'a single request' do
it 'should not set the counter' do
Timecop.freeze do
within_same_period do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'

key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
Expand Down Expand Up @@ -179,7 +179,7 @@
end

it 'should not differentiate requests when throttle_discriminator_normalizer is enabled' do
Timecop.freeze do
within_same_period do
post_logins
key = "rack::attack:#{Time.now.to_i / @period}:logins/email:person@example.com"
_(Rack::Attack.cache.store.read(key)).must_equal 3
Expand All @@ -191,7 +191,7 @@
prev = Rack::Attack.throttle_discriminator_normalizer
Rack::Attack.throttle_discriminator_normalizer = nil

Timecop.freeze do
within_same_period do
post_logins
@emails.each do |email|
key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}"
Expand Down
56 changes: 31 additions & 25 deletions spec/support/cache_store_helper.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require_relative 'freeze_time_helper'

class Minitest::Spec
def self.it_works_for_cache_backed_features(options)
fetch_from_store = options.fetch(:fetch_from_store)
Expand All @@ -9,11 +11,13 @@ def self.it_works_for_cache_backed_features(options)
request.ip
end

get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
assert_equal 200, last_response.status
within_same_period do
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" => "1.2.3.4"
assert_equal 429, last_response.status
end
end

it "works for fail2ban" do
Expand All @@ -23,17 +27,19 @@ def self.it_works_for_cache_backed_features(options)
end
end

get "/"
assert_equal 200, last_response.status
within_same_period do
get "/"
assert_equal 200, last_response.status

get "/private-place"
assert_equal 403, last_response.status
get "/private-place"
assert_equal 403, last_response.status

get "/private-place"
assert_equal 403, last_response.status
get "/private-place"
assert_equal 403, last_response.status

get "/"
assert_equal 403, last_response.status
get "/"
assert_equal 403, last_response.status
end
end

it "works for allow2ban" do
Expand All @@ -43,20 +49,22 @@ def self.it_works_for_cache_backed_features(options)
end
end

get "/"
assert_equal 200, last_response.status
within_same_period do
get "/"
assert_equal 200, last_response.status

get "/scarce-resource"
assert_equal 200, last_response.status
get "/scarce-resource"
assert_equal 200, last_response.status

get "/scarce-resource"
assert_equal 200, last_response.status
get "/scarce-resource"
assert_equal 200, last_response.status

get "/scarce-resource"
assert_equal 403, last_response.status
get "/scarce-resource"
assert_equal 403, last_response.status

get "/"
assert_equal 403, last_response.status
get "/"
assert_equal 403, last_response.status
end
end

it "doesn't leak keys" do
Expand All @@ -66,9 +74,7 @@ def self.it_works_for_cache_backed_features(options)

key = nil

# Freeze time during these statement to be sure that the key used by rack attack is the same
# we pre-calculate in local variable `key`
Timecop.freeze do
within_same_period do
key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4"

get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
Expand Down
9 changes: 9 additions & 0 deletions spec/support/freeze_time_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

require "timecop"

class Minitest::Spec
def within_same_period(&block)
Timecop.freeze(&block)
end
end