[Security] Deprecate UserInterface & TokenInterface's eraseCredentials()

Author: chalasr

Authentication/AuthenticatorManager.php

@@ -69,6 +69,10 @@ public function __construct(
         }
 
         $this->exposeSecurityErrors = $exposeSecurityErrors;
+
+        if ($eraseCredentials) {
+            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__));
+        }
     }
 
     /**
@@ -208,6 +212,7 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req
             // announce the authentication token
             $authenticatedToken = $this->eventDispatcher->dispatch(new AuthenticationTokenCreatedEvent($authenticatedToken, $passport))->getAuthenticatedToken();
 
+            // @deprecated since Symfony 7.3, remove the if statement in 8.0
             if (true === $this->eraseCredentials) {
                 $authenticatedToken->eraseCredentials();
             }

CHANGELOG.md

@@ -6,6 +6,8 @@ CHANGELOG
 
  * Add encryption support to `OidcTokenHandler` (JWE)
  * Replace `$hideAccountStatusExceptions` argument with `$exposeSecurityErrors` in `AuthenticatorManager` constructor
+ * Deprecate passing `true` for the `$eraseCredentials` parameter of `AuthenticatorManager::__construct()`, erase credentials
+  on your own e.g. upon `AuthenticationTokenCreatedEvent` instead. Passing it won't thave any effect in 8.0.
 
 7.2
 ---

Tests/Authentication/AuthenticatorManagerTest.php

@@ -15,6 +15,7 @@
 use PHPUnit\Framework\TestCase;
 use Psr\Log\AbstractLogger;
 use Psr\Log\LoggerInterface;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -40,6 +41,8 @@
 
 class AuthenticatorManagerTest extends TestCase
 {
+    use ExpectDeprecationTrait;
+
     private MockObject&TokenStorageInterface $tokenStorage;
     private EventDispatcher $eventDispatcher;
     private Request $request;
@@ -163,7 +166,7 @@ public function testRequiredBadgeMissing()
 
         $authenticator->expects($this->once())->method('onAuthenticationFailure')->with($this->anything(), $this->callback(fn ($exception) => 'Authentication failed; Some badges marked as required by the firewall config are not available on the passport: "'.CsrfTokenBadge::class.'".' === $exception->getMessage()));
 
-        $manager = $this->createManager([$authenticator], 'main', true, [CsrfTokenBadge::class], exposeSecurityErrors: ExposeSecurityLevel::None);
+        $manager = $this->createManager([$authenticator], 'main', false, [CsrfTokenBadge::class], exposeSecurityErrors: ExposeSecurityLevel::None);
         $manager->authenticateRequest($this->request);
     }
 
@@ -179,11 +182,12 @@ public function testAllRequiredBadgesPresent()
 
         $authenticator->expects($this->once())->method('onAuthenticationSuccess');
 
-        $manager = $this->createManager([$authenticator], 'main', true, [CsrfTokenBadge::class], exposeSecurityErrors: ExposeSecurityLevel::None);
+        $manager = $this->createManager([$authenticator], 'main', false, [CsrfTokenBadge::class], exposeSecurityErrors: ExposeSecurityLevel::None);
         $manager->authenticateRequest($this->request);
     }
 
     /**
+     * @group legacy
      * @dataProvider provideEraseCredentialsData
      */
     public function testEraseCredentials($eraseCredentials)
@@ -197,6 +201,10 @@ public function testEraseCredentials($eraseCredentials)
 
         $this->token->expects($eraseCredentials ? $this->once() : $this->never())->method('eraseCredentials');
 
+        if ($eraseCredentials) {
+            $this->expectDeprecation('Since symfony/security-http 7.3: Passing true as "$eraseCredentials" argument to "Symfony\Component\Security\Http\Authentication\AuthenticatorManager::__construct()" is deprecated and won\'t have any effect in 8.0, pass "false" instead and use your own erasing logic if needed.');
+        }
+
         $manager = $this->createManager([$authenticator], 'main', $eraseCredentials, exposeSecurityErrors: ExposeSecurityLevel::None);
         $manager->authenticateRequest($this->request);
     }
@@ -403,7 +411,7 @@ public function log($level, $message, array $context = []): void
             }
         };
 
-        $manager = $this->createManager([$authenticator], 'main', true, [], $logger, exposeSecurityErrors: ExposeSecurityLevel::None);
+        $manager = $this->createManager([$authenticator], 'main', false, [], $logger, exposeSecurityErrors: ExposeSecurityLevel::None);
         $response = $manager->authenticateRequest($this->request);
         $this->assertSame($this->response, $response);
         $this->assertStringContainsString($authenticator::class, $logger->logContexts[0]['authenticator']);
@@ -422,7 +430,7 @@ private static function createDummySupportsAuthenticator(?bool $supports = true)
         return new DummySupportsAuthenticator($supports);
     }
 
-    private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = true, array $requiredBadges = [], ?LoggerInterface $logger = null, ExposeSecurityLevel $exposeSecurityErrors = ExposeSecurityLevel::AccountStatus)
+    private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = false, array $requiredBadges = [], ?LoggerInterface $logger = null, ExposeSecurityLevel $exposeSecurityErrors = ExposeSecurityLevel::AccountStatus)
     {
         return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, $logger, $eraseCredentials, $exposeSecurityErrors, $requiredBadges);
     }

composer.json

@@ -22,7 +22,7 @@
         "symfony/http-kernel": "^6.4|^7.0",
         "symfony/polyfill-mbstring": "~1.0",
         "symfony/property-access": "^6.4|^7.0",
-        "symfony/security-core": "^7.2",
+        "symfony/security-core": "^7.3",
         "symfony/service-contracts": "^2.5|^3"
     },
     "require-dev": {