From 54acc00769a8bf2a756fd944497b24fda834488c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Sat, 17 Oct 2020 23:28:45 +0200 Subject: [PATCH] Deprecat service "session" --- UPGRADE-5.3.md | 5 ++ UPGRADE-6.0.md | 4 + .../Bundle/FrameworkBundle/CHANGELOG.md | 1 + .../Controller/AbstractController.php | 9 ++- .../Compiler/SessionPass.php | 26 ++++++- .../Bundle/FrameworkBundle/KernelBrowser.php | 3 +- .../Resources/config/security_csrf.php | 2 +- .../Resources/config/session.php | 17 +++-- .../FrameworkBundle/Resources/config/test.php | 2 +- .../Session/DeprecatedSessionFactory.php | 46 ++++++++++++ .../Controller/AbstractControllerTest.php | 6 ++ .../Compiler/SessionPassTest.php | 55 +++++++++++++- .../FrameworkExtensionTest.php | 5 +- .../DeprecatedSessionController.php | 16 ++++ .../TestBundle/Resources/config/routing.yml | 4 + .../Tests/Functional/SessionTest.php | 20 +++++ .../Tests/Functional/app/Session/config.yml | 4 + .../Bundle/FrameworkBundle/composer.json | 8 +- .../RegisterTokenUsageTrackingPass.php | 2 +- .../Resources/config/security.php | 2 +- .../RegisterTokenUsageTrackingPassTest.php | 3 +- .../Tests/Functional/LogoutTest.php | 38 +++++++++- .../Bundle/SecurityBundle/composer.json | 2 +- .../RegisterServiceSubscribersPass.php | 9 ++- .../Component/HttpFoundation/CHANGELOG.md | 1 + .../Exception/SessionNotFoundException.php | 27 +++++++ .../Component/HttpFoundation/RequestStack.php | 17 +++++ ...RegisterControllerArgumentLocatorsPass.php | 3 +- src/Symfony/Component/Security/CHANGELOG.md | 3 + .../Storage/UsageTrackingTokenStorage.php | 25 +++++-- .../Storage/UsageTrackingTokenStorageTest.php | 11 ++- .../Component/Security/Core/composer.json | 3 +- .../TokenStorage/SessionTokenStorageTest.php | 8 +- .../Csrf/TokenStorage/SessionTokenStorage.php | 74 ++++++++++++++----- .../Component/Security/Csrf/composer.json | 4 +- .../Tests/Firewall/ContextListenerTest.php | 28 +++++-- .../CsrfTokenClearingLogoutHandlerTest.php | 15 +++- 37 files changed, 431 insertions(+), 77 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Session/DeprecatedSessionFactory.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/DeprecatedSessionController.php create mode 100644 src/Symfony/Component/HttpFoundation/Exception/SessionNotFoundException.php diff --git a/UPGRADE-5.3.md b/UPGRADE-5.3.md index 707e5d26dd..eae036930e 100644 --- a/UPGRADE-5.3.md +++ b/UPGRADE-5.3.md @@ -23,6 +23,11 @@ Form * Deprecated passing an array as the second argument of the `RadioListMapper::mapDataToForms()` method, pass `\Traversable` instead * Deprecated passing an array as the first argument of the `RadioListMapper::mapFormsToData()` method, pass `\Traversable` instead +FrameworkBundle +--------------- + + * Deprecate the `session` service and the `SessionInterface` alias, use the `\Symfony\Component\HttpFoundation\Request::getSession()` or the new `\Symfony\Component\HttpFoundation\RequestStack::getSession()` methods instead + HttpFoundation -------------- diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index 9b190dcb26..835222fdd3 100644 --- a/UPGRADE-6.0.md +++ b/UPGRADE-6.0.md @@ -69,6 +69,7 @@ Form FrameworkBundle --------------- + * Remove the `session` service and the `SessionInterface` alias, use the `\Symfony\Component\HttpFoundation\Request::getSession()` or the new `\Symfony\Component\HttpFoundation\RequestStack::getSession()` methods instead * `MicroKernelTrait::configureRoutes()` is now always called with a `RoutingConfigurator` * The "framework.router.utf8" configuration option defaults to `true` * Removed `session.attribute_bag` service and `session.flash_bag` service. @@ -165,6 +166,9 @@ Routing Security -------- + * Drop support for `SessionInterface $session` as constructor argument of `SessionTokenStorage`, inject a `\Symfony\Component\HttpFoundation\RequestStack $requestStack` instead + * Drop support for `session` provided by the ServiceLocator injected in `UsageTrackingTokenStorage`, provide a `request_stack` service instead + * Make `SessionTokenStorage` throw a `SessionNotFoundException` when called outside a request context * Removed `ROLE_PREVIOUS_ADMIN` role in favor of `IS_IMPERSONATOR` attribute * Removed `LogoutSuccessHandlerInterface` and `LogoutHandlerInterface`, register a listener on the `LogoutEvent` event instead. * Removed `DefaultLogoutSuccessHandler` in favor of `DefaultLogoutListener`. diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 6cb3fde123..7dd9ccbdb3 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 5.3 --- + * Deprecate the `session` service and the `SessionInterface` alias, use the `Request::getSession()` or the new `RequestStack::getSession()` methods instead * Added `AbstractController::renderForm()` to render a form and set the appropriate HTTP status code * Added support for configuring PHP error level to log levels * Added the `dispatcher` option to `debug:event-dispatcher` diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php index 3074af7a17..697725344c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php @@ -22,6 +22,7 @@ use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\Form\FormFactoryInterface; use Symfony\Component\Form\FormInterface; use Symfony\Component\HttpFoundation\BinaryFileResponse; +use Symfony\Component\HttpFoundation\Exception\SessionNotFoundException; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; @@ -200,11 +201,11 @@ abstract class AbstractController implements ServiceSubscriberInterface */ protected function addFlash(string $type, $message): void { - if (!$this->container->has('session')) { - throw new \LogicException('You can not use the addFlash method if sessions are disabled. Enable them in "config/packages/framework.yaml".'); + try { + $this->container->get('request_stack')->getSession()->getFlashBag()->add($type, $message); + } catch (SessionNotFoundException $e) { + throw new \LogicException('You can not use the addFlash method if sessions are disabled. Enable them in "config/packages/framework.yaml".', 0, $e); } - - $this->container->get('session')->getFlashBag()->add($type, $message); } /** diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/SessionPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/SessionPass.php index 0f4950615f..df82076702 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/SessionPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/SessionPass.php @@ -22,21 +22,41 @@ class SessionPass implements CompilerPassInterface { public function process(ContainerBuilder $container) { - if (!$container->hasDefinition('session')) { + if (!$container->has('session.storage')) { return; } + // BC layer: Make "session" an alias of ".session.do-not-use" when not overriden by the user + if (!$container->has('session')) { + $alias = $container->setAlias('session', '.session.do-not-use'); + $alias->setDeprecated('symfony/framework-bundle', '5.3', 'The "%alias_id%" service is deprecated, use "$requestStack->getSession()" instead.'); + + return; + } + + if ($container->hasDefinition('session')) { + $definition = $container->getDefinition('session'); + $definition->setDeprecated('symfony/framework-bundle', '5.3', 'The "%service_id%" service is deprecated, use "$requestStack->getSession()" instead.'); + } else { + $alias = $container->getAlias('session'); + $alias->setDeprecated('symfony/framework-bundle', '5.3', 'The "%alias_id%" alias is deprecated, use "$requestStack->getSession()" instead.'); + $definition = $container->findDefinition('session'); + } + + // Convert internal service `.session.do-not-use` into alias of `session`. + $container->setAlias('.session.do-not-use', 'session'); + $bags = [ 'session.flash_bag' => $container->hasDefinition('session.flash_bag') ? $container->getDefinition('session.flash_bag') : null, 'session.attribute_bag' => $container->hasDefinition('session.attribute_bag') ? $container->getDefinition('session.attribute_bag') : null, ]; - foreach ($container->getDefinition('session')->getArguments() as $v) { + foreach ($definition->getArguments() as $v) { if (!$v instanceof Reference || !isset($bags[$bag = (string) $v]) || !\is_array($factory = $bags[$bag]->getFactory())) { continue; } - if ([0, 1] !== array_keys($factory) || !$factory[0] instanceof Reference || 'session' !== (string) $factory[0]) { + if ([0, 1] !== array_keys($factory) || !$factory[0] instanceof Reference || !\in_array((string) $factory[0], ['session', '.session.do-not-use'], true)) { continue; } diff --git a/src/Symfony/Bundle/FrameworkBundle/KernelBrowser.php b/src/Symfony/Bundle/FrameworkBundle/KernelBrowser.php index 40381e34aa..19a5005157 100644 --- a/src/Symfony/Bundle/FrameworkBundle/KernelBrowser.php +++ b/src/Symfony/Bundle/FrameworkBundle/KernelBrowser.php @@ -18,6 +18,7 @@ use Symfony\Component\BrowserKit\History; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpKernel\HttpKernelBrowser; use Symfony\Component\HttpKernel\KernelInterface; use Symfony\Component\HttpKernel\Profiler\Profile as HttpProfile; @@ -122,7 +123,7 @@ class KernelBrowser extends HttpKernelBrowser $token = new TestBrowserToken($user->getRoles(), $user); $token->setAuthenticated(true); - $session = $this->getContainer()->get('session'); + $session = new Session($this->getContainer()->get('test.service_container')->get('session.storage')); $session->set('_security_'.$firewallContext, serialize($token)); $session->save(); diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.php index 0afc740cd8..9644d5b449 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.php @@ -27,7 +27,7 @@ return static function (ContainerConfigurator $container) { ->alias(TokenGeneratorInterface::class, 'security.csrf.token_generator') ->set('security.csrf.token_storage', SessionTokenStorage::class) - ->args([service('session')]) + ->args([service('request_stack')]) ->alias(TokenStorageInterface::class, 'security.csrf.token_storage') diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.php index 812ee50e7c..fcaca3ffc4 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.php @@ -11,6 +11,7 @@ namespace Symfony\Component\DependencyInjection\Loader\Configurator; +use Symfony\Bundle\FrameworkBundle\Session\DeprecatedSessionFactory; use Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag; use Symfony\Component\HttpFoundation\Session\Flash\FlashBag; use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; @@ -33,15 +34,17 @@ return static function (ContainerConfigurator $container) { $container->parameters()->set('session.metadata.storage_key', '_sf2_meta'); $container->services() - ->set('session', Session::class) - ->public() + ->set('.session.do-not-use', Session::class) // to be removed in 6.0 ->args([ service('session.storage'), null, // AttributeBagInterface null, // FlashBagInterface [service('session_listener'), 'onSessionUsage'], ]) - ->alias(SessionInterface::class, 'session') + ->set('.session.deprecated', SessionInterface::class) // to be removed in 6.0 + ->factory([inline_service(DeprecatedSessionFactory::class)->args([service('request_stack')]), 'getSession']) + ->alias(SessionInterface::class, '.session.do-not-use') + ->deprecate('symfony/framework-bundle', '5.3', 'The "%alias_id%" alias is deprecated, use "$requestStack->getSession()" instead.') ->alias(SessionStorageInterface::class, 'session.storage') ->alias(\SessionHandlerInterface::class, 'session.handler') @@ -65,12 +68,12 @@ return static function (ContainerConfigurator $container) { ]) ->set('session.flash_bag', FlashBag::class) - ->factory([service('session'), 'getFlashBag']) + ->factory([service('.session.do-not-use'), 'getFlashBag']) ->deprecate('symfony/framework-bundle', '5.1', 'The "%service_id%" service is deprecated, use "$session->getFlashBag()" instead.') ->alias(FlashBagInterface::class, 'session.flash_bag') ->set('session.attribute_bag', AttributeBag::class) - ->factory([service('session'), 'getBag']) + ->factory([service('.session.do-not-use'), 'getBag']) ->args(['attributes']) ->deprecate('symfony/framework-bundle', '5.1', 'The "%service_id%" service is deprecated, use "$session->getAttributeBag()" instead.') @@ -94,8 +97,8 @@ return static function (ContainerConfigurator $container) { ->set('session_listener', SessionListener::class) ->args([ service_locator([ - 'session' => service('session')->ignoreOnInvalid(), - 'initialized_session' => service('session')->ignoreOnUninitialized(), + 'session' => service('.session.do-not-use')->ignoreOnInvalid(), + 'initialized_session' => service('.session.do-not-use')->ignoreOnUninitialized(), 'logger' => service('logger')->ignoreOnInvalid(), 'session_collector' => service('data_collector.request.session_collector')->ignoreOnInvalid(), ]), diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/test.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/test.php index af0df318a0..61e4052521 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/test.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/test.php @@ -38,7 +38,7 @@ return static function (ContainerConfigurator $container) { ->set('test.session.listener', TestSessionListener::class) ->args([ service_locator([ - 'session' => service('session')->ignoreOnInvalid(), + 'session' => service('.session.do-not-use')->ignoreOnInvalid(), ]), ]) ->tag('kernel.event_subscriber') diff --git a/src/Symfony/Bundle/FrameworkBundle/Session/DeprecatedSessionFactory.php b/src/Symfony/Bundle/FrameworkBundle/Session/DeprecatedSessionFactory.php new file mode 100644 index 0000000000..c2c1cd34ae --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Session/DeprecatedSessionFactory.php @@ -0,0 +1,46 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\FrameworkBundle\Session; + +use Symfony\Component\HttpFoundation\Exception\SessionNotFoundException; +use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Session\SessionInterface; + +/** + * Provides session and trigger deprecation. + * + * Used by service that should trigger deprecation when accessed by the user. + * + * @author Jérémy Derussé + * + * @internal to be removed in 6.0 + */ +class DeprecatedSessionFactory +{ + private $requestStack; + + public function __construct(RequestStack $requestStack) + { + $this->requestStack = $requestStack; + } + + public function getSession(): ?SessionInterface + { + trigger_deprecation('symfony/framework-bundle', '5.3', 'The "session" service is deprecated, use "$requestStack->getSession()" instead.'); + + try { + return $this->requestStack->getSession(); + } catch (SessionNotFoundException $e) { + return null; + } + } +} diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php index 41dc62aaea..4c84c2c375 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php @@ -497,8 +497,14 @@ class AbstractControllerTest extends TestCase $session = $this->createMock(Session::class); $session->expects($this->once())->method('getFlashBag')->willReturn($flashBag); + $request = new Request(); + $request->setSession($session); + $requestStack = new RequestStack(); + $requestStack->push($request); + $container = new Container(); $container->set('session', $session); + $container->set('request_stack', $requestStack); $controller = $this->createController(); $controller->setContainer($container); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/SessionPassTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/SessionPassTest.php index afc6f9b4b2..4b3601969d 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/SessionPassTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/SessionPassTest.php @@ -19,26 +19,77 @@ use Symfony\Component\DependencyInjection\Reference; class SessionPassTest extends TestCase { public function testProcess() + { + $container = new ContainerBuilder(); + $container + ->register('session.storage'); // marker service + $container + ->register('.session.do-not-use'); + + (new SessionPass())->process($container); + + $this->assertTrue($container->hasAlias('session')); + $this->assertSame($container->findDefinition('session'), $container->getDefinition('.session.do-not-use')); + $this->assertTrue($container->getAlias('session')->isDeprecated()); + } + + public function testProcessUserDefinedSession() { $arguments = [ new Reference('session.flash_bag'), new Reference('session.attribute_bag'), ]; $container = new ContainerBuilder(); + $container + ->register('session.storage'); // marker service $container ->register('session') ->setArguments($arguments); $container ->register('session.flash_bag') - ->setFactory([new Reference('session'), 'getFlashBag']); + ->setFactory([new Reference('.session.do-not-use'), 'getFlashBag']); $container ->register('session.attribute_bag') - ->setFactory([new Reference('session'), 'getAttributeBag']); + ->setFactory([new Reference('.session.do-not-use'), 'getAttributeBag']); (new SessionPass())->process($container); $this->assertSame($arguments, $container->getDefinition('session')->getArguments()); $this->assertNull($container->getDefinition('session.flash_bag')->getFactory()); $this->assertNull($container->getDefinition('session.attribute_bag')->getFactory()); + $this->assertTrue($container->hasAlias('.session.do-not-use')); + $this->assertSame($container->getDefinition('session'), $container->findDefinition('.session.do-not-use')); + $this->assertTrue($container->getDefinition('session')->isDeprecated()); + } + + public function testProcessUserDefinedAlias() + { + $arguments = [ + new Reference('session.flash_bag'), + new Reference('session.attribute_bag'), + ]; + $container = new ContainerBuilder(); + $container + ->register('session.storage'); // marker service + $container + ->register('trueSession') + ->setArguments($arguments); + $container + ->setAlias('session', 'trueSession'); + $container + ->register('session.flash_bag') + ->setFactory([new Reference('.session.do-not-use'), 'getFlashBag']); + $container + ->register('session.attribute_bag') + ->setFactory([new Reference('.session.do-not-use'), 'getAttributeBag']); + + (new SessionPass())->process($container); + + $this->assertSame($arguments, $container->findDefinition('session')->getArguments()); + $this->assertNull($container->getDefinition('session.flash_bag')->getFactory()); + $this->assertNull($container->getDefinition('session.attribute_bag')->getFactory()); + $this->assertTrue($container->hasAlias('.session.do-not-use')); + $this->assertSame($container->findDefinition('session'), $container->findDefinition('.session.do-not-use')); + $this->assertTrue($container->getAlias('session')->isDeprecated()); } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 3a3678cc4e..f502aad1e6 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -46,6 +46,7 @@ use Symfony\Component\Form\Form; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\RetryableHttpClient; use Symfony\Component\HttpClient\ScopingHttpClient; +use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\HttpKernel\DependencyInjection\LoggerPass; use Symfony\Component\Messenger\Transport\TransportFactory; use Symfony\Component\PropertyAccess\PropertyAccessor; @@ -541,7 +542,7 @@ abstract class FrameworkExtensionTest extends TestCase { $container = $this->createContainerFromFile('full'); - $this->assertTrue($container->hasDefinition('session'), '->registerSessionConfiguration() loads session.xml'); + $this->assertTrue($container->hasAlias(SessionInterface::class), '->registerSessionConfiguration() loads session.xml'); $this->assertEquals('fr', $container->getParameter('kernel.default_locale')); $this->assertEquals('session.storage.native', (string) $container->getAlias('session.storage')); $this->assertEquals('session.handler.native_file', (string) $container->getAlias('session.handler')); @@ -567,7 +568,7 @@ abstract class FrameworkExtensionTest extends TestCase { $container = $this->createContainerFromFile('session'); - $this->assertTrue($container->hasDefinition('session'), '->registerSessionConfiguration() loads session.xml'); + $this->assertTrue($container->hasAlias(SessionInterface::class), '->registerSessionConfiguration() loads session.xml'); $this->assertNull($container->getDefinition('session.storage.native')->getArgument(1)); $this->assertNull($container->getDefinition('session.storage.php_bridge')->getArgument(0)); $this->assertSame('session.handler.native_file', (string) $container->getAlias('session.handler')); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/DeprecatedSessionController.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/DeprecatedSessionController.php new file mode 100644 index 0000000000..75e9673c35 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/DeprecatedSessionController.php @@ -0,0 +1,16 @@ +get('session'); + + return new Response('done'); + } +} diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml index 155871fc27..94eb4d7ac1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml @@ -22,6 +22,10 @@ injected_flashbag_session_setflash: path: injected_flashbag/session_setflash/{message} defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\InjectedFlashbagSessionController::setFlashAction} +deprecated_session_setflash: + path: /deprecated_session/trigger + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\DeprecatedSessionController::triggerAction} + session_showflash: path: /session_showflash defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SessionController::showFlashAction } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SessionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SessionTest.php index 253947d02f..5140604793 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SessionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SessionTest.php @@ -99,6 +99,26 @@ class SessionTest extends AbstractWebTestCase $this->assertStringContainsString('No flash was set.', $crawler->text()); } + /** + * @group legacy + * @dataProvider getConfigs + */ + public function testSessionServiceTriggerDeprecation($config, $insulate) + { + $this->expectDeprecation('Since symfony/framework-bundle 5.3: The "session" service is deprecated, use "$requestStack->getSession()" instead.'); + + $client = $this->createClient(['test_case' => 'Session', 'root_config' => $config]); + if ($insulate) { + $client->insulate(); + } + + // trigger deprecation + $crawler = $client->request('GET', '/deprecated_session/trigger'); + + // check response + $this->assertStringContainsString('done', $crawler->text()); + } + /** * See if two separate insulated clients can run without * polluting each other's session data. diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Session/config.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Session/config.yml index 4807c42d1e..03ee4fb151 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Session/config.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Session/config.yml @@ -9,3 +9,7 @@ services: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\InjectedFlashbagSessionController: autowire: true tags: ['controller.service_arguments'] + + Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\DeprecatedSessionController: + autowire: true + autoconfigure: true diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json index 147f0dd216..3be349d5f2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -20,11 +20,11 @@ "ext-xml": "*", "symfony/cache": "^5.2", "symfony/config": "^5.0", - "symfony/dependency-injection": "^5.2", + "symfony/dependency-injection": "^5.3", "symfony/deprecation-contracts": "^2.1", "symfony/event-dispatcher": "^5.1", "symfony/error-handler": "^4.4.1|^5.0.1", - "symfony/http-foundation": "^5.2.1", + "symfony/http-foundation": "^5.3", "symfony/http-kernel": "^5.2.1", "symfony/polyfill-mbstring": "~1.0", "symfony/polyfill-php80": "^1.15", @@ -52,8 +52,6 @@ "symfony/mime": "^4.4|^5.0", "symfony/process": "^4.4|^5.0", "symfony/security-bundle": "^5.1", - "symfony/security-csrf": "^4.4|^5.0", - "symfony/security-http": "^4.4|^5.0", "symfony/serializer": "^5.2", "symfony/stopwatch": "^4.4|^5.0", "symfony/string": "^5.0", @@ -87,6 +85,8 @@ "symfony/property-info": "<4.4", "symfony/property-access": "<5.2", "symfony/serializer": "<5.2", + "symfony/security-csrf": "<5.3", + "symfony/security-core": "<5.3", "symfony/stopwatch": "<4.4", "symfony/translation": "<5.0", "symfony/twig-bridge": "<4.4", diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/RegisterTokenUsageTrackingPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/RegisterTokenUsageTrackingPass.php index 1cd90fe70a..cedfe3a8d7 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/RegisterTokenUsageTrackingPass.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/RegisterTokenUsageTrackingPass.php @@ -41,7 +41,7 @@ class RegisterTokenUsageTrackingPass implements CompilerPassInterface TokenStorageInterface::class => new BoundArgument(new Reference('security.untracked_token_storage'), false), ]); - if (!$container->has('session')) { + if (!$container->has('session.storage')) { $container->setAlias('security.token_storage', 'security.untracked_token_storage')->setPublic(true); $container->getDefinition('security.untracked_token_storage')->addTag('kernel.reset', ['method' => 'reset']); } elseif ($container->hasDefinition('security.context_listener')) { diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php index 380ef56b20..ecc9fc14fc 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php @@ -73,7 +73,7 @@ return static function (ContainerConfigurator $container) { ->args([ service('security.untracked_token_storage'), service_locator([ - 'session' => service('session'), + 'request_stack' => service('request_stack'), ]), ]) ->tag('kernel.reset', ['method' => 'disableUsageTracking']) diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/RegisterTokenUsageTrackingPassTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/RegisterTokenUsageTrackingPassTest.php index afdbf9afaf..f997a62cd5 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/RegisterTokenUsageTrackingPassTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/RegisterTokenUsageTrackingPassTest.php @@ -18,6 +18,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage; use Symfony\Component\Security\Http\Firewall\ContextListener; @@ -65,7 +66,7 @@ class RegisterTokenUsageTrackingPassTest extends TestCase $container = new ContainerBuilder(); $container->setParameter('security.token_storage.class', UsageTrackingTokenStorage::class); - $container->register('session', Session::class); + $container->register('session.storage', NativeSessionStorage::class); $container->register('security.context_listener', ContextListener::class) ->setArguments([ new Reference('security.untracked_token_storage'), diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php index b5e2b48487..1c032522ed 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php @@ -11,7 +11,12 @@ namespace Symfony\Bundle\SecurityBundle\Tests\Functional; +use Symfony\Bundle\FrameworkBundle\KernelBrowser; use Symfony\Component\BrowserKit\Cookie; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Event\RequestEvent; +use Symfony\Component\HttpKernel\KernelEvents; class LogoutTest extends AbstractWebTestCase { @@ -44,19 +49,26 @@ class LogoutTest extends AbstractWebTestCase public function testCsrfTokensAreClearedOnLogout(array $options) { $client = $this->createClient($options + ['test_case' => 'LogoutWithoutSessionInvalidation', 'root_config' => 'config.yml']); - static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar'); + $client->disableReboot(); + $this->callInRequestContext($client, function () { + static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar'); + }); $client->request('POST', '/login', [ '_username' => 'johannes', '_password' => 'test', ]); - $this->assertTrue(static::$container->get('security.csrf.token_storage')->hasToken('foo')); - $this->assertSame('bar', static::$container->get('security.csrf.token_storage')->getToken('foo')); + $this->callInRequestContext($client, function () { + $this->assertTrue(static::$container->get('security.csrf.token_storage')->hasToken('foo')); + $this->assertSame('bar', static::$container->get('security.csrf.token_storage')->getToken('foo')); + }); $client->request('GET', '/logout'); - $this->assertFalse(static::$container->get('security.csrf.token_storage')->hasToken('foo')); + $this->callInRequestContext($client, function () { + $this->assertFalse(static::$container->get('security.csrf.token_storage')->hasToken('foo')); + }); } /** @@ -85,4 +97,22 @@ class LogoutTest extends AbstractWebTestCase $this->assertRedirect($client->getResponse(), '/'); $this->assertNull($cookieJar->get('flavor')); } + + private function callInRequestContext(KernelBrowser $client, callable $callable): void + { + /** @var EventDispatcherInterface $eventDispatcher */ + $eventDispatcher = static::$container->get(EventDispatcherInterface::class); + $wrappedCallable = function (RequestEvent $event) use (&$callable) { + $callable(); + $event->setResponse(new Response('')); + $event->stopPropagation(); + }; + + $eventDispatcher->addListener(KernelEvents::REQUEST, $wrappedCallable); + try { + $client->request('GET', '/'.uniqid('', true)); + } finally { + $eventDispatcher->removeListener(KernelEvents::REQUEST, $wrappedCallable); + } + } } diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json index 1d1e8a490b..3d9adf3939 100644 --- a/src/Symfony/Bundle/SecurityBundle/composer.json +++ b/src/Symfony/Bundle/SecurityBundle/composer.json @@ -24,7 +24,7 @@ "symfony/event-dispatcher": "^5.1", "symfony/http-kernel": "^5.0", "symfony/polyfill-php80": "^1.15", - "symfony/security-core": "^5.2", + "symfony/security-core": "^5.3", "symfony/security-csrf": "^4.4|^5.0", "symfony/security-guard": "^5.2", "symfony/security-http": "^5.2" diff --git a/src/Symfony/Component/DependencyInjection/Compiler/RegisterServiceSubscribersPass.php b/src/Symfony/Component/DependencyInjection/Compiler/RegisterServiceSubscribersPass.php index 9e08d79408..1ff39ac0c6 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/RegisterServiceSubscribersPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/RegisterServiceSubscribersPass.php @@ -12,12 +12,14 @@ namespace Symfony\Component\DependencyInjection\Compiler; use Psr\Container\ContainerInterface as PsrContainerInterface; +use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\DependencyInjection\Argument\BoundArgument; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\TypedReference; +use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Contracts\Service\ServiceProviderInterface; use Symfony\Contracts\Service\ServiceSubscriberInterface; @@ -66,7 +68,7 @@ class RegisterServiceSubscribersPass extends AbstractRecursivePass throw new InvalidArgumentException(sprintf('Service "%s" must implement interface "%s".', $this->currentId, ServiceSubscriberInterface::class)); } $class = $r->name; - + $replaceDeprecatedSession = $this->container->has('.session.deprecated') && $r->isSubclassOf(AbstractController::class); $subscriberMap = []; foreach ($class::getSubscribedServices() as $key => $type) { @@ -85,6 +87,11 @@ class RegisterServiceSubscribersPass extends AbstractRecursivePass if (!$autowire) { throw new InvalidArgumentException(sprintf('Service "%s" misses a "container.service_subscriber" tag with "key"/"id" attributes corresponding to entry "%s" as returned by "%s::getSubscribedServices()".', $this->currentId, $key, $class)); } + if ($replaceDeprecatedSession && SessionInterface::class === $type) { + // This prevents triggering the deprecation when building the container + // Should be removed in Symfony 6.0 + $type = '.session.deprecated'; + } $serviceMap[$key] = new Reference($type); } diff --git a/src/Symfony/Component/HttpFoundation/CHANGELOG.md b/src/Symfony/Component/HttpFoundation/CHANGELOG.md index 72ffa40363..af065b0d63 100644 --- a/src/Symfony/Component/HttpFoundation/CHANGELOG.md +++ b/src/Symfony/Component/HttpFoundation/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 5.3 --- + * Add the `RequestStack::getSession` method * Deprecate the `NamespacedAttributeBag` class * added `ResponseFormatSame` PHPUnit constraint diff --git a/src/Symfony/Component/HttpFoundation/Exception/SessionNotFoundException.php b/src/Symfony/Component/HttpFoundation/Exception/SessionNotFoundException.php new file mode 100644 index 0000000000..4ccfb18d3f --- /dev/null +++ b/src/Symfony/Component/HttpFoundation/Exception/SessionNotFoundException.php @@ -0,0 +1,27 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpFoundation\Exception; + +/** + * Raised when a session does not exists. This happens in the following cases: + * - the session is not enabled + * - attempt to read a session outside a request context (ie. cli script). + * + * @author Jérémy Derussé + */ +class SessionNotFoundException extends \LogicException implements RequestExceptionInterface +{ + public function __construct($message = 'There is currently no session available.', $code = 0, Throwable $previous = null) + { + parent::__construct($message, $code, $previous); + } +} diff --git a/src/Symfony/Component/HttpFoundation/RequestStack.php b/src/Symfony/Component/HttpFoundation/RequestStack.php index 244a77d631..fe07d594b1 100644 --- a/src/Symfony/Component/HttpFoundation/RequestStack.php +++ b/src/Symfony/Component/HttpFoundation/RequestStack.php @@ -11,6 +11,9 @@ namespace Symfony\Component\HttpFoundation; +use Symfony\Component\HttpFoundation\Exception\SessionNotFoundException; +use Symfony\Component\HttpFoundation\Session\SessionInterface; + /** * Request stack that controls the lifecycle of requests. * @@ -100,4 +103,18 @@ class RequestStack return $this->requests[$pos]; } + + /** + * Gets the current session. + * + * @throws SessionNotFoundException + */ + public function getSession(): SessionInterface + { + if ((null !== $request = end($this->requests) ?: null) && $request->hasSession()) { + return $request->getSession(); + } + + throw new SessionNotFoundException(); + } } diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php index a2a9a0c71b..e24e0e6d64 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php @@ -23,6 +23,7 @@ use Symfony\Component\DependencyInjection\LazyProxy\ProxyHelper; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\TypedReference; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Session\SessionInterface; /** * Creates the service-locators required by ServiceValueResolver. @@ -165,7 +166,7 @@ class RegisterControllerArgumentLocatorsPass implements CompilerPassInterface $invalidBehavior = ContainerInterface::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE; } - if (Request::class === $type) { + if (Request::class === $type || SessionInterface::class === $type) { continue; } diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md index fa3eb5f333..43efcebec4 100644 --- a/src/Symfony/Component/Security/CHANGELOG.md +++ b/src/Symfony/Component/Security/CHANGELOG.md @@ -4,6 +4,9 @@ CHANGELOG 5.3 --- + * Deprecate the `SessionInterface $session` constructor argument of `SessionTokenStorage`, inject a `\Symfony\Component\HttpFoundation\RequestStack $requestStack` 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 * Randomize CSRF tokens to harden BREACH attacks * Deprecated voters that do not return a valid decision when calling the `vote` method. diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/Storage/UsageTrackingTokenStorage.php b/src/Symfony/Component/Security/Core/Authentication/Token/Storage/UsageTrackingTokenStorage.php index b90d5ab28b..0b8d9c3201 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/Storage/UsageTrackingTokenStorage.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/Storage/UsageTrackingTokenStorage.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Core\Authentication\Token\Storage; use Psr\Container\ContainerInterface; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Contracts\Service\ServiceSubscriberInterface; @@ -24,13 +25,13 @@ use Symfony\Contracts\Service\ServiceSubscriberInterface; final class UsageTrackingTokenStorage implements TokenStorageInterface, ServiceSubscriberInterface { private $storage; - private $sessionLocator; + private $container; private $enableUsageTracking = false; - public function __construct(TokenStorageInterface $storage, ContainerInterface $sessionLocator) + public function __construct(TokenStorageInterface $storage, ContainerInterface $container) { $this->storage = $storage; - $this->sessionLocator = $sessionLocator; + $this->container = $container; } /** @@ -40,7 +41,7 @@ final class UsageTrackingTokenStorage implements TokenStorageInterface, ServiceS { if ($this->enableUsageTracking) { // increments the internal session usage index - $this->sessionLocator->get('session')->getMetadataBag(); + $this->getSession()->getMetadataBag(); } return $this->storage->getToken(); @@ -55,7 +56,7 @@ final class UsageTrackingTokenStorage implements TokenStorageInterface, ServiceS if ($token && $this->enableUsageTracking) { // increments the internal session usage index - $this->sessionLocator->get('session')->getMetadataBag(); + $this->getSession()->getMetadataBag(); } } @@ -72,7 +73,19 @@ final class UsageTrackingTokenStorage implements TokenStorageInterface, ServiceS public static function getSubscribedServices(): array { return [ - 'session' => SessionInterface::class, + 'request_stack' => RequestStack::class, ]; } + + private function getSession(): SessionInterface + { + // BC for symfony/security-bundle < 5.3 + if ($this->container->has('session')) { + trigger_deprecation('symfony/security-core', '5.3', 'Injecting the "session" in "%s" is deprecated, inject the "request_stack" instead.', __CLASS__); + + return $this->container->get('session'); + } + + return $this->container->get('request_stack')->getSession(); + } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/Storage/UsageTrackingTokenStorageTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/Storage/UsageTrackingTokenStorageTest.php index 607ccc7504..c5d2eaf543 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/Storage/UsageTrackingTokenStorageTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/Storage/UsageTrackingTokenStorageTest.php @@ -13,6 +13,8 @@ namespace Symfony\Component\Security\Core\Tests\Authentication\Token\Storage; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage; @@ -24,14 +26,19 @@ class UsageTrackingTokenStorageTest extends TestCase public function testGetSetToken() { $sessionAccess = 0; - $sessionLocator = new class(['session' => function () use (&$sessionAccess) { + $sessionLocator = new class(['request_stack' => function () use (&$sessionAccess) { ++$sessionAccess; $session = $this->createMock(SessionInterface::class); $session->expects($this->once()) ->method('getMetadataBag'); - return $session; + $request = new Request(); + $request->setSession($session); + $requestStack = new RequestStack(); + $requestStack->push($request); + + return $requestStack; }]) implements ContainerInterface { use ServiceLocatorTrait; }; diff --git a/src/Symfony/Component/Security/Core/composer.json b/src/Symfony/Component/Security/Core/composer.json index 3d74c1c73d..48a6a46ec4 100644 --- a/src/Symfony/Component/Security/Core/composer.json +++ b/src/Symfony/Component/Security/Core/composer.json @@ -26,7 +26,7 @@ "psr/container": "^1.0", "symfony/event-dispatcher": "^4.4|^5.0", "symfony/expression-language": "^4.4|^5.0", - "symfony/http-foundation": "^4.4|^5.0", + "symfony/http-foundation": "^5.3", "symfony/ldap": "^4.4|^5.0", "symfony/translation": "^4.4|^5.0", "symfony/validator": "^5.2", @@ -34,6 +34,7 @@ }, "conflict": { "symfony/event-dispatcher": "<4.4", + "symfony/http-foundation": "<5.3", "symfony/security-guard": "<4.4", "symfony/ldap": "<4.4", "symfony/validator": "<5.2" diff --git a/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php b/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php index 5046cc0dec..230f33fb25 100644 --- a/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php +++ b/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php @@ -12,6 +12,8 @@ namespace Symfony\Component\Security\Csrf\Tests\TokenStorage; use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; use Symfony\Component\Security\Csrf\Exception\TokenNotFoundException; @@ -37,7 +39,11 @@ class SessionTokenStorageTest extends TestCase protected function setUp(): void { $this->session = new Session(new MockArraySessionStorage()); - $this->storage = new SessionTokenStorage($this->session, self::SESSION_NAMESPACE); + $request = new Request(); + $request->setSession($this->session); + $requestStack = new RequestStack(); + $requestStack->push($request); + $this->storage = new SessionTokenStorage($requestStack, self::SESSION_NAMESPACE); } public function testStoreTokenInNotStartedSessionStartsTheSession() diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorage.php index 04774d0c96..70613f5f26 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorage.php @@ -11,7 +11,12 @@ namespace Symfony\Component\Security\Csrf\TokenStorage; +use Symfony\Component\HttpFoundation\Exception\SessionNotFoundException; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpFoundation\Session\SessionInterface; +use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; use Symfony\Component\Security\Csrf\Exception\TokenNotFoundException; /** @@ -26,17 +31,30 @@ class SessionTokenStorage implements ClearableTokenStorageInterface */ public const SESSION_NAMESPACE = '_csrf'; - private $session; + private $requestStack; private $namespace; + /** + * Tp be remove in Symfony 6.0 + */ + private $session; /** - * Initializes the storage with a Session object and a session namespace. + * Initializes the storage with a RequestStack object and a session namespace. * - * @param string $namespace The namespace under which the token is stored in the session + * @param RequestStack $requestStack + * @param string $namespace The namespace under which the token is stored in the requestStack */ - public function __construct(SessionInterface $session, string $namespace = self::SESSION_NAMESPACE) + public function __construct(/* RequestStack*/ $requestStack, string $namespace = self::SESSION_NAMESPACE) { - $this->session = $session; + if ($requestStack instanceof SessionInterface) { + trigger_deprecation('symfony/security-csrf', '5.3', 'Passing a "%s" to "%s" is deprecated, use a "%s" instead.', SessionInterface::class, __CLASS__, RequestStack::class); + $request = new Request(); + $request->setSession($requestStack); + + $requestStack = new RequestStack(); + $requestStack->push($request); + } + $this->requestStack = $requestStack; $this->namespace = $namespace; } @@ -45,15 +63,16 @@ class SessionTokenStorage implements ClearableTokenStorageInterface */ public function getToken(string $tokenId) { - if (!$this->session->isStarted()) { - $this->session->start(); + $session = $this->getSession(); + if (!$session->isStarted()) { + $session->start(); } - if (!$this->session->has($this->namespace.'/'.$tokenId)) { + if (!$session->has($this->namespace.'/'.$tokenId)) { throw new TokenNotFoundException('The CSRF token with ID '.$tokenId.' does not exist.'); } - return (string) $this->session->get($this->namespace.'/'.$tokenId); + return (string) $session->get($this->namespace.'/'.$tokenId); } /** @@ -61,11 +80,12 @@ class SessionTokenStorage implements ClearableTokenStorageInterface */ public function setToken(string $tokenId, string $token) { - if (!$this->session->isStarted()) { - $this->session->start(); + $session = $this->getSession(); + if (!$session->isStarted()) { + $session->start(); } - $this->session->set($this->namespace.'/'.$tokenId, $token); + $session->set($this->namespace.'/'.$tokenId, $token); } /** @@ -73,11 +93,12 @@ class SessionTokenStorage implements ClearableTokenStorageInterface */ public function hasToken(string $tokenId) { - if (!$this->session->isStarted()) { - $this->session->start(); + $session = $this->getSession(); + if (!$session->isStarted()) { + $session->start(); } - return $this->session->has($this->namespace.'/'.$tokenId); + return $session->has($this->namespace.'/'.$tokenId); } /** @@ -85,11 +106,12 @@ class SessionTokenStorage implements ClearableTokenStorageInterface */ public function removeToken(string $tokenId) { - if (!$this->session->isStarted()) { - $this->session->start(); + $session = $this->getSession(); + if (!$session->isStarted()) { + $session->start(); } - return $this->session->remove($this->namespace.'/'.$tokenId); + return $session->remove($this->namespace.'/'.$tokenId); } /** @@ -97,10 +119,22 @@ class SessionTokenStorage implements ClearableTokenStorageInterface */ public function clear() { - foreach (array_keys($this->session->all()) as $key) { + $session = $this->getSession(); + foreach (array_keys($session->all()) as $key) { if (0 === strpos($key, $this->namespace.'/')) { - $this->session->remove($key); + $session->remove($key); } } } + + private function getSession(): SessionInterface + { + try { + return $this->requestStack->getSession(); + } catch (SessionNotFoundException $e) { + trigger_deprecation('symfony/security-csrf', '5.3', 'Using the "%s" without a session has no effect and is deprecated. It will throw a "%s" in Symfony 6.0', __CLASS__, SessionNotFoundException::class); + + return $this->session ?? $this->session = new Session(new MockArraySessionStorage()); + } + } } diff --git a/src/Symfony/Component/Security/Csrf/composer.json b/src/Symfony/Component/Security/Csrf/composer.json index 3bdea6f7a4..5693034cff 100644 --- a/src/Symfony/Component/Security/Csrf/composer.json +++ b/src/Symfony/Component/Security/Csrf/composer.json @@ -20,10 +20,10 @@ "symfony/security-core": "^4.4|^5.0" }, "require-dev": { - "symfony/http-foundation": "^4.4|^5.0" + "symfony/http-foundation": "^5.3" }, "conflict": { - "symfony/http-foundation": "<4.4" + "symfony/http-foundation": "<5.3" }, "suggest": { "symfony/http-foundation": "For using the class SessionTokenStorage." diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php index c2980c293a..07ce14759f 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php @@ -16,6 +16,7 @@ use Psr\Container\ContainerInterface; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpFoundation\Session\SessionInterface; @@ -375,13 +376,17 @@ class ContextListenerTest extends TestCase protected function runSessionOnKernelResponse($newToken, $original = null) { $session = new Session(new MockArraySessionStorage()); + $request = new Request(); + $request->setSession($session); + $requestStack = new RequestStack(); + $requestStack->push($request); if (null !== $original) { $session->set('_security_session', $original); } - $tokenStorage = new UsageTrackingTokenStorage(new TokenStorage(), new class(['session' => function () use ($session) { - return $session; + $tokenStorage = new UsageTrackingTokenStorage(new TokenStorage(), new class(['request_stack' => function () use ($requestStack) { + return $requestStack; }, ]) implements ContainerInterface { use ServiceLocatorTrait; @@ -389,8 +394,6 @@ class ContextListenerTest extends TestCase $tokenStorage->setToken($newToken); - $request = new Request(); - $request->setSession($session); $request->cookies->set('MOCKSESSID', true); $sessionId = $session->getId(); @@ -424,13 +427,22 @@ class ContextListenerTest extends TestCase $request = new Request(); $request->setSession($session); $request->cookies->set('MOCKSESSID', true); + $requestStack = new RequestStack(); + $requestStack->push($request); $tokenStorage = new TokenStorage(); $usageIndex = $session->getUsageIndex(); - $tokenStorage = new UsageTrackingTokenStorage($tokenStorage, new class(['session' => function () use ($session) { - return $session; - }, - ]) implements ContainerInterface { + $tokenStorage = new UsageTrackingTokenStorage($tokenStorage, new class( + (new \ReflectionClass(UsageTrackingTokenStorage::class))->hasMethod('getSession') ? [ + 'request_stack' => function () use ($requestStack) { + return $requestStack; + }] : [ + // BC for symfony/framework-bundle < 5.3 + 'session' => function () use ($session) { + return $session; + }, + ] + ) implements ContainerInterface { use ServiceLocatorTrait; }); $sessionTrackerEnabler = [$tokenStorage, 'enableUsageTracking']; diff --git a/src/Symfony/Component/Security/Http/Tests/Logout/CsrfTokenClearingLogoutHandlerTest.php b/src/Symfony/Component/Security/Http/Tests/Logout/CsrfTokenClearingLogoutHandlerTest.php index 13f2f33507..6942745e88 100644 --- a/src/Symfony/Component/Security/Http/Tests/Logout/CsrfTokenClearingLogoutHandlerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Logout/CsrfTokenClearingLogoutHandlerTest.php @@ -13,6 +13,7 @@ namespace Symfony\Component\Security\Http\Tests\Logout; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; @@ -23,13 +24,23 @@ use Symfony\Component\Security\Http\Logout\CsrfTokenClearingLogoutHandler; class CsrfTokenClearingLogoutHandlerTest extends TestCase { private $session; + private $requestStack; private $csrfTokenStorage; private $csrfTokenClearingLogoutHandler; protected function setUp(): void { $this->session = new Session(new MockArraySessionStorage()); - $this->csrfTokenStorage = new SessionTokenStorage($this->session, 'foo'); + + // BC for symfony/security-core < 5.3 + if (\method_exists(SessionTokenStorage::class, 'getSession')) { + $request = new Request(); + $request->setSession($this->session); + $this->requestStack = new RequestStack(); + $this->requestStack->push($request); + } + + $this->csrfTokenStorage = new SessionTokenStorage($this->requestStack ?? $this->session, 'foo'); $this->csrfTokenStorage->setToken('foo', 'bar'); $this->csrfTokenStorage->setToken('foobar', 'baz'); $this->csrfTokenClearingLogoutHandler = new CsrfTokenClearingLogoutHandler($this->csrfTokenStorage); @@ -51,7 +62,7 @@ class CsrfTokenClearingLogoutHandlerTest extends TestCase public function testCsrfTokenCookieWithDifferentNamespaceIsNotRemoved() { - $barNamespaceCsrfSessionStorage = new SessionTokenStorage($this->session, 'bar'); + $barNamespaceCsrfSessionStorage = new SessionTokenStorage($this->requestStack ?? $this->session, 'bar'); $barNamespaceCsrfSessionStorage->setToken('foo', 'bar'); $barNamespaceCsrfSessionStorage->setToken('foobar', 'baz');