Skip to content

Commit 9aa1adc

Browse files
nicolas-grekasnicolas-grekas
committed
feature #41982 [Security] Remove getPassword() and getSalt() from UserInterface (chalasr)
This PR was merged into the 6.0 branch. Discussion ---------- [Security] Remove getPassword() and getSalt() from UserInterface | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - with BC layers from #40267 Commits ------- 30e2c00 [Security] Remove getPassword() and getSalt() from UserInterface
2 parents 617bb4c + 30e2c00 commit 9aa1adc

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

0 commit comments

Comments
 (0)