From 4a30ef34c22c8df9e2786ba84fc371ae4d8dcfc0 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 12 Dec 2018 14:43:36 -0300 Subject: [PATCH] Bug 22048: Use set_password in member-password.pl This patch makes member-password.pl use $patron->set_password instead of update_password. The side effect is that setting password and userid become separate steps in the code. For the password all the initial checks are the same, but password strength is checked on calling set_password and an exception is thrown. So instead of checking the password quality, we just wait for exceptions and behave the same as before. Bonus: you will notice I reused the initially fetched $patron object. Things get simpler :-D To test: - Verify that changing the password / userid for a patron works as usual --- .../en/modules/members/member-password.tt | 1 - members/member-password.pl | 59 ++++++++++--------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt index eeb4178791..1ef3891d8c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt @@ -28,7 +28,6 @@
- [% IF ( errormsg ) %]
diff --git a/members/member-password.pl b/members/member-password.pl index e886c41976..34795c30ab 100755 --- a/members/member-password.pl +++ b/members/member-password.pl @@ -20,6 +20,8 @@ use Koha::Token; use Koha::Patrons; use Koha::Patron::Categories; +use Try::Tiny; + my $input = new CGI; my $theme = $input->param('theme') || "default"; @@ -37,22 +39,21 @@ my ( $template, $loggedinuser, $cookie, $staffflags ) = get_template_and_user( } ); -my $member = $input->param('member'); -my $cardnumber = $input->param('cardnumber'); -my $destination = $input->param('destination'); +my $patron_id = $input->param('member'); +my $destination = $input->param('destination'); my $newpassword = $input->param('newpassword'); my $newpassword2 = $input->param('newpassword2'); +my $new_user_id = $input->param('newuserid'); my @errors; my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; -my $patron = Koha::Patrons->find( $member ); +my $patron = Koha::Patrons->find( $patron_id ); output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); my $category_type = $patron->category->category_type; -my $bor = $patron->unblessed; -if ( ( $member ne $loggedinuser ) && ( $category_type eq 'S' ) ) { +if ( ( $patron_id ne $loggedinuser ) && ( $category_type eq 'S' ) ) { push( @errors, 'NOPERMISSION' ) unless ( $staffflags->{'superlibrarian'} || $staffflags->{'staffaccess'} ); @@ -61,15 +62,6 @@ if ( ( $member ne $loggedinuser ) && ( $category_type eq 'S' ) ) { push( @errors, 'NOMATCH' ) if ( ( $newpassword && $newpassword2 ) && ( $newpassword ne $newpassword2 ) ); -if ( $newpassword and not @errors ) { - my ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $newpassword ); - unless ( $is_valid ) { - push @errors, 'ERROR_password_too_short' if $error eq 'too_short'; - push @errors, 'ERROR_password_too_weak' if $error eq 'too_weak'; - push @errors, 'ERROR_password_has_whitespaces' if $error eq 'has_whitespaces'; - } -} - if ( $newpassword and not @errors) { die "Wrong CSRF token" @@ -78,25 +70,36 @@ if ( $newpassword and not @errors) { token => scalar $input->param('csrf_token'), }); - my $uid = $input->param('newuserid') || $bor->{userid}; - my $password = $input->param('newpassword'); - my $dbh = C4::Context->dbh; - if ( Koha::Patrons->find( $member )->update_password($uid, $password) ) { + try { + $patron->set_password({ password => $newpassword }); + $patron->userid($new_user_id)->store + if $new_user_id and $new_user_id ne $patron->userid; $template->param( newpassword => $newpassword ); if ( $destination eq 'circ' ) { - print $input->redirect("/cgi-bin/koha/circ/circulation.pl?findborrower=$cardnumber"); + print $input->redirect("/cgi-bin/koha/circ/circulation.pl?findborrower=" . $patron->cardnumber); } else { - print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$member"); + print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$patron_id"); } } - else { - push( @errors, 'BADUSERID' ); - } + catch { + if ( $_->isa('Koha::Exceptions::Password::TooShort') ) { + push @errors, 'ERROR_password_too_short'; + } + elsif ( $_->isa('Koha::Exceptions::Password::WhitespaceCharacters') ) { + push @errors, 'ERROR_password_has_whitespaces'; + } + elsif ( $_->isa('Koha::Exceptions::Password::TooWeak') ) { + push @errors, 'ERROR_password_too_weak'; + } + else { + push( @errors, 'BADUSERID' ); + } + }; } if ( C4::Context->preference('ExtendedPatronAttributes') ) { - my $attributes = GetBorrowerAttributes( $bor->{'borrowernumber'} ); + my $attributes = GetBorrowerAttributes( $patron_id ); $template->param( ExtendedPatronAttributes => 1, extendedattributes => $attributes @@ -104,9 +107,9 @@ if ( C4::Context->preference('ExtendedPatronAttributes') ) { } $template->param( - patron => $patron, - destination => $destination, - csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $input->cookie('CGISESSID'), }), + patron => $patron, + destination => $destination, + csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $input->cookie('CGISESSID'), }), ); if ( scalar(@errors) ) { -- 2.20.1