Bug 33880

What about using branchcode instead of branchname?

(In reply to Jonathan Druart from comment #1)
> What about using branchcode instead of branchname?

10 characters. Should be fine, I guess.
Kind of weird that this CPAN module stumbles over such a thing..
Thought that issuer was just a label?

I think the length and characters in branchname could quickly become an issue for our libraries. But the branchcode is often nothing the user would connect with the library. Looking at my Google Authenticator app, there are not many clues to what the login is for. 

I think the best solution could be a configuration option in branches, an abbreviated library name. Often institutions will have an abbreviation that is well known or could create one for this use case. 

And maybe we could also show the options as a hint on the form?

Hello folks,

Having a quick read of GD::Barcode::QRcode, it looks like setting version to automatic allows the library to determine which QRcode to pick – https://metacpan.org/pod/GD::Barcode::QRcode#Version (note: it appears Version => 1 is automatic mode, based on the source code).

Do we need to specify which QRcode to use, or should we change it to automatic / 1? I'd imagine most authenticator apps will read any old code (that is what QRcodes were invented for, after all!)

Many thanks,
Jake

Update to my previous comment:

https://metacpan.org/release/KWITKNR/GD-Barcode-1.15/source/Barcode/QRcode.pm#L349 

It actually looks like ommitting the version entirely causes the QRcode generator to automatically iterate up until it finds the right place to stop. Cool, eh?

(Thanks Pedro!)
Jake

If that works, it's certainly the easiest and long term solution!

Created attachment 151981 [details] [review]
Bug 33880: Remove Version param from GB::Barcode->new

Looking at the code here
     if (Koha/Auth/TwoFactorAuth.pmoSelf->{Version}){        #--- auto version select
        for(->{Version}=1; ->{Version} <= 40; ++->{Version}) {
            last if ({->{Ecc}}->[->{Version}]
                        >=  + ->[->{Version}]);
        }
   }

in https://metacpan.org/release/KWITKNR/GD-Barcode-1.15/source/Barcode/QRcode.pm#L349

It appears the version parameter is not required, and will default to whatever version has the necessary bits for the input words given

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Submitting this in Jake's behalf with my signature on it after looking at the code and testing.

Keeping in "Needs sign-off" as would be great to have others looking into this!

Created attachment 152037 [details] [review]
Bug 33880: Remove Version param from GB::Barcode->new

Looking at the code here
     if (Koha/Auth/TwoFactorAuth.pmoSelf->{Version}){        #--- auto version select
        for(->{Version}=1; ->{Version} <= 40; ++->{Version}) {
            last if ({->{Ecc}}->[->{Version}]
                        >=  + ->[->{Version}]);
        }
   }

in https://metacpan.org/release/KWITKNR/GD-Barcode-1.15/source/Barcode/QRcode.pm#L349

It appears the version parameter is not required, and will default to whatever version has the necessary bits for the input words given

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Found bug 33904 when testing this.

Created attachment 152194 [details] [review]
Bug 33880: Remove Version param from GB::Barcode->new

Looking at the code here
     if (Koha/Auth/TwoFactorAuth.pmoSelf->{Version}){        #--- auto version select
        for(->{Version}=1; ->{Version} <= 40; ++->{Version}) {
            last if ({->{Ecc}}->[->{Version}]
                        >=  + ->[->{Version}]);
        }
   }

in https://metacpan.org/release/KWITKNR/GD-Barcode-1.15/source/Barcode/QRcode.pm#L349

It appears the version parameter is not required, and will default to whatever version has the necessary bits for the input words given

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Trivial fix for a nasty problem.. Passing QA

Pushed to master for 23.11.

Nice work everyone, thanks!

Thanks for all the hard work!

Pushed to 23.05.x for the next release

Nice work everyone!

Pushed to oldstable for 22.11.x