Bug 33462 - Force password change for new patrons entered by staff
Summary: Force password change for new patrons entered by staff
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Sam Lau
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-04-08 23:26 UTC by Christopher Brannon
Modified: 2024-03-25 18:15 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 33462: Unit Tests (5.64 KB, patch)
2023-07-19 20:07 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 33462: Update Schema Files [DO NOT PUSH] (7.11 KB, patch)
2023-07-19 20:07 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 33462: Add ability to force password change for new patrons entered by staff (25.22 KB, patch)
2023-07-19 20:07 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 33462: Unit Tests (5.71 KB, patch)
2023-08-12 23:42 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 33462: Update Schema Files [DO NOT PUSH] (7.17 KB, patch)
2023-08-12 23:42 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 33462: Add ability to force password change for new patrons entered by staff (25.28 KB, patch)
2023-08-12 23:42 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2023-04-08 23:26:02 UTC
Patrons that register via OPAC create whatever password they want.  But when staff create a patron account, patrons are given a password.  It would be preferred if patrons were forced to change their password after first logging in when a staff member creates the account.

Unfortunately, the password expiration field is not available when creating the account.  It is only editable after.  It would be ideal if the password field were shown, even when adding a new patron, and the patron category setup page had an option next to expiration settings to force patron to change password at login when entered by staff.  Then, when a staff member is adding the new patron, the date could be defaulted to today.  The OPAC self registration would not need to do this.
Comment 1 koha-US bug tracker 2023-05-10 16:30:14 UTC
+1
Comment 2 George Williams (NEKLS) 2023-05-10 16:30:50 UTC
+1
Comment 3 Christopher Brannon 2023-05-11 19:22:34 UTC
Probably the easiest way to implement this would be to have both a global and patron category enable/disable setting for new patrons created by staff option.  If enabled, when you are entering a new patron on the staff side, the password expiration date would be defaulted to expired as the account is created.  Any other edits would ignore this setting.
Comment 4 Christopher Brannon 2023-05-17 20:03:33 UTC
This feature is rather important for us.  Our consortium wants to stay away from having patron passwords expiring on a regular basis, but we want patrons to get away from default passwords as quickly as possible.  An option for changing the password after a first time login would meet this need.
Comment 5 Sam Lau 2023-07-19 20:07:45 UTC
Created attachment 153687 [details] [review]
Bug 33462: Unit Tests

prove t/db_dependent/Koha/Patron/Category.t
prove t/db_dependent/Koha/Patron.t
Comment 6 Sam Lau 2023-07-19 20:07:47 UTC
Created attachment 153688 [details] [review]
Bug 33462: Update Schema Files [DO NOT PUSH]

This updates the schema files for Borrower.pm, BorrowerModification.pm, Category.pm, and Deletedborrower.pm.
Comment 7 Sam Lau 2023-07-19 20:07:50 UTC
Created attachment 153689 [details] [review]
Bug 33462: Add ability to force password change for new patrons entered by staff

This patch attempts to force a password change for new staff created patrons. This is done by setting the password_expiration_date to an expired date when adding a new patron. This patch adds a new system preference 'ForcePasswordResetWhenSetByStaff', a new column to the categories table 'force_password_reset_when_set_by_staff', and new columns to the borrowers, borrower_modification, and deletedborrowers table 'needs_password_reset'

To test:
1) Apply patch, restart_all, updatedatabase, and also be sure to update schema.
2) Visit Administration->Sytem Preferences and search for 'EnableExpiredPasswordReset'. Make sure this is set to enable.
   Now search for  'ForcePasswordResetWhenSetByStaff'. This should be defaulted to 'Don't force'.
3) Keep that tab open and visit Administration->Patron categories. Click on edit on the Board category. Noitce that there is a now a 'Force new patron password reset' section. Notice that the by default, this is set to follow the ForcePasswordResetWhenSetByStaff system preference (currently set to don't force). Click on the dropdown and change it to 'Force'. Save changes
4) Click on the Patrons tab to visit members-home.pl and then click 'New Patron'. Select on Patron. Fill in the required information and also enter a password.
5) Submit this form and notice that the patron's password expiration date is set to never. This should be the case because the default for 'Force new patron password reset' follows the sys. pref. which is still set to 'Don't force' (You could have some expiry date in this step, but it should at least be set to a date that is not expired. this depends on whether or not you have a defalut password expiration date set in patron categories )
6) Log into the OPAC with this patron and notice it works as expected and log in was successful.
7) Go back to the patron home page and click to add a new patron. This time select 'Board'. Once again fill out the required info, enter a password, and then save the form.
8) Notice that for this patron, the password expiration date is set for today's date. This is because we changed the setting for the 'Board' patron category to force.
9) Log into the OPAC with this patron. You should be redirected to a page with an error that says: "It's your first login! You need to reset your password." Click on the reset password link below this message.
10) You should be sent to a page where you can reset your password. Fill in the form and click 'Update password'. Attempt to sign into the OPAC with this new password. Everything works as expected.
11) Go back to the staff interface and view this patron's detail page. Notice the password expiration date is now set to what the default is in the patron category.
12) Edit this patrons information and set their password expiration date to yesterday. Go back to the OPAC and try to sign in with this patron again. Note that this time, you are also redirected but the message says "Error: Your password has expired!"
13) Go back to the staff interface and visit the sys. pref tab we left open. Set it to the 'Force' option and save changes.
14) Visit the patron home page and click add patron, now select the patron category again. Fill in required info and enter password. Submit form and note that the patron's password expiration date is set to today. Try to login to the OPAC with this patron, you should be redirected to the page with the error that says "Error: It's your first login! You need to reset your password."
15) Sign-off :)
Comment 8 Sam Lau 2023-07-19 20:33:36 UTC
Hi all, 

I have attempted a solution for this bug and would love any feedback about what you think! Because this is a pretty large patch I figured I'd explain more in depth what the patch actually does here: 

I added a new column to the patron categories that determines whether that type of patron should be forced to reset their password after being created. This column, force_password_reset_when_set_by_staff, is defaulted to follow the new system preference 'ForcePasswordResetWhenSetByStaff', but it also has hard "Force" and "Don't force" options as well. 


If a patron's category allows for it, when a staff member creates a new patron account, the password expiration date is automatically set to an expired date. When they attempt to login to the OPAC, they are sent to a page with a custom error message with a link to reset their password. This message is achieved by adding a new column to the borrowers table 'needs_password_reset' which tracks if a patron needs to reset their password after creation. Upon resetting the password, everything proceeds as usual. 

I also wanted to mention, if testing, it goes a lot faster if you turn on the 'autoMemberNum' preference as well as turn off the 'RequireStrongPassword' preference. This will allow for a much faster patron creation.
Comment 9 Christopher Brannon 2023-08-04 15:51:38 UTC
I'm getting a merge failure in the sandbox on this bug.

TASK [Apply bug 33462 via git-bz in docker container] **************************
fatal: [localhost -> koha-bug33462]: FAILED! => {"changed": true, "cmd": "koha-shell -c 'cd /kohadevbox/koha && yes | git bz apply 33462' bug33462", "delta": "0:00:03.143844", "end": "2023-08-04 15:49:21.744724", "msg": "non-zero return code", "rc": 1, "start": "2023-08-04 15:49:18.600880", "stderr": "error: Failed to merge in the changes.\nhint: Use 'git am --show-current-patch=diff' to see the failed patch\nPatch left in /tmp/Bug-33462-Unit-Tests-4t46gml6.patch", "stderr_lines": ["error: Failed to merge in the changes.", "hint: Use 'git am --show-current-patch=diff' to see the failed patch", "Patch left in /tmp/Bug-33462-Unit-Tests-4t46gml6.patch"], "stdout": "\nBug 33462 - Force password change for new patrons entered by staff\n\n153687 - Bug 33462: Unit Tests\n153688 - Bug 33462: Update Schema Files [DO NOT PUSH]\n153689 - Bug 33462: Add ability to force password change for new patrons entered by staff\n\nApply? [(y)es, (n)o, (i)nteractive] Applying: Bug 33462: Unit Tests\nUsing index info to reconstruct a base tree...\nM\tt/db_dependent/Koha/Patron.t\nFalling back to patching base and 3-way merge...\nAuto-merging t/db_dependent/Koha/Patron.t\nCONFLICT (content): Merge conflict in t/db_dependent/Koha/Patron.t\nPatch failed at 0001 Bug 33462: Unit Tests\nWhen you have resolved this problem run \"git bz apply --continue\".\nIf you would prefer to skip this patch, instead run \"git bz apply --skip\".\nTo restore the original branch and stop patching run \"git bz apply --abort\".", "stdout_lines": ["", "Bug 33462 - Force password change for new patrons entered by staff", "", "153687 - Bug 33462: Unit Tests", "153688 - Bug 33462: Update Schema Files [DO NOT PUSH]", "153689 - Bug 33462: Add ability to force password change for new patrons entered by staff", "", "Apply? [(y)es, (n)o, (i)nteractive] Applying: Bug 33462: Unit Tests", "Using index info to reconstruct a base tree...", "M\tt/db_dependent/Koha/Patron.t", "Falling back to patching base and 3-way merge...", "Auto-merging t/db_dependent/Koha/Patron.t", "CONFLICT (content): Merge conflict in t/db_dependent/Koha/Patron.t", "Patch failed at 0001 Bug 33462: Unit Tests", "When you have resolved this problem run \"git bz apply --continue\".", "If you would prefer to skip this patch, instead run \"git bz apply --skip\".", "To restore the original branch and stop patching run \"git bz apply --abort\"."]}

Sam, can you please check.
Comment 10 Sam Lau 2023-08-04 17:01:23 UTC
Hi Christopher, thanks for trying to test! Were you using ByWater's sandbox?
Comment 11 Sam Lau 2023-08-04 17:12:29 UTC
(In reply to Sam Lau from comment #10)
> Hi Christopher, thanks for trying to test! Were you using ByWater's sandbox?

I tried applying patch on the testing docker and everything was still applying cleanly. I then tried going to the sandbox (I used ByWater) and after applying it and I did end up getting the same message as you in the provision logs. I'm not sure what the issue is (not very familiar with the sandbox) but I asked Kyle Hall for help and we are both hoping the issue is fixed now. I would say give it another shot (I think Kyle created a sandbox for this bug) and if it still doesn't work let me know and then maybe try another sandbox?
Comment 12 Sam Lau 2023-08-04 17:34:22 UTC
Everything looks like it is working good on PTFS Europe sandbox so maybe try that one out! :)
Comment 13 Christopher Brannon 2023-08-04 22:12:36 UTC
Yes, I did use ByWater's sandbox.  I'll take a look again.  Thanks.
Comment 14 Biblibre Sandboxes 2023-08-12 23:42:35 UTC
Created attachment 154395 [details] [review]
Bug 33462: Unit Tests

prove t/db_dependent/Koha/Patron/Category.t
prove t/db_dependent/Koha/Patron.t

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Comment 15 Biblibre Sandboxes 2023-08-12 23:42:38 UTC
Created attachment 154396 [details] [review]
Bug 33462: Update Schema Files [DO NOT PUSH]

This updates the schema files for Borrower.pm, BorrowerModification.pm, Category.pm, and Deletedborrower.pm.

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Comment 16 Biblibre Sandboxes 2023-08-12 23:42:41 UTC
Created attachment 154397 [details] [review]
Bug 33462: Add ability to force password change for new patrons entered by staff

This patch attempts to force a password change for new staff created patrons. This is done by setting the password_expiration_date to an expired date when adding a new patron. This patch adds a new system preference 'ForcePasswordResetWhenSetByStaff', a new column to the categories table 'force_password_reset_when_set_by_staff', and new columns to the borrowers, borrower_modification, and deletedborrowers table 'needs_password_reset'

To test:
1) Apply patch, restart_all, updatedatabase, and also be sure to update schema.
2) Visit Administration->Sytem Preferences and search for 'EnableExpiredPasswordReset'. Make sure this is set to enable.
   Now search for  'ForcePasswordResetWhenSetByStaff'. This should be defaulted to 'Don't force'.
3) Keep that tab open and visit Administration->Patron categories. Click on edit on the Board category. Noitce that there is a now a 'Force new patron password reset' section. Notice that the by default, this is set to follow the ForcePasswordResetWhenSetByStaff system preference (currently set to don't force). Click on the dropdown and change it to 'Force'. Save changes
4) Click on the Patrons tab to visit members-home.pl and then click 'New Patron'. Select on Patron. Fill in the required information and also enter a password.
5) Submit this form and notice that the patron's password expiration date is set to never. This should be the case because the default for 'Force new patron password reset' follows the sys. pref. which is still set to 'Don't force' (You could have some expiry date in this step, but it should at least be set to a date that is not expired. this depends on whether or not you have a defalut password expiration date set in patron categories )
6) Log into the OPAC with this patron and notice it works as expected and log in was successful.
7) Go back to the patron home page and click to add a new patron. This time select 'Board'. Once again fill out the required info, enter a password, and then save the form.
8) Notice that for this patron, the password expiration date is set for today's date. This is because we changed the setting for the 'Board' patron category to force.
9) Log into the OPAC with this patron. You should be redirected to a page with an error that says: "It's your first login! You need to reset your password." Click on the reset password link below this message.
10) You should be sent to a page where you can reset your password. Fill in the form and click 'Update password'. Attempt to sign into the OPAC with this new password. Everything works as expected.
11) Go back to the staff interface and view this patron's detail page. Notice the password expiration date is now set to what the default is in the patron category.
12) Edit this patrons information and set their password expiration date to yesterday. Go back to the OPAC and try to sign in with this patron again. Note that this time, you are also redirected but the message says "Error: Your password has expired!"
13) Go back to the staff interface and visit the sys. pref tab we left open. Set it to the 'Force' option and save changes.
14) Visit the patron home page and click add patron, now select the patron category again. Fill in required info and enter password. Submit form and note that the patron's password expiration date is set to today. Try to login to the OPAC with this patron, you should be redirected to the page with the error that says "Error: It's your first login! You need to reset your password."
15) Sign-off :)

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Comment 17 Christopher Brannon 2023-08-12 23:43:36 UTC
Works as expected and is fantastic!  Can't wait to have it in production!!!!
Comment 18 Jonathan Druart 2023-09-01 12:59:20 UTC
1. Please run the QA script and fix the warning (tidy)

2. The manual changes (is_boolean) added to the schema file must be in a separate commit, or it will get lost when pushed

3. indentation is 4 spaces, not 2 (see changes in categories.tt)

4. Could this be achieve without adding a new column (needs_password_reset) but instead compare dateenrolled and password_expiration_date?
If there are the same, then we display "It's your first login", otherwise "Your password has expired". Am I missing something?

5. Not sure how this can behave properly with third-party using the REST API to create patrons (for discussion only).
Comment 19 Christopher Brannon 2023-09-08 16:11:55 UTC
(In reply to Jonathan Druart from comment #18)

> 4. Could this be achieve without adding a new column (needs_password_reset)
> but instead compare dateenrolled and password_expiration_date?
> If there are the same, then we display "It's your first login", otherwise
> "Your password has expired". Am I missing something?

Not being the coder, as long as it can achieve the same result, and only applied when the account is staff entered, and not entered through online registration, I would say anything to reduce extras would be positive.  But let's see what Sam has to say on this.
Comment 20 Christopher Brannon 2023-11-02 15:43:59 UTC
Sam Lau, do you have any comments?
Comment 21 Christopher Brannon 2024-03-13 20:54:39 UTC
We seem to have lost Sam on this conversation.