8000 Add ability to prioritize firewall listeners · symfony/symfony@91388e8 · GitHub
[go: up one dir, main page]
Skip to content

Commit 91388e8

Browse files
schebscheb
committed
Add ability to prioritize firewall listeners
1 parent 04de561 commit 91388e8

File tree

9 files changed

+230
-11
lines changed

9 files changed

+230
-11
lines changed

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/SortFirewallListenersPass.php

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
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\Bundle\SecurityBundle\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
15+
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
16+
use Symfony\Component\DependencyInjection\ContainerBuilder;
17+
use Symfony\Component\DependencyInjection\Definition;
18+
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
19+
use Symfony\Component\DependencyInjection\Reference;
20+
use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
21+
22+
/**
23+
* Sorts firewall listeners based on the execution order provided by FirewallListenerInterface::getPriority().
24+
*
25+
* @author Christian Scheb <me@christianscheb.de>
26+
*/
27+
class SortFirewallListenersPass implements CompilerPassInterface
28+
{
29+
public function process(ContainerBuilder $container): void
30+
{
31+
if (!$container->hasParameter('security.firewalls')) {
32+
return;
33+
}
34+
35+
foreach ($container->getParameter('security.firewalls') as $firewallName) {
36+
$firewallContextDefinition = $container->getDefinition('security.firewall.map.context.'.$firewallName);
37+
$this->sortFirewallContextListeners($firewallContextDefinition, $container);
38+
}
39+
}
40+
41+
private function sortFirewallContextListeners(Definition $definition, ContainerBuilder $container): void
42+
{
43+
/** @var IteratorArgument $listenerIteratorArgument */
44+
$listenerIteratorArgument = $definition->getArgument(0);
45+
$prioritiesByServiceId = $this->getListenerPriorities($listenerIteratorArgument, $container);
46+
47+
$listeners = $listenerIteratorArgument->getValues();
48+
usort($listeners, function (Reference $a, Reference $b) use ($prioritiesByServiceId) {
49+
return $prioritiesByServiceId[(string) $b] <=> $prioritiesByServiceId[(string) $a];
50+
});
51+
52+
$listenerIteratorArgument->setValues(array_values($listeners));
53+
}
54+
55+
private function getListenerPriorities(IteratorArgument $listeners, ContainerBuilder $container): array
56+
{
57+
$priorities = [];
58+
59+
foreach ($listeners->getValues() as $reference) {
60+
$id = (string) $reference;
61+
$def = $container->getDefinition($id);
62+
63+
// We must assume that the class value has been correctly filled, even if the service is created by a factory
64+
$class = $def->getClass();
65+
66+
if (!$r = $container->getReflectionClass($class)) {
67+
throw new InvalidArgumentException(sprintf('Class "%s" used for service "%s" cannot be found.', $class, $id));
68+
}
69+
70+
$priority = 0;
71+
if ($r->isSubclassOf(FirewallListenerInterface::class)) {
72+
$priority = $r->getMethod('getPriority')->invoke(null);
73+
}
74+
75+
$priorities[$id] = $priority;
76+
}
77+
78+
return $priorities;
79+
}
80+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterGlobalSecurityEventListenersPass;
1919
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterLdapLocatorPass;
2020
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterTokenUsageTrackingPass;
21+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\SortFirewallListenersPass;
2122
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AnonymousFactory;
2223
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\CustomAuthenticatorFactory;
2324
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory;
@@ -78,6 +79,8 @@ public function build(ContainerBuilder $container)
7879
$container->addCompilerPass(new RegisterLdapLocatorPass());
7980
// must be registered after RegisterListenersPass (in the FrameworkBundle)
8081
$container->addCompilerPass(new RegisterGlobalSecurityEventListenersPass(), PassConfig::TYPE_BEFORE_REMOVING, -200);
82+
// execute after ResolveChildDefinitionsPass optimization pass, to ensure class names are set
83+
$container->addCompilerPass(new SortFirewallListenersPass(), PassConfig::TYPE_BEFORE_REMOVING);
8184

8285
$container->addCompilerPass(new AddEventAliasesPass([
8386
AuthenticationSuccessEvent::class => AuthenticationEvents::AUTHENTICATION_SUCCESS,

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/SortFirewallListenersPassTest.php

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
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\Bundle\SecurityBundle\Tests\DependencyInjection\Compiler;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\SortFirewallListenersPass;
16+
use Symfony\Bundle\SecurityBundle\Security\FirewallContext;
17+
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
18+
use Symfony\Component\DependencyInjection\ContainerBuilder;
19+
use Symfony\Component\DependencyInjection\Reference;
20+
use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
21+
22+
class SortFirewallListenersPassTest extends TestCase
23+
{
24+
public function testSortFirewallListeners()
25+
{
26+
$container = new ContainerBuilder();
27+
$container->setParameter('security.firewalls', ['main']);
28+
29+
$container->register('listener_priority_minus1', FirewallListenerPriorityMinus1::class);
30+
$container->register('listener_priority_1', FirewallListenerPriority1::class);
31+
$container->register('listener_priority_2', FirewallListenerPriority2::class);
32+
$container->register('listener_interface_not_implemented', \stdClass::class);
33+
34+
$firewallContext = $container->register('security.firewall.map.context.main', FirewallContext::class);
35+
$firewallContext->addTag('security.firewall_map_context');
36+
37+
$listeners = new IteratorArgument([
38+
new Reference('listener_priority_minus1'),
39+
new Reference('listener_priority_1'),
40+
new Reference('listener_priority_2'),
41+
new Reference('listener_interface_not_implemented'),
42+
]);
43+
44+
$firewallContext->setArgument(0, $listeners);
45+
46+
$compilerPass = new SortFirewallListenersPass();
47+
$compilerPass->process($container);
48+
49+
$sortedListeners = $firewallContext->getArgument(0);
50+
$expectedSortedlisteners = [
51+
new Reference('listener_priority_2'),
52+
new Reference('listener_priority_1'),
53+
new Reference('listener_interface_not_implemented'),
54+
new Reference('listener_priority_minus1'),
55+
];
56+
$this->assertEquals($expectedSortedlisteners, $sortedListeners->getValues());
57+
}
58+
}
59+
60+
class FirewallListenerPriorityMinus1 implements FirewallListenerInterface
61+
{
62+
public static function getPriority(): int
63+
{
64+
return -1;
65+
}
66+
}
67+
68+
class FirewallListenerPriority1 implements FirewallListenerInterface
69+
{
70+
public static function getPriority(): int
71+
{
72+
return 1;
73+
}
74+
}
75+
76+
class FirewallListenerPriority2 implements FirewallListenerInterface
77+
{
78+
public static function getPriority(): int
79+
{
80+
return 2;
81+
}
82+
}

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
2424
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
2525
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
26+
use Symfony\Component\DependencyInjection\Compiler\ResolveChildDefinitionsPass;
2627
use Symfony\Component\DependencyInjection\ContainerBuilder;
2728
use Symfony\Component\DependencyInjection\Reference;
2829
use Symfony\Component\ExpressionLanguage\Expression;
@@ -671,7 +672,7 @@ protected function getRawContainer()
671672
$bundle = new SecurityBundle();
672673
$bundle->build($container);
673674

674-
$container->getCompilerPassConfig()->setOptimizationPasses([]);
675+
$container->getCompilerPassConfig()->setOptimizationPasses([new ResolveChildDefinitionsPass()]);
675676
$container->getCompilerPassConfig()->setRemovingPasses([]);
676677
$container->getCompilerPassConfig()->setAfterRemovingPasses([]);
677678

@@ -764,11 +765,16 @@ class TestFirewallListenerFactory implements SecurityFactoryInterface, FirewallL
764765
{
765766
public function createListeners(ContainerBuilder $container, string $firewallName, array $config): array
766767
{
768+
$container->register('custom_firewall_listener_id', \stdClass::class);
769+
767770
return ['custom_firewall_listener_id'];
768771
}
769772

770773
public function create(ContainerBuilder $container, string $id, array $config, string $userProvider, ?string $defaultEntryPoint)
771774
{
775+
$container->register('provider_id', \stdClass::class);
776+
$container->register('listener_id', \stdClass::class);
777+
772778
return ['provider_id', 'listener_id', $defaultEntryPoint];
773779
}
774780

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
1616
use Symfony\Component\HttpKernel\Event\RequestEvent;
1717
use Symfony\Component\HttpKernel\KernelEvents;
18-
use Symfony\Component\Security\Http\Firewall\AccessListener;
18+
use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
1919
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
2020

2121
/**
@@ -59,26 +59,28 @@ public function onKernelRequest(RequestEvent $event)
5959
$exceptionListener->register($this->dispatcher);
6060
}
6161

62+
// Authentication listeners are pre-sorted by SortFirewallListenersPass
6263
$authenticationListeners = function () use ($authenticationListeners, $logoutListener) {
63-
$accessListener = null;
64+
if (null !== $logoutListener) {
65+
$logoutListenerPriority = $this->getListenerPriority($logoutListener);
66+
}
6467

6568
foreach ($authenticationListeners as $listener) {
66-
if ($listener instanceof AccessListener) {
67-
$accessListener = $listener;
69+
$listenerPriority = $this->getListenerPriority($listener);
6870

69-
continue;
71+
// Yielding the LogoutListener at the correct position
72+
if (null !== $logoutListener && $listenerPriority < $logoutListenerPriority) {
73+
yield $logoutListener;
74+
$logoutListener = null;
7075
}
7176

7277
yield $listener;
7378
}
7479

80+
// When LogoutListener has the lowest priority of all listeners
7581
if (null !== $logoutListener) {
7682
yield $logoutListener;
7783
}
78-
79-
if (null !== $accessListener) {
80-
yield $accessListener;
81-
}
8284
};
8385

8486
$this->callListeners($event, $authenticationListeners());
@@ -115,4 +117,9 @@ protected function callListeners(RequestEvent $event, iterable $listeners)
115117
}
116118
}
117119
}
120+
121+
private function getListenerPriority(object $logoutListener): int
122+
{
123+
return $logoutListener instanceof FirewallListenerInterface ? $logoutListener->getPriority() : 0;
124+
}
118125
}

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
*
2020
* @author Nicolas Grekas <p@tchwork.com>
2121
*/
22-
abstract class AbstractListener
22+
abstract class AbstractListener implements FirewallListenerInterface
2323
{
2424
final public function __invoke(RequestEvent $event)
2525
{
@@ -39,4 +39,9 @@ abstract public function supports(Request $request): ?bool;
3939
* Does whatever is required to authenticate the request, typically calling $event->setResponse() internally.
4040
*/
4141
abstract public function authenticate(RequestEvent $event);
42+
43+
public static function getPriority(): int
44+
{
45+
return 0; // Default
46+
}
4247
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,9 @@ private function createAccessDeniedException(Request $request, array $attributes
122122

123123
return $exception;
124124
}
125+
126+
public static function getPriority(): int
127+
{
128+
return -255;
129+
}
125130
}

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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\Http\Firewall;
13+
14+
/**
15+
* Can be implemented by firewall listeners to define their priority in execution.
16+
*
17+
* @author Christian Scheb <me@christianscheb.de>
18+
*/
19+
interface FirewallListenerInterface
20+
{
21+
/**
22+
* Defines the priority of the listener.
23+
* The higher the number, the earlier a listener is executed.
24+
*/
25+
public static function getPriority(): int;
26+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,4 +142,9 @@ protected function requiresLogout(Request $request): bool
142142
{
143143
return isset($this->options['logout_path']) && $this->httpUtils->checkRequestPath($request, $this->options['logout_path']);
144144
}
145+
146+
public static function getPriority(): int
147+
{
148+
return -127;
149+
}
145150
}

0 commit comments

Comments
 (0)
0