feature symfony#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 symfony#57842
| License       | MIT

As promised, this PR adds a commit on top of symfony#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()`

Author: nicolas-grekas

UPGRADE-7.3.md

@@ -6,7 +6,42 @@ backward compatibility breaks. Minor backward compatibility breaks are prefixed
 `[BC BREAK]`, make sure your code is compatible with these entries before upgrading.
 Read more about this in the [Symfony documentation](https://symfony.com/doc/7.3/setup/upgrade_minor.html).
 
-If you're upgrading from a version below 7.1, follow the [7.2 upgrade guide](UPGRADE-7.2.md) first.
+If you're upgrading from a version below 7.2, follow the [7.2 upgrade guide](UPGRADE-7.2.md) first.
+
+Ldap
+----
+
+ * Deprecate `LdapUser::eraseCredentials()` in favor of `__serialize()`
+
+Security
+--------
+
+ * Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`,
+   erase credentials e.g. using `__serialize()` instead
+
+   *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;
+   }
+   ```
 
 Console
 -------

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

@@ -45,6 +45,7 @@ public function getUserIdentifier(): string
         return $this->name;
     }
 
+    #[\Deprecated]
     public function eraseCredentials(): void
     {
     }

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

@@ -336,7 +336,7 @@ public static function handleError($type, $msg, $file, $line, $context = [])
 
             return $h ? $h($type, $msg, $file, $line, $context) : false;
         }
-        // If the message is serialized we need to extract the message. This occurs when the error is triggered by
+        // If the message is serialized we need to extract the message. This occurs when the error is triggered
         // by the isolated test path in \Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::endTest().
         $parsedMsg = @unserialize($msg);
         if (\is_array($parsedMsg)) {

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

@@ -19,6 +19,7 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\Security\Core\Authentication\Token\AbstractToken;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Http\Authentication\AuthenticatorManager;
@@ -89,7 +90,7 @@ public function testOnKernelRequestRecordsAuthenticatorsInfo()
         $supportingAuthenticator
             ->expects($this->once())
             ->method('createToken')
-            ->willReturn($this->createMock(TokenInterface::class));
+            ->willReturn(new class extends AbstractToken {});
 
         $notSupportingAuthenticator = $this->createMock(DummyAuthenticator::class);
         $notSupportingAuthenticator

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

@@ -250,6 +250,7 @@ public function isEnabled(): bool
         return $this->enabled;
     }
 
+    #[\Deprecated]
     public function eraseCredentials(): void
     {
     }

src/Symfony/Component/Ldap/CHANGELOG.md

@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+7.3
+---
+
+ * Deprecate `LdapUser::eraseCredentials()` in favor of `__serialize()`
+
 7.2
 ---
 

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

@@ -60,8 +60,16 @@ public function getUserIdentifier(): string
         return $this->identifier;
     }
 
+    /**
+     * @deprecated since Symfony 7.3
+     */
+    #[\Deprecated(since: 'symfony/ldap 7.3')]
     public function eraseCredentials(): void
     {
+        if (\PHP_VERSION_ID < 80400) {
+            @trigger_error(\sprintf('Method %s::eraseCredentials() is deprecated since symfony/ldap 7.3', self::class), \E_USER_DEPRECATED);
+        }
+
         $this->password = null;
     }
 
@@ -70,7 +78,7 @@ public function getExtraFields(): array
         return $this->extraFields;
     }
 
-    public function setPassword(#[\SensitiveParameter] string $password): void
+    public function setPassword(#[\SensitiveParameter] ?string $password): void
     {
         $this->password = $password;
     }
@@ -95,4 +103,12 @@ public function isEqualTo(UserInterface $user): bool
 
         return true;
     }
+
+    public function __serialize(): array
+    {
+        $data = (array) $this;
+        unset($data[\sprintf("\0%s\0password", self::class)]);
+
+        return $data;
+    }
 }

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

@@ -35,10 +35,9 @@ public function getRoles(): array
         return $this->roles;
     }
 
+    #[\Deprecated]
     public function eraseCredentials(): void
     {
-        // Do nothing
-        return;
     }
 
     public function getUserIdentifier(): string

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

@@ -238,25 +238,9 @@ public function testMigrateFromWithCustomInstance()
 
 class SomeUser implements PasswordAuthenticatedUserInterface
 {
-    public function getRoles(): array
-    {
-    }
-
     public function getPassword(): ?string
     {
     }
-
-    public function getSalt(): ?string
-    {
-    }
-
-    public function getUserIdentifier(): string
-    {
-    }
-
-    public function eraseCredentials()
-    {
-    }
 }
 
 class SomeChildUser extends SomeUser

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

@@ -59,8 +59,15 @@ public function setUser(UserInterface $user): void
         $this->user = $user;
     }
 
+    /**
+     * Removes sensitive information from the token.
+     *
+     * @deprecated since Symfony 7.3, erase credentials using the "__serialize()" method instead
+     */
     public function eraseCredentials(): void
     {
+        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));
+
         if ($this->getUser() instanceof UserInterface) {
             $this->getUser()->eraseCredentials();
         }