[Security] Improve BC-layer to deprecate eraseCredentials methods

symfony · nicolas-grekas · Feb 4, 2025 · Dec 6, 2024 · Feb 3, 2025 · Feb 4, 2025
commit e5c94e62283a11985c7114a13063727f7a9334ce
diff --git a/UPGRADE-7.3.md b/UPGRADE-7.3.md
@@ -11,18 +11,37 @@ If you're upgrading from a version below 7.2, follow the [7.2 upgrade guide](UPG
 Ldap
 ----
 
- * Deprecate `LdapUser::eraseCredentials()`, use `LdapUser::setPassword(null)` instead
+ * Deprecate `LdapUser::eraseCredentials()` in favor of `__serialize()`
 
 Security
 --------
 
  * Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`,
-   use a dedicated DTO or erase credentials on your own e.g. upon `AuthenticationTokenCreatedEvent` instead
+   erase credentials e.g. using `__serialize()` instead
 
-SecurityBundle
---------------
+   *Before*
+   ```php
+   public function eraseCredentials(): void
+   {
+   }
+   ```
 
- * Deprecate the `erase_credentials` config option, erase credentials on your own e.g. upon `AuthenticationTokenCreatedEvent` instead
+   *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
 -------
@@ -131,4 +150,3 @@ VarDumper
 
  * Deprecate `ResourceCaster::castCurl()`, `ResourceCaster::castGd()` and `ResourceCaster::castOpensslX509()`
  * Mark all casters as `@internal`
- * Deprecate the `CompiledClassMetadataFactory` and `CompiledClassMetadataCacheWarmer` classes
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Fixtures/User.php b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/User.php
@@ -45,6 +45,7 @@ public function getUserIdentifier(): string
         return $this->name;
     }
 
+    #[\Deprecated]
     public function eraseCredentials(): void
     {
     }

diff --git a/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php b/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)) {

diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
@@ -8,7 +8,6 @@ CHANGELOG
  * Add encryption support to `OidcTokenHandler` (JWE)
  * Add `expose_security_errors` config option to display `AccountStatusException`
  * Deprecate the `security.hide_user_not_found` config option in favor of `security.expose_security_errors`
- * Deprecate the `erase_credentials` config option, erase credentials on your own e.g. upon `AuthenticationTokenCreatedEvent` instead
 
 7.2
 ---

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LdapFactoryTrait.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LdapFactoryTrait.php
@@ -16,7 +16,6 @@
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\Ldap\Security\CheckLdapCredentialsListener;
-use Symfony\Component\Ldap\Security\EraseLdapUserCredentialsListener;
 use Symfony\Component\Ldap\Security\LdapAuthenticator;
 
 /**
@@ -43,12 +42,6 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal
             ->addArgument(new Reference('security.ldap_locator'))
         ;
 
-        if (class_exists(EraseLdapUserCredentialsListener::class && !$container->getParameter('security.authentication.manager.erase_credentials'))) {
-            $container->setDefinition('security.listener.'.$key.'.'.$firewallName.'erase_ldap_credentials', new Definition(EraseLdapUserCredentialsListener::class))
-                ->addTag('kernel.event_subscriber', ['dispatcher' => 'security.event_dispatcher.'.$firewallName])
-            ;
-        }
-
         $ldapAuthenticatorId = 'security.authenticator.'.$key.'.'.$firewallName;
         $definition = $container->setDefinition($ldapAuthenticatorId, new Definition(LdapAuthenticator::class))
             ->setArguments([

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -136,9 +136,6 @@ public function load(array $configs, ContainerBuilder $container): void
 
         // set some global scalars
         $container->setParameter('security.access.denied_url', $config['access_denied_url']);
-        if (true === $config['erase_credentials']) {
-            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.');
-        }
         $container->setParameter('security.authentication.manager.erase_credentials', $config['erase_credentials']);
         $container->setParameter('security.authentication.session_strategy.strategy', $config['session_fixation_strategy']);
 

@@ -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
@@ -103,9 +104,7 @@ public function testOnKernelRequestRecordsAuthenticatorsInfo()
             [new TraceableAuthenticator($notSupportingAuthenticator), new TraceableAuthenticator($supportingAuthenticator)],
             $tokenStorage,
             $dispatcher,
-            'main',
-            null,
-            false
+            'main'
         );
 
         $listener = new TraceableAuthenticatorManagerListener(new AuthenticatorManagerListener($authenticatorManager));

diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json
@@ -22,7 +22,6 @@
         "symfony/clock": "^6.4|^7.0",
         "symfony/config": "^6.4|^7.0",
         "symfony/dependency-injection": "^6.4.11|^7.1.4",
-        "symfony/deprecation-contracts": "^2.5|^3",
         "symfony/event-dispatcher": "^6.4|^7.0",
         "symfony/http-kernel": "^6.4|^7.0",
         "symfony/http-foundation": "^6.4|^7.0",

diff --git a/src/Symfony/Component/Ldap/CHANGELOG.md b/src/Symfony/Component/Ldap/CHANGELOG.md
@@ -4,8 +4,7 @@ CHANGELOG
 7.3
 ---
 
- * Deprecate `LdapUser::eraseCredentials()`, use `LdapUser::setPassword(null)` instead
- * Add `EraseLdapUserCredentialsListener`
+ * Deprecate `LdapUser::eraseCredentials()` in favor of `__serialize()`
 
 7.2
 ---

diff --git a/src/Symfony/Component/Ldap/Security/EraseLdapUserCredentialsListener.php b/src/Symfony/Component/Ldap/Security/EraseLdapUserCredentialsListener.php
@@ -60,9 +60,15 @@ public function getUserIdentifier(): string
         return $this->identifier;
     }
 
+    /**
+     * @deprecated since Symfony 7.3
+     */
+    #[\Deprecated(since: 'symfony/ldap 7.3')]
     public function eraseCredentials(): void
     {
-        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__));
+        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;
     }
@@ -100,11 +106,9 @@ public function isEqualTo(UserInterface $user): bool
 
     public function __serialize(): array
     {
-        return [$this->entry, $this->identifier, null, $this->roles, $this->extraFields];
-    }
+        $data = (array) $this;
+        unset($data[\sprintf("\0%s\0password", self::class)]);
 
-    public function __unserialize(array $data): void
-    {
-        [$this->entry, $this->identifier, $this->password, $this->roles, $this->extraFields] = $data;
+        return $data;
     }
 }
diff --git a/src/Symfony/Component/Ldap/Tests/Security/EraseLdapUserCredentialsListenerTest.php b/src/Symfony/Component/Ldap/Tests/Security/EraseLdapUserCredentialsListenerTest.php
diff --git a/src/Symfony/Component/Ldap/composer.json b/src/Symfony/Component/Ldap/composer.json
@@ -18,7 +18,6 @@
     "require": {
         "php": ">=8.2",
         "ext-ldap": "*",
-        "symfony/deprecation-contracts": "^2.5|^3",
         "symfony/options-resolver": "^6.4|^7.0"
     },
     "require-dev": {

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

@@ -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

@@ -62,11 +62,11 @@ public function setUser(UserInterface $user): void
     /**
      * Removes sensitive information from the token.
      *
-     * @deprecated since Symfony 7.3
+     * @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()" method is deprecated and will be removed in 8.0, use a DTO instead or implement your own erasing logic if needed.', __METHOD__));
+        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();

@@ -44,12 +44,14 @@ public function getUserIdentifier(): string
     }
 
     /**
-     * Removes sensitive information from the token.
-     *
      * @deprecated since Symfony 7.3
      */
+    #[\Deprecated(since: 'symfony/security-core 7.3')]
     public function eraseCredentials(): void
     {
+    <
E377
span class='blob-code-inner blob-code-marker ' data-code-marker="+">        if (\PHP_VERSION_ID < 80400) {
+            @trigger_error(\sprintf('Method %s::eraseCredentials() is deprecated since symfony/security-core 7.3', self::class), \E_USER_DEPRECATED);
+        }
     }
 
     public function getAttributes(): array

@@ -16,6 +16,9 @@
 /**
  * TokenInterface is the interface for the user authentication information.
  *
+ * The __serialize/__unserialize() magic methods can be implemented on the token
+ * class to prevent sensitive credentials from being put in the session storage.
+ *
  * @author Fabien Potencier <fabien@symfony.com>
  * @author Johannes M. Schmitt <schmittjoh@gmail.com>
  */
@@ -57,8 +60,7 @@ public function setUser(UserInterface $user): void;
     /**
      * Removes sensitive information from the token.
      *
-     * @deprecated since Symfony 7.3, use a dedicated DTO instead or implement your
-     *             own erasing logic instead
+     * @deprecated since Symfony 7.3; erase credentials using the "__serialize()" method instead
      */
     public function eraseCredentials(): void;
 

@@ -8,7 +8,7 @@ CHANGELOG
    For example, users not currently logged in, or while processing a message from a message queue.
  * Add `OfflineTokenInterface` to mark tokens that do not represent the currently logged-in user
  * Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`,
-   use a dedicated DTO or erase credentials on your own e.g. upon `AuthenticationTokenCreatedEvent` instead
+   erase credentials e.g. using `__serialize()` instead
 
 7.2
 ---

@@ -119,6 +119,7 @@ public function getUserIdentifier(): string
     {
     }
 
+    #[\Deprecated]
     public function eraseCredentials(): void
     {
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -45,6 +45,7 @@ public function getUserIdentifier(): string @@
             return $this->name;
         }
+        #[\Deprecated]
         public function eraseCredentials(): void
         {
         }
@@ Expand Down @@