[Security] Namespace generated CSRF tokens depending of the current s…

…cheme
symfony · fabpot · Nov 16, 2017 · Nov 6, 2017 · Nov 16, 2017 · cdb4271975667ed4a31365ab086dec63f25c1e1c
commit cdb4271975667ed4a31365ab086dec63f25c1e1c
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml
@@ -22,6 +22,7 @@
         <service id="security.csrf.token_manager" class="%security.csrf.token_manager.class%">
             <argument type="service" id="security.csrf.token_generator" />
             <argument type="service" id="security.csrf.token_storage" />
+            <argument type="service" id="request_stack" on-invalid="ignore" />
         </service>
     </services>
 </container>
diff --git a/src/Symfony/Component/Security/Csrf/CsrfTokenManager.php b/src/Symfony/Component/Security/Csrf/CsrfTokenManager.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Security\Csrf;
 
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
 use Symfony\Component\Security\Core\Util\StringUtils;
 use Symfony\Component\Security\Csrf\TokenGenerator\UriSafeTokenGenerator;
 use Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface;
@@ -21,29 +23,59 @@
  * Default implementation of {@link CsrfTokenManagerInterface}.
  *
  * @author Bernhard Schussek <bschussek@gmail.com>
+ * @author Kévin Dunglas <dunglas@gmail.com>
  */
 class CsrfTokenManager implements CsrfTokenManagerInterface
 {
     private $generator;
     private $storage;
+    private $namespace;
 
-    public function __construct(TokenGeneratorInterface $generator = null, TokenStorageInterface $storage = null)
+    /**
+     * @param null|string|RequestStack|callable $namespace
+     *   * null: generates a namespace using $_SERVER['HTTPS']
+     *   * string: uses the given string
+     *   * RequestStack: generates a namespace using the current master request
+     *   * callable: uses the result of this callable (must return a string)
+     */
+    public function __construct(TokenGeneratorInterface $generator = null, TokenStorageInterface $storage = null, $namespace = null)
     {
        $this->generator = $generator ?: new UriSafeTokenGenerator();
         $this->storage = $storage ?: new NativeSessionTokenStorage();
+
+        $superGlobalNamespaceGenerator = function () {
+            return !empty($_SERVER['HTTPS']) && 'off' !== strtolower($_SERVER['HTTPS']) ? 'https-' : '';
+        };
+
+        if (null === $namespace) {
+            $this->namespace = $superGlobalNamespaceGenerator;
+        } elseif ($namespace instanceof RequestStack) {
+            $this->namespace = function () use ($namespace, $superGlobalNamespaceGenerator) {
+                if ($request = $namespace->getMasterRequest()) {
+                    return $request->isSecure() ? 'https-' : '';
+                }
+
+                return $superGlobalNamespaceGenerator();
+            };
+        } elseif (is_callable($namespace) || is_string($namespace)) {
+            $this->namespace = $namespace;
+        } else {
+            throw new InvalidArgumentException(sprintf('$namespace must be a string, a callable returning a string, null or an instance of "RequestStack". "%s" given.', gettype($namespace)));
+        }
     }
 
     /**
      * {@inheritdoc}
      */
     public function getToken($tokenId)
     {
-        if ($this->storage->hasToken($tokenId)) {
-            $value = $this->storage->getToken($tokenId);
+        $namespacedId = $this->getNamespace().$tokenId;
+        if ($this->storage->hasToken($namespacedId)) {
+            $value = $this->storage->getToken($namespacedId);
         } else {
             $value = $this->generator->generateToken();
 
-            $this->storage->setToken($tokenId, $value);
+            $this->storage->setToken($namespacedId, $value);
         }
 
         return new CsrfToken($tokenId, $value);
@@ -54,9 +86,10 @@ public function getToken($tokenId)
      */
     public function refreshToken($tokenId)
     {
+        $namespacedId = $this->getNamespace().$tokenId;
         $value = $this->generator->generateToken();
 
-        $this->storage->setToken($tokenId, $value);
+        $this->storage->setToken($namespacedId, $value);

         return new CsrfToken($tokenId, $value);
     }
@@ -66,18 +99,24 @@ public function refreshToken($tokenId)
      */
     public function removeToken($tokenId)
     {
-        return $this->storage->removeToken($tokenId);
+        return $this->storage->removeToken($this->getNamespace().$tokenId);
     }
 
     /**
      * {@inheritdoc}
      */
     public function isTokenValid(CsrfToken $token)
     {
-        if (!$this->storage->hasToken($token->getId())) {
+        $namespacedId = $this->getNamespace().$token->getId();
+        if (!$this->storage->hasToken($namespacedId)) {
             return false;
         }
 
-        return StringUtils::equals($this->storage->getToken($token->getId()), $token->getValue());
+        return StringUtils::equals($this->storage->getToken($namespacedId), $token->getValue());
+    }
+
+    private function getNamespace()
+    {
+        return is_callable($ns = $this->namespace) ? $ns() : $ns;
     }
 }
diff --git a/src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php b/src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php
@@ -12,6 +12,8 @@
 namespace Symfony\Component\Security\Csrf\Tests;
 
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Security\Csrf\CsrfToken;
 use Symfony\Component\Security\Csrf\CsrfTokenManager;
 
@@ -21,145 +23,202 @@
 class CsrfTokenManagerTest extends TestCase
 {
     /**
-     * @var \PHPUnit_Framework_MockObject_MockObject
+     * @dataProvider getManagerGeneratorAndStorage
      */
-    private $generator;
-
-    /**
-     * @var \PHPUnit_Framework_MockObject_MockObject
-     */
-    private $storage;
-
-    /**
-     * @var CsrfTokenManager
-     */
-    private $manager;
-
-    protected function setUp()
-    {
-        $this->generator = $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface')->getMock();
-        $this->storage = $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface')->getMock();
-        $this->manager = new CsrfTokenManager($this->generator, $this->storage);
-    }
-
-    protected function tearDown()
-    {
-        $this->generator = null;
-        $this->storage = null;
-        $this->manager = null;
-    }
-
-    public function testGetNonExistingToken()
+    public function testGetNonExistingToken($namespace, $manager, $storage, $generator)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('hasToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue(false));
 
-        $this->generator->expects($this->once())
+        $generator->expects($this->once())
             ->method('generateToken')
             ->will($this->returnValue('TOKEN'));
 
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('setToken')
-            ->with('token_id', 'TOKEN');
+            ->with($namespace.'token_id', 'TOKEN');
 
-        $token = $this->manager->getToken('token_id');
+        $token = $manager->getToken('token_id');
 
         $this->assertInstanceOf('Symfony\Component\Security\Csrf\CsrfToken', $token);
         $this->assertSame('token_id', $token->getId());
         $this->assertSame('TOKEN', $token->getValue());
     }
 
-    public function testUseExistingTokenIfAvailable()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testUseExistingTokenIfAvailable($namespace, $manager, $storage)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('hasToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue(true));
 
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('getToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue('TOKEN'));
 
-        $token = $this->manager->getToken('token_id');
+        $token = $manager->getToken('token_id');
 
         $this->assertInstanceOf('Symfony\Component\Security\Csrf\CsrfToken', $token);
         $this->assertSame('token_id', $token->getId());
         $this->assertSame('TOKEN', $token->getValue());
     }
 
-    public function testRefreshTokenAlwaysReturnsNewToken()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testRefreshTokenAlwaysReturnsNewToken($namespace, $manager, $storage, $generator)
     {
-        $this->storage->expects($this->never())
+        $storage->expects($this->never())
             ->method('hasToken');
 
-        $this->generator->expects($this->once())
+        $generator->expects($this->once())
             ->method('generateToken')
             ->will($this->returnValue('TOKEN'));
 
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('setToken')
-            ->with('token_id', 'TOKEN');
+            ->with($namespace.'token_id', 'TOKEN');
 
-        $token = $this->manager->refreshToken('token_id');
+        $token = $manager->refreshToken('token_id');
 
         $this->assertInstanceOf('Symfony\Component\Security\Csrf\CsrfToken', $token);
         $this->assertSame('token_id', $token->getId());
         $this->assertSame('TOKEN', $token->getValue());
     }
 
-    public function testMatchingTokenIsValid()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testMatchingTokenIsValid($namespace, $manager, $storage)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('hasToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue(true));
 
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
                ->method('getToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue('TOKEN'));
 
-        $this->assertTrue($this->manager->isTokenValid(new CsrfToken('token_id', 'TOKEN')));
+        $this->assertTrue($manager->isTokenValid(new CsrfToken('token_id', 'TOKEN')));
     }
 
-    public function testNonMatchingTokenIsNotValid()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testNonMatchingTokenIsNotValid($namespace, $manager, $storage)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('hasToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue(true));
 
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('getToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue('TOKEN'));
 
-        $this->assertFalse($this->manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
+        $this->assertFalse($manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
     }
 
-    public function testNonExistingTokenIsNotValid()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testNonExistingTokenIsNotValid($namespace, $manager, $storage)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('hasToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue(false));
 
-        $this->storage->expects($this->never())
+        $storage->expects($this->never())
             ->method('getToken');
 
-        $this->assertFalse($this->manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
+        $this->assertFalse($manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
     }
 
-    public function testRemoveToken()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testRemoveToken($namespace, $manager, $storage)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('removeToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue('REMOVED_TOKEN'));
 
-        $this->assertSame('REMOVED_TOKEN', $this->manager->removeToken('token_id'));
        $this->assertSame('REMOVED_TOKEN', $manager->removeToken('token_id'));
+    }
+
+    public function testNamespaced()
+    {
+        $generator = $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface')->getMock();
+        $storage = $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface')->getMock();
+
+        $requestStack = new RequestStack();
+        $requestStack->push(new Request(array(), array(), array(), array(), array(), array('HTTPS' => 'on')));
+
+        $manager = new CsrfTokenManager($generator, $storage, null, $requestStack);
+
+        $token = $manager->getToken('foo');
+        $this->assertSame('foo', $token->getId());
+    }
+
+    public function getManagerGeneratorAndStorage()
+    {
+        $data = array();
+
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('', new CsrfTokenManager($generator, $storage, ''), $storage, $generator);
+
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('https-', new CsrfTokenManager($generator, $storage), $storage, $generator);
+
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('aNamespace-', new CsrfTokenManager($generator, $storage, 'aNamespace-'), $storage, $generator);
+
+        $requestStack = new RequestStack();
+        $requestStack->push(new Request(array(), array(), array(), array(), array(), array('HTTPS' => 'on')));
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('https-', new CsrfTokenManager($generator, $storage, $requestStack), $storage, $generator);
+
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('generated-', new CsrfTokenManager($generator, $storage, function () {
+            return 'generated-';
+        }), $storage, $generator);
+
+        $requestStack = new RequestStack();
+        $requestStack->push(new Request());
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('', new CsrfTokenManager($generator, $storage, $requestStack), $storage, $generator);
+
+        return $data;
+    }
+
+    private function getGeneratorAndStorage()
+    {
+        return array(
+            $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface')->getMock(),
+            $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface')->getMock(),
+        );
+    }
+
+    public function setUp()
+    {
+        $_SERVER['HTTPS'] = 'on';
+    }
+
+    public function tearDown()
+    {
+        parent::tearDown();
+
+        unset($_SERVER['HTTPS']);
     }
 }