8000 [Security] Deprecate `UserInterface` & `TokenInterface`'s `eraseCrede… · symfony/symfony@e556606 · GitHub
[go: up one dir, main page]
Skip to content

Commit e556606

Browse files
chalasrnicolas-grekas
chalasr
authored and
nicolas-grekas
committed
[Security] Deprecate UserInterface & TokenInterface's eraseCredentials() 
1 parent ecb9728 commit e556606

24 files changed

+213
-14
lines changed

‎UPGRADE-7.3.md

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,23 @@ backward compatibility breaks. Minor backward compatibility breaks are prefixed
66
`[BC BREAK]`, make sure your code is compatible with these entries before upgrading.
77
Read more about this in the [Symfony documentation](https://symfony.com/doc/7.3/setup/upgrade_minor.html).
88

9-
If you're upgrading from a version below 7.1, follow the [7.2 upgrade guide](UPGRADE-7.2.md) first.
9+
If you're upgrading from a version below 7.2, follow the [7.2 upgrade guide](UPGRADE-7.2.md) first.
10+
11+
Ldap
12+
----
13+
14+
* Deprecate `LdapUser::eraseCredentials()`, use `LdapUser::setPassword(null)` instead
15+
16+
Security
17+
--------
18+
19+
* Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`,
20+
use a dedicated DTO or erase credentials on your own e.g. upon `AuthenticationTokenCreatedEvent` instead
21+
22+
SecurityBundle
23+
--------------
24+
25+
* Deprecate the `erase_credentials` config option, erase credentials on your own e.g. upon `AuthenticationTokenCreatedEvent` instead
1026

1127
Console
1228
-------
@@ -115,3 +131,4 @@ VarDumper
115131

116132
* Deprecate `ResourceCaster::castCurl()`, `ResourceCaster::castGd()` and `ResourceCaster::castOpensslX509()`
117133
* Mark all casters as `@internal`
134+
* Deprecate the `CompiledClassMetadataFactory` and `CompiledClassMetadataCacheWarmer` classes

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ CHANGELOG
88
* Add encryption support to `OidcTokenHandler` (JWE)
99
* Add `expose_security_errors` config option to display `AccountStatusException`
1010
* Deprecate the `security.hide_user_not_found` config option in favor of `security.expose_security_errors`
11+
* Deprecate the `erase_credentials` config option, erase credentials on your own e.g. upon `AuthenticationTokenCreatedEvent` instead
1112

1213
7.2
1314
---

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LdapFactoryTrait.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\DependencyInjection\Definition;
1717
use Symfony\Component\DependencyInjection\Reference;
1818
use Symfony\Component\Ldap\Security\CheckLdapCredentialsListener;
19+
use Symfony\Component\Ldap\Security\EraseLdapUserCredentialsListener;
1920
use Symfony\Component\Ldap\Security\LdapAuthenticator;
2021

2122
/**
@@ -42,6 +43,12 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal
4243
->addArgument(new Reference('security.ldap_locator'))
4344
;
4445

46+
if (class_exists(EraseLdapUserCredentialsListener::class && !$container->getParameter('security.authentication.manager.erase_credentials'))) {
47+
$container->setDefinition('security.listener.'.$key.'.'.$firewallName.'erase_ldap_credentials', new Definition(EraseLdapUserCredentialsListener::class))
48+
->addTag('kernel.event_subscriber', ['dispatcher' => 'security.event_dispatcher.'.$firewallName])
49+
;
50+
}
51+
4552
$ldapAuthenticatorId = 'security.authenticator.'.$key.'.'.$firewallName;
4653
$definition = $container->setDefinition($ldapAuthenticatorId, new Definition(LdapAuthenticator::class))
4754
->setArguments([

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ public function load(array $configs, ContainerBuilder $container): void
136136

137137
// set some global scalars
138138
$container->setParameter('security.access.denied_url', $config['access_denied_url']);
139+
if (true === $config['erase_credentials']) {
140+
trigger_deprecation('symfony/security-bundle', '7.3', 'Setting the "security.erase_credentials" config option to true is deprecated and won\'t have any effect in 8.0, set it to false instead and use your own erasing logic if needed.');
141+
}
139142
$container->setParameter('security.authentication.manager.erase_credentials', $config['erase_credentials']);
140143
$container->setParameter('security.authentication.session_strategy.strategy', $config['session_fixation_strategy']);
141144

‎src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ public function testOnKernelRequestRecordsAuthenticatorsInfo()
103103
[new TraceableAuthenticator($notSupportingAuthenticator), new TraceableAuthenticator($supportingAuthenticator)],
104104
$tokenStorage,
105105
$dispatcher,
106-
'main'
106+
'main',
107+
null,
108+
false
107109
);
108110

109111
$listener = new TraceableAuthenticatorManagerListener(new AuthenticatorManagerListener($authenticatorManager));

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,7 @@ public function isEnabled(): bool
250250
return $this->enabled;
251251
}
252252

253+
#[\Deprecated]
253254
public function eraseCredentials(): void
254255
{
255256
}

‎src/Symfony/Bundle/SecurityBundle/composer.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
"symfony/clock": "^6.4|^7.0",
2323
"symfony/config": "^6.4|^7.0",
2424
"symfony/dependency-injection": "^6.4.11|^7.1.4",
25+
"symfony/deprecation-contracts": "^2.5|^3",
2526
"symfony/event-dispatcher": "^6.4|^7.0",
2627
"symfony/http-kernel": "^6.4|^7.0",
2728
"symfony/http-foundation": "^6.4|^7.0",

‎src/Symfony/Component/Ldap/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
CHANGELOG
22
=========
33

4+
7.3
5+
---
6+
7+
* Deprecate `LdapUser::eraseCredentials()`, use `LdapUser::setPassword(null)` instead
8+
* Add `EraseLdapUserCredentialsListener`
9+
410
7.2
511
---
612

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Ldap\Security;
13+
14+
use Psr\Container\ContainerInterface;
15+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
16+
use Symfony\Component\Ldap\Exception\InvalidCredentialsException;
17+
use Symfony\Component\Ldap\Exception\InvalidSearchCredentialsException;
18+
use Symfony\Component\Ldap\LdapInterface;
19+
use Symfony\Component\Security\Core\Event\AuthenticationSuccessEvent;
20+
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
21+
use Symfony\Component\Security\Core\Exception\LogicException;
22+
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
23+
use Symfony\Component\Security\Http\Event\AuthenticationTokenCreatedEvent;
24+
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
25+
26+
/**
27+
* Erases credentials from LdapUser instances upon successful authentication.
28+
*
29+
* @author Robin Chalas <robin.chalas@gmail.com>
30+
*/
31+
class EraseLdapUserCredentialsListener implements EventSubscriberInterface
32+
{
33+
public function onAuthenticationSuccess(AuthenticationSuccessEvent $event): void
34+
{
35+
$user = $event->getAuthenticationToken()->getUser();
36+
37+
if (!$user instanceof LdapUser) {
38+
return;
39+
}
40+
41+
$user->setPassword(null);
42+
}
43+
44+
public static function getSubscribedEvents(): array
45+
{
46+
return [AuthenticationSuccessEvent::class => ['onAuthenticationSuccess', 256]];
47+
}
48+
}

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ public function getUserIdentifier(): string
6262

6363
public function eraseCredentials(): void
6464
{
65+
trigger_deprecation('symfony/security-core', '7.3', sprintf('The "%s()" method is deprecated and will be removed in 8.0, call "setPassword(null)" instead.', __METHOD__));
66+
6567
$this->password = null;
6668
}
6769

@@ -70,7 +72,7 @@ public function getExtraFields(): array
7072
return $this->extraFields;
7173
}
7274

73-
public function setPassword(#[\SensitiveParameter] string $password): void
75+
public function setPassword(#[\SensitiveParameter] ?string $password): void
7476
{
7577
$this->password = $password;
7678
}
@@ -95,4 +97,14 @@ public function isEqualTo(UserInterface $user): bool
9597

9698
return true;
9799
}
100+
101+
public function __serialize(): array
102+
{
103+
return [$this->entry, $this->identifier, null, $this->roles, $this->extraFields];
104+
}
105+
106+
public function __unserialize(array $data): void
107+
{
108+
[$this->entry, $this->identifier, $this->password, $this->roles, $this->extraFields] = $data;
109+
}
98110
}

‎src/Symfony/Component/Ldap/Tests/Security/EraseLdapUserCredentialsListenerTest.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Ldap\Tests\Security;
13+
14+
use PHPUnit\Framework\MockObject\MockObject;
15+
use PHPUnit\Framework\TestCase;
16+
use Psr\Container\ContainerInterface;
17+
use Symfony\Component\HttpFoundation\Request;
18+
use Symfony\Component\HttpFoundation\Response;
19+
use Symfony\Component\Ldap\Adapter\CollectionInterface;
20+
use Symfony\Component\Ldap\Adapter\QueryInterface;
21+
use Symfony\Component\Ldap\Entry;
22+
use Symfony\Component\Ldap\Exception\InvalidCredentialsException;
23+
use Symfony\Component\Ldap\LdapInterface;
24+
use Symfony\Component\Ldap\Security\CheckLdapCredentialsListener;
25+
use Symfony\Component\Ldap\Security\EraseLdapUserCredentialsListener;
26+
use Symfony\Component\Ldap\Security\LdapBadge;
27+
use Symfony\Component\Ldap\Security\LdapUser;
28+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
29+
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
30+
use Symfony\Component\Security\Core\Event\AuthenticationSuccessEvent;
31+
use Symfony\Component\Security\Core\Exception\AuthenticationException;
32+
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
33+
use Symfony\Component\Security\Core\User\InMemoryUser;
34+
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
35+
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
36+
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
37+
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
38+
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
39+
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
40+
use Symfony\Contracts\Service\ServiceLocatorTrait;
41+
42+
class EraseLdapUserCredentialsListenerTest extends TestCase
43+
{
44+
public function testPasswordIsErasedOnAuthenticationSuccess()
45+
{
46+
$user = new LdapUser(new Entry(''), 'chalasr', 'password');
47+
$listener = new EraseLdapUserCredentialsListener();
48+
49+
$listener->onAuthenticationSuccess(new AuthenticationSuccessEvent(new UsernamePasswordToken($user, 'main')));
50+
51+
$this->assertSame(null, $user->getPassword());
52+
}
53+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
"require": {
1919
"php": ">=8.2",
2020
"ext-ldap": "*",
21+
"symfony/deprecation-contracts": "^2.5|^3",
2122
"symfony/options-resolver": "^6.4|^7.0"
2223
},
2324
"require-dev": {

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,15 @@ public function setUser(UserInterface $user): void
5959
$this->user = $user;
6060
}
6161

62+
/**
63+
* Removes sensitive information from the token.
64+
*
65+
* @deprecated since Symfony 7.3
66+
*/
6267
public function eraseCredentials(): void
6368
{
69+
trigger_deprecation('symfony/security-core', '7.3', sprintf('The "%s()" method is deprecated and will be removed in 8.0, use a DTO instead or implement your own erasing logic if needed.', __METHOD__));
70+
6471
if ($this->getUser() instanceof UserInterface) {
6572
$this->getUser()->eraseCredentials();
6673
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ public function getUserIdentifier(): string
4343
return '';
4444
}
4545

46+
/**
47+
* Removes sensitive information from the token.
48+
*
49+
* @deprecated since Symfony 7.3
50+
*/
4651
public function eraseCredentials(): void
4752
{
4853
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ public function setUser(UserInterface $user): void;
5656

5757
/**
5858
* Removes sensitive information from the token.
59+
*
60+
* @deprecated since Symfony 7.3, use a dedicated DTO instead or implement your
61+
* own erasing logic instead
5962
*/
6063
public function eraseCredentials(): void;
6164

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ CHANGELOG
77
* Add `UserAuthorizationChecker::isGrantedForUser()` to test user authorization without relying on the session.
88
For example, users not currently logged in, or while processing a message from a message queue.
99
* Add `OfflineTokenInterface` to mark tokens that do not represent the currently logged-in user
10+
* Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`,
11+
use a dedicated DTO or erase credentials on your own e.g. upon `AuthenticationTokenCreatedEvent` instead
1012

1113
7.2
1214
---

‎src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php

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

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
1516
use Symfony\Component\Security\Core\Authentication\Token\AbstractToken;
1617
use Symfony\Component\Security\Core\User\InMemoryUser;
1718
use Symfony\Component\Security\Core\User\UserInterface;
1819

1920
class AbstractTokenTest extends TestCase
2021
{
22+
use ExpectDeprecationTrait;
23+
2124
/**
2225
* @dataProvider provideUsers
2326
*/
@@ -33,13 +36,17 @@ public static function provideUsers()
3336
yield [new InMemoryUser('fabien', null), 'fabien'];
3437
}
3538

39+
/**
40+
* @group legacy
41+
*/
3642
public function testEraseCredentials()
3743
{
3844
$token = new ConcreteToken(['ROLE_FOO']);
3945

4046
$user = $this->createMock(UserInterface::class);
4147
$user->expects($this->once())->method('eraseCredentials');
4248
$token->setUser($user);
49+
$this->expectDeprecation('The Symfony\Component\Security\Core\User\UserInterface::eraseCredentials method is deprecated (since Symfony 7.3, use a dedicated DTO instead or implement your own erasing logic instead).');
4350

4451
$token->eraseCredentials();
4552
}

‎src/Symfony/Component/Security/Core/Tests/User/InMemoryUserTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ public function testIsEnabled()
5353
$this->assertFalse($user->isEnabled());
5454
}
5555

56+
/**
57+
* @group legacy
58+
*/
5659
public function testEraseCredentials()
5760
{
5861
$user = new InMemoryUser('fabien', 'superpass');

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ public function isEnabled(): bool
7676

7777
public function eraseCredentials(): void
7878
{
79+
trigger_deprecation('symfony/security-core', '7.3', sprintf('The "%s()" method is deprecated and will be removed in 8.0, use a DTO instead or implement your own erasing logic if needed.', __METHOD__));
7980
}
8081

8182
public function isEqualTo(UserInterface $user): bool

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,18 +46,22 @@ interface UserInterface
4646
*/
4747
public function getRoles(): array;
4848

49+
/**
50+
* Returns the identifier for this user (e.g. username or email address).
51+
*
52+
* @return non-empty-string
53+
*/
54+
public function getUserIdentifier(): string;
55+
4956
/**
5057
* Removes sensitive data from the user.
5158
*
5259
* This is important if, at any given point, sensitive information like
5360
* the plain-text password is stored on this object.
61+
*
62+
* @deprecated since Symfony 7.3, use a dedicated DTO instead or implement your
63+
* own erasing logic instead
5464
*/
5565
public function eraseCredentials(): void;
5666

57-
/**
58-
* Returns the identifier for this user (e.g. username or email address).
59-
*
60-
* @return non-empty-string
61-
*/
62-
public function getUserIdentifier(): string;
6367
}

‎src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ public function __construct(
6969
}
7070

7171
$this->exposeSecurityErrors = $exposeSecurityErrors;
72+
73+
if ($eraseCredentials) {
74+
trigger_deprecation('symfony/security-http', '7.3', sprintf('Passing true as "$eraseCredentials" argument to "%s::__construct()" is deprecated and won\'t have any effect in 8.0, pass "false" instead and use your own erasing logic if needed.', __CLASS__));
75+
}
7276
}
7377

7478
/**
@@ -208,6 +212,7 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req
208212
// announce the authentication token
209213
$authenticatedToken = $this->eventDispatcher->dispatch(new AuthenticationTokenCreatedEvent($authenticatedToken, $passport))->getAuthenticatedToken();
210214

215+
// @deprecated since Symfony 7.3, remove the if statement in 8.0
211216
if (true === $this->eraseCredentials) {
212217
$authenticatedToken->eraseCredentials();
213218
}

0 commit comments

Comments
 (0)
0