Implement the "character types" password constraint.

1. +++ b/password_policy_character_types/src/Plugin/PasswordConstraint/CharacterTypes.php
   @@ -0,0 +1,94 @@
   +      preg_match('/[a-z]/', $password),
   +      preg_match('/[A-Z]/', $password),
   +      preg_match('/[0-9]/', $password),
   

   The $password needs wrapping in preg_quote otherwise special chars in the password will break this? We should also add a test for this.

2. +++ b/password_policy_character_types/src/Plugin/PasswordConstraint/CharacterTypes.php
   @@ -0,0 +1,94 @@
   +      strpbrk($password, '\'#$%!^&*()+=-[]\;,./{}|\":<>?~')
   

   So, they only need to have 1 of these characters? Having two of them doesn't help?

3. +++ b/password_policy_character_types/src/Plugin/PasswordConstraint/CharacterTypes.php
   @@ -0,0 +1,94 @@
   +      $validation->setErrorMessage($this->t('The password is required to contain at least @types types of characters from the character sets: lowercase alphabetic characters (a–z), uppercase alphabetic characters (A–Z), numeric characters (0–9), special characters.'));
   

   @types doesn't have a replacement.

4. How about an integration test to validate the schema? Didn't check if the module already has some.

ISM?

http://www.asd.gov.au/infosec/ism/

Thanks, benjy.

Re patch in #5: It'd probably be clearer to use named character classes (e.g., [:punct:]):

https://secure.php.net/manual/en/regexp.reference.character-classes.php

When I read strpbrk($password, '\'#$%!^&*()+=-[]\;,./{}|":<>?~'), I wonder if that list is the complete list of special characters.

+++ b/password_policy_character_types/src/Plugin/PasswordConstraint/CharacterTypes.php
@@ -0,0 +1,94 @@
+      strpbrk($password, '\'#$%!^&*()+=-[]\;,./{}|":<>?~')

So, they only need to have 1 of these characters? Having two of them doesn't help?

That's how the constraint works in 7.x-1.x. The idea is to ensure the password is drawn from a large set of possible characters. If a password has only special characters, it might be easier to crack than a password that has numbers, letters, and special characters.

The constraint checks the number of character types, not the number of characters of each type.

Is there a reason automated testing is disabled? Not sure what the status of HEAD is.

I think the 8.x-3.x tests aren't working currently:

#2635986: Make tests pass

Changes:
- Tweaked .info.yml description to avoid confusion with "Character Type" constraint.
- Corrected @file comment.
- Marked invalid class comment with "FIXME".
- Added comma after last multi-line array item per coding standards.
- Changed a validation message that seemed incorrect.
- Changed function short descriptions to third-person singular per documentation standards.
- Changed wording of test class comment.

Comments:
- The "Character Type" and "Character Types" constraint names are confusingly similar. The names seem to imply a singular/plural relationship, but they do not have that relationship. I think we ought to make the names more distinct. Any ideas? Maybe "Characters of Type" instead of "Character Type"? To me it seems like "Character Type" isn't really a constraint on the character type; it's a constraint on the number of characters of a type. "Character Types" though is a constraint on the number of character types.
- "Character types" and "character sets" are used interchangeably. It would seem to me better to pick one, unless there is some advantage to using different terms in different contexts.
- Would it be better to validate 2 to 4 rather than 1 to 4? Seems pointless to set the value to 1.

Ah okay. Will this block new contributions such as this?

If I say yes, will you fix the tests? :)

nerdstein has been making the 8.x-3.x commits, so I defer to him.

password_policy and password_policy_length have test code in src/Tests, but password_policy_character_types has it in tests/src. Is the latter more proper? Or should we change the location to src/Tests for consistency?

- Removed extra blank line per Coder.
- Corrected class comment.
- Added visibility declaration to validate(). (Absent from existing constraints, but against coding standards.)
- Changed minimum character types to 2.
- Re-ordered conditional for consistency with ordering of same conditions later in code.
- Changed "character sets" to "character types".
- "Select the number of character ..." -> "Select the minimum number of character ..."
- Added word "four" so administrator maybe does not need to count character types.
- Added file comment per Coder.
- Added blank line between last method closing brace and class closing brace per Coder.

I believe "tests/src" is required for unit tests and "src/Tests" is for web tests. These locations can't be changed.

Ah, OK. That makes sense.

I think the unit test is great--short and sweet--but the existing constraints all have web tests. Perhaps we should add those for consistency. There is no testing of setting the configuration for this constraint, though the existing constraints all have "Operations" web tests that test constraint configuration.

- Fixed invalid preg_match() pattern. (The unit test would have caught.)
- Removed test input that causes failure with change to minimum constraint value of 2.

Why is it necessary to stub t()? (Why it is necessary to specify 't' in the array passed to setMethods()?)

I can see that the tests don't run properly if it is not used, but am having trouble understanding why.

Sorry for being behind on this. I'm getting caught up on this thread and will post updates as soon as possible.

The info file was updated in a previous issue, as such the patch needed rebuilt.

And, some feedback...

error_message = @Translation("Your password needs to be more complex.")

This is quite ambiguous. It should be more relevant. My new patch updates this.

'#title' => $this->t('Number of character types'),

This should note minimum, not just number. My patch updates this.

'#type' => 'number',

This accepts any number when we only have a fixed number of character types. This needs to be a select list from 1-4. Patch will have this.

I have tested all of this and it should be good.

Merged to 8.x-3.x

I turned on testing, by the way

My original patch was bad (please disregard), but I got all of the code up to 8.x-3.x and credited to Sam152.

One minor issue I found was the config schema was named "password_length" and not "character_types". I renamed the file in accordance with the appropriate submodule.

The tests don't seem to be passing for 8.x-3.x

Yes, that's a known issue. Care to take a stab?

No afraid not, I've got my hands full with migrate issues.

OK, we're tracking it here:

https://www.drupal.org/node/2635986

Feel free to contribute if you wish to help this along.

Hello Nerdstein,

Hope you are doing good.
I am using LDAP module in Drupal8. I have the LDAP module setup so that when ever the password is changed in drupal, it should reflect in LDAP too. The password update is breaking in LDAP when ever I use these characters in these password --> & " ' < > . Please let me know is there anything that I can do to fix the issue.

--
Thank you
Prashanth