[Security] Unset token roles when serializing it and user implements EquatableInterface

nicolas-grekas · nicolas-grekas · commit b92aae3f6def · 2025-01-20T10:01:54.000+01:00
diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Security\Core\Authentication\Token;
 
+use Symfony\Component\Security\Core\User\EquatableInterface;
 use Symfony\Component\Security\Core\User\InMemoryUser;
 use Symfony\Component\Security\Core\User\UserInterface;
 
@@ -23,24 +24,24 @@
 abstract class AbstractToken implements TokenInterface, \Serializable
 {
     private ?UserInterface $user = null;
-    private array $roleNames = [];
+    private array $roleNames;
     private array $attributes = [];
 
     /**
      * @param string[] $roles An array of roles
-     *
-     * @throws \InvalidArgumentException
      */
     public function __construct(array $roles = [])
     {
+        $this->roleNames = [];
+
         foreach ($roles as $role) {
-            $this->roleNames[] = $role;
+            $this->roleNames[] = (string) $role;
         }
     }
 
     public function getRoleNames(): array
     {
-        return $this->roleNames;
+        return $this->roleNames ??= self::__construct($this->user->getRoles()) ?? $this->roleNames;
     }
 
     public function getUserIdentifier(): string
@@ -82,7 +83,13 @@ public function eraseCredentials(): void
      */
     public function __serialize(): array
     {
-        return [$this->user, true, null, $this->attributes, $this->roleNames];
+        $data = [$this->user, true, null, $this->attributes];
+
+        if (!$this->user instanceof EquatableInterface) {
+            $data[] = $this->roleNames;
+        }
+
+        return $data;
     }
 
     /**
@@ -103,7 +110,12 @@ public function __serialize(): array
      */
     public function __unserialize(array $data): void
     {
-        [$user, , , $this->attributes, $this->roleNames] = $data;
+        [$user, , , $this->attributes] = $data;
+
+        if (\array_key_exists(4, $data)) {
+            $this->roleNames = $data[4];
+        }
+
         $this->user = \is_string($user) ? new InMemoryUser($user, '', $this->roleNames, false) : $user;
     }
 
diff --git a/src/Symfony/Component/Security/Core/Tests/Exception/CustomUserMessageAuthenticationExceptionTest.php b/src/Symfony/Component/Security/Core/Tests/Exception/CustomUserMessageAuthenticationExceptionTest.php
@@ -53,6 +53,7 @@ public function testSharedSerializedData()
         $exception->setSafeMessage('message', ['token' => $token]);
 
         $processed = unserialize(serialize($exception));
+        $this->assertSame($token->getRoleNames(), $processed->getToken()->getRoleNames());
         $this->assertEquals($token, $processed->getToken());
         $this->assertEquals($token, $processed->getMessageData()['token']);
         $this->assertSame($processed->getToken(), $processed->getMessageData()['token']);
@@ -67,6 +68,7 @@ public function testSharedSerializedDataFromChild()
         $exception->setToken($token);
 
         $processed = unserialize(serialize($exception));
+        $this->assertSame($token->getRoleNames(), $processed->getToken()->getRoleNames());
         $this->assertEquals($token, $processed->childMember);
         $this->assertEquals($token, $processed->getToken());
         $this->assertSame($processed->getToken(), $processed->childMember);
diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php
@@ -301,11 +301,12 @@ private static function hasUserChanged(UserInterface $originalUser, TokenInterfa
             }
         }
 
-        $userRoles = array_map('strval', $refreshedUser->getRoles());
+        $refreshedUserRoles = array_map('strval', $refreshedUser->getRoles());
+        $originalUserRoles = $refreshedToken->getRoleNames(); // This comes from cloning the original token, so it still contains the roles of the original user
 
         if (
-            \count($userRoles) !== \count($refreshedToken->getRoleNames())
-            || \count($userRoles) !== \count(array_intersect($userRoles, $refreshedToken->getRoleNames()))
+            \count($refreshedUserRoles) !== \count($originalUserRoles)
+            || \count($refreshedUserRoles) !== \count(array_intersect($refreshedUserRoles, $originalUserRoles))
         ) {
             return true;
         }

Original file line number	Diff line number	Diff line change
`@@ -301,11 +301,12 @@ private static function hasUserChanged(UserInterface $originalUser, TokenInterfa`
`301`	`301`	`}`
`302`	`302`	`}`
`303`	`303`
`304`		`- $userRoles = array_map('strval', $refreshedUser->getRoles());`
	`304`	`+ $refreshedUserRoles = array_map('strval', $refreshedUser->getRoles());`
	`305`	`+ $originalUserRoles = $refreshedToken->getRoleNames(); // This comes from cloning the original token, so it still contains the roles of the original user`
`305`	`306`
`306`	`307`	`if (`
`307`		`- \count($userRoles) !== \count($refreshedToken->getRoleNames())`
`308`		`- \|\| \count($userRoles) !== \count(array_intersect($userRoles, $refreshedToken->getRoleNames()))`
	`308`	`+ \count($refreshedUserRoles) !== \count($originalUserRoles)`
	`309`	`+ \|\| \count($refreshedUserRoles) !== \count(array_intersect($refreshedUserRoles, $originalUserRoles))`
`309`	`310`	`) {`
`310`	`311`	`return true;`
`311`	`312`	`}`