[Bug 32011] New: 2FA - Problem with qr_code generation
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Bug ID: 32011 Summary: 2FA - Problem with qr_code generation Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart+koha@gmail.com Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 29873 Looks like an major bug is caught by this selenium test: STRACE: /usr/share/perl5/Try/Tiny.pm:123 in Selenium::Remote::Driver::catch {...} /usr/share/perl5/Selenium/Remote/Driver.pm:361 in Try::Tiny::try (eval 541):1 in Selenium::Remote::Driver::__ANON__ (eval 543):2 in Selenium::Remote::Driver::__ANON__ /usr/share/perl5/Selenium/Remote/Driver.pm:1058 in Selenium::Remote::Driver::_execute_command /kohadevbox/koha/t/lib/Selenium.pm:203 in Selenium::Remote::Driver::execute_script t/db_dependent/selenium/authentication_2fa.t:264 in t::lib::Selenium::wait_for_ajax /usr/share/perl/5.32/Test/Builder.pm:334 in main::__ANON__ /usr/share/perl/5.32/Test/Builder.pm:334 in (eval) /usr/share/perl/5.32/Test/More.pm:809 in Test::Builder::subtest t/db_dependent/selenium/authentication_2fa.t:291 in Test::More::subtest # Looks like you planned 7 tests but ran 2.t/db_dependent/selenium/authentication_2fa.t .. 4/5 # Failed test 'Enforce 2FA setup on first login' # at t/db_dependent/selenium/authentication_2fa.t line 291.Error while executing command: unexpected alert open: Dismissed user prompt dialog: [object Object] at /usr/share/perl5/Selenium/Remote/Driver.pm line 411. at /usr/share/perl5/Selenium/Remote/Driver.pm line 356. There is a 500 in the logs: "POST /api/v1/app.pl/api/v1/auth/two-factor/registration HTTP/1.1" 500 That is caused by [ERROR] POST /api/v1/auth/two-factor/registration: unhandled exception (Mojo::Exception)<<Overflow error. version 8 total bits: 1268 max bits: 1232>> It's coming from GD::Barcode, in my understanding we should increase ModuleSize. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29873 [Bug 29873] 2FA: Generate QR code without exposing secret via HTTP GET -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32010 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 142696 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142696&action=edit Bug 32011: 2FA - Prevent qr_code to not be generated Caught by selenium/authentication_2fa.t # Failed test 'Enforce 2FA setup on first login' # at t/db_dependent/selenium/authentication_2fa.t line 291.Error while executing command: unexpected alert open: Dismissed user prompt dialog: [object Object] at /usr/share/perl5/Selenium/Remote/Driver.pm line 411. at /usr/share/perl5/Selenium/Remote/Driver.pm line 356. There is a 500 in the logs: "POST /api/v1/app.pl/api/v1/auth/two-factor/registration HTTP/1.1" 500 That is caused by [ERROR] POST /api/v1/auth/two-factor/registration: unhandled exception (Mojo::Exception)<<Overflow error. version 8 total bits: 1268 max bits: 1232>> It's coming from GD::Barcode, in my understanding we should increase ModuleSize. Test plan: Remove all other subtests from the selenium to speed up its exec, then run it in a loop. Without this patch the tests fail quite often (1/10), now it should not fail with this error (maybe another one, see bug 32010) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142696|0 |1 is obsolete| | --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 142697 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142697&action=edit Bug 32011: 2FA - Prevent qr_code to not be generated Caught by selenium/authentication_2fa.t # Failed test 'Enforce 2FA setup on first login' # at t/db_dependent/selenium/authentication_2fa.t line 291.Error while executing command: unexpected alert open: Dismissed user prompt dialog: [object Object] at /usr/share/perl5/Selenium/Remote/Driver.pm line 411. at /usr/share/perl5/Selenium/Remote/Driver.pm line 356. There is a 500 in the logs: "POST /api/v1/app.pl/api/v1/auth/two-factor/registration HTTP/1.1" 500 That is caused by [ERROR] POST /api/v1/auth/two-factor/registration: unhandled exception (Mojo::Exception)<<Overflow error. version 8 total bits: 1268 max bits: 1232>> It's coming from GD::Barcode, in my understanding we should increase 'Version'. Test plan: Remove all other subtests from the selenium to speed up its exec, then run it in a loop. Without this patch the tests fail quite often (1/10), now it should not fail with this error (maybe another one, see bug 32010) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- *** Bug 32010 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Sometimes seeing a 500 in Koha logs which is suspect: Can't use string ("Koha::Patron") as a HASH ref while "strict refs" in use at /kohadevbox/koha/Koha/Object.pm line 830 Koha::Object::_columns('Koha::Patron') called at /kohadevbox/koha/Koha/Object.pm line 858 Koha::Object::AUTOLOAD('Koha::Patron', 'HASH(0x5579840b17b8)') called at /kohadevbox/koha/C4/Auth.pm line 1772 C4::Auth::check_cookie_auth('b712cd069e4808fa8a5da0e29d098b30', undef, 'HASH(0x557983e6ac28)') called at /kohadevbox/koha/C4/Auth.pm line 852 C4::Auth::checkauth('CGI=HASH(0x557986d44b18)', 0, 'HASH(0x557986cf0118)', 'intranet', undef, 'intranet-main.tt') called at /kohadevbox/koha/C4/Auth.pm line 189 C4::Auth::get_template_and_user('HASH(0x557986d0b648)') called at /kohadevbox/koha/mainpage.pl line 41 But not related to this change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Tests ran 50x without failure. Looks good! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_22_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_22_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I cannot run it on the laptop right now, but I've pushed it. Will continue tomorrow. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #6)
I cannot run it on the laptop right now, but I've pushed it. Will continue tomorrow.
I dont understand that. I am looking now here. But this is not just a failing test. This is a module change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Without this patch on commit fd7bc8a9b8de5a8fdea36b046e4621760366dbf0, 2FA seems to work just fine on Debian 11 via the interface. The Selenium stuff is another story. But it should not dictate us to make changes just to pass Selenium imo. Jonathan: Please provide some more details on the system you are running on. And could you confirm that the interface does not work either anymore on your system? If not, this is not a major bug. But maybe just a Selenium issue? I wouldnt mind not having these Selenium tests btw. But I could be biased :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |BLOCKED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Tested both Enable and Enforced btw. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- GD::Barcode::QRcode elsif ($iTotalBits > $iMaxDatBits){ die "Overflow error. version $oSelf->{Version}\n" . "total bits: $iTotalBits max bits: $iMaxDatBits\n"; } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Yes, it's coming from GD::Barcode::QRcode 310 sub _calcVersion($$$) { 311 my ($oSelf, $iTtlBits, $raPlusWords) = @_; 312 313 my %hMaxDatBits= ( 314 'M' =>[ 0, 315 128, 224, 352, 512, 688, 864, 992, 1232, 1456, 1728, So the patch jumps from 1232 to 1728 (in my tests the value was almost always < ~1350, but I saw it failed with 1456. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #7)
(In reply to Tomás Cohen Arazi from comment #6)
I cannot run it on the laptop right now, but I've pushed it. Will continue tomorrow.
I dont understand that. I am looking now here. But this is not just a failing test. This is a module change.
Not in my understand, not a module change: Version Version ie. size of barcode image (Default = auto select). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #8)
Without this patch on commit fd7bc8a9b8de5a8fdea36b046e4621760366dbf0, 2FA seems to work just fine on Debian 11 via the interface.
The Selenium stuff is another story. But it should not dictate us to make changes just to pass Selenium imo.
It's failing randomly, so you may not notice it via the interface with only a couple of tries.
Jonathan: Please provide some more details on the system you are running on. And could you confirm that the interface does not work either anymore on your system? If not, this is not a major bug. But maybe just a Selenium issue?
I am pretty sure it's not a selenium issue, it's coming from the Koha::REST::V1::TwoFactorAuth->registration. If the selenium test make it fails, a normal browser can make it fail.
I wouldnt mind not having these Selenium tests btw. But I could be biased :)
Lol, they are very strong and caught a bug here, so... We have some shitty selenium/www tests, but those ones are great IMO. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Doing some debugging now with: use Modern::Perl; use Convert::Base32; use GD::Barcode; use Koha::AuthUtils; my $duplicate_secret = 4; foreach( 1.. 50) { my $secret = Koha::AuthUtils::generate_salt( 'weak', 16 ); $secret = $secret x $duplicate_secret; my $otpauth = encode_base32($secret); print "$otpauth\n"; GD::Barcode->new( 'QRcode', $otpauth, { Ecc => 'M', Version => 10, ModuleSize => 4 } ); } The problem is the length of the secret. I think that you have a very long secret? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- If I am corrent, a normal secret will give values like: Version 10 Module 4 L104: 228 1728 Version 8 Module 4 L104: 220 1232 where L104 is a debug line in QRcode.pm: warn "L104: $iTotalBits $iMaxDatBits\n"; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #13)
Lol, they are very strong and caught a bug here, so... We have some shitty selenium/www tests, but those ones are great IMO.
No reason to overreact imo. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Try with a very long userid. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #17)
Try with a very long userid.
otpauth://totp/QcQ4WNT:QcQ4WNT_QAQDfB8?secret=3ml253y4p3er7axhrwdlz6by2e&issuer=QcQ4WNT vs otpauth://totp/BtwjEx:BtwjEx_wp9YPWXB91qaOkkwuxT9GydFBV9cMeUlHOLMDrn0?secret=qxiff74gn7jtdiengrfnpreywu&issuer=BtwjEx And 78 my $issuer = $patron->library->branchname; 79 my $key_id = sprintf "%s_%s", 80 $issuer, ( $patron->email || $patron->userid ); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #16)
(In reply to Jonathan Druart from comment #13)
Lol, they are very strong and caught a bug here, so... We have some shitty selenium/www tests, but those ones are great IMO.
No reason to overreact imo.
On the fact that these selenium tests are great? Or that we have several bad ones? I don't understand what you mean.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #17)
Try with a very long userid.
Yeah, still looking for that spot in the code where we build the secret ;) But its length is the culprit. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #18)
otpauth://totp/BtwjEx: BtwjEx_wp9YPWXB91qaOkkwuxT9GydFBV9cMeUlHOLMDrn0?secret=qxiff74gn7jtdiengrfnpr eywu&issuer=BtwjEx
otpauth://totp/BtwjEx:BtwjEx_wp9YPWXB91qaOkkwuxT9GydFBV9cMeUlHOLMDrn0?secret=qxiff74gn7jtdiengrfnpreywu&issuer=BtwjEx L104: 956 1728 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Okay, convinced :) This will probably get us out of trouble in most cases. Perhaps we should have a look at limiting the secret length at some point. So Selenium is just fantastic :) Ugghh -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=32011 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Status now reflects where we are. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #24 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Marcel de Rooy from comment #22)
So Selenium is just fantastic :) Ugghh
XD, well in this case it helped spot a random wrong behavior in Koha IIUC. But for other thing tests themselves randomly fail. So Ugghh! (the use of wait_for_ajax seems to help so thing get better!) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.11.00 released in| | --- Comment #25 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #7)
(In reply to Tomás Cohen Arazi from comment #6)
I cannot run it on the laptop right now, but I've pushed it. Will continue tomorrow.
I dont understand that. I am looking now here. But this is not just a failing test. This is a module change.
I trusted Jonathan knows what he's doing and I don't run Selenium on ktd on the laptop because it always drains my battery (macOS). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 --- Comment #26 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #23)
Status now reflects where we are.
Thanks :-D I was waiting for more reviews and debugging to take place today. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.11.00 |22.11.00, 22.05.07 released in| | CC| |lucas@bywatersolutions.com Status|Pushed to master |Pushed to stable --- Comment #27 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 22.05.x for upcoming 22.05.07 release -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_22_05_candidate, | |rel_22_11_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |arthur.suzuki@biblibre.com --- Comment #28 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- depends on 2FA auth, not present in 21.11.x branch. Not backporting this one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32011 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |33880 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33880 [Bug 33880] "Enable two-factor authentication" fails if patron's library branchname is too long -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org