From 2601f8fdb70c403b3ef34afaac83b15585939121 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 12 Dec 2018 14:43:36 -0300 Subject: [PATCH] Bug 21992: 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..7a7a58f2a9 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( $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