Add API to change patron's password. This is useful especially for 3rd party applications like VuFind.
Created attachment 54575 [details] [review] Bug 17006: Add API route for changing patron's password PATCH /patrons/{borrowernumber}/password (change password) Required body params: - current_password - new_password To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10'
Created attachment 54576 [details] [review] Bug 17006: Respect system preference "OpacPasswordChange" Allow patron to change their own password without borrowers-flag only if OpacPasswordChange system preference is enabled. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Set OpacPasswordChange system preference to "Don't allow". 4. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber for a borrower that does not have any permissions 5. Observe permission error. 6. Set OpacPasswordChange system preference to "Allow". 7. Repeat step 4 and make sure that password was changed.
Created attachment 54629 [details] [review] Bug 17006: Add API route for changing patron's password Fixed trailing whitespace
Created attachment 54630 [details] [review] Bug 17006: Respect system preference "OpacPasswordChange"
Created attachment 54716 [details] [review] [SIGNED-OFF] Bug 17006: Add API route for changing patron's password PATCH /patrons/{borrowernumber}/password (change password) Required body params: - current_password - new_password To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Created attachment 54717 [details] [review] [SIGNED-OFF] Bug 17006: Add API route for changing patron's password PATCH /patrons/{borrowernumber}/password (change password) Required body params: - current_password - new_password To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Created attachment 54718 [details] [review] [SIGNED-OFF] Bug 17006: Respect system preference "OpacPasswordChange" Allow patron to change their own password without borrowers-flag only if OpacPasswordChange system preference is enabled. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Set OpacPasswordChange system preference to "Don't allow". 4. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber for a borrower that does not have any permissions 5. Observe permission error. 6. Set OpacPasswordChange system preference to "Allow". 7. Repeat step 4 and make sure that password was changed. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Created attachment 56155 [details] [review] [SIGNED-OFF] Bug 17006: Add API route for changing patron's password PATCH /patrons/{borrowernumber}/password (change password) Required body params: - current_password - new_password To test: 1. Apply patch and minify swagger.json perl misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json -d api/v1/swagger/swagger.min.json 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Created attachment 56156 [details] [review] [SIGNED-OFF] Bug 17006: Respect system preference "OpacPasswordChange" Allow patron to change their own password without borrowers-flag only if OpacPasswordChange system preference is enabled. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Set OpacPasswordChange system preference to "Don't allow". 4. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber for a borrower that does not have any permissions 5. Observe permission error. 6. Set OpacPasswordChange system preference to "Allow". 7. Repeat step 4 and make sure that password was changed. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Created attachment 56157 [details] [review] [SIGNED-OFF] Bug 17006: Add API route for changing patron's password Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Created attachment 56158 [details] [review] [SIGNED-OFF] Bug 17006: Respect system preference "OpacPasswordChange" Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Rebased on top of master.
Created attachment 56283 [details] [review] [SIGNED-OFF] Bug 17006: Add API route for changing patron's password PATCH /patrons/{borrowernumber}/password (change password) Required body params: - current_password - new_password To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Created attachment 56284 [details] [review] [SIGNED-OFF] Bug 17006: Respect system preference "OpacPasswordChange" Allow patron to change their own password without borrowers-flag only if OpacPasswordChange system preference is enabled. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Set OpacPasswordChange system preference to "Don't allow". 4. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber for a borrower that does not have any permissions 5. Observe permission error. 6. Set OpacPasswordChange system preference to "Allow". 7. Repeat step 4 and make sure that password was changed. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Removed minification step from test plan after Bug 17432 was pushed to master.
Created attachment 56399 [details] [review] Bug 17006: (follow-up) Test for changing another user's password The previous patches work for changing your own password, but the tests are lacking a case where librarian attempts to change patron's password. This feature was unnoticed and not implemented. To test: 1. Run t/db_dependent/api/v1/patrons.t 2. Observe failing test 3. Proceed to next patch to fix the failing test
Created attachment 56400 [details] [review] Bug 17006: (follow-up) Let librarians change patron's passwords This should obviously be part of this feature but was left unnoticed in the first patch. If user has "borrowers"-flag, they should be able to change patron's password without providing old password. To test: 1. Run t/db_dependent/api/v1/patrons.t 2. Observe failing test 3. Apply patch 4. Run t/db_dependent/api/v1/patrons.t 5. Observe passing test 6. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) and where YYY is not the same borrowernumber as your logged-in user's borrowernumber 7. Make sure that password was changed for YYY. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password \ --data '{"new_password":"1234"}' \ --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10'
Created attachment 56401 [details] [review] Bug 17006: (follow-up) Use Koha::Exceptions for change password endpoint Koha::Exceptions are a nice way to handle erros and they should be used much more than they are now. This patch adds Koha::Exceptions into Koha::Patron->change_password_to sub and catches the exceptions in the controller. 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. curl -X PATCH http://library/api/v1/patrons/123/password \ --data '{"current_password":"123", "new_password":"1234"}' \ --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10'
Created attachment 56409 [details] [review] Bug 17006: (follow-up) Use Koha::Exceptions for change password endpoint
Created attachment 57356 [details] [review] [SIGNED-OFF] Bug 17006: Add API route for changing patron's password PATCH /patrons/{borrowernumber}/password (change password) Required body params: - current_password - new_password To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 57357 [details] [review] [SIGNED-OFF] Bug 17006: Respect system preference "OpacPasswordChange" Allow patron to change their own password without borrowers-flag only if OpacPasswordChange system preference is enabled. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Set OpacPasswordChange system preference to "Don't allow". 4. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber for a borrower that does not have any permissions 5. Observe permission error. 6. Set OpacPasswordChange system preference to "Allow". 7. Repeat step 4 and make sure that password was changed. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 57358 [details] [review] [SIGNED-OFF] Bug 17006: (follow-up) Test for changing another user's password The previous patches work for changing your own password, but the tests are lacking a case where librarian attempts to change patron's password. This feature was unnoticed and not implemented. To test: 1. Run t/db_dependent/api/v1/patrons.t 2. Observe failing test 3. Proceed to next patch to fix the failing test Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 57359 [details] [review] [SIGNED-OFF] Bug 17006: (follow-up) Let librarians change patron's passwords This should obviously be part of this feature but was left unnoticed in the first patch. If user has "borrowers"-flag, they should be able to change patron's password without providing old password. To test: 1. Run t/db_dependent/api/v1/patrons.t 2. Observe failing test 3. Apply patch 4. Run t/db_dependent/api/v1/patrons.t 5. Observe passing test 6. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) and where YYY is not the same borrowernumber as your logged-in user's borrowernumber 7. Make sure that password was changed for YYY. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password \ --data '{"new_password":"1234"}' \ --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 57360 [details] [review] [SIGNED-OFF] Bug 17006: (follow-up) Use Koha::Exceptions for change password endpoint Koha::Exceptions are a nice way to handle erros and they should be used much more than they are now. This patch adds Koha::Exceptions into Koha::Patron->change_password_to sub and catches the exceptions in the controller. 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. curl -X PATCH http://library/api/v1/patrons/123/password \ --data '{"current_password":"123", "new_password":"1234"}' \ --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
I can't apply the last patch. Wende an: Bug 17006: (follow-up) Use Koha::Exceptions for change password endpoint fatal: sha1 information is lacking or useless (Koha/REST/V1/Patron.pm). Dem Repository fehlen notwendige Blobs um auf einen 3-Wege-Merge zurückzufallen. Kann nicht zu 3-Wege-Merge zurückfallen. Anwendung des Patches fehlgeschlagen bei 0001 Bug 17006: (follow-up) Use Koha::Exceptions for change password endpoint
Created attachment 63329 [details] [review] Bug 17006: Add API route for changing patron's password PATCH /patrons/{borrowernumber}/password (change password) Required body params: - current_password - new_password To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63330 [details] [review] Bug 17006: Respect system preference "OpacPasswordChange" Allow patron to change their own password without borrowers-flag only if OpacPasswordChange system preference is enabled. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Set OpacPasswordChange system preference to "Don't allow". 4. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber for a borrower that does not have any permissions 5. Observe permission error. 6. Set OpacPasswordChange system preference to "Allow". 7. Repeat step 4 and make sure that password was changed. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63331 [details] [review] Bug 17006: (follow-up) Test for changing another user's password The previous patches work for changing your own password, but the tests are lacking a case where librarian attempts to change patron's password. This feature was unnoticed and not implemented. To test: 1. Run t/db_dependent/api/v1/patrons.t 2. Observe failing test 3. Proceed to next patch to fix the failing test Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63332 [details] [review] Bug 17006: (follow-up) Let librarians change patron's passwords This should obviously be part of this feature but was left unnoticed in the first patch. If user has "borrowers"-flag, they should be able to change patron's password without providing old password. To test: 1. Run t/db_dependent/api/v1/patrons.t 2. Observe failing test 3. Apply patch 4. Run t/db_dependent/api/v1/patrons.t 5. Observe passing test 6. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) and where YYY is not the same borrowernumber as your logged-in user's borrowernumber 7. Make sure that password was changed for YYY. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password \ --data '{"new_password":"1234"}' \ --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63333 [details] [review] Bug 17006: (follow-up) Use Koha::Exceptions for change password endpoint Koha::Exceptions are a nice way to handle erros and they should be used much more than they are now. This patch adds Koha::Exceptions into Koha::Patron->change_password_to sub and catches the exceptions in the controller. 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. curl -X PATCH http://library/api/v1/patrons/123/password \ --data '{"current_password":"123", "new_password":"1234"}' \ --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
(In reply to Mirko Tietgen from comment #25) > I can't apply the last patch. > > Wende an: Bug 17006: (follow-up) Use Koha::Exceptions for change password > endpoint > fatal: sha1 information is lacking or useless (Koha/REST/V1/Patron.pm). > Dem Repository fehlen notwendige Blobs um auf einen 3-Wege-Merge > zurückzufallen. > Kann nicht zu 3-Wege-Merge zurückfallen. > Anwendung des Patches fehlgeschlagen bei 0001 Bug 17006: (follow-up) Use > Koha::Exceptions for change password endpoint Rebased on master
Created attachment 67128 [details] [review] Bug 17006: Add API route for changing patron's password PATCH /patrons/{borrowernumber}/password (change password) Required body params: - current_password - new_password To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 67129 [details] [review] Bug 17006: Respect system preference "OpacPasswordChange" Allow patron to change their own password without borrowers-flag only if OpacPasswordChange system preference is enabled. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Set OpacPasswordChange system preference to "Don't allow". 4. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber for a borrower that does not have any permissions 5. Observe permission error. 6. Set OpacPasswordChange system preference to "Allow". 7. Repeat step 4 and make sure that password was changed. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 67130 [details] [review] Bug 17006: (follow-up) Test for changing another user's password The previous patches work for changing your own password, but the tests are lacking a case where librarian attempts to change patron's password. This feature was unnoticed and not implemented. To test: 1. Run t/db_dependent/api/v1/patrons.t 2. Observe failing test 3. Proceed to next patch to fix the failing test Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 67131 [details] [review] Bug 17006: (follow-up) Let librarians change patron's passwords This should obviously be part of this feature but was left unnoticed in the first patch. If user has "borrowers"-flag, they should be able to change patron's password without providing old password. To test: 1. Run t/db_dependent/api/v1/patrons.t 2. Observe failing test 3. Apply patch 4. Run t/db_dependent/api/v1/patrons.t 5. Observe passing test 6. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) and where YYY is not the same borrowernumber as your logged-in user's borrowernumber 7. Make sure that password was changed for YYY. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password \ --data '{"new_password":"1234"}' \ --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 67132 [details] [review] Bug 17006: (follow-up) Use Koha::Exceptions for change password endpoint Koha::Exceptions are a nice way to handle erros and they should be used much more than they are now. This patch adds Koha::Exceptions into Koha::Patron->change_password_to sub and catches the exceptions in the controller. 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. curl -X PATCH http://library/api/v1/patrons/123/password \ --data '{"current_password":"123", "new_password":"1234"}' \ --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Rebased on master
Note for QA: I think Koha::Patron->change_password_to must call Koha::AuthUtils::is_password_valid (from bug 18298).
Patch doesn't apply, please also take a look at comment#38. Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17006: Add API route for changing patron's password Using index info to reconstruct a base tree... M Koha/Patron.pm M Koha/REST/V1/Patron.pm M api/v1/swagger/paths.json M api/v1/swagger/paths/patrons.json M t/db_dependent/api/v1/patrons.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/api/v1/patrons.t Auto-merging api/v1/swagger/paths/patrons.json Auto-merging api/v1/swagger/paths.json CONFLICT (content): Merge conflict in api/v1/swagger/paths.json Auto-merging Koha/REST/V1/Patron.pm Auto-merging Koha/Patron.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 17006: Add API route for changing patron's password The copy of the patch that failed is found in: .git/rebase-apply/patch 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". Patch left in /tmp/Bug-17006-Add-API-route-for-changing-patrons-passw-7eNHth.patch
Created attachment 70260 [details] [review] Bug 17006: Add API route for changing patron's password PATCH /patrons/{borrowernumber}/password (change password) Required body params: - current_password - new_password To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 70261 [details] [review] Bug 17006: Respect system preference "OpacPasswordChange" Allow patron to change their own password without borrowers-flag only if OpacPasswordChange system preference is enabled. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Set OpacPasswordChange system preference to "Don't allow". 4. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber for a borrower that does not have any permissions 5. Observe permission error. 6. Set OpacPasswordChange system preference to "Allow". 7. Repeat step 4 and make sure that password was changed. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 70262 [details] [review] Bug 17006: (follow-up) Test for changing another user's password The previous patches work for changing your own password, but the tests are lacking a case where librarian attempts to change patron's password. This feature was unnoticed and not implemented. To test: 1. Run t/db_dependent/api/v1/patrons.t 2. Observe failing test 3. Proceed to next patch to fix the failing test Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 70263 [details] [review] Bug 17006: (follow-up) Let librarians change patron's passwords This should obviously be part of this feature but was left unnoticed in the first patch. If user has "borrowers"-flag, they should be able to change patron's password without providing old password. To test: 1. Run t/db_dependent/api/v1/patrons.t 2. Observe failing test 3. Apply patch 4. Run t/db_dependent/api/v1/patrons.t 5. Observe passing test 6. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) and where YYY is not the same borrowernumber as your logged-in user's borrowernumber 7. Make sure that password was changed for YYY. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password \ --data '{"new_password":"1234"}' \ --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 70264 [details] [review] Bug 17006: (follow-up) Use Koha::Exceptions for change password endpoint Koha::Exceptions are a nice way to handle erros and they should be used much more than they are now. This patch adds Koha::Exceptions into Koha::Patron->change_password_to sub and catches the exceptions in the controller. 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. curl -X PATCH http://library/api/v1/patrons/123/password \ --data '{"current_password":"123", "new_password":"1234"}' \ --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
This should be implemented on top of bug 19784. Also, I see no reason to require the 'old password'. Conceptually, I would say we are updating the password "object". This means we need to PUT a new password. If we changed the password by hitting the patron/patron_id endpoint, then PATCH would be advised. But on this implementation we should def use PUT.
I'm taking over this one with Lari's agreement. I will add a RFC proposal to be voted.
Created attachment 77882 [details] [review] Bug 17006: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 77883 [details] [review] Bug 17006: OpenAPI spec Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 77884 [details] [review] Bug 17006: Add /patrons/{patron_id}/password This patch introduces an endpoint for changing a patron's password. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => SUCCESS: tests pass! - Play with the different use cases highlighted by the tests, on your favourite REST testing tool (Postman, RESTer on FF, etc). Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 77923 [details] [review] Bug 17006: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 77924 [details] [review] Bug 17006: OpenAPI spec Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 77925 [details] [review] Bug 17006: Add /patrons/{patron_id}/password This patch introduces an endpoint for changing a patron's password. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => SUCCESS: tests pass! - Play with the different use cases highlighted by the tests, on your favourite REST testing tool (Postman, RESTer on FF, etc). Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
tomas: yet another whitespaces occurrence ;)
Created attachment 78837 [details] [review] Bug 17006: (follow-up) Whitespace chars follow-up Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Marcel de Rooy from comment #54) > tomas: yet another whitespaces occurrence ;) (Marcel follow-up) submitted :-D
Please rebase on master I get error: Using index info to reconstruct a base tree... M api/v1/swagger/paths.json Falling back to patching base and 3-way merge... Auto-merging api/v1/swagger/paths.json CONFLICT (content): Merge conflict in api/v1/swagger/paths.json Failed to merge in the changes. Patch failed at 0001 Bug 17006: OpenAPI spec Thank You Mike
Bug 20944 caused a conflict. easily fixed.
Created attachment 82625 [details] [review] Bug 17006: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 82626 [details] [review] Bug 17006: OpenAPI spec Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 82627 [details] [review] Bug 17006: Add /patrons/{patron_id}/password This patch introduces an endpoint for changing a patron's password. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => SUCCESS: tests pass! - Play with the different use cases highlighted by the tests, on your favourite REST testing tool (Postman, RESTer on FF, etc). Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 82628 [details] [review] Bug 17006: (follow-up) Whitespace chars follow-up Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Rebased. Please mark as 'Patch doesn't apply' so people notice what needs to be done!
Sorry Tomas but I think we should use set_password (from bug 21178) now.
Created attachment 83579 [details] [review] Bug 17006: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83580 [details] [review] Bug 17006: OpenAPI spec Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83581 [details] [review] Bug 17006: Add /patrons/{patron_id}/password This patch introduces an endpoint for changing a patron's password. It targets privileged user with the right permissions, changing some patron's password. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => SUCCESS: tests pass! - Play with the different use cases highlighted by the tests, on your favourite REST testing tool (Postman, RESTer on FF, etc). Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
As we voted for having a /public endpoint for unprivileged routes, I simplified the implementation and removed that use case. I will file a new bug for implementing the on/off switch for the /public namespace, and the endpoint for changing passwords for un-privileged users.
Created attachment 83749 [details] [review] Bug 17006: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83750 [details] [review] Bug 17006: OpenAPI spec Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83751 [details] [review] Bug 17006: Add /patrons/{patron_id}/password This patch introduces an endpoint for changing a patron's password. It targets privileged user with the right permissions, changing some patron's password. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => SUCCESS: tests pass! - Play with the different use cases highlighted by the tests, on your favourite REST testing tool (Postman, RESTer on FF, etc). Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 84285 [details] [review] Bug 17006: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84286 [details] [review] Bug 17006: OpenAPI spec Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84287 [details] [review] Bug 17006: Add /patrons/{patron_id}/password This patch introduces an endpoint for changing a patron's password. It targets privileged user with the right permissions, changing some patron's password. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => SUCCESS: tests pass! - Play with the different use cases highlighted by the tests, on your favourite REST testing tool (Postman, RESTer on FF, etc). Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84488 [details] [review] Bug 17006: (follow-up) set_password expects a hashref Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Awesome work all! Pushed to master for 19.05
As is my policy on non-breaking api additions I have backported this to 18.11.x series for 18.11.03 release. I have also added the missing `Sponsored-by: Municipal Libray Ceska Trebova` lines so due appreciation is shown in the release notes.
missing dependencies, wont backport to 18.05.x