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

Incorrect OTP Timeout Bar #242

Open
anotherbridge opened this issue May 2, 2024 · 3 comments
Open

Incorrect OTP Timeout Bar #242

anotherbridge opened this issue May 2, 2024 · 3 comments

Comments

@anotherbridge
Copy link
Contributor

anotherbridge commented May 2, 2024

Description

When using the Nitrokey App to add an OTP secret I am running into the problem that the generated OTP is not valid anymore and not refreshed, but the timeout bar is still showing it as valid. This results in a wrong OTP given to an application quite frequently.

To me it looks like the timeout in the bar is 60s instead of 30s, which also makes sense according to this change:
https://github.com/Nitrokey/nitrokey-app2/pull/168/files#diff-78e563e8217a49592ec29ff4fa51866a6db38547df2b3aff1ee78cacd6afe918R181

When getting some more insight by investigating some of the values I saw that the period was correctly shown as 30, however when just removing the second period in that statement it didn't change anything according to the timing, but just the progress bar itself.

Expected Behavior

Corret timeout of the OTP or automatic refresh of the OTP so that a correct code is shown at any point in time.

Additional Information

  • Version of the Nitrokey App: v2.3.0
  • Tested operating systems:
    • macOS Sonoma
    • Windows 11
  • Python Version: 3.12.3

P.S.: When reverting the changes in PR #168 the behavior seems correct to me.

@robin-nitrokey
Copy link
Member

Thanks for the report! First, let me add some context to explain the current behavior: TOTPs are typically valid for a 30 s period. This period does not start with the generation of the TOTP, but is based on the 30 s window it was generated in. For example, if you generate an OTP at 12:00:29, it is only valid for one second until 12:00:30. Therefore it is recommended (but not required) that services also accept the previous OTP, i. e. the OTP would be accepted until 12:01:00. As far as I know, this is supported by almost all services using TOTP.

I agree that just updating the OTP would be the best user experience. Unfortunately it is not possible for us to always automatically do that because the credential could be protected by touch or PIN. Therefore we made the decision to show the value for the current period plus the next period, in this case until 12:01:00. After that, it should be hidden. This should work for 99 % of the cases.

But if you enter the OTP after 12:01:00, most services will reject it because it is two periods old. And some services may also reject it after 12:00:30, although I’m not aware of such implementations.

I don’t see a way to improve this as we can’t just re-generate the OTP and we don’t know how long the service will accept it. Do you have the problem with one specific service? In that case, they might not accept OTPs from the last period.

@anotherbridge
Copy link
Contributor Author

@robin-nitrokey Thanks for the explanation.
When taking your example of generating an OTP at 12:00:29, what would be the problem then of immediately hiding it again due to expiration of the 30s window and letting a user actively regenerate a new one (which then would require an additional touch and/or PIN verification)?
Wouldn't this then be working in 100% of the cases or do I have some logical mistake here?
Or is this a question of users' acceptance?

I have at least experienced problems with OTPs from the last period when trying to setup an OTP for Microsoft 365 as well as performing tests with:
https://authenticationtest.com/totpChallenge/

@robin-nitrokey
Copy link
Member

Immediately hiding the OTP would be at least very confusing – you might not even realize that it was displayed at all. Also, it makes the process unnecessarily complicated because it still works for another 30 s for virtually all services. I had the idea to somehow style it differently after the original period is over but we did not really find a good and intuitive solution for that either.

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

2 participants