Bug 15343 - Allow patrons to choose their own password on self registration.
Summary: Allow patrons to choose their own password on self registration.
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 19845
  Show dependency treegraph
 
Reported: 2015-12-09 16:22 UTC by Nick Clemens
Modified: 2017-12-20 15:29 UTC (History)
14 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15343 - Allow user to set password on opac-memberentry.pl (9.12 KB, patch)
2015-12-18 18:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15343 [QA Followup] - Fix issues (5.53 KB, patch)
2015-12-18 19:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15343 [QA Followup] - Fix issues (5.68 KB, patch)
2015-12-18 19:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15343 [QA Followup] - Fix issues (4.23 KB, patch)
2015-12-18 19:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15343 [QA Followup] - Fix issues (4.29 KB, patch)
2015-12-18 20:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15343 [QA Follow up] Fix captcha display (990 bytes, patch)
2016-01-15 15:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15343 [QA Follow up] Fix captcha display (1.56 KB, patch)
2016-01-15 21:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15343 - Allow user to set password on opac-memberentry.pl (9.18 KB, patch)
2016-01-26 22:32 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15343 [QA Followup] - Fix issues (4.34 KB, patch)
2016-01-26 22:32 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15343 [QA Follow up] Fix captcha display (1.62 KB, patch)
2016-01-26 22:32 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15343 - Allow user to set password on opac-memberentry.pl (9.24 KB, patch)
2016-01-28 20:03 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 15343 [QA Followup] - Fix issues (4.40 KB, patch)
2016-01-28 20:04 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 15343 [QA Follow up] Fix captcha display (1.67 KB, patch)
2016-01-28 20:04 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 15343 [QA Followup] (3.35 KB, patch)
2016-02-05 16:02 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15343 - Allow user to set password on opac-memberentry.pl (9.31 KB, patch)
2016-02-09 12:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15343 [QA Followup] - Fix issues (4.47 KB, patch)
2016-02-09 12:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15343 [QA Follow up] Fix captcha display (1.75 KB, patch)
2016-02-09 12:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15343 [QA Followup] (3.43 KB, patch)
2016-02-09 12:10 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2015-12-09 16:22:12 UTC
Currently patron passwords are auto-generated and a patron must either immediately log in to change their password and/or record them.  This patch would simply allow the patron to create their password during registration.

Work to be done:
* Extend AddMember_Opac to allow passing in the password.
* Add update to make password field hidden by default.
* Add password/confirm password fields and relevant error messages to opac-memberentry.pl.
Comment 1 Nick Clemens 2015-12-18 18:34:33 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2015-12-18 19:30:04 UTC
1) "Contact information" should be "Password"
2) Don't pass sysprefs from the script to the template, use Koha.Preference()
3) minPasswordLength is optional, we cannot always assume it is set
4) Password field when does not follow convention completely when mandatory
Comment 3 Kyle M Hall 2015-12-18 19:32:26 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2015-12-18 19:39:02 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2015-12-18 19:56:41 UTC Comment hidden (obsolete)
Comment 6 Nick Clemens 2015-12-18 20:05:00 UTC Comment hidden (obsolete)
Comment 7 Liz Rea 2016-01-07 01:05:55 UTC
With this patch, the verification box doesn't show, and you cannot complete a self registration. You will always get the message "You typed in the wrong characters in the box before submitting. Please try again."

I like the idea of this though.

Liz
Comment 8 Nick Clemens 2016-01-08 14:54:37 UTC
Hi Liz,

I tested in Firefox and Chrome and could not recreate this issue, can you check again?

Moving back to Needs Signoff

-Nick
Comment 9 Owen Leonard 2016-01-15 15:33:03 UTC
(In reply to Liz Rea from comment #7)
> With this patch, the verification box doesn't show, and you cannot complete
> a self registration. You will always get the message "You typed in the wrong
> characters in the box before submitting. Please try again."

Same for me. Iceweasel 38.5.0 and Chromium 47.0.2526.80 on Debian.

Here are my settings:

PatronSelfRegistration - "Allow"
PatronSelfRegistrationAdditionalInstructions - Empty
PatronSelfRegistrationBorrowerMandatoryField - surname|firstname
PatronSelfRegistrationBorrowerUnwantedField - password
PatronSelfRegistrationDefaultCategory - T
PatronSelfRegistrationExpireTemporaryAccountsDelay - 14
PatronSelfRegistrationVerifyByEmail - Require
Comment 10 Nick Clemens 2016-01-15 15:54:52 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2016-01-15 17:06:45 UTC
(In reply to Nick Clemens from comment #1)
> 8 - Register a patron with a password, your password should be used

At this step I'm getting an error:

DBIx::Class::Row::store_column(): No such column 'password2' on Koha::Schema::Result::BorrowerModification at /home/oleonard/kohaclone/Koha/Borrower/Modifications.pm line 65
Comment 12 Nick Clemens 2016-01-15 21:28:42 UTC Comment hidden (obsolete)
Comment 13 Jesse Weaver 2016-01-26 22:32:11 UTC Comment hidden (obsolete)
Comment 14 Jesse Weaver 2016-01-26 22:32:16 UTC Comment hidden (obsolete)
Comment 15 Jesse Weaver 2016-01-26 22:32:20 UTC Comment hidden (obsolete)
Comment 16 Mirko Tietgen 2016-01-28 19:24:03 UTC
I tried to test this, but I can't get the self registration to show in OPAC.
PatronSelfRegistration is set to allow. Am I missing something?
Comment 17 Nicole C. Engard 2016-01-28 19:46:00 UTC
(In reply to Mirko Tietgen from comment #16)
> I tried to test this, but I can't get the self registration to show in OPAC.
> PatronSelfRegistration is set to allow. Am I missing something?

You also need to set the PatronSelfRegistrationDefaultCategory

Nicole
Comment 18 Mirko Tietgen 2016-01-28 19:52:30 UTC
Thanks Nicole, that helped! Maybe it should be in the manual. :P
Comment 19 Mirko Tietgen 2016-01-28 20:03:03 UTC Comment hidden (obsolete)
Comment 20 Mirko Tietgen 2016-01-28 20:04:23 UTC Comment hidden (obsolete)
Comment 21 Mirko Tietgen 2016-01-28 20:04:30 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2016-02-01 11:52:35 UTC
Comment on attachment 47407 [details] [review]
Bug 15343 - Allow user to set password on opac-memberentry.pl

Review of attachment 47407 [details] [review]:
-----------------------------------------------------------------

There is a third case to check: if the password contain leading or trailing whitespaces, see bug 14509.

::: C4/Members.pm
@@ +2482,5 @@
>      $borrower{'cardnumber'} = fixup_cardnumber();
>  
>      my $borrowernumber = AddMember(%borrower);
>  
> +    return ( $borrowernumber, $borrower{'password'} );

Why do you return the password?
You should compare the hashed password (Koha::AuthUtils::hash_password) in the test.

::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
@@ +60,5 @@
>                                  [% IF field == "email" %]<li>Contact information: <a href="#borrower_email">primary email address</a></li>[% END %]
>                                  [% IF field == "emailpro" %]<li>Contact information: <a href="#borrower_emailpro">secondary email address</a></li>[% END %]
>                                  [% IF field == "B_email" %]<li>Alternate address information: <a href="#borrower_B_email">email address</a></li>[% END %]
> +                                [% IF field == "password_match" %]<li>Passwords do not match! <a href="#password">password</a></li>[% END %]
> +                                [% IF field == "password_invalid" %]<li>Password does not meet minium requirements! <a href="#password">password</a></li>[% END %]

typo minimum.
Comment 23 Nick Clemens 2016-02-01 21:07:27 UTC
(In reply to Jonathan Druart from comment #22)
> > +    return ( $borrowernumber, $borrower{'password'} );
> 
> Why do you return the password?

This was already done before my change. 

The password is returned in the case of an auto generated password so that the patron may know their password.  

I suppose it does not need to be captured and displayed if a password is submitted, but in the other case I think it must remain

Will work on that and the others ASAP
Comment 24 Jonathan Druart 2016-02-02 09:50:35 UTC
(In reply to Nick Clemens from comment #23)
> (In reply to Jonathan Druart from comment #22)
> > > +    return ( $borrowernumber, $borrower{'password'} );
> > 
> > Why do you return the password?
> 
> This was already done before my change. 

Ha yes, I misread the diff!
Fix not needed :)
Comment 25 Nick Clemens 2016-02-05 16:02:46 UTC Comment hidden (obsolete)
Comment 26 Jonathan Druart 2016-02-09 12:10:38 UTC
Created attachment 47794 [details] [review]
Bug 15343 - Allow user to set password on opac-memberentry.pl

This patch allows patrons to create thier own password during OPAC self
registration.  It sets the password field to be hidden by default, and
allows for a system generated password if no password is supplied and
field is not mandatory

1 - Apply patch
2 - run updatedatabase.pl
3 - Check syspref "PatronSelfRegistrationBorrowerUnwantedField" - it
should contain 'password'
4 - Check self registration on opac - there should be no change
5 - Remove 'password' from PatronSelfRegistrationBorrowerUnwantedField
6 - Password field should now be visible in OPAC self registration
7 - Register a patron: If no passwor is supplied you should get a system
generated password
8 - Register a patron with a password, your password should be used
9 - Add 'password' to PatronSelfRegistrationBorrowerMnadatoryField
10 - Attempt to register a patron with no password - it should fail
11 - Try all combinations if mismatched/short/missing passwords and view
correct error messages
12 - prove t/db_dependent/Members.t

Sponsored by : Do Space (www.dospace.org)

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 27 Jonathan Druart 2016-02-09 12:10:43 UTC
Created attachment 47795 [details] [review]
Bug 15343 [QA Followup] - Fix issues

1) "Contact information" should be "Password"
2) Don't pass sysprefs from the script to the template, use Koha.Preference()
3) minPasswordLength is optional, we cannot always assume it is set
4) Password field when does not follow convention completely when mandatory

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 28 Jonathan Druart 2016-02-09 12:10:47 UTC
Created attachment 47796 [details] [review]
Bug 15343 [QA Follow up] Fix captcha display

Make captcha visible even if password field is hidden
Remove password confirmation field before submitting modification

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 29 Jonathan Druart 2016-02-09 12:10:52 UTC
Created attachment 47797 [details] [review]
Bug 15343 [QA Followup]

* Check for leading/trailing spaces in password
* Check hashed password in tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 30 Brendan Gallagher 2016-02-24 04:23:08 UTC
Pushed to Master - Should be in the May 2016 Release.  Thanks!
Comment 31 Christopher Brannon 2016-06-16 19:06:53 UTC
Any chance this can be ported to make it in 3.22.8?

Christopher
Comment 32 Christopher Brannon 2016-07-20 21:09:09 UTC
What version will this feature be available in?
Comment 33 Nick Clemens 2016-07-20 21:17:12 UTC
(In reply to Christopher Brannon from comment #32)
> What version will this feature be available in?

Hi Christopher, 

This should be in 16.05 (the version following 3.22), as an enhancement it is not likely to be backported to any earlier versions.

-Nick