Hi,

It seems that the GA login key is assigned to the user, even if he does not approve the code. This causes a serious problem I think. If the user accidentally press the generate button and does not scan the new code, he will not be able to login anymore.

I think the intention of the user interface with the checkbox that the user has to approve the new key is good. But it seems not to work, as intended.

the patch is more complex. I think the simplest is to create a new function in GoogleAuthenticator which creates the key, but does not directly store it.

+
+ // create "user" withOUT insert
+ function unapprovedUser($username, $ttype="HOTP", $key = "", $hexkey="") {
+ $ttype = strtoupper($ttype);
+ if($ttype != "HOTP" && $ttype !="TOTP") return false;
+ if($key == "") $key = $this->createBase32Key();
+ $hkey = $this->helperb322hex($key);
+ if($hexkey != "") $hkey = $hexkey;
+
+ $token = $this->internalGetData($username);
+ $token["tokenkey"] = $hkey;
+ $token["tokentype"] = $ttype;
+ return $token;
+ }

this requires also to extend the URL function, since it cannot load the data from the database:

// create a url compatibile with google authenticator.
- function createURL($user) {
+ function createURL($user,$data=NULL) {
// oddity in the google authenticator... hotp needs to be lowercase.
- $data = $this->internalGetData($user);
+ if ($data == NULL)
+ $data = $this->internalGetData($user);
$toktype = $data["tokentype"];
$key = $this->helperhex2b32($data["tokenkey"]);
// token counter should be one more then current token value, otherwise

finally, after the key is approved by the user, we would need something like:

+
+ // data field manipulation bits
+ function setData($username, $data) {
+ $this->internalPutData($username, $data);
+ }

Comments

Jelle_S’s picture

fixed in latest dev version.

Jelle_S’s picture

Status: Active » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

provided patch idea