!squash addressing minor PR comments

oleg-andreyev · oleg-andreyev · commit 1db8f8906db8 · 2019-07-14T17:08:01.000+03:00
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php
@@ -37,22 +37,28 @@ public function testServiceIsFunctional()
     public function userWillBeMarkedAsChangedIfRolesHasChangedProvider()
     {
         return [
-            [User::class],
-            [UserWithoutEquatable::class],
+            [
+                new User('user1', 'test', ['ROLE_ADMIN']),
+                new User('user1', 'test', ['ROLE_USER']),
+            ],
+            [
+                new UserWithoutEquatable('user1', 'test', ['ROLE_ADMIN']),
+                new UserWithoutEquatable('user1', 'test', ['ROLE_USER']),
+            ],
         ];
     }
 
     /**
      * @dataProvider userWillBeMarkedAsChangedIfRolesHasChangedProvider
      */
-    public function testUserWillBeMarkedAsChangedIfRolesHasChanged($userClass)
+    public function testUserWillBeMarkedAsChangedIfRolesHasChanged(UserInterface $userWithAdminRole, UserInterface $userWithoutAdminRole)
     {
         $client = $this->createClient(['test_case' => 'AbstractTokenCompareRoles', 'root_config' => 'config.yml']);
         $client->disableReboot();
 
         /** @var ArrayUserProvider $userProvider */
         $userProvider = static::$kernel->getContainer()->get('security.user.provider.array');
-        $userProvider->addUser(new $userClass('user1', 'test', ['ROLE_ADMIN']));
+        $userProvider->addUser($userWithAdminRole);
 
         $client->request('POST', '/login', [
             '_username' => 'user1',
@@ -63,8 +69,8 @@ public function testUserWillBeMarkedAsChangedIfRolesHasChanged($userClass)
         $client->request('GET', '/admin');
         $this->assertEquals(200, $client->getResponse()->getStatusCode());
 
-        // revoking ROLE_ADMIN from user1
-        $userProvider->setUser('user1', new $userClass('user1', 'test', ['ROLE_USER']));
+        // updating user provider with same user but revoked ROLE_ADMIN from user1
+        $userProvider->setUser('user1', $userWithoutAdminRole);
 
         // user1 has lost ROLE_ADMIN and MUST be redirected away from secure page
         $client->request('GET', '/admin');
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AbstractTokenCompareRoles/routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AbstractTokenCompareRoles/routing.yml
@@ -6,4 +6,3 @@ logout:
 
 admin_bundle:
   resource: '@SecuredPageBundle/Resources/config/routing.yml'
-
diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php
@@ -318,13 +318,7 @@ private function hasUserChanged(UserInterface $user)
         }
 
         $userRoles = array_map('strval', (array) $user->getRoles());
-        $rolesChanged =
-            (bool) \count(
-                array_merge(
-                    array_diff($this->getRoleNames(), $userRoles),
-                    array_diff($userRoles, $this->getRoleNames())
-                )
-            );
+        $rolesChanged = \count($userRoles) !== \count($this->getRoleNames()) || \count($userRoles) !== \count(array_intersect($userRoles, $this->getRoleNames()));
 
         if ($rolesChanged) {
             return true;
diff --git a/src/Symfony/Component/Security/Core/User/User.php b/src/Symfony/Component/Security/Core/User/User.php
@@ -137,15 +137,7 @@ public function isEqualTo(UserInterface $user)
 
         $currentRoles = array_map('strval', (array) $this->getRoles());
         $newRoles = array_map('strval', (array) $user->getRoles());
-
-        $rolesChanged =
-            (bool) \count(
-                array_merge(
-                    array_diff($currentRoles, $newRoles),
-                    array_diff($newRoles, $currentRoles)
-                )
-            );
-
+        $rolesChanged = \count($currentRoles) !== \count($newRoles) || \count($currentRoles) !== \count(array_intersect($currentRoles, $newRoles));
         if ($rolesChanged) {
             return false;
         }

Original file line number	Diff line number	Diff line change
`@@ -6,4 +6,3 @@ logout:`
`6`	`6`
`7`	`7`	`admin_bundle:`
`8`	`8`	`resource: '@SecuredPageBundle/Resources/config/routing.yml'`
`9`		`-`