Bugzilla – Attachment 177683 Details for
Bug 28907
Potential unauthorized access in public REST routes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[24.05.x] Bug 28907: REST - Remove allow-owner from public guarantors can see charges and checkouts
2405x-Bug-28907-REST---Remove-allow-owner-from-pub.patch (text/plain), 5.85 KB, created by
Lari Taskula
on 2025-02-10 13:01:47 UTC
(
hide
)
Description:
[24.05.x] Bug 28907: REST - Remove allow-owner from public guarantors can see charges and checkouts
Filename:
MIME Type:
Creator:
Lari Taskula
Created:
2025-02-10 13:01:47 UTC
Size:
5.85 KB
patch
obsolete
>From bdfb5fc75a8c443bc9586fe9934d5e270b0e5800 Mon Sep 17 00:00:00 2001 >From: Lari Taskula <lari.taskula@hypernova.fi> >Date: Wed, 18 Sep 2024 08:46:46 +0000 >Subject: [PATCH] [24.05.x] Bug 28907: REST - Remove allow-owner from public > guarantors can see charges and checkouts > >To test: >1. prove t/db_dependent/api/v1/patrons.t >2. Apply patch >3. prove t/db_dependent/api/v1/patrons.t > >Observe success in both cases. > >Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > Koha/REST/V1/Patrons.pm | 4 ++++ > api/v1/swagger/paths/public_patrons.yaml | 4 ---- > t/db_dependent/api/v1/patrons.t | 21 +++++++++++++++++++-- > 3 files changed, 23 insertions(+), 6 deletions(-) > >diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm >index ca620109a0..555d3a171a 100644 >--- a/Koha/REST/V1/Patrons.pm >+++ b/Koha/REST/V1/Patrons.pm >@@ -456,6 +456,8 @@ sub guarantors_can_see_charges { > my $c = shift->openapi->valid_input or return; > > return try { >+ $c->auth->public( $c->param('patron_id') ); >+ > if ( C4::Context->preference('AllowPatronToSetFinesVisibilityForGuarantor') ) { > my $patron = $c->stash( 'koha.user' ); > my $privacy_setting = ($c->req->json->{allowed}) ? 1 : 0; >@@ -492,6 +494,8 @@ sub guarantors_can_see_checkouts { > my $c = shift->openapi->valid_input or return; > > return try { >+ $c->auth->public( $c->param('patron_id') ); >+ > if ( C4::Context->preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') ) { > my $patron = $c->stash( 'koha.user' ); > my $privacy_setting = ( $c->req->json->{allowed} ) ? 1 : 0; >diff --git a/api/v1/swagger/paths/public_patrons.yaml b/api/v1/swagger/paths/public_patrons.yaml >index 0d1e48c6ca..774533a445 100644 >--- a/api/v1/swagger/paths/public_patrons.yaml >+++ b/api/v1/swagger/paths/public_patrons.yaml >@@ -182,8 +182,6 @@ > description: Under maintenance > schema: > $ref: "../swagger.yaml#/definitions/error" >- x-koha-authorization: >- allow-owner: true > "/public/patrons/{patron_id}/guarantors/can_see_checkouts": > put: > x-mojo-to: Patrons#guarantors_can_see_checkouts >@@ -236,8 +234,6 @@ > description: Under maintenance > schema: > $ref: "../swagger.yaml#/definitions/error" >- x-koha-authorization: >- allow-owner: true > "/public/patrons/{patron_id}/holds/{hold_id}": > delete: > x-mojo-to: Patrons::Holds#delete_public >diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t >index 0e3c73e21d..0d8742d974 100755 >--- a/t/db_dependent/api/v1/patrons.t >+++ b/t/db_dependent/api/v1/patrons.t >@@ -1140,7 +1140,7 @@ subtest 'delete() tests' => sub { > > subtest 'guarantors_can_see_charges() tests' => sub { > >- plan tests => 11; >+ plan tests => 17; > > t::lib::Mocks::mock_preference( 'RESTPublicAPI', 1 ); > t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); >@@ -1148,6 +1148,7 @@ subtest 'guarantors_can_see_charges() tests' => sub { > $schema->storage->txn_begin; > > my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { privacy_guarantor_fines => 0 } }); >+ my $other_patron_id = $builder->build_object( { class => 'Koha::Patrons' } )->borrowernumber; > my $password = 'thePassword123'; > $patron->set_password({ password => $password, skip_validation => 1 }); > my $userid = $patron->userid; >@@ -1155,6 +1156,14 @@ subtest 'guarantors_can_see_charges() tests' => sub { > > t::lib::Mocks::mock_preference( 'AllowPatronToSetFinesVisibilityForGuarantor', 0 ); > >+ $t->put_ok( >+ "/api/v1/public/patrons/$other_patron_id/guarantors/can_see_charges" => json => { allowed => Mojo::JSON->true } >+ )->status_is(401)->json_is( { error => "Authentication failure." } ); >+ >+ $t->put_ok( "//$userid:$password@/api/v1/public/patrons/$other_patron_id/guarantors/can_see_charges" => json => >+ { allowed => Mojo::JSON->true } )->status_is(403) >+ ->json_is( { error => "Unprivileged user cannot access another user's resources" } ); >+ > $t->put_ok( "//$userid:$password@/api/v1/public/patrons/$patron_id/guarantors/can_see_charges" => json => { allowed => Mojo::JSON->true } ) > ->status_is( 403 ) > ->json_is( '/error', 'The current configuration doesn\'t allow the requested action.' ); >@@ -1178,7 +1187,7 @@ subtest 'guarantors_can_see_charges() tests' => sub { > > subtest 'guarantors_can_see_checkouts() tests' => sub { > >- plan tests => 11; >+ plan tests => 17; > > t::lib::Mocks::mock_preference( 'RESTPublicAPI', 1 ); > t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); >@@ -1186,11 +1195,19 @@ subtest 'guarantors_can_see_checkouts() tests' => sub { > $schema->storage->txn_begin; > > my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { privacy_guarantor_checkouts => 0 } }); >+ my $other_patron_id = $builder->build_object( { class => 'Koha::Patrons' } )->borrowernumber; > my $password = 'thePassword123'; > $patron->set_password({ password => $password, skip_validation => 1 }); > my $userid = $patron->userid; > my $patron_id = $patron->borrowernumber; > >+ $t->put_ok( "/api/v1/public/patrons/$other_patron_id/guarantors/can_see_checkouts" => json => >+ { allowed => Mojo::JSON->true } )->status_is(401)->json_is( { error => "Authentication failure." } ); >+ >+ $t->put_ok( "//$userid:$password@/api/v1/public/patrons/$other_patron_id/guarantors/can_see_checkouts" => json => >+ { allowed => Mojo::JSON->true } )->status_is(403) >+ ->json_is( { error => "Unprivileged user cannot access another user's resources" } ); >+ > t::lib::Mocks::mock_preference( 'AllowPatronToSetCheckoutsVisibilityForGuarantor', 0 ); > > $t->put_ok( "//$userid:$password@/api/v1/public/patrons/$patron_id/guarantors/can_see_checkouts" => json => { allowed => Mojo::JSON->true } ) >-- >2.34.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28907
:
171666
|
171667
|
171668
|
171669
|
171670
|
171671
|
171672
|
171673
|
171674
|
172434
|
172435
|
172436
|
172437
|
172438
|
172439
|
172440
|
172441
|
172442
|
174662
|
174663
|
174664
|
174665
|
174666
|
174667
|
174668
|
174669
|
174670
|
176708
|
176709
|
176710
|
176711
|
176712
|
176713
|
176714
|
176715
|
176716
|
177679
|
177680
|
177681
|
177682
|
177683
|
177684
|
177685
|
177686
|
177687
|
179000