[Koha-bugs] [Bug 28786] Two-factor authentication for staff client - TOTP

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

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

--- Comment #28 from Jonathan Druart <jonathan.druart+koha at gmail.com> ---
(In reply to Martin Renvoize from comment #27)
> I've finally found a moment to look at this.
> 
> Whilst I like Tomas's proposal to normalise the database by having a
> distinct table.. I think that can certainly come as a followup later.

If we are talking about 3NF, I don't think having a separate table with a
single column (different than the FK) is what can be called normalisation.

If that is what you have in mind, I think it's wrong:
borrowers < 1-1 > secrets
                  - borrowernumber (FK borrowers.borrowernumber)
                  - secret


> I do wonder why you've chosen a super new cpan module for this,
> Auth::GoogleAuth. I had considered Authen::OAth myself, because although
> it's a little smaller (you'de need something to generate QR codes on top),
> it's further up the CPAN river and is written by a trusted author... might
> even already have a debian package.

I've compared them all.
There is a recent version of Auth::GoogleAuth, which show it's still an active
project:
https://metacpan.org/pod/Auth::GoogleAuth
1.03      2021-01-10
1.02      2017-12-26
1.01      2015-08-03
1.00      2015-08-03

Note that 1.03 is only in bullseye.

https://metacpan.org/pod/Authen::OATH
2.0.1     2017-04-12
1.0.0     2010-05-25

The author is active in different other projects:
https://metacpan.org/author/GRYPHON

Also, it can generate QR codes out-of-the-box.

> Finally, and what I would consider a QA failure.. the secret is stored in
> plain text in the database. I feel this should be stored encrypted using the
> a passphrase stored in the config file (we could re-use
> api_secret_passphrase or add a new field to the config.

How would you do that?
You encrypt when you don't want to know the secret, and you will never know it
again.
Here we need the secret (and not one of its hashed version) as it's an external
module that is doing the comparison job. We could eventually generate a new
secret containing borrowers.secret, like we concatenate bcrypt_settings (or
part of it) with borrowers.secret. That could work but feel weird.

-- 
You are receiving this mail because:
You are watching all bug changes.