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.

Command icon 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

smori1983 created an issue. See original summary.

bharath-kondeti’s picture

Status: Active » Needs review
bruno_lima’s picture

Status: Needs review » Reviewed & tested by the community

+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.

jidrone’s picture

I tested the MR and it is working.

vishalkhode’s picture

Status: Reviewed & tested by the community » Needs work

Reviewed changes, looks good. However, we should also add a functional test case for the same.

ankitv18’s picture

Version: 4.0.1 » 4.0.x-dev

Rajeshreeputra made their first commit to this issue’s fork.

vishalkhode’s picture

Status: Needs work » Fixed

Thanks 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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.