Bug 18308

Summary: Default value of minPasswordLength should be increased
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Alex Buckley <alexbuckley>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: 1joynelson, aleisha, alexbuckley, bgkriegel, dcook, dominic.pichette, hayleypelham, josef.moravec, katrin.fischer, kyle, m.de.rooy, nick, peterAtKohaBugzilla, veron
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18298
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19514
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21314
Change sponsored?: Sponsored Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This patch increases the value of minPasswordLength to 8 characters to encourage more secure passwords, for all new installs.
Version(s) released in:
20.05.00
Bug Depends on: 19514, 22451    
Bug Blocks: 18888, 21265, 22681    
Attachments: Bug 18308 - Increased default value of minPasswordValue
Bug 18308 - Followup adding default value to syspref.sql
Bug 18308 - Added step to onboarding tool to set minPasswordLength
Bug 18308 - Added step to onboarding tool to set minPasswordLength
Bug 18308 - Added warning and dialog alert
Bug 18308 - Added step to onboarding tool to set minPasswordLength
Bug 18308 - Added warning and dialog alert
Bug 18308 - Added step to onboarding tool to set minPasswordLength
Bug 18308 - Added warning and dialog alert
Bug 18308 - Added warning and dialog alert
Bug 18308 - Added step to onboarding tool to set minPasswordLength
Bug 18308 - Added warning and dialog alert
Bug 18308: (follow up) Changed order of onboarding steps
Bug 18308: Fixed whitespace for qa tool
Bug 18308: Added missing filters
Bug 18308: Added another filter
Bug 18308: Fixed whitespace and added filters for qa tools
Bug 18308 - Added step to onboarding tool to set minPasswordLength
Bug 18308 - Added warning and dialog alert
Bug 18308: Fixed onboarding_messages.inc by changing order of statements
Bug 18308: Fixed issues with onboardingstep6.tt
Bug 18308: Changed ordering of onboarding steps
Bug 18308: Added missing onboardingstep8.tt
Bug 18308: Added step to onboarding tool to set minPasswordLength
Bug 18308: Added warning and dialog alert
Bug 18308: Added step to onboarding tool to set minPasswordLength
Bug 18308: Added warning and dialog alert
Bug 18308: Fixed issues with onboarding_messages.inc and onboardingstep6.tt
Bug 18308: Changed ordering of onboarding steps
Bug 18308: Added missing filter in memberentrygen.tt
Bug 18308: Added step to onboarding tool to set minPasswordLength
Bug 18308: Added warning and dialog alert
Bug 18308: Fixed issues with onboarding_messages.inc and onboardingstep6.tt
Bug 18308: Changed ordering of onboarding steps
Bug 18308: Added missing filter in memberentrygen.tt
Bug 18308: Changed templates to use Asset plugin rather than full path
Bug 18308: Removed extraneous file
Bug 18308: Added step to onboarding tool to set minPasswordLength
Bug 18308: Added warning and dialog alert
Bug 18308: Fixed issues with onboarding_messages.inc and onboardingstep6.tt
Bug 18308: Changed ordering of onboarding steps
Bug 18308: Added missing filter in memberentrygen.tt
Bug 18308: Changed templates to use Asset plugin rather than full path
Bug 18308: Removed extraneous file
Bug 18308: (QA follow up) Resolving merge conflict
Bug 18308: Added step to onboarding tool to set minPasswordLength
Bug 18308: Added warning and dialog alert
Bug 18308: Fixed issues with onboarding_messages.inc and onboardingstep6.tt
Bug 18308: Changed ordering of onboarding steps
Bug 18308: Added missing filter in memberentrygen.tt
Bug 18308: Changed templates to use Asset plugin rather than full path
Bug 18308: Removed extraneous file
Bug 18308: (QA follow up) Resolving merge conflict
Bug 18308: Removed warns and moved password length warning to template
Bug 18308: Removed warns and moved password length warning to template
Bug 18308: Increase minPasswordLength to 8 characters
Bug 18308 - Followup adding default value to syspref.sql
Bug 18308: Increase minPasswordLength default to 8
Bug 18308: fix failing tests

Description Jonathan Druart 2017-03-21 13:18:17 UTC
It defaults to 3, should be at least 8 or 10.
Comment 1 Marcel de Rooy 2017-03-21 13:39:00 UTC
Definitely +5 :)
Comment 2 Alex Buckley 2017-10-23 09:06:07 UTC
Created attachment 68374 [details] [review]
Bug 18308 - Increased default value of minPasswordValue

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Ensuring PatronSelfRegistration syspref is enabled, and
PatronSelfRegistationDefaultCategory is set register a patron in the
OPAC with a password of 2 characters and notice that a red message is
displayed informing you that the password must be at least 3 characters

3. Run updatedatabase.pl

4. Repeat step 1 and notice that the password must be 8 characters or
more now

5. Repeat step 2 and notice that the password must be 8 characters or
more now

Sponsored-By: Catalyst IT
Comment 3 Marcel de Rooy 2017-10-23 09:24:49 UTC
How about new installations ?
Comment 4 Alex Buckley 2017-10-23 09:49:55 UTC
Created attachment 68381 [details] [review]
Bug 18308 - Followup adding default value to syspref.sql

Sponsored-By: Catalyst IT
Comment 5 Alex Buckley 2017-10-23 09:51:21 UTC
Oops sorry Marcel, I forgot about new installs. I have attached a followup which has the changed minPasswordLength default value set to 8 in syspref.sql and so it should be run in the installation process
Comment 6 Marcel de Rooy 2017-10-23 12:16:10 UTC
From IRC:

marcelr	alexbuckley: thx for followup on 18308; only problem would be now: do all libraries really want this? devs and admins do, but users..
[11:59]	alexbuckley	hmm it could be included in the onboarding tool so when Koha is being installed the value can be set but have it set to 3 by default in sysprefs.sql?
[12:07]	alexbuckley	what do you think of that for a solution marcelr?
[12:08]	marcelr	i strongly think that it should be 8, but i know there will be resistence
[12:08]	marcelr	onboarding could be a good compromise
[12:09]	marcelr	can we set it to 8 and optionally lower it in onboarding ?
[12:09]	alexbuckley	yeah thats a good idea
[12:10]	marcelr	yes with a little security advertisement
Comment 7 Jonathan Druart 2017-10-23 14:10:25 UTC
I do not think we should update existing installations. However we can raise a warning if < X.
Comment 8 Jonathan Druart 2017-10-23 15:03:28 UTC
*** Bug 9920 has been marked as a duplicate of this bug. ***
Comment 9 Alex Buckley 2017-10-23 23:03:20 UTC
Created attachment 68451 [details] [review]
Bug 18308 - Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT

Sponsored-By: Catalyst IT
Comment 10 Alex Buckley 2017-10-23 23:10:09 UTC
Hi Jonathan and Marcel

Have a look at this patch I have just attached it adds a new step to the onboarding tool to optionally alter the minPasswordLength (with security warning which can be made JS warning if you want me to, at the moment it is HTML) and let me know what you think.

I have removed the atomicupate for altering the default value for existing installations as per Jonathan's comment, and in thiss patch I am setting the default value for new installations to 8 as per Marcel and my irc discussion. 

One thing I haven't done yet is raise a warning for existing installations if the minPasswordLength value is < X
Comment 11 Marcel de Rooy 2017-10-24 13:24:31 UTC
Hi Alex
Please add the warning.
And I would suggest to rebase this one on top of 19514. This currently conflicts.

Marcel
Comment 12 Alex Buckley 2017-11-23 22:54:48 UTC
Created attachment 69318 [details] [review]
Bug 18308 - Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT

Sponsored-By: Catalyst IT
Comment 13 Alex Buckley 2017-11-23 22:55:17 UTC
Created attachment 69319 [details] [review]
Bug 18308 - Added warning and dialog alert

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Comment 14 Alex Buckley 2017-11-23 23:47:32 UTC
Created attachment 69320 [details] [review]
Bug 18308 - Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT

Sponsored-By: Catalyst IT
Comment 15 Alex Buckley 2017-11-23 23:48:20 UTC
Created attachment 69321 [details] [review]
Bug 18308 - Added warning and dialog alert

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Comment 16 Alex Buckley 2017-11-24 01:02:58 UTC
Created attachment 69323 [details] [review]
Bug 18308 - Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT
Comment 17 Alex Buckley 2017-11-24 01:05:37 UTC
Created attachment 69324 [details] [review]
Bug 18308 - Added warning and dialog alert

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Comment 18 Dominic Pichette 2017-11-24 21:17:50 UTC
patch failed to install:

Applying: Bug 18308 - Added step to onboarding tool to set minPasswordLength
fatal: sha1 information is lacking or useless (installer/data/mysql/sysprefs.sql).
error: could not build fake ancestor
Patch failed at 0001 Bug 18308 - Added step to onboarding tool to set minPasswordLength
Comment 19 Alex Buckley 2017-11-30 02:00:15 UTC
Hi Dominic 

thanks for testing the patch, can you please try again and apply bug 19514 first.
Comment 20 Dominic Pichette 2017-12-01 21:02:56 UTC
Created attachment 69459 [details] [review]
Bug 18308 - Added warning and dialog alert

Works great if you apply bug 19514 before

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Comment 21 David Bourgault 2017-12-01 21:24:07 UTC
Created attachment 69461 [details] [review]
Bug 18308 - Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Comment 22 David Bourgault 2017-12-01 21:24:11 UTC
Created attachment 69462 [details] [review]
Bug 18308 - Added warning and dialog alert

Works great if you apply bug 19514 before

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Comment 23 Josef Moravec 2017-12-14 11:21:14 UTC
Comment on attachment 69461 [details] [review]
Bug 18308 - Added step to onboarding tool to set minPasswordLength

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

I would like to see the setting of min password length before creating the superlibrarian user account, because now you first use the limit a then set the limit - it is not very logical IMO

::: installer/onboarding.pl
@@ +313,1 @@
>      $template_params->{all_done} = 1;    # If step 5 is complete, we are done!

The comment should be changed too, 5 => 6

::: koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep6.tt
@@ +2,5 @@
> +<title>Koha &rsaquo; Web installer &rsaquo;
> +    [% IF all_done %]
> +        Complete
> +    [% ELSE %]
> +        Create circulation rule

Should be "Set minPasswordLength system preference"

@@ +8,5 @@
> +</title>
> +[% INCLUDE 'installer-doc-head-close.inc' %]
> +[% INCLUDE 'validator-strings.inc' %]
> +[% INCLUDE 'installer-strings.inc' %]
> +<script type="text/javascript" src="[% interface %]/[% theme %]/js/onboarding.js"></script>

Javascript should be at the end - see bug 17418

@@ +14,5 @@
> +
> +<body id="installer" class="installer">
> +    <div class="container-fluid">
> +        <div class="row">
> +            <div id="onboarding-step5" class="installer-main col-sm-8 col-sm-offset-2 col-md-8 col-md-offset-2">

This is actually step6
Comment 24 Hayley Pelham 2019-03-27 21:56:31 UTC
Created attachment 87124 [details] [review]
Bug 18308: (follow up) Changed order of onboarding steps

This patch cleans up the previous patches according to comment 23, fixes
various typos, and changes the order of steps in the onboarding so that
the setting of minPasswordLength occurs before the admin password is
set.

Test plan:
1) Drop and recreate your database, and restart_all
2) Go through installation and onboarding
3) Note that the order of steps
is:
 1 Create a library
 2 Create a ptron category
 3 Create Koha administrator patron
 4 Create a new item type
 5 Create a new circulation rule
 6 Set minPasswordLength syspref/Complete
4) Repeat steps 1 & 2
5) Note that the order of steps is now:
 1 Create a library
 2 Create a patron category
 3 Set minPasswordLength syspref
 4 Create Koha administrator patron
 5 Create a new item type
 6 Create a new circulation rule/Complete
6) Verify that the minPasswordLength syspref is set to the value from
onboarding
7) Verify that the patron, item type, and circulation rule made in the
onboarding exist in the administration
8) Sign off!

Sponsored-by Catalyst IT
Comment 25 Hayley Pelham 2019-03-27 21:57:23 UTC
Created attachment 87125 [details] [review]
Bug 18308: Fixed whitespace for qa tool

Note: This will still fail qa tool due to 'forbidden' javascript
fullpath and not use of Asset plugin. Asset plugin cannot be used for
onboarding currently.

Sponsored-by: Catalyst IT
Comment 26 Hayley Pelham 2019-03-27 22:19:53 UTC
Created attachment 87126 [details] [review]
Bug 18308: Added missing filters

Sponsored-by: Catalyst IT
Comment 27 Hayley Pelham 2019-03-28 00:36:29 UTC
Created attachment 87128 [details] [review]
Bug 18308: Added another filter

Sponsored-by: Catalyst IT
Comment 28 Hayley Pelham 2019-03-28 00:41:31 UTC
Created attachment 87129 [details] [review]
Bug 18308: Fixed whitespace and added filters for qa tools

Note: This will still fail qa tool due to 'forbidden' javascript
fullpath and not use of Asset plugin. Asset plugin cannot be used for
onboarding currently.

Sponsored-by: Catalyst IT
Comment 29 Hayley Pelham 2019-04-02 23:26:30 UTC
Created attachment 87339 [details] [review]
Bug 18308 - Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

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

Hayley Mapley: Fixed merge conflicts
Comment 30 Hayley Pelham 2019-04-02 23:30:38 UTC
Created attachment 87340 [details] [review]
Bug 18308 - Added warning and dialog alert

Works great if you apply bug 19514 before

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Comment 31 Hayley Pelham 2019-04-08 21:33:38 UTC
Tried to integrate this with the other onboarding related patches. Had to rebase and reattach the patches. Needs to be tested once again.
Comment 32 Hayley Pelham 2019-04-08 21:49:15 UTC
Obsoleted patches made in changing order of steps due to merge conflicts and rebasing issues. Will continue working on this patch to ensure it can be used as a base for the other onboarding patches.
Comment 33 Hayley Pelham 2019-04-08 22:43:16 UTC
Created attachment 87564 [details] [review]
Bug 18308: Fixed onboarding_messages.inc by changing order of statements

Sponsored-by: catalyst IT
Comment 34 Hayley Pelham 2019-04-09 01:59:50 UTC
Created attachment 87572 [details] [review]
Bug 18308: Fixed issues with onboardingstep6.tt

Page did not load due to incorrect include and misplaced javascript tag
and includes.

Sponsored-by: Catalyst IT
Comment 35 Hayley Pelham 2019-04-09 02:58:34 UTC
Created attachment 87573 [details] [review]
Bug 18308: Changed ordering of onboarding steps

This patch cleans up the previous patches according to comment 23, fixes
various typos, and changes the order of steps in the onboarding so that
the setting of minPasswordLength occurs before the admin password is
set.

Test plan:
1) Drop and recreate your database, and restart_all
2) Go through installation and onboarding
3) Note that the order of steps
is:
 1 Create a library
 2 Create a ptron category
 3 Create Koha administrator patron
 4 Create a new item type
 5 Create a new circulation rule
 6 Set minPasswordLength syspref/Complete
4) Repeat steps 1 & 2
5) Note that the order of steps is now:
 1 Create a library
 2 Create a patron category
 3 Set minPasswordLength syspref
 4 Create Koha administrator patron
 5 Create a new item type
 6 Create a new circulation rule/Complete
6) Verify that the minPasswordLength syspref is set to the
    value from onboarding
7) Verify that the patron, item type, and circulation rule
    made in the onboarding exist in the administration
8) Sign off!

Sponsored-by Catalyst IT
Comment 36 Hayley Pelham 2019-04-11 01:29:10 UTC
Created attachment 87747 [details] [review]
Bug 18308: Added missing onboardingstep8.tt

Sponsered-by: Catalyt IT
Comment 37 Hayley Pelham 2019-04-11 04:03:12 UTC
*** Bug 22625 has been marked as a duplicate of this bug. ***
Comment 38 Katrin Fischer 2019-04-12 08:31:56 UTC
Hi Hayley,

can you please fix a few things here?

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep6.tt
   OK	  filters
   FAIL	  forbidden patterns
		forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 137)
   OK	  git manipulation
   OK	  js_in_body
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep8.tt
   OK	  filters
   FAIL	  forbidden patterns
		forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 137)
   OK	  git manipulation
   OK	  js_in_body
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
   FAIL	  filters
		missing_filter at line 136 (        <p><strong> Warning: </strong> [% passwordwarning %]</p>)
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  js_in_body
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

If you are using kohadevbox you can run the qa test tools from koha-shell:
qa -c 6 -v 2 

There is also a complaint about the commit messages:

Processing additional checks

	* Commit title does not start with 'Bug XXXXX: ' - df4b236511
	* Commit title does not start with 'Bug XXXXX: ' - a1946b6a31

This can be fixed easiest with: git rebase -i HEAD~6 and then rewording the 2 at the beginning.

Let me know, if I can help more.
Comment 39 Hayley Pelham 2019-04-14 21:17:04 UTC
(In reply to Katrin Fischer from comment #38)
> Hi Hayley,
> 
> can you please fix a few things here?
> 
>  FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep6.tt
>    OK	  filters
>    FAIL	  forbidden patterns
> 		forbidden pattern: Do *not* include full path to js files, use the Asset
> TT plugin (bug 20538) (line 137)
>    OK	  git manipulation
>    OK	  js_in_body
>    OK	  spelling
>    OK	  tt_valid
>    OK	  valid_template
> 
>  FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep8.tt
>    OK	  filters
>    FAIL	  forbidden patterns
> 		forbidden pattern: Do *not* include full path to js files, use the Asset
> TT plugin (bug 20538) (line 137)
>    OK	  git manipulation
>    OK	  js_in_body
>    OK	  spelling
>    OK	  tt_valid
>    OK	  valid_template
> 
>  FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
>    FAIL	  filters
> 		missing_filter at line 136 (        <p><strong> Warning: </strong> [%
> passwordwarning %]</p>)
>    OK	  forbidden patterns
>    OK	  git manipulation
>    OK	  js_in_body
>    OK	  spelling
>    OK	  tt_valid
>    OK	  valid_template
> 
> If you are using kohadevbox you can run the qa test tools from koha-shell:
> qa -c 6 -v 2 
> 
> There is also a complaint about the commit messages:
> 
> Processing additional checks
> 
> 	* Commit title does not start with 'Bug XXXXX: ' - df4b236511
> 	* Commit title does not start with 'Bug XXXXX: ' - a1946b6a31
> 
> This can be fixed easiest with: git rebase -i HEAD~6 and then rewording the
> 2 at the beginning.
> 
> Let me know, if I can help more.

Hi Katrin,

The missing filter I can definitely add. I normally do run the QA tools before attaching, I must have missed it this time though! Thanks for picking it up. 

The Asset plugin issue is trickier because it does not work with onboarding due to how it retrieves Koha version from the database (or tries to before the database is created). There is a patch that will solve this and allow for Asset to be used in onboarding Bug 22451, but it is not yet pushed to Master, so the full path js is all that can be done for now I think. 

I will reattach once I have reworded the commits and added the missing filter.
Comment 40 Hayley Pelham 2019-04-14 22:15:32 UTC
Created attachment 87959 [details] [review]
Bug 18308: Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

https://bugs.koha-community.org/show_bug.cgi?id=6473
Comment 41 Hayley Pelham 2019-04-14 22:16:03 UTC
Created attachment 87960 [details] [review]
Bug 18308: Added warning and dialog alert

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Comment 42 Hayley Pelham 2019-04-14 22:23:04 UTC
Created attachment 87961 [details] [review]
Bug 18308: Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

https://bugs.koha-community.org/show_bug.cgi?id=6473
Comment 43 Hayley Pelham 2019-04-14 22:23:45 UTC
Created attachment 87962 [details] [review]
Bug 18308: Added warning and dialog alert

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Comment 44 Hayley Pelham 2019-04-14 22:24:14 UTC
Created attachment 87963 [details] [review]
Bug 18308: Fixed issues with onboarding_messages.inc and onboardingstep6.tt

Sponsored-by: Catalyst IT
Comment 45 Hayley Pelham 2019-04-14 22:24:45 UTC
Created attachment 87964 [details] [review]
Bug 18308: Changed ordering of onboarding steps

This patch cleans up the previous patches according to comment 23, fixes
various typos, and changes the order of steps in the onboarding so that
the setting of minPasswordLength occurs before the admin password is
set.

Test plan:
1) Drop and recreate your database, and restart_all
2) Go through installation and onboarding
3) Note that the order of steps
is:
 1 Create a library
 2 Create a ptron category
 3 Create Koha administrator patron
 4 Create a new item type
 5 Create a new circulation rule
 6 Set minPasswordLength syspref/Complete
4) Repeat steps 1 & 2
5) Note that the order of steps is now:
 1 Create a library
 2 Create a patron category
 3 Set minPasswordLength syspref
 4 Create Koha administrator patron
 5 Create a new item type
 6 Create a new circulation rule/Complete
6) Verify that the minPasswordLength syspref is set to the
    value from onboarding
7) Verify that the patron, item type, and circulation rule
    made in the onboarding exist in the administration
8) Sign off!

Sponsored-by Catalyst IT
Comment 46 Hayley Pelham 2019-04-14 22:29:27 UTC
Created attachment 87966 [details] [review]
Bug 18308: Added missing filter in memberentrygen.tt

Test plan:
Apply this patch and check that everything still works.

Sponsored-by: Catalyst IT
Comment 47 Liz Rea 2019-04-15 14:51:12 UTC
Created attachment 88010 [details] [review]
Bug 18308: Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

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

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 48 Liz Rea 2019-04-15 14:51:16 UTC
Created attachment 88011 [details] [review]
Bug 18308: Added warning and dialog alert

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 49 Liz Rea 2019-04-15 14:51:20 UTC
Created attachment 88012 [details] [review]
Bug 18308: Fixed issues with onboarding_messages.inc and onboardingstep6.tt

Sponsored-by: Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 50 Liz Rea 2019-04-15 14:51:25 UTC
Created attachment 88013 [details] [review]
Bug 18308: Changed ordering of onboarding steps

This patch cleans up the previous patches according to comment 23, fixes
various typos, and changes the order of steps in the onboarding so that
the setting of minPasswordLength occurs before the admin password is
set.

Test plan:
1) Drop and recreate your database, and restart_all
2) Go through installation and onboarding
3) Note that the order of steps
is:
 1 Create a library
 2 Create a ptron category
 3 Create Koha administrator patron
 4 Create a new item type
 5 Create a new circulation rule
 6 Set minPasswordLength syspref/Complete
4) Repeat steps 1 & 2
5) Note that the order of steps is now:
 1 Create a library
 2 Create a patron category
 3 Set minPasswordLength syspref
 4 Create Koha administrator patron
 5 Create a new item type
 6 Create a new circulation rule/Complete
6) Verify that the minPasswordLength syspref is set to the
    value from onboarding
7) Verify that the patron, item type, and circulation rule
    made in the onboarding exist in the administration
8) Sign off!

Sponsored-by Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 51 Liz Rea 2019-04-15 14:51:29 UTC
Created attachment 88014 [details] [review]
Bug 18308: Added missing filter in memberentrygen.tt

Test plan:
Apply this patch and check that everything still works.

Sponsored-by: Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 52 Katrin Fischer 2019-04-17 06:15:43 UTC
Hi Hayley, can you please rebase? I think the conflict might have been caused by bug 22451 being pushed this week.
Comment 53 Josef Moravec 2019-04-17 06:38:07 UTC
(In reply to Katrin Fischer from comment #52)
> Hi Hayley, can you please rebase? I think the conflict might have been
> caused by bug 22451 being pushed this week.

So now it is possible to use Asset in installer and onboarding templates ;)
Comment 54 Hayley Pelham 2019-04-17 22:03:39 UTC
Created attachment 88242 [details] [review]
Bug 18308: Changed templates to use Asset plugin rather than full path

Test plan:
Same as before

Sponsored-by: Catalyst IT
Comment 55 Hayley Pelham 2019-04-17 22:41:40 UTC
Created attachment 88243 [details] [review]
Bug 18308: Removed extraneous file

Sponsored-by: Catalyst IT
Comment 56 Hayley Pelham 2019-04-17 22:45:16 UTC
Test plan:

0) Apply all patches
1) Drop and recreate your database, and restart_all
2) Go through installation and onboarding
3) Note that the order of steps
is:
 1 Create a library
 2 Create a ptron category
 3 Create Koha administrator patron
 4 Create a new item type
 5 Create a new circulation rule
 6 Set minPasswordLength syspref/Complete
4) Repeat steps 1 & 2
5) Note that the order of steps is now:
 1 Create a library
 2 Create a patron category
 3 Set minPasswordLength syspref
 4 Create Koha administrator patron
 5 Create a new item type
 6 Create a new circulation rule/Complete
6) Verify that the minPasswordLength syspref is set to the
    value from onboarding
7) Verify that the patron, item type, and circulation rule
    made in the onboarding exist in the administration
8) Sign off!
Comment 57 Liz Rea 2019-04-18 15:49:05 UTC
Created attachment 88306 [details] [review]
Bug 18308: Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

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

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 58 Liz Rea 2019-04-18 15:49:15 UTC
Created attachment 88307 [details] [review]
Bug 18308: Added warning and dialog alert

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 59 Liz Rea 2019-04-18 15:49:19 UTC
Created attachment 88308 [details] [review]
Bug 18308: Fixed issues with onboarding_messages.inc and onboardingstep6.tt

Sponsored-by: Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 60 Liz Rea 2019-04-18 15:49:23 UTC
Created attachment 88309 [details] [review]
Bug 18308: Changed ordering of onboarding steps

This patch cleans up the previous patches according to comment 23, fixes
various typos, and changes the order of steps in the onboarding so that
the setting of minPasswordLength occurs before the admin password is
set.

Test plan:
1) Drop and recreate your database, and restart_all
2) Go through installation and onboarding
3) Note that the order of steps
is:
 1 Create a library
 2 Create a ptron category
 3 Create Koha administrator patron
 4 Create a new item type
 5 Create a new circulation rule
 6 Set minPasswordLength syspref/Complete
4) Repeat steps 1 & 2
5) Note that the order of steps is now:
 1 Create a library
 2 Create a patron category
 3 Set minPasswordLength syspref
 4 Create Koha administrator patron
 5 Create a new item type
 6 Create a new circulation rule/Complete
6) Verify that the minPasswordLength syspref is set to the
    value from onboarding
7) Verify that the patron, item type, and circulation rule
    made in the onboarding exist in the administration
8) Sign off!

Sponsored-by Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 61 Liz Rea 2019-04-18 15:49:27 UTC
Created attachment 88310 [details] [review]
Bug 18308: Added missing filter in memberentrygen.tt

Test plan:
Apply this patch and check that everything still works.

Sponsored-by: Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 62 Liz Rea 2019-04-18 15:49:31 UTC
Created attachment 88311 [details] [review]
Bug 18308: Changed templates to use Asset plugin rather than full path

Test plan:
Same as before

Sponsored-by: Catalyst IT
Comment 63 Liz Rea 2019-04-18 15:49:35 UTC
Created attachment 88312 [details] [review]
Bug 18308: Removed extraneous file

Sponsored-by: Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 64 Liz Rea 2019-04-18 15:49:39 UTC
Created attachment 88313 [details] [review]
Bug 18308: (QA follow up) Resolving merge conflict

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 65 Liz Rea 2019-04-18 15:51:56 UTC
Created attachment 88314 [details] [review]
Bug 18308: Added step to onboarding tool to set minPasswordLength

This patch increases the default minPasswordLength syspref value to 8
characters, for new installations.

The final (6th step) of the onboarding tool is now a password length
page where the default value of 8 can be altered. There is a security warning in red recommending to the user they
keep the minimum length of the password at 8 characters or more.

This patch also removes the atomicupdate .sql file to be run in an update to alter the default
minPasswordLength value for existing Koha installations, based on tester
feedback.

Test plan:
1. Create a patron with a password less than 3 characters in length and
notice that a red message is displayed by the input telling you that the
password must be 3 characters minimum length

2. Query the database "select value from systempreferences where
variable="minPasswordLength"; and notice the value is 3

3. Drop and recreate your database and restart memcached

4. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to create a circulation rule

5. Apply patch

6. Repeat step 3

7. Go through the web installer and onboarding tool. Noticing the last
step of the onboarding tool is to set the minimum password length, the
numerical input element has a default value of 8 and notice that it will
go below 3

8. Repeat step 2 and notice the value is 8

9. In the onboarding tool change the minimum password length value to 7
and submit the form

10. Notice the completed page of the onboarding tool is displayed with
the message that the minimum password length has been set.

11. Repeat step 2 and notice the value is now 7.

12. Try to create a patron in Intranet and OPAC with a password less
than 7 characters and notice that a red message is displayed by the
input telling you that the password must be 7 characters minimum length

Sponsored-By: Catalyst IT

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

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

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 66 Liz Rea 2019-04-18 15:52:06 UTC
Created attachment 88315 [details] [review]
Bug 18308: Added warning and dialog alert

If an institution has a minPasswordLength which is < 8 characters then a warning
will be set (which is visible in the intranet-error.log) and a dialog
alert div will display the warning to the user in the memberentry.pl
page in the intranet.

Test plan:
1. Ensure that your minPasswordLength is set to less than 8 characters.
This replicates the use case that a Koha instance is existing (i.e. not
a new Koha instance) and not gone through the password length step of
the onboarding tool (previous patch on this bug report) or that the
institituion is new but set a password length of less than 8 in the
onboarding tool

2. Go to the memberentry.pl page and notice that there is no warning
at the top of the page informing you that your minPasswordLength is less
than 8

3. Apply this patch

4. Refresh the memberentry.pl page and notice that there is a yellow
warning at the top of the page informing you that your minPasswordLength
syspref is less than 8

Sponsored-By: Catalyst IT
Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 67 Liz Rea 2019-04-18 15:52:09 UTC
Created attachment 88316 [details] [review]
Bug 18308: Fixed issues with onboarding_messages.inc and onboardingstep6.tt

Sponsored-by: Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 68 Liz Rea 2019-04-18 15:52:13 UTC
Created attachment 88317 [details] [review]
Bug 18308: Changed ordering of onboarding steps

This patch cleans up the previous patches according to comment 23, fixes
various typos, and changes the order of steps in the onboarding so that
the setting of minPasswordLength occurs before the admin password is
set.

Test plan:
1) Drop and recreate your database, and restart_all
2) Go through installation and onboarding
3) Note that the order of steps
is:
 1 Create a library
 2 Create a ptron category
 3 Create Koha administrator patron
 4 Create a new item type
 5 Create a new circulation rule
 6 Set minPasswordLength syspref/Complete
4) Repeat steps 1 & 2
5) Note that the order of steps is now:
 1 Create a library
 2 Create a patron category
 3 Set minPasswordLength syspref
 4 Create Koha administrator patron
 5 Create a new item type
 6 Create a new circulation rule/Complete
6) Verify that the minPasswordLength syspref is set to the
    value from onboarding
7) Verify that the patron, item type, and circulation rule
    made in the onboarding exist in the administration
8) Sign off!

Sponsored-by Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 69 Liz Rea 2019-04-18 15:52:17 UTC
Created attachment 88318 [details] [review]
Bug 18308: Added missing filter in memberentrygen.tt

Test plan:
Apply this patch and check that everything still works.

Sponsored-by: Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 70 Liz Rea 2019-04-18 15:52:21 UTC
Created attachment 88319 [details] [review]
Bug 18308: Changed templates to use Asset plugin rather than full path

Test plan:
Same as before

Sponsored-by: Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 71 Liz Rea 2019-04-18 15:52:25 UTC
Created attachment 88320 [details] [review]
Bug 18308: Removed extraneous file

Sponsored-by: Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 72 Liz Rea 2019-04-18 15:52:28 UTC
Created attachment 88321 [details] [review]
Bug 18308: (QA follow up) Resolving merge conflict

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 73 Kyle M Hall 2019-04-24 19:26:54 UTC
Comment on attachment 88314 [details] [review]
Bug 18308: Added step to onboarding tool to set minPasswordLength

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

::: installer/onboarding.pl
@@ +292,5 @@
>  
> +if ( $step == 6 ) {
> +    if ( $op eq 'set_validate_min_password_length' ) {
> +        my $minPasswordLength   = $input->param('pref_minPasswordLength');
> +        warn $minPasswordLength;

This should be removed.
Comment 74 Kyle M Hall 2019-04-24 19:27:44 UTC
Comment on attachment 88315 [details] [review]
Bug 18308: Added warning and dialog alert

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

::: members/memberentry.pl
@@ +174,5 @@
> +#Check passwords
> +my $minPasswordLength = C4::Context->preference('minPasswordLength');
> +warn $minPasswordLength;
> +if ($minPasswordLength < 8) {
> +    warn my $warning = "Your minPasswordLength system preference is less than 8 characters this means library patrons can potentially set passwords which are vulnerable to cracking. Please set your minPasswordLength syspref to 8 or more characters";

This is not translatable and should not be warn'ed.
Comment 75 Kyle M Hall 2019-04-24 19:29:45 UTC
Remove the warns and move the string from the perl script to the template and this should be good to go!
Comment 76 Hayley Pelham 2019-04-26 02:55:20 UTC
Created attachment 88840 [details] [review]
Bug 18308: Removed warns and moved password length warning to template

Sponsored-by: Catalyst IT
Comment 77 Liz Rea 2019-05-24 18:02:21 UTC
Created attachment 90079 [details] [review]
Bug 18308: Removed warns and moved password length warning to template

Sponsored-by: Catalyst IT

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 78 Jonathan Druart 2019-05-25 00:33:28 UTC
1. Only one occurrence of passwordlengthhint
koha-tmpl/intranet-tmpl/prog/css/installer.css:#passwordlengthhint

2. Do we really need the warning when adding/editing a patron?
BTW, $warning is not needed, memberentry.pl
177     my $warning = 1;
178     $template->param("passwordwarning"=>$warning);

3. Just a though, next time we add a "step" we will need to rethink the template.  The shift is terrible. And hard to see the real changes.

Not passing QA for 2, waiting for another QA point of view.
Comment 79 Marcel de Rooy 2019-05-31 08:51:05 UTC
(In reply to Jonathan Druart from comment #78)
> 1. Only one occurrence of passwordlengthhint
> koha-tmpl/intranet-tmpl/prog/css/installer.css:#passwordlengthhint
> 
> 2. Do we really need the warning when adding/editing a patron?
> BTW, $warning is not needed, memberentry.pl
> 177     my $warning = 1;
> 178     $template->param("passwordwarning"=>$warning);
> 
> 3. Just a though, next time we add a "step" we will need to rethink the
> template.  The shift is terrible. And hard to see the real changes.
> 
> Not passing QA for 2, waiting for another QA point of view.

I would suggest to squash some patches, getting rid of merge marker stuff and warnings etc.
In general, I would not favor putting more prefs inside onboarding. Keep it simple. This change can be made without changing onboarding.
Comment 80 Katrin Fischer 2019-06-02 08:22:56 UTC
(In reply to Marcel de Rooy from comment #79)
> (In reply to Jonathan Druart from comment #78)
> > 1. Only one occurrence of passwordlengthhint
> > koha-tmpl/intranet-tmpl/prog/css/installer.css:#passwordlengthhint
> > 
> > 2. Do we really need the warning when adding/editing a patron?
> > BTW, $warning is not needed, memberentry.pl
> > 177     my $warning = 1;
> > 178     $template->param("passwordwarning"=>$warning);
> > 
> > 3. Just a though, next time we add a "step" we will need to rethink the
> > template.  The shift is terrible. And hard to see the real changes.
> > 
> > Not passing QA for 2, waiting for another QA point of view.
> 
> I would suggest to squash some patches, getting rid of merge marker stuff
> and warnings etc.
> In general, I would not favor putting more prefs inside onboarding. Keep it
> simple. This change can be made without changing onboarding.

I tend to agree with Marcel and Jonathan here. 

I am not sure if we should add this to the installer as it adds a lot more to read for little gain (slows down the process). I think instead, why not just set it to at least 8 by default?
Comment 81 Nick Clemens 2019-06-06 11:12:09 UTC
I think Jonathan had agreement, setting this as FQA
Comment 82 Aleisha Amohia 2020-04-23 23:57:02 UTC
Created attachment 103617 [details] [review]
Bug 18308: Increase minPasswordLength to 8 characters

for new installs

To test:
1) Go to Administration -> system preferences -> ensure minPasswordLength syspref is set to 3, or some other number less than 8
2) Go to Patrons -> Add a new patron
3) Notice the alert at the top of the page warning that
minPasswordLength is too low. Confirm you are still be able to save your
patron with a password that is less than 8 characters.
4) Edit the minPasswordLength syspref to be 8 or higher.
5) Go back to Patrons -> Add a new patron. Confirm the alert is gone.
6) Drop and recreate database to trigger onboarding tool.
7) Go through onboarding tool until you are required to create the
superlibrarian patron. Try to enter a password of less than 8
characters. Confirm it does not let you continue until you enter a
password that is 8 characters or longer.

Sponsored-by: Catalyst IT
Comment 83 Aleisha Amohia 2020-04-23 23:58:57 UTC
Felt like this was a bit of a mess before, so have written a new patch. Let me know if there's anything missing, otherwise it's ready to test.
Comment 84 Jonathan Druart 2020-04-24 07:42:03 UTC
Hi Aleisha,

I think we should keep 3 as a minimum if the pref is < 3, and not 8 if < 8.

-    $minPasswordLength = 3 if not $minPasswordLength or $minPasswordLength < 3;
Comment 85 Marcel de Rooy 2020-04-24 08:10:37 UTC
(In reply to Jonathan Druart from comment #84)
> Hi Aleisha,
> 
> I think we should keep 3 as a minimum if the pref is < 3, and not 8 if < 8.
> 
> -    $minPasswordLength = 3 if not $minPasswordLength or $minPasswordLength
> < 3;

I think we should really get rid of such a hardcoded low value. The pref should be filled and respected ("own responsibility?"). But if it is undefined, we should certainly not keep it at 3.
Comment 86 Jonathan Druart 2020-04-24 09:10:14 UTC
(In reply to Marcel de Rooy from comment #85)
> (In reply to Jonathan Druart from comment #84)
> > Hi Aleisha,
> > 
> > I think we should keep 3 as a minimum if the pref is < 3, and not 8 if < 8.
> > 
> > -    $minPasswordLength = 3 if not $minPasswordLength or $minPasswordLength
> > < 3;
> 
> I think we should really get rid of such a hardcoded low value. The pref
> should be filled and respected ("own responsibility?"). But if it is
> undefined, we should certainly not keep it at 3.

That is bug 21314.
Comment 87 Marcel de Rooy 2020-04-24 09:22:11 UTC
(In reply to Jonathan Druart from comment #86)
> (In reply to Marcel de Rooy from comment #85)
> > (In reply to Jonathan Druart from comment #84)
> > > Hi Aleisha,
> > > 
> > > I think we should keep 3 as a minimum if the pref is < 3, and not 8 if < 8.
> > > 
> > > -    $minPasswordLength = 3 if not $minPasswordLength or $minPasswordLength
> > > < 3;
> > 
> > I think we should really get rid of such a hardcoded low value. The pref
> > should be filled and respected ("own responsibility?"). But if it is
> > undefined, we should certainly not keep it at 3.
> 
> That is bug 21314.

You have a circular problem. Comment14 from that bug referred me here. Where we adjust it, does not matter to me. We should adjust it.
Comment 88 Jonathan Druart 2020-04-24 09:41:56 UTC
Here we only need to modify the default value of the pref to 8, for new installs.
Comment 89 Aleisha Amohia 2020-04-27 20:37:53 UTC
If this feature is about increasing the minimum password length, then I don't see why we can't include the fix for Bug 21314 in this patch as it is. They go together.
Comment 90 Katrin Fischer 2020-04-27 21:46:11 UTC
If I understand correctly the problematic line is:

 $minPasswordLength = 8 if not $minPasswordLength or $minPasswordLength < 8;

So if we kept the patch as attached, I think we will break people's workflows. If you have set to a lower password value now, Koha will suddenly force you to use a longer password. Another use case I could see is 4 digit PINs for self checks. I know some older self checks don't support alphanumeric, not sure about length.

I feel like we should continue the discussion on a hardcoded value on bug 18308 and keep this one to just change the default value we ship Koha with. It will be a step forward that we can get through quickly.
Comment 91 Bernardo Gonzalez Kriegel 2020-04-30 23:20:36 UTC
(In reply to Katrin Fischer from comment #90)
> 
> I feel like we should continue the discussion on a hardcoded value on bug
> 18308 and keep this one to just change the default value we ship Koha with.
> It will be a step forward that we can get through quickly.

This is bug 18308 :)

What about keeping only the second (comment #4) patch?
It's the only way not to break people's workflows.
Comment 92 Katrin Fischer 2020-04-30 23:25:01 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #91)
> (In reply to Katrin Fischer from comment #90)
> > 
> > I feel like we should continue the discussion on a hardcoded value on bug
> > 18308 and keep this one to just change the default value we ship Koha with.
> > It will be a step forward that we can get through quickly.
> 
> This is bug 18308 :)
> 
> What about keeping only the second (comment #4) patch?
> It's the only way not to break people's workflows.

Sorry, me and bug numbers.... I meant bug 21314. c#4 looks good to me.
Comment 93 David Cook 2020-05-01 00:49:18 UTC
Wow... I can't believe this issue has been open for over 3 years, and it still defaults to 3.

We should do a patch to sysprefs.sql to raise it to 10 (number given by a security consultant).

Let's just push that out ASAP, and then worry about existing installations.
Comment 94 Katrin Fischer 2020-05-03 21:06:57 UTC
Aleisha, would you be ok with using comment #4 patch for now? I would give it a sign-off right away.
Comment 95 Aleisha Amohia 2020-05-03 21:08:11 UTC
(In reply to Katrin Fischer from comment #94)
> Aleisha, would you be ok with using comment #4 patch for now? I would give
> it a sign-off right away.

Yes absolutely. Will make those patch changes right now.
Comment 96 Katrin Fischer 2020-05-03 21:11:38 UTC
Created attachment 104235 [details] [review]
Bug 18308 - Followup adding default value to syspref.sql

Sponsored-By: Catalyst IT

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 97 Katrin Fischer 2020-05-03 21:12:20 UTC
Using my QA powers to move this to PQA directly.
Comment 98 Katrin Fischer 2020-05-03 21:16:08 UTC
Created attachment 104236 [details] [review]
Bug 18308: Increase minPasswordLength default to 8

Sponsored-By: Catalyst IT

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

To test:
- Run the web installer for any language
- Verify that minPasswordLength is set to 8 now
Comment 99 David Cook 2020-05-03 23:18:38 UTC
(In reply to Katrin Fischer from comment #97)
> Using my QA powers to move this to PQA directly.

You're the best, Katrin :).
Comment 100 Martin Renvoize 2020-05-04 07:54:49 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 101 Jonathan Druart 2020-05-05 08:27:36 UTC
Created attachment 104351 [details] [review]
Bug 18308: fix failing tests

#   Failed test 'check_password hook tests'
 #   at t/db_dependent/Koha/Plugins/Patron.t line 83.
 [The password was rejected by a plugin]# Looks like your test exited with 255 just after 5.
Comment 102 Joy Nelson 2020-05-11 20:29:02 UTC
not backported to 19.11.x