https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Comma delimited| |Bibliotek i Västmanland list of Sponsors| | Sponsorship status|--- |Sponsored URL| |https://wiki.koha-community | |.org/wiki/PIN_Code_Authenti | |cation -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #1 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 200364 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200364&action=edit Bug 39943: add systempreference EnablePinAuthentication Sponsored-by: Bibliotek i Västmanland -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #3 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 200366 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200366&action=edit Bug 39943: pin field schema [DO NOT PUSH] Sponsored-by: Bibliotek i Västmanland -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #5 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 200368 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200368&action=edit Bug 39943: allow_pin_code field schema [DO NOT PUSH] Sponsored-by: Bibliotek i Västmanland -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #7 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 200370 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200370&action=edit Bug 39943: pin code authentication via SIP This patch implements pin code authentication via SIP when enabled on the SIP account. Sponsored-by: Bibliotek i Västmanland -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #9 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 200372 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200372&action=edit Bug 39943: add systempreference MinPinLength Sponsored-by: Bibliotek i Västmanland -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #11 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 200374 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200374&action=edit Bug 39943: add Koha::Patron::set_pin Method for setting the pin code, analogous to setting the password. Sponsored-by: Bibliotek i Västmanland -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #13 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 200376 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200376&action=edit Bug 39943: Set PIN for staff. Let staff with edit_patron permission set PIN for others. Sponsored-by: Bibliotek i Västmanland -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #15 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 200378 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200378&action=edit Bug 39943: Change PIN code for OPAC To test: 0. After applying these patches: yarn build koha-upgrade-schema kohadev restart_all 1. Either set system preferences minPasswordLength to 4 and RequireStrongPassword to "Don't require" or replace example passwords in this test plan with conforming passwords. 2. Create a borrower category PIN for testing. 3. Create a borrower with userid 'pintest' i category PIN. Set password '0000'. 4. Verify that it is possible to login to opac with 'pintest'. 5. Set some variables (command line on koha server/docker): instance=kohadev category=PIN patron=pintest su=zippin sp=zippin pincode='0000' password=pintest inst=PINTEST kohabin=/kohadevbox/koha/misc 6. Run migration script: sudo koha-shell -c "perl '$kohabin'/migration_tools/move_pin_codes.pl --category '$category'" "$instance" sudo koha-shell -c "perl '$kohabin'/migration_tools/move_pin_codes.pl --category '$category' --confirm" "$instance" 7. Verify that it is no longer possible to login to OPAC with 'pintest' (because the password hash is moved to the pin field). 8. Enable the system preference 'EnablePinAuthentication'. 9. Create a patron with userid 'zippin' password 'zippin' and give it circulate permissions (to allow it to connect to the SIP-server). 10. Also, set password 'pintest' on the pintest borrower. 11. Go to Koha administration -> Self-service circulation (SIP2) -> Institutions and add an institution with name 'PINTEST'. 12. Under SIP2 accounts add new account tied to userid 'zippin' and institution 'PINTEST'. 13. Verify that the patron password of 'pintest' is valid (the SIP response SHOULD NOT contain the text 'Invalid password') $kohabin/sip_cli_emulator.pl --address 127.0.0.1 --port 6001 \ --su "$su" --sp "$sp" --location "$inst" \ --message patron_information \ --patron "$patron" --password "$password" 14. Verify that the pin code for 'pintest' is NOT valid (the SIP response MUST contain the text 'Invalid password') $kohabin/sip_cli_emulator.pl --address 127.0.0.1 --port 6001 \ --su "$su" --sp "$sp" --location "$inst" \ --message patron_information \ --patron "$patron" --password "$pincode" 15. On the SIP account for 'zippin' select "yes" for the parameter "Allow pin code" and save the settings. Also restart sip server: koha-sip --restart $instance 16. Verify that the pin code for 'pintest' is now valid (the SIP response must NOT contain the text 'Invalid password') $kohabin/sip_cli_emulator.pl --address 127.0.0.1 \ --port 6001 --su "$su" --sp "$sp" --location "$inst" \ --message patron_information --patron "$patron" \ --password "$pincode" 17. In the staff interface go to the patron 'pintest' and click on 'Change password or PIN' 18. In the fields "New pin" and "Confirm new pin" try inputting some letters, 3 digits, many digits and mismatching "Confirm new pin" and verify that validation errors "Pin code too short", "Pin code must consist of only digits" and "Please enter the same pin code as above" appears. 19. Enter the pin code '1111' into both pin code fields. In a separate tab/browser window, change the system preference MinPinLength to 6. Go back to the previous tab/browser window and click "save" to save the new PIN code. Verify that the there is a validation error when saving. 20. Change "MinPinLength" to 4. And change the pin code for patron 'pintest' to '1111'. 21. Verify that the new pin code 1111 for 'pintest' is now valid (the SIP response must NOT contain the text 'Invalid password') $kohabin/sip_cli_emulator.pl --address 127.0.0.1 --port 6001 \ --su "$su" --sp "$sp" --location "$inst" \ --message patron_information --patron "$patron" --password 1111 22. Log in to the OPAC with userid/password pintest/pintest. 23. Go to your account and click "Change PIN" in the left menu. 24. In the fields "New PIN code" and "Repeat new PIN code" try inputting some letters, 3 digits, many digits and mismatching "Confirm new pin" and verify that validation errors "Pin code too short", "Pin code must consist of only digits" and "Please enter the same pin code as above" appears. 25. Type the wrong password in the password field, and enter the same pin code in both pincode fields. Press save and verify that there is an error "Your password was entered incorrectly...". Verify that the pin code was not changed (repeat 21.) 26. Enter the pin code '2222' into both pin code fields. In a separate tab/browser window, change the system preference MinPinLength to 6. Go back to the previous tab/browser window and click "save" to save the new PIN code. Verify that the there is a validation error when saving. 27. Change "MinPinLength" to 4. From the OPAC change the pin code for patron 'pintest' to '2222'. 28. Verify that the new pin code 2222 for 'pintest' is now valid (the SIP response must NOT contain the text 'Invalid password') $kohabin/sip_cli_emulator.pl --address 127.0.0.1 --port 6001 \ --su "$su" --sp "$sp" --location "$inst" \ --message patron_information --patron "$patron" --password 2222 29. Set an email address on the pintest borrower (pintest@example.com) and set the system preference NotifyPasswordChange to "Notify". 30. Set the pin code to '3333' on the pintest borrower and verify that a "Library account PIN code change notification" was added to the message queue for the borrower. 31. Disable the system preference 'EnablePinAuthentication'. 32. Verify that the pin code is now invalid (SIP response MUST contain the text 'Invalid password') $kohabin/sip_cli_emulator.pl --address 127.0.0.1 --port 6001 \ --su "$su" --sp "$sp" --location "$inst" \ --message patron_information --patron "$patron" --password 3333 Sponsored-by: Bibliotek i Västmanland -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200378|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=39943 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #17 from David Cook <dcook@prosentient.com.au> --- Good to see that the PIN is stored as a hash, but I think it's still going to be accessible with the Reports module. I'm trying to push for "*_credentials" tables that are easier to secure. That said, I don't think it would be too difficult to migrate to that. Overall, I think this is a great addition to Koha. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #19 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 201802 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201802&action=edit Bug 39943: Add set_pin tests To test: 1. prove t/db_dependent/Koha/Patrons.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #21 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 201804 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201804&action=edit Bug 39943: (proposal) Add a failing test for minimum pin length Proposing to enforce a fallback for minimum pin length. If agreed upon, this patch should be followed by a change that adds a fallback of 3 digits minimum. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 --- Comment #23 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 201806 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201806&action=edit Bug 39943: pin field schema [DO NOT PUSH] Patch from commit dc91f02 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #24 from Lari Taskula <lari.taskula@hypernova.fi> --- Awesome work, really impressive. This will be highly appreciated in many Finnish libraries. QA notes: 1. prove t/db_dependent/Koha/Patrons.t fails with DBIx::Class::Row::store_column(): No such column 'pin' on Koha::Schema::Result::Deletedborrower at /home/koha/Koha/Koha/Patron.pm line 1497 I see that you later attached a follow up patch adding the column to deletedborrowers, I've also attached a schema patch to make this test pass. 2. Should we enfore an absolute minimum pin length? From Koha::AuthUtils::is_pin_valid() I noticed that you left out the fallback value of 3 that the is_password_valid() method had. With these patches we default to a minimum of 4 by the atomicupdate (which is OK), but setting MinPinLength 0 is possible while on the other hand at least 1 digit is enforced by Koha::AuthUtils::is_pin_valid() regex step "non_digits". I think that like is_password_valid(), enforcing a fallback of 3 digits if MinPinLength < 3 would be reasonable. See my patches tagged with (proposal). 3. Your patch "Bug 39943: allow_pin_code field schema [DO NOT PUSH]" contains a change to kohastructure.sql likely meant for the previous patch. The new default value and the nullability will be dropped if this change is not pushed. 4. GUI questions: I think we should display the PIN code field at: - staff/cgi-bin/koha/members/memberentry.pl?op=edit_form&destination=circ&borrowernumber=123 (after password) - self registration (I could see libraries wanting to store patron's PIN code during self registration. Btw check syspref PatronSelfRegistrationBorrowerUnwantedField, pin is missing from this list and probably the other similar sysprefs as well. See Koha::Database::Columns->columns{'borrowers'}) These additions can be moved into new Bugs so that they won't stop this feature from moving on. Other very minor patch diff issues that stood out when reading your changes line by line: 5. Patch "Bug 39943: Add PIN_CHANGE letter" contains changes to OpenAPI spec, likely meant for the later patch "Bug 39943: patron/{patron_id}/pin endpoint" t::lib::Mocks::mock_preference( 'MinPinLength', undef ); throws_ok { $patron->set_pin( { pin => '12' } ); } 'Koha::Exceptions::Pin::TooShort', 'MinPinLength is undef, fall back to 3, fail test'; is( "$@", 'Pin length (2) is shorter than required (3)', 'Exception parameters passed correctly' ); t::lib::Mocks::mock_preference( 'MinPinLength', 2 ); throws_ok { $patron->set_pin( { pin => '12' } ); } 'Koha::Exceptions::Pin::TooShort', 'MinPinLength is 2, fall back to 3, fail test'; 6. Patch "Bug 39943: patron/{patron_id}/pin endpoint" 6.1 introduces Koha/Exceptions/Pin.pm late as it is already used by the previous patch. 6.2 has a strange looking endpoint edit in "api/v1/swagger/paths/patrons_pin.yaml", probably related to note 5 These do not change the end result nor the functionality of the feature but leave the mentioned patches slightly disorganized. Ran the following unit tests and they passed: 7. t/db_dependent/Auth/pin.t 8. t/db_dependent/api/v1/patrons_pin.t Followed the test plan you've written, and the feature works as advertised. I've added tests for set_pin() and is_pin_valid(). I'm still marking this as Failed QA due to note 3 which should be a trivial fix. Also I would like to hear your take on note 2 about the absolute minimum pin length. Again, really good work. Thank you so much for this. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39943 Sally <sally.healey@cheshiresharedservices.gov.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sally.healey@cheshireshared | |services.gov.uk -- You are receiving this mail because: You are watching all bug changes.