Bug 19845 - Patron password is ignored during self-registration if PatronSelfRegistrationVerifyByEmail is enabled
Summary: Patron password is ignored during self-registration if PatronSelfRegistration...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Kyle M Hall
URL:
Whiteboard:
Keywords:
Depends on: 15343 18298
Blocks:
  Show dependency treegraph
 
Reported: 2017-12-19 21:36 UTC by Arturo
Modified: 2018-06-04 20:15 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19845: Do not generate a password if one is filled on selfreg (2.05 KB, patch)
2017-12-20 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19845: Do not generate a password if one is filled on selfreg (2.09 KB, patch)
2017-12-20 19:54 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 19845: Do not generate a password if one is filled on selfreg (2.14 KB, patch)
2017-12-21 12:16 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Arturo 2017-12-19 21:36:25 UTC
Bug 15343 allows patrons to choose their own passwords during self-registration. But this does not work if the PatronSelfRegistrationVerifyByEmail preference is set to "Require." If e-mail verification is required, whatever password the patron supplied in the form is ignored, and they are given a randomly generated password once they click on the verification link.

To test this behavior:

1. Make sure there is a valid e-mail stored in the KohaAdminEmailAddress preference.
2. Set PatronSelfRegistration to Allow.
3. Set PatronSelfRegistrationVerifyByEmail to Require.
3. Be sure "password" is listed in PatronSelfRegistrationBorrowerMandatoryField and NOT listed in PatronSelfRegistrationBorrowerUnwantedField.
4. Be sure there is a valid patron category in PatronSelfRegistrationDefaultCategory.
5. Set PatronSelfRegistrationPrefillForm to "Display and prefill" so that you can see the generated password.

Then fill out the self-registration form, include a valid e-mail address, and select a password. Wait for the verification e-mail. Click on the link and you'll see that the password you entered in the form is not used. Instead, Koha will have generated a random password for the account.
Comment 1 Jonathan Druart 2017-12-20 15:37:41 UTC
Created attachment 69950 [details] [review]
Bug 19845: Do not generate a password if one is filled on selfreg

Bug 15343 allows patrons to choose their own passwords during self-registration.
But this does not work if the PatronSelfRegistrationVerifyByEmail preference is
set to "Require." If e-mail verification is required, whatever password the patron
supplied in the form is ignored, and they are given a randomly generated password
once they click on the verification link.

Test plan:
1. Make sure there is a valid e-mail stored in the KohaAdminEmailAddress preference.
2. Set PatronSelfRegistration to Allow.
3. Set PatronSelfRegistrationVerifyByEmail to Require.
3. Be sure "password" is listed in PatronSelfRegistrationBorrowerMandatoryField and
NOT listed in PatronSelfRegistrationBorrowerUnwantedField.
4. Be sure there is a valid patron category in PatronSelfRegistrationDefaultCategory.
5. Set PatronSelfRegistrationPrefillForm to "Display and prefill" so that you can see
the generated password.

Then fill out the self-registration form, include a valid e-mail address, and select
a password. Wait for the verification e-mail. Click on the link and you'll see that
the password you entered in the form is used.
Comment 2 sandboxes@biblibre.com 2017-12-20 19:54:11 UTC
Patch tested with a sandbox, by Arturo <alongoria@sll.texas.gov>
Comment 3 sandboxes@biblibre.com 2017-12-20 19:54:32 UTC
Created attachment 69969 [details] [review]
Bug 19845: Do not generate a password if one is filled on selfreg

Bug 15343 allows patrons to choose their own passwords during self-registration.
But this does not work if the PatronSelfRegistrationVerifyByEmail preference is
set to "Require." If e-mail verification is required, whatever password the patron
supplied in the form is ignored, and they are given a randomly generated password
once they click on the verification link.

Test plan:
1. Make sure there is a valid e-mail stored in the KohaAdminEmailAddress preference.
2. Set PatronSelfRegistration to Allow.
3. Set PatronSelfRegistrationVerifyByEmail to Require.
3. Be sure "password" is listed in PatronSelfRegistrationBorrowerMandatoryField and
NOT listed in PatronSelfRegistrationBorrowerUnwantedField.
4. Be sure there is a valid patron category in PatronSelfRegistrationDefaultCategory.
5. Set PatronSelfRegistrationPrefillForm to "Display and prefill" so that you can see
the generated password.

Then fill out the self-registration form, include a valid e-mail address, and select
a password. Wait for the verification e-mail. Click on the link and you'll see that
the password you entered in the form is used.

Signed-off-by: Arturo <alongoria@sll.texas.gov>
Comment 4 Arturo 2017-12-20 20:03:21 UTC
Wow, thank you for submitting a patch so quickly, Jonathan! You are awesome.

I just tested your patch and it worked like a charm. I tested it out with the following preference configurations and they all were successful:

- Require e-mail verification and require user to select their own password
-- after verification, the user-supplied password was displayed and I was able to log in with it

- Require e-mail verification but do not allow user to select their own password
-- after verification, a random password was generated and displayed and I was able to log in with it

- Don't require e-mail verification and require user to select their own password
-- the user-supplied password was displayed and I was able to log in with it

- Don't require e-mail verification and do not allow user to select their own password
-- a random password was generated and displayed and I was able to log in with it
Comment 5 Arturo 2017-12-20 20:12:23 UTC
I also tested the scenario where a user can opt to select their own password but it is not a required field. Regardless of whether the e-mail verification preference is set to require, the expected behavior occurs. A random password is generated if the user leaves the fields blank. The user-supplied password is used if one is entered in the self-reg form.
Comment 6 Kyle M Hall 2017-12-21 12:16:31 UTC
Created attachment 70009 [details] [review]
Bug 19845: Do not generate a password if one is filled on selfreg

Bug 15343 allows patrons to choose their own passwords during self-registration.
But this does not work if the PatronSelfRegistrationVerifyByEmail preference is
set to "Require." If e-mail verification is required, whatever password the patron
supplied in the form is ignored, and they are given a randomly generated password
once they click on the verification link.

Test plan:
1. Make sure there is a valid e-mail stored in the KohaAdminEmailAddress preference.
2. Set PatronSelfRegistration to Allow.
3. Set PatronSelfRegistrationVerifyByEmail to Require.
3. Be sure "password" is listed in PatronSelfRegistrationBorrowerMandatoryField and
NOT listed in PatronSelfRegistrationBorrowerUnwantedField.
4. Be sure there is a valid patron category in PatronSelfRegistrationDefaultCategory.
5. Set PatronSelfRegistrationPrefillForm to "Display and prefill" so that you can see
the generated password.

Then fill out the self-registration form, include a valid e-mail address, and select
a password. Wait for the verification e-mail. Click on the link and you'll see that
the password you entered in the form is used.

Signed-off-by: Arturo <alongoria@sll.texas.gov>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Jonathan Druart 2017-12-21 16:25:08 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 8 Nick Clemens 2018-01-15 13:14:42 UTC
Awesome work all! Pushed to stable for 17.11.02
Comment 9 Fridolin SOMERS 2018-01-15 14:54:20 UTC
This line has been changed by Bug 18298: Add server-side checks and refactor stuffs
Note sure it can be applied on 17.05.x
Comment 10 Jonathan Druart 2018-01-15 15:37:02 UTC
(In reply to Fridolin SOMERS from comment #9)
> This line has been changed by Bug 18298: Add server-side checks and refactor
> stuffs
> Note sure it can be applied on 17.05.x

yes the bug exists in 17.05.x

The same fixes would be:

-            $borrower{password}           = random_string("..........");
+            $borrower{password}           = random_string("..........") unless $borrower{password};

The problem is that we cannot force the user to choose a strong password (and default min password length is 3)