implement password validation for site users
authorTony Cook <tony@develop-help.com>
Sun, 10 Mar 2013 23:59:10 +0000 (10:59 +1100)
committerTony Cook <tony@develop-help.com>
Sat, 16 Mar 2013 00:48:22 +0000 (11:48 +1100)
MANIFEST
site/cgi-bin/modules/BSE/AdminSiteUsers.pm
site/cgi-bin/modules/BSE/UserReg.pm
site/cgi-bin/modules/SiteUser.pm
site/data/db/bse_msg_base.data
site/data/db/bse_msg_defaults.data
site/docs/config.pod
site/templates/admin/users/edit.tmpl
site/templates/common/default.tmpl
site/templates/user/options_base.tmpl

index 1b5f609..c6e3de6 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -278,6 +278,7 @@ site/cgi-bin/modules/BSE/Util/DynSort.pm
 site/cgi-bin/modules/BSE/Util/Format.pm
 site/cgi-bin/modules/BSE/Util/HTML.pm
 site/cgi-bin/modules/BSE/Util/Iterate.pm
+site/cgi-bin/modules/BSE/Util/PasswordValidate.pm
 site/cgi-bin/modules/BSE/Util/Prereq.pm
 site/cgi-bin/modules/BSE/Util/Secure.pm
 site/cgi-bin/modules/BSE/Util/SQL.pm
@@ -857,6 +858,7 @@ t/010-modules/060-sqldates.t
 t/010-modules/070-escape.t
 t/010-modules/080-cfg.t
 t/010-modules/090-subcalc.t
+t/010-modules/100-password-validate.t
 t/020-templater/000-load.t
 t/020-templater/010-token.t
 t/020-templater/020-parse.t
index 07af7ed..c1c8799 100644 (file)
@@ -13,7 +13,7 @@ use constant SITEUSER_GROUP_SECT => 'BSE Siteuser groups validation';
 use BSE::Template;
 use DevHelp::Date qw(dh_parse_date_sql dh_parse_time_sql);
 
-our $VERSION = "1.008";
+our $VERSION = "1.009";
 
 my %actions =
   (
@@ -418,10 +418,16 @@ sub req_save {
     my $confirm = $cgi->param('confirm_password');
     
     if (defined $newpass && length $newpass) {
-      my $min_pass_length = $cfg->entry('basic', 'minpassword') || 4;
-         my $error;
-      if (length $newpass < $min_pass_length) {
-       $errors{password} = "The password must be at least $min_pass_length characters";
+      my @errors;
+      my %other = map { $_ => $user->$_() } SiteUser->password_check_fields;
+      if (!SiteUser->check_password_rules
+         (
+          password => $newpass,
+          username => $user->userId,
+          other => \%other,
+          errors => \@errors,
+         )) {
+       $errors{password} = \@errors;
       }
       elsif (!defined $confirm || length $confirm == 0) {
        $errors{confirm_password} = "Please enter a confirmation password";
@@ -651,18 +657,28 @@ sub req_add {
     $user{password} = '';
   }
   else {
-    my $min_pass_length = $cfg->entry('basic', 'minpassword') || 4;
     my $userid = $cgi->param('userId');
     if (!defined $userid || length $userid == 0) {
       $errors{userId} = "Please enter a userid";
     }
     my $pass = $cgi->param('password');
+    $pass =~ s/\A\s+//, $pass =~ s/\s+\z// if defined $pass;
     my $pass2 = $cgi->param('confirm_password');
+    $pass2 =~ s/\A\s+//, $pass2 =~ s/\s+\z// if defined $pass2;
+    my %other = map { $_ => scalar $cgi->param($_) }
+      SiteUser->password_check_fields;
+    my @errors;
     if (!defined $pass || length $pass == 0) {
       $errors{password} = "Please enter a password";
     }
-    elsif (length $pass < $min_pass_length) {
-      $errors{password} = "The password must be at least $min_pass_length characters";
+    elsif (!SiteUser->check_password_rules
+          (
+           password => $pass,
+           username => $userid,
+           other => \%other,
+           errors => \@errors,
+          )) {
+      $errors{password} = \@errors;
     }
     elsif (!defined $pass2 || length $pass2 == 0) {
       $errors{confirm_password} = "Please enter a confirmation password";
index 537ba47..45b6a28 100644 (file)
@@ -18,7 +18,7 @@ use BSE::Util::Iterate;
 use base 'BSE::UI::UserCommon';
 use Carp qw(confess);
 
-our $VERSION = "1.024";
+our $VERSION = "1.025";
 
 use constant MAX_UNACKED_CONF_MSGS => 3;
 use constant MIN_UNACKED_CONF_GAP => 2 * 24 * 60 * 60;
@@ -621,18 +621,26 @@ sub req_register {
     $user{password} = '';
   }
   else {
-    my $min_pass_length = $cfg->entry('basic', 'minpassword') || 4;
     my $userid = $cgi->param('userid');
+    my @errors;
     if (!defined $userid || length $userid == 0) {
       $errors{userid} = $msgs->(reguser=>"Please enter your username");
     }
+    my %others = map { $_ => scalar($cgi->param($_)) }
+      SiteUser->password_check_fields;
     my $pass = $cgi->param('password');
     my $pass2 = $cgi->param('confirm_password');
     if (!defined $pass || length $pass == 0) {
       $errors{password} = $msgs->(regpass=>"Please enter your password");
     }
-    elsif (length $pass < $min_pass_length) {
-      $errors{password} = $msgs->(regpasslen=>"The password must be at least $min_pass_length characters");
+    elsif (!SiteUser->check_password_rules
+          (
+           password => $pass,
+           username => $userid,
+           errors => \@errors,
+           other => \%others,
+          )) {
+      $errors{password} = \@errors;
     }
     elsif (!defined $pass2 || length $pass2 == 0) {
       $errors{confirm_password} = 
@@ -1057,10 +1065,17 @@ sub req_saveopts {
          $errors{old_password} = $msgs->(optsbadold=>"You need to enter your old password to change your password")
        }
        else {
-         my $error;
-         if (!SiteUser->check_password_rules($newpass, \$error)) {
-           my ($code, @more) = @$error;
-           $errors{password} = $req->catmsg("msg:bse/user/$code", \@more)
+         my @errors;
+         my %others;
+         %others = map { $_ => $user->$_() } SiteUser->password_check_fields;
+         if (!SiteUser->check_password_rules
+             (
+              password => $newpass,
+              errors => \@errors,
+              username => $user->userId,
+              other => \%others
+             )) {
+           $errors{password} = \@errors;
          }
          elsif (!defined $confirm || length $confirm == 0) {
            $errors{confirm_password} = $msgs->(optsconfpass=>"Please enter a confirmation password");
@@ -2648,11 +2663,21 @@ sub req_lost_save {
   $req->validate(fields => \%lost_fields,
                 errors => \%errors);
   my $password = $req->cgi->param("password");
+  my $tmp_user = SiteUsers->lost_password_next($id);
   unless ($errors{password}) {
-    my $error;
-    unless (SiteUser->check_password_rules($password, \$error)) {
-      my ($errorid, @more) = @$error;
-      $errors{password} = $req->catmsg("msg:bse/user/$errorid", \@more)
+    my @errors;
+    my %others = $tmp_user
+      ? map { $_ => $tmp_user->$_() } SiteUser->password_check_fields
+       : ();
+    $DB::single = 1;
+    unless (SiteUser->check_password_rules
+           (
+            password => $password,
+            errors => \@errors,
+            username => $tmp_user ? $tmp_user->userId : undef,
+            other => \%others,
+           )) {
+      $errors{password} = \@errors;
     }
   }
 
index 9893058..9e5fb92 100644 (file)
@@ -8,7 +8,7 @@ use Constants qw($SHOP_FROM);
 use Carp qw(confess);
 use BSE::Util::SQL qw/now_datetime now_sqldate sql_normal_date sql_add_date_days/;
 
-our $VERSION = "1.007";
+our $VERSION = "1.008";
 
 use constant MAX_UNACKED_CONF_MSGS => 3;
 use constant MIN_UNACKED_CONF_GAP => 2 * 24 * 60 * 60;
@@ -897,16 +897,21 @@ sub lost_password {
 }
 
 sub check_password_rules {
-  my ($class, $password, $error) = @_;
+  my ($class, %opts) = @_;
 
-  my $cfg = BSE::Cfg->single;
-  my $min_pass_length = $cfg->entry('basic', 'minpassword') || 4;
-  if (length $password < $min_pass_length) {
-    $$error = [ "passwordlen", $min_pass_length ];
-    return;
-  }
+  require BSE::Util::PasswordValidate;
 
-  return 1;
+  my %rules = BSE::Cfg->single->entries("siteuser passwords");
+
+  return BSE::Util::PasswordValidate->validate
+    (
+     %opts,
+     rules => \%rules,
+    );
+}
+
+sub password_check_fields {
+  return qw(name1 name2);
 }
 
 1;
index 0a72ce4..cb3564e 100644 (file)
@@ -413,3 +413,32 @@ description: Message displayed when only an image file is accepted on upload, bu
 id: bse/files/pdf_file_required
 description: Message displayed when only a PDF is accepted on upload, but the upload isn't a PDF
 
+id: bse/util/
+description: Messages generated by utility classes
+
+id: bse/util/password/
+description: Messages generated by password validation (not verification)
+
+id: bse/util/password/length
+description: Password length validation failed ($1 is the length of password supplied, $2 is the required length)
+
+id: bse/util/password/entropy
+description: Password entropy validation failed ($1 is the entropy of the supplied password, $2 is the required entropy)
+
+id: bse/util/password/symbols
+description: Password symbol validation failed
+
+id: bse/util/password/digits
+description: Password digit validation failed
+
+id: bse/util/password/mixedcase
+description: Password mixed case validation failed
+
+id: bse/util/password/categories
+description: Password character category validation failed ($1 is number of categories in the supplied password, $2 is the required category count)
+
+id: bse/util/password/notuser
+description: Password notuser validation failed
+
+id: bse/util/password/notu5er
+description: Password notu5er validation failed
index 3ffee32..dfb1ae2 100644 (file)
@@ -292,9 +292,34 @@ message: File must be an image file
 id: bse/files/pdf_file_required
 message: File must be a PDF file
 
+id: bse/util/password/length
+message: Password must be at least %2:d characters long
+
+id: bse/util/password/entropy
+message: Your password isn't complex enough (but you're %3:.0f%% of the way there)
+
+id: bse/util/password/symbols
+message: Password must contain some punctuation (or other letters or numbers)
+
+id: bse/util/password/digits
+message: Password must contain some numbers
+
+id: bse/util/password/mixedcase
+message: Password must contain both upper and lower case letters
+
+id: bse/util/password/categories
+message: Password must contain at least %2:d out of upper case letters, lower case, numbers or punctuation.
+
+id: bse/util/password/notuser
+message: Password and your user name may not contain the other
+
+id: bse/util/password/notu5er
+message: Password and your user name may not contain the other (even with substitution)
+
 id: test/test/multiline
 message: <<TEXT
 This message has
 multiple
 lines
 TEXT
+
index 031e314..87a058d 100644 (file)
@@ -393,7 +393,8 @@ Default: 1.
 
 =item minpassword
 
-Minimum password length in characters.  Default: 4.
+Previously the minimum length of site user passwords in characters.
+You must now set C<length> in C<[siteuser passwords]>.
 
 =item no_cache_dynamic
 
@@ -2750,6 +2751,54 @@ is defined for the field:
   [article custom fields]
   customInt1_description=Stock Level
 
+=head2 [siteuser passwords]
+
+Rules for validating (not verifying) site user passwords.  Possible
+keys are:
+
+=over
+
+=item *
+
+C<length> - the minimum length of passwords.  This replaces
+C<minpassword> in C<[basic]>.
+
+=item *
+
+C<entropy> - the minimum password entropy as measured by
+L<Data::Password::Entropy>.
+
+=item *
+
+C<symbols> - if non-zero, the password must contain non-alphanumerics.
+
+=item *
+
+C<digits> - if non-zero, the password must contain digits.
+
+=item *
+
+C<mixedcase> - if non-zero, the password must contain both upper and
+lower case letters.
+
+=item *
+
+C<categories> - the number of categories out of 5 required out of
+symbols, digits, upper case, lower case and extended ASCII/Unicode.
+This should typically never be 5.
+
+=item *
+
+C<notuser> - the password and user name may not match each other
+case-insensitively.
+
+=item *
+
+C<notu5er> - the password may not match the user name
+case-insensitively, even with symbol replacement (e.g. "5" for "S".
+
+=back
+
 =head1 AUTHOR
 
 Tony Cook <tony@develop-help.com>
index a21f7b8..72944ef 100644 (file)
               Alt: <input type="text" name="image_<:imagetemplate id:>_alt" value="<:siteuser_image [imagetemplate id] alt:>" /><br />
        <:ifSiteuser_image [imagetemplate id]:><input type="checkbox" name="image_<:imagetemplate id:>_delete" value="1" /> Delete<br /><img src="<:siteuser_image [imagetemplate id] url:>" /><:or:><:eif:>
               </td>
-            <td class="help"> <:help editsiteuser images:>  <:error_img password:></td>
+            <td class="help"> <:help editsiteuser images:>  <:error_img [concatenate "image_" [imagetemplate id] "_file"] :></td>
           </tr>
 <:iterator end imagetemplates:>
 <:if UserCan siteuser_changepw:>
index d39e4aa..68c56e2 100644 (file)
     </td>
   </tr>
 </table>
-<:if Article body:> 
+<:.if article.body:> 
 <div class="body"><:body:></div>
-<:or Article:><:eif Article:><:if Embedded:><:or Embedded:><:ifAdmin:><:if Children:> 
-<p><font face="Verdana, Arial, Helvetica, sans-serif" size="2">Reorder child articles: 
-  <a href="/cgi-bin/admin/reorder.pl?parentid=<:article id:>&sort=title&refreshto=/cgi-bin/admin/admin.pl?id=<:article id:>">by 
-  title</a> | <a href="/cgi-bin/admin/reorder.pl?parentid=<:article id:>&sort=date&refreshto=/cgi-bin/admin/admin.pl?id=<:article id:>">by 
+<:.end if -:>
+<:.set kids = [ article.all_visible_kids ] :>
+<:.if !embedded:>
+  <:.if bse.admin and kids.size > 1:>
+<div class="adminsort">
+Reorder child articles: 
+  <:.set refresh = cfg.admin_url("admin", { "id":top.id }) -:>
+  <a href="<:= cfg.admin_url("reorder", { "parentid":article.id, "sort":"title",
+   "refreshto":refresh }) :>"">by title</a> 
+| <a href="/cgi-bin/admin/reorder.pl?parentid=<:article id:>&sort=date&refreshto=/cgi-bin/admin/admin.pl?id=<:article id:>">by 
   date</a> | <a href="/cgi-bin/admin/reorder.pl?parentid=<:article id:>&reverse=1&refreshto=/cgi-bin/admin/admin.pl?id=<:article id:>">reverse 
   order</a></font></p>
-<:or Children:><:eif Children:><:or:><:eif:>
-<:if Dynamic:>
-<:iterator begin dynchildren:> 
+  <:.end if -:>
+  <:.if dynamic -:>
+    <:iterator begin dynchildren -:> 
 <p><:thumbnail dynchild:> <a href="<:url dynchild:>"><b><font face="Verdana, Arial, Helvetica, sans-serif" size="3"><:dynchild 
   title:></font></b></a><br>
-  <:if Child summaryLength:><font face="Verdana, Arial, Helvetica, sans-serif" size="2"><:summary:></font><:or 
+    <:if Child summaryLength -:><font face="Verdana, Arial, Helvetica, sans-serif" size="2"><:summary:></font><:or 
   Child:><:eif Child:></p>
-<:iterator separator dynchildren:><:iterator end dynchildren:>
-<:or Dynamic:>
-<:if UnderThreshold:><:iterator begin 
-children:><:ifAdmin:> 
-<hr noshade size="1">
-<:or:><:eif:><:movekid:><:embed child:><:iterator separator children:><br>
-<:iterator end children:> <:or UnderThreshold:> <:iterator begin children:> 
-<p><:thumbnail child:> <a href="<:url child:>"><b><font face="Verdana, Arial, Helvetica, sans-serif" size="3"><:child 
-  title:></font></b></a><:movekid:><br>
-  <:if Child summaryLength:><font face="Verdana, Arial, Helvetica, sans-serif" size="2"><:summary:></font><:or 
-  Child:><:eif Child:></p>
-<:iterator separator children:><:iterator end children:><:eif UnderThreshold:><:eif Dynamic:><:eif 
-Embedded:> <br>
+  <:iterator end dynchildren -:>
+  <:.else -:>
+    <:.if kids.size < article.threshold :>
+      <:.for child in kids -:>
+        <:.call "mover", "parent":article -:>
+        <:= generator.vembed(child) |raw -:>
+       <:.end for -:>
+    <:.else -:>
+      <:.for child in kids -:>
+<div class="child">
+  <h1><:= generator.thumbnail(child) :><a href="<:= url(child):>"><:= child.title -:></a><:.call "mover", "parent":article :><h1>
+        <:.if child.summaryLength -:>
+      <div><:= generator.summary(child) :></div>
+        <:.end if -:>
+</div>
+      <:.end for -:>
+    <:.end if -:><:# if under threshold else -:>
+  <:.end if :><:# if dynamic else -:>
+<:.end if :><:# if embedded else :> <br>
 <:if Files:> <br>
 <table border="0" cellspacing="0" cellpadding="0">
   <tr> 
index f620a89..9879a37 100644 (file)
         <td width="100%"> 
           <input type="password" name="password" value="" size="40" maxlength="40" />
         </td>
+       <td><:error_img password:></td>
       </tr>
       <tr> 
         <th nowrap="nowrap" align="left"><b><font face="Verdana, Arial, Helvetica, sans-serif" size="-2">Confirm