[Bug 33904] New: 2FA registration fails if library name has non-latin characters
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Bug ID: 33904 Summary: 2FA registration fails if library name has non-latin characters Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: major Priority: P5 - low Component: Authentication Assignee: jonathan.druart+koha@gmail.com Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org CC: dpavlin@rot13.org Depends on: 28786 If the library name has non-latin characters the two-factor authentication registration fails with a JS error and a 500 server-side. POST /api/v1/auth/two-factor/registration: unhandled exception (Mojo::Exception)<<Can't escape \x{2714}, try uri_escape_utf8() instead at /usr/share/perl5/Auth/GoogleAuth.pm line 43. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28786 [Bug 28786] Two-factor authentication for staff client - TOTP -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33880 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 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=33904 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152038&action=edit Bug 33904: Add tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152039&action=edit Bug 33904: Fix 2FA registration when library name has non-latin characters If the library name contains non-latin characters, the 2FA registration process will fail with a JS alert coming from a 500 server-side. The problem is that Auth::GoogleAuth is expecting an already UTF8 encoded string. We should set the encoding correctly to make Auth::GoogleAuth deal with the URL escaping internally correctly, then decode on our own (in the REST API controller) Test plan: * Modify your logged in library name and add some non-latin characters (eg. "my ❤ library") * Turn on TwoFactorAuthentication * Go to your account > More > Manage 2FA * Click the enable button => Notice that you see the QR code and that both "issuer" and "key id" entries display the library name correctly. * Test the whole 2FA process, confirm that the library name is correctly displayed on the app you are using. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #3 from David Cook <dcook@prosentient.com.au> --- If someone else signs this one off, I'll QA it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152038|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152196 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152196&action=edit Bug 33904: Add tests 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=33904 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152039|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152197 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152197&action=edit Bug 33904: Fix 2FA registration when library name has non-latin characters If the library name contains non-latin characters, the 2FA registration process will fail with a JS alert coming from a 500 server-side. The problem is that Auth::GoogleAuth is expecting an already UTF8 encoded string. We should set the encoding correctly to make Auth::GoogleAuth deal with the URL escaping internally correctly, then decode on our own (in the REST API controller) Test plan: * Modify your logged in library name and add some non-latin characters (eg. "my ❤ library") * Turn on TwoFactorAuthentication * Go to your account > More > Manage 2FA * Click the enable button => Notice that you see the QR code and that both "issuer" and "key id" entries display the library name correctly. * Test the whole 2FA process, confirm that the library name is correctly displayed on the app you are using. 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=33904 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152198 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152198&action=edit Bug 33904: (follow-up) Perltidy With the recent introduction of a community perltidyrc, I felt it sensible to give the module a tidy with this patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |martin.renvoize@ptfs-europe | |.com --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Tests passing and UX also passing.. signing off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Martin Renvoize from comment #6)
Created attachment 152198 [details] [review] Bug 33904: (follow-up) Perltidy
With the recent introduction of a community perltidyrc, I felt it sensible to give the module a tidy with this patch.
I don't think we should do that. The first 2 patches will need to be backported and we don't want to confuse RMaints with unnecessary conflicts to solve. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch 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=33904 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152196|0 |1 is obsolete| | --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152218 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152218&action=edit Bug 33904: Add tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152197|0 |1 is obsolete| | --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152219&action=edit Bug 33904: Fix 2FA registration when library name has non-latin characters If the library name contains non-latin characters, the 2FA registration process will fail with a JS alert coming from a 500 server-side. The problem is that Auth::GoogleAuth is expecting an already UTF8 encoded string. We should set the encoding correctly to make Auth::GoogleAuth deal with the URL escaping internally correctly, then decode on our own (in the REST API controller) Test plan: * Modify your logged in library name and add some non-latin characters (eg. "my ❤ library") * Turn on TwoFactorAuthentication * Go to your account > More > Manage 2FA * Click the enable button => Notice that you see the QR code and that both "issuer" and "key id" entries display the library name correctly. * Test the whole 2FA process, confirm that the library name is correctly displayed on the app you are using. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152198|0 |1 is obsolete| | --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152220 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152220&action=edit Bug 33904: (follow-up) Perltidy With the recent introduction of a community perltidyrc, I felt it sensible to give the module a tidy with this patch. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The perltidy patch does include a fair bit of whitespace changes. We might want to leave it off as Jonathan suggested. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152220|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 --- 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=33904 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00 |23.11.00,23.05.01 released in| | CC| |fridolin.somers@biblibre.co | |m Status|Pushed to master |Pushed to stable --- Comment #14 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.05.x for 23.05.01 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33904 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00,23.05.01 |23.11.00,23.05.01,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=33904 --- 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