[Koha-bugs] [Bug 28787] Send a notice with the TOTP token

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28787

--- Comment #7 from Jonathan Druart <jonathan.druart+koha at 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.