From 33b8fbdd8a5bb9533165c791d2585116f4c7b82d Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Sat, 10 Jul 2021 18:27:35 +0200 Subject: [PATCH] [Security] Deprecate `TokenInterface::isAuthenticated()` and `setAuthenticated()` --- UPGRADE-5.4.md | 3 + UPGRADE-6.0.md | 3 + .../Processor/AbstractTokenProcessor.php | 2 +- .../Tests/Processor/TokenProcessorTest.php | 2 - .../Bundle/FrameworkBundle/KernelBrowser.php | 2 +- .../DataCollector/SecurityDataCollector.php | 2 +- .../Authentication/Token/AbstractToken.php | 23 +++++- .../Authentication/Token/AnonymousToken.php | 3 +- .../Core/Authentication/Token/NullToken.php | 10 +++ .../Token/PreAuthenticatedToken.php | 2 +- .../Authentication/Token/RememberMeToken.php | 4 +- .../Authentication/Token/TokenInterface.php | 4 + .../Token/UsernamePasswordToken.php | 4 +- .../Authorization/AuthorizationChecker.php | 7 +- .../Component/Security/Core/CHANGELOG.md | 2 + .../Token/AbstractTokenTest.php | 9 +++ .../Token/AnonymousTokenTest.php | 12 ++- .../Token/PreAuthenticatedTokenTest.php | 13 +++- .../Token/RememberMeTokenTest.php | 9 +++ .../Token/SwitchUserTokenTest.php | 6 +- .../Token/UsernamePasswordTokenTest.php | 18 ++++- .../AuthorizationCheckerTest.php | 3 + .../Provider/GuardAuthenticationProvider.php | 2 +- .../Token/PostAuthenticationGuardToken.php | 2 +- .../Token/PreAuthenticationGuardToken.php | 5 +- .../Token/PostAuthenticationToken.php | 4 +- .../Component/Security/Http/CHANGELOG.md | 1 + .../Http/Event/DeauthenticatedEvent.php | 12 ++- .../Security/Http/Firewall/AccessListener.php | 4 +- .../Firewall/BasicAuthenticationListener.php | 2 +- .../Http/Firewall/ContextListener.php | 75 +++++++++++++++++-- .../Tests/Firewall/AccessListenerTest.php | 20 +++-- .../Tests/Firewall/ContextListenerTest.php | 3 + 33 files changed, 228 insertions(+), 45 deletions(-) diff --git a/UPGRADE-5.4.md b/UPGRADE-5.4.md index ade247d78daa1..faa832634a27a 100644 --- a/UPGRADE-5.4.md +++ b/UPGRADE-5.4.md @@ -30,3 +30,6 @@ Security behavior when using `enable_authenticator_manager: true`) * Deprecate not setting the 5th argument (`$exceptionOnNoToken`) of `AccessListener` to `false` (this is the default behavior when using `enable_authenticator_manager: true`) + * Deprecate `TokenInterface:isAuthenticated()` and `setAuthenticated()` methods without replacement. + Security tokens won't have an "authenticated" flag anymore, so they will always be considered authenticated + * Deprecate `DeauthenticatedEvent`, use `TokenDeauthenticatedEvent` instead diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index ca9e58fe4f6fe..d7de1c1323dd7 100644 --- a/UPGRADE-6.0.md +++ b/UPGRADE-6.0.md @@ -316,6 +316,9 @@ Security `UsernamePasswordFormAuthenticationListener`, `UsernamePasswordJsonAuthenticationListener` and `X509AuthenticationListener` from security-http, use the new authenticator system instead * Remove the Guard component, use the new authenticator system instead + * Remove `TokenInterface:isAuthenticated()` and `setAuthenticated()` methods without replacement. + Security tokens won't have an "authenticated" flag anymore, so they will always be considered authenticated + * Remove `DeauthenticatedEvent`, use `TokenDeauthenticatedEvent` instead SecurityBundle -------------- diff --git a/src/Symfony/Bridge/Monolog/Processor/AbstractTokenProcessor.php b/src/Symfony/Bridge/Monolog/Processor/AbstractTokenProcessor.php index 15919978857c3..11aceacf2513f 100644 --- a/src/Symfony/Bridge/Monolog/Processor/AbstractTokenProcessor.php +++ b/src/Symfony/Bridge/Monolog/Processor/AbstractTokenProcessor.php @@ -42,7 +42,7 @@ public function __invoke(array $record): array if (null !== $token = $this->getToken()) { $record['extra'][$this->getKey()] = [ - 'authenticated' => $token->isAuthenticated(), + 'authenticated' => $token->isAuthenticated(false), // @deprecated since Symfony 5.4, always true in 6.0 'roles' => $token->getRoleNames(), ]; diff --git a/src/Symfony/Bridge/Monolog/Tests/Processor/TokenProcessorTest.php b/src/Symfony/Bridge/Monolog/Tests/Processor/TokenProcessorTest.php index b9fa51658e0d4..d36e24772f225 100644 --- a/src/Symfony/Bridge/Monolog/Tests/Processor/TokenProcessorTest.php +++ b/src/Symfony/Bridge/Monolog/Tests/Processor/TokenProcessorTest.php @@ -39,7 +39,6 @@ public function testLegacyProcessor() $this->assertArrayHasKey('token', $record['extra']); $this->assertEquals($token->getUsername(), $record['extra']['token']['username']); - $this->assertEquals($token->isAuthenticated(), $record['extra']['token']['authenticated']); $this->assertEquals(['ROLE_USER'], $record['extra']['token']['roles']); } @@ -59,7 +58,6 @@ public function testProcessor() $this->assertArrayHasKey('token', $record['extra']); $this->assertEquals($token->getUserIdentifier(), $record['extra']['token']['user_identifier']); - $this->assertEquals($token->isAuthenticated(), $record['extra']['token']['authenticated']); $this->assertEquals(['ROLE_USER'], $record['extra']['token']['roles']); } } diff --git a/src/Symfony/Bundle/FrameworkBundle/KernelBrowser.php b/src/Symfony/Bundle/FrameworkBundle/KernelBrowser.php index b6231535b48d2..c889d9c6803e6 100644 --- a/src/Symfony/Bundle/FrameworkBundle/KernelBrowser.php +++ b/src/Symfony/Bundle/FrameworkBundle/KernelBrowser.php @@ -123,7 +123,7 @@ public function loginUser(object $user, string $firewallContext = 'main'): self } $token = new TestBrowserToken($user->getRoles(), $user, $firewallContext); - $token->setAuthenticated(true); + $token->setAuthenticated(true, false); $container = $this->getContainer(); $container->get('security.untracked_token_storage')->setToken($token); diff --git a/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php b/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php index 45447f50e82b5..7d4ee474096b4 100644 --- a/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php +++ b/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php @@ -123,7 +123,7 @@ public function collect(Request $request, Response $response, \Throwable $except $this->data = [ 'enabled' => true, - 'authenticated' => $token->isAuthenticated(), + 'authenticated' => $token->isAuthenticated(false), 'impersonated' => null !== $impersonatorUser, 'impersonator_user' => $impersonatorUser, 'impersonation_exit_path' => null, diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php index 0363e18428a37..bde564330398d 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php @@ -99,7 +99,12 @@ public function setUser($user) throw new \InvalidArgumentException('$user must be an instanceof UserInterface, an object implementing a __toString method, or a primitive string.'); } - if (null === $this->user) { + // @deprecated since Symfony 5.4, remove the whole block if/elseif/else block in 6.0 + if (1 < \func_num_args() && !func_get_arg(1)) { + // ContextListener checks if the user has changed on its own and calls `setAuthenticated()` subsequently, + // avoid doing the same checks twice + $changed = false; + } elseif (null === $this->user) { $changed = false; } elseif ($this->user instanceof UserInterface) { if (!$user instanceof UserInterface) { @@ -113,8 +118,9 @@ public function setUser($user) $changed = (string) $this->user !== (string) $user; } + // @deprecated since Symfony 5.4 if ($changed) { - $this->setAuthenticated(false); + $this->setAuthenticated(false, false); } $this->user = $user; @@ -122,9 +128,15 @@ public function setUser($user) /** * {@inheritdoc} + * + * @deprecated since Symfony 5.4 */ public function isAuthenticated() { + if (1 > \func_num_args() || func_get_arg(0)) { + trigger_deprecation('symfony/security-core', '5.4', 'Method "%s()" is deprecated. In version 6.0, security tokens won\'t have an "authenticated" flag anymore and will always be considered authenticated.', __METHOD__); + } + return $this->authenticated; } @@ -133,6 +145,10 @@ public function isAuthenticated() */ public function setAuthenticated(bool $authenticated) { + if (2 > \func_num_args() || func_get_arg(1)) { + trigger_deprecation('symfony/security-core', '5.4', 'Method "%s()" is deprecated. In version 6.0, security tokens won\'t have an "authenticated" state anymore and will always be considered as authenticated.', __METHOD__); + } + $this->authenticated = $authenticated; } @@ -275,6 +291,9 @@ final public function unserialize($serialized) $this->__unserialize(\is_array($serialized) ? $serialized : unserialize($serialized)); } + /** + * @deprecated since Symfony 5.4 + */ private function hasUserChanged(UserInterface $user): bool { if (!($this->user instanceof UserInterface)) { diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AnonymousToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AnonymousToken.php index db94766d3f166..a7acde46acaed 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/AnonymousToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/AnonymousToken.php @@ -33,7 +33,8 @@ public function __construct(string $secret, $user, array $roles = []) $this->secret = $secret; $this->setUser($user); - $this->setAuthenticated(true); + // @deprecated since Symfony 5.4 + $this->setAuthenticated(true, false); } /** diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php index 28c77d7573684..bcc1db2b9d527 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php @@ -53,11 +53,21 @@ public function getUserIdentifier(): string return ''; } + /** + * @deprecated since Symfony 5.4 + */ public function isAuthenticated() { + if (0 === \func_num_args() || func_get_arg(0)) { + trigger_deprecation('symfony/security-core', '5.4', 'Method "%s()" is deprecated. In version 6.0, security tokens won\'t have an "authenticated" flag anymore and will always be considered authenticated.', __METHOD__); + } + return true; } + /** + * @deprecated since Symfony 5.4 + */ public function setAuthenticated(bool $isAuthenticated) { throw new \BadMethodCallException('Cannot change authentication state of NullToken.'); diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/PreAuthenticatedToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/PreAuthenticatedToken.php index 95a4d2d780cb0..e31670bf94f4c 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/PreAuthenticatedToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/PreAuthenticatedToken.php @@ -41,7 +41,7 @@ public function __construct($user, $credentials, string $firewallName, array $ro $this->firewallName = $firewallName; if ($roles) { - $this->setAuthenticated(true); + $this->setAuthenticated(true, false); } } diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php index c019195eecb62..e00f557ee1efe 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php @@ -44,7 +44,7 @@ public function __construct(UserInterface $user, string $firewallName, string $s $this->secret = $secret; $this->setUser($user); - parent::setAuthenticated(true); + parent::setAuthenticated(true, false); } /** @@ -56,7 +56,7 @@ public function setAuthenticated(bool $authenticated) throw new \LogicException('You cannot set this token to authenticated after creation.'); } - parent::setAuthenticated(false); + parent::setAuthenticated(false, false); } /** diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/TokenInterface.php b/src/Symfony/Component/Security/Core/Authentication/Token/TokenInterface.php index 82633f92051f3..12680037c810d 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/TokenInterface.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/TokenInterface.php @@ -71,11 +71,15 @@ public function setUser($user); * Returns whether the user is authenticated or not. * * @return bool true if the token has been authenticated, false otherwise + * + * @deprecated since Symfony 5.4. In 6.0, security tokens will always be considered authenticated */ public function isAuthenticated(); /** * Sets the authenticated flag. + * + * @deprecated since Symfony 5.4. In 6.0, security tokens will always be considered authenticated */ public function setAuthenticated(bool $isAuthenticated); diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/UsernamePasswordToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/UsernamePasswordToken.php index 8228f6773955d..1dd87ca1016f2 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/UsernamePasswordToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/UsernamePasswordToken.php @@ -42,7 +42,7 @@ public function __construct($user, $credentials, string $firewallName, array $ro $this->credentials = $credentials; $this->firewallName = $firewallName; - parent::setAuthenticated(\count($roles) > 0); + parent::setAuthenticated(\count($roles) > 0, false); } /** @@ -54,7 +54,7 @@ public function setAuthenticated(bool $isAuthenticated) throw new \LogicException('Cannot set this token to trusted after instantiation.'); } - parent::setAuthenticated(false); + parent::setAuthenticated(false, false); } /** diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php index ef678b547a259..3557f04b8a571 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php @@ -62,7 +62,12 @@ final public function isGranted($attribute, $subject = null): bool $token = new NullToken(); } else { - if ($this->alwaysAuthenticate || !$token->isAuthenticated()) { + $authenticated = true; + // @deprecated since Symfony 5.4 + if ($this->alwaysAuthenticate || !$authenticated = $token->isAuthenticated(false)) { + if (!($authenticated ?? true)) { + trigger_deprecation('symfony/core', '5.4', 'Returning false from "%s()" is deprecated and won\'t have any effect in Symfony 6.0 as security tokens will always be considered authenticated.'); + } $this->tokenStorage->setToken($token = $this->authenticationManager->authenticate($token)); } } diff --git a/src/Symfony/Component/Security/Core/CHANGELOG.md b/src/Symfony/Component/Security/Core/CHANGELOG.md index 459ae577342e8..28de957c84316 100644 --- a/src/Symfony/Component/Security/Core/CHANGELOG.md +++ b/src/Symfony/Component/Security/Core/CHANGELOG.md @@ -6,6 +6,8 @@ CHANGELOG * Deprecate setting the 4th argument (`$alwaysAuthenticate`) to `true` and not setting the 5th argument (`$exceptionOnNoToken`) to `false` of `AuthorizationChecker` + * Deprecate methods `TokenInterface::isAuthenticated()` and `setAuthenticated`, + tokens will always be considered authenticated in 6.0 5.3 --- diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php index 5e99ebba65c5c..723aa097a8450 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php @@ -41,6 +41,7 @@ public function getUsername() public function getRoles() { + return []; } public function getPassword() @@ -104,6 +105,9 @@ public function testConstructor() $this->assertEquals(['ROLE_FOO'], $token->getRoleNames()); } + /** + * @group legacy + */ public function testAuthenticatedFlag() { $token = new ConcreteToken(); @@ -158,6 +162,7 @@ public function getUsers() } /** + * @group legacy * @dataProvider getUserChanges */ public function testSetUserSetsAuthenticatedToFalseWhenUserChanges($firstUser, $secondUser) @@ -190,6 +195,7 @@ public function getUserChanges() } /** + * @group legacy * @dataProvider getUsers */ public function testSetUserDoesNotSetAuthenticatedToFalseWhenUserDoesNotChange($user) @@ -205,6 +211,9 @@ public function testSetUserDoesNotSetAuthenticatedToFalseWhenUserDoesNotChange($ $this->assertTrue($token->isAuthenticated()); } + /** + * @group legacy + */ public function testIsUserChangedWhenSerializing() { $token = new ConcreteToken(['ROLE_ADMIN']); diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AnonymousTokenTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AnonymousTokenTest.php index 1b00fd4e76aec..d1acf9477ffae 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AnonymousTokenTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AnonymousTokenTest.php @@ -18,13 +18,19 @@ class AnonymousTokenTest extends TestCase { public function testConstructor() { - $token = new AnonymousToken('foo', 'bar'); - $this->assertTrue($token->isAuthenticated()); - $token = new AnonymousToken('foo', 'bar', ['ROLE_FOO']); $this->assertEquals(['ROLE_FOO'], $token->getRoleNames()); } + /** + * @group legacy + */ + public function testIsAuthenticated() + { + $token = new AnonymousToken('foo', 'bar'); + $this->assertTrue($token->isAuthenticated()); + } + public function testGetKey() { $token = new AnonymousToken('foo', 'bar'); diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/PreAuthenticatedTokenTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/PreAuthenticatedTokenTest.php index 3f38948716cdf..acd407aa818c2 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/PreAuthenticatedTokenTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/PreAuthenticatedTokenTest.php @@ -18,11 +18,7 @@ class PreAuthenticatedTokenTest extends TestCase { public function testConstructor() { - $token = new PreAuthenticatedToken('foo', 'bar', 'key'); - $this->assertFalse($token->isAuthenticated()); - $token = new PreAuthenticatedToken('foo', 'bar', 'key', ['ROLE_FOO']); - $this->assertTrue($token->isAuthenticated()); $this->assertEquals(['ROLE_FOO'], $token->getRoleNames()); $this->assertEquals('key', $token->getFirewallName()); } @@ -45,4 +41,13 @@ public function testEraseCredentials() $token->eraseCredentials(); $this->assertEquals('', $token->getCredentials()); } + + /** + * @group legacy + */ + public function testIsAuthenticated() + { + $token = new PreAuthenticatedToken('foo', 'bar', 'key'); + $this->assertFalse($token->isAuthenticated()); + } } 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 aee9797ce2c83..42df233712ec9 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/RememberMeTokenTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/RememberMeTokenTest.php @@ -26,6 +26,15 @@ public function testConstructor() $this->assertEquals('foo', $token->getSecret()); $this->assertEquals(['ROLE_FOO'], $token->getRoleNames()); $this->assertSame($user, $token->getUser()); + } + + /** + * @group legacy + */ + public function testIsAuthenticated() + { + $user = $this->getUser(); + $token = new RememberMeToken($user, 'fookey', 'foo'); $this->assertTrue($token->isAuthenticated()); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/SwitchUserTokenTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/SwitchUserTokenTest.php index e605615bc9a19..e31213815fd78 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/SwitchUserTokenTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/SwitchUserTokenTest.php @@ -15,6 +15,7 @@ use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Tests\Authentication\Token\Fixtures\CustomUser; +use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\UserInterface; class SwitchUserTokenTest extends TestCase @@ -42,6 +43,9 @@ public function testSerialize() $this->assertEquals(['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH'], $unserializedOriginalToken->getRoleNames()); } + /** + * @group legacy + */ public function testSetUserDoesNotDeauthenticate() { $impersonated = new class() implements UserInterface { @@ -75,7 +79,7 @@ public function getSalt() } }; - $originalToken = new UsernamePasswordToken('impersonator', 'foo', 'provider-key', ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH']); + $originalToken = new UsernamePasswordToken(new InMemoryUser('impersonator', '', ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH']), 'foo', 'provider-key', ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH']); $token = new SwitchUserToken($impersonated, 'bar', 'provider-key', ['ROLE_USER', 'ROLE_PREVIOUS_ADMIN'], $originalToken); $token->setUser($impersonated); $this->assertTrue($token->isAuthenticated()); diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/UsernamePasswordTokenTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/UsernamePasswordTokenTest.php index 7c6961ce587d8..fe095242b82cf 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/UsernamePasswordTokenTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/UsernamePasswordTokenTest.php @@ -17,16 +17,27 @@ class UsernamePasswordTokenTest extends TestCase { public function testConstructor() + { + $token = new UsernamePasswordToken('foo', 'bar', 'key', ['ROLE_FOO']); + $this->assertEquals(['ROLE_FOO'], $token->getRoleNames()); + $this->assertEquals('key', $token->getFirewallName()); + } + + /** + * @group legacy + */ + public function testIsAuthenticated() { $token = new UsernamePasswordToken('foo', 'bar', 'key'); $this->assertFalse($token->isAuthenticated()); $token = new UsernamePasswordToken('foo', 'bar', 'key', ['ROLE_FOO']); - $this->assertEquals(['ROLE_FOO'], $token->getRoleNames()); $this->assertTrue($token->isAuthenticated()); - $this->assertEquals('key', $token->getFirewallName()); } + /** + * @group legacy + */ public function testSetAuthenticatedToTrue() { $this->expectException(\LogicException::class); @@ -34,6 +45,9 @@ public function testSetAuthenticatedToTrue() $token->setAuthenticated(true); } + /** + * @group legacy + */ public function testSetAuthenticatedToFalse() { $token = new UsernamePasswordToken('foo', 'bar', 'key'); diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index 38554de0a1c9a..3ac50c7fb45ab 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -42,6 +42,9 @@ protected function setUp(): void ); } + /** + * @group legacy + */ public function testVoteAuthenticatesTokenIfNecessary() { $token = new UsernamePasswordToken('username', 'password', 'provider'); diff --git a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php index e84a4c6a42ada..4666d132f5a5d 100644 --- a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php @@ -93,7 +93,7 @@ public function authenticate(TokenInterface $token) // this should never happen - but technically, the token is // authenticated... so it could just be returned - if ($token->isAuthenticated()) { + if ($token->isAuthenticated(false)) { return $token; } diff --git a/src/Symfony/Component/Security/Guard/Token/PostAuthenticationGuardToken.php b/src/Symfony/Component/Security/Guard/Token/PostAuthenticationGuardToken.php index 494d1d20960bc..a9050a4d66041 100644 --- a/src/Symfony/Component/Security/Guard/Token/PostAuthenticationGuardToken.php +++ b/src/Symfony/Component/Security/Guard/Token/PostAuthenticationGuardToken.php @@ -49,7 +49,7 @@ public function __construct(UserInterface $user, string $providerKey, array $rol // this token is meant to be used after authentication success, so it is always authenticated // you could set it as non authenticated later if you need to - $this->setAuthenticated(true); + $this->setAuthenticated(true, false); } /** diff --git a/src/Symfony/Component/Security/Guard/Token/PreAuthenticationGuardToken.php b/src/Symfony/Component/Security/Guard/Token/PreAuthenticationGuardToken.php index d9738f049b491..28483bea999ae 100644 --- a/src/Symfony/Component/Security/Guard/Token/PreAuthenticationGuardToken.php +++ b/src/Symfony/Component/Security/Guard/Token/PreAuthenticationGuardToken.php @@ -42,7 +42,7 @@ public function __construct($credentials, string $guardProviderKey) parent::__construct([]); - // never authenticated + // @deprecated since Symfony 5.4 parent::setAuthenticated(false); } @@ -62,6 +62,9 @@ public function getCredentials() return $this->credentials; } + /** + * @deprecated since Symfony 5.4 + */ public function setAuthenticated(bool $authenticated) { throw new \LogicException('The PreAuthenticationGuardToken is *never* authenticated.'); diff --git a/src/Symfony/Component/Security/Http/Authenticator/Token/PostAuthenticationToken.php b/src/Symfony/Component/Security/Http/Authenticator/Token/PostAuthenticationToken.php index be938a08fac48..554fec09fff8a 100644 --- a/src/Symfony/Component/Security/Http/Authenticator/Token/PostAuthenticationToken.php +++ b/src/Symfony/Component/Security/Http/Authenticator/Token/PostAuthenticationToken.php @@ -34,9 +34,9 @@ public function __construct(UserInterface $user, string $firewallName, array $ro $this->setUser($user); $this->firewallName = $firewallName; + // @deprecated since Symfony 5.4 // this token is meant to be used after authentication success, so it is always authenticated - // you could set it as non authenticated later if you need to - $this->setAuthenticated(true); + $this->setAuthenticated(true, false); } /** diff --git a/src/Symfony/Component/Security/Http/CHANGELOG.md b/src/Symfony/Component/Security/Http/CHANGELOG.md index 1bb5da6d0218a..aff91a3f7cccc 100644 --- a/src/Symfony/Component/Security/Http/CHANGELOG.md +++ b/src/Symfony/Component/Security/Http/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG --- * Deprecate not setting the 5th argument (`$exceptionOnNoToken`) of `AccessListener` to `false` + * Deprecate `DeauthenticatedEvent`, use `TokenDeauthenticatedEvent` instead 5.3 --- diff --git a/src/Symfony/Component/Security/Http/Event/DeauthenticatedEvent.php b/src/Symfony/Component/Security/Http/Event/DeauthenticatedEvent.php index cd4e8e01de8ee..f064cceaac56c 100644 --- a/src/Symfony/Component/Security/Http/Event/DeauthenticatedEvent.php +++ b/src/Symfony/Component/Security/Http/Event/DeauthenticatedEvent.php @@ -22,25 +22,35 @@ * a session is deauthenticated. * * @author Hamza Amrouche + * + * @deprecated since Symfony 5.4, use TokenDeauthenticatedEvent instead */ final class DeauthenticatedEvent extends Event { private $originalToken; private $refreshedToken; - public function __construct(TokenInterface $originalToken, TokenInterface $refreshedToken) + public function __construct(TokenInterface $originalToken, TokenInterface $refreshedToken, bool $triggerDeprecation = true) { + if ($triggerDeprecation) { + @trigger_deprecation('symfony/security-http', '5.4', 'Class "%s" is deprecated, use "%s" instead.', __CLASS__, TokenDeauthenticatedEvent::class); + } + $this->originalToken = $originalToken; $this->refreshedToken = $refreshedToken; } public function getRefreshedToken(): TokenInterface { + @trigger_deprecation('symfony/security-http', '5.4', 'Class "%s" is deprecated, use "%s" instead.', __CLASS__, TokenDeauthenticatedEvent::class); + return $this->refreshedToken; } public function getOriginalToken(): TokenInterface { + @trigger_deprecation('symfony/security-http', '5.4', 'Class "%s" is deprecated, use "%s" instead.', __CLASS__, TokenDeauthenticatedEvent::class); + return $this->originalToken; } } diff --git a/src/Symfony/Component/Security/Http/Firewall/AccessListener.php b/src/Symfony/Component/Security/Http/Firewall/AccessListener.php index fdb219a1d61ea..10031f02da488 100644 --- a/src/Symfony/Component/Security/Http/Firewall/AccessListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/AccessListener.php @@ -95,7 +95,9 @@ public function authenticate(RequestEvent $event) $token = new NullToken(); } - if (!$token->isAuthenticated()) { + // @deprecated since Symfony 5.4 + if (!$token->isAuthenticated(false)) { + trigger_deprecation('symfony/core', '5.4', 'Returning false from "%s()" is deprecated and won\'t have any effect in Symfony 6.0 as security tokens will always be considered authenticated.'); $token = $this->authManager->authenticate($token); $this->tokenStorage->setToken($token); } diff --git a/src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php index 9469fa8819c0f..1f675be16c099 100644 --- a/src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php @@ -78,7 +78,7 @@ public function authenticate(RequestEvent $event) if (null !== $token = $this->tokenStorage->getToken()) { // @deprecated since 5.3, change to $token->getUserIdentifier() in 6.0 - if ($token instanceof UsernamePasswordToken && $token->isAuthenticated() && (method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername()) === $username) { + if ($token instanceof UsernamePasswordToken && $token->isAuthenticated(false) && (method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername()) === $username) { return; } } diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php index 25219dfd48eb3..6d756cf6d160e 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php @@ -28,6 +28,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\Exception\UserNotFoundException; +use Symfony\Component\Security\Core\User\EquatableInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Http\Event\DeauthenticatedEvent; @@ -135,6 +136,10 @@ public function authenticate(RequestEvent $event) $token = $this->refreshUser($token); if (!$token) { + if ($this->logger) { + $this->logger->debug('Token was deauthenticated after trying to refresh it.'); + } + if ($this->dispatcher) { $this->dispatcher->dispatch(new TokenDeauthenticatedEvent($originalToken, $request)); } @@ -227,11 +232,13 @@ protected function refreshUser(TokenInterface $token): ?TokenInterface try { $refreshedUser = $provider->refreshUser($user); $newToken = clone $token; - $newToken->setUser($refreshedUser); + $newToken->setUser($refreshedUser, false); // tokens can be deauthenticated if the user has been changed. - if (!$newToken->isAuthenticated()) { + if ($this->hasUserChanged($user, $newToken)) { $userDeauthenticated = true; + // @deprecated since Symfony 5.4 + $newToken->setAuthenticated(false, false); if (null !== $this->logger) { // @deprecated since 5.3, change to $refreshedUser->getUserIdentifier() in 6.0 @@ -268,12 +275,9 @@ protected function refreshUser(TokenInterface $token): ?TokenInterface } if ($userDeauthenticated) { - if (null !== $this->logger) { - $this->logger->debug('Token was deauthenticated after trying to refresh it.'); - } - + // @deprecated since Symfony 5.4 if ($this->dispatcher) { - $this->dispatcher->dispatch(new DeauthenticatedEvent($token, $newToken), DeauthenticatedEvent::class); + $this->dispatcher->dispatch(new DeauthenticatedEvent($token, $newToken, false), DeauthenticatedEvent::class); } return null; @@ -316,6 +320,63 @@ private function safelyUnserialize(string $serializedToken) return $token; } + /** + * @param string|\Stringable|UserInterface $originalUser + */ + private static function hasUserChanged($originalUser, TokenInterface $refreshedToken): bool + { + $refreshedUser = $refreshedToken->getUser(); + + if ($originalUser instanceof UserInterface) { + if (!$refreshedUser instanceof UserInterface) { + return true; + } else { + // noop + } + } elseif ($refreshedUser instanceof UserInterface) { + return true; + } else { + return (string) $originalUser !== (string) $refreshedUser; + } + + if ($originalUser instanceof EquatableInterface) { + return !(bool) $originalUser->isEqualTo($refreshedUser); + } + + // @deprecated since Symfony 5.3, check for PasswordAuthenticatedUserInterface on both user objects before comparing passwords + if ($originalUser->getPassword() !== $refreshedUser->getPassword()) { + return true; + } + + // @deprecated since Symfony 5.3, check for LegacyPasswordAuthenticatedUserInterface on both user objects before comparing salts + if ($originalUser->getSalt() !== $refreshedUser->getSalt()) { + return true; + } + + $userRoles = array_map('strval', (array) $refreshedUser->getRoles()); + + if ($refreshedToken instanceof SwitchUserToken) { + $userRoles[] = 'ROLE_PREVIOUS_ADMIN'; + } + + if ( + \count($userRoles) !== \count($refreshedToken->getRoleNames()) || + \count($userRoles) !== \count(array_intersect($userRoles, $refreshedToken->getRoleNames())) + ) { + return true; + } + + // @deprecated since Symfony 5.3, drop getUsername() in 6.0 + $userIdentifier = function ($refreshedUser) { + return method_exists($refreshedUser, 'getUserIdentifier') ? $refreshedUser->getUserIdentifier() : $refreshedUser->getUsername(); + }; + if ($userIdentifier($originalUser) !== $userIdentifier($refreshedUser)) { + return true; + } + + return false; + } + /** * @internal */ diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php index cbed2baebdc69..965a988acb409 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -16,6 +16,7 @@ use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; +use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; @@ -45,12 +46,16 @@ public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess() ->willReturn([['foo' => 'bar'], null]) ; - $token = $this->createMock(TokenInterface::class); - $token - ->expects($this->any()) - ->method('isAuthenticated') - ->willReturn(true) - ; + $token = new class extends AbstractToken { + public function isAuthenticated(): bool + { + return true; + } + + public function getCredentials() + { + } + }; $tokenStorage = $this->createMock(TokenStorageInterface::class); $tokenStorage @@ -79,6 +84,9 @@ public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess() $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); } + /** + * @group legacy + */ public function testHandleWhenTheTokenIsNotAuthenticated() { $request = new Request(); diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php index bf9b028dcb638..ba07b90d4bb57 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php @@ -305,6 +305,9 @@ public function testAcceptsProvidersAsTraversable() $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); } + /** + * @group legacy + */ public function testDeauthenticatedEvent() { $tokenStorage = new TokenStorage();