Changing behavior so that roles are only refreshed if the token requests

it By default, AbstractToken (and its sub-classes), have a mechanism to check if any extra tokens were added, beyond the user tokens. If there are none, then the roles are refreshed. If there are some, then they are not.
symfony · weaverryan · Sep 26, 2017 · Sep 26, 2017 · Sep 26, 2017 · Sep 26, 2017
commit 87d2826871d308a7c1389e065e686b74fb2998e1
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/RefreshableRolesTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/RefreshableRolesTest.php
@@ -0,0 +1,127 @@
+<?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.
+ */
+
+namespace Symfony\Bundle\SecurityBundle\Tests\Functional;
+
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\User\UserInterface;
+use Symfony\Component\Security\Core\User\UserProviderInterface;
+
+class RefreshableRolesTest extends WebTestCase
+{
+    public function testRolesAreRefreshed()
+    {
+        // log them in!
+        $client = $this->createAuthenticatedClient('cool_user');
+
+        // refresh the page, roles have not changed yet
+        $client->request('GET', '/profile');
+        $rolesData = $client->getProfile()->getCollector('security')->getRoles();
+        $this->assertCount(1, $rolesData);
+        $this->assertEquals('ROLE_ORIGINAL', $rolesData[0]);
+
+        // this will cause the refreshed user to have these new roles
+        $client->request('GET', '/profile?new_role=ROLE_NEW');
+        $rolesData = $client->getProfile()->getCollector('security')->getRoles();
+        $this->assertCount(1, $rolesData);
+        $this->assertEquals('ROLE_NEW', $rolesData[0]);
+
+        // the change should be persistent
+        $client->request('GET', '/profile');
+        $rolesData = $client->getProfile()->getCollector('security')->getRoles();
+        $this->assertCount(1, $rolesData);
+        $this->assertEquals('ROLE_NEW', $rolesData[0]);
+    }
+
+    private function createAuthenticatedClient($username)
+    {
+        $client = $this->createClient(array('test_case' => 'StandardFormLogin', 'root_config' => 'refreshable_roles.yml'));
+        $client->followRedirects(true);
+
+        $form = $client->request('GET', '/login')->selectButton('login')->form();
+        $form['_username'] = $username;
+        $form['_password'] = 'test';
+        $client->submit($form);
+
+        return $client;
+    }
+}
+
+class RefreshableRolesUserProvider implements UserProviderInterface
+{
+    private $requestStack;
+
+    public function __construct(RequestStack $requestStack)
+    {
+        $this->requestStack = $requestStack;
+    }
+
+    public function loadUserByUsername($username)
    {
+        return new RefreshableUser($username, array('ROLE_ORIGINAL'));
+    }
+
+    public function refreshUser(UserInterface $user)
+    {
+        $request = $this->requestStack->getCurrentRequest();
+        // a sneaky way of faking the stored user's roles being changed
+        if ($request->query->has('new_role')) {
+            $user->setRoles(array($request->query->get('new_role')));
+        }
+
+        return $user;
+    }
+
+    public function supportsClass($class)
+    {
+        return RefreshableUser::class === $class;
+    }
+}
+
+class RefreshableUser implements UserInterface
+{
+    private $username;
+    private $roles;
+
+    public function __construct($username, array $roles)
+    {
+        $this->username = $username;
+        $this->roles = $roles;
+    }
+
+    public function getUsername()
+    {
+        return $this->username;
+    }
+
+    public function getRoles()
+    {
+        return $this->roles;
+    }
+
+    public function setRoles($roles)
+    {
+        $this->roles = $roles;
+    }
+
+    public function getPassword()
+    {
+        return 'test';
+    }
+
+    public function getSalt()
+    {
+    }
+
+    public function eraseCredentials()
+    {
+    }
+}
diff --git a/...ymfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/refreshable_roles.yml b/...ymfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/refreshable_roles.yml
@@ -0,0 +1,29 @@
+imports:
+    - { resource: ./../config/default.yml }
+
+services:
+    refreshable_roles_user_provider:
+      class: Symfony\Bundle\SecurityBundle\Tests\Functional\RefreshableRolesUserProvider
+      arguments: ['@request_stack']
+
+security:
+    encoders:
+        Symfony\Bundle\SecurityBundle\Tests\Functional\RefreshableUser: plaintext
+
+    providers:
+        all_users:
+            id: refreshable_roles_user_provider
+
+    firewalls:
+        # This firewall doesn't make sense in combination with the rest of the
+        # configuration file, but it's here for testing purposes (do not use
+        # this file in a real world scenario though)
+        login_form:
+            pattern: ^/login$
+            security: false
+
+        default:
+            form_login:
+                check_path: /login_check
+                default_target_path: /profile
+            anonymous: ~
diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php
@@ -29,6 +29,7 @@ abstract class AbstractToken implements TokenInterface, RefreshableRolesTokenInt
     private $roles = array();
     private $authenticated = false;
     private $attributes = array();
+    private $shouldUpdateRoles = false;
 
     /**
      * Constructor.
@@ -235,6 +236,14 @@ public function updateRoles(array $roles)
         }
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function shouldUpdateRoles()
+    {
+        return $this->shouldUpdateRoles;
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -251,6 +260,18 @@ public function __toString()
         return sprintf('%s(user="%s", authenticated=%s, roles="%s")', $class, $this->getUsername(), json_encode($this->authenticated), implode(', ', $roles));
     }
 
+    /**
+     * Call this from a sub-class if you want the token's roles
+     * to be updated from UserInterface::getRoles() on each
+     * page refresh (when using session-based authentication).
+     *
+     * @param bool $shouldUpdateRoles
+     */
+    protected function setShouldUpdateRoles($shouldUpdateRoles)
+    {
+        $this->shouldUpdateRoles = $shouldUpdateRoles;
+    }
+
     private function hasUserChanged(UserInterface $user)
     {
         if (!($this->user instanceof UserInterface)) {

diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AnonymousToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AnonymousToken.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\Security\Core\Authentication\Token;
 
 use Symfony\Component\Security\Core\Role\Role;
+use Symfony\Component\Security\Core\User\UserInterface;
 
 /
1E0A
**
  * AnonymousToken represents an anonymous token.
@@ -36,6 +37,8 @@ public function __construct($secret, $user, array $roles = array())
         $this->secret = $secret;
         $this->setUser($user);
         $this->setAuthenticated(true);
+
+        $this->setShouldUpdateRoles($user instanceof UserInterface && $user->getRoles() === $roles);
     }
 
     /**

diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/PreAuthenticatedToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/PreAuthenticatedToken.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Security\Core\Authentication\Token;
 
+use Symfony\Component\Security\Core\User\UserInterface;
+
 /**
  * PreAuthenticatedToken implements a pre-authenticated token.
  *
@@ -44,6 +46,8 @@ public function __construct($user, $credentials, $providerKey, array $roles = ar
         if ($roles) {
             $this->setAuthenticated(true);
         }
+
+        $this->setShouldUpdateRoles($user instanceof UserInterface && $user->getRoles() === $roles);
     }
 
     /**

diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/RefreshableRolesTokenInterface.php b/src/Symfony/Component/Security/Core/Authentication/Token/RefreshableRolesTokenInterface.php
@@ -22,4 +22,15 @@ interface RefreshableRolesTokenInterface
      * @param array $roles An array of roles
      */
     public function updateRoles(array $roles);
+
+    /**
+     * Returns whether or not roles *should* be updated on this token.
+     *
+     * This can be useful if your token is adding custom roles,
+     * and so you purposely do not want the roles in the token to
+     * be automatically reset.
+     *
+     * @return bool
+     */
+    public function shouldUpdateRoles();
 }
diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php
@@ -49,6 +49,7 @@ public function __construct(UserInterface $user, $providerKey, $secret)
 
         $this->setUser($user);
         parent::setAuthenticated(true);
+        $this->setShouldUpdateRoles(true);
     }
 
     /**

diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/UsernamePasswordToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/UsernamePasswordToken.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Security\Core\Authentication\Token;
 
+use Symfony\Component\Security\Core\User\UserInterface;
+
 /**
  * UsernamePasswordToken implements a username and password token.
  *
@@ -44,6 +46,8 @@ public function __construct($user, $credentials, $providerKey, array $roles = ar
         $this->providerKey = $providerKey;
 
         parent::setAuthenticated(count($roles) > 0);
+
+        $this->setShouldUpdateRoles($user instanceof UserInterface && $user->getRoles() === $roles);
     }
 
     /**

diff --git a/...ecurity/Core/Tests/Authentication/Provider/PreAuthenticatedAuthenticationProviderTest.php b/...ecurity/Core/Tests/Authentication/Provider/PreAuthenticatedAuthenticationProviderTest.php
@@ -56,7 +56,7 @@ public function testAuthenticate()
     {
         $user = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();
         $user
-            ->expects($this->once())
+            ->expects($this->atLeastOnce())
             ->method('getRoles')
             ->will($this->returnValue(array()))
         ;

diff --git a/.../Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php b/.../Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php
@@ -159,7 +159,7 @@ public function testAuthenticateWhenPostCheckAuthenticationFailsWithHideFalse()
     public function testAuthenticate()
     {
         $user = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();
-        $user->expects($this->once())
+        $user->expects($this->atLeastOnce())
              ->method('getRoles')
              ->will($this->returnValue(array('ROLE_FOO')))
         ;
@@ -193,7 +193,7 @@ public function testAuthenticate()
     public function testAuthenticateWithPreservingRoleSwitchUserRole()
     {
         $user = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();
-        $user->expects($this->once())
+        $user->expects($this->atLeastOnce())
              ->method('getRoles')
              ->will($this->returnValue(array('ROLE_FOO')))
         ;

diff --git a/src/Symfony/Component/Security/Guard/Token/PostAuthenticationGuardToken.php b/src/Symfony/Component/Security/Guard/Token/PostAuthenticationGuardToken.php
@@ -48,6 +48,8 @@ public function __construct(UserInterface $user, $providerKey, array $roles)
         // this token is meant to be used after authentication success, so it is always authenticated
         // you could set it as non authenticated later if you need to
         parent::setAuthenticated(true);
+
+        $this->setShouldUpdateRoles($user instanceof UserInterface && $user->getRoles() === $roles);
     }
 
     /**

diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php
@@ -40,12 +40,14 @@ class ContextListener implements ListenerInterface
     private $tokenStorage;
     private $contextKey;
     private $sessionKey;
+    private $refreshableRolesSessionKey;
     private $logger;
     private $userProviders;
     private $dispatcher;
     private $registered;
     private $trustResolver;
     private $logoutOnUserChange = false;
+    private $refreshedToken;
 
     /**
      * @param TokenStorageInterface                     $tokenStorage
@@ -65,6 +67,8 @@ public function __construct(TokenStorageInterface $tokenStorage, $userProviders,
         $this->userProviders = $userProviders;
         $this->contextKey = $contextKey;
         $this->sessionKey = '_security_'.$contextKey;
+        $this->refreshableRolesSessionKey = $this->sessionKey.'_refreshable_roles';
+
         $this->logger = $logger;
         $this->dispatcher = $dispatcher;
         $this->trustResolver = $trustResolver ?: new AuthenticationTrustResolver(AnonymousToken::class, RememberMeToken::class);
@@ -120,14 +124,16 @@ public function handle(GetResponseEvent $event)
             $token = null;
         }
 
-        if ($token instanceof RefreshableRolesTokenInterface && $token->getUser() instanceof UserInterface) {
+        // see if this token wants the roles refreshed from the User
+        if ($session->get($this->refreshableRolesSessionKey) && $token->getUser() instanceof UserInterface) {
             if (null !== $this->logger) {
                 $this->logger->debug('Refreshing token roles from the User object');
             }
 
             $token->updateRoles($token->getUser()->getRoles());
         }
 
+        $this->refreshedToken = $token;
         $this->tokenStorage->setToken($token);
     }
 
@@ -155,13 +161,20 @@ public function onKernelResponse(FilterResponseEvent $event)
         if ((null === $token = $this->tokenStorage->getToken()) || $this->trustResolver->isAnonymous($token)) {
             if ($request->hasPreviousSession()) {
                 $session->remove($this->sessionKey);
+                $session->remove($this->refreshableRolesSessionKey);
             }
         } else {
             $session->set($this->sessionKey, serialize($token));
 
             if (null !== $this->logger) {
                 $this->logger->debug('Stored the security token in the session.', array('key' => $this->sessionKey));
             }
+
+            // if the token changed, then re-set the refreshable key
+            if ($token !== $this->refreshedToken) {
+                $shouldUpdateRoles = $token instanceof RefreshableRolesTokenInterface && $token->shouldUpdateRoles();
+                $session->set($this->refreshableRolesSessionKey, $shouldUpdateRoles);
+            }
         }
     }
-Original file line number
+Diff line change
@@ Expand Up @@
             $this->setUser($user);
             parent::setAuthenticated(true);
+            $this->setShouldUpdateRoles(true);
         }
         /**
@@ Expand Down @@