From 6ff98aac00d5eaf19ded6b5e7d86338d5cb6195d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johanna=20R=C3=A4is=C3=A4?= Date: Wed, 18 Feb 2026 11:26:13 +0200 Subject: [PATCH] Bug 41869: Change place_holds permission to /holds POST and PATCH This patch changes the place_holds permission to be required for POST and PATCH requests to /holds. Test plan: 1. Create a patron with the place_holds permission. 2. Try to add (POST) a hold to /holds and verify that it is forbidden. 3. Try to edit (PATCH) a hold to /holds and verify that it is forbidden. 4. Apply the patch and repeat steps 2 and 3, verifying that the requests are now allowed. 5. prove t/db_dependent/api/v1/holds.t Sponsored-by: Koha-Suomi Oy --- api/v1/swagger/paths/holds.yaml | 4 +- t/db_dependent/api/v1/holds.t | 95 ++++++++++++++++++++++++++++++++- 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/api/v1/swagger/paths/holds.yaml b/api/v1/swagger/paths/holds.yaml index dcd6413c03..71b9f7dfdc 100644 --- a/api/v1/swagger/paths/holds.yaml +++ b/api/v1/swagger/paths/holds.yaml @@ -281,7 +281,7 @@ $ref: "../swagger.yaml#/definitions/error" x-koha-authorization: permissions: - reserveforothers: "1" + reserveforothers: place_holds "/holds/suspension_bulk": post: x-mojo-to: Holds#suspend_bulk @@ -475,7 +475,7 @@ $ref: "../swagger.yaml#/definitions/error" x-koha-authorization: permissions: - reserveforothers: "1" + reserveforothers: place_holds put: x-mojo-to: Holds#edit operationId: overwriteHold diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index 4181a66756..7e142e9b74 100755 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 20; +use Test::More tests => 21; use Test::NoWarnings; use Test::MockModule; use Test::Mojo; @@ -347,6 +347,99 @@ subtest 'test AllowHoldDateInFuture' => sub { ->json_is( '/hold_date', output_pref( { dt => $future_hold_date, dateformat => 'iso', dateonly => 1 } ) ); }; +subtest 'Test place_holds permission for POST and PATCH' => sub { + plan tests => 7; + + $dbh->do('DELETE FROM reserves'); + + # Create fresh biblio and item for this test + my $test_biblio = $builder->build_sample_biblio; + my $test_item = $builder->build_sample_item( { biblionumber => $test_biblio->biblionumber, itype => $itemtype } ); + + # Create a patron to receive the hold + my $hold_recipient = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $categorycode, + branchcode => $branchcode, + } + } + ); + + # Create two separate patrons to test with and without permissions + # Patron WITHOUT place_holds permission + my $patron_no_perm = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $categorycode, + branchcode => $branchcode, + flags => 0 + } + } + ); + $patron_no_perm->set_password( { password => $password, skip_validation => 1 } ); + my $userid_no_perm = $patron_no_perm->userid; + + # Patron WITH place_holds permission + my $patron_with_perm = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $categorycode, + branchcode => $branchcode, + flags => 0 + } + } + ); + $builder->build( + { + source => 'UserPermission', + value => { + borrowernumber => $patron_with_perm->borrowernumber, + module_bit => 6, + code => 'place_holds', + }, + } + ); + $patron_with_perm->set_password( { password => $password, skip_validation => 1 } ); + my $userid_with_perm = $patron_with_perm->userid; + + # place_holds is for placing holds for others + my $test_post_data = { + patron_id => $hold_recipient->borrowernumber, + biblio_id => $test_biblio->biblionumber, + item_id => $test_item->itemnumber, + pickup_library_id => $branchcode, + }; + + # Make sure pickup location checks doesn't get in the middle + my $mock_biblio = Test::MockModule->new('Koha::Biblio'); + $mock_biblio->mock( 'pickup_locations', sub { return Koha::Libraries->search; } ); + my $mock_item = Test::MockModule->new('Koha::Item'); + $mock_item->mock( 'pickup_locations', sub { return Koha::Libraries->search } ); + + # Test POST without place_holds permission - should fail + $t->post_ok( "//$userid_no_perm:$password@/api/v1/holds" => json => $test_post_data ) + ->status_is(403, 'POST without place_holds permission returns 403'); + + # Test POST with place_holds permission - should succeed + $t->post_ok( "//$userid_with_perm:$password@/api/v1/holds" => json => $test_post_data ) + ->status_is(201, 'POST with place_holds permission returns 201') + ->json_has('/hold_id'); + + my $created_hold_id = $t->tx->res->json->{hold_id}; + + # Test PATCH with place_holds permission - should succeed + my $test_patch_data = { + priority => 1, + }; + + $t->patch_ok( "//$userid_with_perm:$password@/api/v1/holds/$created_hold_id" => json => $test_patch_data ) + ->status_is(200, 'PATCH with place_holds permission returns 200'); +}; + $schema->storage->txn_rollback; subtest 'x-koha-override and AllowHoldPolicyOverride tests' => sub { -- 2.39.5