alexandregz / twofactor_gauthenticator

This RoundCube plugin adds the 2-step verification(OTP) to the login proccess
MIT License
216 stars 76 forks source link

Constantly logged out after inputting 2FA code #139

Closed thakyZ closed 3 years ago

thakyZ commented 3 years ago

I enabled my 2FA code for my account and checked the generated code where it allows you to check it and it passed. Now when I go to login the username and password checks out, the 2FA code seems to check out but I get automatically logged out right away. I should mention since people had issues with this but I am on Cloudflare, and it, I guess, has issues with RC. I have no idea why this is happening as absolutely no logs are saying anything. Your plugin's logs just state the local ipv6 address twice, maybe it's Cloudflare's, not sure?

I am using RC 1.4.11 and dev-master (0a71af6) of your plugin.

dryas commented 3 years ago

I have the same issue with the same combination of versions.

WanWizard commented 3 years ago

Me too. After I disabled activate in the database, I could login again with username and password. After going to the 2FA screen, I noticed that the "Check code" button always responded "Incorrect code", even though I'm sure it is correct.

When I manually increased the time drift in the code from 2 ( = 60sec ) , suddenly it started accepting the code again, with a drift now approaching 3 and a half day !

Weirdly enough, time on the server and the mobile is absolutely correct, and both synced with NTP. Only difference is the timezone (server runs UTC, phone is on GMT), but that shouldn't matter, as it uses unix time which is always UTC.

Even entering a new secret, generating a new QR code and looking that in the authenticator doesn't fix this issue.

@alexandregz, any idea?

alexandregz commented 3 years ago

Hi, can you reset user.preferences (set to null)?

WanWizard commented 3 years ago

I can. but that doesn't solve the problem.

Didn't check "activate", entered a new secret, saved, opened the QR code, scanned it in Google authenticator, then typed in the generated code in the "check code" field, and got "incorrect code" back...

WanWizard commented 3 years ago

Used the Google chart API to create a QR code:

https://chart.googleapis.com/chart?chs=200x200&chld=M|0&cht=qr&chl=otpauth%3A%2F%2Ftotp%2FTEST%3Fsecret%3DSDMFRLWBBSTAYH3N

Then used this script to test the codes generated by the app:

<?php
require_once 'PHPGangsta/GoogleAuthenticator.php';

$secret = 'SDMFRLWBBSTAYH3N';
$oneCode = '410431'; // change to whatever the app displays

$ga = new PHPGangsta_GoogleAuthenticator();

echo "Checking Code '$oneCode' and Secret '$secret':\n";

$checkResult = $ga->verifyCode($secret, $oneCode, 2);    // 2 = 2*30sec clock tolerance
echo $checkResult ? 'OK' : 'FAILED';
echo PHP_EOL;

And ran it a few times. Code always verifies just fine.

From that, I conclude that the issue is not with the Google Authenticator code, the app, or a time issue between client and server, the the only two variables remaining, the plugin code, and the QRcode generator (the js code).

I then checked the plugin code, but there are no issues with capturing the code entered, and passing it to the GoogleAuthenticator object, so an issue there is unlikely either.

Remains, the js code that generates the QR code?

WanWizard commented 3 years ago

Hmm, maybe not.

I've now had Google generate a QR code with the secret I've used in Roundcube. And it generates exactly the same codes as the one generated with the QRcode js code. So that's not it either,

Leaves the code of the plugin itself.

WanWizard commented 3 years ago

Was a bit too quick. The test code generates a code, and then verifies it. The verify code verifies by generating a code, and then compares, so using this test code, this always succeeds.

However, if I put my own secret in the test code, and have it generate codes for a minute, it doesn't generate the same codes as the app does. So it could also be the generating code in GoogleAuthenticator isn't correct (possibly only when using a recent PHP version).

WanWizard commented 3 years ago

F*CK,

Sorry, found the problem.

The Secret Key can only be constructed from base32 valid characters, A-Z, 2-7. No others. But the plugin doesn't validate its input, so you can type in anything you want, but are not warned the internal secretkey generation will fail (also without error), and with that, all your attempts to validate the code!

Alternatively, have the plugin accept all input, but convert all keys to base32 before saving, so the correct representation is used internally.

alexandregz commented 3 years ago

cool 👍

yep, the key it's base32 (you can see PHPGansta library https://github.com/alexandregz/twofactor_gauthenticator/blob/master/PHPGangsta/GoogleAuthenticator.php#L18)

WanWizard commented 3 years ago

Does simply closing this mean you're not going to do anything about it? Like informing the user (who can't and won't want to look at code) what kind of input is needed, and validate the the input on save?