Problem/Motivation

This is a follow up issue to #1463624-45: Move password.inc into DIC. As suggested by Rob Loach, a default parameter should be added to the constructor of PhpassHashedPassword. By also using a constant instead of a naked value, we also regain the advantage that we can document it appropriately in a doxygen comment (instead of an unprocessed plain comment in core.services.yml as it is now).

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#25 1850638-25.patch2.24 KBmfb
#16 1850638-16-D8.patch2.51 KBmohit1604
#6 1850638-log2-count-as-default-argument-6.patch2.5 KBJalandhar
#4 1850638-log2-count-as-default-argument-3.patch2.47 KBznerol
log2-count-as-default-argument.patch2.35 KBznerol

Comments

Anonymous’s picture

Anonymous (not verified) commented

log2-count-as-default-argument.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, log2-count-as-default-argument.patch, failed testing.

Anonymous’s picture

Anonymous (not verified) commented
Assigned: Unassigned »
Issue summary: View changes
znerol’s picture

znerol commented
Assigned: » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 1850638-log2-count-as-default-argument-3.patch2.47 KB

Reroll.

Jalandhar’s picture

Jalandhar commented
Status: Needs review » Needs work
Issue tags: +Needs reroll
Jalandhar’s picture

Jalandhar commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 1850638-log2-count-as-default-argument-6.patch2.5 KB

Please review the updated patch which is working fine.

znerol’s picture

znerol commented
Category: Feature request » Task
Issue summary: View changes
Issue tags: +Quick fix

znerol queued 6: 1850638-log2-count-as-default-argument-6.patch for re-testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

This patch doesn't apply anymore, and thus needs a reroll.

When rerolling this issue, please don't rename the variable, only the constant and comment move should be done.

Anonymous’s picture

Anonymous (not verified) commented
Related issues: +#2165549: Reduce number of password hashing iterations in all tests to improve test performance, +#1845004: Replace custom password hashing library with PHP password_hash()

Perhaps it is worth closing this issue in favor of #2165549: Reduce number of password hashing iterations in all tests to improve test performance. And to discuss what place is better (constructor vs settings).

If we leave it open, then tests are needed. Example, why HASH_COUNT = 16? And how to ensure that this value is not obsolete in future?

From @sun's post it should be:

Increase the iterations by 1 every 2 years.

$default = (int) (15 + (date('Y') - 2011) / 2);

Also have sense epic works in #1845004: Replace custom password hashing library with PHP password_hash() and https://wiki.php.net/rfc/argon2_password_hash (for happy sites with php 7.2)

mohit1604’s picture

mohit1604 commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 1850638-16-D8.patch2.51 KB

Rerolled the patch, please review it:)

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Needs work

@mohit1604, the patch in #16 looks good. I don't think we should rename the contructor parameter in this issue though, that seems like scope creep

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mfb’s picture

mfb
they or he
commented
Version: 9.5.x-dev » 10.1.x-dev
Status: Needs work » Needs review
StatusFileSize
new 1850638-25.patch2.24 KB

I'd say this should be closed in favor of #1845004: Replace custom password hashing library with PHP password_hash(). But in any case, rerolled! and reverted scope creep

smustgrave’s picture

smustgrave commented
Status: Needs review » Closed (duplicate)

Brought this update in #contribute and @andypost also agrees this is probably a duplicate

Closing in favor of #1845004: Replace custom password hashing library with PHP password_hash()