[Bug 33880] New: "Enable two-factor authentication" fails if patron's library branchname is too long
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Bug ID: 33880 Summary: "Enable two-factor authentication" fails if patron's library branchname is too long Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Authentication Assignee: koha-bugs@lists.koha-community.org Reporter: pedro.amorim@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org CC: dpavlin@rot13.org, jake.deery@ptfs-europe.com, jonathan.druart+koha@gmail.com, jonathan.field@ptfs-europe.com, m.de.rooy@rijksmuseum.nl, martin.renvoize@ptfs-europe.com, ryan.henderson@ptfs-europe.com Depends on: 32011 To reproduce, on ktd: 1) Enable TwoFactorAuthentication system preference 2) Go to "Administration->Libraries" edit the library's name "Institut Protestant de Théologie" to "Institut Protestant de Théologie and another word" 3) Go to "My account" in the top right corner menu 4) Click the "Edit" button in the "Library use" box and set the library to the above library 5) Go back to "My account" and click "More" button, select "Manage two-factor authentication" 6) Click "Enable two-factor authentication" button 7) It explodes Error logs: [ERROR] POST /api/v1/auth/two-factor/registration: unhandled exception (Mojo::Exception)<<Overflow error. version 10 total bits: 2036 max bits: 1728>> This is the same (or very similar) issue discussed in bug 32011 Bumping the version from 10 to something higher in GD::Barcode::new on Koha/Auth/TwoFactorAuth.pm works around this issue, but I think we should discuss and come up with a permanent solution. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 [Bug 32011] 2FA - Problem with qr_code generation -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- What about using branchcode instead of branchname? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (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? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #4 from Jake Deery <jake.deery@ptfs-europe.com> --- 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 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #5 from Jake Deery <jake.deery@ptfs-europe.com> --- Update to my previous comment: https://metacpan.org/release/KWITKNR/GD-Barcode-1.15/source/Barcode/QRcode.p... 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 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- If that works, it's certainly the easiest and long term solution! -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #7 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 151981 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=151981&action=edit 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.p... 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> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #8 from 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! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jake.deery@ptfs-europe.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #151981|0 |1 is obsolete| | --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152037 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152037&action=edit 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.p... 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33904 --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Found bug 33904 when testing this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152037|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152194 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152194&action=edit 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.p... 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Trivial fix for a nasty problem.. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00 |23.11.00,23.05.02 released in| | Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for all the hard work! Pushed to 23.05.x for the next release -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00,23.05.02 |23.11.00,23.05.02,22.11.08 released in| | Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 --- Comment #15 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to oldstable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org