@@ -, +, @@ - current_password - new_password where YYY is existing borrowernumber (borrowers flag required) that errors are displayed appropriately. --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' --- Koha/Patron.pm | 27 ++++++++++++++++++ Koha/REST/V1/Patron.pm | 22 ++++++++++++++ api/v1/swagger/paths.json | 3 ++ api/v1/swagger/paths/patrons.json | 60 +++++++++++++++++++++++++++++++++++++++ t/db_dependent/api/v1/patrons.t | 58 +++++++++++++++++++++++++++++++++++-- 5 files changed, 168 insertions(+), 2 deletions(-) --- a/Koha/Patron.pm +++ a/Koha/Patron.pm @@ -24,6 +24,7 @@ use Carp; use C4::Context; use C4::Log; +use Koha::AuthUtils; use Koha::Checkouts; use Koha::Database; use Koha::DateUtils; @@ -654,6 +655,32 @@ sub account_locked { and $self->login_attempts >= $FailedLoginAttempts )? 1 : 0; } +=head3 change_password_to + +my $changed = $patron->change_password_to($cleartext_password); + +Changes patron's password to C<$cleartext_password>. This subroutine +also makes validations for new password, but does not check the old +one. + +=cut + +sub change_password_to { + my ($self, $cleartext_password) = @_; + + my $min_length = C4::Context->preference("minPasswordLength"); + if ($min_length > length($cleartext_password)) { + return (undef, "Password is too short. Minimum length: $min_length."); + } + if ($cleartext_password =~ m|^\s+| or $cleartext_password =~ m|\s+$|) { + return (undef, "Password cannot contain trailing whitespaces."); + } + my $hashed_password = Koha::AuthUtils::hash_password($cleartext_password); + $self->set({ password => $hashed_password })->store; + logaction( "MEMBERS", "CHANGE PASS", $self->borrowernumber, "" ) if C4::Context->preference("BorrowersLog"); + return 1; +} + =head3 type =cut --- a/Koha/REST/V1/Patron.pm +++ a/Koha/REST/V1/Patron.pm @@ -19,6 +19,8 @@ use Modern::Perl; use Mojo::Base 'Mojolicious::Controller'; + +use C4::Auth qw( haspermission checkpw_internal ); use Koha::Patrons; sub list { @@ -44,4 +46,24 @@ sub get { return $c->$cb($patron, 200); } +sub changepassword { + my ($c, $args, $cb) = @_; + + my $user = $c->stash('koha.user'); + my $patron = Koha::Patrons->find($args->{borrowernumber}); + return $c->$cb({ error => "Patron not found." }, 404) unless $patron; + + my $pw = $args->{'body'}; + my $dbh = C4::Context->dbh; + unless (checkpw_internal($dbh, $user->userid, $pw->{'current_password'})) { + return $c->$cb({ error => "Wrong current password." }, 400); + } + + my ($success, $errmsg) = $user->change_password_to($pw->{'new_password'}); + if ($errmsg) { + return $c->$cb({ error => $errmsg }, 400); + } + return $c->$cb({}, 200); +} + 1; --- a/api/v1/swagger/paths.json +++ a/api/v1/swagger/paths.json @@ -16,5 +16,8 @@ }, "/patrons/{borrowernumber}": { "$ref": "paths/patrons.json#/~1patrons~1{borrowernumber}" + }, + "/patrons/{borrowernumber}/password": { + "$ref": "paths/patrons.json#/~1patrons~1{borrowernumber}~1password" } } --- a/api/v1/swagger/paths/patrons.json +++ a/api/v1/swagger/paths/patrons.json @@ -69,5 +69,65 @@ } } } + }, + "/patrons/{borrowernumber}/password": { + "patch": { + "operationId": "changepasswordPatron", + "tags": ["patrons"], + "parameters": [{ + "$ref": "../parameters.json#/borrowernumberPathParam" + }, { + "name": "body", + "in": "body", + "description": "A JSON object containing informations about passwords", + "required": true, + "schema": { + "type": "object", + "properties": { + "current_password": { + "description": "Current password", + "type": "string" + }, + "new_password": { + "description": "New password", + "type": "string" + } + } + } + } + ], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "Password changed" + }, + "400": { + "description": "Bad request", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "404": { + "description": "Patron not found", + "schema": { + "$ref": "../definitions.json#/error" + } + } + }, + "x-koha-authorization": { + "allow-owner": true, + "permissions": { + "borrowers": "1" + } + } + } } } --- a/t/db_dependent/api/v1/patrons.t +++ a/t/db_dependent/api/v1/patrons.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 21; +use Test::More tests => 39; use Test::Mojo; use t::lib::TestBuilder; use t::lib::Mocks; @@ -25,6 +25,7 @@ use t::lib::Mocks; use C4::Auth; use C4::Context; +use Koha::AuthUtils; use Koha::Database; use Koha::Patron; @@ -42,6 +43,7 @@ my $t = Test::Mojo->new('Koha::REST::V1'); my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; + my $guarantor = $builder->build({ source => 'Borrower', value => { @@ -50,6 +52,8 @@ my $guarantor = $builder->build({ flags => 0, } }); + +my $password = "secret"; my $borrower = $builder->build({ source => 'Borrower', value => { @@ -86,6 +90,7 @@ $tx->req->cookies({name => 'CGISESSID', value => $session->id}); $t->request_ok($tx) ->status_is(403); + $tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . ($borrower->{ borrowernumber }-1)); $tx->req->cookies({name => 'CGISESSID', value => $session->id}); $t->request_ok($tx) @@ -105,12 +110,32 @@ $t->request_ok($tx) ->status_is(200) ->json_is('/guarantorid', $guarantor->{borrowernumber}); +my $password_obj = { + current_password => $password, + new_password => "new password", +}; + +$tx = $t->ua->build_tx(PATCH => '/api/v1/patrons/-100/password' => json => $password_obj); +$t->request_ok($tx) + ->status_is(401); + +$tx = $t->ua->build_tx(PATCH => '/api/v1/patrons/'.$borrower->{borrowernumber}.'/password'); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(400); + +$tx = $t->ua->build_tx(PATCH => '/api/v1/patrons/'.$guarantor->{borrowernumber}.'/password' => json => $password_obj); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(403); + my $loggedinuser = $builder->build({ source => 'Borrower', value => { branchcode => $branchcode, categorycode => $categorycode, - flags => 16 # borrowers flag + flags => 16, # borrowers flag + password => Koha::AuthUtils::hash_password($password), } }); @@ -135,4 +160,33 @@ $t->request_ok($tx) ->json_is('/surname' => $borrower->{ surname }) ->json_is('/lost' => 1 ); +$tx = $t->ua->build_tx(PATCH => '/api/v1/patrons/-100/password' => json => $password_obj); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(404); + +$tx = $t->ua->build_tx(PATCH => '/api/v1/patrons/'.$loggedinuser->{borrowernumber}.'/password' => json => $password_obj); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(200); + +ok(C4::Auth::checkpw_hash($password_obj->{'new_password'}, Koha::Patrons->find($loggedinuser->{borrowernumber})->password), "New password in database."); +is(C4::Auth::checkpw_hash($password_obj->{'current_password'}, Koha::Patrons->find($loggedinuser->{borrowernumber})->password), "", "Old password is gone."); + +$password_obj->{'current_password'} = $password_obj->{'new_password'}; +$password_obj->{'new_password'} = "a"; +t::lib::Mocks::mock_preference("minPasswordLength", 5); +$tx = $t->ua->build_tx(PATCH => '/api/v1/patrons/'.$loggedinuser->{borrowernumber}.'/password' => json => $password_obj); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(400) + ->json_like('/error', qr/Password is too short/, "Password too short"); + +$password_obj->{'new_password'} = " abcdef "; +$tx = $t->ua->build_tx(PATCH => '/api/v1/patrons/'.$loggedinuser->{borrowernumber}.'/password' => json => $password_obj); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(400) + ->json_is('/error', "Password cannot contain trailing whitespaces."); + $schema->storage->txn_rollback; --