From 48834fb39bff52483a8a5c4216876abb705476fa Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Thu, 17 Aug 2017 14:51:25 +0300 Subject: [PATCH] Bug 19133: Password recovery via REST API This patch adds password recovery functionality to REST API. POST /public/patrons/password/recovery Creates a new password recovery request. Takes a JSON body with "email" and one of "userid" or "cardnumber" as parameters. Sends patron PASSWORD_RESET email. POST /public/patrons/password/recovery/complete Completes pending password recovery. Takes a JSON body with "uuid", "new_password" and "confirm_new_password" as parameters. If passwords meet Koha's password requirements, changes patron's password to the requested new password. To test: 1. prove t/db_dependent/api/v1/patrons_password_recovery.t 2. Send POST requests to the endpoints listed above. See the description of endpoints above for required parameters. 3. Check message_queue (or your email if you actually send messages in the queue) to make sure your password recovery was enqueued 4. Observe your password has been changed once you completed the recovery process Sponsored-by: Koha-Suomi Oy Signed-off-by: Tomas Cohen Arazi --- Koha/Exceptions/Patron.pm | 3 + Koha/REST/V1/Patrons/Password.pm | 121 +++++++ api/v1/swagger/definitions.json | 3 + .../definitions/patron_password_recovery.json | 13 + api/v1/swagger/paths.json | 6 + api/v1/swagger/paths/patrons.json | 128 +++++++ .../api/v1/patrons_password_recovery.t | 316 ++++++++++++++++++ 7 files changed, 590 insertions(+) create mode 100644 api/v1/swagger/definitions/patron_password_recovery.json create mode 100644 t/db_dependent/api/v1/patrons_password_recovery.t diff --git a/Koha/Exceptions/Patron.pm b/Koha/Exceptions/Patron.pm index 8eac9c9366a..b1654e6ccbe 100644 --- a/Koha/Exceptions/Patron.pm +++ b/Koha/Exceptions/Patron.pm @@ -9,6 +9,9 @@ use Exception::Class ( 'Koha::Exceptions::Patron::FailedDelete' => { description => "Deleting patron failed" }, + 'Koha::Exceptions::Patron::NotFound' => { + description => "Deleting patron failed" + }, ); 1; diff --git a/Koha/REST/V1/Patrons/Password.pm b/Koha/REST/V1/Patrons/Password.pm index 82531d9d498..f1e9691dbaf 100644 --- a/Koha/REST/V1/Patrons/Password.pm +++ b/Koha/REST/V1/Patrons/Password.pm @@ -21,6 +21,12 @@ use Mojo::Base 'Mojolicious::Controller'; use C4::Auth qw(checkpw_internal); +use Koha::AuthUtils qw(hash_password); +use Koha::Patron::Password::Recovery qw( + SendPasswordRecoveryEmail + ValidateBorrowernumber + CompletePasswordRecovery +); use Koha::Patrons; use Scalar::Util qw(blessed); @@ -34,6 +40,121 @@ Koha::REST::V1::Patrons::Password =head2 Methods +=head3 recovery + +=cut + +sub recovery { + my $c = shift->openapi->valid_input or return; + + my $patron; + return try { + my $body = $c->req->json; + + unless (C4::Context->preference('OpacPasswordChange') and + C4::Context->preference('OpacPasswordReset')) + { + return $c->render(status => 403, openapi => { + error => 'Password recovery is disabled.' + }); + } + + unless (defined $body->{userid} or defined $body->{cardnumber}) { + Koha::Exceptions::BadParameter->throw( + error => 'Either userid or cardnumber must be given.' + ); + } + + my $patron = Koha::Patrons->search({ + email => $body->{email}, + '-or' => { + userid => $body->{userid}, + cardnumber => $body->{cardnumber}, + } + })->next; + + unless ($patron) { + Koha::Exceptions::Patron::NotFound->throw( + error => 'Patron not found' + ); + } + + my $resend = ValidateBorrowernumber($patron->borrowernumber); + + SendPasswordRecoveryEmail($patron, $patron->email, $resend); + + return $c->render(status => 201, openapi => { + status => 1, + to_address => $patron->email + }); + } + catch { + unless ( blessed $_ && $_->can('rethrow') ) { + return $c->render( status => 500, openapi => { error => "$_" } ); + } + + if ($_->isa('Koha::Exceptions::BadParameter')) { + return $c->render(status => 400, openapi => { error => $_->error }); + } + elsif ($_->isa('Koha::Exceptions::Patron::NotFound')) { + return $c->render(status => 404, openapi => { error => $_->error }); + } + return $c->render(status => 500, openapi => { error => "$_" }); + }; +} + +=head3 complete_recovery + +=cut + +sub complete_recovery { + my $c = shift->openapi->valid_input or return; + + my $rs = Koha::Database->new->schema->resultset('BorrowerPasswordRecovery'); + return try { + my $body = $c->req->json; + + my $password_recovery = $rs->find({ + uuid => $body->{uuid} + }); + unless ($password_recovery) { + return $c->render(status => 404, openapi => { + error => 'Password recovery request with given uuid not found.' + }); + } + + my $patron = Koha::Patrons->find($password_recovery->borrowernumber); + my $categorycode = $patron->categorycode; + my $password = $body->{new_password}; + $patron->set_password( { password => $password } ); + return $c->render(status => 200, openapi => {}); + } + catch { + unless ( blessed $_ && $_->can('rethrow') ) { + return $c->render( status => 500, openapi => { error => "$_" } ); + } + + if ($_->isa('Koha::Exceptions::Password::TooShort')) { + return $c->render( status => 400, openapi => { + error => 'Password is too short', + minlength => $_->min_length, + length => $_->length + } ); + } + elsif ($_->isa('Koha::Exceptions::Password::TooWeak')) { + return $c->render( status => 400, openapi => { + error => 'Password is too weak', + } ); + } + elsif ($_->isa('Koha::Exceptions::Password::WhitespaceCharacters')) { + return $c->render( status => 400, openapi => { + error => 'Password contains trailing whitespace characters', + } ); + } + return $c->render(status => 500, openapi => { error => "$_" }); + }; +} + =head3 set Controller method that sets a patron's password, permission driven diff --git a/api/v1/swagger/definitions.json b/api/v1/swagger/definitions.json index 61e6c153064..677612da7a4 100644 --- a/api/v1/swagger/definitions.json +++ b/api/v1/swagger/definitions.json @@ -56,6 +56,9 @@ "patron_account_credit": { "$ref": "definitions/patron_account_credit.json" }, + "patron_password_recovery": { + "$ref": "definitions/patron_password_recovery.json" + }, "patron_balance": { "$ref": "definitions/patron_balance.json" }, diff --git a/api/v1/swagger/definitions/patron_password_recovery.json b/api/v1/swagger/definitions/patron_password_recovery.json new file mode 100644 index 00000000000..30d62de385e --- /dev/null +++ b/api/v1/swagger/definitions/patron_password_recovery.json @@ -0,0 +1,13 @@ +{ + "type": "object", + "properties": { + "to_address": { + "description": "Patron email address", + "type": ["string", "null"] + }, + "status": { + "description": "Status code. 1 = email has been enqueued", + "type": "integer" + } + } +} diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json index 8ba95189dc3..24d1d7ac13f 100644 --- a/api/v1/swagger/paths.json +++ b/api/v1/swagger/paths.json @@ -122,6 +122,12 @@ "/public/patrons/{patron_id}/guarantors/can_see_charges": { "$ref": "paths/public_patrons.json#/~1public~1patrons~1{patron_id}~1guarantors~1can_see_charges" }, + "/public/patrons/password/recovery": { + "$ref": "paths/patrons.json#/~1public~1patrons~1password~1recovery" + }, + "/public/patrons/password/recovery/complete": { + "$ref": "paths/patrons.json#/~1public~1patrons~1password~1recovery~1complete" + }, "/public/patrons/{patron_id}/guarantors/can_see_checkouts": { "$ref": "paths/public_patrons.json#/~1public~1patrons~1{patron_id}~1guarantors~1can_see_checkouts" }, diff --git a/api/v1/swagger/paths/patrons.json b/api/v1/swagger/paths/patrons.json index 3e4e7739a10..f57fb4328ce 100644 --- a/api/v1/swagger/paths/patrons.json +++ b/api/v1/swagger/paths/patrons.json @@ -703,5 +703,133 @@ } } } + }, + "/public/patrons/password/recovery": { + "post": { + "x-mojo-to": "Patrons::Password#recovery", + "operationId": "addPasswordRecoveryRequest", + "description": "Creates a new password recovery request.", + "tags": ["password"], + "parameters": [{ + "name": "body", + "in": "body", + "description": "A JSON object containing fields for recovery request", + "required": true, + "schema": { + "type": "object", + "properties": { + "userid": { + "description": "Patron's userid (this field or cardnumber required)", + "type": "string" + }, + "cardnumber": { + "description": "Patron's cardnumber (this field or userid required)", + "type": "string" + }, + "email": { + "description": "Patron's email (required)", + "type": "string" + } + }, + "required": ["email"] + } + }], + "produces": ["application/json"], + "responses": { + "201": { + "description": "Password recovery request created", + "schema": { "$ref": "../definitions.json#/patron_password_recovery" } + }, + "400": { + "description": "Bad parameter(s)", + "schema": { "$ref": "../definitions.json#/error" } + }, + "403": { + "description": "Password recovery disabled, no access to this endpoint", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "404": { + "description": "One or more of the given parameters not found", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "500": { + "description": "Internal server error", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "503": { + "description": "Under maintenance", + "schema": { + "$ref": "../definitions.json#/error" + } + } + } + } + }, + "/public/patrons/password/recovery/complete": { + "post": { + "x-mojo-to": "Patrons::Password#complete_recovery", + "operationId": "completePasswordRecoveryRequest", + "description": "Completes a password recovery request.", + "tags": ["password"], + "parameters": [{ + "name": "body", + "in": "body", + "description": "A JSON object containing fields for completing recovery request", + "required": true, + "schema": { + "type": "object", + "properties": { + "uuid": { + "description": "Uuid generated in /patrons/password/recovery", + "type": "string" + }, + "new_password": { + "description": "Patron's new password", + "type": "string" + }, + "confirm_new_password": { + "description": "Confirm patron's new password", + "type": "string" + } + }, + "required": ["uuid", "new_password", "confirm_new_password"] + } + }], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Password recovery completed", + "schema": { "type": "object" } + }, + "400": { + "description": "Bad parameter(s)", + "schema": { "$ref": "../definitions.json#/error" } + }, + "404": { + "description": "Uuid not found", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "500": { + "description": "Internal server error", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "503": { + "description": "Under maintenance", + "schema": { + "$ref": "../definitions.json#/error" + } + } + } + } } } diff --git a/t/db_dependent/api/v1/patrons_password_recovery.t b/t/db_dependent/api/v1/patrons_password_recovery.t new file mode 100644 index 00000000000..d59979c810f --- /dev/null +++ b/t/db_dependent/api/v1/patrons_password_recovery.t @@ -0,0 +1,316 @@ +#!/usr/bin/env perl + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; + +use Test::More tests => 2; +use Test::Mojo; +use Test::MockModule; + +use t::lib::Mocks; +use t::lib::TestBuilder; + +use C4::Auth; +use C4::Context; + +use Koha::Database; +use Koha::Notice::Messages; + +use Crypt::Eksblowfish::Bcrypt qw(en_base64); + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +# FIXME: sessionStorage defaults to mysql, but it seems to break transaction handling +# this affects the other REST api tests +t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' ); + +my $remote_address = '127.0.0.1'; +my $t = Test::Mojo->new('Koha::REST::V1'); + +subtest 'recovery() tests' => sub { + plan tests => 29; + + $schema->storage->txn_begin; + + my $url = '/api/v1/public/patrons/password/recovery'; + + my ($patron, $session) = create_user_and_session(); + + my $tx = $t->ua->build_tx(POST => $url => json => {}); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(400); + + t::lib::Mocks::mock_preference('OpacPasswordReset', 0); + t::lib::Mocks::mock_preference('OpacPasswordChange', 0); + $tx = $t->ua->build_tx(POST => $url => json => { + email => $patron->email, + cardnumber => $patron->cardnumber + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(403); + + t::lib::Mocks::mock_preference('OpacPasswordChange', 1); + $tx = $t->ua->build_tx(POST => $url => json => { + email => $patron->email, + cardnumber => $patron->cardnumber + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(403); + + t::lib::Mocks::mock_preference('OpacPasswordReset', 1); + t::lib::Mocks::mock_preference('OpacPasswordChange', 0); + $tx = $t->ua->build_tx(POST => $url => json => { + email => $patron->email, + cardnumber => $patron->cardnumber + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(403); + + t::lib::Mocks::mock_preference('OpacPasswordChange', 1); + + $tx = $t->ua->build_tx(POST => $url => json => { + email => 'nonexistent', + cardnumber => $patron->cardnumber + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(404); + + $tx = $t->ua->build_tx(POST => $url => json => { + email => $patron->email, + cardnumber => 'nonexistent' + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(404); + + $tx = $t->ua->build_tx(POST => $url => json => { + email => 'nonexistent', + userid => $patron->userid + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(404); + + $tx = $t->ua->build_tx(POST => $url => json => { + email => $patron->email, + userid => 'nonexistent' + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(404); + + $tx = $t->ua->build_tx(POST => $url => json => { + email => $patron->email, + cardnumber => $patron->cardnumber + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(201) + ->json_is('/status' => 1); + + my $rs = Koha::Database->new->schema->resultset('BorrowerPasswordRecovery'); + is( + $rs->search({ borrowernumber => $patron->borrowernumber })->count, 1, + 'Password modification request found in database' + ); + + $tx = $t->ua->build_tx(POST => $url => json => { + email => $patron->email, + userid => $patron->userid + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(201) + ->json_is('/status' => 1); + + is( + $rs->search({ borrowernumber => $patron->borrowernumber })->count, 1, + 'Password modification request found in database' + ); + + $tx = $t->ua->build_tx(POST => $url => json => { + email => $patron->email, + userid => $patron->userid, + cardnumber => $patron->cardnumber, + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(201) + ->json_is('/status' => 1); + + is( + $rs->search({ borrowernumber => $patron->borrowernumber })->count, 1, + 'Password modification request found in database' + ); + + my $notice = Koha::Notice::Messages->search({ + borrowernumber => $patron->borrowernumber, + letter_code => 'PASSWORD_RESET', + message_transport_type => 'email' + })->count; + is($notice, 3, 'Found password reset letters in message queue.'); + + $schema->storage->txn_rollback; +}; + +subtest 'complete_recovery() tests' => sub { + plan tests => 16; + + $schema->storage->txn_begin; + + my $rs = Koha::Database->new->schema->resultset('BorrowerPasswordRecovery'); + + my ($patron, $session) = create_user_and_session(); + my $uuid_str; + do { + $uuid_str = '$2a$08$'.en_base64(Koha::AuthUtils::generate_salt('weak', 16)); + } while ( substr ( $uuid_str, -1, 1 ) eq '.' ); + my $recovery = $builder->build({ + source => 'BorrowerPasswordRecovery', + value => { + borrowernumber => $patron->borrowernumber, + uuid => $uuid_str + } + }); + + my $url = '/api/v1/public/patrons/password/recovery/complete'; + + my $tx = $t->ua->build_tx(POST => $url => json => {}); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(400); + + $tx = $t->ua->build_tx(POST => $url.'notfound' => json => { + uuid => $uuid_str, + new_password => 'test', + confirm_new_password => 'test', + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(404); + + t::lib::Mocks::mock_preference('minPasswordLength', 6); + my $new_pass = 'short'; + $tx = $t->ua->build_tx(POST => $url => json => { + uuid => $uuid_str, + new_password => $new_pass, + confirm_new_password => $new_pass, + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(400) + ->json_like('/error', qr/too short/); + + $new_pass = ' wh1te Space! '; + $tx = $t->ua->build_tx(POST => $url => json => { + uuid => $uuid_str, + new_password => $new_pass, + confirm_new_password => $new_pass, + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(400) + ->json_like('/error', qr/trailing/); + + $new_pass = 'tooweak'; + $tx = $t->ua->build_tx(POST => $url => json => { + uuid => $uuid_str, + new_password => $new_pass, + confirm_new_password => $new_pass, + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(400) + ->json_like('/error', qr/too weak/); + + t::lib::Mocks::mock_preference('minPasswordLength', 4); + $new_pass = 'STR 0NG ENOugh P@@ssword'; + $tx = $t->ua->build_tx(POST => $url => json => { + uuid => $uuid_str, + new_password => $new_pass, + confirm_new_password => $new_pass, + }); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $tx->req->env({REMOTE_ADDR => '127.0.0.1'}); + $t->request_ok($tx) + ->status_is(200); + + my $stored_pw = Koha::Patrons->find($patron->borrowernumber)->password; + is( + $stored_pw, + Koha::AuthUtils::hash_password($new_pass, $stored_pw), 'Password changed' + ); + + $schema->storage->txn_rollback; +}; + +sub create_user_and_session { + my $args = shift; + + my $flags = ( $args->{authorized} ) ? $args->{authorized} : 0; + my $categorycode = $builder->build({ source => 'Category' })->{categorycode}; + my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; + my $dbh = C4::Context->dbh; + + my $borrower = $builder->build({ + source => 'Borrower', + value => { + branchcode => $branchcode, + categorycode => $categorycode, + lost => 0, + flags => $flags, + } + }); + + my $session = C4::Auth::get_session(''); + $session->param('number', $borrower->{ borrowernumber }); + $session->param('id', $borrower->{ userid }); + $session->param('ip', '127.0.0.1'); + $session->param('lasttime', time()); + $session->flush; + my $patron = Koha::Patrons->find($borrower->{borrowernumber}); + if ( $args->{authorized} ) { + $dbh->do( " + INSERT INTO user_permissions (borrowernumber,module_bit,code) + VALUES (?,4,'get_password_reset_uuid')", undef, + $borrower->{borrowernumber} ); + } + + return ($patron, $session); +} -- 2.29.2