diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index d23cae87..db6a4d4d 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -44,7 +44,9 @@ def authenticate_totp(code, options = {}) drift_ahead: drift, drift_behind: drift, after: totp_timestamp ) return false unless new_timestamp - self.totp_timestamp = new_timestamp + # ROTP >= 3 returns the verified epoch as an Integer, not a boolean. + # Coerce it to a Time so it can be stored in a datetime column. + self.totp_timestamp = Time.at(new_timestamp).utc true end diff --git a/lib/two_factor_authentication/routes.rb b/lib/two_factor_authentication/routes.rb index 543059a2..5e5442ba 100644 --- a/lib/two_factor_authentication/routes.rb +++ b/lib/two_factor_authentication/routes.rb @@ -3,7 +3,7 @@ class Mapper protected def devise_two_factor_authentication(mapping, controllers) - resource :two_factor_authentication, :only => [:show, :update, :resend_code], :path => mapping.path_names[:two_factor_authentication], :controller => controllers[:two_factor_authentication] do + resource :two_factor_authentication, :only => [:show, :update], :path => mapping.path_names[:two_factor_authentication], :controller => controllers[:two_factor_authentication] do collection { get "resend_code" } end end diff --git a/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb b/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb index 6fb4f505..6f5035ab 100644 --- a/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb +++ b/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb @@ -105,6 +105,63 @@ def do_invoke(code, user) it_behaves_like 'authenticate_totp', GuestUser.new it_behaves_like 'authenticate_totp', EncryptedUser.new + + # Regression: ROTP >= 3 returns TOTP#verify as an Integer Unix epoch rather + # than a boolean. When totp_timestamp is a real datetime column (as in a + # consuming app) that raw Integer must be coerced to a Time before + # assignment, otherwise it overflows the column on Postgres under Rails 8. + # The GuestUser/EncryptedUser doubles above store totp_timestamp as a plain + # attribute, so they never exercise the datetime round-trip. + context 'with a datetime-backed totp_timestamp column' do + before(:all) do + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Schema.define do + create_table :totp_timestamp_users, force: true do |t| + t.string :otp_secret_key + t.integer :second_factor_attempts_count, default: 0 + t.datetime :totp_timestamp + end + end + end + + unless defined?(TotpTimestampUser) + klass = Class.new(ActiveRecord::Base) do + self.table_name = 'totp_timestamp_users' + include Devise::Models::TwoFactorAuthenticatable + has_one_time_password + end + Object.const_set(:TotpTimestampUser, klass) + end + end + + let(:instance) { TotpTimestampUser.create!(otp_secret_key: '2z6hxkdwi3uvrnpn') } + let(:otp_length) { instance.class.otp_length } + let(:drift) { instance.class.allowed_otp_drift_seconds } + let(:code) { TotpHelper.new(instance.otp_secret_key, otp_length).totp_code } + + it 'coerces the verified epoch to a Time and persists it without error' do + # The epoch ROTP#verify returns for this code, computed independently. + expected_epoch = ROTP::TOTP.new(instance.otp_secret_key, digits: otp_length). + verify(code, drift_ahead: drift, drift_behind: drift).to_i + + expect(instance.authenticate_totp(code)).to eq(true) + expect { instance.save! }.not_to raise_error + + instance.reload + expect(instance.totp_timestamp.acts_like?(:time)).to be true + expect(instance.totp_timestamp.to_i).to eq(expected_epoch) + end + + it 'feeds the persisted timestamp back to ROTP to prevent code reuse' do + expect(instance.authenticate_totp(code)).to eq(true) + instance.save! + instance.reload + + # totp_timestamp is now a datetime read from the DB; ROTP must still be + # able to derive the same epoch from it via #to_i for replay protection. + expect(instance.authenticate_totp(code)).to eq(false) + end + end end describe '#send_two_factor_authentication_code' do diff --git a/spec/rails_app/config/application.rb b/spec/rails_app/config/application.rb index 2d31d588..17199247 100644 --- a/spec/rails_app/config/application.rb +++ b/spec/rails_app/config/application.rb @@ -3,7 +3,6 @@ require "active_record/railtie" require "action_controller/railtie" require "action_mailer/railtie" -require "sprockets/railtie" Bundler.require(*Rails.groups) require "two_factor_authentication" @@ -47,12 +46,6 @@ class Application < Rails::Application # like if you have constraints or database-specific column types # config.active_record.schema_format = :sql - # Enable the asset pipeline - config.assets.enabled = true - - # Version of your assets, change this if you want to expire all your assets - config.assets.version = '1.0' - config.action_mailer.default_url_options = { host: 'localhost:3000' } config.i18n.enforce_available_locales = false diff --git a/spec/rails_app/config/initializers/inflections.rb b/spec/rails_app/config/initializers/inflections.rb index 5d8d9be2..80d39b23 100644 --- a/spec/rails_app/config/initializers/inflections.rb +++ b/spec/rails_app/config/initializers/inflections.rb @@ -13,3 +13,7 @@ # ActiveSupport::Inflector.inflections do |inflect| # inflect.acronym 'RESTful' # end + +ActiveSupport::Inflector.inflections(:en) do |inflect| + inflect.acronym 'SMS' +end