Bugzilla – Attachment 103617 Details for
Bug 18308
Default value of minPasswordLength should be increased
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18308: Increase minPasswordLength to 8 characters
Bug-18308-Increase-minPasswordLength-to-8-characte.patch (text/plain), 15.77 KB, created by
Aleisha Amohia
on 2020-04-23 23:57:02 UTC
(
hide
)
Description:
Bug 18308: Increase minPasswordLength to 8 characters
Filename:
MIME Type:
Creator:
Aleisha Amohia
Created:
2020-04-23 23:57:02 UTC
Size:
15.77 KB
patch
obsolete
>From b3af499e037f0aaa958798d91935852a040da178 Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >Date: Tue, 21 Apr 2020 08:29:19 +0000 >Subject: [PATCH] Bug 18308: Increase minPasswordLength to 8 characters > >for new installs > >To test: >1) Go to Administration -> system preferences -> ensure minPasswordLength syspref is set to 3, or some other number less than 8 >2) Go to Patrons -> Add a new patron >3) Notice the alert at the top of the page warning that >minPasswordLength is too low. Confirm you are still be able to save your >patron with a password that is less than 8 characters. >4) Edit the minPasswordLength syspref to be 8 or higher. >5) Go back to Patrons -> Add a new patron. Confirm the alert is gone. >6) Drop and recreate database to trigger onboarding tool. >7) Go through onboarding tool until you are required to create the >superlibrarian patron. Try to enter a password of less than 8 >characters. Confirm it does not let you continue until you enter a >password that is 8 characters or longer. > >Sponsored-by: Catalyst IT >--- > C4/Auth.pm | 2 +- > C4/InstallAuth.pm | 2 +- > Koha/AuthUtils.pm | 2 +- > Koha/Patron.pm | 2 +- > installer/data/mysql/sysprefs.sql | 2 +- > .../prog/en/modules/members/memberentrygen.tt | 6 ++++++ > t/AuthUtils.t | 22 ++++++++++---------- > t/db_dependent/Koha/Patrons.t | 24 +++++++++++----------- > t/db_dependent/api/v1/patrons_password.t | 16 +++++++-------- > 9 files changed, 42 insertions(+), 36 deletions(-) > >diff --git a/C4/Auth.pm b/C4/Auth.pm >index 1f79ef6187b..bbc644ae185 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -455,7 +455,7 @@ sub get_template_and_user { > my $using_https = ( defined $https and $https ne 'OFF' ) ? 1 : 0; > > my $minPasswordLength = C4::Context->preference('minPasswordLength'); >- $minPasswordLength = 3 if not $minPasswordLength or $minPasswordLength < 3; >+ $minPasswordLength = 8 if not $minPasswordLength or $minPasswordLength < 8; > $template->param( > "BiblioDefaultView" . C4::Context->preference("BiblioDefaultView") => 1, > EnhancedMessagingPreferences => C4::Context->preference('EnhancedMessagingPreferences'), >diff --git a/C4/InstallAuth.pm b/C4/InstallAuth.pm >index 987045df504..e2f59d2a583 100644 >--- a/C4/InstallAuth.pm >+++ b/C4/InstallAuth.pm >@@ -151,7 +151,7 @@ sub get_template_and_user { > } > > my $minPasswordLength = C4::Context->preference('minPasswordLength'); >- $minPasswordLength = 3 if not $minPasswordLength or $minPasswordLength < 3; >+ $minPasswordLength = 8 if not $minPasswordLength or $minPasswordLength < 8; > $template->param(minPasswordLength => $minPasswordLength,); > } > return ( $template, $borrowernumber, $cookie ); >diff --git a/Koha/AuthUtils.pm b/Koha/AuthUtils.pm >index 7c4dcd1002b..e961b110537 100644 >--- a/Koha/AuthUtils.pm >+++ b/Koha/AuthUtils.pm >@@ -149,7 +149,7 @@ otherwise return $is_valid == 0 and $error will contain the error ('too_short' o > sub is_password_valid { > my ($password) = @_; > my $minPasswordLength = C4::Context->preference('minPasswordLength'); >- $minPasswordLength = 3 if not $minPasswordLength or $minPasswordLength < 3; >+ $minPasswordLength = 8 if not $minPasswordLength or $minPasswordLength < 8; > if ( length($password) < $minPasswordLength ) { > return ( 0, 'too_short' ); > } >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index fe3e80aebb5..ad3231215f5 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -736,7 +736,7 @@ sub set_password { > if ( !$is_valid ) { > if ( $error eq 'too_short' ) { > my $min_length = C4::Context->preference('minPasswordLength'); >- $min_length = 3 if not $min_length or $min_length < 3; >+ $min_length = 8 if not $min_length or $min_length < 8; > > my $password_length = length($password); > Koha::Exceptions::Password::TooShort->throw( >diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql >index e39352448cb..39bcd59fdc2 100644 >--- a/installer/data/mysql/sysprefs.sql >+++ b/installer/data/mysql/sysprefs.sql >@@ -326,7 +326,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('MaxTotalSuggestions','',NULL,'Number of total suggestions used for time limit with NumberOfSuggestionDays','Free'), > ('MembershipExpiryDaysNotice','',NULL,'Send an account expiration notice that a patron\'s card is about to expire after','Integer'), > ('MergeReportFields','',NULL,'Displayed fields for deleted MARC records after merge','Free'), >-('minPasswordLength','3',NULL,'Specify the minimum length of a patron/staff password','free'), >+('minPasswordLength','8',NULL,'Specify the minimum length of a patron/staff password','free'), > ('NewItemsDefaultLocation','','','If set, all new items will have a location of the given Location Code ( Authorized Value type LOC )',''), > ('NewsAuthorDisplay','none','none|opac|staff|both','Display the author name for news items.','Choice'), > ('noissuescharge','5','','Define maximum amount withstanding before check outs are blocked','Integer'), >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 4b5eb35d126..e142c524e31 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >@@ -105,6 +105,12 @@ legend:hover { > </div> > [% END %] > >+ [% IF Koha.Preference('minPasswordLength') < 8 %] >+ <div class="dialog alert"> >+ <p><strong>Warning: </strong>The <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=minPasswordLength">minPasswordLength system preference</a> is less than 8 characters. This means library patrons can set unsecure passwords. Please set the <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=minPasswordLength">minPasswordLength system preference</a> to 8 or higher.</p> >+ </div> >+ [% END %] >+ > [% IF ( nok ) %] > <div class="dialog alert"> > <p>The following fields are wrong. Please fix them.</p> >diff --git a/t/AuthUtils.t b/t/AuthUtils.t >index aa73e1651c5..9380bad17b7 100644 >--- a/t/AuthUtils.t >+++ b/t/AuthUtils.t >@@ -35,34 +35,34 @@ subtest 'is_password_valid' => sub { > 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, 'min password size should be 8' ); >+ is( $error, 'too_short', 'min password size should be 8' ); >+ ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( ' 12345678' ); > 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_valid, $error ) = Koha::AuthUtils::is_password_valid( '12345678 ' ); > 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' ); >+ ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( '12345678' ); >+ is( $is_valid, 1, 'min password size should be 8' ); > > t::lib::Mocks::mock_preference('RequireStrongPassword', 1); >- t::lib::Mocks::mock_preference('minPasswordLength', 8); >- ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( '12345678' ); >+ t::lib::Mocks::mock_preference('minPasswordLength', 9); >+ ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( '123456789' ); > 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_valid, $error ) = Koha::AuthUtils::is_password_valid( 'abcd12345' ); > 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_valid, $error ) = Koha::AuthUtils::is_password_valid( 'abcD12345' ); > 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); >+ t::lib::Mocks::mock_preference('minPasswordLength', 9); > my $all_valid = 1; > for ( 1 .. 10 ) { > my $password = Koha::AuthUtils::generate_password; >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index 0ca5faa7178..f9ea316b1db 100644 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -1788,30 +1788,30 @@ subtest '->set_password' => sub { > t::lib::Mocks::mock_preference( 'minPasswordLength', undef ); > throws_ok { $patron->set_password({ password => 'ab' }); } > 'Koha::Exceptions::Password::TooShort', >- 'minPasswordLength is undef, fall back to 3, fail test'; >+ 'minPasswordLength is undef, fall back to 8, fail test'; > is( "$@", >- 'Password length (2) is shorter than required (3)', >+ 'Password length (2) is shorter than required (8)', > 'Exception parameters passed correctly' > ); > > t::lib::Mocks::mock_preference( 'minPasswordLength', 2 ); > throws_ok { $patron->set_password({ password => 'ab' }); } > 'Koha::Exceptions::Password::TooShort', >- 'minPasswordLength is 2, fall back to 3, fail test'; >+ 'minPasswordLength is 2, fall back to 8, fail test'; > >- t::lib::Mocks::mock_preference( 'minPasswordLength', 5 ); >- throws_ok { $patron->set_password({ password => 'abcb' }); } >+ t::lib::Mocks::mock_preference( 'minPasswordLength', 10 ); >+ throws_ok { $patron->set_password({ password => 'abcdefghi' }); } > 'Koha::Exceptions::Password::TooShort', >- 'minPasswordLength is 5, fail test'; >+ 'minPasswordLength is 10, fail test'; > > # Trailing spaces tests >- throws_ok { $patron->set_password({ password => 'abcD12d ' }); } >+ throws_ok { $patron->set_password({ password => 'abcDef12d ' }); } > 'Koha::Exceptions::Password::WhitespaceCharacters', > 'Password contains trailing spaces, exception is thrown'; > > # Require strong password tests > t::lib::Mocks::mock_preference( 'RequireStrongPassword', 1 ); >- throws_ok { $patron->set_password({ password => 'abcd a' }); } >+ throws_ok { $patron->set_password({ password => 'abcdef a' }); } > 'Koha::Exceptions::Password::TooWeak', > 'Password is too weak, exception is thrown'; > >@@ -1819,7 +1819,7 @@ subtest '->set_password' => sub { > $patron->discard_changes; > is( $patron->login_attempts, 3, 'Previous tests kept login attemps count' ); > >- $patron->set_password({ password => 'abcD12 34' }); >+ $patron->set_password({ password => 'abcD12345e' }); > $patron->discard_changes; > > is( $patron->login_attempts, 0, 'Changing the password resets the login attempts count' ); >@@ -1831,11 +1831,11 @@ subtest '->set_password' => sub { > t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 ); > $patron->login_attempts(3)->store; > my $old_digest = $patron->password; >- $patron->set_password({ password => 'abcd a' }); >+ $patron->set_password({ password => 'abcd abc' }); > $patron->discard_changes; > > isnt( $patron->password, $old_digest, 'Password has been updated' ); >- ok( checkpw_hash('abcd a', $patron->password), 'Password hash is correct' ); >+ ok( checkpw_hash('abcd abc', $patron->password), 'Password hash is correct' ); > is( $patron->login_attempts, 0, 'Login attemps have been reset' ); > > my $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'CHANGE PASS', object => $patron->borrowernumber } )->count; >@@ -1843,7 +1843,7 @@ subtest '->set_password' => sub { > > # Enable logging password changes > t::lib::Mocks::mock_preference( 'BorrowersLog', 1 ); >- $patron->set_password({ password => 'abcd b' }); >+ $patron->set_password({ password => 'abcd babc' }); > > $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'CHANGE PASS', object => $patron->borrowernumber } )->count; > is( $number_of_logs, 1, 'With BorrowerLogs, Koha::Patron->set_password does log password changes' ); >diff --git a/t/db_dependent/api/v1/patrons_password.t b/t/db_dependent/api/v1/patrons_password.t >index 2a57516319d..f2faedacd3d 100644 >--- a/t/db_dependent/api/v1/patrons_password.t >+++ b/t/db_dependent/api/v1/patrons_password.t >@@ -45,10 +45,10 @@ subtest 'set() (authorized user tests)' => sub { > > my ( $patron, $session ) = create_user_and_session({ authorized => 1 }); > >- t::lib::Mocks::mock_preference( 'minPasswordLength', 3 ); >+ t::lib::Mocks::mock_preference( 'minPasswordLength', 8 ); > t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 ); > >- my $new_password = 'abc'; >+ my $new_password = 'abcdefgh'; > > my $tx > = $t->ua->build_tx( POST => "/api/v1/patrons/" >@@ -68,7 +68,7 @@ subtest 'set() (authorized user tests)' => sub { > $tx->req->env( { REMOTE_ADDR => '127.0.0.1' } ); > $t->request_ok($tx)->status_is(400)->json_is({ error => 'Passwords don\'t match' }); > >- t::lib::Mocks::mock_preference( 'minPasswordLength', 5 ); >+ t::lib::Mocks::mock_preference( 'minPasswordLength', 10 ); > $tx > = $t->ua->build_tx( POST => "/api/v1/patrons/" > . $patron->id >@@ -76,9 +76,9 @@ subtest 'set() (authorized user tests)' => sub { > > $tx->req->cookies( { name => 'CGISESSID', value => $session->id } ); > $tx->req->env( { REMOTE_ADDR => '127.0.0.1' } ); >- $t->request_ok($tx)->status_is(400)->json_is({ error => 'Password length (3) is shorter than required (5)' }); >+ $t->request_ok($tx)->status_is(400)->json_is({ error => 'Password length (8) is shorter than required (10)' }); > >- $new_password = 'abc '; >+ $new_password = 'abcdefg '; > $tx > = $t->ua->build_tx( POST => "/api/v1/patrons/" > . $patron->id >@@ -88,7 +88,7 @@ subtest 'set() (authorized user tests)' => sub { > $tx->req->env( { REMOTE_ADDR => '127.0.0.1' } ); > $t->request_ok($tx)->status_is(400)->json_is({ error => '[Password contains leading/trailing whitespace character(s)]' }); > >- $new_password = 'abcdefg'; >+ $new_password = 'abcdefghijkl'; > $tx > = $t->ua->build_tx( POST => "/api/v1/patrons/" > . $patron->id >@@ -134,10 +134,10 @@ subtest 'set_public() (unprivileged user tests)' => sub { > t::lib::Mocks::mock_preference( 'RESTPublicAPI', 1 ); > > t::lib::Mocks::mock_preference( 'OpacPasswordChange', 0 ); >- t::lib::Mocks::mock_preference( 'minPasswordLength', 3 ); >+ t::lib::Mocks::mock_preference( 'minPasswordLength', 8 ); > t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 ); > >- my $new_password = 'abc'; >+ my $new_password = 'abcdefgh'; > > my $tx = $t->ua->build_tx( > POST => "/api/v1/public/patrons/" >-- >2.11.0
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 18308
:
68374
|
68381
|
68451
|
69318
|
69319
|
69320
|
69321
|
69323
|
69324
|
69459
|
69461
|
69462
|
87124
|
87125
|
87126
|
87128
|
87129
|
87339
|
87340
|
87564
|
87572
|
87573
|
87747
|
87959
|
87960
|
87961
|
87962
|
87963
|
87964
|
87966
|
88010
|
88011
|
88012
|
88013
|
88014
|
88242
|
88243
|
88306
|
88307
|
88308
|
88309
|
88310
|
88311
|
88312
|
88313
|
88314
|
88315
|
88316
|
88317
|
88318
|
88319
|
88320
|
88321
|
88840
|
90079
|
103617
|
104235
|
104236
|
104351