Problem/Motivation

PasswordPolicyValidatorInterface::validatePassword() is documented to return PasswordPolicyValidationReport, while PasswordPolicyValidator::validatePassword() returns TRUE when the password is too long.

  /**
   * @return \Drupal\password_policy\PasswordPolicyValidationReport
   *   Validation report object.
   */
  public function validatePassword(string $password, UserInterface $user, array $edited_user_roles = []): PasswordPolicyValidationReport;

  /**
   * {@inheritdoc}
   */
  public function validatePassword(string $password, UserInterface $user, array $edited_user_roles = []): PasswordPolicyValidationReport {
    // Stop before policy-based validation if password exceeds maximum length.
    if (strlen($password) > PasswordInterface::PASSWORD_MAX_LENGTH) {
      return TRUE;
    }

Proposed resolution

Change the @return line in the interface.

Issue fork password_policy-3314908

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:

About issue forks

Comments

ivanhelguera created an issue. See original summary.

ivanhelguera opened merge request !43

ivanhelguera’s picture

ivanhelguera commented
Issue summary: View changes
ivanhelguera’s picture

ivanhelguera commented
Issue summary: View changes
ivanhelguera’s picture

ivanhelguera commented
Issue summary: View changes
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: PasswordPolicyValidator::validatePassword has a wrong @return declarations » PasswordPolicyValidator::validatePassword() doesn't return the declared value
hmendes’s picture

hmendes commented
Assigned: ivanhelguera » Unassigned
Priority: Normal » Minor
Status: Active » Reviewed & tested by the community

Thanks for the MR. Changing this to RTBC.

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

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented

+1 RTBC, rebased

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes

Rajeshreeputra opened merge request !57

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Reviewed & tested by the community » Needs work 
+   * @return \Drupal\password_policy\PasswordPolicyValidationReport|bool
+   *   Validation report object or TRUE if password exceeds maximum length.

The description is missing a indefinite article before Validation report and a definite article before password exceeds.

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

urvashi_vora’s picture

urvashi_vora commented
Status: Needs work » Needs review

Hi @apaderno,

I have added the indefinite and definite article and committed the changes, also I have rebased the branch.

Please review the changes.

Thanks

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

The lat MR is correct.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Assigned: Unassigned » kristen pol

Assigning to myself as I'm reviewing/merging ready RTBC fixes/updates over the next few days.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

Thanks everyone for working on this issue.

I'm surprised that the code returns TRUE in the first place as I would expect it to return FALSE if it doesn't validate properly. But, this issue isn't trying to address that... it's just updating the documentation.

The change seems okay to merge, so I will.

  • Kristen Pol committed 29e5c4fb on 4.0.x 
    Issue #3314908 by ivanhelguera, urvashi_vora, apaderno, Kristen Pol,...
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Assigned: kristen pol » Unassigned
Status: Reviewed & tested by the community » Fixed

The fix has been merged and will be part of the next release.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Version: 8.x-3.x-dev » 4.0.0

This was against 4.0.x as 8.x branches are no longer supported, so changing version.

Status: Fixed » Closed (fixed)

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

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Version: 4.0.0 » 4.0.x-dev

This is part of the new 4.0.1 release.