Bugzilla – Attachment 56157 Details for
Bug 17006
Add route to change patron's password (authenticated)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[SIGNED-OFF] Bug 17006: Add API route for changing patron's password
Bug-17006-Add-API-route-for-changing-patrons-passw.patch (text/plain), 10.28 KB, created by
Lari Taskula
on 2016-10-11 09:50:42 UTC
(
hide
)
Description:
[SIGNED-OFF] Bug 17006: Add API route for changing patron's password
Filename:
MIME Type:
Creator:
Lari Taskula
Created:
2016-10-11 09:50:42 UTC
Size:
10.28 KB
patch
obsolete
>From 573b43394912f16f8287ca46248d71cf46372097 Mon Sep 17 00:00:00 2001 >From: Lari Taskula <larit@student.uef.fi> >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 and minify swagger.json >perl misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json -d api/v1/swagger/swagger.min.json >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 <aleishaamohia@hotmail.com> >--- > 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 | 66 +++++++++++++++++++++++++++++++++++---- > 5 files changed, 172 insertions(+), 6 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index b5449a3..e557ab1 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; >@@ -267,6 +268,32 @@ sub track_login { > $self->lastseen( dt_from_string() )->store; > } > >+=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 b97a154..0bee309 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->unblessed, 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 f00b1b4..331bc99 100644 >--- a/api/v1/swagger/paths.json >+++ b/api/v1/swagger/paths.json >@@ -10,5 +10,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 f4b9410..2f48cd1 100644 >--- a/t/db_dependent/api/v1/patrons.t >+++ b/t/db_dependent/api/v1/patrons.t >@@ -17,27 +17,29 @@ > > use Modern::Perl; > >-use Test::More tests => 20; >+use Test::More tests => 38; > 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 $guarantor = $builder->build({ > source => 'Borrower', > value => { >@@ -46,6 +48,8 @@ my $guarantor = $builder->build({ > flags => 0, > } > }); >+ >+my $password = "secret"; > my $borrower = $builder->build({ > source => 'Borrower', > value => { >@@ -81,6 +85,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) >@@ -100,12 +105,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), > } > }); > >@@ -129,4 +154,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; >-- >1.9.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 17006
:
54575
|
54576
|
54629
|
54630
|
54716
|
54717
|
54718
|
56155
|
56156
|
56157
|
56158
|
56283
|
56284
|
56399
|
56400
|
56401
|
56409
|
57356
|
57357
|
57358
|
57359
|
57360
|
63329
|
63330
|
63331
|
63332
|
63333
|
67128
|
67129
|
67130
|
67131
|
67132
|
70260
|
70261
|
70262
|
70263
|
70264
|
77882
|
77883
|
77884
|
77923
|
77924
|
77925
|
78837
|
82625
|
82626
|
82627
|
82628
|
83579
|
83580
|
83581
|
83749
|
83750
|
83751
|
84285
|
84286
|
84287
|
84488