antonioribeiro / google2fa

A One Time Password Authentication package, compatible with Google Authenticator.
MIT License
1.83k stars 199 forks source link

verifyKeyNewer() not working as expected #163

Closed john-nyxcoder closed 2 years ago

john-nyxcoder commented 3 years ago

I'm using the following code:

$timestamp = Google2FA::verifyKeyNewer(Auth::user()->totp_secret, $request->otp_code, Auth::user()->last_login, 1);

if ($timestamp !== false) {
    dd("valid");
} else {
    dd("invalid");
}

To set the timestamp itself I used the following code (just for testing verifyKeyNewer()):

public function setTimestamp(){
    Auth::user()->update([
        'last_login' => Carbon::now(),
    ]);
    dd("done");
}

After setting the timestamp, I tried to enter a current TOTP code to the validate function above. The timeframe is set to 1 as the login function too:

if($google2fa->verifyKey(Auth::user()->totp_secret, $request->otp_code, 1))

It says valid all the time, when using verifyKeyNewer() any idea where things go wrong? I want to use this library, not the laravel package / bridge.

john-nyxcoder commented 3 years ago

@jhoff maybe this is related to your problem too, isn't it? See: https://github.com/antonioribeiro/google2fa/issues/158

zortje commented 3 years ago

The $timestamp variable is actually the timestamp divided with the key regeneration, so you would need to use the following code.

$keyRegen = 30; // 30 is the default

$timestamp = Google2FA::verifyKeyNewer(Auth::user()->totp_secret, $request->otp_code, Auth::user()->last_login / $keyRegen, 1);

if ($timestamp !== false) {
    // Here the $timestamp variable is already divided with key regen, so if you want to parse this timestamp, remember to multiply $timestamp with $keyRegen first
    dd("valid");
} else {
    dd("invalid");
}
rubensrocha commented 3 years ago

My Solution(Laravel 8).

User Model Field: two_factor_time (timestamp default null)

2Fa Middleware:

if(session()->get('google2fa.otp_timestamp') || auth()->user()->two_factor_time){
    //redirect to 2fa route;
}
return $next($request);

2Fa Controller (show view):

if (session()->get('google2fa.otp_timestamp') && auth()->user()->two_factor_time) {
    //redirect to loggedin area
}
return view('2fa');

2Fa Controller (send/validate token):

$twoFactor = new Google2FA();
$user = User::find(auth()->user()->id);
$request->validate([
    'token' => ['required', 'digits:6',
        function ($attribute, $value, $fail) use ($twoFactor, $user) {
            $lastCheck = strtotime($user->two_factor_time) / 30;
            $timestamp = $twoFactor->verifyKeyNewer($user->two_factor_secret, $value, $lastCheck);
            if ($timestamp === false) {
                $fail('Invalid or expired Token');
            }else{
                $now = \Carbon\Carbon::now();
                session()->put('google2fa.otp_timestamp', $now->toDateTimeString());
                $user->update(['two_factor_time' => $now]);
            }
        }],
 ]);
//redirect user to secure area

I used a timestamp in the session and in the database to validate tokens already used before, because when logging out the session is destroyed, preventing the validation of tokens already used.

This way, if the DB field or the session field is null (or both), the middleware redirects the user to the 2FA page. The DB and session timestamp field must be defined for the user to access the secure area.

imran0 commented 2 years ago

@zortje @rubensrocha I have tried dividing the unix timestamp by 30 as described in your post but the package still claims the code is incorrect. It simply doesn't accept any OTP's, both new or old.

I have verified the old timestamp value is accurate (and there are no timezone issues), could you advise how you got this working as I am not having much luck.

Isild commented 2 years ago

@imran0 there is that what @rubensrocha and @zortje said. You must divide timestamp by 30. But if you change key regeneration you must use another number. Best way to get actual number is use this:

$timeDiv = $this->google2fa->getKeyRegeneration();

And next when you use

$valid2Fa = $this->google2fa->verifyKeyNewer($user->google2fa_secret, $key, $user->google2fa_last_use/$timeDiv, $window);

it should works.

I don't know how looks yours timestamps format but I use Carbon::now()->timestamp which give me seconds since the Unix Epoch(1643290902) and it works.