From d754aa40de8081ee0cf61bbf2b4020a2a3845efc Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Thu, 18 Aug 2016 18:33:56 +0300 Subject: [PATCH] Bug 17006: Add API route for changing patron's password PATCH /patrons/{borrowernumber}/password (change password) Required body params: - current_password - new_password To test: 1. Apply patch 2. Run t/db_dependent/api/v1/patrons.t 3. Send PATCH request to http://library/api/v1/patrons/YYY/password where YYY is existing borrowernumber (borrowers flag required) 4. Make sure that password was changed. 5. Try also too short password, and wrong current password and observe that errors are displayed appropriately. You may find this useful for testing: curl -X PATCH http://library/api/v1/patrons/123/password --data '{"current_password":"123456", "new_password":"1234"}' --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' Signed-off-by: Aleisha Amohia Signed-off-by: Josef Moravec --- 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(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index f7970ea..d4ba052 100644 --- a/Koha/Patron.pm +++ b/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 diff --git a/Koha/REST/V1/Patron.pm b/Koha/REST/V1/Patron.pm index 477980e..3f8e267 100644 --- a/Koha/REST/V1/Patron.pm +++ b/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; diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json index b1fcf40..5984bfc 100644 --- a/api/v1/swagger/paths.json +++ b/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" } } diff --git a/api/v1/swagger/paths/patrons.json b/api/v1/swagger/paths/patrons.json index 67632e1..2f57115 100644 --- a/api/v1/swagger/paths/patrons.json +++ b/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" + } + } + } } } diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 909e36e..eb4850b 100644 --- a/t/db_dependent/api/v1/patrons.t +++ b/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; -- 2.1.4