From f273de28a46788a30bcf3f2a066fa5760949afc4 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 Signed-off-by: Kyle M Hall Signed-off-by: Josef Moravec --- .../prog/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 eeb4178..1ef3891 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 e886c41..34795c3 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.1.4