From ce4884c4d0ca3d70e4cac5d5e3ecec56be8eeccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juhani=20Sepp=C3=A4l=C3=A4?= Date: Mon, 11 Aug 2014 15:58:05 +0300 Subject: [PATCH 1/3] Bug 12617: Koha should let admins to configure automatically generated password complexity/difficulty Adds simple password policy(with regards to complexity) management into categories: - Per category password policy: admins can configure what kind of passwords get generated in member-passwords. User-created passwords are also checked against the policy if it is defined and complexity is enforced for every user based on their set category. - Predefined policies: - simplenumeric: the digits 0-9 allowed only - alphanumeric: passwords must contain only the digits 0-9 and lowercase and uppercase characters. Special characters are not allowed. - complex: patrons are required to use complex passwords containing numbers, uppercase and lowercase characters and special characters. Old passwords for excisting patrons are not affected. To test: 1. Apply this patch and update database. 2. Navigate to categories.pl and note there is new column 'Password policies' has been added. 3. Edit some categories and set password policy for them. 4. Set some values to sysprefs 'minPasswordLength', 'minAlnumPasswordLength' and 'minComplexPasswordLength'. Staff interface: 1. Create new patron. 2. Set their password against their categorys policy and save. 3. Error message is displayed (with content depending on password policy). 4. Set acceptable password and save succesfully. 5. Repeat steps 2-3-4 on patron edit page. 6. Repeat steps 2-3-4 on 'Change password' page. OPAC: 1. Enable 'OpacPasswordChange' and 'OpacResetPassword'. 2. On OPAC, repeat what you did on staff interface (on create, edit and 'Change your password'. 3. Confirm errors are displayed correctly and saving works. 4. Log out and go to 'Forgotten password recovery' page. 5. Send and receive email for password recovery. 6. Set unacceptable password and save, confirm correct error is displayed. 7. Set acceptable password and save succesfully. REST API: 1. With your preferred REST client (curl e.g) sent POST request to /api/v1/patrons/{patron_id}/password with 'password' and 'password_2' parameters. 2. Confirm correct error message is displayed when sending password against password policy. 3. Confirm password is changed when acceptable password is send. Also prove t/AuthUtils.t and t/db_dependent/api/v1/patrons_password.t Sponsored-by: Koha-Suomi Oy --- Koha/AuthUtils.pm | 107 +++++++++++--- Koha/Exceptions/Password.pm | 12 ++ Koha/Patron.pm | 20 ++- Koha/Schema/Result/Category.pm | 8 ++ admin/categories.pl | 3 + ...gure-automatically-generated-password.perl | 18 +++ installer/data/mysql/kohastructure.sql | 1 + installer/data/mysql/sysprefs.sql | 2 + installer/onboarding.pl | 2 +- .../prog/en/includes/password_check.inc | 2 +- .../prog/en/modules/admin/categories.tt | 15 ++ .../en/modules/admin/preferences/patrons.pref | 12 +- .../en/modules/members/member-password.tt | 36 +++-- .../prog/en/modules/members/memberentrygen.tt | 9 ++ .../bootstrap/en/modules/opac-memberentry.tt | 19 ++- .../bootstrap/en/modules/opac-passwd.tt | 16 ++- .../en/modules/opac-password-recovery.tt | 12 ++ members/member-password.pl | 15 +- members/memberentry.pl | 8 +- opac/opac-memberentry.pl | 19 ++- opac/opac-passwd.pl | 13 ++ opac/opac-password-recovery.pl | 26 ++++ t/AuthUtils.t | 136 ++++++++++++++++-- t/db_dependent/api/v1/patrons_password.t | 83 ++++++++++- 24 files changed, 542 insertions(+), 52 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/Bug-12617-Koha-should-let-admins-to-configure-automatically-generated-password.perl diff --git a/Koha/AuthUtils.pm b/Koha/AuthUtils.pm index 7c4dcd1002..493193dfd6 100644 --- a/Koha/AuthUtils.pm +++ b/Koha/AuthUtils.pm @@ -22,9 +22,10 @@ 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 String::Random qw( random_string random_regex ); use C4::Context; +use Koha::Patron::Categories; use base 'Exporter'; @@ -139,48 +140,114 @@ sub generate_salt { =head2 is_password_valid -my ( $is_valid, $error ) = is_password_valid( $password ); + my ( $is_valid, $error ) = is_password_valid( $password, $categorycode ); -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') +Validates a member's password based on category password policy and/or minPasswordLength =cut sub is_password_valid { - my ($password) = @_; - my $minPasswordLength = C4::Context->preference('minPasswordLength'); + my ( $password, $categorycode ) = @_; + + my $categoryinfo = $categorycode ? Koha::Patron::Categories->find($categorycode) : undef; + my $passwordpolicy = $categoryinfo->passwordpolicy; + my $minPasswordLength = min_password_length($categorycode); $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,}|; + + if ($password =~ m|^\s+| or $password =~ m|\s+$|) { + return (0, 'has_whitespaces'); + } else { + if ($passwordpolicy) { + if ($passwordpolicy eq "complex") { + unless ($password =~ /[0-9]/ + && $password =~ /[a-zåäö]/ + && $password =~ /[A-ZÅÄÖ]/ + && $password =~ /[\|\[\]\{\}!@#\$%\^&\*\(\)_\-\+\?]/ + && length($password) >= $minPasswordLength ) { + return (0, "complex_policy_mismatch"); + } + + } + elsif ($passwordpolicy eq "alphanumeric") { + unless ($password =~ /[0-9]/ + && $password =~ /[a-zA-ZöäåÖÄÅ]/ + && $password !~ /\W/ + && $password !~ /[_-]/ + && length($password) >= $minPasswordLength ) { + return (0, "alpha_policy_mismatch"); + } + } + else { + if ($password !~ /^[0-9]+$/ || length($password) < $minPasswordLength) { + return (0, "simple_policy_mismatch"); + } + } + } elsif (defined $minPasswordLength && 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 ); + + return 1; } =head2 generate_password -my password = generate_password(); +my password = generate_password($categorycode); -Generate a password according to the minPasswordLength and RequireStrongPassword. +Returns a new patron password suggestion based on borrowers password policy from categories =cut sub generate_password { - my $minPasswordLength = C4::Context->preference('minPasswordLength'); - $minPasswordLength = 8 if not $minPasswordLength or $minPasswordLength < 8; + my ( $categorycode ) = @_; + my $categoryinfo = Koha::Patron::Categories->find($categorycode) if $categorycode; + my $passwordpolicy = $categoryinfo->passwordpolicy; + my $minpasslength = min_password_length($categorycode); + $minpasslength = 8 if not $minpasslength or ( $minpasslength < 8 and !$passwordpolicy); my ( $password, $is_valid ); do { - $password = random_string('.' x $minPasswordLength ); - ( $is_valid, undef ) = is_password_valid( $password ); + if (!$passwordpolicy || $passwordpolicy eq "complex") { + $password = random_string('.' x $minpasslength ); + } else { + if ($passwordpolicy eq "alphanumeric") { + $password = random_regex('[a-zA-Z0-9]' x $minpasslength ); + } else { + $password = random_regex('[0-9]' x $minpasslength ); + } + } + ( $is_valid, undef ) = is_password_valid( $password, $categorycode ); } while not $is_valid; return $password; } +=head2 minPasswordLength + + $minpasslength = minPasswordLength($categorycode); + +Returns correct minPasswordLength + +=cut + +sub min_password_length { + my $categorycode = shift; + + my $categoryinfo = Koha::Patron::Categories->find($categorycode); + my $passwordpolicy = $categoryinfo ? $categoryinfo->passwordpolicy : undef; + my $minpasslen; + if ($passwordpolicy eq "complex") { + $minpasslen = C4::Context->preference("minComplexPasswordLength"); + }elsif ($passwordpolicy eq "alphanumeric") { + $minpasslen = C4::Context->preference("minAlnumPasswordLength"); + }else { + $minpasslen = C4::Context->preference("minPasswordLength"); + } + return $minpasslen; +} + =head2 get_script_name diff --git a/Koha/Exceptions/Password.pm b/Koha/Exceptions/Password.pm index 9cf24f0db0..44ebf02a4b 100644 --- a/Koha/Exceptions/Password.pm +++ b/Koha/Exceptions/Password.pm @@ -43,6 +43,18 @@ use Exception::Class ( isa => 'Koha::Exceptions::Password', description => 'The password was rejected by a plugin' }, + 'Koha::Exceptions::Password::SimplePolicy' => { + isa => 'Koha::Exceptions::Password', + description => 'Password does not match simplenumeric passwordpolicy', + }, + 'Koha::Exceptions::Password::AlphaPolicy' => { + isa => 'Koha::Exceptions::Password', + description => 'Password does not match alphanumeric passwordpolicy', + }, + 'Koha::Exceptions::Password::ComplexPolicy' => { + isa => 'Koha::Exceptions::Password', + description => 'Password does not match complex passwordpolicy', + }, ); sub full_message { diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 5436ccc448..73461a8eb3 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -734,6 +734,12 @@ Exceptions are thrown if the password is not good enough. =item Koha::Exceptions::Password::Plugin (if a "check password" plugin is enabled) +=item Koha::Exceptions::Password::SimplePolicy + +=item Koha::Exceptions::Password::AlphaPolicy + +=item Koha::Exceptions::Password::ComplexPolicy + =back =cut @@ -744,7 +750,9 @@ sub set_password { my $password = $args->{password}; unless ( $args->{skip_validation} ) { - my ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password ); + + my $categorycode = $self->category->categorycode; + my ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password, $categorycode ); if ( !$is_valid ) { if ( $error eq 'too_short' ) { @@ -761,6 +769,15 @@ sub set_password { elsif ( $error eq 'too_weak' ) { Koha::Exceptions::Password::TooWeak->throw(); } + elsif ( $error eq 'simple_policy_mismatch' ) { + Koha::Exceptions::Password::SimplePolicy->throw(); + } + elsif ( $error eq 'alpha_policy_mismatch' ) { + Koha::Exceptions::Password::AlphaPolicy->throw(); + } + elsif ( $error eq 'complex_policy_mismatch' ) { + Koha::Exceptions::Password::ComplexPolicy->throw(); + } } } @@ -801,7 +818,6 @@ sub set_password { return $self; } - =head3 renew_account my $new_expiry_date = $patron->renew_account diff --git a/Koha/Schema/Result/Category.pm b/Koha/Schema/Result/Category.pm index 16d7a2228c..97bd5d3878 100644 --- a/Koha/Schema/Result/Category.pm +++ b/Koha/Schema/Result/Category.pm @@ -133,6 +133,12 @@ __PACKAGE__->table("categories"); data_type: 'tinyint' is_nullable: 1 +=head2 passwordpolicy + + data_type: 'varchar' + is_nullable: 1 + size: 40 + =cut __PACKAGE__->add_columns( @@ -189,6 +195,8 @@ __PACKAGE__->add_columns( { data_type => "tinyint", is_nullable => 1 }, "change_password", { data_type => "tinyint", is_nullable => 1 }, + "passwordpolicy", + { data_type => "varchar", is_nullable => 1, size => 40 }, ); =head1 PRIMARY KEY diff --git a/admin/categories.pl b/admin/categories.pl index f9144e7ced..4635ef2643 100755 --- a/admin/categories.pl +++ b/admin/categories.pl @@ -94,6 +94,7 @@ elsif ( $op eq 'add_validate' ) { my $default_privacy = $input->param('default_privacy'); my $reset_password = $input->param('reset_password'); my $change_password = $input->param('change_password'); + my $selectedpasswordpolicy = $input->param('passwordpolicy'); my @branches = grep { $_ ne q{} } $input->multi_param('branches'); $reset_password = undef if $reset_password eq -1; @@ -129,6 +130,7 @@ elsif ( $op eq 'add_validate' ) { $category->default_privacy($default_privacy); $category->reset_password($reset_password); $category->change_password($change_password); + $category->passwordpolicy($selectedpasswordpolicy); eval { $category->store; $category->replace_branch_limitations( \@branches ); @@ -157,6 +159,7 @@ elsif ( $op eq 'add_validate' ) { default_privacy => $default_privacy, reset_password => $reset_password, change_password => $change_password, + passwordpolicy => $selectedpasswordpolicy, }); eval { $category->store; diff --git a/installer/data/mysql/atomicupdate/Bug-12617-Koha-should-let-admins-to-configure-automatically-generated-password.perl b/installer/data/mysql/atomicupdate/Bug-12617-Koha-should-let-admins-to-configure-automatically-generated-password.perl new file mode 100644 index 0000000000..89721eb947 --- /dev/null +++ b/installer/data/mysql/atomicupdate/Bug-12617-Koha-should-let-admins-to-configure-automatically-generated-password.perl @@ -0,0 +1,18 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do("ALTER TABLE categories ADD COLUMN passwordpolicy VARCHAR(40) DEFAULT NULL AFTER change_password"); + + $dbh->do( + "INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) VALUES ('minAlnumPasswordLength', '10', null, 'Specify the minimum length for alphanumeric passwords', 'free')" + ); + $dbh->do( + "INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) VALUES ('minComplexPasswordLength', '10', null, 'Specify the minimum length for complex passwords', 'free')" + ); + $dbh->do( + "UPDATE systempreferences set explanation='Specify the minimum length for simplenumeric passwords' where variable='minPasswordLength'" + ); + + # Always end with this (adjust the bug info) + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 12617 - Koha should let admins to configure automatically generated password complexity/difficulty)\n"; +} \ No newline at end of file diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index cc7c133653..33398e4f8f 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -327,6 +327,7 @@ CREATE TABLE `categories` ( -- this table shows information related to Koha patr `checkprevcheckout` varchar(7) NOT NULL default 'inherit', -- produce a warning for this patron category if this item has previously been checked out to this patron if 'yes', not if 'no', defer to syspref setting if 'inherit'. `reset_password` TINYINT(1) NULL DEFAULT NULL, -- if patrons of this category can do the password reset flow, `change_password` TINYINT(1) NULL DEFAULT NULL, -- if patrons of this category can change their passwords in the OAPC + `passwordpolicy` varchar(40) default NULL, -- which password policy patron category uses PRIMARY KEY (`categorycode`), UNIQUE KEY `categorycode` (`categorycode`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index dbb13938ae..2cbd12cc73 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -327,6 +327,8 @@ 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'), +('minAlnumPasswordLength', '10', null, 'Specify the minimum length for alphanumeric passwords', 'free') +('minComplexPasswordLength', '10', null, 'Specify the minimum length for complex passwords', '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'), diff --git a/installer/onboarding.pl b/installer/onboarding.pl index 7a76342311..4973ce3248 100755 --- a/installer/onboarding.pl +++ b/installer/onboarding.pl @@ -145,7 +145,7 @@ if ( $step == 3 ) { my $cardnumber = $input->param('cardnumber'); my $userid = $input->param('userid'); - my ( $is_valid, $passworderror) = Koha::AuthUtils::is_password_valid( $firstpassword ); + my ( $is_valid, $passworderror) = Koha::AuthUtils::is_password_valid( $firstpassword, undef ); if ( my $error_code = checkcardnumber($cardnumber) ) { if ( $error_code == 1 ) { 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..f057509dcc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/password_check.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/password_check.inc @@ -1,6 +1,6 @@ [% USE Koha %] [% BLOCK add_password_check %] - +