Skip to content

Commit

Permalink
bug #37047 [SecurityBundle] Only register CSRF protection listener if…
Browse files Browse the repository at this point in the history
… CSRF is available (wouterj)

This PR was merged into the 5.1 branch.

Discussion
----------

[SecurityBundle] Only register CSRF protection listener if CSRF is available

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | yes
| Tickets       | Fix #37033
| License       | MIT
| Doc PR        | -

I know we're not allowed to add new deprecations in already released versions. However, I don't think anyone is using SecurityBundle's compiler passes except from Symfony itself - so I don't think anyone is affected by this deprecation. The alternatives would be:

* Add a new compiler pass in 5.1 that conditionally registers the CSRF listener
* Do this exact change in 5.2 and...
  * accept a `null` argument in the listener for 5.1
  * or add this to the `RegisterCsrfTokenClearingLogoutHandlerPass` class in 5.1

Commits
-------

2d738b3 Only register CSRF protection listener if CSRF is available
  • Loading branch information
fabpot committed Jun 3, 2020
2 parents 2a9edfa + 2d738b3 commit d341254
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 24 deletions.
@@ -0,0 +1,68 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Security\Http\EventListener\CsrfProtectionListener;
use Symfony\Component\Security\Http\EventListener\CsrfTokenClearingLogoutListener;

/**
* @author Christian Flothmann <christian.flothmann@sensiolabs.de>
* @author Wouter de Jong <wouter@wouterj.nl>
*
* @internal
*/
class RegisterCsrfFeaturesPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
if (!$container->has('security.csrf.token_storage')) {
return;
}

$this->registerCsrfProtectionListener($container);
$this->registerLogoutHandler($container);
}

private function registerCsrfProtectionListener(ContainerBuilder $container)
{
if (!$container->has('security.authenticator.manager')) {
return;
}

$container->register('security.listener.csrf_protection', CsrfProtectionListener::class)
->addArgument(new Reference('security.csrf.token_storage'))
->addTag('kernel.event_subscriber')
->setPublic(false);
}

protected function registerLogoutHandler(ContainerBuilder $container)
{
if (!$container->has('security.logout_listener')) {
return;
}

$csrfTokenStorage = $container->findDefinition('security.csrf.token_storage');
$csrfTokenStorageClass = $container->getParameterBag()->resolveValue($csrfTokenStorage->getClass());

if (!is_subclass_of($csrfTokenStorageClass, 'Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface')) {
return;
}

$container->register('security.logout.listener.csrf_token_clearing', CsrfTokenClearingLogoutListener::class)
->addArgument(new Reference('security.csrf.token_storage'))
->addTag('kernel.event_subscriber')
->setPublic(false);
}
}
Expand Up @@ -11,32 +11,21 @@

namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Security\Http\EventListener\CsrfTokenClearingLogoutListener;

trigger_deprecation('symfony/security-bundle', '5.1', 'The "%s" class is deprecated.', RegisterCsrfTokenClearingLogoutHandlerPass::class);

/**
* @author Christian Flothmann <christian.flothmann@sensiolabs.de>
* @deprecated since symfony/security-bundle 5.1
*/
class RegisterCsrfTokenClearingLogoutHandlerPass implements CompilerPassInterface
class RegisterCsrfTokenClearingLogoutHandlerPass extends RegisterCsrfFeaturesPass
{
public function process(ContainerBuilder $container)
{
if (!$container->has('security.logout_listener') || !$container->has('security.csrf.token_storage')) {
return;
}

$csrfTokenStorage = $container->findDefinition('security.csrf.token_storage');
$csrfTokenStorageClass = $container->getParameterBag()->resolveValue($csrfTokenStorage->getClass());

if (!is_subclass_of($csrfTokenStorageClass, 'Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface')) {
if (!$container->has('security.csrf.token_storage')) {
return;
}

$container->register('security.logout.listener.csrf_token_clearing', CsrfTokenClearingLogoutListener::class)
->addArgument(new Reference('security.csrf.token_storage'))
->addTag('kernel.event_subscriber')
->setPublic(false);
$this->registerLogoutHandler($container);
}
}
Expand Up @@ -66,11 +66,6 @@
<argument type="abstract">stateless firewall keys</argument>
</service>

<service id="security.listener.csrf_protection" class="Symfony\Component\Security\Http\EventListener\CsrfProtectionListener">
<tag name="kernel.event_subscriber" />
<argument type="service" id="security.csrf.token_manager" />
</service>

<service id="security.listener.remember_me"
class="Symfony\Component\Security\Http\EventListener\RememberMeListener"
abstract="true">
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bundle/SecurityBundle/SecurityBundle.php
Expand Up @@ -14,7 +14,7 @@
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterCsrfTokenClearingLogoutHandlerPass;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterCsrfFeaturesPass;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterLdapLocatorPass;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterTokenUsageTrackingPass;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AnonymousFactory;
Expand Down Expand Up @@ -72,7 +72,7 @@ public function build(ContainerBuilder $container)
$container->addCompilerPass(new AddExpressionLanguageProvidersPass());
$container->addCompilerPass(new AddSecurityVotersPass());
$container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_BEFORE_REMOVING);
$container->addCompilerPass(new RegisterCsrfTokenClearingLogoutHandlerPass());
$container->addCompilerPass(new RegisterCsrfFeaturesPass());
$container->addCompilerPass(new RegisterTokenUsageTrackingPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 200);
$container->addCompilerPass(new RegisterLdapLocatorPass());

Expand Down

0 comments on commit d341254

Please sign in to comment.