Skip to content

Commit

Permalink
bug #32831 [Security] Revise UserPasswordEncoderInterface::needsRehas…
Browse files Browse the repository at this point in the history
…h() (ro0NL)

This PR was squashed before being merged into the 4.4 branch (closes #32831).

Discussion
----------

[Security] Revise UserPasswordEncoderInterface::needsRehash()

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #...   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

This reuses the encoded password from the user for the `UserPasswordEncoderInterface`, similar we dont pass the encoded string to `isPasswordValid()`.

This differs from the non-user aware `PasswordEncoderInterface`

cc @nicolas-grekas did i miss something?

Commits
-------

c5a283d [Security] Revise UserPasswordEncoderInterface::needsRehash()
  • Loading branch information
nicolas-grekas committed Jul 31, 2019
2 parents 3b84a1a + c5a283d commit f4ceb91
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ public function isPasswordValid(UserInterface $user, $raw)
/**
* {@inheritdoc}
*/
public function needsRehash(UserInterface $user, string $encoded): bool
public function needsRehash(UserInterface $user): bool
{
$encoder = $this->encoderFactory->getEncoder($user);

return method_exists($encoder, 'needsRehash') && $encoder->needsRehash($encoded);
return method_exists($encoder, 'needsRehash') && $encoder->needsRehash($user->getPassword());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*
* @author Ariel Ferrandini <arielferrandini@gmail.com>
*
* @method bool needsRehash(UserInterface $user, string $encoded)
* @method bool needsRehash(UserInterface $user)
*/
interface UserPasswordEncoderInterface
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ public function testNeedsRehash()

$passwordEncoder = new UserPasswordEncoder($mockEncoderFactory);

$hash = $passwordEncoder->encodePassword($user, 'foo', 'salt');
$this->assertFalse($passwordEncoder->needsRehash($user, $hash));
$this->assertTrue($passwordEncoder->needsRehash($user, $hash));
$this->assertFalse($passwordEncoder->needsRehash($user, $hash));
$user->setPassword($passwordEncoder->encodePassword($user, 'foo', 'salt'));
$this->assertFalse($passwordEncoder->needsRehash($user));
$this->assertTrue($passwordEncoder->needsRehash($user));
$this->assertFalse($passwordEncoder->needsRehash($user));
}
}
5 changes: 5 additions & 0 deletions src/Symfony/Component/Security/Core/User/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,9 @@ public function isEqualTo(UserInterface $user)

return true;
}

public function setPassword(string $password)
{
$this->password = $password;
}
}

0 comments on commit f4ceb91

Please sign in to comment.