Bug 27148

Summary: Internal Server Error during self registration 20.11
Product: Koha Reporter: JanMueck <jan.michael.mueck>
Component: OPACAssignee: Agustín Moyano <agustinmoyano>
Status: CLOSED FIXED QA Contact: Nick Clemens <nick>
Severity: critical    
Priority: P5 - low CC: andrewfh, caroline.cyr-la-rose, david, dcook, fridolin.somers, jonathan.druart, martin.renvoize, mtj, nick
Version: masterKeywords: rel_20_11_candidate
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27626
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes a bug when using self registration and there is no patron category available for selection in the registration form.
Version(s) released in:
21.05.00,20.11.01
Bug Depends on: 23816    
Bug Blocks:    
Attachments: Bug 27148: Fix bug on self registration when user category is hidden
Bug 27148: Fix bug on self registration when user category is hidden
Bug 27148: Fix bug on self registration when user category is hidden
Bug 27148: (follow-up) Fix require_strong variable set
Bug 27148: Fix bug on self registration when user category is hidden
Bug 27148: (follow-up) Fix require_strong variable set
Bug 27148: (QA follow-up) Calculate default_strong_password correctly

Description JanMueck 2020-12-03 14:30:28 UTC
Internal Server Error during self registration if categorycode is checked in PatronSelfRegistrationBorrowerUnwantedField. Error could be reproduced. Installation was updated from 19.05 where is worked fine.

IRC Chat: 
[14:20] <Guest7457> Hello, i just updatet to 20.11. and try to register a new patreon in opac. Upon submitting the form i get an internal server error. Plack-Error.log shows "[You must provide a patron's category to validate password's strength and length]". In the Form there is a message to provide a password with undefind lenght. In the administrator setting the passwor lenght is defindet as 4 letters minimum. Can anyone help?

[14:31] <cait1> do you have patron categories defiend in your system? is the field hidden from the patron form maybe?
[15:12] <Guest7457> @cait1 We have got some categorys defined but can you tell me where i can check the visibility setting? The setup worked fine before the update btw.
[15:12] <huginn> Guest7457: I'll give you the answer just as soon as RDA is ready
[15:12] <wahanui> i already had it that way, huginn.
[15:18] <Joubu> Guest7457: can you open a bug report please?
[15:18] <Joubu> I do recreate the problem if PatronSelfRegistrationBorrowerUnwantedField contains "categorycode"


About Koha: 
Koha-Version: 	20.11.00.000
OS Version ('uname -a'): 	Linux www2 5.4.0-54-generic #60-Ubuntu SMP Fri Nov 6 10:37:59 UTC 2020 x86_64
Perl Interpreter: 	/usr/bin/perl
Perl-Version: 	5.030000
Perl @INC: 	/usr/share/koha/lib
/usr/share/koha/installer
/usr/share/koha/lib/installer
/etc/perl
/usr/local/lib/x86_64-linux-gnu/perl/5.30.0
/usr/local/share/perl/5.30.0
/usr/lib/x86_64-linux-gnu/perl5/5.30
/usr/share/perl5
/usr/lib/x86_64-linux-gnu/perl/5.30
/usr/share/perl/5.30
/usr/local/lib/site_perl
/usr/lib/x86_64-linux-gnu/perl-base
/var/lib/koha/testothek/plugins
MySQL-Version: 	mysql Ver 15.1 Distrib 10.3.25-MariaDB, for debian-linux-gnu (x86_64) using readline 5.2
Apache-Version: 	Server version: Apache/2.4.41 (Ubuntu)
PSGI: 	Plack (deployment)
Memcached: 	Server: 127.0.0.1:11211 | Namespace: koha_testothek | Status: wird ausgeführt. | Konfiguration gelesen aus: koha-conf.xml
Zebraversion: 	Zebra 2.1.4 (C) 1994-2017, Index Data Zebra is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. SHA1 ID: 45c729bb18ecdd3e0455f47765700b1c9754b1ce Using ICU
Message Broker: 	Status: wird ausgeführt
Datum und Zeit: 	03.12.2020 15:28
Comment 1 Jonathan Druart 2020-12-03 14:32:10 UTC
*** Bug 27147 has been marked as a duplicate of this bug. ***
Comment 2 Agustín Moyano 2020-12-03 14:47:51 UTC
This bug started because password strength and length now it's defined by patron's category.

As Jonathan told me, we've got PatronSelfRegistrationDefaultCategory preference. So I believe we should check if a patron category is visible in the form. 

If not visible, we should check fi PatronSelfRegistrationDefaultCategory is set with a valid patron category, and get password requirements from it.

If PatronSelfRegistrationDefaultCategory is not set, then we should fetch system's default password strength and length
Comment 3 Jonathan Druart 2020-12-03 14:50:58 UTC
(In reply to Agustín Moyano from comment #2)
> If PatronSelfRegistrationDefaultCategory is not set, then we should fetch
> system's default password strength and length

The self registration feature is disabled is this pref is not set. So we don't need to take this case into account.
Comment 4 Agustín Moyano 2020-12-03 16:32:51 UTC
Created attachment 114154 [details] [review]
Bug 27148: Fix bug on self registration when user category is hidden

This patch fixes the bug when there is no patron category available for
selection in self registration form. It uses PatronSelfRegistrationDefaultCategory
to get patron's password length and strength.

To test:
1. Do not apply this patch
2. Allow SeflRegistration, set PatronSelfRegistrationDefaultCategory to
   a valid patron category and finally add categorycode to PatronSelfRegistrationBorrowerUnwantedField
3. Go to opac and enter self registration form
CHECK => There is no patron category available for selection
      => Password's info alert shows "Password must contain at least
undefined characters"
4. Fill the form setting a password and send.
CHECH => You get an exception saying "You must provide a patron's
category to validate password's strength and length"
5. Apply this patch and restart_all
6. repeat steps 3 and 4
SUCCESS => Password's info alert message shows a number instead of
"undefined"
        => Password's strength and length checks are working
        => If you change default category's lenght or strength parameter it get's reflected when you refresh the page
        => When you click send, patron is saved
Comment 5 David Nind 2020-12-03 18:43:37 UTC
Created attachment 114161 [details] [review]
Bug 27148: Fix bug on self registration when user category is hidden

This patch fixes the bug when there is no patron category available for
selection in self registration form. It uses PatronSelfRegistrationDefaultCategory
to get patron's password length and strength.

To test:
1. Do not apply this patch
2. Allow PatronSelfRegistration, set PatronSelfRegistrationDefaultCategory to
   a valid patron category and finally add categorycode to PatronSelfRegistrationBorrowerUnwantedField
3. Go to opac and enter self registration form
CHECK => There is no patron category available for selection
      => Password's info alert shows "Password must contain at least
undefined characters"
4. Fill the form setting a password and send.
CHECH => You get an exception saying "You must provide a patron's
category to validate password's strength and length"
5. Apply this patch and restart_all
6. repeat steps 3 and 4
SUCCESS => Password's info alert message shows a number instead of
"undefined"
        => Password's strength and length checks are working
        => If you change default category's lenght or strength parameter it get's reflected when you refresh the page
        => When you click submit, patron is saved

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2020-12-03 18:53:34 UTC
Notes from testing (koha-testing-docker (KTD)):

- Step 2: should be  PatronSelfRegistration rather than SeflRegistration, is set to allow by default in KTD; PatronSelfRegistrationDefaultCategory is set to SELFREG as the patron category in KTD

- Step 6:
  . to change the password settings for the patron category go to Administration >  Patrons and circulation > Patron category and edit the SELFREG category; combinations of settings to experiment with are Minimum password length and Require strong password 
  . system preferences for password security are minPasswordLength and RequireStrongPassword

Everything works as expected with the test plan.
Comment 7 Agustín Moyano 2020-12-03 19:16:20 UTC
(In reply to David Nind from comment #6)
> Notes from testing (koha-testing-docker (KTD)):
> 
> - Step 2: should be  PatronSelfRegistration rather than SeflRegistration, is
> set to allow by default in KTD; PatronSelfRegistrationDefaultCategory is set
> to SELFREG as the patron category in KTD
> 
> - Step 6:
>   . to change the password settings for the patron category go to
> Administration >  Patrons and circulation > Patron category and edit the
> SELFREG category; combinations of settings to experiment with are Minimum
> password length and Require strong password 
>   . system preferences for password security are minPasswordLength and
> RequireStrongPassword
> 
> Everything works as expected with the test plan.

Thanks David for this comments, and testing
Comment 8 Martin Renvoize 2020-12-07 16:44:34 UTC
Created attachment 114235 [details] [review]
Bug 27148: Fix bug on self registration when user category is hidden

This patch fixes the bug when there is no patron category available for
selection in self registration form. It uses PatronSelfRegistrationDefaultCategory
to get patron's password length and strength.

To test:
1. Do not apply this patch
2. Allow SeflRegistration, set PatronSelfRegistrationDefaultCategory to
   a valid patron category and finally add categorycode to PatronSelfRegistrationBorrowerUnwantedField
3. Go to opac and enter self registration form
CHECK => There is no patron category available for selection
      => Password's info alert shows "Password must contain at least
undefined characters"
4. Fill the form setting a password and send.
CHECH => You get an exception saying "You must provide a patron's
category to validate password's strength and length"
5. Apply this patch and restart_all
6. repeat steps 3 and 4
SUCCESS => Password's info alert message shows a number instead of
"undefined"
        => Password's strength and length checks are working
        => If you change default category's lenght or strength parameter it get's reflected when you refresh the page
        => When you click send, patron is saved

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2020-12-07 16:44:59 UTC
Works as expected, no regressions found, passes QA scripts.

Passing QA.
Comment 10 Jonathan Druart 2020-12-09 10:02:08 UTC
+                var require_strong = $('select#borrower_categorycode option:selected').data('pwdStrong')||default_strong_password;

This does not work if the category does not have "Require strong password". If the default from syspref is 1, require_strong will always be 1.
Comment 11 Agustín Moyano 2020-12-09 18:20:12 UTC
Created attachment 114290 [details] [review]
Bug 27148: (follow-up) Fix require_strong variable set
Comment 12 Nick Clemens 2020-12-21 16:11:19 UTC
Created attachment 114563 [details] [review]
Bug 27148: Fix bug on self registration when user category is hidden

This patch fixes the bug when there is no patron category available for
selection in self registration form. It uses PatronSelfRegistrationDefaultCategory
to get patron's password length and strength.

To test:
1. Do not apply this patch
2. Allow SeflRegistration, set PatronSelfRegistrationDefaultCategory to
   a valid patron category and finally add categorycode to PatronSelfRegistrationBorrowerUnwantedField
3. Go to opac and enter self registration form
CHECK => There is no patron category available for selection
      => Password's info alert shows "Password must contain at least
undefined characters"
4. Fill the form setting a password and send.
CHECH => You get an exception saying "You must provide a patron's
category to validate password's strength and length"
5. Apply this patch and restart_all
6. repeat steps 3 and 4
SUCCESS => Password's info alert message shows a number instead of
"undefined"
        => Password's strength and length checks are working
        => If you change default category's lenght or strength parameter it get's reflected when you refresh the page
        => When you click send, patron is saved

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Nick Clemens 2020-12-21 16:11:23 UTC
Created attachment 114564 [details] [review]
Bug 27148: (follow-up) Fix require_strong variable set
Comment 14 Nick Clemens 2020-12-21 16:11:26 UTC
Created attachment 114565 [details] [review]
Bug 27148: (QA follow-up) Calculate default_strong_password correctly

Fix a typo lenght for length

If category strong password is not set, then we fall back to the preference, but that can be "" - which should be treated as 0
Comment 15 Jonathan Druart 2020-12-26 17:15:20 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 16 Fridolin Somers 2020-12-28 14:55:52 UTC
Pushed to 20.11.x for 20.11.01
Comment 17 Andrew Fuerste-Henry 2020-12-28 21:37:07 UTC
Missing dependencies, not backported to 20.05