Skip to content

Commit d2b36b6

Browse files
chalasrchalasr
committed
[Security] Remove getPassword() and getSalt() from UserInterface
1 parent 06c9d62 commit d2b36b6

File tree

20 files changed

+65
-101
lines changed

20 files changed

+65
-101
lines changed

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,20 +133,16 @@ public function supportsClass(string $class)
133133
*
134134
* @final
135135
*/
136-
public function upgradePassword(UserInterface $user, string $newEncodedPassword): 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-
142138
$class = $this->getClass();
143139
if (!$user instanceof $class) {
144140
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user)));
145141
}
146142

147143
$repository = $this->getRepository();
148144
if ($repository instanceof PasswordUpgraderInterface) {
149-
$repository->upgradePassword($user, $newEncodedPassword);
145+
$repository->upgradePassword($user, $newHashedPassword);
150146
}
151147
}
152148

‎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: 4 additions & 3 deletions
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(UserInterface $user, string $newEncodedPassword): 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)));
@@ -143,9 +144,9 @@ public function upgradePassword(UserInterface $user, string $newEncodedPassword)
143144
}
144145

145146
try {
146-
$user->getEntry()->setAttribute($this->passwordAttribute, [$newEncodedPassword]);
147+
$user->getEntry()->setAttribute($this->passwordAttribute, [$newHashedPassword]);
147148
$this->ldap->getEntryManager()->update($user->getEntry());
148-
$user->setPassword($newEncodedPassword);
149+
$user->setPassword($newHashedPassword);
149150
} catch (ExceptionInterface $e) {
150151
// ignore failed password upgrades
151152
}

‎src/Symfony/Component/Ldap/composer.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@
2121
"symfony/options-resolver": "^5.4|^6.0"
2222
},
2323
"require-dev": {
24-
"symfony/security-core": "^5.4|^6.0"
24+
"symfony/security-core": "^6.0"
2525
},
2626
"conflict": {
2727
"symfony/options-resolver": "<5.4",
28-
"symfony/security-core": "<5.4"
28+
"symfony/security-core": "<6.0"
2929
},
3030
"autoload": {
3131
"psr-4": { "Symfony\\Component\\Ldap\\": "" },

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

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,13 @@ public function __construct(UserProviderInterface $userProvider, UserCheckerInte
5151
*/
5252
protected function checkAuthentication(UserInterface $user, UsernamePasswordToken $token)
5353
{
54+
if (!$user instanceof PasswordAuthenticatedUserInterface) {
55+
throw new \LogicException(sprintf('Class "%s" must implement "%s" for using password-based authentication.', get_debug_type($user), PasswordAuthenticatedUserInterface::class));
56+
}
57+
5458
$currentUser = $token->getUser();
55-
if ($currentUser instanceof UserInterface) {
59+
60+
if ($currentUser instanceof PasswordAuthenticatedUserInterface) {
5661
if ($currentUser->getPassword() !== $user->getPassword()) {
5762
throw new BadCredentialsException('The credentials were changed from another session.');
5863
}
@@ -65,15 +70,7 @@ protected function checkAuthentication(UserInterface $user, UsernamePasswordToke
6570
throw new BadCredentialsException('The presented password is invalid.');
6671
}
6772

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-
73+
$salt = $user instanceof LegacyPasswordAuthenticatedUserInterface ? $user->getSalt() : null;
7774
$hasher = $this->hasherFactory->getPasswordHasher($user);
7875

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

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

Lines changed: 10 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,16 @@ 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 || $user instanceof LegacyPasswordAuthenticatedUserInterface) {
283+
if (!$this->user instanceof LegacyPasswordAuthenticatedUserInterface || !$user instanceof LegacyPasswordAuthenticatedUserInterface || $this->user->getSalt() !== $user->getSalt()) {
284+
return true;
285+
}
286+
}
284287
}
285288

286289
$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 & 6 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
{
@@ -371,7 +372,7 @@ public function eraseCredentials()
371372
}
372373
interface PasswordUpgraderProvider extends UserProviderInterface, PasswordUpgraderInterface
373374
{
374-
public function upgradePassword(UserInterface $user, string $newHashedPassword): void;
375+
public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void;
375376

376377
public function loadUserByIdentifier(string $identifier): UserInterface;
377378
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,12 @@ public function supportsClass(string $class)
128128
/**
129129
* {@inheritdoc}
130130
*/
131-
public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newEncodedPassword): void
131+
public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void
132132
{
133133
foreach ($this->providers as $provider) {
134134
if ($provider instanceof PasswordUpgraderInterface) {
135135
try {
136-
$provider->upgradePassword($user, $newEncodedPassword);
136+
$provider->upgradePassword($user, $newHashedPassword);
137137
} catch (UnsupportedUserException $e) {
138138
// ignore: password upgrades are opportunistic
139139
}

0 commit comments

Comments
 (0)