From 43ddfd1a1bc2c2de4c33f2d1608689a5840c59b7 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Thu, 18 Aug 2016 18:52:58 +0300 Subject: [PATCH] [SIGNED-OFF] Bug 17006: Respect system preference "OpacPasswordChange" Allow patron to change their own password without borrowers-flag only if OpacPasswordChange system preference is enabled. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Set OpacPasswordChange system preference to "Don't allow". 4. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber for a borrower that does not have any permissions 5. Observe permission error. 6. Set OpacPasswordChange system preference to "Allow". 7. Repeat step 4 and make sure that password was changed. Signed-off-by: Aleisha Amohia Signed-off-by: Josef Moravec --- Koha/REST/V1/Patron.pm | 10 +++++++++- t/db_dependent/api/v1/patrons.t | 28 +++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/Koha/REST/V1/Patron.pm b/Koha/REST/V1/Patron.pm index 0bee309..2660e7f 100644 --- a/Koha/REST/V1/Patron.pm +++ b/Koha/REST/V1/Patron.pm @@ -19,8 +19,8 @@ use Modern::Perl; use Mojo::Base 'Mojolicious::Controller'; - use C4::Auth qw( haspermission checkpw_internal ); +use C4::Context; use Koha::Patrons; sub list { @@ -51,6 +51,14 @@ sub changepassword { my $user = $c->stash('koha.user'); my $patron = Koha::Patrons->find($args->{borrowernumber}); + + my $OpacPasswordChange = C4::Context->preference("OpacPasswordChange"); + unless ( $user + && ( ($OpacPasswordChange && $user->borrowernumber == $args->{borrowernumber}) + || haspermission($user->userid, {borrowers => 1}) ) ) + { + return $c->$cb({ error => "OPAC password change is disabled" }, 403); + } return $c->$cb({ error => "Patron not found." }, 404) unless $patron; my $pw = $args->{'body'}; diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 2f48cd1..551a4d0 100644 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -17,7 +17,8 @@ use Modern::Perl; -use Test::More tests => 38; + +use Test::More tests => 43; use Test::Mojo; use t::lib::TestBuilder; use t::lib::Mocks; @@ -57,6 +58,7 @@ my $borrower = $builder->build({ categorycode => $categorycode, flags => 0, guarantorid => $guarantor->{borrowernumber}, + password => Koha::AuthUtils::hash_password($password), } }); @@ -141,6 +143,13 @@ $session->param('ip', '127.0.0.1'); $session->param('lasttime', time()); $session->flush; +my $session_nopermission = C4::Auth::get_session(''); +$session_nopermission->param('number', $borrower->{ borrowernumber }); +$session_nopermission->param('id', $borrower->{ userid }); +$session_nopermission->param('ip', '127.0.0.1'); +$session_nopermission->param('lasttime', time()); +$session_nopermission->flush; + $tx = $t->ua->build_tx(GET => '/api/v1/patrons'); $tx->req->cookies({name => 'CGISESSID', value => $session->id}); $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); @@ -183,4 +192,21 @@ $t->request_ok($tx) ->status_is(400) ->json_is('/error', "Password cannot contain trailing whitespaces."); +$password_obj = { + current_password => $password, + new_password => "new password", +}; +t::lib::Mocks::mock_preference("OpacPasswordChange", 0); +$tx = $t->ua->build_tx(PATCH => '/api/v1/patrons/'.$borrower->{borrowernumber}.'/password' => json => $password_obj); +$tx->req->cookies({name => 'CGISESSID', value => $session_nopermission->id}); +$t->request_ok($tx) + ->status_is(403) + ->json_is('/error', "OPAC password change is disabled"); + +t::lib::Mocks::mock_preference("OpacPasswordChange", 1); +$tx = $t->ua->build_tx(PATCH => '/api/v1/patrons/'.$borrower->{borrowernumber}.'/password' => json => $password_obj); +$tx->req->cookies({name => 'CGISESSID', value => $session_nopermission->id}); +$t->request_ok($tx) + ->status_is(200); + $schema->storage->txn_rollback; -- 2.1.4