Skip to content
Permalink
Browse files

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
-------

d763e63 [Security] Added type-hints to password encoders.
  • Loading branch information...
fabpot committed Jul 4, 2019
2 parents b163a95 + d763e63 commit 7af0c73918b0e4ebd9157794e95ce5b1c847da58
@@ -31,11 +31,9 @@ public function needsRehash(string $encoded): bool
/**
* 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 @@ protected function demergePasswordAndSalt($mergedPasswordSalt)
/**
* 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,24 +77,19 @@ protected function mergePasswordAndSalt($password, $salt)
* 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);
}
/**
* 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 @@ public function __construct(string $algorithm = 'sha512', bool $encodeHashAsBase
/**
* {@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 @@ public function encodePassword($raw, $salt)
/**
* {@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,15 +34,15 @@ public function __construct(PasswordEncoderInterface $bestEncoder, PasswordEncod
/**
* {@inheritdoc}
*/
public function encodePassword($raw, $salt): string
public function encodePassword(string $raw, ?string $salt): string
{
return $this->bestEncoder->encodePassword($raw, $salt);
}
/**
* {@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 @@ public function __construct(int $opsLimit = null, int $memLimit = null, int $cos
/**
* {@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 @@ public function encodePassword($raw, $salt)
/**
* {@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 @@ public function encodePassword($raw, $salt);
*
* @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 @@ public function __construct(string $algorithm = 'sha512', bool $encodeHashAsBase
*
* @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 @@ public function encodePassword($raw, $salt)
/**
* {@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 @@ public function __construct(bool $ignorePasswordCase = false)
/**
* {@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 @@ public function encodePassword($raw, $salt)
/**
* {@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 @@ public static function isSupported(): bool
/**
* {@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 @@ public function encodePassword($raw, $salt): string
/**
* {@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 @@ public function __construct(EncoderFactoryInterface $encoderFactory)
/**
* {@inheritdoc}
*/
public function encodePassword(UserInterface $user, $plainPassword)
public function encodePassword(UserInterface $user, string $plainPassword)
{
$encoder = $this->encoderFactory->getEncoder($user);
@@ -40,7 +40,7 @@ public function encodePassword(UserInterface $user, $plainPassword)
/**
* {@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 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 @@ public function testRetrieveUserReturnsUserFromTokenOnReauthentication()
->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 @@ public function testRetrieveUserReturnsUserFromTokenOnReauthentication()
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 @@ public function testCheckAuthenticationWhenCredentialsAreEmpty()
->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 @@ public function testCheckAuthenticationWhenCredentialsAre0()
->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 @@ public function testCheckAuthenticationWhenCredentialsAreNotValid()
->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 @@ public function testCheckAuthentication()
->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 @@ protected function getProvider($user = null, $userChecker = null, $passwordEncod
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()
{
}
}

0 comments on commit 7af0c73

Please sign in to comment.
You can’t perform that action at this time.