Bug 18308 - Default value of minPasswordLength should be increased
Summary: Default value of minPasswordLength should be increased
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Alex Buckley
QA Contact: Josef Moravec
URL:
Whiteboard:
Keywords:
: 9920 (view as bug list)
Depends on: 19514
Blocks:
  Show dependency treegraph
 
Reported: 2017-03-21 13:18 UTC by Jonathan Druart
Modified: 2017-12-14 11:21 UTC (History)
6 users (show)

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


Attachments
Bug 18308 - Increased default value of minPasswordValue (1.52 KB, patch)
2017-10-23 09:06 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18308 - Followup adding default value to syspref.sql (1.53 KB, patch)
2017-10-23 09:49 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18308 - Added step to onboarding tool to set minPasswordLength (7.51 KB, patch)
2017-10-23 23:03 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18308 - Added step to onboarding tool to set minPasswordLength (7.67 KB, patch)
2017-11-23 22:54 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18308 - Added warning and dialog alert (3.26 KB, patch)
2017-11-23 22:55 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18308 - Added step to onboarding tool to set minPasswordLength (7.67 KB, patch)
2017-11-23 23:47 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18308 - Added warning and dialog alert (6.12 KB, patch)
2017-11-23 23:48 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18308 - Added step to onboarding tool to set minPasswordLength (10.82 KB, patch)
2017-11-24 01:02 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18308 - Added warning and dialog alert (4.43 KB, patch)
2017-11-24 01:05 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18308 - Added warning and dialog alert (4.58 KB, patch)
2017-12-01 21:02 UTC, Dominic Pichette
Details | Diff | Splinter Review
Bug 18308 - Added step to onboarding tool to set minPasswordLength (10.88 KB, patch)
2017-12-01 21:24 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 18308 - Added warning and dialog alert (4.59 KB, patch)
2017-12-01 21:24 UTC, David Bourgault
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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