From e47b31c43c4be9241e82ea9b65a32cd730196a1c Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Sat, 2 Nov 2019 08:13:43 +0100 Subject: [PATCH] [Security] Fix SwitchUserToken wrongly deauthenticated --- .../Authentication/Token/AbstractToken.php | 7 ++-- .../Token/SwitchUserTokenTest.php | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php index d39e99c336..8391763ceb 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php @@ -318,9 +318,12 @@ abstract class AbstractToken implements TokenInterface } $userRoles = array_map('strval', (array) $user->getRoles()); - $rolesChanged = \count($userRoles) !== \count($this->getRoleNames()) || \count($userRoles) !== \count(array_intersect($userRoles, $this->getRoleNames())); - if ($rolesChanged) { + if ($this instanceof SwitchUserToken) { + $userRoles[] = 'ROLE_PREVIOUS_ADMIN'; + } + + if (\count($userRoles) !== \count($this->getRoleNames()) || \count($userRoles) !== \count(array_intersect($userRoles, $this->getRoleNames()))) { return true; } 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 5841250959..da7354e9df 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/SwitchUserTokenTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/SwitchUserTokenTest.php @@ -14,6 +14,7 @@ namespace Symfony\Component\Security\Core\Tests\Authentication\Token; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\User\UserInterface; class SwitchUserTokenTest extends TestCase { @@ -38,4 +39,38 @@ class SwitchUserTokenTest extends TestCase $this->assertSame('provider-key', $unserializedOriginalToken->getProviderKey()); $this->assertEquals(['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH'], $unserializedOriginalToken->getRoleNames()); } + + public function testSetUserDoesNotDeauthenticate() + { + $impersonated = new class() implements UserInterface { + public function getUsername() + { + return 'impersonated'; + } + + public function getPassword() + { + return null; + } + + public function eraseCredentials() + { + } + + public function getRoles() + { + return ['ROLE_USER']; + } + + public function getSalt() + { + return null; + } + }; + + $originalToken = new UsernamePasswordToken('impersonator', '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()); + } }