devise-two-factor icon indicating copy to clipboard operation
devise-two-factor copied to clipboard

My TOTP code is getting invalidated before use

Open ab320012 opened this issue 2 years ago • 2 comments

Hey, I am testing 2FA login with the following implementation: generate code:

          if u.method_email?
            u.otp_secret = User.generate_otp_secret
            u.save!

            otp = u.reload.current_otp
            UserMailer.send_otp(u, otp).deliver_now

validatation

            if user.validate_and_consume_otp!(params[:user][:otp_attempt])
              # code for if valid
            end

the validate_and_consume_otp! method is failing incosnsitently i think because of the interval or something although I am not sure, like when i look on the server it looks like the current otp is overwritten and the old otp stops working, can someone help.

ab320012 avatar Aug 17 '22 22:08 ab320012

More to the point with this one, we are seeing an issue where someone can login with an otp, logout, try to login again with a newly generated otp and the new otp is invalidated right away denying login. This seems to happen if the interval between the first and second login is within 30 seconds. Would this be by design?

jasoncross avatar Aug 18 '22 20:08 jasoncross

I think this is happening because by design it is not allowed to use the same otp within the given timestep.

## lib/devise_two_factor/models/two_factor_authenticatable.rb

# 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

But this does not account for the case where the otp secret might have changed and there is a new otp in the same timestep.

You can fix this by setting self.consumed_timestep = nil before you validate the new otp but make sure you have a new otp secret for the same user.

imanpalsingh avatar Feb 17 '23 11:02 imanpalsingh