Skip to content

Commit

Permalink
security #24992 Namespace generated CSRF tokens depending of the curr…
Browse files Browse the repository at this point in the history 
…ent scheme (dunglas)

This PR was merged into the 2.7 branch.

Discussion
----------

Namespace generated CSRF tokens depending of the current scheme

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

<!--
- Bug fixes must be submitted against the lowest branch where they apply
  (lowest branches are regularly merged to upper ones so they get the fixes too).
- Features and deprecations must be submitted against the master branch.
- Please fill in this template according to the PR you're about to submit.
- Replace this comment by a description of what your PR is solving.
-->

Commits
-------

cdb4271 [Security] Namespace generated CSRF tokens depending of the current scheme
  • Loading branch information
@fabpot
fabpot committed Nov 16, 2017
2 parents 2c2253d + cdb4271 commit b4dbdd7
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 75 deletions.
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml
View file
Expand Up @@ -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>
55 changes: 47 additions & 8 deletions src/Symfony/Component/Security/Csrf/CsrfTokenManager.php
View file
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
}
Expand All @@ -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;
}
}
193 changes: 126 additions & 67 deletions src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php
View file
Expand Up @@ -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;

Expand All @@ -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']);
}
}

0 comments on commit b4dbdd7

Please sign in to comment.