diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index bb34cf5bfa2..e171730e71a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -126,9 +126,9 @@ specs: POSTGRES_HOST_AUTH_METHOD: trust RAILS_ENV: test services: - - name: postgres:13.4 + - name: postgres:13.9 alias: db-postgres - - name: redis + - name: redis:7.0 alias: db-redis artifacts: expire_in: 31d diff --git a/app/services/throttle.rb b/app/services/throttle.rb index 139c6a07d94..577e7900244 100644 --- a/app/services/throttle.rb +++ b/app/services/throttle.rb @@ -87,30 +87,25 @@ def increment! end # Retrieve the current state of the throttle from Redis - # We use TTL to calculate when the action was last attempted. - # This approach is introduces some skew since time passes - # between "now" and when we fetch the TTL, but it should be low - # relative to the overall length of the throttle window. - # - # When we upgrade to Redis 7, we can use the EXPIRETIME command instead. + # We use EXPIRETIME to calculate when the action was last attempted. def fetch_state! value = nil - ttl = nil + expiretime = nil REDIS_THROTTLE_POOL.with do |client| - value, ttl = client.multi do |multi| + value, expiretime = client.multi do |multi| multi.get(key) - multi.ttl(key) + multi.expiretime(key) end end @redis_attempts = value.to_i - if ttl < 0 + if expiretime < 0 @redis_attempted_at = nil else @redis_attempted_at = - Time.zone.now - - Throttle.attempt_window_in_minutes(throttle_type).minutes + ttl.seconds + ActiveSupport::TimeZone['UTC'].at(expiretime).in_time_zone(Time.zone) - + Throttle.attempt_window_in_minutes(throttle_type).minutes end self @@ -127,17 +122,18 @@ def reset! def increment_to_throttled! value = Throttle.max_attempts(throttle_type) + now = Time.zone.now REDIS_THROTTLE_POOL.with do |client| - client.setex( + client.set( key, - Throttle.attempt_window_in_minutes(throttle_type).minutes.seconds.to_i, value, + exat: now.to_i + Throttle.attempt_window_in_minutes(throttle_type).minutes.seconds.to_i, ) end @redis_attempts = value.to_i - @redis_attempted_at = Time.zone.now + @redis_attempted_at = now attempts end diff --git a/docs/local-development.md b/docs/local-development.md index 1f33ac62437..a4d2db3e767 100644 --- a/docs/local-development.md +++ b/docs/local-development.md @@ -11,7 +11,7 @@ We recommend using [Homebrew](https://brew.sh/), [rbenv](https://github.com/rben - Ruby ~> 3.2.0 - [PostgreSQL](http://www.postgresql.org/download/) -- [Redis 5+](http://redis.io/) +- [Redis 7+](http://redis.io/) - [Node.js v16](https://nodejs.org) - [Yarn](https://yarnpkg.com/en/) - [chromedriver](https://formulae.brew.sh/cask/chromedriver) diff --git a/spec/controllers/idv/phone_errors_controller_spec.rb b/spec/controllers/idv/phone_errors_controller_spec.rb index 8fe9548c67f..876149999a1 100644 --- a/spec/controllers/idv/phone_errors_controller_spec.rb +++ b/spec/controllers/idv/phone_errors_controller_spec.rb @@ -214,16 +214,9 @@ context 'while throttled' do let(:user) { create(:user) } - let(:attempted_at) do - d = DateTime.now # microsecond precision failing on CI - Time.zone.parse(d.to_s) - end - - before do - Throttle.new(throttle_type: :proof_address, user: user).increment_to_throttled! - end it 'assigns expiration time' do + Throttle.new(throttle_type: :proof_address, user: user).increment_to_throttled! get action expect(assigns(:expires_at)).to be_kind_of(Time) @@ -231,6 +224,8 @@ it 'logs an event' do freeze_time do + attempted_at = Time.zone.now.utc + Throttle.new(throttle_type: :proof_address, user: user).increment_to_throttled! throttle_window = Throttle.attempt_window_in_minutes(:proof_address).minutes get action diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index f6a55c43fe6..4a1e197b1a0 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -110,6 +110,7 @@ scenario 'rate limits sign-up phone confirmation attempts' do allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(999) + allow(IdentityConfig.store).to receive(:phone_confirmation_max_attempts).and_return(1) sign_up_and_set_password