Skip to content

Commit 2ee4f02

Browse files
nicolas-grekasnicolas-grekas
committed
[Security] Make statefull pages NOT turn responses private when the token is not used
1 parent a31f4aa commit 2ee4f02

21 files changed

+317
-54
lines changed

‎src/Symfony/Bridge/Monolog/Processor/TokenProcessor.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
namespace Symfony\Bridge\Monolog\Processor;
1313

1414
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
15+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage;
16+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorageInterface;
1517

1618
/**
1719
* Adds the current security token to the log entry.
@@ -24,13 +26,13 @@ class TokenProcessor implements ProcessorInterface
2426

2527
public function __construct(TokenStorageInterface $tokenStorage)
2628
{
27-
$this->tokenStorage = $tokenStorage;
29+
$this->tokenStorage = $tokenStorage instanceof UsageTrackingTokenStorageInterface ? $tokenStorage : new UsageTrackingTokenStorage($tokenStorage);
2830
}
2931

3032
public function __invoke(array $records)
3133
{
3234
$records['extra']['token'] = null;
33-
if (null !== $token = $this->tokenStorage->getToken()) {
35+
if (null !== $token = $this->tokenStorage->getToken(false)) {
3436
$records['extra']['token'] = array(
3537
'username' => $token->getUsername(),
3638
'authenticated' => $token->isAuthenticated(),

‎src/Symfony/Bridge/Monolog/Tests/Processor/TokenProcessorTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Bridge\Monolog\Processor\TokenProcessor;
16-
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
16+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorageInterface;
1717
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
1818

1919
/**
@@ -26,8 +26,8 @@ class TokenProcessorTest extends TestCase
2626
public function testProcessor()
2727
{
2828
$token = new UsernamePasswordToken('user', 'password', 'provider', array('ROLE_USER'));
29-
$tokenStorage = $this->getMockBuilder(TokenStorageInterface::class)->getMock();
30-
$tokenStorage->method('getToken')->willReturn($token);
29+
$tokenStorage = $this->getMockBuilder(UsageTrackingTokenStorageInterface::class)->getMock();
30+
$tokenStorage->method('getToken')->with(false)->willReturn($token);
3131

3232
$processor = new TokenProcessor($tokenStorage);
3333
$record = array('extra' => array());

‎src/Symfony/Bridge/Monolog/composer.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,12 @@
2424
"require-dev": {
2525
"symfony/console": "~3.4|~4.0",
2626
"symfony/event-dispatcher": "~3.4|~4.0",
27-
"symfony/security-core": "~3.4|~4.0",
27+
"symfony/security-core": "~4.2",
2828
"symfony/var-dumper": "~3.4|~4.0"
2929
},
3030
"conflict": {
31-
"symfony/http-foundation": "<3.4"
31+
"symfony/http-foundation": "<3.4",
32+
"symfony/security-core": "<4.2"
3233
},
3334
"suggest": {
3435
"symfony/http-kernel": "For using the debugging handlers together with the response life cycle of the HTTP kernel.",

‎src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
use Symfony\Component\HttpKernel\DataCollector\DataCollector;
1919
use Symfony\Component\HttpKernel\DataCollector\LateDataCollectorInterface;
2020
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
21+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage;
22+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorageInterface;
2123
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
2224
use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager;
2325
use Symfony\Component\Security\Core\Role\Role;
@@ -44,7 +46,7 @@ class SecurityDataCollector extends DataCollector implements LateDataCollectorIn
4446

4547
public function __construct(TokenStorageInterface $tokenStorage = null, RoleHierarchyInterface $roleHierarchy = null, LogoutUrlGenerator $logoutUrlGenerator = null, AccessDecisionManagerInterface $accessDecisionManager = null, FirewallMapInterface $firewallMap = null, TraceableFirewallListener $firewall = null)
4648
{
47-
$this->tokenStorage = $tokenStorage;
49+
$this->tokenStorage = null === $tokenStorage || $tokenStorage instanceof UsageTrackingTokenStorageInterface ? $tokenStorage : new UsageTrackingTokenStorage($tokenStorage);
4850
$this->roleHierarchy = $roleHierarchy;
4951
$this->logoutUrlGenerator = $logoutUrlGenerator;
5052
$this->accessDecisionManager = $accessDecisionManager;
@@ -73,7 +75,7 @@ public function collect(Request $request, Response $response, \Exception $except
7375
'inherited_roles' => array(),
7476
'supports_role_hierarchy' => null !== $this->roleHierarchy,
7577
);
76-
} elseif (null === $token = $this->tokenStorage->getToken()) {
78+
} elseif (null === $token = $this->tokenStorage->getToken(false)) {
7779
$this->data = array(
7880
'enabled' => true,
7981
'authenticated' => false,

‎src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,11 @@
2121
</service>
2222
<service id="Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface" alias="security.authorization_checker" />
2323

24-
<service id="security.token_storage" class="Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage" public="true">
24+
<service id="security.token_storage" class="Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage" public="true">
2525
<tag name="kernel.reset" method="setToken" />
2626
</service>
2727
<service id="Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface" alias="security.token_storage" />
28+
<service id="Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorageInterface" alias="security.token_storage" />
2829

2930
<service id="security.helper" class="Symfony\Component\Security\Core\Security">
3031
<argument type="service_locator">

‎src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
use Symfony\Component\EventDispatcher\EventDispatcher;
2020
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
2121
use Symfony\Component\HttpKernel\HttpKernelInterface;
22-
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
22+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage;
2323
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2424
use Symfony\Component\Security\Core\Role\Role;
2525
use Symfony\Component\Security\Core\Role\RoleHierarchy;
@@ -51,7 +51,8 @@ public function testCollectWhenSecurityIsDisabled()
5151

5252
public function testCollectWhenAuthenticationTokenIsNull()
5353
{
54-
$tokenStorage = new TokenStorage();
54+
$tokenStorage = new UsageTrackingTokenStorage();
55+
$tokenStorage->setToken(null, \Closure::fromCallable(array($this, 'fail')));
5556
$collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
5657
$collector->collect($this->getRequest(), $this->getResponse());
5758

@@ -71,8 +72,8 @@ public function testCollectWhenAuthenticationTokenIsNull()
7172
/** @dataProvider provideRoles */
7273
public function testCollectAuthenticationTokenAndRoles(array $roles, array $normalizedRoles, array $inheritedRoles)
7374
{
74-
$tokenStorage = new TokenStorage();
75-
$tokenStorage->setToken(new UsernamePasswordToken('hhamon', 'P4$$w0rD', 'provider', $roles));
75+
$tokenStorage = new UsageTrackingTokenStorage();
76+
$tokenStorage->setToken(new UsernamePasswordToken('hhamon', 'P4$$w0rD', 'provider', $roles), \Closure::fromCallable(array($this, 'fail')));
7677

7778
$collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
7879
$collector->collect($this->getRequest(), $this->getResponse());
@@ -99,8 +100,8 @@ public function testCollectImpersonatedToken()
99100
new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $adminToken),
100101
);
101102

102-
$tokenStorage = new TokenStorage();
103-
$tokenStorage->setToken(new UsernamePasswordToken('hhamon', 'P4$$w0rD', 'provider', $userRoles));
103+
$tokenStorage = new UsageTrackingTokenStorage();
104+
$tokenStorage->setToken(new UsernamePasswordToken('hhamon', 'P4$$w0rD', 'provider', $userRoles), \Closure::fromCallable(array($this, 'fail')));
104105

105106
$collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
106107
$collector->collect($this->getRequest(), $this->getResponse());

‎src/Symfony/Component/HttpFoundation/Session/Session.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ public function count()
146146
*
147147
* @internal
148148
*/
149-
public function getUsageIndex()
149+
public function &getUsageIndex()
150150
{
151151
return $this->usageIndex;
152152
}

‎src/Symfony/Component/Security/Core/Authentication/Token/Storage/UsageTrackingTokenStorage.php

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Core\Authentication\Token\Storage;
13+
14+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
15+
16+
/**
17+
* A token storage that optionally calls a closure when the token is accessed.
18+
*
19+
* @author Nicolas Grekas <p@tchwork.com>
20+
*/
21+
class UsageTrackingTokenStorage implements UsageTrackingTokenStorageInterface
22+
{
23+
private $storage;
24+
private $usageTracker;
25+
26+
public function __construct(TokenStorageInterface $storage = null)
27+
{
28+
$this->storage = $storage ?? new TokenStorage();
29+
}
30+
31+
/**
32+
* {@inheritdoc}
33+
*/
34+
public function getToken(bool $trackUsage = true): ?TokenInterface
35+
{
36+
if ($trackUsage && $usageTracker = $this->usageTracker) {
37+
$usageTracker();
38+
}
39+
40+
return $this->storage->getToken();
41+
}
42+
43+
/**
44+
* {@inheritdoc}
45+
*/
46+
public function setToken(TokenInterface $token = null, \Closure $usageTracker = null): void
47+
{
48+
$this->usageTracker = $usageTracker;
49+
$this->storage->setToken($token);
50+
}
51+
}

‎src/Symfony/Component/Security/Core/Authentication/Token/Storage/UsageTrackingTokenStorageInterface.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Core\Authentication\Token\Storage;
13+
14+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
15+
16+
/**
17+
* A token storage that optionally calls a closure when the token is accessed.
18+
*
19+
* @author Nicolas Grekas <p@tchwork.com>
20+
*/
21+
interface UsageTrackingTokenStorageInterface extends TokenStorageInterface
22+
{
23+
/**
24+
* {@inheritdoc}
25+
*/
26+
public function getToken(bool $trackUsage = true): ?TokenInterface;
27+
28+
/**
29+
* {@inheritdoc}
30+
*/
31+
public function setToken(TokenInterface $token = null, \Closure $usageTracker = null): void;
32+
}

‎src/Symfony/Component/Security/Core/Tests/Authentication/Token/Storage/UsageTrackingTokenStorageTest.php

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Core\Tests\Authentication\Token\Storage;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage;
16+
17+
class UsageTrackingTokenStorageTest extends TestCase
18+
{
19+
public function testGetSetToken()
20+
{
21+
$counter = 0;
22+
$usageTracker = function () use (&$counter) { ++$counter; };
23+
24+
$tokenStorage = new UsageTrackingTokenStorage();
25+
$this->assertNull($tokenStorage->getToken());
26+
$token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
27+
28+
$tokenStorage->setToken($token);
29+
$this->assertSame($token, $tokenStorage->getToken());
30+
31+
$tokenStorage->setToken($token, $usageTracker);
32+
$this->assertSame($token, $tokenStorage->getToken(false));
33+
$this->assertSame(0, $counter);
34+
$this->assertSame($token, $tokenStorage->getToken());
35+
$this->assertSame(1, $counter);
36+
37+
$tokenStorage->setToken(null, $usageTracker);
38+
$this->assertNull($tokenStorage->getToken(false));
39+
$this->assertSame(1, $counter);
40+
$this->assertNull($tokenStorage->getToken());
41+
$this->assertSame(2, $counter);
42+
}
43+
}

‎src/Symfony/Component/Security/Http/Firewall/AccessListener.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
1515
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
1616
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
17+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage;
18+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorageInterface;
1719
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
1820
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
1921
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
@@ -33,7 +35,7 @@ class AccessListener implements ListenerInterface
3335

3436
public function __construct(TokenStorageInterface $tokenStorage, AccessDecisionManagerInterface $accessDecisionManager, AccessMapInterface $map, AuthenticationManagerInterface $authManager)
3537
{
36-
$this->tokenStorage = $tokenStorage;
38+
$this->tokenStorage = $tokenStorage instanceof UsageTrackingTokenStorageInterface ? $tokenStorage : new UsageTrackingTokenStorage($tokenStorage);
3739
$this->accessDecisionManager = $accessDecisionManager;
3840
$this->map = $map;
3941
$this->authManager = $authManager;
@@ -47,18 +49,18 @@ public function __construct(TokenStorageInterface $tokenStorage, AccessDecisionM
4749
*/
4850
public function handle(GetResponseEvent $event)
4951
{
50-
if (null === $token = $this->tokenStorage->getToken()) {
51-
throw new AuthenticationCredentialsNotFoundException('A Token was not found in the TokenStorage.');
52-
}
53-
5452
$request = $event->getRequest();
5553

5654
list($attributes) = $this->map->getPatterns($request);
5755

58-
if (null === $attributes) {
56+
if (!$attributes) {
5957
return;
6058
}
6159

60+
if (null === $token = $this->tokenStorage->getToken()) {
61+
throw new AuthenticationCredentialsNotFoundException('A Token was not found in the TokenStorage.');
62+
}
63+
6264
if (!$token->isAuthenticated()) {
6365
$token = $this->authManager->authenticate($token);
6466
$this->tokenStorage->setToken($token);

‎src/Symfony/Component/Security/Http/Firewall/AnonymousAuthenticationListener.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
1717
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
1818
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
19+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage;
20+
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorageInterface;
1921
use Symfony\Component\Security\Core\Exception\AuthenticationException;
2022

2123
/**
@@ -33,7 +35,7 @@ class AnonymousAuthenticationListener implements ListenerInterface
3335

3436
public function __construct(TokenStorageInterface $tokenStorage, string $secret, LoggerInterface $logger = null, AuthenticationManagerInterface $authenticationManager = null)
3537
{
36-
$this->tokenStorage = $tokenStorage;
38+
$this->tokenStorage = $tokenStorage instanceof UsageTrackingTokenStorageInterface ? $tokenStorage : new UsageTrackingTokenStorage($tokenStorage);
3739
$this->secret = $secret;
3840
$this->authenticationManager = $authenticationManager;
3941
$this->logger = $logger;
@@ -44,7 +46,7 @@ public function __construct(TokenStorageInterface $tokenStorage, string $secret,
4446
*/
4547
public function handle(GetResponseEvent $event)
4648
{
47-
if (null !== $this->tokenStorage->getToken()) {
49+
if (null !== $this->tokenStorage->getToken(false)) {
4850
return;
4951
}
5052

0 commit comments

Comments
 (0)