From 5848da810e412a69932da6935285df4e91058b6f Mon Sep 17 00:00:00 2001 From: Agustin Moyano Date: Wed, 15 Apr 2020 17:13:02 -0300 Subject: [PATCH] Bug 23816: Add minimum password length and require strong password overrides by category MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This patch adds the capability to override minPasswordLenth and RequireStrongPassword settings by category To test: 1. koha-shell kohadev 2. koha-mysql kohadev 3. drop database koha_kohadev; 4. create database koha_kohadev; 5. go to admin page and start webinstaller. There continue the steps until onboarding. 6. reach step 3 of onboarding and create a new administrator patron CHECH => Password control woks as normal (Minimum length 3 and strong required) 7. finish Koha installation and enter admin with your new administrator 8. set minPasswordLength to 3 and RequireStrongPassword to “Don’t require” 9. Create a new category (CAT2 from now on.. CAT1 is the category you made in onboarding process) and set minimum password length to 8 and require strong password 10. Create two new patrons, one with CAT1(patron1) and one with CAT2 (patron2) CHECK => In both cases, try different combinations of length and strength. For patron1 the only requirement is to have 3 letters, but for patron2 the minimum length will be 8 and will require strong password. CHECK => Try changing patron category before saving. Password requirements will change with category change. 11. Edit CAT1 and set minimum password length to 5 12. Go to patron1 details page, and change password. CHECH => Now password minimum length is 5, but still it doesn’t require strong password 13. Edit CAT1, leave blank minimum password length and set require strong password to yes. 14. Go to patron1 details page, and change password. CHECH => Password minimum length is back to 3, but now strong password is required 15. Set minimum password length in CAT2 to 12. 16. Go to patron2 details page, and click to fill a random generated password CHECK => generated password should be 12 characters length 17. Set PatronSelfRegistration to Allow in admin settings 18. Go to OPAC and fill self registration from. CHECK => Play with patron category. For each change in category, password requirements are modified. CHECK => Set CAT1 as patron category, set ‘aA1’ as password (or another valid password for CAT1) and before hitting submit button, change to CAT2. Form should enter invalid state, and CAT2 password requirements should be displayed as error in password input. 19. Create a patron for CAT1 and another for CAT2, leaving password blank CHECK => For CAT1’s patron, generated password length is 8 (minimum length for generated passwords), but for CAT2’s patron should be 12 20. In admin set PatronSelfRegistrationVerifyByEmail to require 21. Fill self registration form again with CAT2 as category CHECK => Password requirements works as previous case. 22. Leave password blank and click submit 23. select * from message_queue; 24. Copy the link in the message and paste it in OPAC CHECH => Generated password is 12 characters long. (Copy user id for next steps) 25. In admin set OpacResetPassword to Allow 26. Go back to OPAC, reload and click on “Forgot password?” link 27. Paste user id and click submit 28. Repeat steps 23 and 24 CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.” CHECK => enter an invalid password and you’ll get the same message in warning. 29. Login OPAC with the last user and your newly created password 30. Go to “Change your password” option CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.” CHECK => enter an invalid password and you’ll get the same message in below “New password” input. 31. prove t/db_dependent/AuthUtils.t t/db_dependent/Koha/Patron/Category.t 32. Sign off Sponsored-by: Northeast Kansas Library - NEKLS Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart --- Koha/AuthUtils.pm | 19 ++++---- Koha/Patron.pm | 4 +- Koha/Patron/Category.pm | 32 ++++++++++++++ admin/categories.pl | 8 ++++ .../prog/en/includes/password_check.inc | 40 ++++++++++++----- .../prog/en/modules/admin/categories.tt | 37 ++++++++++++++++ .../en/modules/members/member-password.tt | 15 +++---- .../prog/en/modules/members/memberentrygen.tt | 7 ++- .../en/modules/onboarding/onboardingstep3.tt | 5 +-- .../bootstrap/en/includes/password_check.inc | 44 +++++++++++++------ .../bootstrap/en/modules/opac-memberentry.tt | 35 +++++++++++---- .../bootstrap/en/modules/opac-passwd.tt | 9 ++-- .../en/modules/opac-password-recovery.tt | 6 +-- members/memberentry.pl | 6 ++- misc/admin/set_password.pl | 13 +++--- opac/opac-memberentry.pl | 8 ++-- opac/opac-password-recovery.pl | 15 ++++++- opac/opac-registration-verify.pl | 3 +- 18 files changed, 224 insertions(+), 82 deletions(-) diff --git a/Koha/AuthUtils.pm b/Koha/AuthUtils.pm index 7c4dcd1002..a325c9cbdd 100644 --- a/Koha/AuthUtils.pm +++ b/Koha/AuthUtils.pm @@ -139,21 +139,21 @@ sub generate_salt { =head2 is_password_valid -my ( $is_valid, $error ) = is_password_valid( $password ); +my ( $is_valid, $error ) = is_password_valid( $password, $category ); -return $is_valid == 1 if the password match minPasswordLength and RequireStrongPassword conditions +return $is_valid == 1 if the password match category's minimum password length and strength if provided, or general 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'); + my ($password, $category) = @_; + my $minPasswordLength = $category?$category->effective_min_password_length: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') ) { + elsif ( $category?$category->effective_require_strong_password:C4::Context->preference('RequireStrongPassword') ) { return ( 0, 'too_weak' ) if $password !~ m|(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{$minPasswordLength,}|; } @@ -163,20 +163,21 @@ sub is_password_valid { =head2 generate_password -my password = generate_password(); +my password = generate_password($category); -Generate a password according to the minPasswordLength and RequireStrongPassword. +Generate a password according to category's minimum password length and strength if provided, or to the minPasswordLength and RequireStrongPassword system preferences. =cut sub generate_password { - my $minPasswordLength = C4::Context->preference('minPasswordLength'); + my ($category) = @_; + my $minPasswordLength = $category?$category->effective_min_password_length: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 ); + ( $is_valid, undef ) = is_password_valid( $password, $category ); } while not $is_valid; return $password; } diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 9c8de9b014..25ceac1470 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -744,11 +744,11 @@ sub set_password { my $password = $args->{password}; unless ( $args->{skip_validation} ) { - my ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password ); + my ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password, $self->category ); if ( !$is_valid ) { if ( $error eq 'too_short' ) { - my $min_length = C4::Context->preference('minPasswordLength'); + my $min_length = $self->category->effective_min_password_length; $min_length = 3 if not $min_length or $min_length < 3; my $password_length = length($password); diff --git a/Koha/Patron/Category.pm b/Koha/Patron/Category.pm index 72493cda7a..346a5d9778 100644 --- a/Koha/Patron/Category.pm +++ b/Koha/Patron/Category.pm @@ -255,6 +255,38 @@ sub effective_change_password { : C4::Context->preference('OpacPasswordChange'); } +=head3 effective_min_password_length + + $category->effective_min_password_length() + +Retrieve category's password length if setted, or minPasswordLength otherwise + +=cut + +sub effective_min_password_length { + my ($self) = @_; + + return C4::Context->preference('minPasswordLength') unless defined $self->min_password_length; + + return $self->min_password_length; +} + +=head3 effective_require_strong_password + + $category->effective_require_strong_password() + +Retrieve category's password strength if setted, or RequireStrongPassword otherwise + +=cut + +sub effective_require_strong_password { + my ($self) = @_; + + return C4::Context->preference('RequireStrongPassword') unless defined $self->require_strong_password; + + return $self->require_strong_password; +} + =head3 override_hidden_items if ( $patron->category->override_hidden_items ) { diff --git a/admin/categories.pl b/admin/categories.pl index cefe020890..749db2d0a0 100755 --- a/admin/categories.pl +++ b/admin/categories.pl @@ -94,10 +94,14 @@ elsif ( $op eq 'add_validate' ) { my $reset_password = $input->param('reset_password'); my $change_password = $input->param('change_password'); my $exclude_from_local_holds_priority = $input->param('exclude_from_local_holds_priority'); + my $min_password_length = $input->param('min_password_length'); + my $require_strong_password = $input->param('require_strong_password'); my @branches = grep { $_ ne q{} } $input->multi_param('branches'); $reset_password = undef if $reset_password eq -1; $change_password = undef if $change_password eq -1; + $min_password_length = undef unless length($min_password_length); + $require_strong_password = undef if $require_strong_password eq -1; my $is_a_modif = $input->param("is_a_modif"); @@ -130,6 +134,8 @@ elsif ( $op eq 'add_validate' ) { $category->reset_password($reset_password); $category->change_password($change_password); $category->exclude_from_local_holds_priority($exclude_from_local_holds_priority); + $category->min_password_length($min_password_length); + $category->require_strong_password($require_strong_password); eval { $category->store; $category->replace_branch_limitations( \@branches ); @@ -159,6 +165,8 @@ elsif ( $op eq 'add_validate' ) { reset_password => $reset_password, change_password => $change_password, exclude_from_local_holds_priority => $exclude_from_local_holds_priority, + min_password_length => $min_password_length, + require_strong_password => $require_strong_password, }); eval { $category->store; diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/password_check.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/password_check.inc index 9f4266b792..1c11896c57 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/password_check.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/password_check.inc @@ -1,20 +1,37 @@ [% USE Koha %] -[% BLOCK add_password_check %] -[% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt index 12de84c771..400db6241b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt @@ -229,6 +229,43 @@ [% END %] +
  • + + + Leave blank to use system default ([% Koha.Preference('minPasswordLength') | html %]) +
    • +
  • + + +
    • - [% SET password_pattern = ".{" _ Koha.Preference('minPasswordLength') _ ",}" %] - [% IF Koha.Preference('RequireStrongPassword') %] - [% SET password_pattern = '(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{' _ Koha.Preference('minPasswordLength') _ ',}' %] + [% SET password_pattern = ".{" _ patron.category.effective_min_password_length _ ",}" %] + [% IF patron.category.effective_require_strong_password %] + [% SET password_pattern = '(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{' _ patron.category.effective_min_password_length _ ',}' %] [% END %]
  • @@ -105,7 +105,7 @@ function generate_password() { // Always generate a strong password var chars = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; - var length = [% Koha.Preference('minPasswordLength') | html %]; + var length = [% patron.category.effective_min_password_length | html %]; if ( length < 8 ) length = 8; var password=''; for ( var i = 0 ; i < length ; i++){ @@ -117,7 +117,7 @@ $("body").on('click', "#fillrandom",function(e) { e.preventDefault(); var password = ''; - var pattern_regex = /(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{[% Koha.Preference('minPasswordLength') | html %],}/; + var pattern_regex = /(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{[% patron.category.effective_min_password_length | html %],}/; while ( ! pattern_regex.test( password ) ) { password = generate_password(); } @@ -156,8 +156,7 @@ }); }); - [% PROCESS 'password_check.inc' %] - [% PROCESS 'add_password_check' new_password => 'newpassword' %] + [% PROCESS 'password_check.inc' new_password => 'newpassword', minPasswordLength => patron.category.effective_min_password_length, RequireStrongPassword => patron.category.effective_require_strong_password %] [% END %] [% INCLUDE 'intranet-bottom.inc' %] 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 f50e686016..f0c235b7c6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -838,9 +838,9 @@ legend:hover { [% IF ( typeloo.typename_X ) %][% END %] [% END %] [% IF ( categoryloo.categorycodeselected ) %] - + [% ELSE %] - + [% END %] [% IF ( loop.last ) %] @@ -1738,8 +1738,7 @@ legend:hover { [% Asset.js("js/members.js") | $raw %] [% Asset.js("js/messaging-preference-form.js") | $raw %] - [% PROCESS 'password_check.inc' %] - [% PROCESS 'add_password_check' new_password => 'password' %] + [% PROCESS 'password_check.inc' new_password => 'password', category_selector => '#categorycode_entry', minPasswordLength => patron.category.effective_min_password_length, RequireStrongPassword => patron.category.effective_require_strong_password %] [% END %] [% INCLUDE 'intranet-bottom.inc' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep3.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep3.tt index 63314b6fa8..a5702eb7eb 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep3.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep3.tt @@ -61,7 +61,7 @@ Required

    @@ -116,8 +116,7 @@ [% INCLUDE 'validator-strings.inc' %] [% INCLUDE 'installer-strings.inc' %] [% Asset.js("js/onboarding.js") | $raw %] - [% PROCESS 'password_check.inc' %] - [% PROCESS 'add_password_check' new_password => 'password' %] + [% PROCESS 'password_check.inc' new_password => 'password', category_selector => '#categorycode_entry', RequireStrongPassword => Koha.Preference('RequireStrongPassword') %] [% END %] [% INCLUDE 'installer-intranet-bottom.inc' %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/password_check.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/password_check.inc index 1a94309ae8..338d07bbbb 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/password_check.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/password_check.inc @@ -1,25 +1,41 @@ [% USE Koha %] -[% BLOCK add_password_check %] -[% END %] + \ No newline at end of file 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 d001cb5145..60eaa58379 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt @@ -92,7 +92,7 @@ [% IF field == "B_email" %]
  • Alternate address information: email address
    • [% END %] [% IF field == "password_match" %]
  • Passwords do not match! password
    • [% END %] [% IF field == "password_too_short" %] -
  • Password must be at least [% minPasswordLength | html %] characters long.
    • +
  • Password must be at least [% patron.category.effective_min_password_length | html %] characters long.
    • [% END %] [% IF field == "password_too_weak" %]
  • Password must contain at least one digit, one lowercase and one uppercase.
    • @@ -269,9 +269,9 @@ @@ -896,10 +896,14 @@
    Password
    - [% IF ( Koha.Preference('RequireStrongPassword') ) %] -

    Your password must contain at least [% Koha.Preference('minPasswordLength') | html %] characters, including UPPERCASE, lowercase and numbers.

    + [% IF patron %] + [% IF ( patron.category.effective_require_strong_password ) %] +

    Your password must contain at least [% patron.category.effective_min_password_length | html %] characters, including UPPERCASE, lowercase and numbers.

    + [% ELSE %] +

    Your password must be at least [% patron.category.effective_min_password_length | html %] characters long.

    + [% END %] [% ELSE %] -

    Your password must be at least [% Koha.Preference('minPasswordLength') | html %] characters long.

    +

    [% END %] [% UNLESS mandatory.defined('password') %]

    If you do not enter a password a system generated password will be created.

    @@ -1064,9 +1068,8 @@ [% INCLUDE 'opac-bottom.inc' %] [% BLOCK jsinclude %] [% Asset.js("lib/jquery/plugins/jquery.validate.min.js") | $raw %] - [% PROCESS 'password_check.inc' %] - [% PROCESS 'add_password_check' new_password => 'borrower_password' %] [% INCLUDE 'calendar.inc' %] + + [% PROCESS 'password_check.inc' new_password => 'borrower_password', category_selector => '#borrower_categorycode', RequireStrongPassword => patron.category.effective_require_strong_password, minPasswordLength => patron.category.effective_min_password_length %] + [% 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 f61cc2a38a..7bc9a59045 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt @@ -63,10 +63,10 @@ [% IF ( Ask_data ) %]
    - [% IF ( Koha.Preference('RequireStrongPassword') ) %] -
    Your password must contain at least [% Koha.Preference('minPasswordLength') | html %] characters, including UPPERCASE, lowercase and numbers.
    + [% IF ( logged_in_user.category.effective_require_strong_password ) %] +
    Your password must contain at least [% logged_in_user.category.effective_min_password_length | html %] characters, including UPPERCASE, lowercase and numbers.
    [% ELSE %] -
    Your password must be at least [% Koha.Preference('minPasswordLength') | html %] characters long.
    +
    Your password must be at least [% logged_in_user.category.effective_min_password_length | html %] characters long.
    [% END %]
    @@ -112,8 +112,7 @@ [% INCLUDE 'opac-bottom.inc' %] [% BLOCK jsinclude %] [% Asset.js("lib/jquery/plugins/jquery.validate.min.js") | $raw %] - [% PROCESS 'password_check.inc' %] - [% PROCESS 'add_password_check' new_password => 'Newkey' %] + [% PROCESS 'password_check.inc' new_password => 'Newkey', minPasswordLength => logged_in_user.category.effective_min_password_length, RequireStrongPassword => logged_in_user.category.effective_require_strong_password %]