From 69cdf3ae28e4ab3eb50abf1c2fe4cd90f2049d26 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 2 Feb 2026 13:58:31 +0000 Subject: [PATCH] Bug 41751: Allow anonymous_refund permission to access cashups API The cash register transaction history page (pos/register.pl) allows access with either cashup OR anonymous_refund permission, but the API endpoints for fetching cashups only checked for cashup permission. This caused a 403 error when users with only anonymous_refund permission tried to view the transaction history page, as the cashups table failed to load. This patch updates the API permissions for: - GET /cash_registers/{id}/cashups - GET /cashups/{id} to accept either cashup or anonymous_refund permission, matching the page access logic. Test plan: 1. Create a staff user with only cash_management > anonymous_refund permission (not cashup) 2. Navigate to Point of Sale > Transaction history for any cash register 3. Verify the cashups table loads successfully (previously returned 403) 4. Run: prove t/db_dependent/api/v1/cashups.t --- api/v1/swagger/paths/cash_registers.yaml | 8 ++- t/db_dependent/api/v1/cashups.t | 69 +++++++++++++++++++++--- 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/api/v1/swagger/paths/cash_registers.yaml b/api/v1/swagger/paths/cash_registers.yaml index 21090fb33f5..f95b9f69547 100644 --- a/api/v1/swagger/paths/cash_registers.yaml +++ b/api/v1/swagger/paths/cash_registers.yaml @@ -115,7 +115,9 @@ $ref: "../swagger.yaml#/definitions/error" x-koha-authorization: permissions: - cash_management: cashup + cash_management: + - cashup + - anonymous_refund "/cashups/{cashup_id}": get: x-mojo-to: CashRegisters::Cashups#get @@ -167,4 +169,6 @@ $ref: "../swagger.yaml#/definitions/error" x-koha-authorization: permissions: - cash_management: cashup + cash_management: + - cashup + - anonymous_refund diff --git a/t/db_dependent/api/v1/cashups.t b/t/db_dependent/api/v1/cashups.t index 6110ff81a5c..05fc7ea2054 100755 --- a/t/db_dependent/api/v1/cashups.t +++ b/t/db_dependent/api/v1/cashups.t @@ -36,7 +36,7 @@ t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); subtest 'list() tests' => sub { - plan tests => 17; + plan tests => 19; $schema->storage->txn_begin; @@ -68,7 +68,8 @@ subtest 'list() tests' => sub { my $non_existent_cr_id = $cash_register_to_delete->id; $cash_register_to_delete->delete; - $t->get_ok("//$userid:$password@/api/v1/cash_registers/$non_existent_cr_id/cashups")->status_is(404) + $t->get_ok("//$userid:$password@/api/v1/cash_registers/$non_existent_cr_id/cashups") + ->status_is(404) ->json_is( '/error' => 'Register not found' ); my $register = $builder->build_object( { class => 'Koha::Cash::Registers' } ); @@ -89,7 +90,8 @@ subtest 'list() tests' => sub { ); # One cashup created, should get returned - $t->get_ok("//$userid:$password@/api/v1/cash_registers/$register_id/cashups")->status_is(200) + $t->get_ok("//$userid:$password@/api/v1/cash_registers/$register_id/cashups") + ->status_is(200) ->json_is( [ $cashup->to_api ] ); my $another_cashup = $builder->build_object( @@ -104,11 +106,13 @@ subtest 'list() tests' => sub { ); # One more cashup created, both should be returned - $t->get_ok("//$userid:$password@/api/v1/cash_registers/$register_id/cashups")->status_is(200) + $t->get_ok("//$userid:$password@/api/v1/cash_registers/$register_id/cashups") + ->status_is(200) ->json_is( [ $another_cashup->to_api, $cashup->to_api, ] ); # Warn on unsupported query parameter - $t->get_ok("//$userid:$password@/api/v1/cash_registers/$register_id/cashups?cashup_blah=blah")->status_is(400) + $t->get_ok("//$userid:$password@/api/v1/cash_registers/$register_id/cashups?cashup_blah=blah") + ->status_is(400) ->json_is( [ { @@ -121,12 +125,37 @@ subtest 'list() tests' => sub { # Unauthorized access $t->get_ok("//$unauth_userid:$password@/api/v1/cash_registers/$register_id/cashups")->status_is(403); + # Test that anonymous_refund permission also grants access (bug fix) + my $refund_librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 0 } + } + ); + $refund_librarian->set_password( { password => $password, skip_validation => 1 } ); + my $refund_userid = $refund_librarian->userid; + + $builder->build( + { + source => 'UserPermission', + value => { + borrowernumber => $refund_librarian->borrowernumber, + module_bit => 25, # cash_management + code => 'anonymous_refund', + }, + } + ); + + $t->get_ok("//$refund_userid:$password@/api/v1/cash_registers/$register_id/cashups") + ->status_is(200) + ->or( sub { diag "anonymous_refund permission should allow access to cashups list" } ); + $schema->storage->txn_rollback; }; subtest 'get() tests' => sub { - plan tests => 8; + plan tests => 10; $schema->storage->txn_begin; @@ -159,8 +188,34 @@ subtest 'get() tests' => sub { my $non_existent_id = $cashup_to_delete->id; $cashup_to_delete->delete; - $t->get_ok("//$userid:$password@/api/v1/cashups/$non_existent_id")->status_is(404) + $t->get_ok("//$userid:$password@/api/v1/cashups/$non_existent_id") + ->status_is(404) ->json_is( '/error' => 'Cashup not found' ); + # Test that anonymous_refund permission also grants access (bug fix) + my $refund_librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 0 } + } + ); + $refund_librarian->set_password( { password => $password, skip_validation => 1 } ); + my $refund_userid = $refund_librarian->userid; + + $builder->build( + { + source => 'UserPermission', + value => { + borrowernumber => $refund_librarian->borrowernumber, + module_bit => 25, # cash_management + code => 'anonymous_refund', + }, + } + ); + + $t->get_ok( "//$refund_userid:$password@/api/v1/cashups/" . $cashup->id ) + ->status_is(200) + ->or( sub { diag "anonymous_refund permission should allow access to get cashup" } ); + $schema->storage->txn_rollback; }; -- 2.52.0