heapsource / active_model_otp

Adds methods to set and authenticate against one time passwords (Two-Factor Authentication). Inspired in AM::SecurePassword
MIT License
773 stars 81 forks source link

last_otp_at misbehaving #109

Open danielb2 opened 1 year ago

danielb2 commented 1 year ago

I'm having an issue with last_otp_at not being set

in the user model and migration:


## user model
has_one_time_password backup_codes_count: 6, one_time_backup_codes: true, after_column_name: :last_otp_at

## migration
class AddLastOtpAtToUsers < ActiveRecord::Migration[7.0]
  def change
    add_column :users, :last_otp_at, :integer, description: 'Preventing reuse of Time based OTPs'
  end
end
[11] pry(main)> u.authenticate_otp u.otp_code
true
[12] pry(main)> u.authenticate_otp u.otp_code
true
[13] pry(main)> u.last_otp_at
nil

any ideas why this would happen?

guilleiguaran commented 1 year ago

Hi @danielb2, can you try to check u.errors after checking u.authenticate_otp u.otp_code?

I see that in the code we are using update instead of update! so this might be failing silently:

https://github.com/heapsource/active_model_otp/blob/main/lib/active_model/one_time_password.rb#L157

danielb2 commented 1 year ago

Hi thank you for your response. it's empty: #<ActiveModel::Errors []>

danielb2 commented 1 year ago

any more ideas?

aditya-kreditz commented 1 year ago

Hi @danielb2 Have you tried upgrading gem to 2.3.2. I did the same and last otp at column updated. But issue I am facing is with interval. I have set interval to 10 minutes. And once authenticated, it returns false later but it doesn't generate new otp for 10 minutes. So I get same otp again for 10 minutes. I tried it with default interval but same issue occurs. Though 30 seconds is not a very long interval so goes unnoticed. But for longer interval, this functionality is useless.