Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5a09d52

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
feature #59682 [Security] Deprecate UserInterface & TokenInterface's eraseCredentials() (chalasr, nicolas-grekas)
This PR was merged into the 7.3 branch. Discussion ---------- [Security] Deprecate UserInterface & TokenInterface's `eraseCredentials()` | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | no | New feature? | yes | Deprecations? | yes | Issues | Fix #57842 | License | MIT As promised, this PR adds a commit on top of #59106 to improve the BC layer. This approach didn't fit in a review comment :) /cc `@chalasr` This PR leverages the new `#[\Deprecated]` attribute to decide if some `eraseCredentials()` method is to be called or not. My target DX here is to save us all (the community) from having to add `erase_credentials: false` configuration in all our apps. So, instead of having to opt-out from the deprecation using this config option, the opt-out is done by adding the attribute on the method: Before: ```php public function eraseCredentials(): void { } ``` After: ```php #[\Deprecated] public function eraseCredentials(): void { } // If your eraseCredentials() method was used to empty a "password" property: public function __serialize(): array { $data = (array) $this; unset($data["\0".self::class."\0password"]); return $data; } ``` This should provide a smoother upgrade path (and maker-bundle could be updated right-away). Commits ------- e5c94e6 [Security] Improve BC-layer to deprecate eraseCredentials methods e556606 [Security] Deprecate `UserInterface` & `TokenInterface`'s `eraseCredentials()`
2 parents f3d614c + e5c94e6 commit 5a09d52
Copy full SHA for 5a09d52

27 files changed

+197
-51
lines changed

‎UPGRADE-7.3.md

Copy file name to clipboardExpand all lines: UPGRADE-7.3.md
+36-1Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,42 @@ 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()` in favor of `__serialize()`
15+
16+
Security
17+
--------
18+
19+
* Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`,
20+
erase credentials e.g. using `__serialize()` instead
21+
22+
*Before*
23+
```php
24+
public function eraseCredentials(): void
25+
{
26+
}
27+
```
28+
29+
*After*
30+
```php
31+
#[\Deprecated]
32+
public function eraseCredentials(): void
33+
{
34+
}
35+
36+
// If your eraseCredentials() method was used to empty a "password" property:
37+
public function __serialize(): array
38+
{
39+
$data = (array) $this;
40+
unset($data["\0".self::class."\0password"]);
41+
42+
return $data;
43+
}
44+
```
1045

1146
Console
1247
-------

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Tests/Fixtures/User.php
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public function getUserIdentifier(): string
4545
return $this->name;
4646
}
4747

48+
#[\Deprecated]
4849
public function eraseCredentials(): void
4950
{
5051
}

‎src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ public static function handleError($type, $msg, $file, $line, $context = [])
336336

337337
return $h ? $h($type, $msg, $file, $line, $context) : false;
338338
}
339-
// If the message is serialized we need to extract the message. This occurs when the error is triggered by
339+
// If the message is serialized we need to extract the message. This occurs when the error is triggered
340340
// by the isolated test path in \Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::endTest().
341341
$parsedMsg = @unserialize($msg);
342342
if (\is_array($parsedMsg)) {

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Symfony\Component\HttpFoundation\Response;
2020
use Symfony\Component\HttpKernel\Event\RequestEvent;
2121
use Symfony\Component\HttpKernel\HttpKernelInterface;
22+
use Symfony\Component\Security\Core\Authentication\Token\AbstractToken;
2223
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
2324
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2425
use Symfony\Component\Security\Http\Authentication\AuthenticatorManager;
@@ -89,7 +90,7 @@ public function testOnKernelRequestRecordsAuthenticatorsInfo()
8990
$supportingAuthenticator
9091
->expects($this->once())
9192
->method('createToken')
92-
->willReturn($this->createMock(TokenInterface::class));
93+
->willReturn(new class extends AbstractToken {});
9394

9495
$notSupportingAuthenticator = $this->createMock(DummyAuthenticator::class);
9596
$notSupportingAuthenticator

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php
+1Lines 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/Component/Ldap/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Component/Ldap/CHANGELOG.md
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
7.3
5+
---
6+
7+
* Deprecate `LdapUser::eraseCredentials()` in favor of `__serialize()`
8+
49
7.2
510
---
611

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Ldap/Security/LdapUser.php
+17-1Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,16 @@ public function getUserIdentifier(): string
6060
return $this->identifier;
6161
}
6262

63+
/**
64+
* @deprecated since Symfony 7.3
65+
*/
66+
#[\Deprecated(since: 'symfony/ldap 7.3')]
6367
public function eraseCredentials(): void
6468
{
69+
if (\PHP_VERSION_ID < 80400) {
70+
@trigger_error(\sprintf('Method %s::eraseCredentials() is deprecated since symfony/ldap 7.3', self::class), \E_USER_DEPRECATED);
71+
}
72+
6573
$this->password = null;
6674
}
6775

@@ -70,7 +78,7 @@ public function getExtraFields(): array
7078
return $this->extraFields;
7179
}
7280

73-
public function setPassword(#[\SensitiveParameter] string $password): void
81+
public function setPassword(#[\SensitiveParameter] ?string $password): void
7482
{
7583
$this->password = $password;
7684
}
@@ -95,4 +103,12 @@ public function isEqualTo(UserInterface $user): bool
95103

96104
return true;
97105
}
106+
107+
public function __serialize(): array
108+
{
109+
$data = (array) $this;
110+
unset($data[\sprintf("\0%s\0password", self::class)]);
111+
112+
return $data;
113+
}
98114
}

‎src/Symfony/Component/PasswordHasher/Tests/Fixtures/TestLegacyPasswordAuthenticatedUser.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/PasswordHasher/Tests/Fixtures/TestLegacyPasswordAuthenticatedUser.php
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,9 @@ public function getRoles(): array
3535
return $this->roles;
3636
}
3737

38+
#[\Deprecated]
3839
public function eraseCredentials(): void
3940
{
40-
// Do nothing
41-
return;
4241
}
4342

4443
public function getUserIdentifier(): string

‎src/Symfony/Component/PasswordHasher/Tests/Hasher/PasswordHasherFactoryTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/PasswordHasher/Tests/Hasher/PasswordHasherFactoryTest.php
-16Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -238,25 +238,9 @@ public function testMigrateFromWithCustomInstance()
238238

239239
class SomeUser implements PasswordAuthenticatedUserInterface
240240
{
241-
public function getRoles(): array
242-
{
243-
}
244-
245241
public function getPassword(): ?string
246242
{
247243
}
248-
249-
public function getSalt(): ?string
250-
{
251-
}
252-
253-
public function getUserIdentifier(): string
254-
{
255-
}
256-
257-
public function eraseCredentials()
258-
{
259-
}
260244
}
261245

262246
class SomeChildUser extends SomeUser

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php
+7Lines 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, erase credentials using the "__serialize()" method instead
66+
*/
6267
public function eraseCredentials(): void
6368
{
69+
trigger_deprecation('symfony/security-core', '7.3', \sprintf('The "%s::eraseCredentials()" method is deprecated and will be removed in 8.0, erase credentials using the "__serialize()" method instead.', TokenInterface::class));
70+
6471
if ($this->getUser() instanceof UserInterface) {
6572
$this->getUser()->eraseCredentials();
6673
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,15 @@ public function getUserIdentifier(): string
4343
return '';
4444
}
4545

46+
/**
47+
* @deprecated since Symfony 7.3
48+
*/
49+
#[\Deprecated(since: 'symfony/security-core 7.3')]
4650
public function eraseCredentials(): void
4751
{
52+
if (\PHP_VERSION_ID < 80400) {
53+
@trigger_error(\sprintf('Method %s::eraseCredentials() is deprecated since symfony/security-core 7.3', self::class), \E_USER_DEPRECATED);
54+
}
4855
}
4956

5057
public function getAttributes(): array

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Authentication/Token/TokenInterface.php
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
/**
1717
* TokenInterface is the interface for the user authentication information.
1818
*
19+
* The __serialize/__unserialize() magic methods can be implemented on the token
20+
* class to prevent sensitive credentials from being put in the session storage.
21+
*
1922
* @author Fabien Potencier <fabien@symfony.com>
2023
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
2124
*/
@@ -56,6 +59,8 @@ public function setUser(UserInterface $user): void;
5659

5760
/**
5861
* Removes sensitive information from the token.
62+
*
63+
* @deprecated since Symfony 7.3; erase credentials using the "__serialize()" method instead
5964
*/
6065
public function eraseCredentials(): void;
6166

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/CHANGELOG.md
+2Lines 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+
erase credentials e.g. using `__serialize()` instead
1012

1113
7.2
1214
---

‎src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationTrustResolverTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationTrustResolverTest.php
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ public function getUserIdentifier(): string
119119
{
120120
}
121121

122+
#[\Deprecated]
122123
public function eraseCredentials(): void
123124
{
124125
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,16 @@
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;
17+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1618
use Symfony\Component\Security\Core\User\InMemoryUser;
1719
use Symfony\Component\Security\Core\User\UserInterface;
1820

1921
class AbstractTokenTest extends TestCase
2022
{
23+
use ExpectDeprecationTrait;
24+
2125
/**
2226
* @dataProvider provideUsers
2327
*/
@@ -33,6 +37,9 @@ public static function provideUsers()
3337
yield [new InMemoryUser('fabien', null), 'fabien'];
3438
}
3539

40+
/**
41+
* @group legacy
42+
*/
3643
public function testEraseCredentials()
3744
{
3845
$token = new ConcreteToken(['ROLE_FOO']);
@@ -41,6 +48,8 @@ public function testEraseCredentials()
4148
$user->expects($this->once())->method('eraseCredentials');
4249
$token->setUser($user);
4350

51+
$this->expectDeprecation(\sprintf('Since symfony/security-core 7.3: The "%s::eraseCredentials()" method is deprecated and will be removed in 8.0, erase credentials using the "__serialize()" method instead.', TokenInterface::class));
52+
4453
$token->eraseCredentials();
4554
}
4655

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Tests/Authentication/Token/Fixtures/CustomUser.php
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ public function getRoles(): array
3131
return $this->roles;
3232
}
3333

34+
public function getPassword(): ?string
35+
{
36+
return null;
37+
}
38+
39+
#[\Deprecated]
3440
public function eraseCredentials(): void
3541
{
3642
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Tests/User/InMemoryUserTest.php
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,14 @@
1212
namespace Symfony\Component\Security\Core\Tests\User;
1313

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

1819
class InMemoryUserTest extends TestCase
1920
{
21+
use ExpectDeprecationTrait;
22+
2023
public function testConstructorException()
2124
{
2225
$this->expectException(\InvalidArgumentException::class);
@@ -53,9 +56,13 @@ public function testIsEnabled()
5356
$this->assertFalse($user->isEnabled());
5457
}
5558

59+
/**
60+
* @group legacy
61+
*/
5662
public function testEraseCredentials()
5763
{
5864
$user = new InMemoryUser('fabien', 'superpass');
65+
$this->expectDeprecation(\sprintf('%sMethod %s::eraseCredentials() is deprecated since symfony/security-core 7.3', \PHP_VERSION_ID >= 80400 ? 'Unsilenced deprecation: ' : '', InMemoryUser::class));
5966
$user->eraseCredentials();
6067
$this->assertEquals('superpass', $user->getPassword());
6168
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/User/InMemoryUser.php
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,15 @@ public function isEnabled(): bool
7474
return $this->enabled;
7575
}
7676

77+
/**
78+
* @deprecated since Symfony 7.3
79+
*/
80+
#[\Deprecated(since: 'symfony/security-core 7.3')]
7781
public function eraseCredentials(): void
7882
{
83+
if (\PHP_VERSION_ID < 80400) {
84+
@trigger_error(\sprintf('Method %s::eraseCredentials() is deprecated since symfony/security-core 7.3', self::class), \E_USER_DEPRECATED);
85+
}
7986
}
8087

8188
public function isEqualTo(UserInterface $user): bool

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/User/OidcUser.php
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,15 @@ public function getUserIdentifier(): string
7171
return (string) ($this->userIdentifier ?? $this->getSub());
7272
}
7373

74+
/**
75+
* @deprecated since Symfony 7.3
76+
*/
77+
#[\Deprecated(since: 'symfony/security-core 7.3')]
7478
public function eraseCredentials(): void
7579
{
80+
if (\PHP_VERSION_ID < 80400) {
81+
@trigger_error(\sprintf('Method %s::eraseCredentials() is deprecated since symfony/security-core 7.3', self::class), \E_USER_DEPRECATED);
82+
}
7683
}
7784

7885
public function getSub(): ?string

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/User/PasswordAuthenticatedUserInterface.php
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ interface PasswordAuthenticatedUserInterface
2323
* Returns the hashed password used to authenticate the user.
2424
*
2525
* Usually on authentication, a plain-text password will be compared to this value.
26+
*
27+
* The __serialize/__unserialize() magic methods can be implemented on the user
28+
* class to prevent hashed passwords from being put in the session storage.
2629
*/
2730
public function getPassword(): ?string;
2831
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/User/UserInterface.php
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
* this interface. Objects that implement this interface are created and
2525
* loaded by different objects that implement UserProviderInterface.
2626
*
27+
* The __serialize/__unserialize() magic methods can be implemented on the user
28+
* class to prevent sensitive credentials from being put in the session storage.
29+
*
2730
* @see UserProviderInterface
2831
*
2932
* @author Fabien Potencier <fabien@symfony.com>
@@ -51,6 +54,8 @@ public function getRoles(): array;
5154
*
5255
* This is important if, at any given point, sensitive information like
5356
* the plain-text password is stored on this object.
57+
*
58+
* @deprecated since Symfony 7.3, erase credentials using the "__serialize()" method instead
5459
*/
5560
public function eraseCredentials(): void;
5661

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.