Use Redis 7 EXPIRETIME command for calculating rate limit state

.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

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

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)

spec/controllers/idv/phone_errors_controller_spec.rb

@@ -214,23 +214,18 @@
 
     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)
       end
 
       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

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