Skip to content

Commit b83d250

Browse files
fabpotfabpot
committed
bug symfony#37368 [Security] Resolve event bubbling of logout + new events in a compiler pass (wouterj)
This PR was merged into the 5.1 branch. Discussion ---------- [Security] Resolve event bubbling of logout + new events in a compiler pass | Q | A | ------------- | --- | Branch? | 5.1 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix symfony#37292 | License | MIT | Doc PR | - This PR proposes to create a compiler pass that registers listeners on the main `event_dispatcher` on the firewall-specific event dispatcher during compile time. This allows to still specify listener priorities while listening on a bubbled-up event (instead of a fix moment where the event bubbling occurs). It probably also improves performance, as it doesn't use duplicated event dispatching logic to provide event bubbling. Nothing changes on the user side. I proposed this as a bugfix, as it fixes the bug mentioned in symfony#37292 (not being able to use listener priorities). I did remove a class, which was introduced in 5.1 and is very internal. I think it's safe, but we can also keep it and remove in master. Commits ------- f962c26 Resolve event bubbling logic in a compiler pass
2 parents 0878dd4 + f962c26 commit b83d250

File tree

5 files changed

+241
-52
lines changed

5 files changed

+241
-52
lines changed

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

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
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\Compiler\CompilerPassInterface;
15+
use Symfony\Component\DependencyInjection\ContainerBuilder;
16+
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
17+
use Symfony\Component\Security\Http\Event\LoginFailureEvent;
18+
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
19+
use Symfony\Component\Security\Http\Event\LogoutEvent;
20+
21+
/**
22+
* Makes sure all event listeners on the global dispatcher are also listening
23+
* to events on the firewall-specific dipatchers.
24+
*
25+
* This compiler pass must be run after RegisterListenersPass of the
26+
* EventDispatcher component.
27+
*
28+
* @author Wouter de Jong <[email protected]>
29+
*
30+
* @internal
31+
*/
32+
class RegisterGlobalSecurityEventListenersPass implements CompilerPassInterface
33+
{
34+
private static $eventBubblingEvents = [CheckPassportEvent::class, LoginFailureEvent::class, LoginSuccessEvent::class, LogoutEvent::class];
35+
36+
/**
37+
* {@inheritdoc}
38+
*/
39+
public function process(ContainerBuilder $container)
40+
{
41+
if (!$container->has('event_dispatcher') || !$container->hasParameter('security.firewalls')) {
42+
return;
43+
}
44+
45+
$firewallDispatchers = [];
46+
foreach ($container->getParameter('security.firewalls') as $firewallName) {
47+
if (!$container->has('security.event_dispatcher.'.$firewallName)) {
48+
continue;
49+
}
50+
51+
$firewallDispatchers[] = $container->findDefinition('security.event_dispatcher.'.$firewallName);
52+
}
53+
54+
$globalDispatcher = $container->findDefinition('event_dispatcher');
55+
foreach ($globalDispatcher->getMethodCalls() as $methodCall) {
56+
if ('addListener' !== $methodCall[0]) {
57+
continue;
58+
}
59+
60+
$methodCallArguments = $methodCall[1];
61+
if (!\in_array($methodCallArguments[0], self::$eventBubblingEvents, true)) {
62+
continue;
63+
}
64+
65+
foreach ($firewallDispatchers as $firewallDispatcher) {
66+
$firewallDispatcher->addMethodCall('addListener', $methodCallArguments);
67+
}
68+
}
69+
}
70+
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,8 @@ private function createFirewalls(array $config, ContainerBuilder $container)
236236
$firewalls = $config['firewalls'];
237237
$providerIds = $this->createUserProviders($config, $container);
238238

239+
$container->setParameter('security.firewalls', array_keys($firewalls));
240+
239241
// make the ContextListener aware of the configured user providers
240242
$contextListenerDefinition = $container->getDefinition('security.context_listener');
241243
$arguments = $contextListenerDefinition->getArguments();
@@ -348,8 +350,6 @@ private function createFirewall(ContainerBuilder $container, string $id, array $
348350
// Register Firewall-specific event dispatcher
349351
$firewallEventDispatcherId = 'security.event_dispatcher.'.$id;
350352
$container->register($firewallEventDispatcherId, EventDispatcher::class);
351-
$container->setDefinition($firewallEventDispatcherId.'.event_bubbling_listener', new ChildDefinition('security.event_dispatcher.event_bubbling_listener'))
352-
->addTag('kernel.event_subscriber', ['dispatcher' => $firewallEventDispatcherId]);
353353

354354
// Register listeners
355355
$listeners = [];

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

-50
This file was deleted.

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

+3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
1616
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
1717
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterCsrfFeaturesPass;
18+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterGlobalSecurityEventListenersPass;
1819
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterLdapLocatorPass;
1920
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterTokenUsageTrackingPass;
2021
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AnonymousFactory;
@@ -75,6 +76,8 @@ public function build(ContainerBuilder $container)
7576
$container->addCompilerPass(new RegisterCsrfFeaturesPass());
7677
$container->addCompilerPass(new RegisterTokenUsageTrackingPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 200);
7778
$container->addCompilerPass(new RegisterLdapLocatorPass());
79+
// must be registered after RegisterListenersPass (in the FrameworkBundle)
80+
$container->addCompilerPass(new RegisterGlobalSecurityEventListenersPass(), PassConfig::TYPE_BEFORE_REMOVING, -200);
7881

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

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

+166
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
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\SecurityExtension;
16+
use Symfony\Bundle\SecurityBundle\SecurityBundle;
17+
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
18+
use Symfony\Component\DependencyInjection\ContainerBuilder;
19+
use Symfony\Component\EventDispatcher\DependencyInjection\RegisterListenersPass;
20+
use Symfony\Component\EventDispatcher\EventDispatcher;
21+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
22+
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
23+
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
24+
use Symfony\Component\Security\Http\Event\LogoutEvent;
25+
26+
class RegisterGlobalSecurtyEventListenersPassTest extends TestCase
27+
{
28+
private $container;
29+
30+
protected function setUp(): void
31+
{
32+
$this->container = new ContainerBuilder();
33+
$this->container->setParameter('kernel.debug', false);
34+
$this->container->register('request_stack', \stdClass::class);
35+
$this->container->register('event_dispatcher', EventDispatcher::class);
36+
37+
$this->container->registerExtension(new SecurityExtension());
38+
39+
$this->container->addCompilerPass(new RegisterListenersPass(), PassConfig::TYPE_BEFORE_REMOVING);
40+
$this->container->getCompilerPassConfig()->setRemovingPasses([]);
41+
$this->container->getCompilerPassConfig()->setAfterRemovingPasses([]);
42+
43+
$securityBundle = new SecurityBundle();
44+
$securityBundle->build($this->container);
45+
}
46+
47+
public function testRegisterCustomListener()
48+
{
49+
$this->container->loadFromExtension('security', [
50+
'enable_authenticator_manager' => true,
51+
'firewalls' => ['main' => ['pattern' => '/', 'http_basic' => true]],
52+
]);
53+
54+
$this->container->register('app.security_listener', \stdClass::class)
55+
->addTag('kernel.event_listener', ['method' => 'onLogout', 'event' => LogoutEvent::class])
56+
->addTag('kernel.event_listener', ['method' => 'onLoginSuccess', 'event' => LoginSuccessEvent::class, 'priority' => 20]);
57+
58+
$this->container->compile();
59+
60+
$this->assertListeners([
61+
[LogoutEvent::class, ['app.security_listener', 'onLogout'], 0],
62+
[LoginSuccessEvent::class, ['app.security_listener', 'onLoginSuccess'], 20],
63+
]);
64+
}
65+
66+
public function testRegisterCustomSubscriber()
67+
{
68+
$this->container->loadFromExtension('security', [
69+
'enable_authenticator_manager' => true,
70+
'firewalls' => ['main' => ['pattern' => '/', 'http_basic' => true]],
71+
]);
72+
73+
$this->container->register(TestSubscriber::class)
74+
->addTag('kernel.event_subscriber');
75+
76+
$this->container->compile();
77+
78+
$this->assertListeners([
79+
[LogoutEvent::class, [TestSubscriber::class, 'onLogout'], -200],
80+
[CheckPassportEvent::class, [TestSubscriber::class, 'onCheckPassport'], 120],
81+
[LoginSuccessEvent::class, [TestSubscriber::class, 'onLoginSuccess'], 0],
82+
]);
83+
}
84+
85+
public function testMultipleFirewalls()
86+
{
87+
$this->container->loadFromExtension('security', [
88+
'enable_authenticator_manager' => true,
89+
'firewalls' => ['main' => ['pattern' => '/', 'http_basic' => true], 'api' => ['pattern' => '/api', 'http_basic' => true]],
90+
]);
91+
92+
$this->container->register('security.event_dispatcher.api', EventDispatcher::class)
93+
->addTag('security.event_dispatcher')
94+
->setPublic(true);
95+
96+
$this->container->register('app.security_listener', \stdClass::class)
97+
->addTag('kernel.event_listener', ['method' => 'onLogout', 'event' => LogoutEvent::class])
98+
->addTag('kernel.event_listener', ['method' => 'onLoginSuccess', 'event' => LoginSuccessEvent::class, 'priority' => 20]);
99+
100+
$this->container->compile();
101+
102+
$this->assertListeners([
103+
[LogoutEvent::class, ['app.security_listener', 'onLogout'], 0],
104+
[LoginSuccessEvent::class, ['app.security_listener', 'onLoginSuccess'], 20],
105+
], 'security.event_dispatcher.main');
106+
$this->assertListeners([
107+
[LogoutEvent::class, ['app.security_listener', 'onLogout'], 0],
108+
[LoginSuccessEvent::class, ['app.security_listener', 'onLoginSuccess'], 20],
109+
], 'security.event_dispatcher.api');
110+
}
111+
112+
public function testListenerAlreadySpecific()
113+
{
114+
$this->container->loadFromExtension('security', [
115+
'enable_authenticator_manager' => true,
116+
'firewalls' => ['main' => ['pattern' => '/', 'http_basic' => true]],
117+
]);
118+
119+
$this->container->register('security.event_dispatcher.api', EventDispatcher::class)
120+
->addTag('security.event_dispatcher')
121+
->setPublic(true);
122+
123+
$this->container->register('app.security_listener', \stdClass::class)
124+
->addTag('kernel.event_listener', ['method' => 'onLogout', 'event' => LogoutEvent::class, 'dispatcher' => 'security.event_dispatcher.main'])
125+
->addTag('kernel.event_listener', ['method' => 'onLoginSuccess', 'event' => LoginSuccessEvent::class, 'priority' => 20]);
126+
127+
$this->container->compile();
128+
129+
$this->assertListeners([
130+
[LogoutEvent::class, ['app.security_listener', 'onLogout'], 0],
131+
[LoginSuccessEvent::class, ['app.security_listener', 'onLoginSuccess'], 20],
132+
], 'security.event_dispatcher.main');
133+
}
134+
135+
private function assertListeners(array $expectedListeners, string $dispatcherId = 'security.event_dispatcher.main')
136+
{
137+
$actualListeners = [];
138+
foreach ($this->container->findDefinition($dispatcherId)->getMethodCalls() as $methodCall) {
139+
[$method, $arguments] = $methodCall;
140+
if ('addListener' !== $method) {
141+
continue;
142+
}
143+
144+
$arguments[1] = [(string) $arguments[1][0]->getValues()[0], $arguments[1][1]];
145+
$actualListeners[] = $arguments;
146+
}
147+
148+
$foundListeners = array_uintersect($expectedListeners, $actualListeners, function (array $a, array $b) {
149+
return $a === $b;
150+
});
151+
152+
$this->assertEquals($expectedListeners, $foundListeners);
153+
}
154+
}
155+
156+
class TestSubscriber implements EventSubscriberInterface
157+
{
158+
public static function getSubscribedEvents(): array
159+
{
160+
return [
161+
LogoutEvent::class => ['onLogout', -200],
162+
CheckPassportEvent::class => ['onCheckPassport', 120],
163+
LoginSuccessEvent::class => 'onLoginSuccess',
164+
];
165+
}
166+
}

0 commit comments

Comments
 (0)