@@ -, +, @@ --- 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 | 15 +++--- opac/opac-passwd.pl | 57 +++++++++++----------- opac/opac-password-recovery.pl | 57 ++++++++++------------ t/AuthUtils.t | 49 ++++++++++++++++++- 12 files changed, 227 insertions(+), 97 deletions(-) --- a/Koha/AuthUtils.pm +++ a/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 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt @@ -93,9 +93,15 @@ [% IF ( BADUSERID ) %]
  • You have entered a username that already exists. Please choose another one.
  • [% END %] - [% IF ( SHORTPASSWORD ) %] -
  • The password entered is too short. Password must be at least [% minPasswordLength %] characters.
  • - [% END %] + [% IF ( ERROR_password_too_short ) %] +
  • Password must be at least [% minPasswordLength %] characters long.
  • + [% END %] + [% IF ( ERROR_password_too_weak ) %] +
  • Password must contain at least one digit, one lowercase and one uppercase.
  • + [% END %] + [% IF ( ERROR_password_has_whitespaces ) %] +
  • Password must not contain leading or trailing whitespaces.
  • + [% END %] [% IF ( NOPERMISSION ) %]
  • You do not have permission to edit this patron's login information.
  • [% END %] --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -198,9 +198,15 @@ $(document).ready(function() { [% IF ( ERROR_dateexpiry ) %]
  • Date of expiration is invalid.
  • [% END %] - [% IF ( ERROR_short_password ) %] -
  • Password must be at least [% minPasswordLength %] characters long.
  • - [% END %] + [% IF ( ERROR_password_too_short ) %] +
  • Password must be at least [% minPasswordLength %] characters long.
  • + [% END %] + [% IF ( ERROR_password_too_weak ) %] +
  • Password must contain at least one digit, one lowercase and one uppercase.
  • + [% END %] + [% IF ( ERROR_password_has_whitespaces ) %] +
  • Password must not contain leading or trailing whitespaces.
  • + [% END %] [% IF ( ERROR_password_mismatch ) %]
  • Passwords do not match.
  • [% END %] @@ -887,8 +893,11 @@ $(document).ready(function() { [% END %] [% END %] [% END %] - [% IF ( mandatorypassword ) %]Required[% END %][% IF ( ERROR_short_password ) %]Password is too short[% END %] -
    Minimum password length: [% minPasswordLength %]
    + [% IF ( mandatorypassword ) %]Required[% END %] + [% IF ( ERROR_password_too_short ) %]Password is too short[% END %] + [% IF ( ERROR_password_too_weak ) %]Password is too weak[% END %] + [% IF ( ERROR_password_has_whitespaces ) %]Password has leading or trailing whitespaces[% END %] +
    Minimum password length: [% minPasswordLength %]
  • [% IF ( mandatorypassword ) %] --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt +++ a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt @@ -63,8 +63,15 @@ [% IF field == "emailpro" %]
  • Contact information: secondary email address
  • [% END %] [% IF field == "B_email" %]
  • Alternate address information: email address
  • [% END %] [% IF field == "password_match" %]
  • Passwords do not match! password
  • [% END %] - [% IF field == "password_invalid" %]
  • Password does not meet minimum requirements! password
  • [% END %] - [% IF field == "password_spaces" %]
  • Password contains leading and/or trailing spaces! password
  • [% END %] + [% IF field == "password_too_short" %] +
  • Password must be at least [% minPasswordLength %] characters long.
  • + [% END %] + [% IF field == "password_too_weak" %] +
  • Password must contain at least one digit, one lowercase and one uppercase.
  • + [% END %] + [% IF field == "password_has_whitespaces" %] +
  • Password must not contain leading or trailing whitespaces.
  • + [% END %] [% IF field == "duplicate_email" %]
  • This email address already exists in our database.
  • [% END %] --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt +++ a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt @@ -29,18 +29,22 @@

    There was a problem with your submission

    - [% 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 %]

    [% END # /IF Error_messages %] --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt +++ a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt @@ -60,9 +60,12 @@
    If you did not receive this email, you can request a new one: Get new password recovery link [% ELSIF (errPassNotMatch) %] The passwords do not match. - [% ELSIF (errPassTooShort) %] - Your chosen password is too short. -
    The password must contain at least [% minPassLength %] characters. + [% ELSIF password_too_short %] +
  • Password must be at least [% minPasswordLength %] characters long.
  • + [% ELSIF password_too_weak %] +
  • Password must contain at least one digit, one lowercase and one uppercase.
  • + [% ELSIF password_has_whitespaces %] +
  • Password must not contain leading or trailing whitespaces.
  • [% ELSIF (errLinkNotValid) %] The link you clicked is either invalid, or expired.
    Be sure you used the link from the email, or contact library staff for assistance. @@ -92,7 +95,7 @@
    -
    The password must contain at least [% minPassLength %] characters.
    +
    The password must contain at least [% minPasswordLength %] characters.
    --- a/members/member-password.pl +++ a/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::Patron::Images; use Koha::Token; @@ -62,11 +63,16 @@ if ( ( $member ne $loggedinuser ) && ( $bor->{'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({ --- a/members/memberentry.pl +++ a/members/memberentry.pl @@ -39,6 +39,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; @@ -343,13 +344,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'); --- a/opac/opac-memberentry.pl +++ a/opac/opac-memberentry.pl @@ -26,6 +26,7 @@ use C4::Auth; use C4::Output; use C4::Members; use C4::Form::MessagingPreferences; +use Koha::AuthUtils; use Koha::Patrons; use Koha::Patron::Modification; use Koha::Patron::Modifications; @@ -144,7 +145,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(); @@ -365,8 +366,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'}) { @@ -388,11 +387,13 @@ sub CheckForInvalidFields { if ( $borrower->{'password'} ne $borrower->{'password2'} ){ 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; --- a/opac/opac-passwd.pl +++ a/opac/opac-passwd.pl @@ -45,47 +45,46 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( ); my $borr = C4::Members::GetMember( borrowernumber => $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 { --- a/opac/opac-password-recovery.pl +++ a/opac/opac-password-recovery.pl @@ -45,8 +45,6 @@ my $errBadEmail; #new password form error my $errLinkNotValid; -my $errPassNotMatch; -my $errPassTooShort; if ( $query->param('sendEmail') || $query->param('resendEmail') ) { @@ -132,38 +130,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, ); } } --- a/t/AuthUtils.t +++ a/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 . 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' ); +}; --