Skip to content

Commit f46b6eb

Browse files
committed
[Security] Decouple passwords from UserInterface
1 parent 4d91b8f commit f46b6eb

27 files changed

+427
-61
lines changed

UPGRADE-5.3.md

+82
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,88 @@ PropertyInfo
7171
Security
7272
--------
7373

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

UPGRADE-6.0.md

+84
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,90 @@ Routing
168168
Security
169169
--------
170170

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

+6-1
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;
@@ -116,8 +117,12 @@ public function supportsClass(string $class)
116117
/**
117118
* {@inheritdoc}
118119
*/
119-
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
120+
public function upgradePassword($user, string $newEncodedPassword): void
120121
{
122+
if (!$user instanceof PasswordAuthenticatedUserInterface) {
123+
trigger_deprecation('symfony/security-core', '5.3', 'The first argument of "%s:upgradePassword()" method should be an instance of "%s", you should make the "%s" class implement it.', PasswordUpgraderInterface::class, PasswordAuthenticatedUserInterface::class, \get_class($user));
124+
}
125+
121126
$class = $this->getClass();
122127
if (!$user instanceof $class) {
123128
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user)));

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

+2-1
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

+1
Original file line numberDiff line numberDiff line change
@@ -234,4 +234,5 @@ abstract class UserLoaderRepository implements ObjectRepository, UserLoaderInter
234234

235235
abstract class PasswordUpgraderRepository implements ObjectRepository, PasswordUpgraderInterface
236236
{
237+
abstract public function upgradePassword(UserInterface $user, string $newHashedPassword): void;
237238
}

src/Symfony/Bridge/Doctrine/composer.json

+2-2
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/Tests/Functional/SecurityTest.php

+3-2
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

+8-2
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_class($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

+2-1
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

+2-1
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\UsernameNotFoundException;
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;
@@ -123,7 +124,7 @@ public function refreshUser(UserInterface $user)
123124
/**
124125
* {@inheritdoc}
125126
*/
126-
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
127+
public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newEncodedPassword): void
127128
{
128129
if (!$user instanceof LdapUser) {
129130
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_debug_type($user)));

src/Symfony/Component/Ldap/composer.json

+2-2
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\\": "" },

0 commit comments

Comments
 (0)