Revert symfony#31177, fixing roles equality check in AbstractToken

wouterj · wouterj · commit e27e80f512b9 · 2020-05-29T23:45:48.000+02:00
diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php
@@ -317,13 +317,6 @@ private function hasUserChanged(UserInterface $user): bool
             return true;
         }
 
-        $currentUserRoles = array_map('strval', (array) $this->user->getRoles());
-        $userRoles = array_map('strval', (array) $user->getRoles());
-
-        if (\count($userRoles) !== \count($currentUserRoles) || \count($userRoles) !== \count(array_intersect($userRoles, $currentUserRoles))) {
-            return true;
-        }
-
         if ($this->user->getUsername() !== $user->getUsername()) {
             return true;
         }
diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php
@@ -248,6 +248,21 @@ public function testSetUserDoesNotSetAuthenticatedToFalseWhenUserDoesNotChange($
         $token->setUser($user);
         $this->assertTrue($token->isAuthenticated());
     }
+
+    public function testIsUserChangedWhenSerializing()
+    {
+        $token = new ConcreteToken(['ROLE_ADMIN']);
+        $token->setAuthenticated(true);
+        $this->assertTrue($token->isAuthenticated());
+
+        $user = new SerializableUser('wouter', ['ROLE_ADMIN']);
+        $token->setUser($user);
+        $this->assertTrue($token->isAuthenticated());
+
+        $token = unserialize(serialize($token));
+        $token->setUser($user);
+        $this->assertTrue($token->isAuthenticated());
+    }
 }
 
 class TestUser
@@ -265,6 +280,55 @@ public function __toString(): string
     }
 }
 
+class SerializableUser implements UserInterface, \Serializable
+{
+    private $roles;
+    private $name;
+
+    public function __construct($name, array $roles = [])
+    {
+        $this->name = $name;
+        $this->roles = $roles;
+    }
+
+    public function getUsername()
+    {
+        return $this->name;
+    }
+
+    public function getPassword()
+    {
+        return '***';
+    }
+
+    public function getRoles()
+    {
+        if (empty($this->roles)) {
+            return ['ROLE_USER'];
+        }
+
+        return $this->roles;
+    }
+
+    public function eraseCredentials()
+    {}
+
+    public function getSalt()
+    {
+        return null;
+    }
+
+    public function serialize()
+    {
+        return serialize($this->name);
+    }
+
+    public function unserialize($serialized)
+    {
+        $this->name = unserialize($serialized);
+    }
+}
+
 class ConcreteToken extends AbstractToken
 {
     private $credentials = 'credentials_value';

Original file line number	Diff line number	Diff line change
`@@ -317,13 +317,6 @@ private function hasUserChanged(UserInterface $user): bool`
`317`	`317`	`return true;`
`318`	`318`	`}`
`319`	`319`
`320`		`- $currentUserRoles = array_map('strval', (array) $this->user->getRoles());`
`321`		`- $userRoles = array_map('strval', (array) $user->getRoles());`
`322`		`-`
`323`		`- if (\count($userRoles) !== \count($currentUserRoles) \|\| \count($userRoles) !== \count(array_intersect($userRoles, $currentUserRoles))) {`
`324`		`- return true;`
`325`		`- }`
`326`		`-`
`327`	`320`	`if ($this->user->getUsername() !== $user->getUsername()) {`
`328`	`321`	`return true;`
`329`	`322`	`}`