Bugzilla – Attachment 6172 Details for
Bug 5280
Fix password field in members.pl so that the password is masked as it is entered
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Fix for Bug 5280 - Fix password field so that the password is masked as it is entered
Fix-for-Bug-5280---Fix-password-field-so-that-the-.patch (text/plain), 16.58 KB, created by
Chris Cormack
on 2011-11-04 00:58:30 UTC
(
hide
)
Description:
Fix for Bug 5280 - Fix password field so that the password is masked as it is entered
Filename:
MIME Type:
Creator:
Chris Cormack
Created:
2011-11-04 00:58:30 UTC
Size:
16.58 KB
patch
obsolete
>From 53f6189a13aeab7fb38648671b154e26eb18bea9 Mon Sep 17 00:00:00 2001 >From: Owen Leonard <oleonard@myacpl.org> >Date: Wed, 5 Oct 2011 12:04:12 -0400 >Subject: [PATCH] Fix for Bug 5280 - Fix password field so that the password > is masked as it is entered > >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. > >Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> > >Works well, kudos Owen >--- > koha-tmpl/intranet-tmpl/prog/en/js/members.js | 11 +++ > .../prog/en/modules/members/member-password.tt | 52 +++++++++++++-- > .../prog/en/modules/members/memberentrygen.tt | 69 ++++++++++++++++--- > members/member-password.pl | 36 +++++----- > members/memberentry.pl | 2 + > 5 files changed, 134 insertions(+), 36 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/js/members.js b/koha-tmpl/intranet-tmpl/prog/en/js/members.js >index 796db0a..c4c6e6c 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/js/members.js >+++ b/koha-tmpl/intranet-tmpl/prog/en/js/members.js >@@ -128,6 +128,17 @@ function check_form_borrowers(nav){ > } > } > } >+ >+ if ( document.form.password.value != document.form.password2.value ){ >+ if ( message_champ != '' ){ >+ message_champ += "\n"; >+ } else { >+ message = ""; >+ } >+ 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)){ >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 93b1828..1269270 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 >@@ -1,6 +1,25 @@ > [% INCLUDE 'doc-head-open.inc' %] > <title>Koha › Patrons › [% 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' %] >@@ -20,31 +39,50 @@ > > [% 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> >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 47ea6c9..48a9030 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >@@ -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> >@@ -143,6 +144,9 @@ > [% 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> >@@ -1069,39 +1073,82 @@ > [% 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></ol> >+ </li> >+ <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> > <!--this zones are not necessary in modif mode --> > [% UNLESS ( opadd ) %] >diff --git a/members/member-password.pl b/members/member-password.pl >index 25e9551..dce3e78 100755 >--- a/members/member-password.pl >+++ b/members/member-password.pl >@@ -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,15 +120,15 @@ if (C4::Context->preference('ExtendedPatronAttributes')) { > userid => $bor->{'userid'}, > destination => $destination, > is_child => ($bor->{'category_type'} eq 'C'), >- defaultnewpassword => $defaultnewpassword, >+ 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; >diff --git a/members/memberentry.pl b/members/memberentry.pl >index f01261e..cbb479a 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -274,6 +274,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')) { >-- >1.7.5.4
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 5280
:
6013
|
6172
|
6265
|
6268
|
6272
|
7381
|
7473
|
7488