diff --git a/UPGRADE-5.3.md b/UPGRADE-5.3.md index 4c5576adb9..8da8a34e0d 100644 --- a/UPGRADE-5.3.md +++ b/UPGRADE-5.3.md @@ -80,6 +80,88 @@ Routing Security -------- + * Deprecate `UserInterface::getPassword()` + If your `getPassword()` method does not return `null` (i.e. you are using password-based authentication), + you should implement `PasswordAuthenticatedUserInterface`. + + Before: + ```php + use Symfony\Component\Security\Core\User\UserInterface; + + class User implements UserInterface + { + // ... + + public function getPassword() + { + return $this->password; + } + } + ``` + + After: + ```php + use Symfony\Component\Security\Core\User\UserInterface; + use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; + + class User implements UserInterface, PasswordAuthenticatedUserInterface + { + // ... + + public function getPassword(): ?string + { + return $this->password; + } + } + ``` + + * Deprecate `UserInterface::getSalt()` + If your `getSalt()` method does not return `null` (i.e. you are using password-based authentication with an old password hash algorithm that requires user-provided salts), + implement `LegacyPasswordAuthenticatedUserInterface`. + + Before: + ```php + use Symfony\Component\Security\Core\User\UserInterface; + + class User implements UserInterface + { + // ... + + public function getPassword() + { + return $this->password; + } + + public function getSalt() + { + return $this->salt; + } + } + ``` + + After: + ```php + use Symfony\Component\Security\Core\User\UserInterface; + use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface; + + class User implements UserInterface, LegacyPasswordAuthenticatedUserInterface + { + // ... + + public function getPassword(): ?string + { + return $this->password; + } + + public function getSalt(): ?string + { + return $this->salt; + } + } + ``` + + * Deprecate calling `PasswordUpgraderInterface::upgradePassword()` with a `UserInterface` instance that does not implement `PasswordAuthenticatedUserInterface` + * Deprecate calling methods `hashPassword()`, `isPasswordValid()` and `needsRehash()` on `UserPasswordHasherInterface` with a `UserInterface` instance that does not implement `PasswordAuthenticatedUserInterface` * Deprecate all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead * Deprecated voters that do not return a valid decision when calling the `vote` method diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index a254493d4e..5dcfabf494 100644 --- a/UPGRADE-6.0.md +++ b/UPGRADE-6.0.md @@ -172,6 +172,90 @@ Routing Security -------- + * Remove `UserInterface::getPassword()` + If your `getPassword()` method does not return `null` (i.e. you are using password-based authentication), + you should implement `PasswordAuthenticatedUserInterface`. + + Before: + ```php + use Symfony\Component\Security\Core\User\UserInterface; + + class User implements UserInterface + { + // ... + + public function getPassword() + { + return $this->password; + } + } + ``` + + After: + ```php + use Symfony\Component\Security\Core\User\UserInterface; + use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; + + class User implements UserInterface, PasswordAuthenticatedUserInterface + { + // ... + + public function getPassword(): ?string + { + return $this->password; + } + } + ``` + + * Remove `UserInterface::getSalt()` + If your `getSalt()` method does not return `null` (i.e. you are using password-based authentication with an old password hash algorithm that requires user-provided salts), + implement `LegacyPasswordAuthenticatedUserInterface`. + + Before: + ```php + use Symfony\Component\Security\Core\User\UserInterface; + + class User implements UserInterface + { + // ... + + public function getPassword() + { + return $this->password; + } + + public function getSalt() + { + return $this->salt; + } + } + ``` + + After: + ```php + use Symfony\Component\Security\Core\User\UserInterface; + use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface; + + class User implements UserInterface, LegacyPasswordAuthenticatedUserInterface + { + // ... + + public function getPassword(): ?string + { + return $this->password; + } + + public function getSalt(): ?string + { + return $this->salt; + } + } + ``` + + * Calling `PasswordUpgraderInterface::upgradePassword()` with a `UserInterface` instance that + does not implement `PasswordAuthenticatedUserInterface` now throws a `\TypeError`. + * Calling methods `hashPassword()`, `isPasswordValid()` and `needsRehash()` on `UserPasswordHasherInterface` + with a `UserInterface` instance that does not implement `PasswordAuthenticatedUserInterface` now throws a `\TypeError` * Drop all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead * Drop support for `SessionInterface $session` as constructor argument of `SessionTokenStorage`, inject a `\Symfony\Component\HttpFoundation\RequestStack $requestStack` instead * Drop support for `session` provided by the ServiceLocator injected in `UsageTrackingTokenStorage`, provide a `request_stack` service instead diff --git a/src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php b/src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php index 1763631d61..e0174e6bbb 100644 --- a/src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php +++ b/src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php @@ -17,6 +17,7 @@ use Doctrine\Persistence\ObjectManager; use Doctrine\Persistence\ObjectRepository; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordUpgraderInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; @@ -115,9 +116,15 @@ class EntityUserProvider implements UserProviderInterface, PasswordUpgraderInter /** * {@inheritdoc} + * + * @final */ public function upgradePassword(UserInterface $user, string $newEncodedPassword): void { + if (!$user instanceof PasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/doctrine-bridge', '5.3', 'The "%s::upgradePassword()" method expects an instance of "%s" as first argument, the "%s" class should implement it.', PasswordUpgraderInterface::class, PasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + $class = $this->getClass(); if (!$user instanceof $class) { throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user))); diff --git a/src/Symfony/Bridge/Doctrine/Tests/Fixtures/User.php b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/User.php index c5cbc662fc..f4ff7c2840 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Fixtures/User.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/User.php @@ -14,10 +14,11 @@ namespace Symfony\Bridge\Doctrine\Tests\Fixtures; use Doctrine\ORM\Mapping\Column; use Doctrine\ORM\Mapping\Entity; use Doctrine\ORM\Mapping\Id; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; /** @Entity */ -class User implements UserInterface +class User implements UserInterface, PasswordAuthenticatedUserInterface { /** @Id @Column(type="integer") */ protected $id1; diff --git a/src/Symfony/Bridge/Doctrine/Tests/Security/User/EntityUserProviderTest.php b/src/Symfony/Bridge/Doctrine/Tests/Security/User/EntityUserProviderTest.php index 4f4755037d..216eee0e3f 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Security/User/EntityUserProviderTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Security/User/EntityUserProviderTest.php @@ -22,6 +22,7 @@ use Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface; use Symfony\Bridge\Doctrine\Tests\DoctrineTestHelper; use Symfony\Bridge\Doctrine\Tests\Fixtures\User; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordUpgraderInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -234,4 +235,5 @@ abstract class UserLoaderRepository implements ObjectRepository, UserLoaderInter abstract class PasswordUpgraderRepository implements ObjectRepository, PasswordUpgraderInterface { + abstract public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void; } diff --git a/src/Symfony/Bridge/Doctrine/composer.json b/src/Symfony/Bridge/Doctrine/composer.json index 5a58adf607..6e7384341c 100644 --- a/src/Symfony/Bridge/Doctrine/composer.json +++ b/src/Symfony/Bridge/Doctrine/composer.json @@ -38,7 +38,7 @@ "symfony/property-access": "^4.4|^5.0", "symfony/property-info": "^5.0", "symfony/proxy-manager-bridge": "^4.4|^5.0", - "symfony/security-core": "^5.0", + "symfony/security-core": "^5.3", "symfony/expression-language": "^4.4|^5.0", "symfony/uid": "^5.1", "symfony/validator": "^5.2", @@ -60,7 +60,7 @@ "symfony/messenger": "<4.4", "symfony/property-info": "<5", "symfony/security-bundle": "<5", - "symfony/security-core": "<5", + "symfony/security-core": "<5.3", "symfony/validator": "<5.2" }, "suggest": { diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 23b20c1aed..3f358b7319 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -42,6 +42,7 @@ use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder; use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder; use Symfony\Component\Security\Core\User\ChainUserProvider; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Http\Event\CheckPassportEvent; @@ -664,6 +665,9 @@ class SecurityExtension extends Extension implements PrependExtensionInterface { $encoderMap = []; foreach ($encoders as $class => $encoder) { + if (class_exists($class) && !is_a($class, PasswordAuthenticatedUserInterface::class, true)) { + trigger_deprecation('symfony/security-bundle', '5.3', 'Configuring an encoder for a user class that does not implement "%s" is deprecated, class "%s" should implement it.', PasswordAuthenticatedUserInterface::class, $class); + } $encoderMap[$class] = $this->createEncoder($encoder); } @@ -775,6 +779,10 @@ class SecurityExtension extends Extension implements PrependExtensionInterface { $hasherMap = []; foreach ($hashers as $class => $hasher) { + // @deprecated since Symfony 5.3, remove the check in 6.0 + if (class_exists($class) && !is_a($class, PasswordAuthenticatedUserInterface::class, true)) { + trigger_deprecation('symfony/security-bundle', '5.3', 'Configuring a password hasher for a user class that does not implement "%s" is deprecated, class "%s" should implement it.', PasswordAuthenticatedUserInterface::class, $class); + } $hasherMap[$class] = $this->createHasher($hasher); } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php index 1f41e2646d..7115c73be1 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php @@ -13,6 +13,7 @@ namespace Symfony\Bundle\SecurityBundle\Tests\Functional; use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User\ArrayUserProvider; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\User; use Symfony\Component\Security\Core\User\UserInterface; @@ -78,7 +79,7 @@ class SecurityTest extends AbstractWebTestCase } } -final class UserWithoutEquatable implements UserInterface +final class UserWithoutEquatable implements UserInterface, PasswordAuthenticatedUserInterface { private $username; private $password; @@ -119,7 +120,7 @@ final class UserWithoutEquatable implements UserInterface /** * {@inheritdoc} */ - public function getPassword() + public function getPassword(): ?string { return $this->password; } diff --git a/src/Symfony/Component/Ldap/Security/CheckLdapCredentialsListener.php b/src/Symfony/Component/Ldap/Security/CheckLdapCredentialsListener.php index c9abc92f26..e1b79443ac 100644 --- a/src/Symfony/Component/Ldap/Security/CheckLdapCredentialsListener.php +++ b/src/Symfony/Component/Ldap/Security/CheckLdapCredentialsListener.php @@ -17,6 +17,7 @@ use Symfony\Component\Ldap\Exception\ConnectionException; use Symfony\Component\Ldap\LdapInterface; use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\Exception\LogicException; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials; use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface; use Symfony\Component\Security\Http\Event\CheckPassportEvent; @@ -68,6 +69,11 @@ class CheckLdapCredentialsListener implements EventSubscriberInterface throw new BadCredentialsException('The presented password cannot be empty.'); } + $user = $passport->getUser(); + if (!$user instanceof PasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/ldap', '5.3', 'Not implementing the "%s" interface in class "%s" while using password-based authenticators is deprecated.', PasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + /** @var LdapInterface $ldap */ $ldap = $this->ldapLocator->get($ldapBadge->getLdapServiceId()); try { @@ -77,7 +83,7 @@ class CheckLdapCredentialsListener implements EventSubscriberInterface } else { throw new LogicException('Using the "query_string" config without using a "search_dn" and a "search_password" is not supported.'); } - $username = $ldap->escape($passport->getUser()->getUsername(), '', LdapInterface::ESCAPE_FILTER); + $username = $ldap->escape($user->getUsername(), '', LdapInterface::ESCAPE_FILTER); $query = str_replace('{username}', $username, $ldapBadge->getQueryString()); $result = $ldap->query($ldapBadge->getDnString(), $query)->execute(); if (1 !== $result->count()) { @@ -86,7 +92,7 @@ class CheckLdapCredentialsListener implements EventSubscriberInterface $dn = $result[0]->getDn(); } else { - $username = $ldap->escape($passport->getUser()->getUsername(), '', LdapInterface::ESCAPE_DN); + $username = $ldap->escape($user->getUsername(), '', LdapInterface::ESCAPE_DN); $dn = str_replace('{username}', $username, $ldapBadge->getDnString()); } diff --git a/src/Symfony/Component/Ldap/Security/LdapUser.php b/src/Symfony/Component/Ldap/Security/LdapUser.php index f82d44ef88..7c6a2c261c 100644 --- a/src/Symfony/Component/Ldap/Security/LdapUser.php +++ b/src/Symfony/Component/Ldap/Security/LdapUser.php @@ -13,6 +13,7 @@ namespace Symfony\Component\Ldap\Security; use Symfony\Component\Ldap\Entry; use Symfony\Component\Security\Core\User\EquatableInterface; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; /** @@ -20,7 +21,7 @@ use Symfony\Component\Security\Core\User\UserInterface; * * @final */ -class LdapUser implements UserInterface, EquatableInterface +class LdapUser implements UserInterface, PasswordAuthenticatedUserInterface, EquatableInterface { private $entry; private $username; diff --git a/src/Symfony/Component/Ldap/Security/LdapUserProvider.php b/src/Symfony/Component/Ldap/Security/LdapUserProvider.php index 50b82f76cb..6b0b4304ba 100644 --- a/src/Symfony/Component/Ldap/Security/LdapUserProvider.php +++ b/src/Symfony/Component/Ldap/Security/LdapUserProvider.php @@ -122,6 +122,8 @@ class LdapUserProvider implements UserProviderInterface, PasswordUpgraderInterfa /** * {@inheritdoc} + * + * @final */ public function upgradePassword(UserInterface $user, string $newEncodedPassword): void { diff --git a/src/Symfony/Component/Ldap/composer.json b/src/Symfony/Component/Ldap/composer.json index 4504b8356e..dc6b0a5d5a 100644 --- a/src/Symfony/Component/Ldap/composer.json +++ b/src/Symfony/Component/Ldap/composer.json @@ -22,11 +22,11 @@ "ext-ldap": "*" }, "require-dev": { - "symfony/security-core": "^5.0" + "symfony/security-core": "^5.3" }, "conflict": { "symfony/options-resolver": "<4.4", - "symfony/security-core": "<5" + "symfony/security-core": "<5.3" }, "autoload": { "psr-4": { "Symfony\\Component\\Ldap\\": "" }, diff --git a/src/Symfony/Component/PasswordHasher/Hasher/PasswordHasherFactoryInterface.php b/src/Symfony/Component/PasswordHasher/Hasher/PasswordHasherFactoryInterface.php index 038c34a318..0646f9f184 100644 --- a/src/Symfony/Component/PasswordHasher/Hasher/PasswordHasherFactoryInterface.php +++ b/src/Symfony/Component/PasswordHasher/Hasher/PasswordHasherFactoryInterface.php @@ -12,6 +12,7 @@ namespace Symfony\Component\PasswordHasher\Hasher; use Symfony\Component\PasswordHasher\PasswordHasherInterface; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; /** @@ -25,7 +26,7 @@ interface PasswordHasherFactoryInterface /** * Returns the password hasher to use for the given user. * - * @param UserInterface|string $user A UserInterface instance or a class name + * @param PasswordAuthenticatedUserInterface|UserInterface|string $user A PasswordAuthenticatedUserInterface/UserInterface instance or a class name * * @throws \RuntimeException When no password hasher could be found for the user */ diff --git a/src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasher.php b/src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasher.php index bb11680665..f26164d7e5 100644 --- a/src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasher.php +++ b/src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasher.php @@ -11,12 +11,16 @@ namespace Symfony\Component\PasswordHasher\Hasher; +use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; /** * Hashes passwords based on the user and the PasswordHasherFactory. * * @author Ariel Ferrandini + * + * @final */ class UserPasswordHasher implements UserPasswordHasherInterface { @@ -27,36 +31,70 @@ class UserPasswordHasher implements UserPasswordHasherInterface $this->hasherFactory = $hasherFactory; } - public function hashPassword(UserInterface $user, string $plainPassword): string + /** + * @param PasswordAuthenticatedUserInterface $user + */ + public function hashPassword($user, string $plainPassword): string { + if (!$user instanceof PasswordAuthenticatedUserInterface) { + if (!$user instanceof UserInterface) { + throw new \TypeError(sprintf('Expected an instance of "%s" as first argument, but got "%s".', UserInterface::class, get_debug_type($user))); + } + trigger_deprecation('symfony/password-hasher', '5.3', 'The "%s()" method expects a "%s" instance as first argument. Not implementing it in class "%s" is deprecated.', __METHOD__, PasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + + $salt = $user->getSalt(); + if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/password-hasher', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + $hasher = $this->hasherFactory->getPasswordHasher($user); - return $hasher->hash($plainPassword, $user->getSalt()); + return $hasher->hash($plainPassword, $salt); } /** - * {@inheritdoc} + * @param PasswordAuthenticatedUserInterface $user */ - public function isPasswordValid(UserInterface $user, string $plainPassword): bool + public function isPasswordValid($user, string $plainPassword): bool { + if (!$user instanceof PasswordAuthenticatedUserInterface) { + if (!$user instanceof UserInterface) { + throw new \TypeError(sprintf('Expected an instance of "%s" as first argument, but got "%s".', UserInterface::class, get_debug_type($user))); + } + trigger_deprecation('symfony/password-hasher', '5.3', 'The "%s()" method expects a "%s" instance as first argument. Not implementing it in class "%s" is deprecated.', __METHOD__, PasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + + $salt = $user->getSalt(); + if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/password-hasher', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + if (null === $user->getPassword()) { return false; } $hasher = $this->hasherFactory->getPasswordHasher($user); - return $hasher->verify($user->getPassword(), $plainPassword, $user->getSalt()); + return $hasher->verify($user->getPassword(), $plainPassword, $salt); } /** - * {@inheritdoc} + * @param PasswordAuthenticatedUserInterface $user */ - public function needsRehash(UserInterface $user): bool + public function needsRehash($user): bool { if (null === $user->getPassword()) { return false; } + if (!$user instanceof PasswordAuthenticatedUserInterface) { + if (!$user instanceof UserInterface) { + throw new \TypeError(sprintf('Expected an instance of "%s" as first argument, but got "%s".', UserInterface::class, get_debug_type($user))); + } + trigger_deprecation('symfony/password-hasher', '5.3', 'The "%s()" method expects a "%s" instance as first argument. Not implementing it in class "%s" is deprecated.', __METHOD__, PasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + $hasher = $this->hasherFactory->getPasswordHasher($user); return $hasher->needsRehash($user->getPassword()); diff --git a/src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasherInterface.php b/src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasherInterface.php index 13a5b51a8a..cce264a9b2 100644 --- a/src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasherInterface.php +++ b/src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasherInterface.php @@ -11,27 +11,17 @@ namespace Symfony\Component\PasswordHasher\Hasher; -use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; /** * Interface for the user password hasher service. * * @author Ariel Ferrandini + * + * @method string hashPassword(PasswordAuthenticatedUserInterface $user, string $plainPassword) Hashes the plain password for the given user. + * @method string isPasswordValid(PasswordAuthenticatedUserInterface $user, string $plainPassword) Checks if the plaintext password matches the user's password. + * @method bool needsRehash(PasswordAuthenticatedUserInterface $user) Checks if the plaintext password matches the user's password. */ interface UserPasswordHasherInterface { - /** - * Hashes the plain password for the given user. - */ - public function hashPassword(UserInterface $user, string $plainPassword): string; - - /** - * Checks if the plaintext password matches the user's password. - */ - public function isPasswordValid(UserInterface $user, string $plainPassword): bool; - - /** - * Checks if a password hash would benefit from rehashing. - */ - public function needsRehash(UserInterface $user): bool; } diff --git a/src/Symfony/Component/PasswordHasher/Tests/Hasher/UserPasswordHasherTest.php b/src/Symfony/Component/PasswordHasher/Tests/Hasher/UserPasswordHasherTest.php index 5aaee750af..1f2ebe37ce 100644 --- a/src/Symfony/Component/PasswordHasher/Tests/Hasher/UserPasswordHasherTest.php +++ b/src/Symfony/Component/PasswordHasher/Tests/Hasher/UserPasswordHasherTest.php @@ -12,18 +12,52 @@ namespace Symfony\Component\PasswordHasher\Tests\Hasher; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher; use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasher; use Symfony\Component\PasswordHasher\PasswordHasherInterface; +use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\User; use Symfony\Component\Security\Core\User\UserInterface; class UserPasswordHasherTest extends TestCase { + use ExpectDeprecationTrait; + + /** + * @group legacy + */ + public function testHashWithNonPasswordAuthenticatedUser() + { + $this->expectDeprecation('Since symfony/password-hasher 5.3: Returning a string from "getSalt()" without implementing the "Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface" interface is deprecated, the "%s" class should implement it.'); + + $userMock = $this->createMock('Symfony\Component\Security\Core\User\UserInterface'); + $userMock->expects($this->any()) + ->method('getSalt') + ->willReturn('userSalt'); + + $mockHasher = $this->createMock(PasswordHasherInterface::class); + $mockHasher->expects($this->any()) + ->method('hash') + ->with($this->equalTo('plainPassword'), $this->equalTo('userSalt')) + ->willReturn('hash'); + + $mockPasswordHasherFactory = $this->createMock(PasswordHasherFactoryInterface::class); + $mockPasswordHasherFactory->expects($this->any()) + ->method('getPasswordHasher') + ->with($this->equalTo($userMock)) + ->willReturn($mockHasher); + + $passwordHasher = new UserPasswordHasher($mockPasswordHasherFactory); + + $encoded = $passwordHasher->hashPassword($userMock, 'plainPassword'); + $this->assertEquals('hash', $encoded); + } + public function testHash() { - $userMock = $this->createMock('Symfony\Component\Security\Core\User\UserInterface'); + $userMock = $this->createMock(TestPasswordAuthenticatedUser::class); $userMock->expects($this->any()) ->method('getSalt') ->willReturn('userSalt'); @@ -48,7 +82,7 @@ class UserPasswordHasherTest extends TestCase public function testVerify() { - $userMock = $this->createMock(UserInterface::class); + $userMock = $this->createMock(TestPasswordAuthenticatedUser::class); $userMock->expects($this->any()) ->method('getSalt') ->willReturn('userSalt'); @@ -93,3 +127,7 @@ class UserPasswordHasherTest extends TestCase $this->assertFalse($passwordHasher->needsRehash($user)); } } + +abstract class TestPasswordAuthenticatedUser implements LegacyPasswordAuthenticatedUserInterface, UserInterface +{ +} diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md index 7e0b6b2a33..652330b8ce 100644 --- a/src/Symfony/Component/Security/CHANGELOG.md +++ b/src/Symfony/Component/Security/CHANGELOG.md @@ -4,6 +4,8 @@ CHANGELOG 5.3 --- + * Add `PasswordAuthenticatedUserInterface` for user classes that use passwords + * Add `LegacyPasswordAuthenticatedUserInterface` for user classes that use user-provided salts in addition to passwords * Deprecate all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead * Deprecate the `SessionInterface $session` constructor argument of `SessionTokenStorage`, inject a `\Symfony\Component\HttpFoundation\RequestStack $requestStack` instead * Deprecate the `session` service provided by the ServiceLocator injected in `UsageTrackingTokenStorage`, provide a `request_stack` service instead diff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php index 26beb6b945..eca9357f63 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php @@ -11,16 +11,18 @@ namespace Symfony\Component\Security\Core\Authentication\Provider; +use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface; use Symfony\Component\Security\Core\Exception\AuthenticationServiceException; use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; +use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordUpgraderInterface; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; -use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; /** * DaoAuthenticationProvider uses a UserProviderInterface to retrieve the user @@ -67,11 +69,20 @@ class DaoAuthenticationProvider extends UserAuthenticationProvider throw new BadCredentialsException('The presented password is invalid.'); } + if (!$user instanceof PasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/security-core', '5.3', 'Using password-based authentication listeners while not implementing "%s" interface from class "%s" is deprecated.', PasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + + $salt = $user->getSalt(); + if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/security-core', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + // deprecated since Symfony 5.3 if ($this->hasherFactory instanceof EncoderFactoryInterface) { $encoder = $this->hasherFactory->getEncoder($user); - if (!$encoder->isPasswordValid($user->getPassword(), $presentedPassword, $user->getSalt())) { + if (!$encoder->isPasswordValid($user->getPassword(), $presentedPassword, $salt)) { throw new BadCredentialsException('The presented password is invalid.'); } @@ -84,12 +95,12 @@ class DaoAuthenticationProvider extends UserAuthenticationProvider $hasher = $this->hasherFactory->getPasswordHasher($user); - if (!$hasher->verify($user->getPassword(), $presentedPassword, $user->getSalt())) { + if (!$hasher->verify($user->getPassword(), $presentedPassword, $salt)) { throw new BadCredentialsException('The presented password is invalid.'); } if ($this->userProvider instanceof PasswordUpgraderInterface && $hasher->needsRehash($user->getPassword())) { - $this->userProvider->upgradePassword($user, $hasher->hash($presentedPassword, $user->getSalt())); + $this->userProvider->upgradePassword($user, $hasher->hash($presentedPassword, $salt)); } } } diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php index 9106334b99..0083ae3957 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Core\Authentication\Token; use Symfony\Component\Security\Core\User\EquatableInterface; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; /** @@ -262,10 +263,12 @@ abstract class AbstractToken implements TokenInterface return !(bool) $this->user->isEqualTo($user); } + // @deprecated since Symfony 5.3, check for PasswordAuthenticatedUserInterface on both user objects before comparing passwords if ($this->user->getPassword() !== $user->getPassword()) { return true; } + // @deprecated since Symfony 5.3, check for LegacyPasswordAuthenticatedUserInterface on both user objects before comparing salts if ($this->user->getSalt() !== $user->getSalt()) { return true; } diff --git a/src/Symfony/Component/Security/Core/Encoder/UserPasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/UserPasswordEncoder.php index 32ab07c690..bbbb5d1bbd 100644 --- a/src/Symfony/Component/Security/Core/Encoder/UserPasswordEncoder.php +++ b/src/Symfony/Component/Security/Core/Encoder/UserPasswordEncoder.php @@ -12,6 +12,8 @@ namespace Symfony\Component\Security\Core\Encoder; use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasher; +use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; trigger_deprecation('symfony/security-core', '5.3', sprintf('The "%s" class is deprecated, use "%s" instead.', UserPasswordEncoder::class, UserPasswordHasher::class)); @@ -39,6 +41,15 @@ class UserPasswordEncoder implements UserPasswordEncoderInterface { $encoder = $this->encoderFactory->getEncoder($user); + if (!$user instanceof PasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/password-hasher', '5.3', 'Not implementing the "%s" interface while using "%s" is deprecated, the "%s" class should implement it.', PasswordAuthenticatedUserInterface::class, __CLASS__, get_debug_type($user)); + } + + $salt = $user->getSalt(); + if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/password-hasher', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + return $encoder->encodePassword($plainPassword, $user->getSalt()); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/DaoAuthenticationProviderTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/DaoAuthenticationProviderTest.php index 20e75b8076..a308cc6c62 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/DaoAuthenticationProviderTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/DaoAuthenticationProviderTest.php @@ -12,6 +12,9 @@ namespace Symfony\Component\Security\Core\Tests\Authentication\Provider; use PHPUnit\Framework\TestCase; +use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; +use Symfony\Component\PasswordHasher\Hasher\PlaintextPasswordHasher; +use Symfony\Component\PasswordHasher\PasswordHasherInterface; use Symfony\Component\Security\Core\Authentication\Provider\DaoAuthenticationProvider; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface; @@ -23,9 +26,6 @@ use Symfony\Component\Security\Core\User\User; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; -use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; -use Symfony\Component\PasswordHasher\Hasher\PlaintextPasswordHasher; -use Symfony\Component\PasswordHasher\PasswordHasherInterface; class DaoAuthenticationProviderTest extends TestCase { @@ -380,4 +380,5 @@ class TestUser implements UserInterface } interface PasswordUpgraderProvider extends UserProviderInterface, PasswordUpgraderInterface { + public function upgradePassword(UserInterface $user, string $newHashedPassword): void; } diff --git a/src/Symfony/Component/Security/Core/Tests/User/ChainUserProviderTest.php b/src/Symfony/Component/Security/Core/Tests/User/ChainUserProviderTest.php index b7e2a411b3..35075a77de 100644 --- a/src/Symfony/Component/Security/Core/Tests/User/ChainUserProviderTest.php +++ b/src/Symfony/Component/Security/Core/Tests/User/ChainUserProviderTest.php @@ -15,6 +15,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\User\ChainUserProvider; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordUpgraderInterface; use Symfony\Component\Security\Core\User\User; use Symfony\Component\Security\Core\User\UserInterface; @@ -251,14 +252,14 @@ class ChainUserProviderTest extends TestCase { $user = new User('user', 'pwd'); - $provider1 = $this->createMock(PasswordUpgraderInterface::class); + $provider1 = $this->getMockForAbstractClass(MigratingProvider::class); $provider1 ->expects($this->once()) ->method('upgradePassword') ->willThrowException(new UnsupportedUserException('unsupported')) ; - $provider2 = $this->createMock(PasswordUpgraderInterface::class); + $provider2 = $this->getMockForAbstractClass(MigratingProvider::class); $provider2 ->expects($this->once()) ->method('upgradePassword') @@ -269,3 +270,8 @@ class ChainUserProviderTest extends TestCase $provider->upgradePassword($user, 'foobar'); } } + +abstract class MigratingProvider implements PasswordUpgraderInterface +{ + abstract public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void; +} diff --git a/src/Symfony/Component/Security/Core/User/ChainUserProvider.php b/src/Symfony/Component/Security/Core/User/ChainUserProvider.php index 233212508b..fedcdb6a53 100644 --- a/src/Symfony/Component/Security/Core/User/ChainUserProvider.php +++ b/src/Symfony/Component/Security/Core/User/ChainUserProvider.php @@ -73,7 +73,7 @@ class ChainUserProvider implements UserProviderInterface, PasswordUpgraderInterf foreach ($this->providers as $provider) { try { - if (!$provider->supportsClass(\get_class($user))) { + if (!$provider->supportsClass(get_debug_type($user))) { continue; } @@ -110,10 +110,16 @@ class ChainUserProvider implements UserProviderInterface, PasswordUpgraderInterf } /** + * @param PasswordAuthenticatedUserInterface $user + * * {@inheritdoc} */ - public function upgradePassword(UserInterface $user, string $newEncodedPassword): void + public function upgradePassword($user, string $newEncodedPassword): void { + if (!$user instanceof PasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/security-core', '5.3', 'The "%s::upgradePassword()" method expects an instance of "%s" as first argument, the "%s" class should implement it.', PasswordUpgraderInterface::class, PasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + foreach ($this->providers as $provider) { if ($provider instanceof PasswordUpgraderInterface) { try { diff --git a/src/Symfony/Component/Security/Core/User/LegacyPasswordAuthenticatedUserInterface.php b/src/Symfony/Component/Security/Core/User/LegacyPasswordAuthenticatedUserInterface.php new file mode 100644 index 0000000000..fcffe0b91b --- /dev/null +++ b/src/Symfony/Component/Security/Core/User/LegacyPasswordAuthenticatedUserInterface.php @@ -0,0 +1,28 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\User; + +/** + * For users that can be authenticated using a password/salt couple. + * + * Once all password hashes have been upgraded to a modern algorithm via password migrations, + * implement {@see PasswordAuthenticatedUserInterface} instead. + * + * @author Robin Chalas + */ +interface LegacyPasswordAuthenticatedUserInterface extends PasswordAuthenticatedUserInterface +{ + /** + * Returns the salt that was originally used to hash the password. + */ + public function getSalt(): ?string; +} diff --git a/src/Symfony/Component/Security/Core/User/PasswordAuthenticatedUserInterface.php b/src/Symfony/Component/Security/Core/User/PasswordAuthenticatedUserInterface.php new file mode 100644 index 0000000000..e9d7863071 --- /dev/null +++ b/src/Symfony/Component/Security/Core/User/PasswordAuthenticatedUserInterface.php @@ -0,0 +1,30 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\User; + +/** + * For users that can be authenticated using a password. + * + * @author Robin Chalas + * @author Wouter de Jong + */ +interface PasswordAuthenticatedUserInterface +{ + /** + * Returns the hashed password used to authenticate the user. + * + * Usually on authentication, a plain-text password will be compared to this value. + * + * @return string|null The hashed password or null (if not set or erased) + */ + public function getPassword(): ?string; +} diff --git a/src/Symfony/Component/Security/Core/User/PasswordUpgraderInterface.php b/src/Symfony/Component/Security/Core/User/PasswordUpgraderInterface.php index ef62023d53..16195fad56 100644 --- a/src/Symfony/Component/Security/Core/User/PasswordUpgraderInterface.php +++ b/src/Symfony/Component/Security/Core/User/PasswordUpgraderInterface.php @@ -13,15 +13,12 @@ namespace Symfony\Component\Security\Core\User; /** * @author Nicolas Grekas + * + * @method void upgradePassword(PasswordAuthenticatedUserInterface|UserInterface $user, string $newHashedPassword) Upgrades the hashed password of a user, typically for using a better hash algorithm. + * This method should persist the new password in the user storage and update the $user object accordingly. + * Because you don't want your users not being able to log in, this method should be opportunistic: + * it's fine if it does nothing or if it fails without throwing any exception. */ interface PasswordUpgraderInterface { - /** - * Upgrades the hashed password of a user, typically for using a better hash algorithm. - * - * This method should persist the new password in the user storage and update the $user object accordingly. - * Because you don't want your users not being able to log in, this method should be opportunistic: - * it's fine if it does nothing or if it fails without throwing any exception. - */ - public function upgradePassword(UserInterface $user, string $newHashedPassword): void; } diff --git a/src/Symfony/Component/Security/Core/User/User.php b/src/Symfony/Component/Security/Core/User/User.php index 5429baa0cd..9a749dc217 100644 --- a/src/Symfony/Component/Security/Core/User/User.php +++ b/src/Symfony/Component/Security/Core/User/User.php @@ -18,7 +18,7 @@ namespace Symfony\Component\Security\Core\User; * * @author Fabien Potencier */ -final class User implements UserInterface, EquatableInterface +final class User implements UserInterface, PasswordAuthenticatedUserInterface, EquatableInterface { private $username; private $password; diff --git a/src/Symfony/Component/Security/Core/User/UserInterface.php b/src/Symfony/Component/Security/Core/User/UserInterface.php index c005e3ca9c..47661de0b7 100644 --- a/src/Symfony/Component/Security/Core/User/UserInterface.php +++ b/src/Symfony/Component/Security/Core/User/UserInterface.php @@ -52,6 +52,8 @@ interface UserInterface * This should be the hashed password. On authentication, a plain-text * password will be hashed, and then compared to this value. * + * This method is deprecated since Symfony 5.3, implement it from {@link PasswordAuthenticatedUserInterface} instead. + * * @return string|null The hashed password if any */ public function getPassword(); @@ -61,6 +63,8 @@ interface UserInterface * * This can return null if the password was not hashed using a salt. * + * This method is deprecated since Symfony 5.3, implement it from {@link LegacyPasswordAuthenticatedUserInterface} instead. + * * @return string|null The salt */ public function getSalt(); diff --git a/src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php b/src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php index 0181ccbcbb..bf273f2fa0 100644 --- a/src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php +++ b/src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php @@ -11,11 +11,13 @@ namespace Symfony\Component\Security\Core\Validator\Constraints; +use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface; use Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface; +use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; -use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\ConstraintValidator; use Symfony\Component\Validator\Exception\ConstraintDefinitionException; @@ -32,7 +34,7 @@ class UserPasswordValidator extends ConstraintValidator public function __construct(TokenStorageInterface $tokenStorage, $hasherFactory) { if ($hasherFactory instanceof EncoderFactoryInterface) { - trigger_deprecation('symfony/security-core', '5.3', 'Passing a "%s" instance to the "%s" constructor is deprecated, use "%s" instead.', EncoderFactoryInterface::class, __CLASS__, PasswordHasherFactoryInterface::class); + trigger_deprecation('symfony/security-core', '5.3', 'Passing a "%s" instance to the "%s" constructor is deprecated, use "%s" instead.', EncoderFactoryInterface::class, __CLASS__, PasswordHasherFactoryInterface::class); } $this->tokenStorage = $tokenStorage; @@ -60,6 +62,15 @@ class UserPasswordValidator extends ConstraintValidator throw new ConstraintDefinitionException('The User object must implement the UserInterface interface.'); } + if (!$user instanceof PasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/security-core', '5.3', 'Using the "%s" validation constraint is deprecated.', PasswordAuthenticatedUserInterface::class, get_debug_type($user), UserPassword::class); + } + + $salt = $user->getSalt(); + if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/security-core', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + $hasher = $this->hasherFactory instanceof EncoderFactoryInterface ? $this->hasherFactory->getEncoder($user) : $this->hasherFactory->getPasswordHasher($user); if (null === $user->getPassword() || !($hasher instanceof PasswordEncoderInterface ? $hasher->isPasswordValid($user->getPassword(), $password, $user->getSalt()) : $hasher->verify($user->getPassword(), $password, $user->getSalt()))) { diff --git a/src/Symfony/Component/Security/Guard/Authenticator/GuardBridgeAuthenticator.php b/src/Symfony/Component/Security/Guard/Authenticator/GuardBridgeAuthenticator.php index 1d7cec6c6e..8e67822780 100644 --- a/src/Symfony/Component/Security/Guard/Authenticator/GuardBridgeAuthenticator.php +++ b/src/Symfony/Component/Security/Guard/Authenticator/GuardBridgeAuthenticator.php @@ -16,6 +16,7 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordUpgraderInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; @@ -76,6 +77,10 @@ class GuardBridgeAuthenticator implements InteractiveAuthenticatorInterface, Aut $user = $this->getUser($credentials); } + if ($this->guard instanceof PasswordAuthenticatedInterface && !$user instanceof PasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/security-guard', '5.3', 'Not implementing the "%s" interface in class "%s" while using password-based guard authenticators is deprecated.', PasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + $passport = new Passport($user, new CustomCredentials([$this->guard, 'checkCredentials'], $credentials)); if ($this->userProvider instanceof PasswordUpgraderInterface && $this->guard instanceof PasswordAuthenticatedInterface && (null !== $password = $this->guard->getPassword($credentials))) { $passport->addBadge(new PasswordUpgradeBadge($password, $this->userProvider)); diff --git a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php index 806b1b6504..71d663e3dc 100644 --- a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php @@ -19,6 +19,7 @@ use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\AuthenticationExpiredException; use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordUpgraderInterface; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -121,6 +122,10 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface throw new \UnexpectedValueException(sprintf('The "%s::getUser()" method must return a UserInterface. You returned "%s".', get_debug_type($guardAuthenticator), get_debug_type($user))); } + if ($guardAuthenticator instanceof PasswordAuthenticatedInterface && !$user instanceof PasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/security-guard', '5.3', 'Not implementing the "%s" interface in class "%s" while using password-based guard authenticators is deprecated.', PasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + $this->userChecker->checkPreAuth($user); if (true !== $checkCredentialsResult = $guardAuthenticator->checkCredentials($token->getCredentials(), $user)) { if (false !== $checkCredentialsResult) { diff --git a/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php b/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php index 95f9c88542..8a3db46ede 100644 --- a/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php +++ b/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php @@ -12,14 +12,16 @@ namespace Symfony\Component\Security\Http\EventListener; use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface; use Symfony\Component\Security\Core\Exception\BadCredentialsException; +use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PasswordUpgradeBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\CustomCredentials; use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials; use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface; use Symfony\Component\Security\Http\Event\CheckPassportEvent; -use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; /** * This listeners uses the interfaces of authenticators to @@ -52,6 +54,11 @@ class CheckCredentialsListener implements EventSubscriberInterface if ($passport instanceof UserPassportInterface && $passport->hasBadge(PasswordCredentials::class)) { // Use the password hasher to validate the credentials $user = $passport->getUser(); + + if (!$user instanceof PasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/security-http', '5.3', 'Not implementing the "%s" interface in class "%s" while using password-based authentication is deprecated.', PasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + /** @var PasswordCredentials $badge */ $badge = $passport->getBadge(PasswordCredentials::class); @@ -68,13 +75,18 @@ class CheckCredentialsListener implements EventSubscriberInterface throw new BadCredentialsException('The presented password is invalid.'); } + $salt = $user->getSalt(); + if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) { + trigger_deprecation('symfony/security-http', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user)); + } + // @deprecated since Symfony 5.3 if ($this->hasherFactory instanceof EncoderFactoryInterface) { - if (!$this->hasherFactory->getEncoder($user)->isPasswordValid($user->getPassword(), $presentedPassword, $user->getSalt())) { + if (!$this->hasherFactory->getEncoder($user)->isPasswordValid($user->getPassword(), $presentedPassword, $salt)) { throw new BadCredentialsException('The presented password is invalid.'); } } else { - if (!$this->hasherFactory->getPasswordHasher($user)->verify($user->getPassword(), $presentedPassword, $user->getSalt())) { + if (!$this->hasherFactory->getPasswordHasher($user)->verify($user->getPassword(), $presentedPassword, $salt)) { throw new BadCredentialsException('The presented password is invalid.'); } } diff --git a/src/Symfony/Component/Security/Http/Tests/EventListener/PasswordMigratingListenerTest.php b/src/Symfony/Component/Security/Http/Tests/EventListener/PasswordMigratingListenerTest.php index 0e32433c5b..1583ce62a1 100644 --- a/src/Symfony/Component/Security/Http/Tests/EventListener/PasswordMigratingListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/EventListener/PasswordMigratingListenerTest.php @@ -13,7 +13,10 @@ namespace Symfony\Component\Security\Http\Tests\EventListener; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; +use Symfony\Component\PasswordHasher\PasswordHasherInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordUpgraderInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; @@ -25,8 +28,6 @@ use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPasspor use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface; use Symfony\Component\Security\Http\Event\LoginSuccessEvent; use Symfony\Component\Security\Http\EventListener\PasswordMigratingListener; -use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; -use Symfony\Component\PasswordHasher\PasswordHasherInterface; class PasswordMigratingListenerTest extends TestCase { @@ -36,7 +37,7 @@ class PasswordMigratingListenerTest extends TestCase protected function setUp(): void { - $this->user = $this->createMock(UserInterface::class); + $this->user = $this->createMock(TestPasswordAuthenticatedUser::class); $this->user->expects($this->any())->method('getPassword')->willReturn('old-hash'); $encoder = $this->createMock(PasswordHasherInterface::class); $encoder->expects($this->any())->method('needsRehash')->willReturn(true); @@ -62,7 +63,7 @@ class PasswordMigratingListenerTest extends TestCase yield [$this->createEvent(new SelfValidatingPassport(new UserBadge('test', function () { return $this->createMock(UserInterface::class); })))]; // blank password - yield [$this->createEvent(new SelfValidatingPassport(new UserBadge('test', function () { return $this->createMock(UserInterface::class); }), [new PasswordUpgradeBadge('', $this->createPasswordUpgrader())]))]; + yield [$this->createEvent(new SelfValidatingPassport(new UserBadge('test', function () { return $this->createMock(TestPasswordAuthenticatedUser::class); }), [new PasswordUpgradeBadge('', $this->createPasswordUpgrader())]))]; // no user yield [$this->createEvent($this->createMock(PassportInterface::class))]; @@ -96,7 +97,7 @@ class PasswordMigratingListenerTest extends TestCase public function testUpgradeWithoutUpgrader() { - $userLoader = $this->createMock(MigratingUserProvider::class); + $userLoader = $this->getMockForAbstractClass(TestMigratingUserProvider::class); $userLoader->expects($this->any())->method('loadUserByUsername')->willReturn($this->user); $userLoader->expects($this->once()) @@ -110,7 +111,7 @@ class PasswordMigratingListenerTest extends TestCase private function createPasswordUpgrader() { - return $this->createMock(PasswordUpgraderInterface::class); + return $this->getMockForAbstractClass(TestMigratingUserProvider::class); } private function createEvent(PassportInterface $passport) @@ -119,6 +120,12 @@ class PasswordMigratingListenerTest extends TestCase } } -abstract class MigratingUserProvider implements UserProviderInterface, PasswordUpgraderInterface +abstract class TestMigratingUserProvider implements UserProviderInterface, PasswordUpgraderInterface { + abstract public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void; +} + +abstract class TestPasswordAuthenticatedUser implements UserInterface, PasswordAuthenticatedUserInterface +{ + abstract public function getPassword(): ?string; }