Old OTP can be used after a new one has been generated

[monfresh]

Steps to reproduce:

1. User signs in, and is prompted to enter the OTP they were sent
2. User requests a new OTP to be sent
3. User receives new OTP that is different from the one in step 1
4. User enters OTP from step 1

Expected Result: OTP should no longer be valid since a new one was generated. There should only ever be one valid OTP at any given time

Actual Result: The old OTP is considered valid and the user is able to fully authenticate

[Houdini]

Hello,
it could happened because of:

totp.verify_with_drift(code, drift)

Could you please confirm, that if you push down drift to zero, old otp will no longer be available?

[monfresh]

If I set allowed_otp_drift_seconds to 0, it makes all OTPs expire immediately.

[monfresh]

In other words, I can keep generating new ones, but none of them will be valid.

[monfresh]

I think this is an rotp issue. I'm not sure if they support invalidating previous OTPs once a new one is generated.

[Houdini]

It's strange it should not expire all OTP immediately.
Here is the game:

require 'rotp'
totp = ROTP::TOTP.new("base32secret3232")
totp.verify_with_drift(totp.now, 0) # true
totp.verify_with_drift(totp.at(Time.now - 60), 0) # false
totp.verify_with_drift(totp.at(Time.now - 60), 60) # true

[monfresh]

Maybe I'm not understanding how this works, but it looks like the authenticate_otp method uses a static drift based on the Devise setting for allowed_otp_drift_seconds:
https://github.com/Houdini/two_factor_authentication/blob/master/lib/two_factor_authentication/models/two_factor_authenticatable.rb#L29-L34

How can a call with a different drift be called based on which OTP is requested?

[monfresh]

Sorry, I misread what you typed. I added some debugging statements to authenticate_otp to compare the code sent to the user (via current_user.send_two_factor_authentication_code) with totp.at(Time.now) and I think I see what's causing this. It think it has to do with the rotp interval, which is set to 30 seconds by default.

Scenario 1: User enters OTP within specified interval

1. User signs in at 9:52:00 -> they receive an OTP
2. If they enter this OTP within 30 seconds, then it is valid

Scenario 2: User enters OTP after specified interval and requests and enters a new one within the time step

1. User signs in at 9:52:00 -> they receive an OTP
2. Wait more than 30 seconds and try to enter the OTP -> it is invalid
3. Request a new OTP between 9:52:30 and 9:53:00 and enter it before 9:53:00 -> it is valid

Scenario 3: User requests a new OTP inside current time step but enters it after time step

1. User signs in at 9:52:25 -> they receive an OTP
2. Wait until after 9:52:30 and try to enter the OTP -> it is invalid
3. Request a new OTP before 9:53:00, but enter it after 9:53:00 -> it is invalid

This all seems to work as expected, but what if I want to increase the interval so that a user doesn't have to enter the OTP so fast? So, I added an interval option to both authenticate_otp and otp_code, and set it to 300, but it didn't work as I'd like it to. Every time you request a new OTP, it still sends the same OTP during the 5-minute interval.

What I would like is this behavior:

• Every time a user requests a new OTP, a brand new OTP is generated and the previous one is immediately expired.
• Every new OTP that is generated should be valid within a specified amount of time.

Is this possible?

[monfresh]

I asked this question on the rotp repo, and it looks like this is not possible. What I'm trying to do is not how ROTP is meant to work. See this comment: mdp/rotp#46 (comment)

[bijoysijo]

Hi,

If this issue still persists, I got it working by resetting the OTP secret key for the user every time they request a new OTP.
Using ROTP, I generate an OTP secret for the user when they sign up. Then generate the OTP with the secret and send it to the user.

If the user requests a new OTP, I reset the secret, i.e I generate the secret again. Then use the new secret to send a new OTP. Since I'm using the secret while validating an OTP, the old OTP instantly becomes invalid.

Generating OTP:
@user.email_otp_secret = ROTP::Base32.random_base32
@user.save

secret = @user.email_otp_secret
otp = ROTP::TOTP.new(secret).now

Verifying the OTP:
otp = ROTP::TOTP.new(@user.email_otp_secret)
otp.verify_with_drift(otp_code)