@@ -, +, @@
* reverts changes on misc/admin/set_password.pl
* makes category param mandatory for AuthUtils::is_valid_password and AuthUtils::generate_password
* changes onboarding.pl to set category param in AuthUtils::is_valid_password
* Completes t/db_dependent/AuthUtils.t and drops t/AuthUtils.t
* Removes offending and replaces it by
---
Koha/AuthUtils.pm | 13 +-
Koha/Exceptions/Password.pm | 4 +
installer/onboarding.pl | 12 +-
.../prog/en/modules/admin/categories.tt | 2 +-
misc/admin/set_password.pl | 13 +-
t/AuthUtils.t | 73 ---------
t/db_dependent/AuthUtils.t | 140 ++++++++++++------
7 files changed, 123 insertions(+), 134 deletions(-)
delete mode 100644 t/AuthUtils.t
--- a/Koha/AuthUtils.pm
+++ a/Koha/AuthUtils.pm
@@ -23,6 +23,7 @@ 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 Koha::Exceptions::Password;
use C4::Context;
@@ -148,12 +149,15 @@ otherwise return $is_valid == 0 and $error will contain the error ('too_short' o
sub is_password_valid {
my ($password, $category) = @_;
- my $minPasswordLength = $category?$category->effective_min_password_length:C4::Context->preference('minPasswordLength');
+ if(!$category) {
+ Koha::Exceptions::Password::NoCategoryProvided->throw();
+ }
+ my $minPasswordLength = $category->effective_min_password_length;
$minPasswordLength = 3 if not $minPasswordLength or $minPasswordLength < 3;
if ( length($password) < $minPasswordLength ) {
return ( 0, 'too_short' );
}
- elsif ( $category?$category->effective_require_strong_password:C4::Context->preference('RequireStrongPassword') ) {
+ elsif ( $category->effective_require_strong_password ) {
return ( 0, 'too_weak' )
if $password !~ m|(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{$minPasswordLength,}|;
}
@@ -171,7 +175,10 @@ Generate a password according to category's minimum password length and strength
sub generate_password {
my ($category) = @_;
- my $minPasswordLength = $category?$category->effective_min_password_length:C4::Context->preference('minPasswordLength');
+ if(!$category) {
+ Koha::Exceptions::Password::NoCategoryProvided->throw();
+ }
+ my $minPasswordLength = $category->effective_min_password_length;
$minPasswordLength = 8 if not $minPasswordLength or $minPasswordLength < 8;
my ( $password, $is_valid );
--- a/Koha/Exceptions/Password.pm
+++ a/Koha/Exceptions/Password.pm
@@ -43,6 +43,10 @@ use Exception::Class (
isa => 'Koha::Exceptions::Password',
description => 'The password was rejected by a plugin'
},
+ 'Koha::Exceptions::Password::NoCategoryProvided' => {
+ isa => 'Koha::Exceptions::Password',
+ description => 'You must provide a patron\'s category to validate password\'s strength and length'
+ }
);
sub full_message {
--- a/installer/onboarding.pl
+++ a/installer/onboarding.pl
@@ -144,8 +144,14 @@ if ( $step == 3 ) {
my $secondpassword = $input->param('password2') || '';
my $cardnumber = $input->param('cardnumber');
my $userid = $input->param('userid');
+ my $categorycode = $input->param('categorycode_entry');
+ my $patron_category =
+ Koha::Patron::Categories->find( $categorycode );
+
+ my ( $is_valid, $passworderror ) =
+ Koha::AuthUtils::is_password_valid( $firstpassword,
+ $patron_category );
- my ( $is_valid, $passworderror) = Koha::AuthUtils::is_password_valid( $firstpassword );
if ( my $error_code = checkcardnumber($cardnumber) ) {
if ( $error_code == 1 ) {
@@ -171,7 +177,7 @@ if ( $step == 3 ) {
firstname => scalar $input->param('firstname'),
cardnumber => scalar $input->param('cardnumber'),
branchcode => scalar $input->param('libraries'),
- categorycode => scalar $input->param('categorycode_entry'),
+ categorycode => $categorycode,
userid => scalar $input->param('userid'),
privacy => "default",
address => "",
@@ -179,8 +185,6 @@ if ( $step == 3 ) {
flags => 1, # Will be superlibrarian
};
- my $patron_category =
- Koha::Patron::Categories->find( $patron_data->{categorycode} );
$patron_data->{dateexpiry} =
$patron_category->get_expiry_date( $patron_data->{dateenrolled} );
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt
+++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt
@@ -231,7 +231,7 @@
-
+
Leave blank to use system default ([% Koha.Preference('minPasswordLength') | html %])
--- a/misc/admin/set_password.pl
+++ a/misc/admin/set_password.pl
@@ -44,6 +44,11 @@ unless ( $userid or $patron_id or $cardnumber ) {
pod2usage("cardnumber is mandatory") unless $cardnumber;
}
+unless ($password) {
+ my $generator = String::Random->new( rand_gen => \&alt_rand );
+ $password = $generator->randregex('[A-Za-z][A-Za-z0-9_]{6}.[A-Za-z][A-Za-z0-9_]{6}\d');
+}
+
my $filter;
if ( $userid ) {
@@ -65,14 +70,6 @@ unless ( $patrons->count > 0 ) {
}
my $patron = $patrons->next;
-
-unless ($password) {
- my $generator = String::Random->new( rand_gen => \&alt_rand );
- my $n = $patron->category->effective_min_password_length;
- $n = $n<6?6:$n;
- $password = $generator->randregex('[A-Za-z][A-Za-z0-9_]{6}.[A-Za-z][A-Za-z0-9_]{'.$n.'}\d');
-}
-
$patron->set_password({ password => $password, skip_validation => 1 });
print $patron->userid . " " . $password . "\n";
--- a/t/AuthUtils.t
+++ a/t/AuthUtils.t
@@ -1,73 +0,0 @@
-# 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
-# the Free Software Foundation; either version 3 of the License, or
-# (at your option) any later version.
-#
-# Koha is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Koha; if not, see .
-
-use Modern::Perl;
-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' );
-};
--- a/t/db_dependent/AuthUtils.t
+++ a/t/db_dependent/AuthUtils.t
@@ -18,6 +18,7 @@
use Modern::Perl;
use Test::More tests => 2;
+use Test::Exception;
use Test::MockModule;
use t::lib::Mocks;
use t::lib::TestBuilder;
@@ -28,87 +29,136 @@ my $builder = t::lib::TestBuilder->new;
$schema->storage->txn_begin;
-my $category1 = $builder->build_object({class=>'Koha::Patron::Categories', value=>{min_password_length => 15, require_strong_password => 1}});
-my $category2 = $builder->build_object({class=>'Koha::Patron::Categories', value=>{min_password_length => 5, require_strong_password => undef}});
-my $category3 = $builder->build_object({class=>'Koha::Patron::Categories', value=>{min_password_length => undef, require_strong_password => 1}});
-my $category4 = $builder->build_object({class=>'Koha::Patron::Categories', value=>{min_password_length => undef, require_strong_password => undef}});
-
-my $p_3l_weak = '123';
-my $p_3l_strong = '1Ab';
-my $p_5l_weak = 'abcde';
-my $p_15l_weak = '0123456789abcdf';
-my $p_5l_strong = 'Abc12';
+my $category1 = $builder->build_object(
+ {
+ class => 'Koha::Patron::Categories',
+ value => { min_password_length => 15, require_strong_password => 1 }
+ }
+);
+my $category2 = $builder->build_object(
+ {
+ class => 'Koha::Patron::Categories',
+ value => { min_password_length => 5, require_strong_password => undef }
+ }
+);
+my $category3 = $builder->build_object(
+ {
+ class => 'Koha::Patron::Categories',
+ value => { min_password_length => undef, require_strong_password => 1 }
+ }
+);
+my $category4 = $builder->build_object(
+ {
+ class => 'Koha::Patron::Categories',
+ value =>
+ { min_password_length => undef, require_strong_password => undef }
+ }
+);
+
+my $p_2l = '1A';
+my $p_3l_weak = '123';
+my $p_3l_strong = '1Ab';
+my $p_5l_weak = 'abcde';
+my $p_15l_weak = '0123456789abcdf';
+my $p_5l_strong = 'Abc12';
my $p_15l_strong = '0123456789AbCdF';
subtest 'is_password_valid for category' => sub {
- plan tests => 12;
+ plan tests => 15;
my ( $is_valid, $error );
- t::lib::Mocks::mock_preference('RequireStrongPassword', 0);
- t::lib::Mocks::mock_preference('minPasswordLength', 3);
+ t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 );
+ t::lib::Mocks::mock_preference( 'minPasswordLength', 3 );
#Category 1 - override=>1, length=>15, strong=>1
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $p_5l_strong, $category1 );
- is($is_valid, 0, 'min password length for this category is 15');
- is($error, 'too_short', 'min password length for this category is 15');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $p_5l_strong, $category1 );
+ is( $is_valid, 0, 'min password length for this category is 15' );
+ is( $error, 'too_short', 'min password length for this category is 15' );
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $p_15l_weak, $category1 );
- is($is_valid, 0, 'password should be strong for this category');
- is($error, 'too_weak', 'password should be strong for this category');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $p_15l_weak, $category1 );
+ is( $is_valid, 0, 'password should be strong for this category' );
+ is( $error, 'too_weak', 'password should be strong for this category' );
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $p_15l_strong, $category1 );
- is($is_valid, 1, 'password should be ok for this category');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $p_15l_strong, $category1 );
+ is( $is_valid, 1, 'password should be ok for this category' );
#Category 2 - override=>1, length=>5, strong=>0
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $p_3l_strong, $category2 );
- is($is_valid, 0, 'min password length for this category is 5');
- is($error, 'too_short', 'min password length for this category is 5');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $p_3l_strong, $category2 );
+ is( $is_valid, 0, 'min password length for this category is 5' );
+ is( $error, 'too_short', 'min password length for this category is 5' );
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $p_5l_weak, $category2 );
- is($is_valid, 1, 'password should be ok for this category');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $p_5l_weak, $category2 );
+ is( $is_valid, 1, 'password should be ok for this category' );
#Category 3 - override=>0, length=>20, strong=>0
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $p_3l_weak, $category3 );
- is($is_valid, 0, 'password should be strong');
- is($error, 'too_weak', 'password should be strong');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $p_3l_weak, $category3 );
+ is( $is_valid, 0, 'password should be strong' );
+ is( $error, 'too_weak', 'password should be strong' );
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $p_3l_strong, $category3 );
- is($is_valid, 1, 'password should be ok');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $p_3l_strong, $category3 );
+ is( $is_valid, 1, 'password should be ok' );
#Category 4 - default settings - override=>undef, length=>undef, strong=>undef
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $p_3l_weak, $category4 );
- is($is_valid, 1, 'password should be ok');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $p_3l_weak, $category4 );
+ is( $is_valid, 1, 'password should be ok' );
+
+ t::lib::Mocks::mock_preference( 'minPasswordLength', 0 );
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $p_2l, $category4 );
+ is( $is_valid, 0, '3 is absolute minimum password' );
+ is( $error, 'too_short', '3 is absolute minimum password' );
+
+ throws_ok { Koha::AuthUtils::is_password_valid($p_2l); }
+ 'Koha::Exceptions::Password::NoCategoryProvided',
+ 'Category should always be provided';
+
};
subtest 'generate_password for category' => sub {
- plan tests => 4;
+ plan tests => 5;
my ( $is_valid, $error );
- t::lib::Mocks::mock_preference('RequireStrongPassword', 0);
- t::lib::Mocks::mock_preference('minPasswordLength', 3);
+ t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 );
+ t::lib::Mocks::mock_preference( 'minPasswordLength', 3 );
#Category 4
my $password = Koha::AuthUtils::generate_password($category4);
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password, $category4 );
- is($is_valid, 1, 'password should be ok');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $password, $category4 );
+ is( $is_valid, 1, 'password should be ok' );
#Category 3
$password = Koha::AuthUtils::generate_password($category3);
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password, $category3 );
- is($is_valid, 1, 'password should be ok');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $password, $category3 );
+ is( $is_valid, 1, 'password should be ok' );
#Category 2
$password = Koha::AuthUtils::generate_password($category2);
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password, $category2 );
- is($is_valid, 1, 'password should be ok');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $password, $category2 );
+ is( $is_valid, 1, 'password should be ok' );
#Category 1
$password = Koha::AuthUtils::generate_password($category1);
- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password, $category1 );
- is($is_valid, 1, 'password should be ok');
+ ( $is_valid, $error ) =
+ Koha::AuthUtils::is_password_valid( $password, $category1 );
+ is( $is_valid, 1, 'password should be ok' );
+
+ throws_ok { Koha::AuthUtils::generate_password(); }
+ 'Koha::Exceptions::Password::NoCategoryProvided',
+ 'Category should always be provided';
};
-$schema->storage->txn_rollback;
+$schema->storage->txn_rollback;
--