Cancel hold, suspend the hold and resume the hold endpoints have too high permissions for librarians with only "place_holds" permissions. As this was changed in the views, then the change should be added to the REST API also.
Created attachment 135052 [details] [review] Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API This patch enables librarians with only "place_holds" permissions to cancel, suspend and resume holds via REST API. Test plan: 1) Try to cancel or suspend a hold with only "place_holds" permissions 2) See that it is forbidden. 3) Apply the patch 4) Cancel a hold again 5) The cancellation succeeds 6) prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy
Created attachment 135080 [details] [review] Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API This patch enables librarians with only "place_holds" permissions to cancel, suspend and resume holds via REST API. Test plan: 1) Try to cancel or suspend a hold with only "place_holds" permissions 2) See that it is forbidden. 3) Apply the patch 4) Cancel a hold again 5) The cancellation succeeds 6) prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy Signed-off-by: David Nind <david@davidnind.com>
Testing notes using koha-testing-docker: 1. I used the Firefox addon RESTr to test. 2. Place two holds in the staff interface. 3. Change the username and password for a staff patron. 4. List the holds using the REST API and note down the hold_ids: - Method: GET - URL: http://127.0.0.1/8080/api/v1/holds/ - Authentication: use koha + koha for basic authentication (or give the staff patron used Add, modify and view patron information (borrowers) access so they can view the list of holds) 5. Suspend a hold (where {hold_id} is one of the holds listed in step 3 and YYYY-MM-DD is a future date): - Method: POST - URL: http://127.0.0.1:8080/api/v1/holds/{hold_id}/suspension - Authentication: staff patron with place_holds permission - Body (JSON): { "end_date": "YYYY-MM-DD" } 6. Cancel a hold (where {hold_id} is one of the holds listed in step 3): - Method: DELETE - URL: http://127.0.0.1:8080/api/v1/holds/{hold_id} - Authentication: staff patron with place_holds permission
Created attachment 135946 [details] [review] Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API This patch enables librarians with only "place_holds" permissions to cancel, suspend and resume holds via REST API. Test plan: 1) Try to cancel or suspend a hold with only "place_holds" permissions 2) See that it is forbidden. 3) Apply the patch 4) Cancel a hold again 5) The cancellation succeeds 6) prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
QA Notes: none of the affected APIs modifies priority, so this seems reasonable. Would be nice to see unit tests to prove the minimum user permissions needed work.
Created attachment 136031 [details] [review] Bug 30780: Regression tests This patch lowers the used permissions on the suspend/resume tests, and adds tests for fine-grained permissions on cancelling a hold. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 136032 [details] [review] Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API This patch enables librarians with only "place_holds" permissions to cancel, suspend and resume holds via REST API. Test plan: 1) Try to cancel or suspend a hold with only "place_holds" permissions 2) See that it is forbidden. 3) Apply the patch 4) Cancel a hold again 5) The cancellation succeeds 6) prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Kyle M Hall from comment #5) > Would be nice to see unit tests to prove the minimum user permissions needed > work. Good catch, done.
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.03