symfony · weaverryan · Sep 26, 2017 · Sep 26, 2017 · Sep 26, 2017 · ro0NL
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
@@ -23,12 +23,13 @@
 * @author Fabien Potencier <fabien@symfony.com>
 * @author Johannes M. Schmitt <schmittjoh@gmail.com>
 */
-abstract class AbstractToken implements TokenInterface
+abstract class AbstractToken implements TokenInterface, RefreshableRolesTokenInterface
 {
    private $user;
    private $roles = array();
    private $authenticated = false;
    private $attributes = array();
+    private $shouldUpdateRoles = false;

    /**
     * Constructor.
@@ -39,15 +40,7 @@ abstract class AbstractToken implements TokenInterface
     */
    public function __construct(array $roles = array())
    {
-        foreach ($roles as $role) {
-            if (is_string($role)) {
-                $role = new Role($role);
-            } elseif (!$role instanceof RoleInterface) {
-                throw new \InvalidArgumentException(sprintf('$roles must be an array of strings, Role instances or RoleInterface instances, but got %s.', gettype($role)));
-            }
-
-            $this->roles[] = $role;
-        }
+        $this->updateRoles($roles);
    }

    /**
@@ -225,6 +218,32 @@ public function setAttribute($name, $value)
        $this->attributes[$name] = $value;
    }

+    /**
+     * {@inheritdoc}
+     */
+    public function updateRoles(array $roles)
+    {
+        $this->roles = array();
+
+        foreach ($roles as $role) {
+            if (is_string($role)) {
+                $role = new Role($role);
+            } elseif (!$role instanceof RoleInterface) {
+                throw new \InvalidArgumentException(sprintf('$roles must be an array of strings, Role instances or RoleInterface instances, but got %s.', gettype($role)));
+            }
+
+            $this->roles[] = $role;
+        }
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function shouldUpdateRoles()
+    {
+        return $this->shouldUpdateRoles;
+    }
+
    /**
     * {@inheritdoc}
     */
@@ -241,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;

 /**
 * 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
@@ -0,0 +1,36 @@
+<?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\Component\Security\Core\Authentication\Token;
+
+/**
+ * Allows the roles of a token to be updated when security is persisted across a session.
+ *
+ * @author Ryan Weaver <ryan@knpuniversity.com>
+ */
+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/Core/Tests/Authentication/Token/AbstractTokenTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php
@@ -138,6 +138,13 @@ public function testConstructor()
        $this->assertEquals(array(new Role('ROLE_FOO'), new Role('ROLE_BAR')), $token->getRoles());
    }

+    public function testUpdateRoles()
+    {
+        $token = $this->getToken();
+        $token->updateRoles(array('ROLE_FOO'));
+        $this->assertEquals(array(new Role('ROLE_FOO')), $token->getRoles());
+    }
+
    public function testAuthenticatedFlag()
    {
        $token = $this->getToken();

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

    /**
-Original file line number
+Diff line change
@@ -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);
             Copy link

  
      
    
  

  
      

  
    Contributor


      

  

  
    
      

      
            ro0NL
  

      

      

      


        Sep 29, 2017


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      Is this is expected... this affects all tokens today no? Should we optin thru config instead?
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



  
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  Author


  

  
    
      

      
            weaverryan
  

      

      

      


        Oct 3, 2017


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      This was my way of trying to safely opt everyone in automatically... but I don't think that's possible anymore, thanks to #24331 (comment)
So yea, I think we'll need to have a way to opt into this somehow...
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
         }
         /**
-          Expand Down