8000 [Security] Remove getPassword() and getSalt() from UserInterface · symfony/symfony@30e2c00 · GitHub
[go: up one dir, main page]
Skip to content

Commit 30e2c00

Browse files
chalasrchalasr
committed
[Security] Remove getPassword() and getSalt() from UserInterface
1 parent ed78403 commit 30e2c00

File tree

17 files changed

+66
-106
lines changed

17 files changed

+66
-106
lines changed

‎src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,16 +133,8 @@ public function supportsClass(string $class)
133133
*
134134
* @final
135135
*/
136-
public function upgradePassword($user, string $newHashedPassword): void
136+
public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void
137137
{
138-
if (!$user instanceof PasswordAuthenticatedUserInterface) {
139-
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));
140-
141-
if (!$user instanceof UserInterface) {
142-
throw new \TypeError(sprintf('The "%s::upgradePassword()" method expects an instance of "%s" as first argument, "%s" given.', PasswordAuthenticatedUserInterface::class, get_debug_type($user)));
143-
}
144-
}
145-
146138
$class = $this->getClass();
147139
if (!$user instanceof $class) {
148140
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user)));

‎src/Symfony/Bridge/Doctrine/Tests/Fixtures/User.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,6 @@ public function getPassword(): ?string
4444
{
4545
}
4646

47-
public function getSalt(): ?string
48-
{
49-
}
50-
5147
public function getUsername(): string
5248
{
5349
return $this->name;

‎src/Symfony/Bridge/Doctrine/composer.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
"symfony/property-access": "^5.4|^6.0",
3838
"symfony/property-info": "^5.4|^6.0",
3939
"symfony/proxy-manager-bridge": "^5.4|^6.0",
40-
"symfony/security-core": "^5.4|^6.0",
40+
"symfony/security-core": "^6.0",
4141
"symfony/expression-language": "^5.4|^6.0",
4242
"symfony/uid": "^5.4|^6.0",
4343
"symfony/validator": "^5.4|^6.0",
@@ -58,7 +58,7 @@
5858
"symfony/messenger": "<5.4",
5959
"symfony/property-info": "<5.4",
6060
"symfony/security-bundle": "<5.4",
61-
"symfony/security-core": "<5.4",
61+
"symfony/security-core": "<6.0",
6262
"symfony/validator": "<5.4"
6363
},
6464
"suggest": {

‎src/Symfony/Component/Ldap/Security/LdapUserProvider.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
1919
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
2020
use Symfony\Component\Security\Core\Exception\UserNotFoundException;
21+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
2122
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
2223
use Symfony\Component\Security\Core\User\UserInterface;
2324
use Symfony\Component\Security\Core\User\UserProviderInterface;
@@ -132,7 +133,7 @@ public function refreshUser(UserInterface $user)
132133
*
133134
* @final
134135
*/
135-
public function upgradePassword($user, string $newHashedPassword): void
136+
public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void
136137
{
137138
if (!$user instanceof LdapUser) {
138139
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user)));

‎src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
1616
use Symfony\Component\Security\Core\Exception\AuthenticationServiceException;
1717
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
18+
use < F438 span class=pl-v>Symfony\Component\Security\Core\Exception\InvalidArgumentException;
1819
use Symfony\Component\Security\Core\Exception\UserNotFoundException;
1920
use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface;
2021
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
@@ -51,8 +52,13 @@ public function __construct(UserProviderInterface $userProvider, UserCheckerInte
5152
*/
5253
protected function checkAuthentication(UserInterface $user, UsernamePasswordToken $token)
5354
{
55+
if (!$user instanceof PasswordAuthenticatedUserInterface) {
56+
throw new InvalidArgumentException(sprintf('Class "%s" must implement "%s" for using password-based authentication.', get_debug_type($user), PasswordAuthenticatedUserInterface::class));
57+
}
58+
5459
$currentUser = $token->getUser();
55-
if ($currentUser instanceof UserInterface) {
60+
61+
if ($currentUser instanceof PasswordAuthenticatedUserInterface) {
5662
if ($currentUser->getPassword() !== $user->getPassword()) {
5763
throw new BadCredentialsException('The credentials were changed from another session.');
5864
}
@@ -65,15 +71,7 @@ protected function checkAuthentication(UserInterface $user, UsernamePasswordToke
6571
throw new BadCredentialsException('The presented password is invalid.');
6672
}
6773

68-
if (!$user instanceof PasswordAuthenticatedUserInterface) {
69-
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));
70-
}
71-
72-
$salt = $user->getSalt();
73-
if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) {
74-
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));
75-
}
76-
74+
$salt = $user instanceof LegacyPasswordAuthenticatedUserInterface ? $user->getSalt() : null;
7775
$hasher = $this->hasherFactory->getPasswordHasher($user);
7876

7977
if (!$hasher->verify($user->getPassword(), $presentedPassword, $salt)) {

‎src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Security\Core\Authentication\Token;
1313

1414
use Symfony\Component\Security\Core\User\EquatableInterface;
15+
use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface;
1516
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
1617
use Symfony\Component\Security\Core\User\UserInterface;
1718

@@ -273,14 +274,18 @@ private function hasUserChanged(UserInterface $user): bool
273274
return !(bool) $this->user->isEqualTo($user);
274275
}
275276

276-
// @deprecated since Symfony 5.3, check for PasswordAuthenticatedUserInterface on both user objects before comparing passwords
277-
if ($this->user->getPassword() !== $user->getPassword()) {
278-
return true;
279-
}
277+
if ($this->user instanceof PasswordAuthenticatedUserInterface || $user instanceof PasswordAuthenticatedUserInterface) {
278+
if (!$this->user instanceof PasswordAuthenticatedUserInterface || !$user instanceof PasswordAuthenticatedUserInterface || $this->user->getPassword() !== $user->getPassword()) {
279+
return true;
280+
}
280281

281-
// @deprecated since Symfony 5.3, check for LegacyPasswordAuthenticatedUserInterface on both user objects before comparing salts
282-
if ($this->user->getSalt() !== $user->getSalt()) {
283-
return true;
282+
if ($this->user instanceof LegacyPasswordAuthenticatedUserInterface xor $user instanceof LegacyPasswordAuthenticatedUserInterface) {
283+
return true;
284+
}
285+
286+
if ($this->user instanceof LegacyPasswordAuthenticatedUserInterface && $user instanceof LegacyPasswordAuthenticatedUserInterface && $this->user->getSalt() !== $user->getSalt()) {
287+
return true;
288+
}
284289
}
285290

286291
$userRoles = array_map('strval', (array) $user->getRoles());

‎src/Symfony/Component/Security/Core/CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ CHANGELOG
55
---
66

77
* `TokenInterface` does not extend `Serializable` anymore
8-
* Remove all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead
8+
* Remove all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead
9+
* Remove methods `getPassword()` and `getSalt()` from `UserInterface`, use `PasswordAuthenticatedUserInterface`
10+
or `LegacyPasswordAuthenticatedUserInterface` instead
911

1012
5.3
1113
---

‎src/Symfony/Component/Security/Core/Tests/Authentication/Provider/DaoAuthenticationProviderTest.php

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use Symfony\Component\Security\Core\Exception\UserNotFoundException;
2424
use Symfony\Component\Security\Core\User\InMemoryUser;
2525
use Symfony\Component\Security\Core\User\InMemoryUserProvider;
26+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
2627
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
2728
use Symfony\Component\Security\Core\User\UserCheckerInterface;
2829
use Symfony\Component\Security\Core\User\UserInterface;
@@ -188,7 +189,7 @@ public function testCheckAuthenticationWhenCredentialsAreNotValid()
188189
public function testCheckAuthenticationDoesNotReauthenticateWhenPasswordHasChanged()
189190
{
190191
$this->expectException(BadCredentialsException::class);
191-
$user = $this->createMock(UserInterface::class);
192+
$user = $this->createMock(TestUser::class);
192193
$user->expects($this->once())
193194
->method('getPassword')
194195
->willReturn('foo')
@@ -199,7 +200,7 @@ public function testCheckAuthenticationDoesNotReauthenticateWhenPasswordHasChang
199200
->method('getUser')
200201
->willReturn($user);
201202

202-
$dbUser = $this->createMock(UserInterface::class);
203+
$dbUser = $this->createMock(TestUser::class);
203204
$dbUser->expects($this->once())
204205
->method('getPassword')
205206
->willReturn('newFoo')
@@ -213,7 +214,7 @@ public function testCheckAuthenticationDoesNotReauthenticateWhenPasswordHasChang
213214

214215
public function testCheckAuthenticationWhenTokenNeedsReauthenticationWorksWithoutOriginalCredentials()
215216
{
216-
$user = $this->createMock(UserInterface::class);
217+
$user = $this->createMock(TestUser::class);
217218
$user->expects($this->once())
218219
->method('getPassword')
219220
->willReturn('foo')
@@ -224,7 +225,7 @@ public function testCheckAuthenticationWhenTokenNeedsReauthenticationWorksWithou
224225
->method('getUser')
225226
->willReturn($user);
226227

227-
$dbUser = $this->createMock(UserInterface::class);
228+
$dbUser = $this->createMock(TestUser::class);
228229
$dbUser->expects($this->once())
229230
->method('getPassword')
230231
->willReturn('foo')
@@ -338,7 +339,7 @@ protected function getProvider($user = null, $userChecker = null, $passwordHashe
338339
}
339340
}
340341

341-
class TestUser implements UserInterface
342+
class TestUser implements PasswordAuthenticatedUserInterface, UserInterface
342343
{
343344
public function getRoles(): array
344345
{
@@ -350,11 +351,6 @@ public function getPassword(): ?string
350351
return 'secret';
351352
}
352353

353-
public function getSalt(): ?string
354-
{
355-
return null;
356-
}
357-
358354
public function getUsername(): string
359355
{
360356
return 'jane_doe';
@@ -371,7 +367,7 @@ public function eraseCredentials()
371367
}
372368
interface PasswordUpgraderProvider extends UserProviderInterface, PasswordUpgraderInterface
373369
{
374-
public function upgradePassword($user, string $newHashedPassword): void;
370+
public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void;
375371

376372
public function loadUserByIdentifier(string $identifier): UserInterface;
377373
}

‎src/Symfony/Component/Security/Core/User/ChainUserProvider.php

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,16 +128,8 @@ public function supportsClass(string $class)
128128
/**
129129
* {@inheritdoc}
130130
*/
131-
public function upgradePassword($user, string $newHashedPassword): void
131+
public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void
132132
{
133-
if (!$user instanceof PasswordAuthenticatedUserInterface) {
134-
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));
135-
136-
if (!$user instanceof UserInterface) {
137-
throw new \TypeError(sprintf('The "%s::upgradePassword()" method expects an instance of "%s" as first argument, "%s" given.', PasswordAuthenticatedUserInterface::class, get_debug_type($user)));
138-
}
139-
}
140-
141133
foreach ($this->providers as $provider) {
142134
if ($provider instanceof PasswordUpgraderInterface) {
143135
try {

‎src/Symfony/Component/Security/Core/User/PasswordUpgraderInterface.php

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@
1313

1414
/**
1515
* @author Nicolas Grekas <p@tchwork.com>
16-
*
17-
* @method void upgradePassword(PasswordAuthenticatedUserInterface|UserInterface $user, string $newHashedPassword) Upgrades the hashed password of a user, typically for using a better hash algorithm.
18-
* This method should persist the new password in the user storage and update the $user object accordingly.
19-
* Because you don't want your users not being able to log in, this method should be opportunistic:
20-
* it's fine if it does nothing or if it fails without throwing any exception.
2116
*/
2217
interface PasswordUpgraderInterface
2318
{
19+
/**
20+
* Upgrades the hashed password of a user, typically for using a better hash algorithm.
21+
*
22+
* This method should persist the new password in the user storage and update the $user object accordingly.
23+
* Because you don't want your users not being able to log in, this method should be opportunistic:
24+
* it's fine if it does nothing or if it fails without throwing any exception.
25+
*/
26+
public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void;
2427
}

‎src/Symfony/Component/Security/Core/User/UserInterface.php

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -48,29 +48,6 @@ interface UserInterface
4848
*/
4949
public function getRoles();
5050

51-
/**
52-
* Returns the password used to authenticate the user.
53-
*
54-
* This should be the hashed password. On authentication, a plain-text
55-
* password will be hashed, and then compared to this value.
56-
*
57-
* This method is deprecated since Symfony 5.3, implement it from {@link PasswordAuthenticatedUserInterface} instead.
58-
*
59-
* @return string|null The hashed password if any
60-
*/
61-
public function getPassword();
62-
63-
/**
64-
* Returns the salt that was originally used to hash the password.
65-
*
66-
* This can return null if the password was not hashed using a salt.
67-
*
68-
* This method is deprecated since Symfony 5.3, implement it from {@link LegacyPasswordAuthenticatedUserInterface} instead.
69-
*
70-
* @return string|null The salt
71-
*/
72-
public function getSalt();
73-
7451
/**
7552
* Removes sensitive data from the user.
7653
*

‎src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
1616
use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface;
1717
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
18-
use Symfony\Component\Security\Core\User\UserInterface;
1918
use Symfony\Component\Validator\Constraint;
2019
use Symfony\Component\Validator\ConstraintValidator;
2120
use Symfony\Component\Validator\Exception\ConstraintDefinitionException;
@@ -53,22 +52,13 @@ public function validate(mixed $password, Constraint $constraint)
5352

5453
$user = $this->tokenStorage->getToken()->getUser();
5554

56-
if (!$user instanceof UserInterface) {
57-
throw new ConstraintDefinitionException('The User object must implement the UserInterface interface.');
58-
}
59-
6055
if (!$user instanceof PasswordAuthenticatedUserInterface) {
61-
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));
62-
}
63-
64-
$salt = $user->getSalt();
65-
if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) {
66-
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));
56+
throw new ConstraintDefinitionException(sprintf('The "%s" class must implement the "%s" interface.', PasswordAuthenticatedUserInterface::class, get_debug_type($user)));
6757
}
6858

6959
$hasher = $this->hasherFactory->getPasswordHasher($user);
7060

71-
if (null === $user->getPassword() || !$hasher->verify($user->getPassword(), $password, $user->getSalt())) {
61+
if (null === $user->getPassword() || !$hasher->verify($user->getPassword(), $password, $user instanceof LegacyPasswordAuthenticatedUserInterface ? $user->getSalt() : null)) {
7262
$this->context->addViolation($constraint->message);
7363
}
7464
}

‎src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ private function authenticateViaGuard(AuthenticatorInterface $guardAuthenticator
133133

134134
throw new BadCredentialsException(sprintf('Authentication failed because "%s::checkCredentials()" did not return true.', get_debug_type($guardAuthenticator)));
135135
}
136-
if ($this->userProvider instanceof PasswordUpgraderInterface && $guardAuthenticator instanceof PasswordAuthenticatedInterface && null !== $this->passwordHasher && (null !== $password = $guardAuthenticator->getPassword($token->getCredentials())) && $this->passwordHasher->needsRehash($user)) {
136+
137+
if ($user instanceof PasswordAuthenticatedUserInterface && $this->userProvider instanceof PasswordUpgraderInterface && $guardAuthenticator instanceof PasswordAuthenticatedInterface && null !== $this->passwordHasher && (null !== $password = $guardAuthenticator->getPassword($token->getCredentials())) && $this->passwordHasher->needsRehash($user)) {
137138
$this->userProvider->upgradePassword($user, $this->passwordHasher->hashPassword($user, $password));
138139
}
139140
$this->userChecker->checkPostAuth($user);

‎src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public function checkPassport(CheckPassportEvent $event): void
4747
$user = $passport->getUser();
4848

4949
if (!$user instanceof PasswordAuthenticatedUserInterface) {
50-
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));
50+
throw new \LogicException(sprintf('Class "%s" must implement "%s" for using password-based authentication.', get_debug_type($user), PasswordAuthenticatedUserInterface::class));
5151
}
5252

5353
/** @var PasswordCredentials $badge */
@@ -66,12 +66,7 @@ public function checkPassport(CheckPassportEvent $event): void
6666
throw new BadCredentialsException('The presented password is invalid.');
6767
}
6868

69-
$salt = $user->getSalt();
70-
if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) {
71-
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));
72-
}
73-
74-
if (!$this->hasherFactory->getPasswordHasher($user)->verify($user->getPassword(), $presentedPassword, $salt)) {
69+
if (!$this->hasherFactory->getPasswordHasher($user)->verify($user->getPassword(), $presentedPassword, $user instanceof LegacyPasswordAuthenticatedUserInterface ? $user->getSalt() : null)) {
7570
throw new BadCredentialsException('The presented password is invalid.');
7671
}
7772

0 commit comments

Comments
 (0)
0