Bugzilla – Attachment 37740 Details for
Bug 12617
Koha should let admins to configure automatically generated password complexity/difficulty
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12617 - Koha should let admins to configure automatically generated password complexity/difficulty
Bug-12617---Koha-should-let-admins-to-configure-au.patch (text/plain), 17.89 KB, created by
Olli-Antti Kivilahti
on 2015-04-13 13:37:07 UTC
(
hide
)
Description:
Bug 12617 - Koha should let admins to configure automatically generated password complexity/difficulty
Filename:
MIME Type:
Creator:
Olli-Antti Kivilahti
Created:
2015-04-13 13:37:07 UTC
Size:
17.89 KB
patch
obsolete
>From 3bb7c76967e7b265475f29b5f8fb759917a5c84f Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Juhani=20Sepp=C3=A4l=C3=A4?= <jseppal@student.uef.fi> >Date: Mon, 11 Aug 2014 15:58:05 +0300 >Subject: [PATCH] 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. >- Reworks the old custom password generation code in member-password to use a pretty powerful perl >module from the CPAN: App::Genpass > >- 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. >--- > C4/Installer/PerlDependencies.pm | 5 + > C4/Members.pm | 102 ++++++++++++++++++++ > admin/categorie.pl | 9 +- > installer/data/mysql/kohastructure.sql | 1 + > installer/data/mysql/updatedatabase.pl | 11 +++ > .../prog/en/modules/admin/categorie.tt | 20 ++++ > .../prog/en/modules/members/member-password.tt | 11 ++- > members/member-password.pl | 35 +++---- > 8 files changed, 168 insertions(+), 26 deletions(-) > >diff --git a/C4/Installer/PerlDependencies.pm b/C4/Installer/PerlDependencies.pm >index ca12320..22734c2 100644 >--- a/C4/Installer/PerlDependencies.pm >+++ b/C4/Installer/PerlDependencies.pm >@@ -742,6 +742,11 @@ our $PERL_DEPS = { > 'required' => '0', > 'min_ver' => '5.836', > }, >+ 'App::Genpass' => { >+ 'usage' => 'Member password generation', >+ 'required' => '1', >+ 'min_ver' => '2.23', >+ }, > }; > > 1; >diff --git a/C4/Members.pm b/C4/Members.pm >index 6cc72e9..462df7a 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -41,6 +41,7 @@ use Koha::DateUtils; > use Koha::Borrower::Debarments qw(IsDebarred); > use Text::Unaccent qw( unac_string ); > use Koha::AuthUtils qw(hash_password); >+use App::Genpass; > > our ($VERSION,@ISA,@EXPORT,@EXPORT_OK,$debug); > >@@ -105,6 +106,9 @@ BEGIN { > GetBorrowersWithEmail > > HasOverdues >+ >+ &GenMemberPasswordSuggestion >+ &ValidateMemberPassword > ); > > #Modify data >@@ -329,6 +333,7 @@ sub GetMemberDetails { > SELECT borrowers.*, > category_type, > categories.description, >+ categories.passwordpolicy, > categories.BlockExpiredPatronOpacActions, > reservefee, > enrolmentperiod >@@ -343,6 +348,7 @@ sub GetMemberDetails { > SELECT borrowers.*, > category_type, > categories.description, >+ categories.passwordpolicy, > categories.BlockExpiredPatronOpacActions, > reservefee, > enrolmentperiod >@@ -2613,6 +2619,102 @@ sub HasOverdues { > return $count; > } > >+=head2 GenMemberPasswordSuggestion >+ >+ $password = GenMemberPasswordSuggestion($borrowernumber); >+ >+Returns a new patron password suggestion based on borrowers password policy from categories >+ >+=cut >+ >+sub GenMemberPasswordSuggestion { >+ my $borrowernumber = shift; >+ my $minpasslength = C4::Context->preference('minPasswordLength'); >+ >+ my $borrowerinfo = GetMemberDetails($borrowernumber); >+ my $policycode = $borrowerinfo->{'passwordpolicy'}; >+ >+ my $pass; >+ my $length = int(rand(3)) + $minpasslength; >+ if ($policycode) { >+ # Generates complex passwords with numbers and uppercase, lowercase and special characters >+ if ($policycode eq "complex") { >+ my $specials = ['!', '@', '#', '$', '%', '^', '&', '*', '(', ')', '_']; >+ $pass = App::Genpass->new(specials => $specials, readable => 0, length => $length); >+ } >+ >+ # Generates simple passwords with numbers and uppercase and lowercase characters >+ elsif ($policycode eq "alphanumeric") { >+ $pass = App::Genpass->new(length => $length); >+ } >+ >+ # Generates passwords with digits 0-9 only >+ else { >+ # verify => 0 due to a bug in generate() (workaround) >+ $pass = App::Genpass->new(lowercase => [], uppercase => [], verify => 0, length => $length); >+ } >+ } >+ # Defaults to empty constuctor which gives readable complex passwords >+ else { >+ $pass = App::Genpass->new(length => $length); >+ } >+ >+ return $pass->generate(); >+} >+ >+=head2 ValidateMemberPassword >+ >+ ($success, $errorcode, $errormessage) = ValidateMemberPassword($borrowernumber, $newpassword1, $newpassword2); >+ >+Validates a member's password based on category password policy and/or minPasswordLength >+ >+=cut >+ >+sub ValidateMemberPassword { >+ my ($borrowernumber, $newpassword1, $newpassword2) = @_; >+ my $minpasslength = C4::Context->preference('minPasswordLength'); >+ >+ if (!$borrowernumber) { >+ return (0, ,"NOBORROWER", "No borrowernumber given"); >+ } >+ >+ if ((!$newpassword1 || !$newpassword2) || ($newpassword1 ne $newpassword2)) { >+ return (0, "NOMATCH", "The passwords do not match"); >+ } >+ >+ if (length($newpassword1) < $minpasslength) { >+ return (0, "SHORTPASSWORD", "The password is too short"); >+ } >+ >+ my $memberinfo = GetMemberDetails($borrowernumber); >+ my $passwordpolicy = $memberinfo->{'passwordpolicy'}; >+ >+ if ($passwordpolicy) { >+ if ($passwordpolicy eq "simplenumeric") { >+ if ($newpassword1 !~ /[0-9]+/) { >+ return (0, "NOPOLICYMATCH", "Password policy: password can only contain digits 0-9"); >+ } >+ } >+ elsif ($passwordpolicy eq "alphanumeric") { >+ unless ($newpassword1 =~ /[0-9]/ >+ && $newpassword1 =~ /[a-zA-ZöäåÃÃÃ]/ >+ && $newpassword1 !~ /\W/ >+ && $newpassword1 !~ /[_-]/) { >+ return (0, "NOPOLICYMATCH", "Password policy: password must contain both numbers and non-special characters (at least one of both)"); >+ } >+ } >+ else { >+ unless ($newpassword1 =~ /[0-9]/ >+ && $newpassword1 =~ /[a-zåäö]/ >+ && $newpassword1 =~ /[A-ZÃÃÃ]/ >+ && $newpassword1 =~ /[\|\[\]\{\}!@#\$%\^&\*\(\)_\-\+\?]/) { >+ return (0, "NOPOLICYMATCH", "Password policy: password must contain numbers, characters and special characters (at least one of each)"); >+ } >+ } >+ } >+ return 1; >+} >+ > END { } # module clean-up code here (global destructor) > > 1; >diff --git a/admin/categorie.pl b/admin/categorie.pl >index e2ea4c6..46f6e98 100755 >--- a/admin/categorie.pl >+++ b/admin/categorie.pl >@@ -45,6 +45,7 @@ use C4::Branch; > use C4::Output; > use C4::Dates; > use C4::Form::MessagingPreferences; >+use C4::Members; > > sub StringSearch { > my ($searchstring,$type)=@_; >@@ -139,6 +140,7 @@ if ($op eq 'add_form') { > SMSSendDriver => C4::Context->preference("SMSSendDriver"), > TalkingTechItivaPhone => C4::Context->preference("TalkingTechItivaPhoneNotification"), > "type_".$data->{'category_type'} => 1, >+ selectedpasswordpolicy => $data->{'passwordpolicy'}, > branches_loop => \@branches_loop, > BlockExpiredPatronOpacActions => $data->{'BlockExpiredPatronOpacActions'}, > ); >@@ -169,6 +171,7 @@ if ($op eq 'add_form') { > hidelostitems=?, > overduenoticerequired=?, > category_type=?, >+ passwordpolicy=?, > BlockExpiredPatronOpacActions=? > WHERE categorycode=?" > ); >@@ -184,6 +187,7 @@ if ($op eq 'add_form') { > 'hidelostitems', > 'overduenoticerequired', > 'category_type', >+ 'password-policy', > 'block_expired', > 'categorycode' > ) >@@ -219,9 +223,10 @@ if ($op eq 'add_form') { > hidelostitems, > overduenoticerequired, > category_type, >+ passwordpolicy, > BlockExpiredPatronOpacActions > ) >- VALUES (?,?,?,?,?,?,?,?,?,?,?,?)"); >+ VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?)"); > $sth->execute( > map { $input->param($_) } ( > 'categorycode', >@@ -235,6 +240,7 @@ if ($op eq 'add_form') { > 'hidelostitems', > 'overduenoticerequired', > 'category_type', >+ 'password-policy', > 'block_expired' > ) > ); >@@ -331,6 +337,7 @@ if ($op eq 'add_form') { > category_type => $results->[$i]{'category_type'}, > "type_".$results->[$i]{'category_type'} => 1, > branches => \@selected_branches, >+ passwordpolicy => $results->[$i]{'passwordpolicy'} > ); > if (C4::Context->preference('EnhancedMessagingPreferences')) { > my $brief_prefs = _get_brief_messaging_prefs($results->[$i]{'categorycode'}); >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index f7c31fe..3ce15e2 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -466,6 +466,7 @@ CREATE TABLE `categories` ( -- this table shows information related to Koha patr > `reservefee` decimal(28,6) default NULL, -- cost to place holds > `hidelostitems` tinyint(1) NOT NULL default '0', -- are lost items shown to this category (1 for yes, 0 for no) > `category_type` varchar(1) NOT NULL default 'A', -- type of Koha patron (Adult, Child, Professional, Organizational, Statistical, Staff) >+ `passwordpolicy` varchar(40) default NULL, > `BlockExpiredPatronOpacActions` tinyint(1) NOT NULL default '-1', -- wheither or not a patron of this category can renew books or place holds once their card has expired. 0 means they can, 1 means they cannot, -1 means use syspref BlockExpiredPatronOpacActions > PRIMARY KEY (`categorycode`), > UNIQUE KEY `categorycode` (`categorycode`) >diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl >index e64406f..dc8261e 100755 >--- a/installer/data/mysql/updatedatabase.pl >+++ b/installer/data/mysql/updatedatabase.pl >@@ -8624,6 +8624,17 @@ if (CheckVersion($DBversion)) { > SetVersion($DBversion); > } > >+$DBversion = "3.16.00.XXX"; >+if ( CheckVersion($DBversion) ) { >+ >+ $dbh->do("ALTER TABLE categories >+ ADD COLUMN passwordpolicy VARCHAR(40) DEFAULT NULL >+ "); >+ print "Upgrade to KD-156 done \n"; >+ SetVersion ($DBversion); >+} >+ >+ > =head1 FUNCTIONS > > =head2 TableExists($table) >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt >index 6a64c01..48929aa 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt >@@ -25,6 +25,11 @@ > if ( $("#branches option:selected").length < 1 ) { > $("#branches option:first").attr("selected", "selected"); > } >+ >+ var selectedPassPolicy = "[% selectedpasswordpolicy %]"; >+ if (selectedPassPolicy) { >+ $("#password-policy").val(selectedPassPolicy); >+ } > }); > function isNotNull(f,noalert) { > if (f.value.length ==0) { >@@ -189,6 +194,19 @@ > <span>Select All if this category type must to be displayed all the time. Otherwise select librairies you want to associate with this value. > </span> > </li> >+ <li> >+ <label for="password-policy">Category password policy</label> >+ <select name="password-policy" id="password-policy"> >+ <option value=""></option> >+ <option value="complex">Complex</option> >+ <option value="alphanumeric">Alphanumeric</option> >+ <option value="simplenumeric">Numbers only</option> >+ </select> >+ <span> >+ Selecting a password policy for a category affects both automatically created suggested passwords and enfo$ >+ of rules. >+ </span> >+ </li> > <li><label for="block_expired">Block expired patrons</label> > <select name="block_expired" id="block_expired"> > [% IF ( BlockExpiredPatronOpacActions == -1 ) %] >@@ -309,6 +327,7 @@ Confirm deletion of category [% categorycode |html %][% END %]</legend> > <th scope="col">Messaging</th> > [% END %] > <th scope="col">Branches limitations</th> >+ <th scope="col">Password policy</th> > <th scope="col"> </th> > <th scope="col"> </th> > </tr> >@@ -382,6 +401,7 @@ Confirm deletion of category [% categorycode |html %][% END %]</legend> > No limitation > [% END %] > </td> >+ <td>[% loo.passwordpolicy %]</td> > <td><a href="[% loo.script_name %]?op=add_form&categorycode=[% loo.categorycode |uri %]">Edit</a></td> > <td><a href="[% loo.script_name %]?op=delete_confirm&categorycode=[% loo.categorycode |uri %]">Delete</a></td> > </tr> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt >index d035a6d..29cb2a1 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt >@@ -48,18 +48,21 @@ > <div class="dialog alert"> > <h4>The following errors have occurred:</h4> > <ul> >- [% IF ( BADUSERID ) %] >+ [% IF ( errors.BADUSERID ) %] > <li>You have entered a username that already exists. Please choose another one.</li> > [% END %] >- [% IF ( SHORTPASSWORD ) %] >+ [% IF ( errors.SHORTPASSWORD ) %] > <li><strong>The password entered is too short</strong>. Password must be at least [% minPasswordLength %] characters.</li> > [% END %] >- [% IF ( NOPERMISSION ) %] >+ [% IF ( errors.NOPERMISSION ) %] > <li>You do not have permission to edit this patron's login information.</li> > [% END %] >- [% IF ( NOMATCH ) %] >+ [% IF ( errors.NOMATCH ) %] > <li><strong>The passwords entered do not match</strong>. Please re-enter the new password.</li> > [% END %] >+ [% IF ( errors.NOPOLICYMATCH ) %] >+ <li><strong>[% errors.NOPOLICYMATCH %]</strong>. Please re-enter the new password.</li> >+ [% END %] > </ul> > </div> > [% END %] >diff --git a/members/member-password.pl b/members/member-password.pl >index ac25da5..8fb6739 100755 >--- a/members/member-password.pl >+++ b/members/member-password.pl >@@ -41,21 +41,23 @@ $flagsrequired->{borrowers}=1; > my $member=$input->param('member'); > my $cardnumber = $input->param('cardnumber'); > my $destination = $input->param('destination'); >-my @errors; >+my %errors; > my ($bor)=GetMember('borrowernumber' => $member); > if(( $member ne $loggedinuser ) && ($bor->{'category_type'} eq 'S' ) ) { >- push(@errors,'NOPERMISSION') unless($staffflags->{'superlibrarian'} || $staffflags->{'staffaccess'} ); >+ $errors{'NOPERMISSION'} = 1 unless($staffflags->{'superlibrarian'} || $staffflags->{'staffaccess'} ); > # need superlibrarian for koha-conf.xml fakeuser. > } > my $newpassword = $input->param('newpassword'); > my $newpassword2 = $input->param('newpassword2'); > >-push(@errors,'NOMATCH') if ( ( $newpassword && $newpassword2 ) && ($newpassword ne $newpassword2) ); >- >-my $minpw = C4::Context->preference('minPasswordLength'); >-push(@errors,'SHORTPASSWORD') if( $newpassword && $minpw && (length($newpassword) < $minpw ) ); >+if ($newpassword) { >+ my ($success, $errorcode, $errormessage) = ValidateMemberPassword($member, $newpassword, $newpassword2); >+ if ($errorcode) { >+ $errors{$errorcode} = $errormessage; >+ } >+} > >-if ( $newpassword && !scalar(@errors) ) { >+if ( $newpassword && !scalar(keys %errors) ) { > my $digest=Koha::AuthUtils::hash_password($input->param('newpassword')); > my $uid = $input->param('newuserid'); > my $dbh=C4::Context->dbh; >@@ -67,18 +69,12 @@ if ( $newpassword && !scalar(@errors) ) { > print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$member"); > } > } else { >- push(@errors,'BADUSERID'); >+ $errors{'BADUSERID'} = 1; > } > } else { > my $userid = $bor->{'userid'}; > >- my $chars='abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'; >- my $length=int(rand(2))+C4::Context->preference("minPasswordLength"); >- my $defaultnewpassword=''; >- for (my $i=0; $i<$length; $i++) { >- $defaultnewpassword.=substr($chars, int(rand(length($chars))),1); >- } >- >+ my $defaultnewpassword = GenMemberPasswordSuggestion($member); > $template->param( defaultnewpassword => $defaultnewpassword ); > } > if ( $bor->{'category_type'} eq 'C') { >@@ -122,16 +118,13 @@ if (C4::Context->preference('ExtendedPatronAttributes')) { > destination => $destination, > is_child => ($bor->{'category_type'} eq 'C'), > activeBorrowerRelationship => (C4::Context->preference('borrowerRelationship') ne ''), >- minPasswordLength => $minpw, >+ minPasswordLength => C4::Context->preference('minPasswordLength'), > RoutingSerials => C4::Context->preference('RoutingSerials'), >+ errors => \%errors > ); > >-if( scalar(@errors )){ >+if( scalar(keys %errors )){ > $template->param( errormsg => 1 ); >- foreach my $error (@errors) { >- $template->param($error) || $template->param( $error => 1); >- } >- > } > > output_html_with_http_headers $input, $cookie, $template->output; >-- >1.7.9.5
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 12617
:
37740
|
107382
|
107383
|
107384
|
109388
|
109389
|
110526
|
110527
|
110528
|
111297
|
111298
|
111299