feature #40785 [Security] Deprecate using UsageTrackingTokenStorage outside the request-response cycle (wouterj)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[Security] Deprecate using UsageTrackingTokenStorage outside the request-response cycle

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | yes
| Tickets       | Fix #40778
| License       | MIT
| Doc PR        | -

Currently, you get an "There is currently no session available" exception when using the `security.token_storage` service outside the main request-response cycle (e.g. in a `kernel.terminate` listener). This PR deprecates such usage and requires developers to update their definitions to explicitly use `security.untracked_token_storage` instead.

A different solution would be to silently disable tracking in these cases, but I think that might create some unnecessary technical debt.

Commits
-------

7452476156 [Security] Fix UsageTrackingTokenStorage outside the request cycle
This commit is contained in:
Fabien Potencier 2021-04-13 08:27:23 +02:00
commit 64cc54835d
4 changed files with 56 additions and 8 deletions

View File

@ -102,6 +102,9 @@ Routing
Security Security
-------- --------
* Deprecate using `UsageTrackingTokenStorage` with tracking enabled without a main request. Use the untracked token
storage (service ID: `security.untracked_token_storage`) instead, or disable usage tracking
completely using `UsageTrackingTokenStorage::disableUsageTracking()`.
* [BC BREAK] Remove method `checkIfCompletelyResolved()` from `PassportInterface`, checking that passport badges are * [BC BREAK] Remove method `checkIfCompletelyResolved()` from `PassportInterface`, checking that passport badges are
resolved is up to `AuthenticatorManager` resolved is up to `AuthenticatorManager`
* Deprecate class `User`, use `InMemoryUser` or your own implementation instead. * Deprecate class `User`, use `InMemoryUser` or your own implementation instead.

View File

@ -24,6 +24,7 @@ CHANGELOG
* Add `LegacyPasswordAuthenticatedUserInterface` for user classes that use user-provided salts in addition to passwords * Add `LegacyPasswordAuthenticatedUserInterface` for user classes that use user-provided salts in addition to passwords
* Deprecate all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead * Deprecate all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead
* Deprecate the `SessionInterface $session` constructor argument of `SessionTokenStorage`, inject a `\Symfony\Component\HttpFoundation\RequestStack $requestStack` instead * Deprecate the `SessionInterface $session` constructor argument of `SessionTokenStorage`, inject a `\Symfony\Component\HttpFoundation\RequestStack $requestStack` instead
* Deprecate using `UsageTrackingTokenStorage` without a main request
* Deprecate the `session` service provided by the ServiceLocator injected in `UsageTrackingTokenStorage`, provide a `request_stack` service instead * Deprecate the `session` service provided by the ServiceLocator injected in `UsageTrackingTokenStorage`, provide a `request_stack` service instead
* Deprecate using `SessionTokenStorage` outside a request context, it will throw a `SessionNotFoundException` in Symfony 6.0 * Deprecate using `SessionTokenStorage` outside a request context, it will throw a `SessionNotFoundException` in Symfony 6.0
* Randomize CSRF tokens to harden BREACH attacks * Randomize CSRF tokens to harden BREACH attacks

View File

@ -39,7 +39,7 @@ final class UsageTrackingTokenStorage implements TokenStorageInterface, ServiceS
*/ */
public function getToken(): ?TokenInterface public function getToken(): ?TokenInterface
{ {
if ($this->enableUsageTracking) { if ($this->shouldTrackUsage()) {
// increments the internal session usage index // increments the internal session usage index
$this->getSession()->getMetadataBag(); $this->getSession()->getMetadataBag();
} }
@ -54,7 +54,7 @@ final class UsageTrackingTokenStorage implements TokenStorageInterface, ServiceS
{ {
$this->storage->setToken($token); $this->storage->setToken($token);
if ($token && $this->enableUsageTracking) { if ($token && $this->shouldTrackUsage()) {
// increments the internal session usage index // increments the internal session usage index
$this->getSession()->getMetadataBag(); $this->getSession()->getMetadataBag();
} }
@ -88,4 +88,24 @@ final class UsageTrackingTokenStorage implements TokenStorageInterface, ServiceS
return $this->container->get('request_stack')->getSession(); return $this->container->get('request_stack')->getSession();
} }
private function shouldTrackUsage(): bool
{
if (!$this->enableUsageTracking) {
return false;
}
// BC for symfony/security-bundle < 5.3
if ($this->container->has('session')) {
return true;
}
if (!$this->container->get('request_stack')->getMainRequest()) {
trigger_deprecation('symfony/security-core', '5.3', 'Using "%s" (service ID: "security.token_storage") outside the request-response cycle is deprecated, use the "%s" class (service ID: "security.untracked_token_storage") instead or disable usage tracking using "disableUsageTracking()".', __CLASS__, TokenStorage::class);
return false;
}
return true;
}
} }

View File

@ -13,31 +13,37 @@ namespace Symfony\Component\Security\Core\Tests\Authentication\Token\Storage;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Psr\Container\ContainerInterface; use Psr\Container\ContainerInterface;
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\HttpFoundation\Session\SessionInterface;
use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\NullToken;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Contracts\Service\ServiceLocatorTrait; use Symfony\Contracts\Service\ServiceLocatorTrait;
class UsageTrackingTokenStorageTest extends TestCase class UsageTrackingTokenStorageTest extends TestCase
{ {
use ExpectDeprecationTrait;
public function testGetSetToken() public function testGetSetToken()
{ {
$sessionAccess = 0; $sessionAccess = 0;
$sessionLocator = new class(['request_stack' => function () use (&$sessionAccess) { $sessionLocator = new class(['request_stack' => function () use (&$sessionAccess) {
++$sessionAccess;
$session = $this->createMock(SessionInterface::class); $session = $this->createMock(SessionInterface::class);
$session->expects($this->once())
->method('getMetadataBag');
$request = new Request(); $request = new Request();
$request->setSession($session); $request->setSession($session);
$requestStack = new RequestStack(); $requestStack = $this->getMockBuilder(RequestStack::class)->setMethods(['getSession'])->getMock();
$requestStack->push($request); $requestStack->push($request);
$requestStack->expects($this->any())->method('getSession')->willReturnCallback(function () use ($session, &$sessionAccess) {
++$sessionAccess;
$session->expects($this->once())
->method('getMetadataBag');
return $session;
});
return $requestStack; return $requestStack;
}]) implements ContainerInterface { }]) implements ContainerInterface {
@ -62,4 +68,22 @@ class UsageTrackingTokenStorageTest extends TestCase
$this->assertSame($token, $trackingStorage->getToken()); $this->assertSame($token, $trackingStorage->getToken());
$this->assertSame(1, $sessionAccess); $this->assertSame(1, $sessionAccess);
} }
/**
* @group legacy
*/
public function testWithoutMainRequest()
{
$locator = new class(['request_stack' => function () {
return new RequestStack();
}]) implements ContainerInterface {
use ServiceLocatorTrait;
};
$tokenStorage = new TokenStorage();
$trackingStorage = new UsageTrackingTokenStorage($tokenStorage, $locator);
$trackingStorage->enableUsageTracking();
$this->expectDeprecation('Since symfony/security-core 5.3: Using "%s" (service ID: "security.token_storage") outside the request-response cycle is deprecated, use the "%s" class (service ID: "security.untracked_token_storage") instead or disable usage tracking using "disableUsageTracking()".');
$trackingStorage->getToken();
}
} }