Fix for Bug 5280 - Fix password field so that the password is masked as it is entered
authorOwen Leonard <oleonard@myacpl.org>
Wed, 5 Oct 2011 16:04:12 +0000 (12:04 -0400)
committerPaul Poulain <paul.poulain@biblibre.com>
Mon, 13 Feb 2012 15:39:59 +0000 (16:39 +0100)
This patch changes the password field to a password type input on
member-password.pl and adds a confirmation field to both member-password.pl
and memberentry.pl requiring that the password be re-entered to
confirm.

Client-side and server-side validation for the two password fields has been added
to both pages. Multiple error messages can now be displayed together on
member-password.pl.

If the user wishes for Koha to suggest a random password on member-password.pl
they can click a link which will remove the password-type input fields, replace
them with text-type fields, and automatically fill them with the random
password suggestion.

Follow-up fix lets the members.js correctly handling errors when there are
no mandatory fields

LR followup: fixing slight error that corrects previously reported template error.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Tested password setting/changing utilities - all work as expected and described.
Passes prove t xt t/db_dependent tests congruent with current master failures (adds no new fails).

Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
koha-tmpl/intranet-tmpl/prog/en/js/members.js
koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt
koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
members/member-password.pl
members/memberentry.pl

index 796db0a..3ed6195 100644 (file)
@@ -85,6 +85,8 @@ var myDate2=document.form.dateexpiry.value.split ('/');
 // function to test all fields in forms and nav in different forms(1 ,2 or 3)
 function check_form_borrowers(nav){
        var statut=0;
+       var message = "";
+       var message_champ="";
        if (document.form.check_member.value == 1 )
        {
                if (document.form_double.answernodouble) {
@@ -101,9 +103,8 @@ function check_form_borrowers(nav){
        else
        {
            var champ_verif = document.form.BorrowerMandatoryField.value.split ('|');
-           var message = MSG_MISSING_MANDATORY
+           message += MSG_MISSING_MANDATORY
            message += "\n";
-           var message_champ="";
                for (var i=0; i<champ_verif.length; i++) {
                        if (document.getElementsByName(""+champ_verif[i]+"")[0]) {
                                var val_champ=eval("document.form."+champ_verif[i]+".value");
@@ -128,10 +129,18 @@ function check_form_borrowers(nav){
                        }
                }
        }
+
+       if ( document.form.password.value != document.form.password2.value ){
+                       if ( message_champ != '' ){
+                               message_champ += "\n";
+                       }
+                       message_champ+= MSG_PASSWORD_MISMATCH;
+                       statut=1;
+       }
+
        //patrons form to test if you checked no to the question of double
        if (statut!=1 && document.form.check_member.value > 0 ) {
                if (!(document.form_double.answernodouble.checked)){
-                       message ="";
                        message_champ+= MSG_DUPLICATE_SUSPICION;
                        statut=1;
                        document.form.nodouble.value=0;
index 93b1828..1269270 100644 (file)
@@ -1,6 +1,25 @@
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Patrons &rsaquo; [% IF ( newpassword ) %]Password Updated [% ELSE %]Update Password for [% surname %], [% firstname %][% END %]</title>
 [% INCLUDE 'doc-head-close.inc' %]
+<script type="text/JavaScript">
+//<![CDATA[
+    $(document).ready(function() {
+        $("#changepasswordf").submit(function(){
+            if($("input[name='newpassword']").val() != $("input[name='newpassword2']").val()){
+                alert(_("Passwords do not match"));
+                return false;
+            } else {
+                return true;
+            }
+        });
+        $("#fillrandom").live('click', function() {
+            $("#newpassword").after("<input type=\"text\" name=\"newpassword\" value=\"[% defaultnewpassword %]\">").remove();
+            $("#newpassword2").after("<input type=\"text\" name=\"newpassword2\" value=\"[% defaultnewpassword %]\">").remove();
+        });
+        $("div.hint").eq(0).after(" <div class=\"hint\"><a href=\"#\" id=\"fillrandom\">"+_("Click to fill with a randomly generated suggestion. ")+"<strong>"+_("Passwords will be displayed as text")+"</strong>.</a></div>");
+    });
+//]]>
+</script>
 </head>
 <body>
 [% INCLUDE 'header.inc' %]
 
 [% ELSE %]
 
-<form method="post" action="/cgi-bin/koha/members/member-password.pl">
+<form method="post" id="changepasswordf" action="/cgi-bin/koha/members/member-password.pl">
 <input type="hidden" name="destination" value="[% destination %]" />   
 <input type="hidden" name="cardnumber" value="[% cardnumber %]" />
 <input type="hidden" name="borrowernumber" id="borrowernumber" value="[% borrowernumber %]" />
        [% IF ( errormsg ) %]
+               <div class="dialog alert">
+               <h4>The following errors have occurred:</h4>
+               <ul>
                [% IF ( BADUSERID ) %]
-               <div class="dialog alert">You have entered a User ID that already exists.  Please choose another one.</div>
+               <li>You have entered a username that already exists.  Please choose another one.</li>
                [% END %]
                [% IF ( SHORTPASSWORD ) %]
-               <div class="dialog alert"><strong>The password entered is too short</strong>. Password must be at least [% minPasswordLength %] characters.</div>
+               <li><strong>The password entered is too short</strong>. Password must be at least [% minPasswordLength %] characters.</li>
                [% END %]
                [% IF ( NOPERMISSION ) %]
-               <div class="dialog alert">You do not have permission to edit this patron's login information.</div>
+               <li>You do not have permission to edit this patron's login information.</li>
                [% END %]
+               [% IF ( NOMATCH ) %]
+               <li><strong>The passwords entered do not match</strong>. Please re-enter the new password.</li>
+               [% END %]
+               </ul>
+               </div>
        [% END %]
 
 
        <fieldset class="brief"><legend>Change Username and/or Password for [% firstname %] [% surname %]</legend>
        <ol>
        <li><label for="newuserid">New Username:</label>
-       <input type="hidden" name="member" value="[% member %]" /><input type="text" id="newuserid" name="newuserid" size="20" value="[% userid %]" /></li>
+       <input type="hidden" name="member" value="[% borrowernumber %]" /><input type="text" id="newuserid" name="newuserid" size="20" value="[% userid %]" /></li>
        <li><label for="newpassword">New Password:</label>
-       <div class="hint">Koha cannot display existing passwords. Below is a randomly generated suggestion.  Leave the field blank to leave password unchanged.</div>
+       <div class="hint">Koha cannot display existing passwords. Leave the field blank to leave password unchanged.</div>
        [% IF ( minPasswordLength ) %]<div class="hint">Minimum password length: [% minPasswordLength %]</div>[% END %]
-       <input name="newpassword"  id="newpassword" type="text" size="20" value="[% defaultnewpassword %]" /></li>
+       [% IF ( NOMATCH ) %]
+       <input name="newpassword"  id="newpassword" type="password" size="20" class="focus" />
+       <input name="newpassword" id="newpassword_random" readonly="readonly" disabled="disabled" type="hidden" />
+       [% ELSE %]
+       <input name="newpassword"  id="newpassword" type="password" size="20" />
+       <input name="newpassword" readonly="readonly" disabled="disabled" type="hidden" />
+       [% END %]
+       </li>
+       <li><label for="newpassword2">Confirm New Password:</label>
+       <input name="newpassword2"  id="newpassword2" type="password" size="20" />
+       <input name="newpassword2" id="newpassword2_random" readonly="readonly" disabled="disabled" type="hidden" />
+       </li>
        </ol>
 </fieldset>
        <fieldset class="action"><input type="submit" value="Save" /> <a class="cancel" href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">Cancel</a></fieldset>
index 02d39f5..34fef89 100644 (file)
@@ -3,7 +3,7 @@
 [% IF ( opadd ) %]Add[% ELSIF ( opduplicate ) %]Duplicate[% ELSE %] Modify[% END %] [% IF ( categoryname ) %] [% categoryname %] patron[% ELSE %][% IF ( I ) %] Organization patron[% END %][% IF ( A ) %] Adult patron[% END %][% IF ( C ) %] Child patron[% END %][% IF ( P ) %] Professional patron[% END %][% IF ( S ) %] Staff patron[% END %][% END %][% UNLESS ( opadd ) %] [% surname %], [% firstname %][% END %]</title>
 [% INCLUDE 'doc-head-close.inc' %]
 [% INCLUDE 'calendar.inc' %]
-<script type="text/JavaScript" language="JavaScript">
+<script type="text/JavaScript">
 //<![CDATA[
     $(document).ready(function() {
                $("fieldset.rows input").keydown(function(e){ return checkEnter(e); });
@@ -60,6 +60,7 @@
         var MSG_LATE_EXPIRY = _("Warning: Expiration date falls before enrollment date");
         var MSG_MISSING_MANDATORY = _("The following fields are mandatory:");
         var MSG_DUPLICATE_SUSPICION = _("Please confirm whether this is a duplicate patron");
+        var MSG_PASSWORD_MISMATCH = _("The passwords entered do not match");
 //]]>
 </script>
 <script type="text/javascript" src="[% themelang %]/js/members.js"></script>
                        [% IF ( ERROR_short_password ) %]
                                <li id="ERROR_short_password">Password must be at least [% minPasswordLength %] characters long.</li>
                        [% END %]
+                       [% IF ( ERROR_password_mismatch ) %]
+                               <li id="ERROR_password_mismatch">Passwords do not match.</li>
+                       [% END %]
             [% IF ( ERROR_extended_unique_id_failed ) %]
                 <li id="ERROR_extended_unique_id_failed">The attribute value 
                     [% ERROR_extended_unique_id_failed %] is already is use by another patron record.</li>
                        [% IF ( opadd ) %]
                        [% IF ( NoUpdateLogin ) %]
                                [% IF ( opduplicate ) %]
-                                       <input type="text" id="password" name="password" size="20"  disabled="disabled" />
+                                       <input type="password" id="password" name="password" size="20"  disabled="disabled" />
                                [% ELSE %]
-                                       <input type="text" id="password" name="password" size="20"  disabled="disabled" value="[% password %]" />
+                                       <input type="password" id="password" name="password" size="20"  disabled="disabled" value="[% password %]" />
                                [% END %]
 [% ELSE %]
                                [% IF ( opduplicate ) %]
-                                       <input type="text" id="password" name="password" size="20" />
+                                       <input type="password" id="password" name="password" size="20" />
                                [% ELSE %]
-                                       <input type="text" id="password" name="password" size="20" value="[% password %]" />
+                                       <input type="password" id="password" name="password" size="20" value="[% password %]" />
                                [% END %]
 [% END %]
                        [% ELSE %]
                        [% IF ( password ) %]
                                [% IF ( NoUpdateLogin ) %]
-                                       <input type="text" id="password" name="password" size="20"  disabled="disabled" value="****" />
+                                       <input type="password" id="password" name="password" size="20"  disabled="disabled" value="****" />
                                [% ELSE %]
                                        [% IF ( opduplicate ) %]
-                                               <input type="text" id="password" name="password" size="20" />
+                                               <input type="password" id="password" name="password" size="20" />
                                        [% ELSE %]
-                                               <input type="text" id="password" name="password" size="20" value="****" />
+                                               <input type="password" id="password" name="password" size="20" value="****" />
                                        [% END %]
                                [% END %]
                        [% ELSE %]
                                [% IF ( NoUpdateLogin ) %]
-                                       <input type="text" id="password" name="password" size="20"  disabled="disabled" value="" />
+                                       <input type="password" id="password" name="password" size="20"  disabled="disabled" value="" />
                                [% ELSE %]
-                                       <input type="text" id="password" name="password" size="20" value="" />
+                                       <input type="password" id="password" name="password" size="20" value="" />
                                [% END %]
                        [% END %]
                        [% END %]
          [% IF ( mandatorypassword ) %]<span class="required">Required</span>[% END %][% IF ( ERROR_short_password ) %]<span class="required">Password is too short</span>[% END %]
 [% IF ( minPasswordLength ) %]<div class="hint">Minimum password length: [% minPasswordLength %]</div>[% END %]
                </li>
-        [% END %]
-    </ol>
+               <li>
+                       [% IF ( mandatorypassword ) %]
+                       <label for="password2" class="required">
+                       [% ELSE %]
+                       <label for="password2">
+                       [% END %]
+                       Confirm password: </label>
+                       [% IF ( opadd ) %]
+                       [% IF ( NoUpdateLogin ) %]
+                               [% IF ( opduplicate ) %]
+                                       <input type="password" id="password2" name="password2" size="20"  disabled="disabled" />
+                               [% ELSE %]
+                                       <input type="password" id="password2" name="password2" size="20"  disabled="disabled" value="[% password %]" />
+                               [% END %]
+[% ELSE %]
+                               [% IF ( opduplicate ) %]
+                                       <input type="password" id="password2" name="password2" size="20" />
+                               [% ELSE %]
+                                       <input type="password" id="password2" name="password2" size="20" value="[% password %]" />
+                               [% END %]
+[% END %]
+                       [% ELSE %]
+                       [% IF ( password ) %]
+                               [% IF ( NoUpdateLogin ) %]
+                                       <input type="password" id="password2" name="password2" size="20"  disabled="disabled" value="****" />
+                               [% ELSE %]
+                                       [% IF ( opduplicate ) %]
+                                               <input type="password" id="password2" name="password2" size="20" />
+                                       [% ELSE %]
+                                               <input type="password" id="password2" name="password2" size="20" value="****" />
+                                       [% END %]
+                               [% END %]
+                       [% ELSE %]
+                               [% IF ( NoUpdateLogin ) %]
+                                       <input type="password" id="password2" name="password2" size="20"  disabled="disabled" value="" />
+                               [% ELSE %]
+                                       <input type="password" id="password2" name="password2" size="20" value="" />
+                               [% END %]
+                       [% END %]
+                       [% END %]
+         [% IF ( mandatorypassword ) %]<span class="required">Required</span>[% END %][% IF ( ERROR_password_mismatch ) %]<span class="required">Passwords do not match</span>[% END %]
+               </li>
+               </ol>
                </fieldset>
         [% END # hide fieldset %]
                <!--this zones are not necessary in modif mode -->
 [% INCLUDE 'members-menu.inc' %]
 </div>[% END %]
 [% END %]
+[% END %]
 </div>
 [% INCLUDE 'intranet-bottom.inc' %]
 
index 8128a3d..fe24df3 100755 (executable)
@@ -40,17 +40,21 @@ $flagsrequired->{borrowers}=1;
 my $member=$input->param('member');
 my $cardnumber = $input->param('cardnumber');
 my $destination = $input->param('destination');
-my $errormsg;
+my @errors;
 my ($bor)=GetMember('borrowernumber' => $member);
 if(( $member ne $loggedinuser ) && ($bor->{'category_type'} eq 'S' ) ) {
-       $errormsg = 'NOPERMISSION' unless($staffflags->{'superlibrarian'} || $staffflags->{'staffaccess'} );
+       push(@errors,'NOPERMISSION') 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');
-$errormsg = 'SHORTPASSWORD' if( $newpassword && $minpw && (length($newpassword) < $minpw ) );
+push(@errors,'SHORTPASSWORD') if( $newpassword && $minpw && (length($newpassword) < $minpw ) );
 
-if ( $newpassword  && ! $errormsg ) {
+if ( $newpassword  && !scalar(@errors) ) {
     my $digest=md5_base64($input->param('newpassword'));
     my $uid = $input->param('newuserid');
     my $dbh=C4::Context->dbh;
@@ -62,13 +66,7 @@ if ( $newpassword  && ! $errormsg ) {
                    print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$member");
                }
     } else {
-                       $errormsg = 'BADUSERID';
-           $template->param(othernames => $bor->{'othernames'},
-                                               surname     => $bor->{'surname'},
-                                               firstname   => $bor->{'firstname'},
-                                               userid      => $bor->{'userid'},
-                                               defaultnewpassword => $newpassword 
-                                               );
+                       push(@errors,'BADUSERID');
     }
 } else {
     my $userid = $bor->{'userid'};
@@ -79,7 +77,9 @@ if ( $newpassword  && ! $errormsg ) {
     for (my $i=0; $i<$length; $i++) {
        $defaultnewpassword.=substr($chars, int(rand(length($chars))),1);
     }
-       
+
+       $template->param( defaultnewpassword => $defaultnewpassword );
+}
     if ( $bor->{'category_type'} eq 'C') {
         my  ( $catcodes, $labels ) =  GetborCatFromCatType( 'A', 'WHERE category_type = ?' );
         my $cnt = scalar(@$catcodes);
@@ -120,16 +120,16 @@ if (C4::Context->preference('ExtendedPatronAttributes')) {
            userid      => $bor->{'userid'},
            destination => $destination,
                is_child        => ($bor->{'category_type'} eq 'C'),
-           defaultnewpassword => $defaultnewpassword,
                activeBorrowerRelationship => (C4::Context->preference('borrowerRelationship') ne ''),
+        minPasswordLength => $minpw
        );
 
+if( scalar(@errors )){
+       $template->param( errormsg => 1 );
+       foreach my $error (@errors) {
+        $template->param($error) || $template->param( $error => 1);
+       }
 
 }
 
-$template->param( member => $member,
-                                       errormsg => $errormsg,
-                                       $errormsg => 1 ,
-                                       minPasswordLength => $minpw );
-
 output_html_with_http_headers $input, $cookie, $template->output;
index 6de07bf..86dd6ed 100755 (executable)
@@ -292,6 +292,8 @@ if ($op eq 'save' || $op eq 'insert'){
   }
   
   my $password = $input->param('password');
+  my $password2 = $input->param('password2');
+  push @errors, "ERROR_password_mismatch" if ( $password ne $password2 );
   push @errors, "ERROR_short_password" if( $password && $minpw && $password ne '****' && (length($password) < $minpw) );
 
   if (C4::Context->preference('ExtendedPatronAttributes')) {