From f167e96a934aacc8fd0f1f0adad50c396888fce0 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 18 Sep 2024 08:39:48 +0000 Subject: [PATCH] Bug 28097: REST - Remove allow-owner from public password route To test: 1. prove t/db_dependent/api/v1/patrons_password.t 2. Apply patch 3. prove t/db_dependent/api/v1/patrons_password.t Observe success in both cases. https://bugs.koha-community.org/show_bug.cgi?id=28907 --- Koha/REST/V1/Patrons/Password.pm | 50 ++++++++++-------------- api/v1/swagger/paths/public_patrons.yaml | 2 - t/db_dependent/api/v1/patrons_password.t | 16 +++++++- 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/Koha/REST/V1/Patrons/Password.pm b/Koha/REST/V1/Patrons/Password.pm index 77614ead5b..24522a3dc0 100644 --- a/Koha/REST/V1/Patrons/Password.pm +++ b/Koha/REST/V1/Patrons/Password.pm @@ -91,34 +91,25 @@ sub set_public { my $user = $c->stash('koha.user'); - unless ( $user->borrowernumber == $patron_id ) { - return $c->render( - status => 403, - openapi => { - error => "Changing other patron's password is forbidden" - } - ); - } + return try { + $c->auth->public($patron_id); - unless ( $user->category->effective_change_password ) { - return $c->render( - status => 403, - openapi => { - error => "Changing password is forbidden" - } - ); - } + unless ( $user->category->effective_change_password ) { + return $c->render( + status => 403, + openapi => { error => "Changing password is forbidden" } + ); + } - my $old_password = $body->{old_password}; - my $password = $body->{password}; - my $password_2 = $body->{password_repeated}; + my $old_password = $body->{old_password}; + my $password = $body->{password}; + my $password_2 = $body->{password_repeated}; - unless ( $password eq $password_2 ) { - return $c->render( status => 400, openapi => { error => "Passwords don't match" } ); - } + unless ( $password eq $password_2 ) { + return $c->render( status => 400, openapi => { error => "Passwords don't match" } ); + } - return try { - unless ( checkpw_internal($user->userid, $old_password ) ) { + unless ( checkpw_internal( $user->userid, $old_password ) ) { return $c->render( status => 400, openapi => { error => "Invalid password" } @@ -126,19 +117,20 @@ sub set_public { } ## Change password - $user->set_password({ password => $password }); + $user->set_password( { password => $password } ); return $c->render( status => 200, openapi => "" ); - } - catch { + } catch { if ( blessed $_ and $_->isa('Koha::Exceptions::Password') ) { return $c->render( status => 400, openapi => { error => "$_" } ); + } elsif ( blessed $_ and $_->isa('Koha::Exceptions::REST::Public::Unauthorized') ) { + return $c->render( status => 403, json => { error => "Changing other patron's password is forbidden" } ); + } else { + $c->unhandled_exception($_); } - - $c->unhandled_exception($_); }; } diff --git a/api/v1/swagger/paths/public_patrons.yaml b/api/v1/swagger/paths/public_patrons.yaml index 2f2526e827..9a987ee0ed 100644 --- a/api/v1/swagger/paths/public_patrons.yaml +++ b/api/v1/swagger/paths/public_patrons.yaml @@ -60,8 +60,6 @@ description: Under maintenance schema: $ref: "../swagger.yaml#/definitions/error" - x-koha-authorization: - allow-owner: true "/public/patrons/{patron_id}/checkouts": post: x-mojo-to: Checkouts#add diff --git a/t/db_dependent/api/v1/patrons_password.t b/t/db_dependent/api/v1/patrons_password.t index 8f79a4b81b..75e6669a0a 100755 --- a/t/db_dependent/api/v1/patrons_password.t +++ b/t/db_dependent/api/v1/patrons_password.t @@ -117,7 +117,7 @@ subtest 'set() (authorized user tests)' => sub { subtest 'set_public() (unprivileged user tests)' => sub { - plan tests => 18; + plan tests => 21; $schema->storage->txn_begin; @@ -162,6 +162,18 @@ subtest 'set_public() (unprivileged user tests)' => sub { } ); + $t->post_ok( + "/api/v1/public/patrons/" . $patron->id . "/password" => json => { + password => $new_password, + password_repeated => $new_password, + old_password => $password + } + )->status_is(401)->json_is( + '/error', + "Authentication failure." + ); + + t::lib::Mocks::mock_preference( 'OpacPasswordChange', 1 ); $t->post_ok( @@ -174,7 +186,7 @@ subtest 'set_public() (unprivileged user tests)' => sub { } )->status_is(403) ->json_is( '/error', - "Authorization failure. Missing required permission(s)." ); + "Changing other patron's password is forbidden" ); $t->post_ok( "//$userid:$password@/api/v1/public/patrons/" -- 2.34.1