Simplify PasswordInterface so it's not coupled to UserInterface

patch

errr... crap... we both made patches

1. +++ b/core/lib/Drupal/Core/Password/PasswordInterface.php
   @@ -2,48 +2,47 @@
   +   * Maximum password length.
   +   */
   +  const PASSWORD_MAX_LENGTH = 512;
   +
   +  /**
   

   I didn't include this since it was out of scope from the IS. Are we sure we want to include that in this?

2. +++ b/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php
   @@ -79,14 +88,13 @@ public function testWithinBounds() {
   +    // The weaker hash password should be flagged as needing an update.
   +    $this->assertTrue($this->passwordHasher->needsRehash($this->weakerHashedPassword), 'User with md5 password needs a new hash.');
   
   @@ -116,25 +123,22 @@ public function testPasswordHashing() {
   +    $password_hasher = new PhpassHashedPassword(PhpassHashedPassword::MIN_HASH_COUNT + 2);
   +    $this->assertTrue($password_hasher->needsRehash($this->hashedPassword), 'User needs a new hash after incrementing the log2 count.');
   

   These test the same thing. Also the first test is not testing md5 as implied by the comment. We should remove the first test.

No, and I don't appreciate you repeatedly implying I haven't read your patch. I read this about 5 times before you pinged me in IRC to make sure I understood it and additionally twice after talking on IRC to make sure I wasn't mistaken. It is not testing MD5 it testing a lower hash count which we have an explict test for later in test suite.

Let me see if I can explain this again more clearly.

1. +++ b/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php
   @@ -58,10 +66,12 @@ class PasswordHashingTest extends UnitTestCase {
   +    $this->md5HashedPassword = 'U' . $this->passwordHasher->hash(md5($this->password));
   +    $this->weakerHashedPassword = $weak_hasher->hash($this->password);
   

   Here you set your hashes. The md5 hash was missing previously, that's a good catch.

2. +++ b/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php
   @@ -79,14 +89,13 @@ public function testWithinBounds() {
   +    $this->assertTrue($this->passwordHasher->needsRehash($this->md5HashedPassword), 'User with md5 password needs a new hash.');
   +    // The weaker hash password should be flagged as needing an update.
   +    $this->assertTrue($this->passwordHasher->needsRehash($this->weakerHashedPassword), 'User with md5 password needs a new hash.');
   

   Here you test the MD5 hash, same as before, but a bit shorter then you also check the PhPass hashed password with the lower count but the comment mentions MD5 hash.

3. +++ b/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php
   @@ -116,25 +124,22 @@ public function testPasswordHashing() {
      public function testPasswordRehashing() {
    
        // Increment the log2 iteration to MIN + 1.
   -    $this->passwordHasher = new PhpassHashedPassword(PhpassHashedPassword::MIN_HASH_COUNT + 1);
   -    $this->assertTrue($this->passwordHasher->userNeedsNewHash($this->user), 'User needs a new hash after incrementing the log2 count.');
   +    $password_hasher = new PhpassHashedPassword(PhpassHashedPassword::MIN_HASH_COUNT + 2);
   +    $this->assertTrue($password_hasher->needsRehash($this->hashedPassword), 'User needs a new hash after incrementing the log2 count.');
   

   Here we have an explicit test where we assert that we flag lower hash counts to be rehashed.

+++ b/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php
@@ -161,19 +152,20 @@ public function testLongPassword($password, $allowed) {
+    $len = (PasswordInterface::PASSWORD_MAX_LENGTH - 2) / 3;

nit. (int) (PasswordInterface::PASSWORD_MAX_LENGTH / 3) will achieve the same thing and handle future changes better.

If it may be disruptive it needs a change record.

Patch works for me.

As you already touched the interface docs, why not cleaning up all the docs in that file? I already did that in #1845004-70: Replace custom password hashing library with PHP password_hash().

Ah, sorry, my bad. The non-standard docs were not in interface but in PhpassHashedPassword. We'll clean that after #1845004: Replace custom password hashing library with PHP password_hash() will go in.

The patch looks good to me.

Still need a change record.

Man its hard to find the link to create a change record... I think I've got it done.

Back to RTBC with the change record.

The lack of interoperability is a regression from D7 and disentangling PasswordInterace from UserInterface reduces fragility and makes it more useful. Committed fd2bc62 and pushed to 8.0.x. Thanks!

diff --git a/core/modules/migrate/src/MigratePassword.php b/core/modules/migrate/src/MigratePassword.php
index 6e69887..479aac8 100644
--- a/core/modules/migrate/src/MigratePassword.php
+++ b/core/modules/migrate/src/MigratePassword.php
@@ -8,7 +8,6 @@
 namespace Drupal\migrate;
 
 use Drupal\Core\Password\PasswordInterface;
-use Drupal\migrate\Entity\MigrationInterface;
 
 /**
  * Replaces the original 'password' service in order to prefix the MD5 re-hashed

Fixed unnecessary use added by the patch on commit.

This issue already has a change record.