From 6909ec9e452dae9813ed6ecce651bfdf03d5fe20 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 21 May 2024 11:22:57 +0200 Subject: [PATCH] [Security] Deprecate argument $secret of RememberMeToken and RememberMeAuthenticator --- UPGRADE-7.2.md | 14 +++++++++++ .../Security/Factory/RememberMeFactory.php | 2 +- .../security_authenticator_remember_me.php | 1 - .../Bundle/SecurityBundle/composer.json | 4 ++-- .../Authentication/Token/RememberMeToken.php | 20 +++++++++------- .../Component/Security/Core/CHANGELOG.md | 4 ++++ .../AuthenticationTrustResolverTest.php | 2 +- .../Token/RememberMeTokenTest.php | 18 +++++++------- .../Authorization/ExpressionLanguageTest.php | 2 +- .../Voter/AuthenticatedVoterTest.php | 2 +- .../Component/Security/Core/composer.json | 1 + .../Authenticator/RememberMeAuthenticator.php | 24 ++++++++++++++----- .../Component/Security/Http/CHANGELOG.md | 5 ++++ .../RememberMeAuthenticatorTest.php | 2 +- .../Component/Security/Http/composer.json | 2 +- 15 files changed, 71 insertions(+), 32 deletions(-) create mode 100644 UPGRADE-7.2.md diff --git a/UPGRADE-7.2.md b/UPGRADE-7.2.md new file mode 100644 index 0000000000000..9395804d14bac --- /dev/null +++ b/UPGRADE-7.2.md @@ -0,0 +1,14 @@ +UPGRADE FROM 7.1 to 7.2 +======================= + +Symfony 7.2 is a minor release. According to the Symfony release process, there should be no significant +backward compatibility breaks. Minor backward compatibility breaks are prefixed in this document with +`[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.2/setup/upgrade_minor.html). + +If you're upgrading from a version below 7.1, follow the [7.1 upgrade guide](UPGRADE-7.1.md) first. + +Security +-------- + + * Deprecate argument `$secret` of `RememberMeToken` and `RememberMeAuthenticator` diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php index d474e96c16016..a5ba19e98aa84 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php @@ -107,7 +107,7 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal $container ->setDefinition($authenticatorId, new ChildDefinition('security.authenticator.remember_me')) ->replaceArgument(0, new Reference($rememberMeHandlerId)) - ->replaceArgument(3, $config['name'] ?? $this->options['name']) + ->replaceArgument(2, $config['name'] ?? $this->options['name']) ; return $authenticatorId; diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_remember_me.php b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_remember_me.php index b861d0de4199e..ecf93e5061732 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_remember_me.php +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_remember_me.php @@ -85,7 +85,6 @@ ->abstract() ->args([ abstract_arg('remember me handler'), - param('kernel.secret'), service('security.token_storage'), abstract_arg('options'), service('logger')->nullOnInvalid(), diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json index b335e73e07a9d..ca85d48602c14 100644 --- a/src/Symfony/Bundle/SecurityBundle/composer.json +++ b/src/Symfony/Bundle/SecurityBundle/composer.json @@ -26,9 +26,9 @@ "symfony/http-kernel": "^6.4|^7.0", "symfony/http-foundation": "^6.4|^7.0", "symfony/password-hasher": "^6.4|^7.0", - "symfony/security-core": "^6.4|^7.0", + "symfony/security-core": "^7.2", "symfony/security-csrf": "^6.4|^7.0", - "symfony/security-http": "^7.1", + "symfony/security-http": "^7.2", "symfony/service-contracts": "^2.5|^3" }, "require-dev": { diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php index ad218f1b3de7d..643c40d466888 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php @@ -21,20 +21,19 @@ */ class RememberMeToken extends AbstractToken { - private string $secret; + private ?string $secret = null; private string $firewallName; /** - * @param string $secret A secret used to make sure the token is created by the app and not by a malicious client - * * @throws \InvalidArgumentException */ - public function __construct(UserInterface $user, string $firewallName, #[\SensitiveParameter] string $secret) + public function __construct(UserInterface $user, string $firewallName) { parent::__construct($user->getRoles()); - if (!$secret) { - throw new InvalidArgumentException('A non-empty secret is required.'); + if (\func_num_args() > 2) { + trigger_deprecation('symfony/security-core', '7.2', 'The "$secret" argument of "%s()" is deprecated.', __METHOD__); + $this->secret = func_get_arg(2); } if (!$firewallName) { @@ -42,7 +41,6 @@ public function __construct(UserInterface $user, string $firewallName, #[\Sensit } $this->firewallName = $firewallName; - $this->secret = $secret; $this->setUser($user); } @@ -52,13 +50,19 @@ public function getFirewallName(): string return $this->firewallName; } + /** + * @deprecated since Symfony 7.2 + */ public function getSecret(): string { - return $this->secret; + trigger_deprecation('symfony/security-core', '7.2', 'The "%s()" method is deprecated.', __METHOD__); + + return $this->secret ??= base64_encode(random_bytes(8)); } public function __serialize(): array { + // $this->firewallName should be kept at index 1 for compatibility with payloads generated before Symfony 8 return [$this->secret, $this->firewallName, parent::__serialize()]; } diff --git a/src/Symfony/Component/Security/Core/CHANGELOG.md b/src/Symfony/Component/Security/Core/CHANGELOG.md index 47b4a21082738..208f0d4854305 100644 --- a/src/Symfony/Component/Security/Core/CHANGELOG.md +++ b/src/Symfony/Component/Security/Core/CHANGELOG.md @@ -1,6 +1,10 @@ CHANGELOG ========= +7.2 +--- + + * Deprecate argument `$secret` of `RememberMeToken` 7.0 --- diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationTrustResolverTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationTrustResolverTest.php index 3e0a8d50955fb..fc559983afe8f 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationTrustResolverTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationTrustResolverTest.php @@ -72,7 +72,7 @@ protected function getRememberMeToken() { $user = new InMemoryUser('wouter', '', ['ROLE_USER']); - return new RememberMeToken($user, 'main', 'secret'); + return new RememberMeToken($user, 'main'); } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/RememberMeTokenTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/RememberMeTokenTest.php index a63d481b97022..b0cdbaf18c657 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/RememberMeTokenTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/RememberMeTokenTest.php @@ -20,22 +20,22 @@ class RememberMeTokenTest extends TestCase public function testConstructor() { $user = $this->getUser(); - $token = new RememberMeToken($user, 'fookey', 'foo'); + $token = new RememberMeToken($user, 'fookey'); $this->assertEquals('fookey', $token->getFirewallName()); - $this->assertEquals('foo', $token->getSecret()); $this->assertEquals(['ROLE_FOO'], $token->getRoleNames()); $this->assertSame($user, $token->getUser()); } - public function testConstructorSecretCannotBeEmptyString() + /** + * @group legacy + */ + public function testSecret() { - $this->expectException(\InvalidArgumentException::class); - new RememberMeToken( - $this->getUser(), - '', - '' - ); + $user = $this->getUser(); + $token = new RememberMeToken($user, 'fookey', 'foo'); + + $this->assertEquals('foo', $token->getSecret()); } protected function getUser($roles = ['ROLE_FOO']) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/ExpressionLanguageTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/ExpressionLanguageTest.php index 8cc4810a09a16..1a4db41e7b39a 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/ExpressionLanguageTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/ExpressionLanguageTest.php @@ -50,7 +50,7 @@ public static function provider() $user = new InMemoryUser('username', 'password', $roles); $noToken = null; - $rememberMeToken = new RememberMeToken($user, 'firewall-name', 'firewall'); + $rememberMeToken = new RememberMeToken($user, 'firewall-name'); $usernamePasswordToken = new UsernamePasswordToken($user, 'firewall-name', $roles); return [ diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php index 88544c081f78c..3a3b9d4e7efff 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -101,7 +101,7 @@ public function getCredentials() } if ('remembered' === $authenticated) { - return new RememberMeToken($user, 'foo', 'bar'); + return new RememberMeToken($user, 'foo'); } if ('impersonated' === $authenticated) { diff --git a/src/Symfony/Component/Security/Core/composer.json b/src/Symfony/Component/Security/Core/composer.json index a923768be51da..0aaff1e3645bf 100644 --- a/src/Symfony/Component/Security/Core/composer.json +++ b/src/Symfony/Component/Security/Core/composer.json @@ -17,6 +17,7 @@ ], "require": { "php": ">=8.2", + "symfony/deprecation-contracts": "^2.5|^3", "symfony/event-dispatcher-contracts": "^2.5|^3", "symfony/service-contracts": "^2.5|^3", "symfony/password-hasher": "^6.4|^7.0" diff --git a/src/Symfony/Component/Security/Http/Authenticator/RememberMeAuthenticator.php b/src/Symfony/Component/Security/Http/Authenticator/RememberMeAuthenticator.php index 0e6db6a12f1b6..9cd6fcba4fbf9 100644 --- a/src/Symfony/Component/Security/Http/Authenticator/RememberMeAuthenticator.php +++ b/src/Symfony/Component/Security/Http/Authenticator/RememberMeAuthenticator.php @@ -19,7 +19,6 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\CookieTheftException; -use Symfony\Component\Security\Core\Exception\InvalidArgumentException; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\Exception\UserNotFoundException; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; @@ -50,14 +49,23 @@ class RememberMeAuthenticator implements InteractiveAuthenticatorInterface private string $cookieName; private ?LoggerInterface $logger; - public function __construct(RememberMeHandlerInterface $rememberMeHandler, #[\SensitiveParameter] string $secret, TokenStorageInterface $tokenStorage, string $cookieName, ?LoggerInterface $logger = null) + /** + * @param TokenStorageInterface $tokenStorage + * @param string $cookieName + * @param ?LoggerInterface $logger + */ + public function __construct(RememberMeHandlerInterface $rememberMeHandler, #[\SensitiveParameter] TokenStorageInterface|string $tokenStorage, string|TokenStorageInterface $cookieName, LoggerInterface|string|null $logger = null) { - if (!$secret) { - throw new InvalidArgumentException('A non-empty secret is required.'); + if (\is_string($tokenStorage)) { + trigger_deprecation('symfony/security-core', '7.2', 'The "$secret" argument of "%s()" is deprecated.', __METHOD__); + + $this->secret = $tokenStorage; + $tokenStorage = $cookieName; + $cookieName = $logger; + $logger = \func_num_args() > 4 ? func_get_arg(4) : null; } $this->rememberMeHandler = $rememberMeHandler; - $this->secret = $secret; $this->tokenStorage = $tokenStorage; $this->cookieName = $cookieName; $this->logger = $logger; @@ -99,7 +107,11 @@ public function authenticate(Request $request): Passport public function createToken(Passport $passport, string $firewallName): TokenInterface { - return new RememberMeToken($passport->getUser(), $firewallName, $this->secret); + if (isset($this->secret)) { + return new RememberMeToken($passport->getUser(), $firewallName, $this->secret); + } + + return new RememberMeToken($passport->getUser(), $firewallName); } public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response diff --git a/src/Symfony/Component/Security/Http/CHANGELOG.md b/src/Symfony/Component/Security/Http/CHANGELOG.md index a8e710597a989..b3d38d924e7d1 100644 --- a/src/Symfony/Component/Security/Http/CHANGELOG.md +++ b/src/Symfony/Component/Security/Http/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +7.2 +--- + + * Deprecate argument `$secret` of `RememberMeAuthenticator` + 7.1 --- diff --git a/src/Symfony/Component/Security/Http/Tests/Authenticator/RememberMeAuthenticatorTest.php b/src/Symfony/Component/Security/Http/Tests/Authenticator/RememberMeAuthenticatorTest.php index 2c8e70585072d..fe262c22dd863 100644 --- a/src/Symfony/Component/Security/Http/Tests/Authenticator/RememberMeAuthenticatorTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Authenticator/RememberMeAuthenticatorTest.php @@ -34,7 +34,7 @@ protected function setUp(): void { $this->rememberMeHandler = $this->createMock(RememberMeHandlerInterface::class); $this->tokenStorage = new TokenStorage(); - $this->authenticator = new RememberMeAuthenticator($this->rememberMeHandler, 's3cr3t', $this->tokenStorage, '_remember_me_cookie'); + $this->authenticator = new RememberMeAuthenticator($this->rememberMeHandler, $this->tokenStorage, '_remember_me_cookie'); } public function testSupportsTokenStorageWithToken() diff --git a/src/Symfony/Component/Security/Http/composer.json b/src/Symfony/Component/Security/Http/composer.json index 7431b3c5694ee..fc0b447b60363 100644 --- a/src/Symfony/Component/Security/Http/composer.json +++ b/src/Symfony/Component/Security/Http/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": "^6.4|^7.0", + "symfony/security-core": "^7.2", "symfony/service-contracts": "^2.5|^3" }, "require-dev": {