Skip to content

Commit b4dbdd7

Browse files
fabpotfabpot
committed
security #24992 Namespace generated CSRF tokens depending of the current 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
2 parents 2c2253d + cdb4271 commit b4dbdd7

File tree

3 files changed

+174
-75
lines changed

3 files changed

+174
-75
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
<service id="security.csrf.token_manager" class="%security.csrf.token_manager.class%">
2323
<argument type="service" id="security.csrf.token_generator" />
2424
<argument type="service" id="security.csrf.token_storage" />
25+
<argument type="service" id="request_stack" on-invalid="ignore" />
2526
</service>
2627
</services>
2728
</container>

‎src/Symfony/Component/Security/Csrf/CsrfTokenManager.php

+47-8
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Security\Csrf;
1313

14+
use Symfony\Component\HttpFoundation\RequestStack;
15+
use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
1416
use Symfony\Component\Security\Core\Util\StringUtils;
1517
use Symfony\Component\Security\Csrf\TokenGenerator\UriSafeTokenGenerator;
1618
use Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface;
@@ -21,29 +23,59 @@
2123
* Default implementation of {@link CsrfTokenManagerInterface}.
2224
*
2325
* @author Bernhard Schussek <bschussek@gmail.com>
26+
* @author Kévin Dunglas <dunglas@gmail.com>
2427
*/
2528
class CsrfTokenManager implements CsrfTokenManagerInterface
2629
{
2730
private $generator;
2831
private $storage;
32+
private $namespace;
2933

30-
public function __construct(TokenGeneratorInterface $generator = null, TokenStorageInterface $storage = null)
34+
/**
35+
* @param null|string|RequestStack|callable $namespace
36+
* * null: generates a namespace using $_SERVER['HTTPS']
37+
* * string: uses the given string
38+
* * RequestStack: generates a namespace using the current master request
39+
* * callable: uses the result of this callable (must return a string)
40+
*/
41+
public function __construct(TokenGeneratorInterface $generator = null, TokenStorageInterface $storage = null, $namespace = null)
3142
{
3243
$this->generator = $generator ?: new UriSafeTokenGenerator();
3344
$this->storage = $storage ?: new NativeSessionTokenStorage();
45+
46+
$superGlobalNamespaceGenerator = function () {
47+
return !empty($_SERVER['HTTPS']) && 'off' !== strtolower($_SERVER['HTTPS']) ? 'https-' : '';
48+
};
49+
50+
if (null === $namespace) {
51+
$this->namespace = $superGlobalNamespaceGenerator;
52+
} elseif ($namespace instanceof RequestStack) {
53+
$this->namespace = function () use ($namespace, $superGlobalNamespaceGenerator) {
54+
if ($request = $namespace->getMasterRequest()) {
55+
return $request->isSecure() ? 'https-' : '';
56+
}
57+
58+
return $superGlobalNamespaceGenerator();
59+
};
60+
} elseif (is_callable($namespace) || is_string($namespace)) {
61+
$this->namespace = $namespace;
62+
} else {
63+
throw new InvalidArgumentException(sprintf('$namespace must be a string, a callable returning a string, null or an instance of "RequestStack". "%s" given.', gettype($namespace)));
64+
}
3465
}
3566

3667
/**
3768
* {@inheritdoc}
3869
*/
3970
public function getToken($tokenId)
4071
{
41-
if ($this->storage->hasToken($tokenId)) {
42-
$value = $this->storage->getToken($tokenId);
72+
$namespacedId = $this->getNamespace().$tokenId;
73+
if ($this->storage->hasToken($namespacedId)) {
74+
$value = $this->storage->getToken($namespacedId);
4375
} else {
4476
$value = $this->generator->generateToken();
4577

46-
$this->storage->setToken($tokenId, $value);
78+
$this->storage->setToken($namespacedId, $value);
4779
}
4880

4981
return new CsrfToken($tokenId, $value);
@@ -54,9 +86,10 @@ public function getToken($tokenId)
5486
*/
5587
public function refreshToken($tokenId)
5688
{
89+
$namespacedId = $this->getNamespace().$tokenId;
5790
$value = $this->generator->generateToken();
5891

59-
$this->storage->setToken($tokenId, $value);
92+
$this->storage->setToken($namespacedId, $value);
6093

6194
return new CsrfToken($tokenId, $value);
6295
}
@@ -66,18 +99,24 @@ public function refreshToken($tokenId)
6699
*/
67100
public function removeToken($tokenId)
68101
{
69-
return $this->storage->removeToken($tokenId);
102+
return $this->storage->removeToken($this->getNamespace().$tokenId);
70103
}
71104

72105
/**
73106
* {@inheritdoc}
74107
*/
75108
public function isTokenValid(CsrfToken $token)
76109
{
77-
if (!$this->storage->hasToken($token->getId())) {
110+
$namespacedId = $this->getNamespace().$token->getId();
111+
if (!$this->storage->hasToken($namespacedId)) {
78112
return false;
79113
}
80114

81-
return StringUtils::equals($this->storage->getToken($token->getId()), $token->getValue());
115+
return StringUtils::equals($this->storage->getToken($namespacedId), $token->getValue());
116+
}
117+
118+
private function getNamespace()
119+
{
120+
return is_callable($ns = $this->namespace) ? $ns() : $ns;
82121
}
83122
}

‎src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php

+126-67
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
namespace Symfony\Component\Security\Csrf\Tests;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\HttpFoundation\Request;
16+
use Symfony\Component\HttpFoundation\RequestStack;
1517
use Symfony\Component\Security\Csrf\CsrfToken;
1618
use Symfony\Component\Security\Csrf\CsrfTokenManager;
1719

@@ -21,145 +23,202 @@
2123
class CsrfTokenManagerTest extends TestCase
2224
{
2325
/**
24-
* @var \PHPUnit_Framework_MockObject_MockObject
26+
* @dataProvider getManagerGeneratorAndStorage
2527
*/
26-
private $generator;
27-
28-
/**
29-
* @var \PHPUnit_Framework_MockObject_MockObject
30-
*/
31-
private $storage;
32-
33-
/**
34-
* @var CsrfTokenManager
35-
*/
36-
private $manager;
37-
38-
protected function setUp()
39-
{
40-
$this->generator = $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface')->getMock();
41-
$this->storage = $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface')->getMock();
42-
$this->manager = new CsrfTokenManager($this->generator, $this->storage);
43-
}
44-
45-
protected function tearDown()
46-
{
47-
$this->generator = null;
48-
$this->storage = null;
49-
$this->manager = null;
50-
}
51-
52-
public function testGetNonExistingToken()
28+
public function testGetNonExistingToken($namespace, $manager, $storage, $generator)
5329
{
54-
$this->storage->expects($this->once())
30+
$storage->expects($this->once())
5531
->method('hasToken')
56-
->with('token_id')
32+
->with($namespace.'token_id')
5733
->will($this->returnValue(false));
5834

59-
$this->generator->expects($this->once())
35+
$generator->expects($this->once())
6036
->method('generateToken')
6137
->will($this->returnValue('TOKEN'));
6238

63-
$this->storage->expects($this->once())
39+
$storage->expects($this->once())
6440
->method('setToken')
65-
->with('token_id', 'TOKEN');
41+
->with($namespace.'token_id', 'TOKEN');
6642

67-
$token = $this->manager->getToken('token_id');
43+
$token = $manager->getToken('token_id');
6844

6945
$this->assertInstanceOf('Symfony\Component\Security\Csrf\CsrfToken', $token);
7046
$this->assertSame('token_id', $token->getId());
7147
$this->assertSame('TOKEN', $token->getValue());
7248
}
7349

74-
public function testUseExistingTokenIfAvailable()
50+
/**
51+
* @dataProvider getManagerGeneratorAndStorage
52+
*/
53+
public function testUseExistingTokenIfAvailable($namespace, $manager, $storage)
7554
{
76-
$this->storage->expects($this->once())
55+
$storage->expects($this->once())
7756
->method('hasToken')
78-
->with('token_id')
57+
->with($namespace.'token_id')
7958
->will($this->returnValue(true));
8059

81-
$this->storage->expects($this->once())
60+
$storage->expects($this->once())
8261
->method('getToken')
83-
->with('token_id')
62+
->with($namespace.'token_id')
8463
->will($this->returnValue('TOKEN'));
8564

86-
$token = $this->manager->getToken('token_id');
65+
$token = $manager->getToken('token_id');
8766

8867
$this->assertInstanceOf('Symfony\Component\Security\Csrf\CsrfToken', $token);
8968
$this->assertSame('token_id', $token->getId());
9069
$this->assertSame('TOKEN', $token->getValue());
9170
}
9271

93-
public function testRefreshTokenAlwaysReturnsNewToken()
72+
/**
73+
* @dataProvider getManagerGeneratorAndStorage
74+
*/
75+
public function testRefreshTokenAlwaysReturnsNewToken($namespace, $manager, $storage, $generator)
9476
{
95-
$this->storage->expects($this->never())
77+
$storage->expects($this->never())
9678
->method('hasToken');
9779

98-
$this->generator->expects($this->once())
80+
$generator->expects($this->once())
9981
->method('generateToken')
10082
->will($this->returnValue('TOKEN'));
10183

102-
$this->storage->expects($this->once())
84+
$storage->expects($this->once())
10385
->method('setToken')
104-
->with('token_id', 'TOKEN');
86+
->with($namespace.'token_id', 'TOKEN');
10587

106-
$token = $this->manager->refreshToken('token_id');
88+
$token = $manager->refreshToken('token_id');
10789

10890
$this->assertInstanceOf('Symfony\Component\Security\Csrf\CsrfToken', $token);
10991
$this->assertSame('token_id', $token->getId());
11092
$this->assertSame('TOKEN', $token->getValue());
11193
}
11294

113-
public function testMatchingTokenIsValid()
95+
/**
96+
* @dataProvider getManagerGeneratorAndStorage
97+
*/
98+
public function testMatchingTokenIsValid($namespace, $manager, $storage)
11499
{
115-
$this->storage->expects($this->once())
100+
$storage->expects($this->once())
116101
->method('hasToken')
117-
->with('token_id')
102+
->with($namespace.'token_id')
118103
->will($this->returnValue(true));
119104

120-
$this->storage->expects($this->once())
105+
$storage->expects($this->once())
121106
->method('getToken')
122-
->with('token_id')
107+
->with($namespace.'token_id')
123108
->will($this->returnValue('TOKEN'));
124109

125-
$this->assertTrue($this->manager->isTokenValid(new CsrfToken('token_id', 'TOKEN')));
110+
$this->assertTrue($manager->isTokenValid(new CsrfToken('token_id', 'TOKEN')));
126111
}
127112

128-
public function testNonMatchingTokenIsNotValid()
113+
/**
114+
* @dataProvider getManagerGeneratorAndStorage
115+
*/
116+
public function testNonMatchingTokenIsNotValid($namespace, $manager, $storage)
129117
{
130-
$this->storage->expects($this->once())
118+
$storage->expects($this->once())
131119
->method('hasToken')
132-
->with('token_id')
120+
->with($namespace.'token_id')
133121
->will($this->returnValue(true));
134122

135-
$this->storage->expects($this->once())
123+
$storage->expects($this->once())
136124
->method('getToken')
137-
->with('token_id')
125+
->with($namespace.'token_id')
138126
->will($this->returnValue('TOKEN'));
139127

140-
$this->assertFalse($this->manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
128+
$this->assertFalse($manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
141129
}
142130

143-
public function testNonExistingTokenIsNotValid()
131+
/**
132+
* @dataProvider getManagerGeneratorAndStorage
133+
*/
134+
public function testNonExistingTokenIsNotValid($namespace, $manager, $storage)
144135
{
145-
$this->storage->expects($this->once())
136+
$storage->expects($this->once())
146137
->method('hasToken')
147-
->with('token_id')
138+
->with($namespace.'token_id')
148139
->will($this->returnValue(false));
149140

150-
$this->storage->expects($this->never())
141+
$storage->expects($this->never())
151142
->method('getToken');
152143

153-
$this->assertFalse($this->manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
144+
$this->assertFalse($manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
154145
}
155146

156-
public function testRemoveToken()
147+
/**
148+
* @dataProvider getManagerGeneratorAndStorage
149+
*/
150+
public function testRemoveToken($namespace, $manager, $storage)
157151
{
158-
$this->storage->expects($this->once())
152+
$storage->expects($this->once())
159153
->method('removeToken')
160-
->with('token_id')
154+
->with($namespace.'token_id')
161155
->will($this->returnValue('REMOVED_TOKEN'));
162156

163-
$this->assertSame('REMOVED_TOKEN', $this->manager->removeToken('token_id'));
157+
$this->assertSame('REMOVED_TOKEN', $manager->removeToken('token_id'));
158+
}
159+
160+
public function testNamespaced()
161+
{
162+
$generator = $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface')->getMock();
163+
$storage = $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface')->getMock();
164+
165+
$requestStack = new RequestStack();
166+
$requestStack->push(new Request(array(), array(), array(), array(), array(), array('HTTPS' => 'on')));
167+
168+
$manager = new CsrfTokenManager($generator, $storage, null, $requestStack);
169+
170+
$token = $manager->getToken('foo');
171+
$this->assertSame('foo', $token->getId());
172+
}
173+
174+
public function getManagerGeneratorAndStorage()
175+
{
176+
$data = array();
177+
178+
list($generator, $storage) = $this->getGeneratorAndStorage();
179+
$data[] = array('', new CsrfTokenManager($generator, $storage, ''), $storage, $generator);
180+
181+
list($generator, $storage) = $this->getGeneratorAndStorage();
182+
$data[] = array('https-', new CsrfTokenManager($generator, $storage), $storage, $generator);
183+
184+
list($generator, $storage) = $this->getGeneratorAndStorage();
185+
$data[] = array('aNamespace-', new CsrfTokenManager($generator, $storage, 'aNamespace-'), $storage, $generator);
186+
187+
$requestStack = new RequestStack();
188+
$requestStack->push(new Request(array(), array(), array(), array(), array(), array('HTTPS' => 'on')));
189+
list($generator, $storage) = $this->getGeneratorAndStorage();
190+
$data[] = array('https-', new CsrfTokenManager($generator, $storage, $requestStack), $storage, $generator);
191+
192+
list($generator, $storage) = $this->getGeneratorAndStorage();
193+
$data[] = array('generated-', new CsrfTokenManager($generator, $storage, function () {
194+
return 'generated-';
195+
}), $storage, $generator);
196+
197+
$requestStack = new RequestStack();
198+
$requestStack->push(new Request());
199+
list($generator, $storage) = $this->getGeneratorAndStorage();
200+
$data[] = array('', new CsrfTokenManager($generator, $storage, $requestStack), $storage, $generator);
201+
202+
return $data;
203+
}
204+
205+
private function getGeneratorAndStorage()
206+
{
207+
return array(
208+
$this->getMockBuilder('Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface')->getMock(),
209+
$this->getMockBuilder('Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface')->getMock(),
210+
);
211+
}
212+
213+
public function setUp()
214+
{
215+
$_SERVER['HTTPS'] = 'on';
216+
}
217+
218+
public function tearDown()
219+
{
220+
parent::tearDown();
221+
222+
unset($_SERVER['HTTPS']);
164223
}
165224
}

0 commit comments

Comments
 (0)