8000 [Security] Decouple passwords from UserInterface · symfony/symfony@5066319 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5066319

Browse files
chalasrchalasr
committed
[Security] Decouple passwords from UserInterface
1 parent fdabaf2 commit 5066319

33 files changed

+467
-67
lines changed

‎UPGRADE-5.3.md

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,88 @@ Routing
8080
Security
8181
--------
8282

83+
* Deprecate `UserInterface::getPassword()`
84+
If your `getPassword()` method does not return `null` (i.e. you are using password-based authentication),
85+
you should implement `PasswordAuthenticatedUserInterface`.
86+
87+
Before:
88+
```php
89+
use Symfony\Component\Security\Core\User\UserInterface;
90+
91+
class User implements UserInterface
92+
{
93+
// ...
94+
95+
public function getPassword()
96+
{
97+
return $this->password;
98+
}
99+
}
100+
```
101+
102+
After:
103+
```php
104+
use Symfony\Component\Security\Core\User\UserInterface;
105+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
106+
107+
class User implements UserInterface, PasswordAuthenticatedUserInterface
108+
{
109+
// ...
110+
111+
public function getPassword(): ?string
112+
{
113+
return $this->password;
114+
}
115+
}
116+
```
117+
118+
* Deprecate `UserInterface::getSalt()`
119+
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),
120+
implement `LegacyPasswordAuthenticatedUserInterface`.
121+
122+
Before:
123+
```php
124+
use Symfony\Component\Security\Core\User\UserInterface;
125+
126+
class User implements UserInterface
127+
{
128+
// ...
129+
130+
public function getPassword()
131+
{
132+
return $this->password;
133+
}
134+
135+
public function getSalt()
136+
{
137+
return $this->salt;
138+
}
139+
}
140+
```
141+
142+
After:
143+
```php
144+
use Symfony\Component\Security\Core\User\UserInterface;
145+
use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface;
146+
147+
class User implements UserInterface, LegacyPasswordAuthenticatedUserInterface
148+
{
149+
// ...
150+
151+
public function getPassword(): ?string
152+
{
153+
return $this->password;
154+
}
155+
156+
public function getSalt(): ?string
157+
{
158+
return $this->salt;
159+
}
160+
}
161+
```
162+
163+
* Deprecate calling `PasswordUpgraderInterface::upgradePassword()` with a `UserInterface` instance that does not implement `PasswordAuthenticatedUserInterface`
164+
* Deprecate calling methods `hashPassword()`, `isPasswordValid()` and `needsRehash()` on `UserPasswordHasherInterface` with a `UserInterface` instance that does not implement `PasswordAuthenticatedUserInterface`
83165
* Deprecate all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead
84166
* Deprecated voters that do not return a valid decision when calling the `vote` method
85167

‎UPGRADE-6.0.md

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,90 @@ Routing
172172
Security
173173
--------
174174

175+
* Remove `UserInterface::getPassword()`
176+
If your `getPassword()` method does not return `null` (i.e. you are using password-based authentication),
177+
you should implement `PasswordAuthenticatedUserInterface`.
178+
179+
Before:
180+
```php
181+
use Symfony\Component\Security\Core\User\UserInterface;
182+
183+
class User implements UserInterface
184+
{
185+
// ...
186+
187+
public function getPassword()
188+
{
189+
return $this->password;
190+
}
191+
}
192+
```
193+
194+
After:
195+
```php
196+
use Symfony\Component\Security\Core\User\UserInterface;
197+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
198+
199+
class User implements UserInterface, PasswordAuthenticatedUserInterface
200+
{
201+
// ...
202+
203+
public function getPassword(): ?string
204+
{
205+
return $this->password;
206+
}
207+
}
208+
```
209+
210+
* Remove `UserInterface::getSalt()`
211+
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),
212+
implement `LegacyPasswordAuthenticatedUserInterface`.
213+
214+
Before:
215+
```php
216+
use Symfony\Component\Security\Core\User\UserInterface;
217+
218+
class User implements UserInterface
219+
{
220+
// ...
221+
222+
public function getPassword()
223+
{
224+
return $this->password;
225+
}
226+
227+
public function getSalt()
228+
{
229+
return $this->salt;
230+
}
231+
}
232+
```
233+
234+
After:
235+
```php
236+
use Symfony\Component\Security\Core\User\UserInterface;
237+
use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface;
238+
239+
class User implements UserInterface, LegacyPasswordAuthenticatedUserInterface
240+
{
241+
// ...
242+
243+
public function getPassword(): ?string
244+
{
245+
return $this->password;
246+
}
247+
248+
public function getSalt(): ?string
249+
{
250+
return $this->salt;
251+
}
252+
}
253+
```
254+
255+
* Calling `PasswordUpgraderInterface::upgradePassword()` with a `UserInterface` instance that
256+
does not implement `PasswordAuthenticatedUserInterface` now throws a `\TypeError`.
257+
* Calling methods `hashPassword()`, `isPasswordValid()` and `needsRehash()` on `UserPasswordHasherInterface`
258+
with a `UserInterface` instance that does not implement `PasswordAuthenticatedUserInterface` now throws a `\TypeError`
175259
* Drop all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead
176260
* Drop support for `SessionInterface $session` as constructor argument of `SessionTokenStorage`, inject a `\Symfony\Component\HttpFoundation\RequestStack $requestStack` instead
177261
* Drop support for `session` provided by the ServiceLocator injected in `UsageTrackingTokenStorage`, provide a `request_stack` service instead

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Doctrine\Persistence\ObjectRepository;
1818
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
1919
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
20+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
2021
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
2122
use Symfony\Component\Security\Core\User\UserInterface;
2223
use Symfony\Component\Security\Core\User\UserProviderInterface;
@@ -115,9 +116,15 @@ public function supportsClass(string $class)
115116

116117
/**
117118
* {@inheritdoc}
119+
*
120+
* @final
118121
*/
119122
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
120123
{
124+
if (!$user instanceof PasswordAuthenticatedUserInterface) {
125+
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));
126+
}
127+
121128
$class = $this->getClass();
122129
if (!$user instanceof $class) {
123130
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user)));

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,11 @@
1414
use Doctrine\ORM\Mapping\Column;
1515
use Doctrine\ORM\Mapping\Entity;
1616
use Doctrine\ORM\Mapping\Id;
17+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
1718
use Symfony\Component\Security\Core\User\UserInterface;
1819

1920
/** @Entity */
20-
class User implements UserInterface
21+
class User implements UserInterface, PasswordAuthenticatedUserInterface
2122
{
2223
/** @Id @Column(type="integer") */
2324
protected $id1;

‎src/Symfony/Bridge/Doctrine/Tests/Security/User/EntityUserProviderTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use Symfony\Bridge\Doctrine\Tests\DoctrineTestHelper;
2323
use Symfony\Bridge\Doctrine\Tests\Fixtures\User;
2424
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
25+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
2526
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
2627
use Symfony\Component\Security\Core\User\UserInterface;
2728

@@ -234,4 +235,5 @@ abstract class UserLoaderRepository implements ObjectRepository, UserLoaderInter
234235

235236
abstract class PasswordUpgraderRepository implements ObjectRepository, PasswordUpgraderInterface
236237
{
238+
abstract public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void;
237239
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
"symfony/property-access": "^4.4|^5.0",
3939
"symfony/property-info": "^5.0",
4040
"symfony/proxy-manager-bridge": "^4.4|^5.0",
41-
"symfony/security-core": "^5.0",
41+
"symfony/security-core": "^5.3",
4242
"symfony/expression-language": "^4.4|^5.0",
4343
"symfony/uid": "^5.1",
4444
"symfony/validator": "^5.2",
@@ -60,7 +60,7 @@
6060
"symfony/messenger": "<4.4",
6161
"symfony/property-info": "<5",
6262
"symfony/security-bundle": "<5",
63-
"symfony/security-core": "<5",
63+
"symfony/security-core": "<5.3",
6464
"symfony/validator": "<5.2"
6565
},
6666
"suggest": {

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
4343
use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder;
4444
use Symfony\Component\Security\Core\User\ChainUserProvider;
45+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
4546
use Symfony\Component\Security\Core\User\UserProviderInterface;
4647
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
4748

@@ -664,6 +665,9 @@ private function createEncoders(array $encoders, ContainerBuilder $container)
664665
{
665666
$encoderMap = [];
666667
foreach ($encoders as $class => $encoder) {
668+
if (class_exists($class) && !is_a($class, PasswordAuthenticatedUserInterface::class, true)) {
669+
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);
670+
}
667671
$encoderMap[$class] = $this->createEncoder($encoder);
668672
}
669673

@@ -775,6 +779,10 @@ private function createHashers(array $hashers, ContainerBuilder $container)
775779
{
776780
$hasherMap = [];
777781
foreach ($hashers as $class => $hasher) {
782+
// @deprecated since Symfony 5.3, remove the check in 6.0
783+
if (class_exists($class) && !is_a($class, PasswordAuthenticatedUserInterface::class, true)) {
784+
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);
785+
}
778786
$hasherMap[$class] = $this->createHasher($hasher);
779787
}
780788

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User\ArrayUserProvider;
1515
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
16+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
1617
use Symfony\Component\Security\Core\User\User;
1718
use Symfony\Component\Security\Core\User\UserInterface;
1819

@@ -78,7 +79,7 @@ public function testUserWillBeMarkedAsChangedIfRolesHasChanged(UserInterface $us
7879
}
7980
}
8081

81-
final class UserWithoutEquatable implements UserInterface
82+
final class UserWithoutEquatable implements UserInterface, PasswordAuthenticatedUserInterface
8283
{
8384
private $username;
8485
private $password;
@@ -119,7 +120,7 @@ public function getRoles()
119120
/**
120121
* {@inheritdoc}
121122
*/
122-
public function getPassword()
123+
public function getPassword(): ?string
123124
{
124125
return $this->password;
125126
}

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

Lines changed: 8 additions & 2 deletions
10000
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\Ldap\LdapInterface;
1818
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
1919
use Symfony\Component\Security\Core\Exception\LogicException;
20+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
2021
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
2122
use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface;
2223
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
@@ -68,6 +69,11 @@ public function onCheckPassport(CheckPassportEvent $event)
6869
throw new BadCredentialsException('The presented password cannot be empty.');
6970
}
7071

72+
$user = $passport->getUser();
73+
if (!$user instanceof PasswordAuthenticatedUserInterface) {
74+
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));
75+
}
76+
7177
/** @var LdapInterface $ldap */
7278
$ldap = $this->ldapLocator->get($ldapBadge->getLdapServiceId());
7379
try {
@@ -77,7 +83,7 @@ public function onCheckPassport(CheckPassportEvent $event)
7783
} else {
7884
throw new LogicException('Using the "query_string" config without using a "search_dn" and a "search_password" is not supported.');
7985
}
80-
$username = $ldap->escape($passport->getUser()->getUsername(), '', LdapInterface::ESCAPE_FILTER);
86+
$username = $ldap->escape($user->getUsername(), '', LdapInterface::ESCAPE_FILTER);
8187
$query = str_replace('{username}', $username, $ldapBadge->getQueryString());
8288
$result = $ldap->query($ldapBadge->getDnString(), $query)->execute();
8389
if (1 !== $result->count()) {
@@ -86,7 +92,7 @@ public function onCheckPassport(CheckPassportEvent $event)
8692

8793
$dn = $result[0]->getDn();
8894
} else {
89-
$username = $ldap->escape($passport->getUser()->getUsername(), '', LdapInterface::ESCAPE_DN);
95+
$username = $ldap->escape($user->getUsername(), '', LdapInterface::ESCAPE_DN);
9096
$dn = str_replace('{username}', $username, $ldapBadge->getDnString());
9197
}
9298

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,15 @@
1313

1414
use Symfony\Component\Ldap\Entry;
1515
use Symfony\Component\Security\Core\User\EquatableInterface;
16+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
1617
use Symfony\Component\Security\Core\User\UserInterface;
1718

1819
/**
1920
* @author Robin Chalas <robin.chalas@gmail.com>
2021
*
2122
* @final
2223
*/
23-
class LdapUser implements UserInterface, EquatableInterface
24+
class LdapUser implements UserInterface, PasswordAuthenticatedUserInterface, EquatableInterface
2425
{
2526
private $entry;
2627
private $username;

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ public function refreshUser(UserInterface $user)
122122

123123
/**
124124
* {@inheritdoc}
125+
*
126+
* @final
125127
*/
126128
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
127129
{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@
2222
"ext-ldap": "*"
2323
},
2424
"require-dev": {
25-
"symfony/security-core": "^5.0"
25+
"symfony/security-core": "^5.3"
2626
},
2727
"conflict": {
2828
"symfony/options-resolver": "<4.4",
29-
"symfony/security-core": "<5"
29+
"symfony/security-core": "<5.3"
3030
},
3131
"autoload": {
3232
"psr-4": { "Symfony\\Component\\Ldap\\": "" },

‎src/Symfony/Component/PasswordHasher/Hasher/PasswordHasherFactoryInterface.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\PasswordHasher\Hasher;
1313

1414
use Symfony\Component\PasswordHasher\PasswordHasherInterface;
15+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
1516
use Symfony\Component\Security\Core\User\UserInterface;
1617

1718
/**
@@ -25,7 +26,7 @@ interface PasswordHasherFactoryInterface
2526
/**
2627
* Returns the password hasher to use for the given user.
2728
*
28-
* @param UserInterface|string $user A UserInterface instance or a class name
29+
* @param PasswordAuthenticatedUserInterface|UserInterface|string $user A PasswordAuthenticatedUserInterface/UserInterface instance or a class name
2930
*
3031
* @throws \RuntimeException When no password hasher could be found for the user
3132
*/

0 commit comments

Comments
 (0)
0