minor symfony#59558 [Security] Unset token roles when serializing it and user implements EquatableInterface (nicolas-grekas)

This PR was merged into the 7.3 branch.

Discussion
----------

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

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

When the user object implement EquatableInterface, we never read the roles stored in the token object that wraps the user in the session storage.

This PR ensures we don't store these roles either - they're just wasting space.

Commits
-------

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

Author: nicolas-grekas

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;
     }
 

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);

src/Symfony/Component/Security/Http/Firewall/ContextListener.php

@@ -308,11 +308,12 @@ private static function hasUserChanged(UserInterface $originalUser, TokenInterfa
             }
         }
 
-        $userRoles = array_map('strval', $refreshedUser->getRoles());
+        $refreshedRoles = array_map('strval', $refreshedUser->getRoles());
+        $originalRoles = $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($refreshedRoles) !== \count($originalRoles)
+            || \count($refreshedRoles) !== \count(array_intersect($refreshedRoles, $originalRoles))
         ) {
             return true;
         }