Disallow TOTP reuse

app/models/concerns/user_totp_methods.rb

@@ -46,8 +46,10 @@ def verify_totp(seed, otp)
     return false if seed.blank?
 
     totp = ROTP::TOTP.new(seed)
-    return false unless totp.verify(otp, drift_behind: 30, drift_ahead: 30)
+    totp_at = totp.verify(otp, drift_behind: 30, drift_ahead: 30, after: last_totp_at)
+    return false if totp_at.blank?
 
+    self.last_totp_at = Time.at(totp_at).utc
     save!(validate: false)
   end
 end

db/migrate/20240720201622_add_last_totp_at_to_users.rb

@@ -0,0 +1,5 @@
+class AddLastTotpAtToUsers < ActiveRecord::Migration[7.1]
+  def change
+    add_column :users, :last_totp_at, :timestamp, null: true
+  end
+end

db/schema.rb

@@ -519,6 +519,7 @@
     t.string "mfa_hashed_recovery_codes", default: [], array: true
     t.boolean "public_email", default: false, null: false
     t.datetime "deleted_at"
+    t.datetime "last_totp_at", precision: nil
     t.index "lower((email)::text) varchar_pattern_ops", name: "index_users_on_lower_email"
     t.index ["email"], name: "index_users_on_email"
     t.index ["handle"], name: "index_users_on_handle"

test/functional/api/v1/deletions_controller_test.rb

@@ -328,6 +328,8 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase
 
           should "not include mfa warnings" do
             @gems.each_value do |gem|
+              travel_to 30.seconds.from_now
+
               @request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.totp_seed).now
               delete :create, params: { gem_name: gem[:name], version: gem[:version] }
 

test/functional/multifactor_auths_controller_test.rb

@@ -672,6 +672,8 @@ class MultifactorAuthsControllerTest < ActionController::TestCase
 
         should "redirect user back to mfa_redirect_uri after successful mfa setup" do
           @redirect_paths.each do |path|
+            travel_to 30.seconds.from_now
+
             session[:mfa_redirect_uri] = path
             put :update, params: { level: "ui_and_api" }
             put :otp_update, params: { otp: ROTP::TOTP.new(@seed).now, level: "ui_and_api" }
@@ -694,6 +696,8 @@ class MultifactorAuthsControllerTest < ActionController::TestCase
 
         should "redirect user back to mfa_redirect_uri after a failed setup + successful setup" do
           @redirect_paths.each do |path|
+            travel_to 30.seconds.from_now
+
             session[:mfa_redirect_uri] = path
             put :update, params: { level: "ui_and_api" }
             put :otp_update, params: { otp: "12345", level: "ui_and_api" }

test/models/concerns/user_multifactor_methods_test.rb

@@ -410,6 +410,23 @@ class UserMultifactorMethodsTest < ActiveSupport::TestCase
 
         refute @user.ui_mfa_verified?(ROTP::TOTP.new(ROTP::Base32.random_base32).now)
       end
+
+      should "return false when reused" do
+        code = ROTP::TOTP.new(@user.totp_seed).now
+
+        assert @user.ui_mfa_verified?(code)
+        refute @user.ui_mfa_verified?(code)
+      end
+
+      should "return true after waiting for the next interval" do
+        code = ROTP::TOTP.new(@user.totp_seed).now
+
+        assert @user.ui_mfa_verified?(code)
+        travel_to 30.seconds.from_now do
+          refute @user.ui_mfa_verified?(code)
+          assert @user.ui_mfa_verified?(ROTP::TOTP.new(@user.totp_seed).now)
+        end
+      end
     end
 
     context "with webauthn otp" do
@@ -453,6 +470,23 @@ class UserMultifactorMethodsTest < ActiveSupport::TestCase
       should "return false if otp is incorrect" do
         refute @user.api_mfa_verified?(ROTP::TOTP.new(ROTP::Base32.random_base32).now)
       end
+
+      should "return false when reused" do
+        code = ROTP::TOTP.new(@user.totp_seed).now
+
+        assert @user.api_mfa_verified?(code)
+        refute @user.api_mfa_verified?(code)
+      end
+
+      should "return true after waiting for the next interval" do
+        code = ROTP::TOTP.new(@user.totp_seed).now
+
+        assert @user.api_mfa_verified?(code)
+        travel_to 30.seconds.from_now do
+          refute @user.api_mfa_verified?(code)
+          assert @user.api_mfa_verified?(ROTP::TOTP.new(@user.totp_seed).now)
+        end
+      end
     end
 
     context "with webauthn otp" do