Bug 12617 - Koha should let admins to configure automatically generated password complexity/difficulty
Summary: Koha should let admins to configure automatically generated password complexi...
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
: 18424 (view as bug list)
Depends on: 23816
Blocks:
  Show dependency treegraph
 
Reported: 2014-07-21 06:40 UTC by Juhani Seppälä
Modified: 2023-09-19 10:44 UTC (History)
13 users (show)

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


Attachments
Bug 12617 - Koha should let admins to configure automatically generated password complexity/difficulty (17.89 KB, patch)
2015-04-13 13:37 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12617: Koha should let admins to configure automatically generated password complexity/difficulty (62.23 KB, patch)
2020-07-27 07:22 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12617: Add new regex patterns to password_check.inc (5.80 KB, patch)
2020-07-27 07:23 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12617: Add syspref PasswordLengthsForCategories (9.89 KB, patch)
2020-07-27 07:32 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12617: Koha should let admins to configure automatically generated password complexity/difficulty (59.06 KB, patch)
2020-09-01 07:41 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12617: Add new regex patterns to password_check.inc (15.19 KB, patch)
2020-09-01 07:42 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12617: Koha should let admins to configure automatically generated password complexity/difficulty (58.77 KB, patch)
2020-09-22 07:14 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12617: DO NOT PUSH! Schema change (1014 bytes, patch)
2020-09-22 07:14 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12617: Add new regex patterns to password_check.inc (15.16 KB, patch)
2020-09-22 07:15 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12617: Koha should let admins to configure automatically generated password complexity/difficulty (58.75 KB, patch)
2020-10-06 12:24 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12617: DO NOT PUSH! Schema change (1014 bytes, patch)
2020-10-06 12:25 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12617: Add new regex patterns to password_check.inc (17.10 KB, patch)
2020-10-06 12:26 UTC, Emmi Takkinen
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Juhani Seppälä 2014-07-21 06:40:08 UTC
Currently, the suggested passwords generated by members/member-password only honor the minimum password length configuration setting - we've had some discussions in my library that we'd want to be able to configure the suggested passwords - not only in terms of length, but also in terms of complexity/difficulty. This could be easily achieved by adding new global configuration setting and refactoring the current password generation to use a standard random string generator library from the cpan - such as Data::Random or something similiar.

see also: bug 11888 - I looked into enhancing the password generation while preparing this patch.
Comment 1 Olli-Antti Kivilahti 2015-04-13 13:37:07 UTC
Created attachment 37740 [details] [review]
Bug 12617 - Koha should let admins to configure automatically generated password complexity/difficulty

Adds simple password policy(with regards to complexity) management into categories:
- Per category password policy: admins can configure what kind of passwords get generated
in member-passwords. User-created passwords are also checked against the policy if it is
defined and complexity is enforced for every user based on their set category.
- Reworks the old custom password generation code in member-password to use a pretty powerful perl
module from the CPAN: App::Genpass

- Predefined policies:
	- simplenumeric: the digits 0-9 allowed only
	- alphanumeric: passwords must contain only the digits 0-9 and lowercase and uppercase characters.
	Special characters are not allowed.
	- complex: patrons are required to use complex passwords containing numbers, uppercase and lowercase
	characters and special characters.
Comment 2 Marc Véron 2017-07-31 15:54:35 UTC
*** Bug 18424 has been marked as a duplicate of this bug. ***
Comment 3 Magnus Enger 2019-04-15 11:27:58 UTC
Changing to NSO since there is a 4 years old patch.
Comment 4 Emmi Takkinen 2020-07-21 10:49:09 UTC
I've been working with rebase of this patch (or technically it's patch fetched from https://github.com/KohaSuomi/Koha including changes to OPAC and API). Hopefully I'll be able to provide it and some additional changes during this week.
Comment 5 Emmi Takkinen 2020-07-27 07:22:37 UTC
Created attachment 107382 [details] [review]
Bug 12617: Koha should let admins to configure automatically generated password complexity/difficulty

Adds simple password policy(with regards to complexity) management into categories:
- Per category password policy: admins can configure what kind of passwords get generated
in member-passwords. User-created passwords are also checked against the policy if it is
defined and complexity is enforced for every user based on their set category.
- Predefined policies:
	- simplenumeric: the digits 0-9 allowed only
	- alphanumeric: passwords must contain only the digits 0-9 and lowercase and uppercase characters.
	Special characters are not allowed.
	- complex: patrons are required to use complex passwords containing numbers, uppercase and lowercase
	characters and special characters.
Old passwords for excisting patrons are not affected.

To test:
1. Apply this patch and update database.
2. Navigate to categories.pl and note there is new column 'Password policies' has been added.
3. Edit some categories and set password policy for them.
4. Set some values to sysprefs 'minPasswordLength', 'minAlnumPasswordLength' and
'minComplexPasswordLength'.

Staff interface:
1. Create new patron.
2. Set their password against their categorys policy and save.
3. Error message is displayed (with content depending on password policy).
4. Set acceptable password and save succesfully.
5. Repeat steps 2-3-4 on patron edit page.
6. Repeat steps 2-3-4 on 'Change password' page.

OPAC:
1. Enable 'OpacPasswordChange' and 'OpacResetPassword'.
2. On OPAC, repeat what you did on staff interface (on create, edit and 'Change your password'.
3. Confirm errors are displayed correctly and saving works.
4. Log out and go to 'Forgotten password recovery' page.
5. Send and receive email for password recovery.
6. Set unacceptable password and save, confirm correct error is displayed.
7. Set acceptable password and save succesfully.

REST API:
1. With your preferred REST client (curl e.g) sent POST request to /api/v1/patrons/{patron_id}/password
with 'password' and 'password_2' parameters.
2. Confirm correct error message is displayed when sending password against password policy.
3. Confirm password is changed when acceptable password is send.

Also prove t/AuthUtils.t and t/db_dependent/api/v1/patrons_password.t

Sponsored-by: Koha-Suomi Oy

---

NOTE: Unlike in original patch, no App::GenPass is used due it's complex dependencies. I've instead added new patterns to follow in sub generate_password and member-password.tt when generating password successions.
Comment 6 Emmi Takkinen 2020-07-27 07:23:52 UTC
Created attachment 107383 [details] [review]
Bug 12617: Add new regex patterns to password_check.inc

This patch adds new regex patterns to follow into password_check.inc
files.

To test:
1. Create or find existing patron.
2. Set or edit invalid password for patron in OPAC and staff interfaces:
	a) create and edit patron pages
	b) 'Change password' pages
3. Confirm alert text next to input field is displayed and it follows
set passwordpolicy.

Sponsored-by: Koha-Suomi Oy
Comment 7 Emmi Takkinen 2020-07-27 07:32:49 UTC
Created attachment 107384 [details] [review]
Bug 12617: Add syspref PasswordLengthsForCategories

Sometimes different patron categories using same password policy would
require different lengths in their passwords. In these cases having one
length per policy isn't always ideal.

This patch adds new syspref 'PasswordLengthsForCategories' to define
password lengths for different patron categories in YAML format and removes
use of 'minAlnumPasswordLength' and 'minComplexPasswordLength'. If category
has no rule set syspref 'minPasswordLength' is used.

To test:
1. Run updatedatabase.pl
2. Define some rules to 'PasswordLengthsForCategories' (e.g example provided)
3. Try setting and changing password in different pages (basically follow test
plans from previous patches).
4. Confirm error messages are displayed correctly.

Also prove t/AuthUtils.t.

Sponsored-by: Koha-Suomi Oy
Comment 8 Jonathan Druart 2020-08-19 07:11:29 UTC
This is going to conflict with bug 23816, it needs to be rebuilt on top of it.
Comment 9 Emmi Takkinen 2020-09-01 07:41:29 UTC
Created attachment 109388 [details] [review]
Bug 12617: Koha should let admins to configure automatically generated password complexity/difficulty

Rebased and reworked patch against bug 23816. Changes made 'PasswordLengthsForCategories' syspref obsolete so patch for it is dropped.
Comment 10 Emmi Takkinen 2020-09-01 07:42:15 UTC
Created attachment 109389 [details] [review]
Bug 12617: Add new regex patterns to password_check.inc

Rebased and reworked patch.
Comment 11 Emmi Takkinen 2020-09-22 07:14:31 UTC
Created attachment 110526 [details] [review]
Bug 12617: Koha should let admins to configure automatically generated password complexity/difficulty

Adds simple password policy(with regards to complexity) management into categories:
- Per category password policy: admins can configure what kind of passwords get generated
in member-passwords. User-created passwords are also checked against the policy if it is
defined and complexity is enforced for every user based on their set category.
- Predefined policies:
	- simplenumeric: the digits 0-9 allowed only
	- alphanumeric: passwords must contain only the digits 0-9 and lowercase and uppercase characters.
	Special characters are not allowed.
	- complex: patrons are required to use complex passwords containing numbers, uppercase and lowercase
	characters and special characters.
Old passwords for excisting patrons are not affected.

To test:
1. Apply this patch and update database.
2. Navigate to categories.pl and note there is new column 'Password policies' has been added.
3. Edit some categories and set password policy for them.
4. Set some values to sysprefs 'minPasswordLength', 'minAlnumPasswordLength' and
'minComplexPasswordLength'.

Staff interface:
1. Create new patron.
2. Set their password against their categorys policy and save.
3. Error message is displayed (with content depending on password policy).
4. Set acceptable password and save succesfully.
5. Repeat steps 2-3-4 on patron edit page.
6. Repeat steps 2-3-4 on 'Change password' page.

OPAC:
1. Enable 'OpacPasswordChange' and 'OpacResetPassword'.
2. On OPAC, repeat what you did on staff interface (on create, edit and 'Change your password'.
3. Confirm errors are displayed correctly and saving works.
4. Log out and go to 'Forgotten password recovery' page.
5. Send and receive email for password recovery.
6. Set unacceptable password and save, confirm correct error is displayed.
7. Set acceptable password and save succesfully.

REST API:
1. With your preferred REST client (curl e.g) sent POST request to /api/v1/patrons/{patron_id}/password
with 'password' and 'password_2' parameters.
2. Confirm correct error message is displayed when sending password against password policy.
3. Confirm password is changed when acceptable password is send.

Also prove t/AuthUtils.t and t/db_dependent/api/v1/patrons_password.t

Sponsored-by: Koha-Suomi Oy
Comment 12 Emmi Takkinen 2020-09-22 07:14:58 UTC
Created attachment 110527 [details] [review]
Bug 12617: DO NOT PUSH! Schema change
Comment 13 Emmi Takkinen 2020-09-22 07:15:37 UTC
Created attachment 110528 [details] [review]
Bug 12617: Add new regex patterns to password_check.inc

This patch adds new regex patterns to follow into password_check.inc
files.

To test:
1. Create or find existing patron.
2. Set or edit invalid password for patron in OPAC and staff interfaces:
	a) create and edit patron pages
	b) 'Change password' pages
3. Confirm alert text next to input field is displayed and it follows
set passwordpolicy.

Sponsored-by: Koha-Suomi Oy
Comment 14 Fridolin Somers 2020-09-25 08:57:39 UTC
Hi, nice work.

Just a question :
I see in first patch : $password =~ /[a-zA-ZöäåÖÄÅ]/
I don't see the caracters öäå in patch on password_check.inc.

Should we use POSIX =character class like [:alnum:] ?
https://www.regular-expressions.info/posixbrackets.html
Comment 15 Emmi Takkinen 2020-09-29 11:14:28 UTC
(In reply to Fridolin SOMERS from comment #14)
> Hi, nice work.
> 
> Just a question :
> I see in first patch : $password =~ /[a-zA-ZöäåÖÄÅ]/
> I don't see the caracters öäå in patch on password_check.inc.
> 
> Should we use POSIX =character class like [:alnum:] ?
> https://www.regular-expressions.info/posixbrackets.html

With Perl we could yes. However Javascript doesn't seem to support POSIX. [a-zäöå] and [A-ZÄÖÅ] seem to work. 

I guess we could also use POSIX in first patch and Unicode on password_check.inc e.g \u00DF-\u00FF and allow characters like éëêẽ etc.
Comment 16 Fridolin Somers 2020-09-29 12:55:52 UTC
> However Javascript doesn't seem to support POSIX
Ah ok good point.

But why are öäåÄÖÅ not in :
  if ( password_policy == 'complex' ){
    chars = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ|[]{}!@#$%^&*()_-+?';

In my opinion, for a heavy international software like Koha you may stick to [a-zA-Z] as the only alphabetic characters.

Or create lang-based policies complex-en, complex-fi ...
Comment 17 David Cook 2020-09-29 23:58:34 UTC
(In reply to Fridolin SOMERS from comment #16)
> > However Javascript doesn't seem to support POSIX
> Ah ok good point.
> 
> But why are öäåÄÖÅ not in :
>   if ( password_policy == 'complex' ){
>     chars =
> '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ|[]{}!
> @#$%^&*()_-+?';
> 
> In my opinion, for a heavy international software like Koha you may stick to
> [a-zA-Z] as the only alphabetic characters.
> 
> Or create lang-based policies complex-en, complex-fi ...

I think the default with password managers tend to be printable ASCII characters (ie: Upper-case, Lower-case, Digits, Special). That said, I have seen "High ANSI characters" as an option...

But then I thought about Chinese password habits (https://medium.com/@ye.sunnia/an-analysis-of-chinese-passwords-e49b97b91919 or https://www.theverge.com/tldr/2019/3/5/18252150/bad-password-security-data-breach-taiwan-ji32k7au4a83-have-i-been-pwned), which seem to fall into ASCII. 

I just spun up a Keycloak container (an Identity Provider created by Red Hat), and I'm trying to reset my password (as a user) to a Chinese password in Windows Chrome, but it seems to be preventing my software-based pinyin input from working. It seems to be forcing my hardware keyboard. (Like if I type in "wo", I see 2 masked characters appearing in the password field, rather than being able to select the 1 我 character.)

That said, as a Keycloak admin, I was able to input a 我 character into the user's password field. I wasn't able to manually enter it as a user, but if I copied and pasted 我 into the password field as a user, it worked. 

Going back to the user view, I notice when I move from the username field to the password field, my software keyboard changes modes from Chinese mode to English mode. If manually change the mode... it doesn't seem to make a difference. 

Here's some reading on Keycloak password policies: https://www.keycloak.org/docs/latest/server_admin/#password-policy-types, which might be useful.
Comment 18 David Cook 2020-09-30 00:20:04 UTC
(In reply to Fridolin SOMERS from comment #16)
> > However Javascript doesn't seem to support POSIX
> Ah ok good point.
> 
> But why are öäåÄÖÅ not in :
>   if ( password_policy == 'complex' ){
>     chars =
> '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ|[]{}!
> @#$%^&*()_-+?';
> 
> In my opinion, for a heavy international software like Koha you may stick to
> [a-zA-Z] as the only alphabetic characters.
> 
> Or create lang-based policies complex-en, complex-fi ...

Based on my earlier comment, especially regarding Red Hat's Keycloak password policies, I think that limiting to '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ|[]{}!@#$%^&*()_-+?' might not be wise for a "complex" policy.

Look at pam_cracklib too which is used by massive international Linux providers (https://linux.die.net/man/8/pam_cracklib and https://github.com/linux-pam/linux-pam/blob/master/modules/pam_cracklib/pam_cracklib.c). There complexity is determined by "minlen, maxclassrepeat, dcredit, ucredit, lcredit, and ocredit". 

Looking at https://github.com/linux-pam/linux-pam/blob/master/modules/pam_cracklib/pam_cracklib.c, it looks like the ctype library includes functions for isupper, islower,  isdigit, etc. "ocredit" for "other characters" is just whatever isn't a digit, uppercase letter, or lowercase letter. The Javascript would be more verbose, but it should be doable using a similar algorithm. 

Chinese is probably still an issue if your password policy has uppercase and lowercase requirements, but that's probably why there are so many Chinese ASCII passwords as per my previous post, but Latin characters with diacritics should be fine using this kind of complexity.

But... that's just my 2 cents. I'm not the one writing this patch. I also haven't extensively tested it, so maybe I'm missing the current point of the conversaton.
Comment 19 David Cook 2020-09-30 00:20:49 UTC
Comment on attachment 110528 [details] [review]
Bug 12617: Add new regex patterns to password_check.inc

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

::: koha-tmpl/opac-tmpl/bootstrap/en/includes/password_check.inc
@@ +13,5 @@
>  
>      if(category_selector && $('select'+category_selector).length) {
>          jQuery.validator.addMethod("password_strong", function(value, element){
>              var require_strong = $('select'+category_selector+' option:selected').data('pwdStrong');
>              var min_lenght = $('select'+category_selector+' option:selected').data('pwdLength');

Btw this should be min_length, although it does look like you've used the misspelled variable consistently...
Comment 20 David Cook 2020-09-30 00:38:55 UTC
Ok because I'm insufferable...

The following passes:

<html>
    <head>
        <script>
            var min_lenght = 3;
            var regex_text = "(?=.*\\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[!@#$%\^&*(){}[\\]<>?/|\-]).{"+min_lenght+",}";
            var pattern_regex = new RegExp(regex_text);
            var output = pattern_regex.test("Aa1!");
            alert(output);
        </script>
    </head>
</html>

The following fails:

<html>
    <head>
        <script>
            var min_lenght = 3;
            var regex_text = "(?=.*\\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[!@#$%\^&*(){}[\\]<>?/|\-]).{"+min_lenght+",}";
            var pattern_regex = new RegExp(regex_text);
            var output = pattern_regex.test("Ää1!");
            alert(output);
        </script>
    </head>
</html>

That's not good.
Comment 21 David Cook 2020-09-30 00:46:57 UTC
The following is incomplete but could be useful for testing individual characters (you'd just have to iterate through the password string). 

Tangentially, Chinese characters will pass both the "is_upper" and "is_lower" case tests. In terms of pam_cracklib, it seems that Chinese characters would give uppercase credits but not lowercase credits (since a character can only be classed with 1 class). 

Unfortunately, special characters also pass the "is_upper" and "is_lower" tests, so that algorithm isn't really good enough. THat said, C implementations for isupper is super problematic too as they're ASCII based... (https://git.musl-libc.org/cgit/musl/tree/include/ctype.h). 

But I guess that goes back to supporting ASCII-only passwords... 

<html>
    <head>
        <script>
            function is_upper(value){
                var is_upper = false;
                if (value.toUpperCase() === value){
                    is_upper = true;
                }
                return is_upper;
            }
            function is_lower(value){
                var is_lower = false;
                if (value.toLowerCase() === value){
                    is_lower = true;
                }
                return is_lower;
            }
            function is_digit(value){
                return /\d/.test(value);
            }
            alert(is_lower('?'));
            /*
            alert(is_digit('A'));
            alert(is_digit('1'));
            alert(is_lower('我'));
            alert(is_lower('ä'));
            alert(is_lower('Ä'));
            */
        </script>
    </head>
</html>
Comment 22 David Cook 2020-09-30 03:57:38 UTC
Apologies if people find this uninteresting.

Keycloak's LowerCasePasswordPolicyProvider.java file uses Java's Character.isLowerCase() function...

Looking at OpenJDK 8...

http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/dc4322602480/src/share/classes/java/lang/Character.java#l5451

That appears to use Unicode code points (as integers)...

With CharacterData, it does some bit math to use some generated character class lookups it seems... 
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/CharacterData.java

It's a rabbit hole, but Java really does seem to be king when it comes to working with character data...

Going back to Keycloak, SpecialCharsPasswordPolicyProvider.java seems to indicate that anything that doesn't return true for Character.isLetterOrDigit is a "special character". 

According to Java, "Not all letters have case. Many characters are letters but are neither uppercase nor lowercase nor titlecase". Interesting. Apparently that's in regards to the "Lo" "Other Letter" Unicode category (https://www.compart.com/en/unicode/category/Lo) with Arabic being one of the most obvious examples. 

And according to Java, "isLetter" is only uppercase letters, lowercase letters, title case letters, modifier letters, and other letters with "modifier letters" being the "Lm" category (https://www.compart.com/en/unicode/category/Lm). 

Java has an isIdeographic function for checking CJKV characters, but that seems to rely on a generated code point lookup table (although I haven't fully explored that path).
Comment 23 David Cook 2020-09-30 04:30:13 UTC
I mention Keycloak as well, since Keycloak is actually an Angular app which uses Java for its backend. 

When I set a password policy and try to set a password that doesn't match it, I get a 400 error, so it's clearly sending the password to the backend for validation. 

We may want to do the same thing here so that we can centralize the password validation code in Perl (rather than trying to have equivalents in both Perl and Javascript).

Note also that Perl has \p{} and \P{} constructs for matching Unicode properties. For example, \p{Uppercase}, although of course you could just use the [:upper:] POSIX construct instead.

That said, it turns out that Javascript (unsure of versions) does have support for \p and \P in regular expressions as well..., so a person can do the following to check if a character is uppercase:

function is_upper(value){
  return /\p{Uppercase}/u.test(value);
}

Likewise to detect a "letter":
function is_letter(value){
  return /\p{Letter}/u.test(value);
}
This matches alphabetic characters as well as Chinese characters it appears. According to https://www.compart.com/en/unicode/U+6211, 我 is a member of the "Other Letter" category. 

Take a look at https://www.regular-expressions.info/unicode.html for a full list of Unicode categories. 

To get the equivalent of Java Character.isLetterOrDigit, we'd basically just need a regular expression like the following (the difference being that the following includes the Cased_Letter category too which is OK as it's already covered by Lowercase_Letter and Uppercase_Letter):

(\p{Letter}|\p{Decimal_Digit_Number}). 

Going back to Javascript and browser compatibility: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#Browser_compatibility

It looks like Unicode property escapes are supported in Javascript except for... "Internet Explorer" and "Firefox for Android". There is also a note about case folding for Edge, although I think that's for the pre-Chromium Edge. 

Anyway, again, just my 2 cents. It was interesting research/experiments, and hopefully it is useful. If not for this patch then at least other parts of Koha.
Comment 24 David Cook 2020-09-30 04:33:31 UTC
One very last thing... 

Perl also has the Unicode::UCD module, which also allows us to interrogate the properties of Unicode characters without using regular expressions. (This would be more in the Java path I mentioned before.)

You can read more at https://perldoc.pl/perluniprops.

I reckon supporting Unicode for passwords isn't a bad thing. We support UTF-8 encoded characters throughout Koha, so may as well for the passwords, yes? 

It could also help users of Koha that use non-Latin alphabets, which would improve Koha usability globally.

Final 2 cents ;).
Comment 25 David Cook 2020-10-05 23:07:36 UTC
For what it's worth, I've tested Keycloak again and now my Chinese software keyboard is working for inputting passwords.
Comment 26 David Cook 2020-10-05 23:12:48 UTC
Does Koha-Suomi Oy already have these patches running in Koha?

I'm going to mark as Failed QA, as I think the current password policies won't be workable for many Koha implementations.
Comment 27 Emmi Takkinen 2020-10-06 09:17:48 UTC
(In reply to David Cook from comment #26)
> Does Koha-Suomi Oy already have these patches running in Koha?
> 
> I'm going to mark as Failed QA, as I think the current password policies
> won't be workable for many Koha implementations.

Thanks David for looking into this! There's a lot to consider there... 

We've had the first patch running in Koha for a quite while now (not as it's implemented here of course, this is a rebased patch against master). We run version 17.05 and don't have password_check.inc so second patch applies to current community master. 

I'm thinking of scraping those äöåÄÖÅ's from the first patch (and fixing that spelling error in the second) for now. Then I'll add new patch with improved regexs (that is if we need them). If someone has more opinions on this please comment!
Comment 28 Emmi Takkinen 2020-10-06 12:24:53 UTC
Created attachment 111297 [details] [review]
Bug 12617: Koha should let admins to configure automatically generated password complexity/difficulty

Adds simple password policy(with regards to complexity) management into categories:
- Per category password policy: admins can configure what kind of passwords get generated
in member-passwords. User-created passwords are also checked against the policy if it is
defined and complexity is enforced for every user based on their set category.
- Predefined policies:
	- simplenumeric: the digits 0-9 allowed only
	- alphanumeric: passwords must contain only the digits 0-9 and lowercase and uppercase characters.
	Special characters are not allowed.
	- complex: patrons are required to use complex passwords containing numbers, uppercase and lowercase
	characters and special characters.
Old passwords for excisting patrons are not affected.

To test:
1. Apply this patch and update database.
2. Navigate to categories.pl and note there is new column 'Password policies' has been added.
3. Edit some categories and set password policy for them.
4. Set some values to sysprefs 'minPasswordLength', 'minAlnumPasswordLength' and
'minComplexPasswordLength'.

Staff interface:
1. Create new patron.
2. Set their password against their categorys policy and save.
3. Error message is displayed (with content depending on password policy).
4. Set acceptable password and save succesfully.
5. Repeat steps 2-3-4 on patron edit page.
6. Repeat steps 2-3-4 on 'Change password' page.

OPAC:
1. Enable 'OpacPasswordChange' and 'OpacResetPassword'.
2. On OPAC, repeat what you did on staff interface (on create, edit and 'Change your password'.
3. Confirm errors are displayed correctly and saving works.
4. Log out and go to 'Forgotten password recovery' page.
5. Send and receive email for password recovery.
6. Set unacceptable password and save, confirm correct error is displayed.
7. Set acceptable password and save succesfully.

REST API:
1. With your preferred REST client (curl e.g) sent POST request to /api/v1/patrons/{patron_id}/password
with 'password' and 'password_2' parameters.
2. Confirm correct error message is displayed when sending password against password policy.
3. Confirm password is changed when acceptable password is send.

Also prove t/AuthUtils.t and t/db_dependent/api/v1/patrons_password.t

Sponsored-by: Koha-Suomi Oy
Comment 29 Emmi Takkinen 2020-10-06 12:25:27 UTC
Created attachment 111298 [details] [review]
Bug 12617: DO NOT PUSH! Schema change
Comment 30 Emmi Takkinen 2020-10-06 12:26:15 UTC
Created attachment 111299 [details] [review]
Bug 12617: Add new regex patterns to password_check.inc

This patch adds new regex patterns to follow into password_check.inc
files.

To test:
1. Create or find existing patron.
2. Set or edit invalid password for patron in OPAC and staff interfaces:
	a) create and edit patron pages
	b) 'Change password' pages
3. Confirm alert text next to input field is displayed and it follows
set passwordpolicy.

Sponsored-by: Koha-Suomi Oy
Comment 31 Fridolin Somers 2021-08-24 09:20:24 UTC
Note that there is now a plugin hook :
Bug 22706
Comment 32 Emmi Takkinen 2023-09-19 10:44:59 UTC
I'm unable to continue working with this. I'm setting assignee as default, so someone interested/able to continue work here can adopt this.