Browse Source

Bug 23816: (follow-up) Fix many things

This patch:
 * 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 <input type="number"/> and replaces it by <input type="text" inputmode="numeric" pattern="[0-9]*"/>

https://bugs.koha-community.org/show_bug.cgi?id=23826

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
20.11.x
Agustin Moyano 3 years ago
committed by Jonathan Druart
parent
commit
52deed335e
  1. 13
      Koha/AuthUtils.pm
  2. 4
      Koha/Exceptions/Password.pm
  3. 12
      installer/onboarding.pl
  4. 2
      koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt
  5. 13
      misc/admin/set_password.pl
  6. 73
      t/AuthUtils.t
  7. 140
      t/db_dependent/AuthUtils.t

13
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 );

4
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 {

12
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} );

2
koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt

@ -231,7 +231,7 @@
</li>
<li>
<label for="min_password_length">Minimum password length:</label>
<input id="min_password_length" type="number" name="min_password_length" value="[% category.min_password_length | html %]"/>
<input id="min_password_length" type="text" inputmode="numeric" pattern="[0-9]*" name="min_password_length" value="[% category.min_password_length | html %]"/>
<span>Leave blank to use system default ([% Koha.Preference('minPasswordLength') | html %])</span>
</li>
<li class="pwd_setting_wrapper">

13
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";

73
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 <http://www.gnu.org/licenses>.
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' );
};

140
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;

Loading…
Cancel
Save