[Bug 23517] New: Incorrect permission requirements for holds operations via the API
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Bug ID: 23517 Summary: Incorrect permission requirements for holds operations via the API Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: REST api Assignee: koha-bugs@lists.koha-community.org Reporter: oleonard@myacpl.org CC: tomascohen@gmail.com When using the staff client, the holds modification page requires "reserveforothers => 'place_holds'" permission. This is the minimum required permission to modify a hold's pickup location, suspended status, or to delete it. The API requires "reserveforothers" permission (both "place_holds" AND "modify_holds_priority") to delete a hold. This means that operations which a user could perform in the staff client are forbidden by the API. Add and update operations are also problematic because of the granularity of the sub-permissions: - Someone with only "place_holds" permission should be able to add holds and modify every aspect of the hold EXCEPT priority. - Someone with only "modify_holds_priority" permission should be able to modify only the priority of a hold (I guess??). -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Owen, would you say we should better redefine the holds permissions in a more fine-grained way and then adjust the API to reflect that? Hm, maybe we can set 'priority' as read-only and change 'reserveforothers' for 'place_holds' for the first problem. And for the second one, add a new endpoint like PUT /holds/:hold_id/priority requiring the 'modify_holds_priority'. What do you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #2 from Owen Leonard <oleonard@myacpl.org> --- I think we can work within the permissions definitions the staff client enforces right now, and I think you've got it right: - 'place_holds' allows changes to any hold detail except priority - 'modify_holds_priority' allows changes to priority independently of any other detail -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Owen Leonard from comment #2)
I think we can work within the permissions definitions the staff client enforces right now, and I think you've got it right:
- 'place_holds' allows changes to any hold detail except priority - 'modify_holds_priority' allows changes to priority independently of any other detail
+1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 92588 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92588&action=edit Bug 23517: Add a spec for PUT /holds/{hold_id}/priority This patch adds the OpenAPI spec for the endpoint, and tests for the desired behaviour. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 92589 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92589&action=edit Bug 23517: Add the controller method for holds priorities handling This patch implements the PUT /holds/:hold_id/priority endpoint To test: - Apply this patches - Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 92590 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92590&action=edit Bug 23517: (follow-up) Tidy tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- My implementation adds the: PUT /holds/{hold_id}/priority endpoint for setting the priority of a hold. It uses the same approach rotas endpoint does. And it checks for the very specific permissions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Incorrect permission |Add |requirements for holds |/holds/{hold_id}/priority |operations via the API |route (PUT) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- URL| |https://wiki.koha-community | |.org/wiki/Holds_priority_en | |dpoint_RFC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92588|0 |1 is obsolete| | Attachment #92589|0 |1 is obsolete| | Attachment #92590|0 |1 is obsolete| | --- Comment #8 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 92710 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92710&action=edit Bug 23517: Add a spec for PUT /holds/{hold_id}/priority This patch adds the OpenAPI spec for the endpoint, and tests for the desired behaviour. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #9 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 92711 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92711&action=edit Bug 23517: Add the controller method for holds priorities handling This patch implements the PUT /holds/:hold_id/priority endpoint To test: - Apply this patches - Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #10 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 92712 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92712&action=edit Bug 23517: (follow-up) Tidy tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |enhancement -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 92836 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92836&action=edit Bug 23517: (follow-up) More test cases Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92710|0 |1 is obsolete| | Attachment #92711|0 |1 is obsolete| | Attachment #92712|0 |1 is obsolete| | Attachment #92836|0 |1 is obsolete| | --- Comment #12 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 92927 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92927&action=edit Bug 23517: Add a spec for PUT /holds/{hold_id}/priority This patch adds the OpenAPI spec for the endpoint, and tests for the desired behaviour. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #13 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 92928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92928&action=edit Bug 23517: Add the controller method for holds priorities handling This patch implements the PUT /holds/:hold_id/priority endpoint To test: - Apply this patches - Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #14 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 92929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92929&action=edit Bug 23517: (follow-up) Tidy tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #15 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 92930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92930&action=edit Bug 23517: (follow-up) More test cases Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com QA Contact| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92927|0 |1 is obsolete| | Attachment #92928|0 |1 is obsolete| | Attachment #92929|0 |1 is obsolete| | Attachment #92930|0 |1 is obsolete| | --- Comment #16 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 92986 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92986&action=edit Bug 23517: Add a spec for PUT /holds/{hold_id}/priority This patch adds the OpenAPI spec for the endpoint, and tests for the desired behaviour. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #17 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 92987 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92987&action=edit Bug 23517: Add the controller method for holds priorities handling This patch implements the PUT /holds/:hold_id/priority endpoint To test: - Apply this patches - Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #18 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 92988 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92988&action=edit Bug 23517: (follow-up) Tidy tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #19 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 92989 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92989&action=edit Bug 23517: (follow-up) More test cases Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wizzyrea@gmail.com Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Can we add updating https://wiki.koha-community.org/wiki/REST_api_RFCs to the workflow once this is pushed? :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #20)
Can we add updating https://wiki.koha-community.org/wiki/REST_api_RFCs to the workflow once this is pushed? :)
Sure! I try to keep this up to date. I missed the voting meeting, but will add that as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work! Pushed to master for 19.11.00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Keywords| |Manual, | |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This development enhances release notes| |with API with routes to to | |change holds priorities. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #23 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- I've tried to apply on 19.05.x but UT does not work. Can anyone confirm ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 93143 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93143&action=edit Bug 23517: (follow-up) AddReserve expects a priority parameter Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |new feature -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Sally <sally.healey@cheshiresharedservices.gov.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sally.healey@cheshireshared | |services.gov.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Add |Add API route to update a |/holds/{hold_id}/priority |holds priority |route (PUT) | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This development enhances | release notes|with API with routes to to | |change holds priorities. | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23517 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org