https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28786 --- Comment #58 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #56)
There are several issues. Some are blockers, some could be dealt with on follow-up reports, as is already the case for storing the secret.
[1] This code pattern looks wrong: my $secret = Koha::AuthUtils::generate_salt( 'weak', 16 ); my $auth = Koha::Auth::TwoFactorAuth->new( { patron => $logged_in_user, secret => $secret } ); my $secret32 = $auth->generate_secret32; What you do with generate_secret32 is effectively wasting your own secret. Note that Auth::GoogleAuth uses 16 random numbers 0-31 to create a new secret. If you pass a secret, GoogleAuth will convert it to base32 internally. The synopsis in the module is wrong too. This is a blocker. But simple to correct.
Indeed, good catch. I think it's a leftover of a first version.
[2] The randomizer (which we should not use) depends on Math::Random::MT. This module says: This algorithm has a very uniform distribution and is good for modelling purposes but do not use it for cryptography. As long as we pass our own stronger secrets, no worries. See [1].
Sorry, you lost me here. Which randomizer module?
[3] Martin refers earlier to Authen::OATH. I follow him there, but no blocker.
Why? Did you see my answer on comment 23? On the other hand if we do not longer use the QR code generator from Auth::GoogleAuth there is no bad reason to not switch. But what are your good reasons to switch then? :)
[4] The argument used before "Also, it can generate QR codes out-of-the-box" is currently actually an issue. It creates a link like https://chart.googleapis.com/chart?&cht=qr with the secret as a URL GET parameter. That's obviously unsafe! Furthermore, this Google API has been deprecated for some years already. https://developers.google.com/chart/infographics/docs/qr_codes
We could use the $return_otpauth parameter to create the QR code ourselves. Personally, I would not use this in its current form. But I would propose to solve this issue on a follow-up report.
Agreed, I will be happy to test it.
[5] This one I did not yet fully tackle. But I have the impression that we have an issue in C4/Auth now. A few times already I had to login twice. This MIGHT be a blocker. Needs still investigation.
Hum, I didn't experience it. -- You are receiving this mail because: You are watching all bug changes.