Do not restore a flushed refresh token during a refresh

lib/jwt_sessions/session.rb

@@ -221,7 +221,12 @@ def issue_tokens_after_refresh
     end
 
     def update_refresh_token
-      @_refresh.update(@_access.uid, @_access.expiration, @_csrf.encoded)
+      updated = @_refresh.update(@_access.uid, @_access.expiration, @_csrf.encoded)
+      if updated == false
+        AccessToken.destroy(@_access.uid, store)
+        raise Errors::Unauthorized, "Refresh token not found"
+      end
+
       @refresh_token = @_refresh.token
       link_access_to_refresh
     end

lib/jwt_sessions/store_adapters/abstract_store_adapter.rb

@@ -20,6 +20,8 @@ def persist_refresh(_uid:, _access_expiration:, _access_uid:, _csrf:, _expiratio
         raise NotImplementedError
       end
 
+      # Return false when the refresh token no longer exists. A refresh then
+      # fails instead of writing back a token that a flush has just removed.
       def update_refresh(_uid:, _access_expiration:, _access_uid:, _csrf:, _namespace:)
         raise NotImplementedError
       end

lib/jwt_sessions/store_adapters/memory_store_adapter.rb

@@ -46,13 +46,16 @@ def persist_refresh(uid:, access_expiration:, access_uid:, csrf:, expiration:, n
       end
 
       def update_refresh(uid:, access_expiration:, access_uid:, csrf:, namespace: "")
+        return false if value_if_not_expired(uid, "refresh", namespace.to_s).empty?
+
         update_refresh_fields(
           uid,
           namespace.to_s,
           csrf: csrf,
           access_expiration: access_expiration,
           access_uid: access_uid
         )
+        true
       end
 
       def all_refresh_tokens(namespace)

lib/jwt_sessions/store_adapters/redis_store_adapter.rb

@@ -7,6 +7,15 @@ class RedisStoreAdapter < AbstractStoreAdapter
 
       REFRESH_KEYS = %i[csrf access_uid access_expiration expiration].freeze
       DEFAULT_POOL_SIZE = 5
+      # Updates a refresh token only while it exists, so that a refresh which
+      # races a flush cannot write the flushed token back.
+      UPDATE_REFRESH_SCRIPT = <<~LUA
+        if redis.call("EXISTS", KEYS[1]) == 0 then
+          return 0
+        end
+        redis.call("HMSET", KEYS[1], "csrf", ARGV[1], "access_expiration", ARGV[2], "access_uid", ARGV[3])
+        return 1
+      LUA
 
       def initialize(token_prefix: JWTSessions.token_prefix, redis_client: nil, **options)
         @prefix = token_prefix
@@ -51,24 +60,27 @@ def fetch_refresh(uid, namespace, first_match = false)
 
       def persist_refresh(uid:, access_expiration:, access_uid:, csrf:, expiration:, namespace: nil)
         key = full_refresh_key(uid, namespace)
-        update_refresh(
-          uid: uid,
-          access_expiration: access_expiration,
-          access_uid: access_uid,
-          csrf: csrf,
-          namespace: namespace
+        storage.call("HMSET",
+          key,
+          :csrf, csrf,
+          :access_expiration, access_expiration,
+          :access_uid, access_uid,
+          :expiration, expiration
         )
-        storage.call("HSET", key, :expiration, expiration)
         storage.call("EXPIREAT", key, expiration)
       end
 
       def update_refresh(uid:, access_expiration:, access_uid:, csrf:, namespace: nil)
-        storage.call("HMSET",
+        updated = storage.call(
+          "EVAL",
+          UPDATE_REFRESH_SCRIPT,
+          1,
           full_refresh_key(uid, namespace),
-          :csrf, csrf,
-          :access_expiration, access_expiration,
-          :access_uid, access_uid
+          csrf,
+          access_expiration,
+          access_uid
         )
+        updated == 1
       end
 
       def all_refresh_tokens(namespace)

test/units/jwt_sessions/store_adapters/test_memory_store_adapter.rb

@@ -60,7 +60,7 @@ def test_update_refresh
       expiration: expiration,
       namespace: ""
     )
-    store.update_refresh(
+    assert store.update_refresh(
       uid: "uid",
       access_expiration: expiration,
       access_uid: "access_uid",
@@ -71,6 +71,28 @@ def test_update_refresh
     assert_equal "csrf2", refresh[:csrf]
   end
 
+  def test_update_refresh_after_destroy
+    expiration = Time.now.to_i + 3600
+    store.persist_refresh(
+      uid: "uid",
+      access_expiration: expiration,
+      access_uid: "access_uid",
+      csrf: "csrf",
+      expiration: expiration,
+      namespace: ""
+    )
+    store.destroy_refresh("uid", "")
+
+    refute store.update_refresh(
+      uid: "uid",
+      access_expiration: expiration,
+      access_uid: "access_uid",
+      csrf: "csrf2",
+      namespace: ""
+    )
+    assert_equal({}, store.fetch_refresh("uid", ""))
+  end
+
   def test_all_refresh_tokens
     expiration = Time.now.to_i + 3600
     store.persist_refresh(

test/units/jwt_sessions/store_adapters/test_redis_store_adapter.rb

@@ -11,6 +11,55 @@ def teardown
     JWTSessions.remove_instance_variable(:@redis_url) if JWTSessions.instance_variable_defined?(:@redis_url)
   end
 
+  def test_update_refresh
+    store = JWTSessions::StoreAdapters::RedisStoreAdapter.new
+    expiration = Time.now.to_i + 3600
+    store.persist_refresh(
+      uid: "uid",
+      access_expiration: expiration,
+      access_uid: "access_uid",
+      csrf: "csrf",
+      expiration: expiration,
+      namespace: "update_refresh"
+    )
+
+    assert store.update_refresh(
+      uid: "uid",
+      access_expiration: expiration,
+      access_uid: "access_uid",
+      csrf: "csrf2",
+      namespace: "update_refresh"
+    )
+    assert_equal "csrf2", store.fetch_refresh("uid", "update_refresh")[:csrf]
+  ensure
+    store.destroy_refresh("uid", "update_refresh")
+  end
+
+  def test_update_refresh_after_destroy
+    store = JWTSessions::StoreAdapters::RedisStoreAdapter.new
+    expiration = Time.now.to_i + 3600
+    store.persist_refresh(
+      uid: "uid",
+      access_expiration: expiration,
+      access_uid: "access_uid",
+      csrf: "csrf",
+      expiration: expiration,
+      namespace: "update_refresh_after_destroy"
+    )
+    store.destroy_refresh("uid", "update_refresh_after_destroy")
+
+    refute store.update_refresh(
+      uid: "uid",
+      access_expiration: expiration,
+      access_uid: "access_uid",
+      csrf: "csrf2",
+      namespace: "update_refresh_after_destroy"
+    )
+    assert_empty store.storage.call("KEYS", "#{store.prefix}_update_refresh_after_destroy_*")
+  ensure
+    store.destroy_refresh("uid", "update_refresh_after_destroy")
+  end
+
   def test_error_on_mixed_redis_options
     assert_raises ArgumentError do
       JWTSessions::StoreAdapters::RedisStoreAdapter.new(

test/units/jwt_sessions/test_session.rb

@@ -336,6 +336,21 @@ def test_flush_namespaced_access_tokens_after_flush_namespaced
     assert_equal 0, session.flush_namespaced_access_tokens
   end
 
+  def test_refresh_after_concurrent_flush
+    refresh_token = tokens[:refresh]
+    racing_store = JWTSessions.token_store.dup
+    racing_store.define_singleton_method(:update_refresh) do |**fields|
+      JWTSessions::Session.new.flush_by_token(refresh_token)
+      super(**fields)
+    end
+    racing_session = JWTSessions::Session.new(payload: payload, store: racing_store)
+
+    assert_raises JWTSessions::Errors::Unauthorized do
+      racing_session.refresh(refresh_token)
+    end
+    assert_empty RedisClient.new.call("KEYS", "jwt_*")
+  end
+
   def test_flush_all
     refresh_token = @session.instance_variable_get(:"@_refresh")
     flushed_count = JWTSessions::Session.flush_all