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

TOTP: introduce new interface that prevents code reuse #232

Open
wants to merge 2 commits into
base: 11.4.x
Choose a base branch
from

Conversation

glaubinix
Copy link

Target branch: 11.4.x
Resolves issue #226

  • It is a Bug fix
  • It is a New feature
  • Breaks BC
  • Includes Deprecations

We did run into the same issue that was previously mentioned here where users can submit the same TOTP code multiple times.

To prevent this, a new interface was introduced with a new verifyWithPreviousTimestamp method that accepts an additional parameter, the timestamp of the last used code. Passing the parameter will prevent the code matching the timestamp, and in case leeway is used, the previous code, from being reused again. To make it possible to store the timestamp for the submitted code, the timestamp needs to be returned by the verifyWithPreviousTimestamp method. Happy to adjust this to return a result object, in case that is preferred.

@Spomky Spomky linked an issue Sep 26, 2024 that may be closed by this pull request
@Spomky Spomky added this to the 11.4.0 milestone Sep 26, 2024
@Spomky Spomky self-assigned this Sep 26, 2024
@glaubinix glaubinix force-pushed the totp-prevent-code-reuse branch from c300614 to 2c8f53e Compare September 26, 2024 10:09
@glaubinix glaubinix force-pushed the totp-prevent-code-reuse branch from 2c8f53e to 79aad1f Compare October 10, 2024 09:08
@Spomky
Copy link
Member

Spomky commented Oct 11, 2024

Hi,

Many thanks for this PR.
I like the interface, but I would prefer another class that extends the TOTP class with the new interface.

@glaubinix
Copy link
Author

Thank you for the feedback! The TOTP class is marked as final and should probably stay that way. Do you have a preference on introducing an AbstractTOTP class that implement the basic logic for both classes, or do you prefer adding a new TOTPWithPreviousTimestamp class that extends the two interface and then extract the verify logic into an internal TOTPVerifier class?

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

Successfully merging this pull request may close these issues.

Add function/parameter to check if previous OTP was used
2 participants