@@ -, +, @@ - 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.json | 410 ++++++++++++++++++++++++++++++++++++++ api/v1/swagger/paths.json | 3 + api/v1/swagger/paths/patrons.json | 60 ++++++ t/db_dependent/api/v1/patrons.t | 66 +++++- 6 files changed, 582 insertions(+), 6 deletions(-) create mode 100644 api/v1/swagger.json --- 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::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 --- 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->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; --- a/api/v1/swagger.json +++ a/api/v1/swagger.json @@ -0,0 +1,410 @@ +{ + "swagger": "2.0", + "info": { + "title": "Koha REST API", + "version": "1", + "license": { + "name": "GPL v3", + "url": "http://www.gnu.org/licenses/gpl.txt" + }, + "contact": { + "name": "Koha Team", + "url": "http://koha-community.org/" + } + }, + "basePath": "/api/v1", + "paths": { + "/patrons": { + "get": { + "operationId": "listPatrons", + "tags": ["patrons"], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "A list of patrons", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/patron" + } + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + } + } + } + }, + "/patrons/{borrowernumber}": { + "get": { + "operationId": "getPatron", + "tags": ["patrons"], + "parameters": [ + { + "$ref": "#/parameters/borrowernumberPathParam" + } + ], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "A patron", + "schema": { + "$ref": "#/definitions/patron" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "#/definitions/error" + } + }, + "404": { + "description": "Patron not found", + "schema": { + "$ref": "#/definitions/error" + } + } + } + } + }, + "/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", + "tags": ["borrowers", "holds"], + "parameters": [ + { + "name": "reserve_id", + "in": "query", + "description": "Internal reserve identifier", + "type": "integer" + }, + { + "name": "borrowernumber", + "in": "query", + "description": "Internal borrower identifier", + "type": "integer" + }, + { + "name": "reservedate", + "in": "query", + "description": "Reserve date", + "type": "string" + }, + { + "name": "biblionumber", + "in": "query", + "description": "Internal biblio identifier", + "type": "integer" + }, + { + "name": "branchcode", + "in": "query", + "description": "Branch code", + "type": "string" + }, + { + "name": "notificationdate", + "in": "query", + "description": "Notification date", + "type": "string" + }, + { + "name": "reminderdate", + "in": "query", + "description": "Reminder date", + "type": "string" + }, + { + "name": "cancellationdate", + "in": "query", + "description": "Cancellation date", + "type": "string" + }, + { + "name": "reservenotes", + "in": "query", + "description": "Reserve notes", + "type": "string" + }, + { + "name": "priority", + "in": "query", + "description": "Priority", + "type": "integer" + }, + { + "name": "found", + "in": "query", + "description": "Found status", + "type": "string" + }, + { + "name": "timestamp", + "in": "query", + "description": "Time of latest update", + "type": "string" + }, + { + "name": "itemnumber", + "in": "query", + "description": "Internal item identifier", + "type": "integer" + }, + { + "name": "waitingdate", + "in": "query", + "description": "Date the item was marked as waiting for the patron", + "type": "string" + }, + { + "name": "expirationdate", + "in": "query", + "description": "Date the hold expires", + "type": "string" + }, + { + "name": "lowestPriority", + "in": "query", + "description": "Lowest priority", + "type": "integer" + }, + { + "name": "suspend", + "in": "query", + "description": "Suspended", + "type": "integer" + }, + { + "name": "suspend_until", + "in": "query", + "description": "Suspended until", + "type": "string" + } + ], + "produces": ["application/json"], + "responses": { + "200": { + "description": "A list of holds", + "schema": { "$ref": "#/definitions/holds" } + }, + "404": { + "description": "Borrower not found", + "schema": { "$ref": "#/definitions/error" } + } + } + }, + "post": { + "operationId": "addHold", + "tags": ["borrowers", "holds"], + "parameters": [ + { + "name": "body", + "in": "body", + "description": "A JSON object containing informations about the new hold", + "required": true, + "schema": { + "type": "object", + "properties": { + "borrowernumber": { + "description": "Borrower internal identifier", + "type": "integer" + }, + "biblionumber": { + "description": "Biblio internal identifier", + "type": "integer" + }, + "itemnumber": { + "description": "Item internal identifier", + "type": "integer" + }, + "branchcode": { + "description": "Pickup location", + "type": "string" + }, + "expirationdate": { + "description": "Hold end date", + "type": "string", + "format": "date" + } + } + } + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "201": { + "description": "Created hold", + "schema": { "$ref": "#/definitions/hold" } + }, + "400": { + "description": "Missing or wrong parameters", + "schema": { "$ref": "#/definitions/error" } + }, + "403": { + "description": "Hold not allowed", + "schema": { "$ref": "#/definitions/error" } + }, + "404": { + "description": "Borrower not found", + "schema": { "$ref": "#/definitions/error" } + }, + "500": { + "description": "Internal error", + "schema": { "$ref": "#/definitions/error" } + } + } + } + }, + "/holds/{reserve_id}": { + "put": { + "operationId": "editHold", + "tags": ["holds"], + "parameters": [ + { "$ref": "#/parameters/holdIdPathParam" }, + { + "name": "body", + "in": "body", + "description": "A JSON object containing fields to modify", + "required": true, + "schema": { + "type": "object", + "properties": { + "priority": { + "description": "Position in waiting queue", + "type": "integer", + "minimum": 1 + }, + "branchcode": { + "description": "Pickup location", + "type": "string" + }, + "suspend_until": { + "description": "Suspend until", + "type": "string", + "format": "date" + } + } + } + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Updated hold", + "schema": { "$ref": "#/definitions/hold" } + }, + "400": { + "description": "Missing or wrong parameters", + "schema": { "$ref": "#/definitions/error" } + }, + "404": { + "description": "Hold not found", + "schema": { "$ref": "#/definitions/error" } + } + } + }, + "delete": { + "operationId": "deleteHold", + "tags": ["holds"], + "parameters": [ + { "$ref": "#/parameters/holdIdPathParam" } + ], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Successful deletion", + "schema": { + "type": "object" + } + }, + "404": { + "description": "Hold not found", + "schema": { "$ref": "#/definitions/error" } + } + } + } + } + }, + "definitions": { + "$ref": "./definitions/index.json" + }, + "parameters": { + "borrowernumberPathParam": { + "name": "borrowernumber", + "in": "path", + "description": "Internal patron identifier", + "required": true, + "type": "integer" + }, + "holdIdPathParam": { + "name": "reserve_id", + "in": "path", + "description": "Internal hold identifier", + "required": true, + "type": "integer" + } + } +} --- a/api/v1/swagger/paths.json +++ a/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" } } --- 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,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; --