Bug 30780 - Librarians with only "place_holds" permissions can not update holds data via REST API
Summary: Librarians with only "place_holds" permissions can not update holds data via ...
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Johanna Räisä
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-05-17 05:53 UTC by Johanna Räisä
Modified: 2022-07-13 19:42 UTC (History)
4 users (show)

See Also:
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 (1.70 KB, patch)
2022-05-17 06:09 UTC, Johanna Räisä
Details | Diff | Splinter Review
Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API (1.75 KB, patch)
2022-05-17 22:41 UTC, David Nind
Details | Diff | Splinter Review
Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API (1.80 KB, patch)
2022-06-10 15:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30780: Regression tests (3.26 KB, patch)
2022-06-13 14:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30780: Librarians with only "place_holds" permissions can not update holds data via REST API (1.86 KB, patch)
2022-06-13 14:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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