Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

My TOTP code is getting invalidated before use #222

Open
ab320012 opened this issue Aug 17, 2022 · 2 comments
Open

My TOTP code is getting invalidated before use #222

ab320012 opened this issue Aug 17, 2022 · 2 comments

Comments

@ab320012
Copy link

ab320012 commented Aug 17, 2022

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.

@jasoncross
Copy link

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?

@imanpalsingh
Copy link

imanpalsingh commented Feb 17, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants