From e604fc80862ccefb8f3c3adcd22ee45ea7168de0 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Thu, 18 Aug 2016 18:33:56 +0300 Subject: [PATCH] [SIGNED-OFF] 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 --- Koha/Patron.pm | 27 +++++++++++++++++++ Koha/REST/V1/Patron.pm | 28 ++++++++++++++++++- api/v1/swagger.json | 54 +++++++++++++++++++++++++++++++++++++ t/db_dependent/api/v1/patrons.t | 60 +++++++++++++++++++++++++++++++++++------ 4 files changed, 160 insertions(+), 9 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 42b066b..5e24938 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::Database; use Koha::DateUtils; use Koha::Issues; @@ -208,6 +209,32 @@ sub update_password { return 1; } +=head2 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 2851308..cacd3ae 100644 --- a/Koha/REST/V1/Patron.pm +++ b/Koha/REST/V1/Patron.pm @@ -19,7 +19,7 @@ use Modern::Perl; use Mojo::Base 'Mojolicious::Controller'; -use C4::Auth qw( haspermission ); +use C4::Auth qw( haspermission checkpw_internal ); use Koha::Patrons; sub list { @@ -55,4 +55,30 @@ sub get { return $c->$cb($patron->unblessed, 200); } +sub changepassword { + my ($c, $args, $cb) = @_; + + my $user = $c->stash('koha.user'); + my $patron = Koha::Patrons->find($args->{borrowernumber}); + unless ( $user + && ( $user->borrowernumber == $args->{borrowernumber} + || haspermission($user->userid, {borrowers => 1}) ) ) + { + return $c->$cb({ error => "You don't have the required permission" }, 403); + } + 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.json b/api/v1/swagger.json index b3f30f8..4612275 100644 --- a/api/v1/swagger.json +++ b/api/v1/swagger.json @@ -74,6 +74,60 @@ } } }, + "/patrons/{borrowernumber}/password": { + "patch": { + "operationId": "changepasswordPatron", + "tags": ["patrons"], + "parameters": [ + { "$ref": "#/parameters/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/error" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + }, + "404": { + "description": "Patron not found", + "schema": { + "$ref": "#/definitions/error" + } + } + } + } + }, "/holds": { "get": { "operationId": "listHolds", diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 6bd4e1a..816e320 100644 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -17,32 +17,34 @@ use Modern::Perl; -use Test::More tests => 10; +use Test::More tests => 26; use Test::Mojo; use t::lib::TestBuilder; +use t::lib::Mocks; use C4::Auth; use C4::Context; +use Koha::AuthUtils; use Koha::Database; use Koha::Patron; my $builder = t::lib::TestBuilder->new(); -my $dbh = C4::Context->dbh; -$dbh->{AutoCommit} = 0; -$dbh->{RaiseError} = 1; +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; $ENV{REMOTE_ADDR} = '127.0.0.1'; my $t = Test::Mojo->new('Koha::REST::V1'); my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; +my $password = "secret"; my $borrower = $builder->build({ source => 'Borrower', value => { branchcode => $branchcode, - categorycode => $categorycode + categorycode => $categorycode, } }); @@ -52,12 +54,25 @@ $t->get_ok('/api/v1/patrons') $t->get_ok("/api/v1/patrons/" . $borrower->{ borrowernumber }) ->status_is(403); +$t->patch_ok("/api/v1/patrons/-100/password") + ->status_is(400); + +my $password_obj = { + current_password => $password, + new_password => "new password", +}; + +my $tx = $t->ua->build_tx(PATCH => '/api/v1/patrons/-100/password' => json => $password_obj); +$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), } }); @@ -68,7 +83,7 @@ $session->param('ip', '127.0.0.1'); $session->param('lasttime', time()); $session->flush; -my $tx = $t->ua->build_tx(GET => '/api/v1/patrons'); +$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'}); $t->request_ok($tx) @@ -81,4 +96,33 @@ $t->request_ok($tx) ->json_is('/borrowernumber' => $borrower->{ borrowernumber }) ->json_is('/surname' => $borrower->{ surname }); -$dbh->rollback; +$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