Bugzilla – Attachment 67132 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]
Bug 17006: (follow-up) Use Koha::Exceptions for change password endpoint
Bug-17006-follow-up-Use-KohaExceptions-for-change-.patch (text/plain), 7.42 KB, created by
Josef Moravec
on 2017-09-14 08:36:20 UTC
(
hide
)
Description:
Bug 17006: (follow-up) Use Koha::Exceptions for change password endpoint
Filename:
MIME Type:
Creator:
Josef Moravec
Created:
2017-09-14 08:36:20 UTC
Size:
7.42 KB
patch
obsolete
>From 7891256c516d0e5b3d50388582c5737d495575fd Mon Sep 17 00:00:00 2001 >From: Lari Taskula <lari.taskula@jns.fi> >Date: Thu, 13 Oct 2016 15:49:42 +0300 >Subject: [PATCH] Bug 17006: (follow-up) Use Koha::Exceptions for change > password endpoint > >Koha::Exceptions are a nice way to handle erros and they should be used much >more than they are now. This patch adds Koha::Exceptions into >Koha::Patron->change_password_to sub and catches the exceptions in the controller. > >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. > >curl -X PATCH http://library/api/v1/patrons/123/password \ > --data '{"current_password":"123", "new_password":"1234"}' \ > --cookie 'CGISESSID=055ca5a9a3ad3fb5052143bd015c1c10' > >Signed-off-by: Josef Moravec <josef.moravec@gmail.com> >--- > Koha/Exceptions.pm | 5 ++++ > Koha/Exceptions/Password.pm | 24 +++++++++++++++ > Koha/Patron.pm | 11 +++++-- > Koha/REST/V1/Patron.pm | 73 ++++++++++++++++++++++++++++++++------------- > 4 files changed, 89 insertions(+), 24 deletions(-) > create mode 100644 Koha/Exceptions/Password.pm > >diff --git a/Koha/Exceptions.pm b/Koha/Exceptions.pm >index 60b9e6a..7979489 100644 >--- a/Koha/Exceptions.pm >+++ b/Koha/Exceptions.pm >@@ -8,6 +8,11 @@ use Exception::Class ( > 'Koha::Exceptions::Exception' => { > description => 'Something went wrong!', > }, >+ 'Koha::Exceptions::BadSystemPreference' => { >+ isa => 'Koha::Exceptions::Exception', >+ description => 'System preference value is incomprehensible', >+ fields => ['preference'], >+ }, > 'Koha::Exceptions::DuplicateObject' => { > isa => 'Koha::Exceptions::Exception', > description => 'Same object already exists', >diff --git a/Koha/Exceptions/Password.pm b/Koha/Exceptions/Password.pm >new file mode 100644 >index 0000000..3fb5d04 >--- /dev/null >+++ b/Koha/Exceptions/Password.pm >@@ -0,0 +1,24 @@ >+package Koha::Exceptions::Password; >+ >+use Modern::Perl; >+ >+use Exception::Class ( >+ >+ 'Koha::Exceptions::Password' => { >+ description => 'Something went wrong!', >+ }, >+ 'Koha::Exceptions::Password::Invalid' => { >+ isa => 'Koha::Exceptions::Password', >+ description => 'Invalid password', >+ }, >+ 'Koha::Exceptions::Password::TooShort' => { >+ isa => 'Koha::Exceptions::Password', >+ description => 'Password is too short', >+ }, >+ 'Koha::Exceptions::Password::TrailingWhitespaces' => { >+ isa => 'Koha::Exceptions::Password', >+ description => 'Password contains trailing whitespace(s)', >+ } >+); >+ >+1; >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index d4ba052..17ca896 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -28,6 +28,7 @@ use Koha::AuthUtils; > use Koha::Checkouts; > use Koha::Database; > use Koha::DateUtils; >+use Koha::Exceptions::Password; > use Koha::Holds; > use Koha::Old::Checkouts; > use Koha::Patron::Categories; >@@ -670,10 +671,14 @@ sub change_password_to { > > my $min_length = C4::Context->preference("minPasswordLength"); > if ($min_length > length($cleartext_password)) { >- return (undef, "Password is too short. Minimum length: $min_length."); >+ Koha::Exceptions::Password::TooShort->throw( >+ "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."); >+ elsif ($cleartext_password =~ m|^\s+| or $cleartext_password =~ m|\s+$|) { >+ Koha::Exceptions::Password::TrailingWhitespaces->throw( >+ "Password cannot contain trailing whitespaces." >+ ); > } > my $hashed_password = Koha::AuthUtils::hash_password($cleartext_password); > $self->set({ password => $hashed_password })->store; >diff --git a/Koha/REST/V1/Patron.pm b/Koha/REST/V1/Patron.pm >index dc32217..dff5208 100644 >--- a/Koha/REST/V1/Patron.pm >+++ b/Koha/REST/V1/Patron.pm >@@ -21,8 +21,13 @@ use Mojo::Base 'Mojolicious::Controller'; > > use C4::Auth qw( haspermission checkpw_internal ); > use C4::Context; >+use Koha::Exceptions; >+use Koha::Exceptions::Password; > use Koha::Patrons; > >+use Scalar::Util qw(blessed); >+use Try::Tiny; >+ > sub list { > my ($c, $args, $cb) = @_; > >@@ -49,29 +54,55 @@ sub get { > sub changepassword { > my ($c, $args, $cb) = @_; > >- my $user = $c->stash('koha.user'); >- my $patron = Koha::Patrons->find($args->{borrowernumber}); >- >- my $OpacPasswordChange = C4::Context->preference("OpacPasswordChange"); >- my $haspermission = haspermission($user->userid, {borrowers => 1}); >- unless ($OpacPasswordChange && $user->borrowernumber == $args->{borrowernumber} >- || $haspermission) { >- return $c->$cb({ error => "OPAC password change is disabled" }, 403); >+ my $patron; >+ my $user; >+ try { >+ >+ $patron = Koha::Patrons->find($args->{borrowernumber}); >+ $user = $c->stash('koha.user'); >+ >+ my $OpacPasswordChange = C4::Context->preference("OpacPasswordChange"); >+ my $haspermission = haspermission($user->userid, {borrowers => 1}); >+ unless ($OpacPasswordChange && $user->borrowernumber == $args->{borrowernumber}) { >+ Koha::Exceptions::BadSystemPreference->throw( >+ preference => 'OpacPasswordChange' >+ ) unless $haspermission; >+ } >+ >+ my $pw = $args->{'body'}; >+ my $dbh = C4::Context->dbh; >+ unless ($haspermission || checkpw_internal($dbh, $patron->userid, $pw->{'current_password'})) { >+ Koha::Exceptions::Password::Invalid->throw; >+ } >+ $patron->change_password_to($pw->{'new_password'}); >+ return $c->$cb({}, 200); > } >- return $c->$cb({ error => "Patron not found." }, 404) unless $patron; >- >- my $pw = $args->{'body'}; >- my $dbh = C4::Context->dbh; >- unless ($haspermission >- || checkpw_internal($dbh, $patron->userid, $pw->{'current_password'})) { >- return $c->$cb({ error => "Wrong current password." }, 400); >- } >- >- my ($success, $errmsg) = $patron->change_password_to($pw->{'new_password'}); >- if ($errmsg) { >- return $c->$cb({ error => $errmsg }, 400); >+ catch { >+ if (not defined $patron) { >+ return $c->$cb({ error => "Patron not found." }, 404); >+ } >+ elsif (not defined $user) { >+ return $c->$cb({ error => "User must be defined." }, 500); >+ } >+ >+ die $_ unless blessed $_ && $_->can('rethrow'); >+ if ($_->isa('Koha::Exceptions::Password::Invalid')) { >+ return $c->$cb({ error => "Wrong current password." }, 400); >+ } >+ elsif ($_->isa('Koha::Exceptions::Password::TooShort')) { >+ return $c->$cb({ error => $_->error }, 400); >+ } >+ elsif ($_->isa('Koha::Exceptions::Password::TrailingWhitespaces')) { >+ return $c->$cb({ error => $_->error }, 400); >+ } >+ elsif ($_->isa('Koha::Exceptions::BadSystemPreference') >+ && $_->preference eq 'OpacPasswordChange') { >+ return $c->$cb({ error => "OPAC password change is disabled" }, 403); >+ } >+ else { >+ return $c->$cb({ error => "Something went wrong. $_" }, 500); >+ } > } >- return $c->$cb({}, 200); > } > > 1; >-- >2.1.4
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