Problem/Motivation
#3308607: Password Character Length Policy allows duplicate and invalid intended to prevent registration of contradictory combination of minimum and/or maximum length constraints.
There are 2 problems added.
Problem 1:
The code checks $form_state->getBuildInfo()['args'][1].
As far as I'm understanding, it intends to check the constraint is new one or not.
If the constriant is new one, return value will be:
array (
0 => 'password_length',
1 => '{policy machine name}',
2 => NULL,
)
If the constraint is existing one, return value will be:
array (
0 => '{index of the constraint in the policy}',
1 => 'password_length',
2 => '{policy machine name}',
)
By checking the value of index 1 is NOT password_length, this code focuses on the situation when the new length constraint was added.
But if user registered the policy which machine name is password_length, this code fundamentally cannot identify the constraint is new or not.
In such situation, user will be able to add multiple minimum or maximum constraints, because added validation will not be invoked.
Problem 2:
The code iterates constraints registered to the policy and checks constraint configs of character_operation and character_length.
\Drupal\password_policy_length\Plugin\PasswordConstraint\PasswordLength::validateConfigurationForm():
$constraints = $entity->get('policy_constraints');
foreach ($constraints as $constraint) {
$constraint_operation = $constraint["character_operation"];
$constraint_length = $constraint["character_length"];
// performe 3 types of validation here.
}
These 2 constraint configs are password length specific.
This logic does not consider the situation that there are other constraint types already added to the policy.
In such situation, user will get error message like below, and fails to add the constraint.
The selected length (1) is higher than the maximum length defined ().
Target of this issue
To resolve Problem 1, I think we need to modify code of \Drupal\password_policy\Form\ConstraintEdit too.
I would like to treat Problem 2 only in this issue.
Steps to reproduce
- Create policy with arbitrary machine name.
- Register any constraint other than password length.
- Register the constraint of password length with arbitrary 'Number of characters' and 'Operation' values.
Proposed resolution
Check id of the constraint is password_length inside foreach.
Ignore other constraints.
Remaining tasks
User interface changes
None.
API changes
None.
Data model changes
None.
Issue fork password_policy-3452575
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
bharath-kondeti commentedComment #4
bruno_lima commented+1 to MR !82
Fixed the problem I was having where I couldn't save the 'Password character length' policy.
Quite simple fix, so i'm changing to RTBC.
Comment #5
jidrone commentedI tested the MR and it is working.
Comment #6
vishalkhode commentedReviewed changes, looks good. However, we should also add a functional test case for the same.
Comment #7
ankitv18 commentedComment #10
vishalkhode commentedThanks everyone for working on this. Added test coverage for this bug and for the validations that were added as part of #3308607: Password Character Length Policy allows duplicate and invalid .
@smori1983 If would be great, if you can create a separate issue for problem #2 you mentioned above with detailed steps to reproduce, so we can take a look and fix that up. Thanks.