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

Add function/parameter to check if previous OTP was used #226

Open
ingin97 opened this issue Aug 1, 2024 · 1 comment · May be fixed by #232
Open

Add function/parameter to check if previous OTP was used #226

ingin97 opened this issue Aug 1, 2024 · 1 comment · May be fixed by #232
Labels

Comments

@ingin97
Copy link

ingin97 commented Aug 1, 2024

Description

As the RFC6238 states the following:

The verifier MUST NOT accept
the second attempt of the OTP after the successful validation has
been issued for the first OTP, which ensures one-time only use of an
OTP.

It would be nice to have the Totp->verify() function to optionally accept an argument of the previous time the verify function was used. Though this timestamp of course has to be stored in the app itself, it would be nice to have the functionality to check the timestamp inside the package.
This could also be done by mentioning explicitly in the documentation that it is best practice to not accept reuse of OTPs.

If this you see the benefit of adding this I will happily open a PR.

Example

Could be achieved by e.g.:

public function verify(string $otp, null|int $timestamp = null, null|int $leeway = null, null|int $previousTimestamp = null): bool
    {
        $timestamp ??= $this->clock->now()
            ->getTimestamp();
        $timestamp >= 0 || throw new InvalidArgumentException('Timestamp must be at least 0.');

        if ($previousTimestamp !== null) {
            $previousTimestamp >= 0 || throw new InvalidArgumentException('Previous timestamp must be at least 0.');
            if ($this->at($timestamp) === $this->at($previousTimestamp)) {
                return false;
            }
        }
...

OR

public function verify(string $otp, null|int $timestamp = null, null|int $leeway = null, null|int $previousTimestamp = null): bool
    {
        $timestamp ??= $this->clock->now()
            ->getTimestamp();
        $timestamp >= 0 || throw new InvalidArgumentException('Timestamp must be at least 0.');

       if ($previousTimestamp !== null) {
            $previousTimestamp >= 0 || throw new InvalidArgumentException('Previous timestamp must be at least 0.');
            if ($timestamp < $this->timecode($previousTimestamp) + $this->getPeriod()) {
                return false;
            }
        }
...
@Spomky
Copy link
Member

Spomky commented Aug 2, 2024

Hi @ingin97,

There is no reason to introduce a new interface to keep track on past OTPs in this library.
This can easily be achieved on the application side with a caching system, a database or files.
Same goes for the bruteforce prevention means.

@Spomky Spomky added the wontfix label Aug 2, 2024
@glaubinix glaubinix linked a pull request Sep 26, 2024 that will close this issue
4 tasks
@Spomky Spomky linked a pull request Sep 26, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants