https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=42916 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch -- 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=42916 CJ Lynce <cj.lynce@westlakelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200992|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 CJ Lynce <cj.lynce@westlakelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |cj.lynce@westlakelibrary.or | |g -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 --- Comment #4 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- (In reply to Emily Lamancusa (emlam) from comment #3)

I'm afraid this one isn't going to be that simple, unfortunately. That API endpoint also allows modifying the hold priority, which has a separate permission. I'm not seeing another endpoint that allows modifying the expiration date but not the priority, so I think we're going to have to add a new one.

This fix may be to broad, I agree. However, I really don't want to have to implement a whole new endpoint to fix a regression. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 --- Comment #5 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- (In reply to Lucas Gass (lukeg) from comment #4)

This fix may be to broad, I agree. However, I really don't want to have to implement a whole new endpoint to fix a regression.

I'm not excited about it either, and I'm definitely open to it if there's another solution! We can't fix one regression by introducing another, though. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 ayoung <ayoung@oslri.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ayoung@oslri.net --- Comment #6 from ayoung <ayoung@oslri.net> --- +1 Our member staff have noted this regression, and it is causing some issues in workflow. Just commenting to say I believe this deserves attention. Thank you! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 susan.murphy@kcpls.org changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |susan.murphy@kcpls.org --- Comment #8 from susan.murphy@kcpls.org --- Yes, this was reported to us as well and needs some attention as it does change our workflow. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 --- Comment #10 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Created attachment 203129 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203129&action=edit Bug 42916: Add PUT /holds/{hold_id}/expiration_date To test: 1. APPLY PATCH, restart_all, yarn build 2. Have a superlibrarian, staff user with place_holds permission, and staff user without place_holds permission 3. Have at least 1 hold, note the expiration date and the hold ID. 4. Use your favorite API tool to make a request o update the hold expiration date. 5. Make the request as superlibrarian, staff user with place_holds permission, and staff user without place_holds permission. It should work for the superlibrarian and the staff user with place_holds permission. The expiration date should be updated. 6. The staff user without place_holds permission should get something like: Authorization failure. Missing required permission(s).","required_permissions":{"reserveforothers":"modify_holds_priority -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 Kristi Krueger <kkrueger@cuyahogalibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kkrueger@cuyahogalibrary.or | |g --- Comment #15 from Kristi Krueger <kkrueger@cuyahogalibrary.org> --- Our staff have noticed this issue as well, and it is impacting our workflow. Would love to see this fixed soon! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 Michelle Spinney <mspinney@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mspinney@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 --- Comment #16 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #9)

Hi, we could reuse the endpoint by adding permission checks in the controller itself. This being an actual bug, and given the fact an extra endpoint with the right permissions doesn't hurt at all, I'd go for it.

Best regards.

I'm setting this back to NSO based on this. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|lucas@bywatersolutions.com |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #203209|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Trivial patch |Medium patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 --- Comment #18 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205478 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205478&action=edit Bug 42916: Allow modifying holds with place_holds, keep priority gated Modifying a hold via PATCH /holds/{hold_id} required the top-level reserveforothers permission, so staff with only place_holds could no longer edit a hold's expiration date. This is a regression. This patch lowers the operation-level permission to place_holds and uses the attribute-level permissions framework to keep the priority attribute gated behind reserveforothers/modify_holds_priority. Expiration date, pickup location and suspension can now be changed with place_holds, while changing the priority still requires modify_holds_priority. The deprecated PUT /holds/{hold_id} (full replacement) is intentionally left untouched. Changes: - Lower PATCH /holds/{hold_id} authorization to reserveforothers/place_holds - Annotate the priority attribute with x-mod-permissions (reserveforothers/modify_holds_priority) Test plan: 1. Create a staff user with place_holds but not modify_holds_priority 2. Place a hold 3. From reserve/request.pl?biblionumber=X try to edit the expiration date => FAIL: the date disappears and is not updated 4. Apply this patch, yarn build, restart_all 5. Repeat step 3 => SUCCESS: the expiration date is updated 6. Try to change the priority as that same user => SUCCESS: rejected with 403 (needs modify_holds_priority) 7. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 8. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=42916 --- Comment #19 from David Nind <david@davidnind.com> --- Created attachment 205661 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205661&action=edit Bug 42916: Add attribute-level permissions framework (x-mod-permissions) This patch adds a framework for declaring attribute-level permissions in the OpenAPI specification. A body property can carry an x-mod-permissions extension listing the permissions required to modify it: priority: type: integer x-mod-permissions: reserveforothers: modify_holds_priority The mapping is parsed once during the authentication chain and stashed for later use; it is then enforced automatically on PATCH requests, so no per-controller code is required. The explicit helper remains available for controllers that need to validate a body manually. Changes: - Add Koha::REST::Plugin::AttributePermissions, registering two helpers: - stash_attribute_permissions({ spec => $spec }): parses the operation body properties for x-mod-permissions and stashes the attribute -> permissions mapping under koha.attribute_permissions - validate_attribute_permissions($body): enforces the stashed mapping with haspermission(), throwing 403 (naming the offending attribute) - Register the plugin in Koha::REST::V1 - Koha::REST::V1::Auth::authenticate_api_request stashes the mapping and automatically enforces it for PATCH requests at both success paths - Add a PATCH fixture route and controller action to the Test plugin - No changes to existing controllers Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/attribute_permissions.t => SUCCESS: Tests pass! 3. Tests cover: - Ungated attribute allowed with the base permission - Gated attribute rejected with 403 without the attribute permission - Mixed body rejected when a gated attribute is present - Gated attribute allowed when the user has the attribute permission 4. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #21 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Step 1: I used Henry Acevedo: - Changed user name and password, so I can log in as them - Gave permissions: - Staff access, allows viewing of catalogue in staff interface (catalogue) - Place and modify holds for patrons . Click to expand "Place and modify holds for patrons (reserveforothers)" . Select only "Place holds for patrons (place_holds)" - Add, modify and view patron information (borrowers) 2. Step 2: Placed a hold for Mary Burton on Programming the Perl DBI (13) 3. Step 3: Hold placed has no expiration date, click to add one, select date, date selected does not get added 4. Step 6: - Place another hold on the record, for example for Lisa Charles - Can't change the hold priority: . selecting the priority the "Priority" column is greyed out . there are no arrows in the "Change priority" column -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42916 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |lucas@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.