From 30e2c008b4cfd6eefdb278fb11e67945730db057 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Mon, 5 Jul 2021 00:26:05 +0200 Subject: [PATCH] [Security] Remove getPassword() and getSalt() from UserInterface --- .../Security/User/EntityUserProvider.php | 10 +------- .../Bridge/Doctrine/Tests/Fixtures/User.php | 4 ---- src/Symfony/Bridge/Doctrine/composer.json | 4 ++-- .../Ldap/Security/LdapUserProvider.php | 3 ++- .../Provider/DaoAuthenticationProvider.php | 18 +++++++-------- .../Authentication/Token/AbstractToken.php | 19 +++++++++------ .../Component/Security/Core/CHANGELOG.md | 4 +++- .../DaoAuthenticationProviderTest.php | 18 ++++++--------- .../Security/Core/User/ChainUserProvider.php | 10 +------- .../Core/User/PasswordUpgraderInterface.php | 13 +++++++---- .../Security/Core/User/UserInterface.php | 23 ------------------- .../Constraints/UserPasswordValidator.php | 14 ++--------- .../Provider/GuardAuthenticationProvider.php | 3 ++- .../CheckCredentialsListener.php | 9 ++------ .../PasswordMigratingListener.php | 12 +++++++--- .../TokenBasedRememberMeServices.php | 5 ++++ .../Fixtures/PasswordUpgraderProvider.php | 3 ++- 17 files changed, 66 insertions(+), 106 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php b/src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php index e24b71367dc2f..4b5d795cbb0f7 100644 --- a/src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php +++ b/src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php @@ -133,16 +133,8 @@ public function supportsClass(string $class) * * @final */ - public function upgradePassword($user, string $newHashedPassword): void + public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): 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)); - - if (!$user instanceof UserInterface) { - throw new \TypeError(sprintf('The "%s::upgradePassword()" method expects an instance of "%s" as first argument, "%s" given.', 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 0ffaf19883361..b8b10094dd6bd 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Fixtures/User.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/User.php @@ -44,10 +44,6 @@ public function getPassword(): ?string { } - public function getSalt(): ?string - { - } - public function getUsername(): string { return $this->name; diff --git a/src/Symfony/Bridge/Doctrine/composer.json b/src/Symfony/Bridge/Doctrine/composer.json index cd867186b598b..adb5790dfb489 100644 --- a/src/Symfony/Bridge/Doctrine/composer.json +++ b/src/Symfony/Bridge/Doctrine/composer.json @@ -37,7 +37,7 @@ "symfony/property-access": "^5.4|^6.0", "symfony/property-info": "^5.4|^6.0", "symfony/proxy-manager-bridge": "^5.4|^6.0", - "symfony/security-core": "^5.4|^6.0", + "symfony/security-core": "^6.0", "symfony/expression-language": "^5.4|^6.0", "symfony/uid": "^5.4|^6.0", "symfony/validator": "^5.4|^6.0", @@ -58,7 +58,7 @@ "symfony/messenger": "<5.4", "symfony/property-info": "<5.4", "symfony/security-bundle": "<5.4", - "symfony/security-core": "<5.4", + "symfony/security-core": "<6.0", "symfony/validator": "<5.4" }, "suggest": { diff --git a/src/Symfony/Component/Ldap/Security/LdapUserProvider.php b/src/Symfony/Component/Ldap/Security/LdapUserProvider.php index 01bc03ad06451..ac9ed288f4768 100644 --- a/src/Symfony/Component/Ldap/Security/LdapUserProvider.php +++ b/src/Symfony/Component/Ldap/Security/LdapUserProvider.php @@ -18,6 +18,7 @@ use Symfony\Component\Security\Core\Exception\InvalidArgumentException; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\Exception\UserNotFoundException; +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; @@ -132,7 +133,7 @@ public function refreshUser(UserInterface $user) * * @final */ - public function upgradePassword($user, string $newHashedPassword): void + public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void { if (!$user instanceof LdapUser) { throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user))); diff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php index 03f096416cd28..323418b35484f 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php @@ -15,6 +15,7 @@ use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Exception\AuthenticationServiceException; use Symfony\Component\Security\Core\Exception\BadCredentialsException; +use Symfony\Component\Security\Core\Exception\InvalidArgumentException; use Symfony\Component\Security\Core\Exception\UserNotFoundException; use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; @@ -51,8 +52,13 @@ public function __construct(UserProviderInterface $userProvider, UserCheckerInte */ protected function checkAuthentication(UserInterface $user, UsernamePasswordToken $token) { + if (!$user instanceof PasswordAuthenticatedUserInterface) { + throw new InvalidArgumentException(sprintf('Class "%s" must implement "%s" for using password-based authentication.', get_debug_type($user), PasswordAuthenticatedUserInterface::class)); + } + $currentUser = $token->getUser(); - if ($currentUser instanceof UserInterface) { + + if ($currentUser instanceof PasswordAuthenticatedUserInterface) { if ($currentUser->getPassword() !== $user->getPassword()) { throw new BadCredentialsException('The credentials were changed from another session.'); } @@ -65,15 +71,7 @@ protected function checkAuthentication(UserInterface $user, UsernamePasswordToke 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)); - } - + $salt = $user instanceof LegacyPasswordAuthenticatedUserInterface ? $user->getSalt() : null; $hasher = $this->hasherFactory->getPasswordHasher($user); if (!$hasher->verify($user->getPassword(), $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 507e0d48276d9..3d16e5b0c17c7 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\LegacyPasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -273,14 +274,18 @@ private function hasUserChanged(UserInterface $user): bool 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; - } + if ($this->user instanceof PasswordAuthenticatedUserInterface || $user instanceof PasswordAuthenticatedUserInterface) { + if (!$this->user instanceof PasswordAuthenticatedUserInterface || !$user instanceof PasswordAuthenticatedUserInterface || $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; + if ($this->user instanceof LegacyPasswordAuthenticatedUserInterface xor $user instanceof LegacyPasswordAuthenticatedUserInterface) { + return true; + } + + if ($this->user instanceof LegacyPasswordAuthenticatedUserInterface && $user instanceof LegacyPasswordAuthenticatedUserInterface && $this->user->getSalt() !== $user->getSalt()) { + return true; + } } $userRoles = array_map('strval', (array) $user->getRoles()); diff --git a/src/Symfony/Component/Security/Core/CHANGELOG.md b/src/Symfony/Component/Security/Core/CHANGELOG.md index 6e836c3048b3d..bd41fd6963d85 100644 --- a/src/Symfony/Component/Security/Core/CHANGELOG.md +++ b/src/Symfony/Component/Security/Core/CHANGELOG.md @@ -5,7 +5,9 @@ CHANGELOG --- * `TokenInterface` does not extend `Serializable` anymore - * Remove all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead + * Remove all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead + * Remove methods `getPassword()` and `getSalt()` from `UserInterface`, use `PasswordAuthenticatedUserInterface` + or `LegacyPasswordAuthenticatedUserInterface` instead 5.3 --- 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 90702a2f79023..3d4f6048980eb 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/DaoAuthenticationProviderTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/DaoAuthenticationProviderTest.php @@ -23,6 +23,7 @@ use Symfony\Component\Security\Core\Exception\UserNotFoundException; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\InMemoryUserProvider; +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; @@ -188,7 +189,7 @@ public function testCheckAuthenticationWhenCredentialsAreNotValid() public function testCheckAuthenticationDoesNotReauthenticateWhenPasswordHasChanged() { $this->expectException(BadCredentialsException::class); - $user = $this->createMock(UserInterface::class); + $user = $this->createMock(TestUser::class); $user->expects($this->once()) ->method('getPassword') ->willReturn('foo') @@ -199,7 +200,7 @@ public function testCheckAuthenticationDoesNotReauthenticateWhenPasswordHasChang ->method('getUser') ->willReturn($user); - $dbUser = $this->createMock(UserInterface::class); + $dbUser = $this->createMock(TestUser::class); $dbUser->expects($this->once()) ->method('getPassword') ->willReturn('newFoo') @@ -213,7 +214,7 @@ public function testCheckAuthenticationDoesNotReauthenticateWhenPasswordHasChang public function testCheckAuthenticationWhenTokenNeedsReauthenticationWorksWithoutOriginalCredentials() { - $user = $this->createMock(UserInterface::class); + $user = $this->createMock(TestUser::class); $user->expects($this->once()) ->method('getPassword') ->willReturn('foo') @@ -224,7 +225,7 @@ public function testCheckAuthenticationWhenTokenNeedsReauthenticationWorksWithou ->method('getUser') ->willReturn($user); - $dbUser = $this->createMock(UserInterface::class); + $dbUser = $this->createMock(TestUser::class); $dbUser->expects($this->once()) ->method('getPassword') ->willReturn('foo') @@ -338,7 +339,7 @@ protected function getProvider($user = null, $userChecker = null, $passwordHashe } } -class TestUser implements UserInterface +class TestUser implements PasswordAuthenticatedUserInterface, UserInterface { public function getRoles(): array { @@ -350,11 +351,6 @@ public function getPassword(): ?string return 'secret'; } - public function getSalt(): ?string - { - return null; - } - public function getUsername(): string { return 'jane_doe'; @@ -371,7 +367,7 @@ public function eraseCredentials() } interface PasswordUpgraderProvider extends UserProviderInterface, PasswordUpgraderInterface { - public function upgradePassword($user, string $newHashedPassword): void; + public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void; public function loadUserByIdentifier(string $identifier): UserInterface; } diff --git a/src/Symfony/Component/Security/Core/User/ChainUserProvider.php b/src/Symfony/Component/Security/Core/User/ChainUserProvider.php index 38e92f2eaf33a..407d4b7b928c4 100644 --- a/src/Symfony/Component/Security/Core/User/ChainUserProvider.php +++ b/src/Symfony/Component/Security/Core/User/ChainUserProvider.php @@ -128,16 +128,8 @@ public function supportsClass(string $class) /** * {@inheritdoc} */ - public function upgradePassword($user, string $newHashedPassword): void + public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): 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)); - - if (!$user instanceof UserInterface) { - throw new \TypeError(sprintf('The "%s::upgradePassword()" method expects an instance of "%s" as first argument, "%s" given.', 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/PasswordUpgraderInterface.php b/src/Symfony/Component/Security/Core/User/PasswordUpgraderInterface.php index 16195fad56f8c..fd21f14a81285 100644 --- a/src/Symfony/Component/Security/Core/User/PasswordUpgraderInterface.php +++ b/src/Symfony/Component/Security/Core/User/PasswordUpgraderInterface.php @@ -13,12 +13,15 @@ /** * @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(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void; } diff --git a/src/Symfony/Component/Security/Core/User/UserInterface.php b/src/Symfony/Component/Security/Core/User/UserInterface.php index f30d36e17d4d6..39f517717d555 100644 --- a/src/Symfony/Component/Security/Core/User/UserInterface.php +++ b/src/Symfony/Component/Security/Core/User/UserInterface.php @@ -48,29 +48,6 @@ interface UserInterface */ public function getRoles(); - /** - * Returns the password used to authenticate the user. - * - * 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(); - - /** - * Returns the salt that was originally used to hash the password. - * - * 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(); - /** * Removes sensitive data from the user. * diff --git a/src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php b/src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php index 2e0c49f92e1ed..42b13921f357f 100644 --- a/src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php +++ b/src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php @@ -15,7 +15,6 @@ use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; -use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\ConstraintValidator; use Symfony\Component\Validator\Exception\ConstraintDefinitionException; @@ -53,22 +52,13 @@ public function validate(mixed $password, Constraint $constraint) $user = $this->tokenStorage->getToken()->getUser(); - if (!$user instanceof UserInterface) { - 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 without implementing the "%s" interface is deprecated, the "%s" class should implement it.', UserPassword::class, 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)); + throw new ConstraintDefinitionException(sprintf('The "%s" class must implement the "%s" interface.', PasswordAuthenticatedUserInterface::class, get_debug_type($user))); } $hasher = $this->hasherFactory->getPasswordHasher($user); - if (null === $user->getPassword() || !$hasher->verify($user->getPassword(), $password, $user->getSalt())) { + if (null === $user->getPassword() || !$hasher->verify($user->getPassword(), $password, $user instanceof LegacyPasswordAuthenticatedUserInterface ? $user->getSalt() : null)) { $this->context->addViolation($constraint->message); } } diff --git a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php index e5fea5b509877..a00fe0ab831a6 100644 --- a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php @@ -133,7 +133,8 @@ private function authenticateViaGuard(AuthenticatorInterface $guardAuthenticator throw new BadCredentialsException(sprintf('Authentication failed because "%s::checkCredentials()" did not return true.', get_debug_type($guardAuthenticator))); } - if ($this->userProvider instanceof PasswordUpgraderInterface && $guardAuthenticator instanceof PasswordAuthenticatedInterface && null !== $this->passwordHasher && (null !== $password = $guardAuthenticator->getPassword($token->getCredentials())) && $this->passwordHasher->needsRehash($user)) { + + if ($user instanceof PasswordAuthenticatedUserInterface && $this->userProvider instanceof PasswordUpgraderInterface && $guardAuthenticator instanceof PasswordAuthenticatedInterface && null !== $this->passwordHasher && (null !== $password = $guardAuthenticator->getPassword($token->getCredentials())) && $this->passwordHasher->needsRehash($user)) { $this->userProvider->upgradePassword($user, $this->passwordHasher->hashPassword($user, $password)); } $this->userChecker->checkPostAuth($user); diff --git a/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php b/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php index 21406f058c1cf..f968bfd552a70 100644 --- a/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php +++ b/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php @@ -47,7 +47,7 @@ public function checkPassport(CheckPassportEvent $event): void $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)); + throw new \LogicException(sprintf('Class "%s" must implement "%s" for using password-based authentication.', get_debug_type($user), PasswordAuthenticatedUserInterface::class)); } /** @var PasswordCredentials $badge */ @@ -66,12 +66,7 @@ public function checkPassport(CheckPassportEvent $event): void 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)); - } - - if (!$this->hasherFactory->getPasswordHasher($user)->verify($user->getPassword(), $presentedPassword, $salt)) { + if (!$this->hasherFactory->getPasswordHasher($user)->verify($user->getPassword(), $presentedPassword, $user instanceof LegacyPasswordAuthenticatedUserInterface ? $user->getSalt() : null)) { throw new BadCredentialsException('The presented password is invalid.'); } diff --git a/src/Symfony/Component/Security/Http/EventListener/PasswordMigratingListener.php b/src/Symfony/Component/Security/Http/EventListener/PasswordMigratingListener.php index 0b15284f62d62..a68d1904a8f34 100644 --- a/src/Symfony/Component/Security/Http/EventListener/PasswordMigratingListener.php +++ b/src/Symfony/Component/Security/Http/EventListener/PasswordMigratingListener.php @@ -13,7 +13,8 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; -use Symfony\Component\PasswordHasher\PasswordHasherInterface; +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\Http\Authenticator\Passport\Badge\PasswordUpgradeBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; @@ -50,7 +51,7 @@ public function onLoginSuccess(LoginSuccessEvent $event): void } $user = $passport->getUser(); - if (null === $user->getPassword()) { + if (!$user instanceof PasswordAuthenticatedUserInterface || null === $user->getPassword()) { return; } @@ -76,7 +77,12 @@ public function onLoginSuccess(LoginSuccessEvent $event): void } } - $passwordUpgrader->upgradePassword($user, $passwordHasher instanceof PasswordHasherInterface ? $passwordHasher->hash($plaintextPassword, $user->getSalt()) : $passwordHasher->encodePassword($plaintextPassword, $user->getSalt())); + $salt = null; + if ($user instanceof LegacyPasswordAuthenticatedUserInterface) { + $salt = $user->getSalt(); + } + + $passwordUpgrader->upgradePassword($user, $passwordHasher->hash($plaintextPassword, $salt)); } public static function getSubscribedEvents(): array diff --git a/src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php b/src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php index a6e79f42e56d7..881efb2b56c07 100644 --- a/src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php +++ b/src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.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\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; /** @@ -61,6 +62,10 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request) throw new \RuntimeException(sprintf('The UserProviderInterface implementation must return an instance of UserInterface, but returned "%s".', get_debug_type($user))); } + if (!$user instanceof PasswordAuthenticatedUserInterface) { + throw new \RuntimeException(sprintf('Class "%s" must implement "%s" for using "%s".', get_debug_type($user), PasswordAuthenticatedUserInterface::class, __CLASS__)); + } + if (true !== hash_equals($this->generateCookieHash($class, $userIdentifier, $expires, $user->getPassword()), $hash)) { throw new AuthenticationException('The cookie\'s hash is invalid.'); } diff --git a/src/Symfony/Component/Security/Http/Tests/Authenticator/Fixtures/PasswordUpgraderProvider.php b/src/Symfony/Component/Security/Http/Tests/Authenticator/Fixtures/PasswordUpgraderProvider.php index 768a6ab78d0ac..cd96b9cf5dbf1 100644 --- a/src/Symfony/Component/Security/Http/Tests/Authenticator/Fixtures/PasswordUpgraderProvider.php +++ b/src/Symfony/Component/Security/Http/Tests/Authenticator/Fixtures/PasswordUpgraderProvider.php @@ -12,13 +12,14 @@ namespace Symfony\Component\Security\Http\Tests\Authenticator\Fixtures; use Symfony\Component\Security\Core\User\InMemoryUserProvider; +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; class PasswordUpgraderProvider extends InMemoryUserProvider implements PasswordUpgraderInterface { - public function upgradePassword($user, string $newHashedPassword): void + public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void { } }