Skip to content

Commit 510e506

Browse files
MatTheCatMatTheCat
committed
[Security] Deprecate callable firewall listeners
1 parent bf1b343 commit 510e506

17 files changed

+215
-92
lines changed

‎UPGRADE-7.4.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,10 @@ HttpFoundation
2727
--------------
2828

2929
* Deprecate using `Request::sendHeaders()` after headers have already been sent; use a `StreamedResponse` instead
30+
31+
Security
32+
--------
33+
34+
* Deprecate callable firewall listeners, extend `AbstractListener` or implement `FirewallListenerInterface` instead
35+
* Deprecate `AbstractListener::__invoke`
36+
* Deprecate `LazyFirewallContext::__invoke()`

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ CHANGELOG
2222
) {
2323
}
2424
```
25+
* Deprecate `LazyFirewallContext::__invoke()`
2526

2627
7.3
2728
---

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Bundle\SecurityBundle\Security\LazyFirewallContext;
1717
use Symfony\Component\HttpKernel\Event\RequestEvent;
1818
use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticatorManagerListener;
19+
use Symfony\Component\Security\Http\Firewall\AbstractListener;
1920
use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
2021
use Symfony\Contracts\Service\ResetInterface;
2122

@@ -88,7 +89,11 @@ protected function callListeners(RequestEvent $event, iterable $listeners): void
8889
}
8990

9091
foreach ($requestListeners as $listener) {
91-
$listener($event);
92+
if (!$listener instanceof FirewallListenerInterface) {
93+
$listener($event);
94+
} elseif (false !== $listener->supports($event->getRequest())) {
95+
$listener->authenticate($event);
96+
}
9297

9398
if ($event->hasResponse()) {
9499
break;

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Bundle\SecurityBundle\Security;
1313

1414
use Symfony\Component\Security\Http\Firewall\ExceptionListener;
15+
use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
1516
use Symfony\Component\Security\Http\Firewall\LogoutListener;
1617

1718
/**
@@ -39,7 +40,7 @@ public function getConfig(): ?FirewallConfig
3940
}
4041

4142
/**
42-
* @return iterable<mixed, callable>
43+
* @return iterable<mixed, FirewallListenerInterface|callable>
4344
*/
4445
public function getListeners(): iterable
4546
{

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

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@
1111

1212
namespace Symfony\Bundle\SecurityBundle\Security;
1313

14+
use Symfony\Component\HttpFoundation\Request;
1415
use Symfony\Component\HttpKernel\Event\RequestEvent;
1516
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
1617
use Symfony\Component\Security\Http\Event\LazyResponseEvent;
18+
use Symfony\Component\Security\Http\Firewall\AbstractListener;
1719
use Symfony\Component\Security\Http\Firewall\ExceptionListener;
1820
use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
1921
use Symfony\Component\Security\Http\Firewall\LogoutListener;
@@ -23,7 +25,7 @@
2325
*
2426
* @author Nicolas Grekas <[email protected]>
2527
*/
26-
class LazyFirewallContext extends FirewallContext
28+
class LazyFirewallContext extends FirewallContext implements FirewallListenerInterface
2729
{
2830
public function __construct(
2931
iterable $listeners,
@@ -40,19 +42,26 @@ public function getListeners(): iterable
4042
return [$this];
4143
}
4244

43-
public function __invoke(RequestEvent $event): void
45+
public function supports(Request $request): ?bool
46+
{
47+
return true;
48+
}
49+
50+
public function authenticate(RequestEvent $event): void
4451
{
4552
$listeners = [];
4653
$request = $event->getRequest();
4754
$lazy = $request->isMethodCacheable();
4855

4956
foreach (parent::getListeners() as $listener) {
50-
if (!$lazy || !$listener instanceof FirewallListenerInterface) {
57+
if (!$listener instanceof FirewallListenerInterface) {
58+
trigger_deprecation('symfony/security-http', '7.4', 'Using a callable as firewall listener is deprecated, extend "%s" or implement "%s" instead.', AbstractListener::class, FirewallListenerInterface::class);
59+
5160
$listeners[] = $listener;
52-
$lazy = $lazy && $listener instanceof FirewallListenerInterface;
61+
$lazy = false;
5362
} elseif (false !== $supports = $listener->supports($request)) {
5463
$listeners[] = [$listener, 'authenticate'];
55-
$lazy = null === $supports;
64+
$lazy = $lazy && null === $supports;
5665
}
5766
}
5867

@@ -75,4 +84,19 @@ public function __invoke(RequestEvent $event): void
7584
}
7685
});
7786
}
87+
88+
public static function getPriority(): int
89+
{
90+
return 0;
91+
}
92+
93+
/**
94+
* @deprecated since Symfony 7.4, to be removed in 8.0
95+
*/
96+
public function __invoke(RequestEvent $event): void
97+
{
98+
trigger_deprecation('symfony/security-bundle', '7.4', 'The "%s()" method is deprecated since Symfony 7.4 and will be removed in 8.0.', __METHOD__);
99+
100+
$this->authenticate($event);
101+
}
78102
}

‎src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
3333
use Symfony\Component\Security\Core\Role\RoleHierarchy;
3434
use Symfony\Component\Security\Core\User\InMemoryUser;
35+
use Symfony\Component\Security\Http\Firewall\AbstractListener;
36+
use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
3537
use Symfony\Component\Security\Http\FirewallMapInterface;
3638
use Symfony\Component\Security\Http\Logout\LogoutUrlGenerator;
3739
use Symfony\Component\VarDumper\Caster\ClassStub;
@@ -193,8 +195,18 @@ public function testGetListeners()
193195
$request = new Request();
194196
$event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST);
195197
$event->setResponse($response = new Response());
196-
$listener = function ($e) use ($event, &$listenerCalled) {
197-
$listenerCalled += $e === $event;
198+
$listener = new class extends AbstractListener {
199+
public int $callCount = 0;
200+
201+
public function supports(Request $request): ?bool
202+
{
203+
return true;
204+
}
205+
206+
public function authenticate(RequestEvent $event): void
207+
{
208+
++$this->callCount;
209+
}
198210
};
199211
$firewallMap = $this
200212
->getMockBuilder(FirewallMap::class)
@@ -217,9 +229,9 @@ public function testGetListeners()
217229
$collector = new SecurityDataCollector(null, null, null, null, $firewallMap, $firewall, true);
218230
$collector->collect($request, $response);
219231

220-
$this->assertNotEmpty($collected = $collector->getListeners()[0]);
232+
$this->assertCount(1, $collector->getListeners());
221233
$collector->lateCollect();
222-
$this->assertSame(1, $listenerCalled);
234+
$this->assertSame(1, $listener->callCount);
223235
}
224236

225237
public function testCollectCollectsDecisionLogWhenStrategyIsAffirmative()

‎src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
3030
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
3131
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
32+
use Symfony\Component\Security\Http\Firewall\AbstractListener;
3233
use Symfony\Component\Security\Http\Firewall\AuthenticatorManagerListener;
34+
use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
3335
use Symfony\Component\Security\Http\Logout\LogoutUrlGenerator;
3436

3537
/**
@@ -41,9 +43,19 @@ public function testOnKernelRequestRecordsListeners()
4143
{
4244
$request = new Request();
4345
$event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST);
44-
$event->setResponse($response = new Response());
45-
$listener = function ($e) use ($event, &$listenerCalled) {
46-
$listenerCalled += $e === $event;
46+
$event->setResponse(new Response());
47+
$listener = new class extends AbstractListener {
48+
public int $callCount = 0;
49+
50+
public function supports(Request $request): ?bool
51+
{
52+
return true;
53+
}
54+
55+
public function authenticate(RequestEvent $event): void
56+
{
57+
++$this->callCount;
58+
}
4759
};
4860
$firewallMap = $this->createMock(FirewallMap::class);
4961
$firewallMap

‎src/Symfony/Bundle/SecurityBundle/composer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
"symfony/clock": "^6.4|^7.0|^8.0",
2323
"symfony/config": "^7.3|^8.0",
2424
"symfony/dependency-injection": "^6.4.11|^7.1.4|^8.0",
25+
"symfony/deprecation-contracts": "^2.5|^3",
2526
"symfony/event-dispatcher": "^6.4|^7.0|^8.0",
2627
"symfony/http-kernel": "^6.4|^7.0|^8.0",
2728
"symfony/http-foundation": "^6.4|^7.0|^8.0",

‎src/Symfony/Component/Security/Http/CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
CHANGELOG
22
=========
33

4+
7.4
5+
---
6+
7+
* Deprecate callable firewall listeners, extend `AbstractListener` or implement `FirewallListenerInterface` instead
8+
* Deprecate `AbstractListener::__invoke`
9+
410
7.3
511
---
612

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
1717
use Symfony\Component\HttpKernel\Event\RequestEvent;
1818
use Symfony\Component\HttpKernel\KernelEvents;
19+
use Symfony\Component\Security\Http\Firewall\AbstractListener;
1920
use Symfony\Component\Security\Http\Firewall\ExceptionListener;
2021
use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
2122
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
@@ -123,6 +124,8 @@ protected function callListeners(RequestEvent $event, iterable $listeners)
123124
{
124125
foreach ($listeners as $listener) {
125126
if (!$listener instanceof FirewallListenerInterface) {
127+
trigger_deprecation('symfony/security-http', '7.4', 'Using a callable as firewall listener is deprecated, extend "%s" or implement "%s" instead.', AbstractListener::class, FirewallListenerInterface::class);
128+
126129
$listener($event);
127130
} elseif (false !== $listener->supports($event->getRequest())) {
128131
$listener->authenticate($event);
@@ -134,8 +137,8 @@ protected function callListeners(RequestEvent $event, iterable $listeners)
134137
}
135138
}
136139

137-
private function getListenerPriority(object $logoutListener): int
140+
private function getListenerPriority(object $listener): int
138141
{
139-
return $logoutListener instanceof FirewallListenerInterface ? $logoutListener->getPriority() : 0;
142+
return $listener instanceof FirewallListenerInterface ? $listener->getPriority() : 0;
140143
}
141144
}

0 commit comments

Comments
 (0)