Bug 23816 - Allow to have different password strength and length settings for different patron categories
Summary: Allow to have different password strength and length settings for different p...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Agustín Moyano
QA Contact: Testopia
URL:
Keywords:
Depends on: 25086
Blocks: 12617 27148 27430
  Show dependency treegraph
 
Reported: 2019-10-15 17:27 UTC by Joy Nelson
Modified: 2022-06-06 20:25 UTC (History)
9 users (show)

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


Attachments
Bug 23816: Update kohastructure and atomic update (2.73 KB, patch)
2020-04-16 21:15 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: [DO NOT PUSH] Update category schema (1.52 KB, patch)
2020-04-16 21:15 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: (follow-up) Define boolean column in shcema (679 bytes, patch)
2020-04-16 21:15 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Add tests (7.10 KB, patch)
2020-04-16 21:16 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Add minimum password length and require strong password overrides by category (46.20 KB, patch)
2020-04-16 21:16 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Fix other tests (13.80 KB, patch)
2020-04-16 21:16 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Update kohastructure and atomic update (2.78 KB, patch)
2020-04-16 22:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: [DO NOT PUSH] Update category schema (1.57 KB, patch)
2020-04-16 22:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: (follow-up) Define boolean column in shcema (726 bytes, patch)
2020-04-16 22:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Add tests (7.15 KB, patch)
2020-04-16 22:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Add minimum password length and require strong password overrides by category (46.25 KB, patch)
2020-04-16 22:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Fix other tests (13.85 KB, patch)
2020-04-16 22:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Update kohastructure and atomic update (2.84 KB, patch)
2020-04-27 21:21 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23816: [DO NOT PUSH] Update category schema (1.64 KB, patch)
2020-04-27 21:21 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23816: (follow-up) Define boolean column in shcema (793 bytes, patch)
2020-04-27 21:21 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23816: Add tests (7.22 KB, patch)
2020-04-27 21:21 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23816: Add minimum password length and require strong password overrides by category (46.43 KB, patch)
2020-04-27 21:21 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23816: Fix other tests (13.91 KB, patch)
2020-04-27 21:21 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23816: Update kohastructure and atomic update (2.84 KB, patch)
2020-07-21 15:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: [DO NOT PUSH] Update category schema (1.23 KB, patch)
2020-07-21 15:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: (follow-up) Define boolean column in shcema (793 bytes, patch)
2020-07-21 15:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Add tests (7.21 KB, patch)
2020-07-21 15:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Add minimum password length and require strong password overrides by category (47.20 KB, patch)
2020-07-21 15:53 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Fix other tests (13.53 KB, patch)
2020-07-21 15:53 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Update kohastructure and atomic update (2.90 KB, patch)
2020-08-18 21:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: [DO NOT PUSH] Update category schema (1.29 KB, patch)
2020-08-18 21:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: (follow-up) Define boolean column in shcema (851 bytes, patch)
2020-08-18 21:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: Add tests (7.28 KB, patch)
2020-08-18 21:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: Add minimum password length and require strong password overrides by category (47.37 KB, patch)
2020-08-18 21:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: Fix other tests (13.58 KB, patch)
2020-08-18 21:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: Set default value in tests (897 bytes, patch)
2020-08-19 07:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23816: trivial code simplification (2.42 KB, patch)
2020-08-19 07:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23816: Set default value in tests (1008 bytes, patch)
2020-08-27 16:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: trivial code simplification (2.53 KB, patch)
2020-08-27 16:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: (follow-up) Fix many things (18.91 KB, patch)
2020-08-27 16:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23816: Update kohastructure and atomic update (2.90 KB, patch)
2020-08-27 18:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: [DO NOT PUSH] Update category schema (1.29 KB, patch)
2020-08-27 18:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: (follow-up) Define boolean column in shcema (851 bytes, patch)
2020-08-27 18:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: Add tests (7.28 KB, patch)
2020-08-27 18:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: Add minimum password length and require strong password overrides by category (47.37 KB, patch)
2020-08-27 18:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: Set default value in tests (1.04 KB, patch)
2020-08-27 18:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: trivial code simplification (2.59 KB, patch)
2020-08-27 18:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: (follow-up) Fix many things (19.00 KB, patch)
2020-08-27 18:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: (QA follow-up) Use existing form validation to validate min password length (2.14 KB, patch)
2020-08-27 18:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23816: Update kohastructure and atomic update (2.80 KB, patch)
2020-09-02 12:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23816: [DO NOT PUSH] Update category schema (1.80 KB, patch)
2020-09-02 12:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23816: (follow-up) Define boolean column in shcema (906 bytes, patch)
2020-09-02 12:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23816: Add tests (7.32 KB, patch)
2020-09-02 12:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23816: Add minimum password length and require strong password overrides by category (47.42 KB, patch)
2020-09-02 12:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23816: Set default value in tests (1.09 KB, patch)
2020-09-02 12:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23816: trivial code simplification (2.64 KB, patch)
2020-09-02 12:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23816: (follow-up) Fix many things (19.02 KB, patch)
2020-09-02 12:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23816: (QA follow-up) Use existing form validation to validate min password length (2.19 KB, patch)
2020-09-02 12:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23816: Fix patron edition (1.48 KB, patch)
2020-09-10 07:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23816: Fix selenium tests (2.78 KB, patch)
2020-09-10 08:07 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 Joy Nelson 2019-10-15 17:27:20 UTC
Allow specified categories to be exempt from the system preferences of "Require Strong Password" and "Minimum Password Length".  The goal is to force staff to have long complex passwords while simultaneously allowing patrons to have non-strong, shorter passwords if they wish.
Comment 1 Katrin Fischer 2019-10-15 18:49:30 UTC
If you make them exempt, they could get away with '1' character passwords. I'd like to suggest that we leave the preferences global, but allow an overwrite on patron category level (we have other examples for this).

The default on patron category would be "inherit from pref", but allow to set or unset.
Comment 2 Agustín Moyano 2020-04-16 21:15:49 UTC
Created attachment 103099 [details] [review]
Bug 23816: Update kohastructure and atomic update
Comment 3 Agustín Moyano 2020-04-16 21:15:52 UTC
Created attachment 103100 [details] [review]
Bug 23816: [DO NOT PUSH] Update category schema
Comment 4 Agustín Moyano 2020-04-16 21:15:56 UTC
Created attachment 103101 [details] [review]
Bug 23816: (follow-up) Define boolean column in shcema
Comment 5 Agustín Moyano 2020-04-16 21:16:00 UTC
Created attachment 103102 [details] [review]
Bug 23816: Add tests
Comment 6 Agustín Moyano 2020-04-16 21:16:04 UTC
Created attachment 103103 [details] [review]
Bug 23816: Add minimum password length and require strong password overrides by category

This patch adds the capability to override minPasswordLenth and RequireStrongPassword settings by category

To test:
1. koha-shell kohadev
2. koha-mysql kohadev

3. drop database koha_kohadev;
4. create database koha_kohadev;

5. go to admin page and start webinstaller. There continue the steps until onboarding.
6. reach step 3 of onboarding and create a new administrator patron
CHECH => Password control woks as normal (Minimum length 3 and strong required)

7. finish Koha installation and enter admin with your new administrator
8. set minPasswordLength to 3 and RequireStrongPassword to “Don’t require”
9. Create a new category (CAT2 from now on.. CAT1 is the category you made in onboarding process) and set minimum password length to 8 and require strong password
10. Create two new patrons, one with CAT1(patron1) and one with CAT2 (patron2)
CHECK => In both cases, try different combinations of length and strength. For patron1 the only requirement is to have 3 letters, but for patron2 the minimum length will be 8 and will require strong password.
CHECK => Try changing patron category before saving. Password requirements will change with category change.

11. Edit CAT1 and set minimum password length to 5
12. Go to patron1 details page, and change password.
CHECH => Now password minimum length is 5, but still it doesn’t require strong password

13. Edit CAT1, leave blank minimum password length and set require strong password to yes.
14. Go to patron1 details page, and change password.
CHECH => Password minimum length is back to 3, but now strong password is required

15. Set minimum password length in CAT2 to 12.
16. Go to patron2 details page, and click to fill a random generated password
CHECK => generated password should be 12 characters length

17. Set PatronSelfRegistration to Allow in admin settings
18. Go to OPAC and fill self registration from.
CHECK => Play with patron category. For each change in category, password requirements are modified.
CHECK => Set CAT1 as patron category, set ‘aA1’ as password (or another valid password for CAT1) and before hitting submit button, change to CAT2. Form should enter invalid state, and CAT2 password requirements should be displayed as error in password input.

19. Create a patron for CAT1 and another for CAT2, leaving password blank
CHECK => For CAT1’s patron, generated password length is 8 (minimum length for generated passwords), but for CAT2’s patron should be 12

20. In admin set PatronSelfRegistrationVerifyByEmail to require
21. Fill self registration form again with CAT2 as category
CHECK => Password requirements works as previous case.
22. Leave password blank and click submit

23. select * from message_queue;
24. Copy the link in the message and paste it in OPAC
CHECH => Generated password is 12 characters long. (Copy user id for next steps)

25. In admin set OpacResetPassword to Allow
26. Go back to OPAC, reload and click on “Forgot password?” link
27. Paste user id and click submit
28. Repeat steps 23 and 24
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in warning.

29. Login OPAC with the last user and your newly created password
30. Go to “Change your password” option
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in below “New password” input.

31. prove t/db_dependent/AuthUtils.t t/db_dependent/Koha/Patron/Category.t

32. Sign off
Comment 7 Agustín Moyano 2020-04-16 21:16:08 UTC
Created attachment 103104 [details] [review]
Bug 23816: Fix other tests

This patch fixes other tests broken by new password length and strength override

To test:
1. prove t/db_dependent/Auth.t \
         t/db_dependent/Koha/Patron.t \
         t/db_dependent/Koha/Patrons.t \
         t/db_dependent/Koha/Patrons/Import.t \
         t/db_dependent/Koha/Plugins/Patron.t \
         t/db_dependent/Search/History.t \
         t/db_dependent/api/v1/auth_basic.t \
         t/db_dependent/api/v1/checkouts.t \
         t/db_dependent/api/v1/patrons_password.t \
         t/db_dependent/selenium/authentication.t \
         t/db_dependent/selenium/regressions.t
2. Sign off
Comment 8 Agustín Moyano 2020-04-16 22:52:09 UTC
Created attachment 103108 [details] [review]
Bug 23816: Update kohastructure and atomic update

Sponsored-by: Northeast Kansas Library - NEKLS
Comment 9 Agustín Moyano 2020-04-16 22:52:13 UTC
Created attachment 103109 [details] [review]
Bug 23816: [DO NOT PUSH] Update category schema

Sponsored-by: Northeast Kansas Library - NEKLS
Comment 10 Agustín Moyano 2020-04-16 22:52:16 UTC
Created attachment 103110 [details] [review]
Bug 23816: (follow-up) Define boolean column in shcema

Sponsored-by: Northeast Kansas Library - NEKLS
Comment 11 Agustín Moyano 2020-04-16 22:52:20 UTC
Created attachment 103111 [details] [review]
Bug 23816: Add tests

Sponsored-by: Northeast Kansas Library - NEKLS
Comment 12 Agustín Moyano 2020-04-16 22:52:24 UTC
Created attachment 103112 [details] [review]
Bug 23816: Add minimum password length and require strong password overrides by category

This patch adds the capability to override minPasswordLenth and RequireStrongPassword settings by category

To test:
1. koha-shell kohadev
2. koha-mysql kohadev

3. drop database koha_kohadev;
4. create database koha_kohadev;

5. go to admin page and start webinstaller. There continue the steps until onboarding.
6. reach step 3 of onboarding and create a new administrator patron
CHECH => Password control woks as normal (Minimum length 3 and strong required)

7. finish Koha installation and enter admin with your new administrator
8. set minPasswordLength to 3 and RequireStrongPassword to “Don’t require”
9. Create a new category (CAT2 from now on.. CAT1 is the category you made in onboarding process) and set minimum password length to 8 and require strong password
10. Create two new patrons, one with CAT1(patron1) and one with CAT2 (patron2)
CHECK => In both cases, try different combinations of length and strength. For patron1 the only requirement is to have 3 letters, but for patron2 the minimum length will be 8 and will require strong password.
CHECK => Try changing patron category before saving. Password requirements will change with category change.

11. Edit CAT1 and set minimum password length to 5
12. Go to patron1 details page, and change password.
CHECH => Now password minimum length is 5, but still it doesn’t require strong password

13. Edit CAT1, leave blank minimum password length and set require strong password to yes.
14. Go to patron1 details page, and change password.
CHECH => Password minimum length is back to 3, but now strong password is required

15. Set minimum password length in CAT2 to 12.
16. Go to patron2 details page, and click to fill a random generated password
CHECK => generated password should be 12 characters length

17. Set PatronSelfRegistration to Allow in admin settings
18. Go to OPAC and fill self registration from.
CHECK => Play with patron category. For each change in category, password requirements are modified.
CHECK => Set CAT1 as patron category, set ‘aA1’ as password (or another valid password for CAT1) and before hitting submit button, change to CAT2. Form should enter invalid state, and CAT2 password requirements should be displayed as error in password input.

19. Create a patron for CAT1 and another for CAT2, leaving password blank
CHECK => For CAT1’s patron, generated password length is 8 (minimum length for generated passwords), but for CAT2’s patron should be 12

20. In admin set PatronSelfRegistrationVerifyByEmail to require
21. Fill self registration form again with CAT2 as category
CHECK => Password requirements works as previous case.
22. Leave password blank and click submit

23. select * from message_queue;
24. Copy the link in the message and paste it in OPAC
CHECH => Generated password is 12 characters long. (Copy user id for next steps)

25. In admin set OpacResetPassword to Allow
26. Go back to OPAC, reload and click on “Forgot password?” link
27. Paste user id and click submit
28. Repeat steps 23 and 24
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in warning.

29. Login OPAC with the last user and your newly created password
30. Go to “Change your password” option
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in below “New password” input.

31. prove t/db_dependent/AuthUtils.t t/db_dependent/Koha/Patron/Category.t

32. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS
Comment 13 Agustín Moyano 2020-04-16 22:52:28 UTC
Created attachment 103113 [details] [review]
Bug 23816: Fix other tests

This patch fixes other tests broken by new password length and strength override

To test:
1. prove t/db_dependent/Auth.t \
         t/db_dependent/Koha/Patron.t \
         t/db_dependent/Koha/Patrons.t \
         t/db_dependent/Koha/Patrons/Import.t \
         t/db_dependent/Koha/Plugins/Patron.t \
         t/db_dependent/Search/History.t \
         t/db_dependent/api/v1/auth_basic.t \
         t/db_dependent/api/v1/checkouts.t \
         t/db_dependent/api/v1/patrons_password.t \
         t/db_dependent/selenium/authentication.t \
         t/db_dependent/selenium/regressions.t
2. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS
Comment 14 Andrew Fuerste-Henry 2020-04-27 21:21:14 UTC
Created attachment 103805 [details] [review]
Bug 23816: Update kohastructure and atomic update

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 15 Andrew Fuerste-Henry 2020-04-27 21:21:17 UTC
Created attachment 103806 [details] [review]
Bug 23816: [DO NOT PUSH] Update category schema

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 16 Andrew Fuerste-Henry 2020-04-27 21:21:21 UTC
Created attachment 103807 [details] [review]
Bug 23816: (follow-up) Define boolean column in shcema

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 17 Andrew Fuerste-Henry 2020-04-27 21:21:24 UTC
Created attachment 103808 [details] [review]
Bug 23816: Add tests

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 18 Andrew Fuerste-Henry 2020-04-27 21:21:28 UTC
Created attachment 103809 [details] [review]
Bug 23816: Add minimum password length and require strong password overrides by category

This patch adds the capability to override minPasswordLenth and RequireStrongPassword settings by category

To test:
1. koha-shell kohadev
2. koha-mysql kohadev

3. drop database koha_kohadev;
4. create database koha_kohadev;

5. go to admin page and start webinstaller. There continue the steps until onboarding.
6. reach step 3 of onboarding and create a new administrator patron
CHECH => Password control woks as normal (Minimum length 3 and strong required)

7. finish Koha installation and enter admin with your new administrator
8. set minPasswordLength to 3 and RequireStrongPassword to “Don’t require”
9. Create a new category (CAT2 from now on.. CAT1 is the category you made in onboarding process) and set minimum password length to 8 and require strong password
10. Create two new patrons, one with CAT1(patron1) and one with CAT2 (patron2)
CHECK => In both cases, try different combinations of length and strength. For patron1 the only requirement is to have 3 letters, but for patron2 the minimum length will be 8 and will require strong password.
CHECK => Try changing patron category before saving. Password requirements will change with category change.

11. Edit CAT1 and set minimum password length to 5
12. Go to patron1 details page, and change password.
CHECH => Now password minimum length is 5, but still it doesn’t require strong password

13. Edit CAT1, leave blank minimum password length and set require strong password to yes.
14. Go to patron1 details page, and change password.
CHECH => Password minimum length is back to 3, but now strong password is required

15. Set minimum password length in CAT2 to 12.
16. Go to patron2 details page, and click to fill a random generated password
CHECK => generated password should be 12 characters length

17. Set PatronSelfRegistration to Allow in admin settings
18. Go to OPAC and fill self registration from.
CHECK => Play with patron category. For each change in category, password requirements are modified.
CHECK => Set CAT1 as patron category, set ‘aA1’ as password (or another valid password for CAT1) and before hitting submit button, change to CAT2. Form should enter invalid state, and CAT2 password requirements should be displayed as error in password input.

19. Create a patron for CAT1 and another for CAT2, leaving password blank
CHECK => For CAT1’s patron, generated password length is 8 (minimum length for generated passwords), but for CAT2’s patron should be 12

20. In admin set PatronSelfRegistrationVerifyByEmail to require
21. Fill self registration form again with CAT2 as category
CHECK => Password requirements works as previous case.
22. Leave password blank and click submit

23. select * from message_queue;
24. Copy the link in the message and paste it in OPAC
CHECH => Generated password is 12 characters long. (Copy user id for next steps)

25. In admin set OpacResetPassword to Allow
26. Go back to OPAC, reload and click on “Forgot password?” link
27. Paste user id and click submit
28. Repeat steps 23 and 24
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in warning.

29. Login OPAC with the last user and your newly created password
30. Go to “Change your password” option
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in below “New password” input.

31. prove t/db_dependent/AuthUtils.t t/db_dependent/Koha/Patron/Category.t

32. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 19 Andrew Fuerste-Henry 2020-04-27 21:21:31 UTC
Created attachment 103810 [details] [review]
Bug 23816: Fix other tests

This patch fixes other tests broken by new password length and strength override

To test:
1. prove t/db_dependent/Auth.t \
         t/db_dependent/Koha/Patron.t \
         t/db_dependent/Koha/Patrons.t \
         t/db_dependent/Koha/Patrons/Import.t \
         t/db_dependent/Koha/Plugins/Patron.t \
         t/db_dependent/Search/History.t \
         t/db_dependent/api/v1/auth_basic.t \
         t/db_dependent/api/v1/checkouts.t \
         t/db_dependent/api/v1/patrons_password.t \
         t/db_dependent/selenium/authentication.t \
         t/db_dependent/selenium/regressions.t
2. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 20 Katrin Fischer 2020-07-05 16:58:47 UTC
Patch doesn't apply and I am not sure about the fix, can you please rebase?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 23816: Update kohastructure and atomic update
Applying: Bug 23816: [DO NOT PUSH] Update category schema
Applying: Bug 23816: (follow-up) Define boolean column in shcema
Applying: Bug 23816: Add tests
Applying: Bug 23816: Add minimum password length and require strong password overrides by category
Using index info to reconstruct a base tree...
M	Koha/Patron.pm
M	koha-tmpl/intranet-tmpl/prog/en/includes/password_check.inc
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt
M	members/memberentry.pl
M	opac/opac-memberentry.pl
Falling back to patching base and 3-way merge...
Auto-merging opac/opac-memberentry.pl
Auto-merging members/memberentry.pl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/password_check.inc
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/password_check.inc
Auto-merging Koha/Patron.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 23816: Add minimum password length and require strong password overrides by category
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-23816-Add-minimum-password-length-and-require--PKRJhY.patch
Comment 21 David Cook 2020-07-09 00:24:01 UTC
Wow this is intimidating.

I want to make the password policy much more complex via Bug 25934, but it looks like a lot of the password code needs refactoring.

Kudos to you for doing all this work O_O.
Comment 22 Martin Renvoize 2020-07-09 15:40:19 UTC
Another rebase request ;)... two QAers interested here.. grab them whilst it's hot ;)
Comment 23 Agustín Moyano 2020-07-21 15:52:45 UTC
Created attachment 107158 [details] [review]
Bug 23816: Update kohastructure and atomic update

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 24 Agustín Moyano 2020-07-21 15:52:49 UTC
Created attachment 107159 [details] [review]
Bug 23816: [DO NOT PUSH] Update category schema

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 25 Agustín Moyano 2020-07-21 15:52:53 UTC
Created attachment 107160 [details] [review]
Bug 23816: (follow-up) Define boolean column in shcema

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 26 Agustín Moyano 2020-07-21 15:52:58 UTC
Created attachment 107161 [details] [review]
Bug 23816: Add tests

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 27 Agustín Moyano 2020-07-21 15:53:02 UTC
Created attachment 107162 [details] [review]
Bug 23816: Add minimum password length and require strong password overrides by category

This patch adds the capability to override minPasswordLenth and RequireStrongPassword settings by category

To test:
1. koha-shell kohadev
2. koha-mysql kohadev

3. drop database koha_kohadev;
4. create database koha_kohadev;

5. go to admin page and start webinstaller. There continue the steps until onboarding.
6. reach step 3 of onboarding and create a new administrator patron
CHECH => Password control woks as normal (Minimum length 3 and strong required)

7. finish Koha installation and enter admin with your new administrator
8. set minPasswordLength to 3 and RequireStrongPassword to “Don’t require”
9. Create a new category (CAT2 from now on.. CAT1 is the category you made in onboarding process) and set minimum password length to 8 and require strong password
10. Create two new patrons, one with CAT1(patron1) and one with CAT2 (patron2)
CHECK => In both cases, try different combinations of length and strength. For patron1 the only requirement is to have 3 letters, but for patron2 the minimum length will be 8 and will require strong password.
CHECK => Try changing patron category before saving. Password requirements will change with category change.

11. Edit CAT1 and set minimum password length to 5
12. Go to patron1 details page, and change password.
CHECH => Now password minimum length is 5, but still it doesn’t require strong password

13. Edit CAT1, leave blank minimum password length and set require strong password to yes.
14. Go to patron1 details page, and change password.
CHECH => Password minimum length is back to 3, but now strong password is required

15. Set minimum password length in CAT2 to 12.
16. Go to patron2 details page, and click to fill a random generated password
CHECK => generated password should be 12 characters length

17. Set PatronSelfRegistration to Allow in admin settings
18. Go to OPAC and fill self registration from.
CHECK => Play with patron category. For each change in category, password requirements are modified.
CHECK => Set CAT1 as patron category, set ‘aA1’ as password (or another valid password for CAT1) and before hitting submit button, change to CAT2. Form should enter invalid state, and CAT2 password requirements should be displayed as error in password input.

19. Create a patron for CAT1 and another for CAT2, leaving password blank
CHECK => For CAT1’s patron, generated password length is 8 (minimum length for generated passwords), but for CAT2’s patron should be 12

20. In admin set PatronSelfRegistrationVerifyByEmail to require
21. Fill self registration form again with CAT2 as category
CHECK => Password requirements works as previous case.
22. Leave password blank and click submit

23. select * from message_queue;
24. Copy the link in the message and paste it in OPAC
CHECH => Generated password is 12 characters long. (Copy user id for next steps)

25. In admin set OpacResetPassword to Allow
26. Go back to OPAC, reload and click on “Forgot password?” link
27. Paste user id and click submit
28. Repeat steps 23 and 24
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in warning.

29. Login OPAC with the last user and your newly created password
30. Go to “Change your password” option
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in below “New password” input.

31. prove t/db_dependent/AuthUtils.t t/db_dependent/Koha/Patron/Category.t

32. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 28 Agustín Moyano 2020-07-21 15:53:06 UTC
Created attachment 107163 [details] [review]
Bug 23816: Fix other tests

This patch fixes other tests broken by new password length and strength override

To test:
1. prove t/db_dependent/Auth.t \
         t/db_dependent/Koha/Patron.t \
         t/db_dependent/Koha/Patrons.t \
         t/db_dependent/Koha/Patrons/Import.t \
         t/db_dependent/Koha/Plugins/Patron.t \
         t/db_dependent/Search/History.t \
         t/db_dependent/api/v1/auth_basic.t \
         t/db_dependent/api/v1/checkouts.t \
         t/db_dependent/api/v1/patrons_password.t \
         t/db_dependent/selenium/authentication.t \
         t/db_dependent/selenium/regressions.t
2. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 29 Agustín Moyano 2020-07-21 15:53:53 UTC
Patches rebased!
Comment 30 Agustín Moyano 2020-07-21 15:57:42 UTC
Please keep in mind that when test plan was written, default values for minPasswordLength and RequireStrongPassword preferences where 3 and 0 respectively.
Comment 31 Katrin Fischer 2020-08-16 22:20:00 UTC
There is an error in the QA tools I haven't seen before - can you please check?

FAIL	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
   OK	  filters
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  js_in_body
   OK	  spelling
   OK	  tt_valid
   FAIL	  valid_template
		The method Koha::Patron::Category->effective_min_password_length is not covered by tests!

Trace begun at /home/vagrant/kohaclone/Koha/Object.pm line 780
Koha::Object::AUTOLOAD('Koha::Patron::Category=HASH(0x5577edf29f10)') called at /home/vagrant/kohaclone/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt line 264
eval {...} at /home/vagrant/kohaclone/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt line 264
eval {...} at /home/vagrant/kohaclone/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt line 273
eval {...} at /home/vagrant/kohaclone/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt line 7
Template::Document::__ANON__('Template::Context=HASH(0x5577edf4e430)') called at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Document.pm line 163
eval {...} at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Document.pm line 161
Template::Document::process('Template::Document=HASH(0x5577edf45388)', 'Template::Context=HASH(0x5577edf4e430)') called at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Context.pm line 351
eval {...} at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Context.pm line 321
Template::Context::process('Template::Context=HASH(0x5577edf4e430)', 'Template::Document=HASH(0x5577edf45388)') called at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Service.pm line 94
eval {...} at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Service.pm line 91
Template::Service::process('Template::Service=HASH(0x5577edbc73e0)', '/home/vagrant/kohaclone/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt', 'HASH(0x5577eda95578)') called at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template.pm line 66
Template::process('Template=HASH(0x5577edbe2820)', '/home/vagrant/kohaclone/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt', 'HASH(0x5577eda95578)', 'SCALAR(0x5577e509f0c0)') called at /home/vagrant/qa-test-tools/QohA/File/Template.pm line 94
QohA::File::Template::check_valid_template('QohA::File::Template=HASH(0x5577e51f9490)') called at /home/vagrant/qa-test-tools/QohA/File/Template.pm line 23
QohA::File::Template::run_checks('QohA::File::Template=HASH(0x5577e51f9490)', 6) called at /home/vagrant/qa-test-tools/koha-qa.pl line 98
eval {...} at /home/vagrant/qa-test-tools/koha-qa.pl line 63
Comment 32 Tomás Cohen Arazi 2020-08-17 11:51:32 UTC
(In reply to Katrin Fischer from comment #31)
> There is an error in the QA tools I haven't seen before - can you please
> check?
> 
> FAIL	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
>    OK	  filters
>    OK	  forbidden patterns
>    OK	  git manipulation
>    OK	  js_in_body
>    OK	  spelling
>    OK	  tt_valid
>    FAIL	  valid_template
> 		The method Koha::Patron::Category->effective_min_password_length is not
> covered by tests!

This is probably some false positive, as the method exists and is broadly used.
Comment 33 Katrin Fischer 2020-08-18 21:24:35 UTC
Created attachment 108554 [details] [review]
Bug 23816: Update kohastructure and atomic update

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 34 Katrin Fischer 2020-08-18 21:24:39 UTC
Created attachment 108555 [details] [review]
Bug 23816: [DO NOT PUSH] Update category schema

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 35 Katrin Fischer 2020-08-18 21:24:43 UTC
Created attachment 108556 [details] [review]
Bug 23816: (follow-up) Define boolean column in shcema

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 36 Katrin Fischer 2020-08-18 21:24:47 UTC
Created attachment 108557 [details] [review]
Bug 23816: Add tests

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 37 Katrin Fischer 2020-08-18 21:24:52 UTC
Created attachment 108558 [details] [review]
Bug 23816: Add minimum password length and require strong password overrides by category

This patch adds the capability to override minPasswordLenth and RequireStrongPassword settings by category

To test:
1. koha-shell kohadev
2. koha-mysql kohadev

3. drop database koha_kohadev;
4. create database koha_kohadev;

5. go to admin page and start webinstaller. There continue the steps until onboarding.
6. reach step 3 of onboarding and create a new administrator patron
CHECH => Password control woks as normal (Minimum length 3 and strong required)

7. finish Koha installation and enter admin with your new administrator
8. set minPasswordLength to 3 and RequireStrongPassword to “Don’t require”
9. Create a new category (CAT2 from now on.. CAT1 is the category you made in onboarding process) and set minimum password length to 8 and require strong password
10. Create two new patrons, one with CAT1(patron1) and one with CAT2 (patron2)
CHECK => In both cases, try different combinations of length and strength. For patron1 the only requirement is to have 3 letters, but for patron2 the minimum length will be 8 and will require strong password.
CHECK => Try changing patron category before saving. Password requirements will change with category change.

11. Edit CAT1 and set minimum password length to 5
12. Go to patron1 details page, and change password.
CHECH => Now password minimum length is 5, but still it doesn’t require strong password

13. Edit CAT1, leave blank minimum password length and set require strong password to yes.
14. Go to patron1 details page, and change password.
CHECH => Password minimum length is back to 3, but now strong password is required

15. Set minimum password length in CAT2 to 12.
16. Go to patron2 details page, and click to fill a random generated password
CHECK => generated password should be 12 characters length

17. Set PatronSelfRegistration to Allow in admin settings
18. Go to OPAC and fill self registration from.
CHECK => Play with patron category. For each change in category, password requirements are modified.
CHECK => Set CAT1 as patron category, set ‘aA1’ as password (or another valid password for CAT1) and before hitting submit button, change to CAT2. Form should enter invalid state, and CAT2 password requirements should be displayed as error in password input.

19. Create a patron for CAT1 and another for CAT2, leaving password blank
CHECK => For CAT1’s patron, generated password length is 8 (minimum length for generated passwords), but for CAT2’s patron should be 12

20. In admin set PatronSelfRegistrationVerifyByEmail to require
21. Fill self registration form again with CAT2 as category
CHECK => Password requirements works as previous case.
22. Leave password blank and click submit

23. select * from message_queue;
24. Copy the link in the message and paste it in OPAC
CHECH => Generated password is 12 characters long. (Copy user id for next steps)

25. In admin set OpacResetPassword to Allow
26. Go back to OPAC, reload and click on “Forgot password?” link
27. Paste user id and click submit
28. Repeat steps 23 and 24
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in warning.

29. Login OPAC with the last user and your newly created password
30. Go to “Change your password” option
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in below “New password” input.

31. prove t/db_dependent/AuthUtils.t t/db_dependent/Koha/Patron/Category.t

32. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 38 Katrin Fischer 2020-08-18 21:24:57 UTC
Created attachment 108559 [details] [review]
Bug 23816: Fix other tests

This patch fixes other tests broken by new password length and strength override

To test:
1. prove t/db_dependent/Auth.t \
         t/db_dependent/Koha/Patron.t \
         t/db_dependent/Koha/Patrons.t \
         t/db_dependent/Koha/Patrons/Import.t \
         t/db_dependent/Koha/Plugins/Patron.t \
         t/db_dependent/Search/History.t \
         t/db_dependent/api/v1/auth_basic.t \
         t/db_dependent/api/v1/checkouts.t \
         t/db_dependent/api/v1/patrons_password.t \
         t/db_dependent/selenium/authentication.t \
         t/db_dependent/selenium/regressions.t
2. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 39 Jonathan Druart 2020-08-19 07:01:22 UTC
Created attachment 108571 [details] [review]
Bug 23816: Set default value in tests

We tell TestBuilder to generate the categories with NULL (and rely on
the sysprefs)
Comment 40 Jonathan Druart 2020-08-19 07:01:26 UTC
Created attachment 108572 [details] [review]
Bug 23816: trivial code simplification

Ease readability
Comment 41 Jonathan Druart 2020-08-19 07:01:57 UTC
Hola Agustin,

1. The first patch ("Set default value in tests") is dealing with default values we want to apply for all tests.  It replaces your last patch.
It's the way to go when you face such situations, easier than update all test scripts ;)

2. I am not sure we should modify misc/admin/set_password.pl
It's an sysop script, and sysops (are supposed to) know what they are doing.
It's a behaviour change that is not advertised.
(for discussion)

3. It seems that we need make categorycode param mandatory. The only call to adjust is from the onboarding tool

4. I don't think we need a new test file, t/AuthUtils.t can be reused easily. The only thing you need is to mock the categories table (search for 'fixtures_ok' in t)
Comment 42 Jonathan Druart 2020-08-19 07:07:33 UTC
(In reply to Jonathan Druart from comment #41)
> 2. I am not sure we should modify misc/admin/set_password.pl
> It's an sysop script, and sysops (are supposed to) know what they are doing.
> It's a behaviour change that is not advertised.
> (for discussion)

Sorry, I misread. That is for password generation, if none provided.

Where is this '6' coming from? :)
Comment 44 Agustín Moyano 2020-08-26 14:24:25 UTC
(In reply to Jonathan Druart from comment #42)
> (In reply to Jonathan Druart from comment #41)
> > 2. I am not sure we should modify misc/admin/set_password.pl
> > It's an sysop script, and sysops (are supposed to) know what they are doing.
> > It's a behaviour change that is not advertised.
> > (for discussion)
> 
> Sorry, I misread. That is for password generation, if none provided.
> 
> Where is this '6' coming from? :)

That 6 comes from here

$generator->randregex('[A-Za-z][A-Za-z0-9_]{6}.[A-Za-z][A-Za-z0-9_]{6}\d');
                                                                    ^
The last 6 in the regex means the length it will generate, and in my script I replaced that part with:

'[A-Za-z][A-Za-z0-9_]{6}.[A-Za-z][A-Za-z0-9_]{'.$n.'}\d'

where $n was the minimum password length for the user's category
Comment 45 Emmi Takkinen 2020-08-27 10:38:36 UTC
There's a typo on last patch:

syntax error at /var/Koha/Koha/Patron/Category.pm line 238, near "->reset_password ) "
Comment 46 Agustín Moyano 2020-08-27 16:49:06 UTC
Created attachment 109243 [details] [review]
Bug 23816: Set default value in tests

We tell TestBuilder to generate the categories with NULL (and rely on
the sysprefs)

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>

https://bugs.koha-community.org/show_bug.cgi?id=23826
Comment 47 Agustín Moyano 2020-08-27 16:49:11 UTC
Created attachment 109244 [details] [review]
Bug 23816: trivial code simplification

Ease readability

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>

https://bugs.koha-community.org/show_bug.cgi?id=23826
Comment 48 Agustín Moyano 2020-08-27 16:49:16 UTC
Created attachment 109245 [details] [review]
Bug 23816: (follow-up) Fix many things

This patch:
 * reverts changes on misc/admin/set_password.pl
 * makes category param mandatory for AuthUtils::is_valid_password and AuthUtils::generate_password
 * changes onboarding.pl to set category param in AuthUtils::is_valid_password
 * Completes t/db_dependent/AuthUtils.t and drops t/AuthUtils.t
 * Removes offending <input type="number"/> and replaces it by <input type="text" inputmode="numeric" pattern="[0-9]*"/>

https://bugs.koha-community.org/show_bug.cgi?id=23826
Comment 49 Agustín Moyano 2020-08-27 16:50:38 UTC
(In reply to Emmi Takkinen from comment #45)
> There's a typo on last patch:
> 
> syntax error at /var/Koha/Koha/Patron/Category.pm line 238, near
> "->reset_password ) "

Thanks Emmi for catching this up
Comment 50 Agustín Moyano 2020-08-27 16:55:26 UTC
When I execute qa of this patchset, I get

 FAIL	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
   FAIL	  valid_template
		The method Koha::Patron::Category->effective_min_password_length is not covered by tests!

This must be a qa script bug, because Koha::Patron::Category has the method effective_min_password_length, and it's covered by tests.. maybe it's checking in Schema class, instead of Koha::Object?
Comment 51 Katrin Fischer 2020-08-27 17:38:28 UTC
(In reply to Agustín Moyano from comment #50)
> When I execute qa of this patchset, I get
> 
>  FAIL	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
>    FAIL	  valid_template
> 		The method Koha::Patron::Category->effective_min_password_length is not
> covered by tests!
> 
> This must be a qa script bug, because Koha::Patron::Category has the method
> effective_min_password_length, and it's covered by tests.. maybe it's
> checking in Schema class, instead of Koha::Object?

I don't see that fail when I run the QA tools on it.
Comment 52 Katrin Fischer 2020-08-27 17:39:07 UTC
(In reply to Katrin Fischer from comment #51)
> (In reply to Agustín Moyano from comment #50)
> > When I execute qa of this patchset, I get
> > 
> >  FAIL	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
> >    FAIL	  valid_template
> > 		The method Koha::Patron::Category->effective_min_password_length is not
> > covered by tests!
> > 
> > This must be a qa script bug, because Koha::Patron::Category has the method
> > effective_min_password_length, and it's covered by tests.. maybe it's
> > checking in Schema class, instead of Koha::Object?
> 
> I don't see that fail when I run the QA tools on it.

... I was looking in the wrong spot - It does fail...
Comment 53 Katrin Fischer 2020-08-27 18:11:33 UTC
Created attachment 109249 [details] [review]
Bug 23816: Update kohastructure and atomic update

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 54 Katrin Fischer 2020-08-27 18:11:39 UTC
Created attachment 109250 [details] [review]
Bug 23816: [DO NOT PUSH] Update category schema

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 55 Katrin Fischer 2020-08-27 18:11:45 UTC
Created attachment 109251 [details] [review]
Bug 23816: (follow-up) Define boolean column in shcema

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 56 Katrin Fischer 2020-08-27 18:11:50 UTC
Created attachment 109252 [details] [review]
Bug 23816: Add tests

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 57 Katrin Fischer 2020-08-27 18:11:55 UTC
Created attachment 109253 [details] [review]
Bug 23816: Add minimum password length and require strong password overrides by category

This patch adds the capability to override minPasswordLenth and RequireStrongPassword settings by category

To test:
1. koha-shell kohadev
2. koha-mysql kohadev

3. drop database koha_kohadev;
4. create database koha_kohadev;

5. go to admin page and start webinstaller. There continue the steps until onboarding.
6. reach step 3 of onboarding and create a new administrator patron
CHECH => Password control woks as normal (Minimum length 3 and strong required)

7. finish Koha installation and enter admin with your new administrator
8. set minPasswordLength to 3 and RequireStrongPassword to “Don’t require”
9. Create a new category (CAT2 from now on.. CAT1 is the category you made in onboarding process) and set minimum password length to 8 and require strong password
10. Create two new patrons, one with CAT1(patron1) and one with CAT2 (patron2)
CHECK => In both cases, try different combinations of length and strength. For patron1 the only requirement is to have 3 letters, but for patron2 the minimum length will be 8 and will require strong password.
CHECK => Try changing patron category before saving. Password requirements will change with category change.

11. Edit CAT1 and set minimum password length to 5
12. Go to patron1 details page, and change password.
CHECH => Now password minimum length is 5, but still it doesn’t require strong password

13. Edit CAT1, leave blank minimum password length and set require strong password to yes.
14. Go to patron1 details page, and change password.
CHECH => Password minimum length is back to 3, but now strong password is required

15. Set minimum password length in CAT2 to 12.
16. Go to patron2 details page, and click to fill a random generated password
CHECK => generated password should be 12 characters length

17. Set PatronSelfRegistration to Allow in admin settings
18. Go to OPAC and fill self registration from.
CHECK => Play with patron category. For each change in category, password requirements are modified.
CHECK => Set CAT1 as patron category, set ‘aA1’ as password (or another valid password for CAT1) and before hitting submit button, change to CAT2. Form should enter invalid state, and CAT2 password requirements should be displayed as error in password input.

19. Create a patron for CAT1 and another for CAT2, leaving password blank
CHECK => For CAT1’s patron, generated password length is 8 (minimum length for generated passwords), but for CAT2’s patron should be 12

20. In admin set PatronSelfRegistrationVerifyByEmail to require
21. Fill self registration form again with CAT2 as category
CHECK => Password requirements works as previous case.
22. Leave password blank and click submit

23. select * from message_queue;
24. Copy the link in the message and paste it in OPAC
CHECH => Generated password is 12 characters long. (Copy user id for next steps)

25. In admin set OpacResetPassword to Allow
26. Go back to OPAC, reload and click on “Forgot password?” link
27. Paste user id and click submit
28. Repeat steps 23 and 24
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in warning.

29. Login OPAC with the last user and your newly created password
30. Go to “Change your password” option
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in below “New password” input.

31. prove t/db_dependent/AuthUtils.t t/db_dependent/Koha/Patron/Category.t

32. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 58 Katrin Fischer 2020-08-27 18:11:59 UTC
Created attachment 109254 [details] [review]
Bug 23816: Set default value in tests

We tell TestBuilder to generate the categories with NULL (and rely on
the sysprefs)

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>

https://bugs.koha-community.org/show_bug.cgi?id=23826

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 59 Katrin Fischer 2020-08-27 18:12:04 UTC
Created attachment 109255 [details] [review]
Bug 23816: trivial code simplification

Ease readability

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>

https://bugs.koha-community.org/show_bug.cgi?id=23826

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 60 Katrin Fischer 2020-08-27 18:12:09 UTC
Created attachment 109256 [details] [review]
Bug 23816: (follow-up) Fix many things

This patch:
 * reverts changes on misc/admin/set_password.pl
 * makes category param mandatory for AuthUtils::is_valid_password and AuthUtils::generate_password
 * changes onboarding.pl to set category param in AuthUtils::is_valid_password
 * Completes t/db_dependent/AuthUtils.t and drops t/AuthUtils.t
 * Removes offending <input type="number"/> and replaces it by <input type="text" inputmode="numeric" pattern="[0-9]*"/>

https://bugs.koha-community.org/show_bug.cgi?id=23826

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 61 Katrin Fischer 2020-08-27 18:12:14 UTC
Created attachment 109257 [details] [review]
Bug 23816: (QA follow-up) Use existing form validation to validate min password length

The pattern check didn't work for me, but I figured we might
want to use the same validation as for the other numeric fields
on the form instead (upper age limit, etc.)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 62 Jonathan Druart 2020-08-28 11:43:04 UTC
(In reply to Agustín Moyano from comment #50)
> When I execute qa of this patchset, I get
> 
>  FAIL	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
>    FAIL	  valid_template
> 		The method Koha::Patron::Category->effective_min_password_length is not
> covered by tests!
> 
> This must be a qa script bug, because Koha::Patron::Category has the method
> effective_min_password_length, and it's covered by tests.. maybe it's
> checking in Schema class, instead of Koha::Object?

This is a bug in the QA script, I opened an issue and submitted a merge request.
https://gitlab.com/koha-community/qa-test-tools/-/issues/37
Comment 63 Jonathan Druart 2020-09-02 10:16:16 UTC
Please rebase (conflict with bug 19889).
Comment 64 Tomás Cohen Arazi 2020-09-02 12:54:37 UTC
Created attachment 109520 [details] [review]
Bug 23816: Update kohastructure and atomic update

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 65 Tomás Cohen Arazi 2020-09-02 12:54:42 UTC
Created attachment 109521 [details] [review]
Bug 23816: [DO NOT PUSH] Update category schema

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 66 Tomás Cohen Arazi 2020-09-02 12:54:46 UTC
Created attachment 109522 [details] [review]
Bug 23816: (follow-up) Define boolean column in shcema

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 67 Tomás Cohen Arazi 2020-09-02 12:54:52 UTC
Created attachment 109523 [details] [review]
Bug 23816: Add tests

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 68 Tomás Cohen Arazi 2020-09-02 12:54:58 UTC
Created attachment 109524 [details] [review]
Bug 23816: Add minimum password length and require strong password overrides by category

This patch adds the capability to override minPasswordLenth and RequireStrongPassword settings by category

To test:
1. koha-shell kohadev
2. koha-mysql kohadev

3. drop database koha_kohadev;
4. create database koha_kohadev;

5. go to admin page and start webinstaller. There continue the steps until onboarding.
6. reach step 3 of onboarding and create a new administrator patron
CHECH => Password control woks as normal (Minimum length 3 and strong required)

7. finish Koha installation and enter admin with your new administrator
8. set minPasswordLength to 3 and RequireStrongPassword to “Don’t require”
9. Create a new category (CAT2 from now on.. CAT1 is the category you made in onboarding process) and set minimum password length to 8 and require strong password
10. Create two new patrons, one with CAT1(patron1) and one with CAT2 (patron2)
CHECK => In both cases, try different combinations of length and strength. For patron1 the only requirement is to have 3 letters, but for patron2 the minimum length will be 8 and will require strong password.
CHECK => Try changing patron category before saving. Password requirements will change with category change.

11. Edit CAT1 and set minimum password length to 5
12. Go to patron1 details page, and change password.
CHECH => Now password minimum length is 5, but still it doesn’t require strong password

13. Edit CAT1, leave blank minimum password length and set require strong password to yes.
14. Go to patron1 details page, and change password.
CHECH => Password minimum length is back to 3, but now strong password is required

15. Set minimum password length in CAT2 to 12.
16. Go to patron2 details page, and click to fill a random generated password
CHECK => generated password should be 12 characters length

17. Set PatronSelfRegistration to Allow in admin settings
18. Go to OPAC and fill self registration from.
CHECK => Play with patron category. For each change in category, password requirements are modified.
CHECK => Set CAT1 as patron category, set ‘aA1’ as password (or another valid password for CAT1) and before hitting submit button, change to CAT2. Form should enter invalid state, and CAT2 password requirements should be displayed as error in password input.

19. Create a patron for CAT1 and another for CAT2, leaving password blank
CHECK => For CAT1’s patron, generated password length is 8 (minimum length for generated passwords), but for CAT2’s patron should be 12

20. In admin set PatronSelfRegistrationVerifyByEmail to require
21. Fill self registration form again with CAT2 as category
CHECK => Password requirements works as previous case.
22. Leave password blank and click submit

23. select * from message_queue;
24. Copy the link in the message and paste it in OPAC
CHECH => Generated password is 12 characters long. (Copy user id for next steps)

25. In admin set OpacResetPassword to Allow
26. Go back to OPAC, reload and click on “Forgot password?” link
27. Paste user id and click submit
28. Repeat steps 23 and 24
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in warning.

29. Login OPAC with the last user and your newly created password
30. Go to “Change your password” option
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in below “New password” input.

31. prove t/db_dependent/AuthUtils.t t/db_dependent/Koha/Patron/Category.t

32. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 69 Tomás Cohen Arazi 2020-09-02 12:55:04 UTC
Created attachment 109525 [details] [review]
Bug 23816: Set default value in tests

We tell TestBuilder to generate the categories with NULL (and rely on
the sysprefs)

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>

https://bugs.koha-community.org/show_bug.cgi?id=23826

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 70 Tomás Cohen Arazi 2020-09-02 12:55:08 UTC
Created attachment 109526 [details] [review]
Bug 23816: trivial code simplification

Ease readability

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>

https://bugs.koha-community.org/show_bug.cgi?id=23826

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 71 Tomás Cohen Arazi 2020-09-02 12:55:13 UTC
Created attachment 109527 [details] [review]
Bug 23816: (follow-up) Fix many things

This patch:
 * reverts changes on misc/admin/set_password.pl
 * makes category param mandatory for AuthUtils::is_valid_password and AuthUtils::generate_password
 * changes onboarding.pl to set category param in AuthUtils::is_valid_password
 * Completes t/db_dependent/AuthUtils.t and drops t/AuthUtils.t
 * Removes offending <input type="number"/> and replaces it by <input type="text" inputmode="numeric" pattern="[0-9]*"/>

https://bugs.koha-community.org/show_bug.cgi?id=23826

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 72 Tomás Cohen Arazi 2020-09-02 12:55:18 UTC
Created attachment 109528 [details] [review]
Bug 23816: (QA follow-up) Use existing form validation to validate min password length

The pattern check didn't work for me, but I figured we might
want to use the same validation as for the other numeric fields
on the form instead (upper age limit, etc.)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 73 Jonathan Druart 2020-09-09 13:40:33 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 74 Jonathan Druart 2020-09-10 07:58:34 UTC
Created attachment 109833 [details] [review]
Bug 23816: Fix patron edition

The patron edition was broken, we always got the pattern alert
Password:
Password must contain at least 8 characters, including UPPERCASE, lowercase and numbers
Comment 75 Jonathan Druart 2020-09-10 08:07:21 UTC
Created attachment 109834 [details] [review]
Bug 23816: Fix selenium tests
Comment 76 Jonathan Druart 2020-09-10 08:08:11 UTC
Last 2 follow-ups pushed to master.
Comment 77 Lucas Gass 2020-09-14 22:25:46 UTC
enhancement will not be backported to 20.05.x series
Comment 78 Jonathan Druart 2020-12-03 14:27:26 UTC
Removing the "release notes needed" keyword, as it's released now... Would have been good to have it in time.
Comment 79 Jonathan Druart 2020-12-03 14:27:52 UTC
There is a critical bug for 20.11 caused by this patchset, see bug 27147. Please fix ASAP.