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

Verify OTP without marking it as used. #39

Merged
merged 6 commits into from
Dec 23, 2024
Merged

Conversation

BoddaSaad
Copy link
Contributor

Summary

Some application may only need to check the validity of the OTP before using it, so, this pull request introduces a new method isValid to the Otp class. The isValid method checks the existence and validity of an OTP (the same way you use) without marking it as used.

Benefits

  • Added isValid method to check OTP existence and validity without marking it as used.

Usage

<?php

use Ichtrojan\Otp\Otp;

(new Otp)->isValid(string $identifier, string $token);

src/Otp.php Outdated
Comment on lines 56 to 65
if($otp){
$now = Carbon::now();
$validity = $otp->created_at->addMinutes($otp->validity);

if( $otp->valid && (strtotime($validity) >= strtotime($now))) {
return true;
}
}

return false;
Copy link
Owner

@ichtrojan ichtrojan Dec 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ($otp instanceof Model) {
    $validity = $otp->created_at->addMinutes($otp->validity);
    
    return Carbon::now()->lt($validity);
}

return false;    

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion. I have added a condition to check the "valid" property in the model, as it could still be invalid even if Carbon::now()->lt($validity); returns true.

Copy link
Owner

@ichtrojan ichtrojan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dropped a few comments and suggested code change

Copy link
Owner

@ichtrojan ichtrojan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple more changes

@ichtrojan ichtrojan merged commit 47ed90e into ichtrojan:master Dec 23, 2024
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

Successfully merging this pull request may close these issues.

2 participants