This is standard practice with password entry on forms.
Changing it to a password type input is easy enough, but if you're going to mask the input you have to offer another field to re-type the password. Otherwise there's no way for the user to know they've entered it correctly.
I've got a work in progress for this, which adds second password-confirmation entry field on both the change password page and the patron entry page. The main blocker to adding this is that I don't see how the update to the change password page (member-password.pl) can work with the "suggest random password" feature currently in place. If we accept the random password suggestion it's masked by the password-type input field. What good is entering a password if the patron can't know what it is? Is the "suggest random password" feature worth keeping?
Created attachment 6013 [details] [review] Proposed fix 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. Here's very brief screencast showing the two options (sorry, Flash required): http://screencast.com/t/zi1Tb03B7fBu
bug affected to Koha 3.6. Entries must be attached to rel_3_8 only when the patch is pushed here (and if it's an ENH)
Created attachment 6172 [details] [review] 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
This is primarily a template change; the changes to the perl are brief and easily readable. Implements a needed security feature for double-entry. Random generation still supported. Marking as Passed QA
I've found a small bug when testing : if you enter a different password on the 2 forms, you get: undefined undefined The passwords entered do not match Marking failed QA. Owen, when you submit your follow-up, I think you can go directly "passed QA", it's a small glitch
Paul, I can't reproduce this error. Which page is it on? What browser are you using?
Created attachment 6265 [details] "undefined" error Owen, it's on memberentry.pl, when adding a patron (category ADO here, but same with other categories) Here is the screenshot
Created attachment 6268 [details] [review] Revised fix Revision lets members.js correctly handle errors when there are no mandatory fields defined.
Created attachment 6272 [details] [review] 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. Follow-up fix lets the members.js correctly handling errors when there are no mandatory fields Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
QA comment: I get a "Debug is on (level 1)" just below the "Modify Adult patron xxxx" (I have DEBUG set to yes in my setup) Is it because DEBUG=1 or a test that should be removed ?
(In reply to comment #12) > QA comment: > I get a "Debug is on (level 1)" > just below the "Modify Adult patron xxxx" > (I have DEBUG set to yes in my setup) > Is it because DEBUG=1 or a test that should be removed ? Owen / chris, bumping my question !
That debug messages dates back to 2007 and commit c102b3c6f5197fc695cadc1941d3331889fd6561
Paul: Could you update the status of this bug report? It seems to be pending as a result of your comments.
Marcel: Owen answered my question, you can QA this patch if you want/can
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
Created attachment 7381 [details] [review] 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. Follow-up fix lets the members.js correctly handling errors when there are no mandatory fields Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Sorry, but patch does not apply anymore. I tried to fix the conflict on members/member-password.pl, but a few pages after, I got: Template process failed: file error - parse error - /home/paul/koha.dev/koha-community/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt line 1477: unexpected end of input at /home/paul/koha.dev/koha-community/C4/Templates.pm line 126. So I suspect there's something wrong, either in your patch or git make an undetected mistake (no conflict but wrong things)
Created attachment 7473 [details] [review] 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. Follow-up fix lets the members.js correctly handling errors when there are no mandatory fields
I'm not sure how to handle patch rebases to keep the sign-off, or even if that is appropriate? A rebase could certainly introduce new bugs, so it might warrant another signoff. I'll leave this "Needs signoff" and let QA decide.
I got this patch to apply cleanly, but when I go to add a patron, I get the following: Template process failed: file error - parse error - /home/liz/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt line 1480: unexpected end of input at /home/liz/kohaclone/C4/Templates.pm line 126.
I was able to fix this - will test and submit the fixed patch as my signoff - the fix was trivial.
Created attachment 7488 [details] [review] [Signed Off] 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. 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).
QA comment: * nothing wrong in the feature itself * template still translatable after the patch * switches permission control on member-password to a list of errors, instead of just one, that's good passed QA
Note to RMaint: not sure this feature should be in 3.6.x or no: it's an important change in the behaviour and, even if ppl will be happy to see it, I think it's more something for a major upgrade: less experienced librarians will be surprised and troubled by the change. Switching to ENH and 3.8. If someone disagree, arguments welcomed.
(In reply to comment #13) > (In reply to comment #12) > > QA comment: > > I get a "Debug is on (level 1)" > > just below the "Modify Adult patron xxxx" > > (I have DEBUG set to yes in my setup) > > Is it because DEBUG=1 or a test that should be removed ? > > Owen / chris, bumping my question ! I'm still confused by this :/. I'm getting 3 "Use of uninitialized value in concatenation (.) or string" errors (for "dateofbirth: \tdateenrolled: \tdateexpiry", I think). I'm also getting 3 "xsub: [(($6||0),($5||0),($4||0),$3, $2 - 1, $1 - 1900)]" errors. I don't see any fundamental changes from 3.2 in regards to these fields, so I'm at a loss for why this "Debug is on (level 1)" message is still being displayed in memberentrygen.pl, even when DebugLevel is set to "no" :S. Any thoughts?
(In reply to comment #27) > I'm getting 3 "Use of uninitialized value in concatenation (.) or string" > errors (for "dateofbirth: \tdateenrolled: \tdateexpiry", I think). > > I'm also getting 3 "xsub: [(($6||0),($5||0),($4||0),$3, $2 - 1, $1 - 1900)]" > errors. Are these still problems? I have heard no other reports about it.
(In reply to comment #28) > (In reply to comment #27) > > I'm getting 3 "Use of uninitialized value in concatenation (.) or string" > > errors (for "dateofbirth: \tdateenrolled: \tdateexpiry", I think). > > > > I'm also getting 3 "xsub: [(($6||0),($5||0),($4||0),$3, $2 - 1, $1 - 1900)]" > > errors. > > Are these still problems? I have heard no other reports about it. Well, the warning messages themselves aren't really a problem, but there is still this "Debug is on (level 1)" message on the memberentry.pl page which is irritating. It even appears when DebugLevel is turned off :/.
(In reply to comment #28) > (In reply to comment #27) > > I'm getting 3 "Use of uninitialized value in concatenation (.) or string" > > errors (for "dateofbirth: \tdateenrolled: \tdateexpiry", I think). > > > > I'm also getting 3 "xsub: [(($6||0),($5||0),($4||0),$3, $2 - 1, $1 - 1900)]" > > errors. > > Are these still problems? I have heard no other reports about it. However, since it's most likely not related to this bug, I'll file a new report for it.