Disallow subsequent OTPs once validated via timesteps

demo/db/migrate/20140516191259_add_devise_two_factor_to_users.rb

@@ -3,6 +3,7 @@ def change
     add_column :users, :encrypted_otp_secret, :string
     add_column :users, :encrypted_otp_secret_iv, :string
     add_column :users, :encrypted_otp_secret_salt, :string
+    add_column :users, :consumed_timestep, :integer
     add_column :users, :otp_required_for_login, :boolean
   end
 end

demo/db/schema.rb

@@ -32,6 +32,7 @@
     t.string   "encrypted_otp_secret"
     t.string   "encrypted_otp_secret_iv"
     t.string   "encrypted_otp_secret_salt"
+    t.integer  "consumed_timestep"
     t.boolean  "otp_required_for_login"
   end
 

lib/devise_two_factor/models/two_factor_authenticatable.rb

@@ -15,17 +15,19 @@ module TwoFactorAuthenticatable
       end
 
       def self.required_fields(klass)
-        [:encrypted_otp_secret, :encrypted_otp_secret_iv, :encrypted_otp_secret_salt]
+        [:encrypted_otp_secret, :encrypted_otp_secret_iv, :encrypted_otp_secret_salt, :consumed_timestep]
       end
 
       # This defaults to the model's otp_secret
-      # If this hasn't been generated yet, pass a secret as an  option
-      def valid_otp?(code, options = {})
+      # If this hasn't been generated yet, pass a secret as an option
+      def validate_and_consume_otp!(code, options = {})
         otp_secret = options[:otp_secret] || self.otp_secret
         return false unless otp_secret.present?
 
         totp = self.otp(otp_secret)
-        totp.verify_with_drift(code, self.class.otp_allowed_drift)
+        return consume_otp! if totp.verify_with_drift(code, self.class.otp_allowed_drift)
+
+        false
       end
 
       def otp(otp_secret = self.otp_secret)
@@ -36,6 +38,11 @@ def current_otp
         otp.at(Time.now)
       end
 
+      # ROTP's TOTP#timecode is private, so we duplicate it here
+      def current_otp_timestep
+         Time.now.utc.to_i / otp.interval
+      end
+
       def otp_provisioning_uri(account, options = {})
         otp_secret = options[:otp_secret] || self.otp_secret
         ROTP::TOTP.new(otp_secret, options).provisioning_uri(account)
@@ -47,6 +54,17 @@ def clean_up_passwords
 
     protected
 
+      # An OTP cannot be used more than once in a given timestep
+      # Storing timestep of last valid OTP is sufficient to satisfy this requirement
+      def consume_otp!
+        if self.consumed_timestep != current_otp_timestep
+          self.consumed_timestep = current_otp_timestep
+          return save(validate: false)
+        end
+
+        false
+      end
+
       module ClassMethods
         Devise::Models.config(self, :otp_secret_length,
                                     :otp_allowed_drift,

lib/devise_two_factor/spec_helpers/two_factor_authenticatable_shared_examples.rb

@@ -5,7 +5,7 @@
 
   describe 'required_fields' do
     it 'should have the attr_encrypted fields for otp_secret' do
-      expect(Devise::Models::TwoFactorAuthenticatable.required_fields(subject.class)).to contain_exactly(:encrypted_otp_secret, :encrypted_otp_secret_iv, :encrypted_otp_secret_salt)
+      expect(Devise::Models::TwoFactorAuthenticatable.required_fields(subject.class)).to contain_exactly(:encrypted_otp_secret, :encrypted_otp_secret_iv, :encrypted_otp_secret_salt, :consumed_timestep)
     end
   end
 
@@ -27,7 +27,7 @@
     end
   end
 
-  describe '#valid_otp?' do
+  describe '#validate_and_consume_otp!' do
     let(:otp_secret) { '2z6hxkdwi3uvrnpn' }
 
     before :each do
@@ -39,24 +39,50 @@
       Timecop.return
     end
 
+    context 'with a stored consumed_timestep' do
+      context 'given a precisely correct OTP' do
+        let(:consumed_otp) { ROTP::TOTP.new(otp_secret).at(Time.now) }
+
+        before do
+          subject.validate_and_consume_otp!(consumed_otp)
+        end
+
+        it 'fails to validate' do
+          expect(subject.validate_and_consume_otp!(consumed_otp)).to be false
+        end
+      end
+
+      context 'given a previously valid OTP within the allowed drift' do
+        let(:consumed_otp) { ROTP::TOTP.new(otp_secret).at(Time.now + subject.class.otp_allowed_drift, true) }
+
+        before do
+          subject.validate_and_consume_otp!(consumed_otp)
+        end
+
+        it 'fails to validate' do
+          expect(subject.validate_and_consume_otp!(consumed_otp)).to be false
+        end
+      end
+    end
+
     it 'validates a precisely correct OTP' do
       otp = ROTP::TOTP.new(otp_secret).at(Time.now)
-      expect(subject.valid_otp?(otp)).to be true
+      expect(subject.validate_and_consume_otp!(otp)).to be true
     end
 
     it 'validates an OTP within the allowed drift' do
       otp = ROTP::TOTP.new(otp_secret).at(Time.now + subject.class.otp_allowed_drift, true)
-      expect(subject.valid_otp?(otp)).to be true
+      expect(subject.validate_and_consume_otp!(otp)).to be true
     end
 
     it 'does not validate an OTP above the allowed drift' do
       otp = ROTP::TOTP.new(otp_secret).at(Time.now + subject.class.otp_allowed_drift * 2, true)
-      expect(subject.valid_otp?(otp)).to be false
+      expect(subject.validate_and_consume_otp!(otp)).to be false
     end
 
     it 'does not validate an OTP below the allowed drift' do
       otp = ROTP::TOTP.new(otp_secret).at(Time.now - subject.class.otp_allowed_drift * 2, true)
-      expect(subject.valid_otp?(otp)).to be false
+      expect(subject.validate_and_consume_otp!(otp)).to be false
     end
   end
 

lib/devise_two_factor/strategies/two_factor_authenticatable.rb

@@ -22,7 +22,7 @@ def authenticate!
       def validate_otp(resource)
         return true unless resource.otp_required_for_login
         return if params[scope]['otp_attempt'].nil?
-        resource.valid_otp?(params[scope]['otp_attempt'])
+        resource.validate_and_consume_otp!(params[scope]['otp_attempt'])
       end
     end
   end

lib/generators/devise_two_factor/devise_two_factor_generator.rb

@@ -22,6 +22,7 @@ def create_devise_two_factor_migration
                                 "encrypted_otp_secret:string",
                                 "encrypted_otp_secret_iv:string",
                                 "encrypted_otp_secret_salt:string",
+                                "consumed_timestep:integer",
                                 "otp_required_for_login:boolean"
                               ]
 

spec/devise/models/two_factor_authenticatable_spec.rb

@@ -6,6 +6,13 @@ class TwoFactorAuthenticatableDouble
   extend  ::Devise::Models
 
   devise :two_factor_authenticatable, :otp_secret_encryption_key => 'test-key'
+
+  attr_accessor :consumed_timestep
+
+  def save(validate)
+    # noop for testing
+    true
+  end
 end
 
 describe ::Devise::Models::TwoFactorAuthenticatable do