Skip to content

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