Bugzilla – Attachment 68105 Details for
Bug 18298
Allow enforcing password complexity
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18298: Add server-side checks and refactor stuffs
Bug-18298-Add-server-side-checks-and-refactor-stuf.patch (text/plain), 27.77 KB, created by
Tomás Cohen Arazi (tcohen)
on 2017-10-13 19:57:06 UTC
(
hide
)
Description:
Bug 18298: Add server-side checks and refactor stuffs
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2017-10-13 19:57:06 UTC
Size:
27.77 KB
patch
obsolete
>From e6d1c12a7efd4aae7abb5cfd8012b676c2cb24e7 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Thu, 16 Mar 2017 23:03:20 -0300 >Subject: [PATCH] Bug 18298: Add server-side checks and refactor stuffs >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >Now that we have a check client-side, nothing prevents us from a smart guy to >bypass it and force an invalid password. >This patch adds two new subroutines to Koha::AuthUtils to check the >validity of passwords and generate a password server-side. It is used >only once (self-registration) but could be useful later. > >Moreover the 3 different cases of password rejection (too leak, too >short, contains leading or trailing whitespaces) were not tested >everywhere. Now they are! > >This patch makes things consistent everywhere and clean up some code. > >Signed-off-by: Marc Véron <veron@veron.ch> >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > Koha/AuthUtils.pm | 48 ++++++++++++++++++ > .../prog/en/modules/members/member-password.tt | 12 +++-- > .../prog/en/modules/members/memberentrygen.tt | 19 ++++++-- > .../bootstrap/en/modules/opac-memberentry.tt | 11 ++++- > .../opac-tmpl/bootstrap/en/modules/opac-passwd.tt | 16 +++--- > .../bootstrap/en/modules/opac-password-recovery.tt | 11 +++-- > members/member-password.pl | 14 ++++-- > members/memberentry.pl | 15 ++++-- > opac/opac-memberentry.pl | 18 ++++--- > opac/opac-passwd.pl | 57 +++++++++++----------- > opac/opac-password-recovery.pl | 57 ++++++++++------------ > t/AuthUtils.t | 49 ++++++++++++++++++- > 12 files changed, 230 insertions(+), 97 deletions(-) > >diff --git a/Koha/AuthUtils.pm b/Koha/AuthUtils.pm >index a8b391d0d5..a42705fd9b 100644 >--- a/Koha/AuthUtils.pm >+++ b/Koha/AuthUtils.pm >@@ -22,6 +22,9 @@ use Crypt::Eksblowfish::Bcrypt qw(bcrypt en_base64); > use Encode qw( encode is_utf8 ); > use Fcntl qw/O_RDONLY/; # O_RDONLY is used in generate_salt > use List::MoreUtils qw/ any /; >+use String::Random qw( random_string ); >+ >+use C4::Context; > > use base 'Exporter'; > >@@ -134,6 +137,51 @@ sub generate_salt { > return $string; > } > >+=head2 is_password_valid >+ >+my ( $is_valid, $error ) = is_password_valid( $password ); >+ >+return $is_valid == 1 if the password match minPasswordLength and RequireStrongPassword conditions >+otherwise return $is_valid == 0 and $error will contain the error ('too_short' or 'too_weak') >+ >+=cut >+ >+sub is_password_valid { >+ my ($password) = @_; >+ my $minPasswordLength = C4::Context->preference('minPasswordLength'); >+ $minPasswordLength = 3 if not $minPasswordLength or $minPasswordLength < 3; >+ if ( length($password) < $minPasswordLength ) { >+ return ( 0, 'too_short' ); >+ } >+ elsif ( C4::Context->preference('RequireStrongPassword') ) { >+ return ( 0, 'too_weak' ) >+ if $password !~ m|(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{$minPasswordLength,}|; >+ } >+ return ( 0, 'has_whitespaces' ) if $password =~ m[^\s|\s$]; >+ return ( 1, undef ); >+} >+ >+=head2 generate_password >+ >+my password = generate_password(); >+ >+Generate a password according to the minPasswordLength and RequireStrongPassword. >+ >+=cut >+ >+sub generate_password { >+ my $minPasswordLength = C4::Context->preference('minPasswordLength'); >+ $minPasswordLength = 8 if not $minPasswordLength or $minPasswordLength < 8; >+ >+ my ( $password, $is_valid ); >+ do { >+ $password = random_string('.' x $minPasswordLength ); >+ ( $is_valid, undef ) = is_password_valid( $password ); >+ } while not $is_valid; >+ return $password; >+} >+ >+ > =head2 get_script_name > > This returns the correct script name, for use in redirecting back to the correct page after showing >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 cf5e33d59e..18749ae06f 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 >@@ -93,9 +93,15 @@ > [% IF ( BADUSERID ) %] > <li>You have entered a username that already exists. Please choose another one.</li> > [% END %] >- [% IF ( SHORTPASSWORD ) %] >- <li><strong>The password entered is too short</strong>. Password must be at least [% minPasswordLength %] characters.</li> >- [% END %] >+ [% IF ( ERROR_password_too_short ) %] >+ <li id="ERROR_short_password">Password must be at least [% minPasswordLength %] characters long.</li> >+ [% END %] >+ [% IF ( ERROR_password_too_weak ) %] >+ <li id="ERROR_weak_password">Password must contain at least one digit, one lowercase and one uppercase.</li> >+ [% END %] >+ [% IF ( ERROR_password_has_whitespaces ) %] >+ <li id="ERROR_weak_password">Password must not contain leading or trailing whitespaces.</li> >+ [% END %] > [% IF ( NOPERMISSION ) %] > <li>You do not have permission to edit this patron's login information.</li> > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >index df84f9c982..6d8ddc0142 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >@@ -196,9 +196,15 @@ $(document).ready(function() { > [% IF ( ERROR_dateexpiry ) %] > <li id="ERROR_dateexpiry">Date of expiration is invalid.</li> > [% END %] >- [% IF ( ERROR_short_password ) %] >- <li id="ERROR_short_password">Password must be at least [% minPasswordLength %] characters long.</li> >- [% END %] >+ [% IF ( ERROR_password_too_short ) %] >+ <li id="ERROR_short_password">Password must be at least [% minPasswordLength %] characters long.</li> >+ [% END %] >+ [% IF ( ERROR_password_too_weak ) %] >+ <li id="ERROR_weak_password">Password must contain at least one digit, one lowercase and one uppercase.</li> >+ [% END %] >+ [% IF ( ERROR_password_has_whitespaces ) %] >+ <li id="ERROR_weak_password">Password must not contain leading or trailing whitespaces.</li> >+ [% END %] > [% IF ( ERROR_password_mismatch ) %] > <li id="ERROR_password_mismatch">Passwords do not match.</li> > [% END %] >@@ -910,8 +916,11 @@ $(document).ready(function() { > [% END %] > [% END %] > [% END %] >- [% IF ( mandatorypassword ) %]<span class="required">Required</span>[% END %][% IF ( ERROR_short_password ) %]<span class="required">Password is too short</span>[% END %] >- <div class="hint">Minimum password length: [% minPasswordLength %]</div> >+ [% IF ( mandatorypassword ) %]<span class="required">Required</span>[% END %] >+ [% IF ( ERROR_password_too_short ) %]<span class="required">Password is too short</span>[% END %] >+ [% IF ( ERROR_password_too_weak ) %]<span class="required">Password is too weak</span>[% END %] >+ [% IF ( ERROR_password_has_whitespaces ) %]<span class="required">Password has leading or trailing whitespaces</span>[% END %] >+ <div class="hint">Minimum password length: [% minPasswordLength %]</div> > </li> > <li> > [% IF ( mandatorypassword ) %] >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt >index 8730c47e5e..9181a205ab 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt >@@ -65,8 +65,15 @@ > [% IF field == "emailpro" %]<li>Contact information: <a href="#borrower_emailpro">secondary email address</a></li>[% END %] > [% IF field == "B_email" %]<li>Alternate address information: <a href="#borrower_B_email">email address</a></li>[% END %] > [% IF field == "password_match" %]<li>Passwords do not match! <a href="#password">password</a></li>[% END %] >- [% IF field == "password_invalid" %]<li>Password does not meet minimum requirements! <a href="#password">password</a></li>[% END %] >- [% IF field == "password_spaces" %]<li>Password contains leading and/or trailing spaces! <a href="#password">password</a></li>[% END %] >+ [% IF field == "password_too_short" %] >+ <li>Password must be at least [% minPasswordLength %] characters long.</li> >+ [% END %] >+ [% IF field == "password_too_weak" %] >+ <li>Password must contain at least one digit, one lowercase and one uppercase.</li> >+ [% END %] >+ [% IF field == "password_has_whitespaces" %] >+ <li>Password must not contain leading or trailing whitespaces.</li> >+ [% END %] > [% IF field == "duplicate_email" %] > <li>This email address already exists in our database.</li> > [% END %] >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt >index 307f71d728..cc04d35c60 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt >@@ -29,18 +29,22 @@ > <div class="alert"> > <h3>There was a problem with your submission</h3> > <p> >- [% IF ( PassMismatch ) %] >+ [% IF ( passwords_mismatch ) %] > Passwords do not match. Please re-type your new password. > [% END %] >- [% IF ( ShortPass ) %] >- Your new password must be at least [% minPasswordLength%] characters long. >+ [% IF password_too_short %] >+ Password must be at least [% minPasswordLength %] characters long. > [% END %] >+ [% IF password_too_weak %] >+ Password must contain at least one digit, one lowercase and one uppercase. >+ [% END %] >+ [% IF password_has_whitespaces %] >+ Password must not contain leading or trailing whitespaces. >+ [% END %] >+ > [% IF ( WrongPass ) %] > Your current password was entered incorrectly. If this problem persists, please ask a librarian to reset your password for you. > [% END %] >- [% IF PasswordContainsTrailingSpaces %] >- Your password contains leading and/or trailing spaces. >- [% END %] > </p> > </div> > [% END # /IF Error_messages %] >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt >index 4f3f8571d5..fd36ba4597 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt >@@ -63,9 +63,12 @@ > <br/>If you did not receive this email, you can request a new one: <a href="/cgi-bin/koha/opac-password-recovery.pl?resendEmail=true&email=[% email %]&username=[% username %]">Get new password recovery link</a> > [% ELSIF (errPassNotMatch) %] > The passwords do not match. >- [% ELSIF (errPassTooShort) %] >- Your chosen password is too short. >- <br/>The password must contain at least [% minPassLength %] characters. >+ [% ELSIF password_too_short %] >+ <li>Password must be at least [% minPasswordLength %] characters long.</li> >+ [% ELSIF password_too_weak %] >+ <li>Password must contain at least one digit, one lowercase and one uppercase.</li> >+ [% ELSIF password_has_whitespaces %] >+ <li>Password must not contain leading or trailing whitespaces.</li> > [% ELSIF (errLinkNotValid) %] > The link you clicked is either invalid, or expired. > <br/>Be sure you used the link from the email, or contact library staff for assistance. >@@ -95,7 +98,7 @@ > <form action="/cgi-bin/koha/opac-password-recovery.pl" method="post" autocomplete="off"> > <input type="hidden" name="koha_login_context" value="opac" /> > <fieldset> >- <div class="alert alert-info">The password must contain at least [% minPassLength %] characters.</div> >+ <div class="alert alert-info">The password must contain at least [% minPasswordLength %] characters.</div> > <label for="password">New password:</label> > <input type="password" id="password" size="40" name="password" /> > <label for="repeatPassword">Confirm new password:</label> >diff --git a/members/member-password.pl b/members/member-password.pl >index a3c36e5348..bb82bbc2f9 100755 >--- a/members/member-password.pl >+++ b/members/member-password.pl >@@ -15,6 +15,7 @@ use C4::Members; > use C4::Circulation; > use CGI qw ( -utf8 ); > use C4::Members::Attributes qw(GetBorrowerAttributes); >+use Koha::AuthUtils; > use Koha::Token; > > use Koha::Patrons; >@@ -66,11 +67,16 @@ if ( ( $member ne $loggedinuser ) && ( $category_type eq 'S' ) ) { > > push( @errors, 'NOMATCH' ) if ( ( $newpassword && $newpassword2 ) && ( $newpassword ne $newpassword2 ) ); > >-my $minpw = C4::Context->preference('minPasswordLength'); >-$minpw = 3 if not $minpw or $minpw < 3; >-push( @errors, 'SHORTPASSWORD' ) if ( $newpassword && $minpw && ( length($newpassword) < $minpw ) ); >+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 && !scalar(@errors) ) { >+if ( $newpassword and not @errors) { > > die "Wrong CSRF token" > unless Koha::Token->new->check_csrf({ >diff --git a/members/memberentry.pl b/members/memberentry.pl >index 8661a59fb0..ed35e1b92d 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -37,6 +37,7 @@ use C4::Koha; > use C4::Log; > use C4::Letters; > use C4::Form::MessagingPreferences; >+use Koha::AuthUtils; > use Koha::AuthorisedValues; > use Koha::Patron::Debarments; > use Koha::Cities; >@@ -353,13 +354,19 @@ if ($op eq 'save' || $op eq 'insert'){ > unless (Check_Userid($userid,$borrowernumber)) { > push @errors, "ERROR_login_exist"; > } >- >+ > my $password = $input->param('password'); > my $password2 = $input->param('password2'); > push @errors, "ERROR_password_mismatch" if ( $password ne $password2 ); >- my $minpw = C4::Context->preference('minPasswordLength'); >- $minpw = 3 if not $minpw or $minpw < 3; >- push @errors, "ERROR_short_password" if( $password && $minpw && $password ne '****' && (length($password) < $minpw) ); >+ >+ if ( $password and $password ne '****' ) { >+ my ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password ); >+ 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'; >+ } >+ } > > # Validate emails > my $emailprimary = $input->param('email'); >diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl >index f7d08b65c9..33988c113f 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -28,6 +28,10 @@ use C4::Output; > use C4::Members; > use C4::Members::Attributes qw( GetBorrowerAttributes ); > use C4::Form::MessagingPreferences; >+use Koha::AuthUtils; >+use Koha::Patrons; >+use Koha::Patron::Modification; >+use Koha::Patron::Modifications; > use C4::Scrubber; > use Email::Valid; > use Koha::DateUtils; >@@ -165,7 +169,7 @@ if ( $action eq 'create' ) { > $verification_token = md5_hex( time().{}.rand().{}.$$ ); > } > >- $borrower{password} = random_string(".........."); >+ $borrower{password} = Koha::AuthUtils::generate_password; > $borrower{verification_token} = $verification_token; > > Koha::Patron::Modification->new( \%borrower )->store(); >@@ -386,8 +390,6 @@ sub CheckMandatoryFields { > } > > sub CheckForInvalidFields { >- my $minpw = C4::Context->preference('minPasswordLength'); >- $minpw = 3 if not $minpw or $minpw < 3; > my $borrower = shift; > my @invalidFields; > if ($borrower->{'email'}) { >@@ -421,11 +423,13 @@ sub CheckForInvalidFields { > { > push( @invalidFields, "password_match" ); > } >- if ( $borrower->{'password'} && $minpw && (length($borrower->{'password'}) < $minpw) ) { >- push(@invalidFields, "password_invalid"); >- } > if ( $borrower->{'password'} ) { >- push(@invalidFields, "password_spaces") if ($borrower->{'password'} =~ /^\s/ or $borrower->{'password'} =~ /\s$/); >+ my ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $borrower->{password} ); >+ unless ( $is_valid ) { >+ push @invalidFields, 'password_too_short' if $error eq 'too_short'; >+ push @invalidFields, 'password_too_weak' if $error eq 'too_weak'; >+ push @invalidFields, 'password_has_whitespaces' if $error eq 'has_whitespaces'; >+ } > } > > return \@invalidFields; >diff --git a/opac/opac-passwd.pl b/opac/opac-passwd.pl >index bef1f99aae..b5ae895947 100755 >--- a/opac/opac-passwd.pl >+++ b/opac/opac-passwd.pl >@@ -46,47 +46,46 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( > ); > > my $patron = Koha::Patrons->find( $borrowernumber ); >-my $minpasslen = C4::Context->preference("minPasswordLength"); >-$minpasslen = 3 if not $minpasslen or $minpasslen < 3; > if ( C4::Context->preference("OpacPasswordChange") ) { > my $sth = $dbh->prepare("UPDATE borrowers SET password = ? WHERE borrowernumber=?"); > if ( $query->param('Oldkey') > && $query->param('Newkey') > && $query->param('Confirm') ) > { >+ my $error; >+ my $new_password = $query->param('Newkey'); >+ my $confirm_password = $query->param('Confirm'); > if ( goodkey( $dbh, $borrowernumber, $query->param('Oldkey') ) ) { >- if ( $query->param('Newkey') =~ m|^\s+| or $query->param('Newkey') =~ m|\s+$| ) { >- $template->param( >- Error_messages => 1, >- PasswordContainsTrailingSpaces => 1, >- ); >- } >- elsif ( $query->param('Newkey') eq $query->param('Confirm') >- && length( $query->param('Confirm') ) >= $minpasslen ) >- { # Record password >- my $clave = hash_password( $query->param('Newkey') ); >- $sth->execute( $clave, $borrowernumber ); >- $template->param( 'password_updated' => '1' ); >- $template->param( 'borrowernumber' => $borrowernumber ); >- } >- elsif ( $query->param('Newkey') ne $query->param('Confirm') ) { >- $template->param( 'Ask_data' => '1' ); >- $template->param( 'Error_messages' => '1' ); >- $template->param( 'PassMismatch' => '1' ); >- } >- elsif ( length( $query->param('Confirm') ) < $minpasslen ) { >+ >+ if ( $new_password ne $confirm_password ) { > $template->param( 'Ask_data' => '1' ); > $template->param( 'Error_messages' => '1' ); >- $template->param( 'ShortPass' => '1' ); >- } >- else { >- $template->param( 'Error_messages' => '1' ); >+ $template->param( 'passwords_mismatch' => '1' ); >+ } else { >+ my ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $new_password ); >+ unless ( $is_valid ) { >+ $error = 'password_too_short' if $error eq 'too_short'; >+ $error = 'password_too_weak' if $error eq 'too_weak'; >+ $error = 'password_has_whitespaces' if $error eq 'has_whitespaces'; >+ } else { >+ # Password is valid and match >+ my $clave = hash_password( $new_password ); >+ $sth->execute( $clave, $borrowernumber ); >+ $template->param( 'password_updated' => '1' ); >+ $template->param( 'borrowernumber' => $borrowernumber ); >+ } > } > } > else { >- $template->param( 'Ask_data' => '1' ); >- $template->param( 'Error_messages' => '1' ); >- $template->param( 'WrongPass' => '1' ); >+ $error = 'WrongPass'; >+ } >+ if ($error) { >+ $template->param( >+ Ask_data => 1, >+ Error_messages => 1, >+ $error => 1, >+ ); >+ > } > } > else { >diff --git a/opac/opac-password-recovery.pl b/opac/opac-password-recovery.pl >index 1a4abf7181..d08d101475 100755 >--- a/opac/opac-password-recovery.pl >+++ b/opac/opac-password-recovery.pl >@@ -46,8 +46,6 @@ my $errBadEmail; > > #new password form error > my $errLinkNotValid; >-my $errPassNotMatch; >-my $errPassTooShort; > > if ( $query->param('sendEmail') || $query->param('resendEmail') ) { > >@@ -144,38 +142,33 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { > elsif ( $query->param('passwordReset') ) { > ( $borrower_number, $username ) = GetValidLinkInfo($uniqueKey); > >- my $minPassLength = C4::Context->preference('minPasswordLength'); >- $minPassLength = 3 if not $minPassLength or $minPassLength < 3; >- #validate password length & match >- if ( ($borrower_number) >- && ( $password eq $repeatPassword ) >- && ( length($password) >= $minPassLength ) ) >- { #apply changes >- Koha::Patrons->find($borrower_number)->update_password( $username, hash_password($password) ); >- CompletePasswordRecovery($uniqueKey); >- $template->param( >- password_reset_done => 1, >- username => $username >- ); >- } >- else { #errors >- if ( !$borrower_number ) { #parameters not valid >- $errLinkNotValid = 1; >- } >- elsif ( $password ne $repeatPassword ) { #passwords does not match >- $errPassNotMatch = 1; >- } >- elsif ( length($password) < $minPassLength ) { #password too short >- $errPassTooShort = 1; >+ my $error; >+ if ( not $borrower_number ) { >+ $error = 'errLinkNotValid'; >+ } elsif ( $password ne $repeatPassword ) { >+ $error = 'errPassNotMatch'; >+ } else { >+ my ( $is_valid, $err) = Koha::AuthUtils::is_password_valid( $password ); >+ unless ( $is_valid ) { >+ $error = 'password_too_short' if $err eq 'too_short'; >+ $error = 'password_too_weak' if $err eq 'too_weak'; >+ $error = 'password_has_whitespaces' if $err eq 'has_whitespaces'; >+ } else { >+ Koha::Patrons->find($borrower_number)->update_password( $username, hash_password($password) ); >+ CompletePasswordRecovery($uniqueKey); >+ $template->param( >+ password_reset_done => 1, >+ username => $username >+ ); > } >+ } >+ if ( $error ) { > $template->param( >- new_password => 1, >- email => $email, >- uniqueKey => $uniqueKey, >- errLinkNotValid => $errLinkNotValid, >- errPassNotMatch => $errPassNotMatch, >- errPassTooShort => $errPassTooShort, >- hasError => 1 >+ new_password => 1, >+ email => $email, >+ uniqueKey => $uniqueKey, >+ hasError => 1, >+ $error => 1, > ); > } > } >diff --git a/t/AuthUtils.t b/t/AuthUtils.t >index f0fa81bd46..aa73e1651c 100644 >--- a/t/AuthUtils.t >+++ b/t/AuthUtils.t >@@ -1,6 +1,7 @@ > # This file is part of Koha. > # > # Copyright (C) 2013 Equinox Software, Inc. >+# Copyright 2017 Koha Development Team > # > # Koha is free software; you can redistribute it and/or modify it > # under the terms of the GNU General Public License as published by >@@ -16,11 +17,57 @@ > # along with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; >-use Test::More tests => 1; >+use Test::More tests => 3; > >+use t::lib::Mocks; > use Koha::AuthUtils qw/hash_password/; > > my $hash1 = hash_password('password'); > my $hash2 = hash_password('password'); > > ok($hash1 ne $hash2, 'random salts used when generating password hash'); >+ >+subtest 'is_password_valid' => sub { >+ plan tests => 12; >+ >+ my ( $is_valid, $error ); >+ >+ t::lib::Mocks::mock_preference('RequireStrongPassword', 0); >+ t::lib::Mocks::mock_preference('minPasswordLength', 0); >+ ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( '12' ); >+ is( $is_valid, 0, 'min password size should be 3' ); >+ is( $error, 'too_short', 'min password size should be 3' ); >+ ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( ' 123' ); >+ is( $is_valid, 0, 'password should not contain leading spaces' ); >+ is( $error, 'has_whitespaces', 'password should not contain leading spaces' ); >+ ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( '123 ' ); >+ is( $is_valid, 0, 'password should not contain trailing spaces' ); >+ is( $error, 'has_whitespaces', 'password should not contain trailing spaces' ); >+ ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( '123' ); >+ is( $is_valid, 1, 'min password size should be 3' ); >+ >+ t::lib::Mocks::mock_preference('RequireStrongPassword', 1); >+ t::lib::Mocks::mock_preference('minPasswordLength', 8); >+ ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( '12345678' ); >+ is( $is_valid, 0, 'password should be strong' ); >+ is( $error, 'too_weak', 'password should be strong' ); >+ ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( 'abcd1234' ); >+ is( $is_valid, 0, 'strong password should contain uppercase' ); >+ is( $error, 'too_weak', 'strong password should contain uppercase' ); >+ >+ ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( 'abcD1234' ); >+ is( $is_valid, 1, 'strong password should contain uppercase' ); >+}; >+ >+subtest 'generate_password' => sub { >+ plan tests => 1; >+ t::lib::Mocks::mock_preference('RequireStrongPassword', 1); >+ t::lib::Mocks::mock_preference('minPasswordLength', 8); >+ my $all_valid = 1; >+ for ( 1 .. 10 ) { >+ my $password = Koha::AuthUtils::generate_password; >+ my ( $is_valid, undef ) = Koha::AuthUtils::is_password_valid( $password ); >+ $all_valid = 0 unless $is_valid; >+ } >+ is ( $all_valid, 1, 'generate_password should generate valid passwords' ); >+}; >-- >2.14.2
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 18298
:
61227
|
61228
|
61229
|
61230
|
61231
|
61232
|
61237
|
61331
|
61332
|
61333
|
61334
|
61335
|
61336
|
62976
|
62977
|
62978
|
62979
|
62980
|
62981
|
66621
|
66622
|
66623
|
66624
|
66625
|
66626
|
68096
|
68097
|
68098
|
68100
|
68101
|
68102
|
68103
|
68104
| 68105 |
68106
|
68220