Bug 30780

Summary: Librarians with only "place_holds" permissions can not update holds data via REST API
Product: Koha Reporter: Johanna Räisä <johanna.raisa>
Component: REST APIAssignee: Johanna Räisä <johanna.raisa>
Status: CLOSED FIXED QA Contact: Kyle M Hall <kyle>
Severity: normal    
Priority: P5 - low CC: david, kyle, lucas, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30207
Change sponsored?: Sponsored Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement enables librarians with only "place_holds" permissions to cancel, suspend and resume holds using the REST API.
Version(s) released in:
22.11.00, 22.05.03
Attachments: Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API
Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API
Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API
Bug 30780: Regression tests
Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API

Description Johanna Räisä 2022-05-17 05:53:47 UTC
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.
Comment 1 Johanna Räisä 2022-05-17 06:09:25 UTC
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
Comment 2 David Nind 2022-05-17 22:41:34 UTC
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>
Comment 3 David Nind 2022-05-17 22:53:58 UTC
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
Comment 4 Kyle M Hall 2022-06-10 15:48:42 UTC
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>
Comment 5 Kyle M Hall 2022-06-10 15:53:15 UTC
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.
Comment 6 Tomás Cohen Arazi 2022-06-13 14:49:48 UTC
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>
Comment 7 Tomás Cohen Arazi 2022-06-13 14:49:52 UTC
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>
Comment 8 Tomás Cohen Arazi 2022-06-13 14:50:19 UTC
(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.
Comment 9 Tomás Cohen Arazi 2022-06-13 14:50:48 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 10 Lucas Gass 2022-07-13 19:42:38 UTC
Backported to 22.05.x for 22.05.03