minor #32352 [Security] Added type-hints to password encoders (derrabus)
This PR was merged into the 5.0-dev branch.
Discussion
----------
[Security] Added type-hints to password encoders
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #32179
| License | MIT
| Doc PR | N/A
This PR adds type declarations to all implementations of `PasswordEncoderInterface` and `UserPasswordEncoderInterface`.
Commits
-------
d763e63210
[Security] Added type-hints to password encoders.
This commit is contained in:
commit
7af0c73918
@ -31,11 +31,9 @@ abstract class BasePasswordEncoder implements PasswordEncoderInterface
|
||||
/**
|
||||
* Demerges a merge password and salt string.
|
||||
*
|
||||
* @param string $mergedPasswordSalt The merged password and salt string
|
||||
*
|
||||
* @return array An array where the first element is the password and the second the salt
|
||||
*/
|
||||
protected function demergePasswordAndSalt($mergedPasswordSalt)
|
||||
protected function demergePasswordAndSalt(string $mergedPasswordSalt)
|
||||
{
|
||||
if (empty($mergedPasswordSalt)) {
|
||||
return ['', ''];
|
||||
@ -56,14 +54,11 @@ abstract class BasePasswordEncoder implements PasswordEncoderInterface
|
||||
/**
|
||||
* Merges a password and a salt.
|
||||
*
|
||||
* @param string $password The password to be used
|
||||
* @param string|null $salt The salt to be used
|
||||
*
|
||||
* @return string a merged password and salt
|
||||
*
|
||||
* @throws \InvalidArgumentException
|
||||
*/
|
||||
protected function mergePasswordAndSalt($password, $salt)
|
||||
protected function mergePasswordAndSalt(string $password, ?string $salt)
|
||||
{
|
||||
if (empty($salt)) {
|
||||
return $password;
|
||||
@ -82,12 +77,9 @@ abstract class BasePasswordEncoder implements PasswordEncoderInterface
|
||||
* This method implements a constant-time algorithm to compare passwords to
|
||||
* avoid (remote) timing attacks.
|
||||
*
|
||||
* @param string $password1 The first password
|
||||
* @param string $password2 The second password
|
||||
*
|
||||
* @return bool true if the two passwords are the same, false otherwise
|
||||
*/
|
||||
protected function comparePasswords($password1, $password2)
|
||||
protected function comparePasswords(string $password1, string $password2)
|
||||
{
|
||||
return hash_equals($password1, $password2);
|
||||
}
|
||||
@ -95,11 +87,9 @@ abstract class BasePasswordEncoder implements PasswordEncoderInterface
|
||||
/**
|
||||
* Checks if the password is too long.
|
||||
*
|
||||
* @param string $password The password to check
|
||||
*
|
||||
* @return bool true if the password is too long, false otherwise
|
||||
*/
|
||||
protected function isPasswordTooLong($password)
|
||||
protected function isPasswordTooLong(string $password)
|
||||
{
|
||||
return \strlen($password) > static::MAX_PASSWORD_LENGTH;
|
||||
}
|
||||
|
@ -39,7 +39,7 @@ class MessageDigestPasswordEncoder extends BasePasswordEncoder
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function encodePassword($raw, $salt)
|
||||
public function encodePassword(string $raw, ?string $salt)
|
||||
{
|
||||
if ($this->isPasswordTooLong($raw)) {
|
||||
throw new BadCredentialsException('Invalid password.');
|
||||
@ -63,7 +63,7 @@ class MessageDigestPasswordEncoder extends BasePasswordEncoder
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function isPasswordValid($encoded, $raw, $salt)
|
||||
public function isPasswordValid(string $encoded, string $raw, ?string $salt)
|
||||
{
|
||||
return !$this->isPasswordTooLong($raw) && $this->comparePasswords($encoded, $this->encodePassword($raw, $salt));
|
||||
}
|
||||
|
@ -34,7 +34,7 @@ final class MigratingPasswordEncoder extends BasePasswordEncoder implements Self
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function encodePassword($raw, $salt): string
|
||||
public function encodePassword(string $raw, ?string $salt): string
|
||||
{
|
||||
return $this->bestEncoder->encodePassword($raw, $salt);
|
||||
}
|
||||
@ -42,7 +42,7 @@ final class MigratingPasswordEncoder extends BasePasswordEncoder implements Self
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function isPasswordValid($encoded, $raw, $salt): bool
|
||||
public function isPasswordValid(string $encoded, string $raw, ?string $salt): bool
|
||||
{
|
||||
if ($this->bestEncoder->isPasswordValid($encoded, $raw, $salt)) {
|
||||
return true;
|
||||
|
@ -57,7 +57,7 @@ final class NativePasswordEncoder implements PasswordEncoderInterface, SelfSalti
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function encodePassword($raw, $salt)
|
||||
public function encodePassword(string $raw, ?string $salt)
|
||||
{
|
||||
if (\strlen($raw) > self::MAX_PASSWORD_LENGTH) {
|
||||
throw new BadCredentialsException('Invalid password.');
|
||||
@ -78,7 +78,7 @@ final class NativePasswordEncoder implements PasswordEncoderInterface, SelfSalti
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function isPasswordValid($encoded, $raw, $salt): bool
|
||||
public function isPasswordValid(string $encoded, string $raw, ?string $salt): bool
|
||||
{
|
||||
if (72 < \strlen($raw) && 0 === strpos($encoded, '$2')) {
|
||||
// BCrypt encodes only the first 72 chars
|
||||
|
@ -23,15 +23,12 @@ interface PasswordEncoderInterface
|
||||
/**
|
||||
* Encodes the raw password.
|
||||
*
|
||||
* @param string $raw The password to encode
|
||||
* @param string|null $salt The salt
|
||||
*
|
||||
* @return string The encoded password
|
||||
*
|
||||
* @throws BadCredentialsException If the raw password is invalid, e.g. excessively long
|
||||
* @throws \InvalidArgumentException If the salt is invalid
|
||||
*/
|
||||
public function encodePassword($raw, $salt);
|
||||
public function encodePassword(string $raw, ?string $salt);
|
||||
|
||||
/**
|
||||
* Checks a raw password against an encoded password.
|
||||
@ -44,7 +41,7 @@ interface PasswordEncoderInterface
|
||||
*
|
||||
* @throws \InvalidArgumentException If the salt is invalid
|
||||
*/
|
||||
public function isPasswordValid($encoded, $raw, $salt);
|
||||
public function isPasswordValid(string $encoded, string $raw, ?string $salt);
|
||||
|
||||
/**
|
||||
* Checks if an encoded password would benefit from rehashing.
|
||||
|
@ -52,7 +52,7 @@ class Pbkdf2PasswordEncoder extends BasePasswordEncoder
|
||||
*
|
||||
* @throws \LogicException when the algorithm is not supported
|
||||
*/
|
||||
public function encodePassword($raw, $salt)
|
||||
public function encodePassword(string $raw, ?string $salt)
|
||||
{
|
||||
if ($this->isPasswordTooLong($raw)) {
|
||||
throw new BadCredentialsException('Invalid password.');
|
||||
@ -70,7 +70,7 @@ class Pbkdf2PasswordEncoder extends BasePasswordEncoder
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function isPasswordValid($encoded, $raw, $salt)
|
||||
public function isPasswordValid(string $encoded, string $raw, ?string $salt)
|
||||
{
|
||||
return !$this->isPasswordTooLong($raw) && $this->comparePasswords($encoded, $this->encodePassword($raw, $salt));
|
||||
}
|
||||
|
@ -33,7 +33,7 @@ class PlaintextPasswordEncoder extends BasePasswordEncoder
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function encodePassword($raw, $salt)
|
||||
public function encodePassword(string $raw, ?string $salt)
|
||||
{
|
||||
if ($this->isPasswordTooLong($raw)) {
|
||||
throw new BadCredentialsException('Invalid password.');
|
||||
@ -45,7 +45,7 @@ class PlaintextPasswordEncoder extends BasePasswordEncoder
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function isPasswordValid($encoded, $raw, $salt)
|
||||
public function isPasswordValid(string $encoded, string $raw, ?string $salt)
|
||||
{
|
||||
if ($this->isPasswordTooLong($raw)) {
|
||||
return false;
|
||||
|
@ -54,7 +54,7 @@ final class SodiumPasswordEncoder implements PasswordEncoderInterface, SelfSalti
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function encodePassword($raw, $salt): string
|
||||
public function encodePassword(string $raw, ?string $salt): string
|
||||
{
|
||||
if (\strlen($raw) > self::MAX_PASSWORD_LENGTH) {
|
||||
throw new BadCredentialsException('Invalid password.');
|
||||
@ -74,7 +74,7 @@ final class SodiumPasswordEncoder implements PasswordEncoderInterface, SelfSalti
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function isPasswordValid($encoded, $raw, $salt): bool
|
||||
public function isPasswordValid(string $encoded, string $raw, ?string $salt): bool
|
||||
{
|
||||
if (\strlen($raw) > self::MAX_PASSWORD_LENGTH) {
|
||||
return false;
|
||||
|
@ -30,7 +30,7 @@ class UserPasswordEncoder implements UserPasswordEncoderInterface
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function encodePassword(UserInterface $user, $plainPassword)
|
||||
public function encodePassword(UserInterface $user, string $plainPassword)
|
||||
{
|
||||
$encoder = $this->encoderFactory->getEncoder($user);
|
||||
|
||||
@ -40,7 +40,7 @@ class UserPasswordEncoder implements UserPasswordEncoderInterface
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function isPasswordValid(UserInterface $user, $raw)
|
||||
public function isPasswordValid(UserInterface $user, string $raw)
|
||||
{
|
||||
$encoder = $this->encoderFactory->getEncoder($user);
|
||||
|
||||
|
@ -23,20 +23,14 @@ interface UserPasswordEncoderInterface
|
||||
/**
|
||||
* Encodes the plain password.
|
||||
*
|
||||
* @param UserInterface $user The user
|
||||
* @param string $plainPassword The password to encode
|
||||
*
|
||||
* @return string The encoded password
|
||||
*/
|
||||
public function encodePassword(UserInterface $user, $plainPassword);
|
||||
public function encodePassword(UserInterface $user, string $plainPassword);
|
||||
|
||||
/**
|
||||
* @param UserInterface $user The user
|
||||
* @param string $raw A raw password
|
||||
*
|
||||
* @return bool true if the password is valid, false otherwise
|
||||
*/
|
||||
public function isPasswordValid(UserInterface $user, $raw);
|
||||
public function isPasswordValid(UserInterface $user, string $raw);
|
||||
|
||||
/**
|
||||
* Checks if an encoded password would benefit from rehashing.
|
||||
|
@ -15,6 +15,7 @@ use PHPUnit\Framework\TestCase;
|
||||
use Symfony\Component\Security\Core\Authentication\Provider\DaoAuthenticationProvider;
|
||||
use Symfony\Component\Security\Core\Encoder\PlaintextPasswordEncoder;
|
||||
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
|
||||
use Symfony\Component\Security\Core\User\UserInterface;
|
||||
|
||||
class DaoAuthenticationProviderTest extends TestCase
|
||||
{
|
||||
@ -73,7 +74,7 @@ class DaoAuthenticationProviderTest extends TestCase
|
||||
->method('loadUserByUsername')
|
||||
;
|
||||
|
||||
$user = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserInterface')->getMock();
|
||||
$user = new TestUser();
|
||||
$token = $this->getSupportedToken();
|
||||
$token->expects($this->once())
|
||||
->method('getUser')
|
||||
@ -90,7 +91,7 @@ class DaoAuthenticationProviderTest extends TestCase
|
||||
|
||||
public function testRetrieveUser()
|
||||
{
|
||||
$user = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserInterface')->getMock();
|
||||
$user = new TestUser();
|
||||
|
||||
$userProvider = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserProviderInterface')->getMock();
|
||||
$userProvider->expects($this->once())
|
||||
@ -127,11 +128,7 @@ class DaoAuthenticationProviderTest extends TestCase
|
||||
->willReturn('')
|
||||
;
|
||||
|
||||
$method->invoke(
|
||||
$provider,
|
||||
$this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserInterface')->getMock(),
|
||||
$token
|
||||
);
|
||||
$method->invoke($provider, new TestUser(), $token);
|
||||
}
|
||||
|
||||
public function testCheckAuthenticationWhenCredentialsAre0()
|
||||
@ -154,11 +151,7 @@ class DaoAuthenticationProviderTest extends TestCase
|
||||
->willReturn('0')
|
||||
;
|
||||
|
||||
$method->invoke(
|
||||
$provider,
|
||||
$this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserInterface')->getMock(),
|
||||
$token
|
||||
);
|
||||
$method->invoke($provider, new TestUser(), $token);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -182,7 +175,7 @@ class DaoAuthenticationProviderTest extends TestCase
|
||||
->willReturn('foo')
|
||||
;
|
||||
|
||||
$method->invoke($provider, $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserInterface')->getMock(), $token);
|
||||
$method->invoke($provider, new TestUser(), $token);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -256,7 +249,7 @@ class DaoAuthenticationProviderTest extends TestCase
|
||||
->willReturn('foo')
|
||||
;
|
||||
|
||||
$method->invoke($provider, $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserInterface')->getMock(), $token);
|
||||
$method->invoke($provider, new TestUser(), $token);
|
||||
}
|
||||
|
||||
protected function getSupportedToken()
|
||||
@ -299,3 +292,30 @@ class DaoAuthenticationProviderTest extends TestCase
|
||||
return new DaoAuthenticationProvider($userProvider, $userChecker, 'key', $encoderFactory);
|
||||
}
|
||||
}
|
||||
|
||||
class TestUser implements UserInterface
|
||||
{
|
||||
public function getRoles()
|
||||
{
|
||||
return [];
|
||||
}
|
||||
|
||||
public function getPassword()
|
||||
{
|
||||
return 'secret';
|
||||
}
|
||||
|
||||
public function getSalt()
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
public function getUsername()
|
||||
{
|
||||
return 'jane_doe';
|
||||
}
|
||||
|
||||
public function eraseCredentials()
|
||||
{
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user