https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28787 --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Martin Renvoize from comment #4)
Hmm, I'm not so sure about this.. whilst I understand TOTP over SMS delivery makes sense as SMS is in theory immediate delivery.. Email has lots of caveats around delivery speed and so it's more common to send an HOTP or even a simple random string OTP in the email case due to the timeout factor?
Well, the description was clear enough and the bug has been flagged as sponsored. I don't think it's fair to ask for a rewrite once the code has been written. (In reply to David Cook from comment #5)
Comment on attachment 135287 [details] [review]
+ if ( $content =~ m|\[% otp_token %\]| ) { + my $patron = Koha::Patrons->find(C4::Context->userenv->{number}); + $tt_params->{otp_token} = Koha::Auth::TwoFactorAuth->new({patron => $patron})->code; + }
This looks like a hack. We should pass the code in via a public method/function. That said, it looks like this OTP will wind up in the message_queue table?
Hum yes, maybe. It seemed weird to add a pattern/variable that would be available for a single notice template. (In reply to David Cook from comment #6)
Another thing we could do is add the range parameter to the verify() function I believe. At the moment, it looks like we're not following the recommendations of rfc6238 to allow additional backwards steps. (Typically, with a TOTP, you can usually use up to 2-3 old codes and still work to allow for clock drift and slow users.)
Yes, that's a bug. I was pretty sure it was allowing at least 1 old code. It's in the POD of ->verify, and members/two_factor_auth.pl, but C4/Auth.pm -- You are receiving this mail because: You are watching all bug changes.