symfony · fabpot · Sep 6, 2019 · Apr 18, 2019
@@ -0,0 +1,17 @@
+<?php
+
+namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Controller;
+
+use Symfony\Component\DependencyInjection\ContainerAwareInterface;
+use Symfony\Component\DependencyInjection\ContainerAwareTrait;
+use Symfony\Component\HttpFoundation\Response;
+
+class AdminController implements ContainerAwareInterface
+{
+    use ContainerAwareTrait;
+
+    public function indexAction()
+    {
+        return new Response('admin');
+    }
+}
@@ -0,0 +1,3 @@
+admin:
+    path:     /admin
+    defaults: { _controller: \Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Controller\AdminController::indexAction }
@@ -0,0 +1,9 @@
+<?php
+
+namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle;
+
+use Symfony\Component\HttpKernel\Bundle\Bundle;
+
+class SecuredPageBundle extends Bundle
+{
+}
@@ -0,0 +1,57 @@
+<?php
+
+namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User;
+
+use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
+use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
+use Symfony\Component\Security\Core\User\UserInterface;
+use Symfony\Component\Security\Core\User\UserProviderInterface;
+
+class ArrayUserProvider implements UserProviderInterface
+{
+    /** @var UserInterface[] */
+    private $users = [];
+
+    public function addUser(UserInterface $user)
+    {
+        $this->users[$user->getUsername()] = $user;
+    }
+
+    public function setUser($username, UserInterface $user)
+    {
+        $this->users[$username] = $user;
+    }
+
+    public function getUser($username)
+    {
+        return $this->users[$username];
+    }
+
+    public function loadUserByUsername($username)
+    {
+        $user = $this->getUser($username);
+
+        if (null === $user) {
+            throw new UsernameNotFoundException(sprintf('User "%s" not found.', $username));
+        }
+
+        return $user;
+    }
+
+    public function refreshUser(UserInterface $user)
+    {
+        if (!$user instanceof UserInterface) {
+            throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
+        }
+
+        $storedUser = $this->getUser($user->getUsername());
+        $class = \get_class($storedUser);
+
+        return new $class($storedUser->getUsername(), $storedUser->getPassword(), $storedUser->getRoles(), $storedUser->isEnabled(), $storedUser->isAccountNonExpired(), $storedUser->isCredentialsNonExpired() && $storedUser->getPassword() === $user->getPassword(), $storedUser->isAccountNonLocked());
+    }
+
+    public function supportsClass($class)
+    {
+        return 'Symfony\Component\Security\Core\User\User' === $class;
+    }
+}
@@ -11,8 +11,10 @@
 
 namespace Symfony\Bundle\SecurityBundle\Tests\Functional;
 
+use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User\ArrayUserProvider;
 use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
 use Symfony\Component\Security\Core\User\User;
+use Symfony\Component\Security\Core\User\UserInterface;
 
 class SecurityTest extends AbstractWebTestCase
 {
@@ -31,4 +33,148 @@ public function testServiceIsFunctional()
         $this->assertTrue($security->isGranted('ROLE_USER'));
         $this->assertSame($token, $security->getToken());
     }
+
+    public function userWillBeMarkedAsChangedIfRolesHasChangedProvider()
+    {
+        return [
+            [
+                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(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($userWithAdminRole);
+
+        $client->request('POST', '/login', [
+            '_username' => 'user1',
+            '_password' => 'test',
+        ]);
+
+        // user1 has ROLE_ADMIN and can visit secure page
+        $client->request('GET', '/admin');
+        $this->assertEquals(200, $client->getResponse()->getStatusCode());
+
+        // 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');
+        $this->assertEquals(302, $client->getResponse()->getStatusCode());
+    }
+}
+
+final class UserWithoutEquatable implements UserInterface
+{
+    private $username;
+    private $password;
+    private $enabled;
+    private $accountNonExpired;
+    private $credentialsNonExpired;
+    private $accountNonLocked;
+    private $roles;
+
+    public function __construct(?string $username, ?string $password, array $roles = [], bool $enabled = true, bool $userNonExpired = true, bool $credentialsNonExpired = true, bool $userNonLocked = true)
+    {
+        if ('' === $username || null === $username) {
+            throw new \InvalidArgumentException('The username cannot be empty.');
+        }
+
+        $this->username = $username;
+        $this->password = $password;
+        $this->enabled = $enabled;
+        $this->accountNonExpired = $userNonExpired;
+        $this->credentialsNonExpired = $credentialsNonExpired;
+        $this->accountNonLocked = $userNonLocked;
+        $this->roles = $roles;
+    }
+
+    public function __toString()
+    {
+        return $this->getUsername();
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getRoles()
+    {
+        return $this->roles;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getPassword()
+    {
+        return $this->password;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getSalt()
+    {
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getUsername()
+    {
+        return $this->username;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isAccountNonExpired()
+    {
+        return $this->accountNonExpired;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isAccountNonLocked()
+    {
+        return $this->accountNonLocked;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isCredentialsNonExpired()
+    {
+        return $this->credentialsNonExpired;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isEnabled()
+    {
+        return $this->enabled;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function eraseCredentials()
+    {
+    }
 }
@@ -0,0 +1,20 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
+use Symfony\Bundle\SecurityBundle\SecurityBundle;
+use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\SecuredPageBundle;
+
+return [
+    new FrameworkBundle(),
+    new SecurityBundle(),
+    new SecuredPageBundle(),
+];
@@ -0,0 +1,32 @@
+imports:
+  - { resource: ./../config/framework.yml }
+
+services:
+  _defaults: { public: true }
+
+  security.user.provider.array:
+    class: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User\ArrayUserProvider
+
+security:
+
+  encoders:
+    \Symfony\Component\Security\Core\User\UserInterface: plaintext
+
+  providers:
+    array:
+      id: security.user.provider.array
+
+  firewalls:
+ 
10000
   default:
+      form_login:
+        check_path: login
+        remember_me: true
+        require_previous_session: false
+      logout: ~
+      anonymous: ~
+      stateless: false
+
+  access_control:
+    - { path: ^/admin$, roles: ROLE_ADMIN }
+    - { path: ^/login$, roles: IS_AUTHENTICATED_ANONYMOUSLY }
+    - { path: .*, roles: IS_AUTHENTICATED_FULLY }
@@ -0,0 +1,8 @@
+login:
+  path: /login
+
+logout:
+  path: /logout
+
+admin_bundle:
+  resource: '@SecuredPageBundle/Resources/config/routing.yml'
@@ -21,7 +21,7 @@
         "symfony/config": "^4.2|^5.0",
         "symfony/dependency-injection": "^4.2|^5.0",
         "symfony/http-kernel": "^4.4",
-        "symfony/security-core": "^4.3",
+        "symfony/security-core": "^4.4",
         "symfony/security-csrf": "^4.2|^5.0",
         "symfony/security-guard": "^4.2|^5.0",
         "symfony/security-http": "^4.3"

@@ -166,6 +166,7 @@ public function __serialize(): array
      * @return string
         *
      * @final since Symfony 4.3, use __serialize() instead
+     *
      * @internal since Symfony 4.3, use __serialize() instead
      */
     public function serialize()
@@ -316,6 +317,13 @@ private function hasUserChanged(UserInterface $user)
             return true;
         }
 
+        $userRoles = array_map('strval', (array) $user->getRoles());
+        $rolesChanged = \count($userRoles) !== \count($this->getRoleNames()) || \count($userRoles) !== \count(array_intersect($userRoles, $this->getRoleNames()));
+
+        if ($rolesChanged) {
+            return true;
+        }
+
         if ($this->user->getUsername() !== $user->getUsername()) {
             return true;
         }

@@ -117,8 +117,8 @@ public static function isEqualToData()
     {
         return [
             [true, new User('username', 'password'), new User('username', 'password')],
-            [true, new User('username', 'password', ['ROLE']), new User('username', 'password')],
-            [true, new User('username', 'password', ['ROLE']), new User('username', 'password', ['NO ROLE'])],
+            [false, new User('username', 'password', ['ROLE']), new User('username', 'password')],
+            [false, new User('username', 'password', ['ROLE']), new User('username', 'password', ['NO ROLE'])],
             [false, new User('diff', 'diff'), new User('username', 'password')],
             [false, new User('diff', 'diff', [], false), new User('username', 'password')],
             [false, new User('diff', 'diff', [], false, false), new User('username', 'password')],

@@ -143,6 +143,13 @@ public function isEqualTo(UserInterface $user): bool
             return false;
         }
 
+        $currentRoles = array_map('strval', (array) $this->getRoles());
+        $newRoles = array_map('strval', (array) $user->getRoles());
+        $rolesChanged = \count($currentRoles) !== \count($newRoles) || \count($currentRoles) !== \count(array_intersect($currentRoles, $newRoles));
+        if ($rolesChanged) {
+            return false;
+        }
+
         if ($this->getUsername() !== $user->getUsername()) {
             return false;
         }