8000 [Security] Deprecate callable firewall listeners · symfony/symfony@2b96194 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2b96194

Browse files
MatTheCatMatTheCat
committed
[Security] Deprecate callable firewall listeners
1 parent 9cae55b commit 2b96194

File tree

8 files changed

+104
-24
lines changed

8 files changed

+104
-24
lines changed

‎UPGRADE-7.4.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,8 @@ HttpClient
2222
----------
2323

2424
* Deprecate using amphp/http-client < 5
25+
26+
Security
27+
--------
28+
29+
* Deprecate callable firewall listeners, extend `AbstractListener` or implement `FirewallListenerInterface` instead

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

Lines changed: 24 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 <p@tchwork.com>
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,14 @@ public function __invoke(RequestEvent $event): void
7584
}
7685
});
7786
}
87+
88+
public static function getPriority(): int
89+
{
90+
return 0;
91+
}
92+
93+
public function __invoke(RequestEvent $event): void
94+
{
95+
$this->authenticate($event);
96+
}
7897
}

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

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
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\FirewallListenerInterface;
3536
use Symfony\Component\Security\Http\FirewallMapInterface;
3637
use Symfony\Component\Security\Http\Logout\LogoutUrlGenerator;
3738
use Symfony\Component\VarDumper\Caster\ClassStub;
@@ -193,8 +194,24 @@ public function testGetListeners()
193194
$request = new Request();
194195
$event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST);
195196
$event->setResponse($response = new Response());
196-
$listener = function ($e) use ($event, &$listenerCalled) {
197-
$listenerCalled += $e === $event;
197+
$listener = new class implements FirewallListenerInterface
198+
{
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+
}
210+
211+
public static function getPriority(): int
212+
{
213+
return 0;
214+
}
198215
};
199216
$firewallMap = $this
200217
->getMockBuilder(FirewallMap::class)
@@ -217,9 +234,9 @@ public function testGetListeners()
217234
$collector = new SecurityDataCollector(null, null, null, null, $firewallMap, $firewall, true);
218235
$collector->collect($request, $response);
219236

220-
$this->assertNotEmpty($collected = $collector->getListeners()[0]);
237+
$this->assertCount(1, $collector->getListeners());
221238
$collector->lateCollect();
222-
$this->assertSame(1, $listenerCalled);
239+
$this->assertSame(1, $listener->callCount);
223240
}
224241

225242
public function testCollectCollectsDecisionLogWhenStrategyIsAffirmative()

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

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
3131
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
3232
use Symfony\Component\Security\Http\Firewall\AuthenticatorManagerListener;
33+
use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
3334
use Symfony\Component\Security\Http\Logout\LogoutUrlGenerator;
3435

3536
/**
@@ -41,9 +42,25 @@ public function testOnKernelRequestRecordsListeners()
4142
{
4243
$request = new Request();
4344
$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;
45+
$event->setResponse(new Response());
46+
$listener = new class implements FirewallListenerInterface
47+
{
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+
}
59+
60+
public static function getPriority(): int
61+
{
62+
return 0;
63+
}
4764
};
4865
$firewallMap = $this->createMock(FirewallMap::class);
4966
$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: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
7.4
5+
---
6+
7+
* Deprecate callable firewall listeners, extend `AbstractListener` or implement `FirewallListenerInterface` instead
8+
49
7.3
510
---
611

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

Lines changed: 7 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;
@@ -122,6 +123,10 @@ public static function getSubscribedEvents()
122123
protected function callListeners(RequestEvent $event, iterable $listeners)
123124
{
124125
foreach ($listeners as $listener) {
126+
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+
}
129+
125130
$listener($event);
126131

127132
if ($event->hasResponse()) {
@@ -130,8 +135,8 @@ protected function callListeners(RequestEvent $event, iterable $listeners)
130135
}
131136
}
132137

133-
private function getListenerPriority(object $logoutListener): int
138+
private function getListenerPriority(object $listener): int
134139
{
135-
return $logoutListener instanceof FirewallListenerInterface ? $logoutListener->getPriority() : 0;
140+
return $listener instanceof FirewallListenerInterface ? $listener->getPriority() : 0;
136141
}
137142
}

‎src/Symfony/Component/Security/Http/Tests/FirewallTest.php

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\HttpKernel\Event\RequestEvent;
1919
use Symfony\Component\HttpKernel\HttpKernelInterface;
2020
use Symfony\Component\Security\Http\Firewall;
21+
use Symfony\Component\Security\Http\Firewall\AbstractListener;
2122
use Symfony\Component\Security\Http\Firewall\ExceptionListener;
2223
use Symfony\Component\Security\Http\FirewallMapInterface;
2324

@@ -52,21 +53,31 @@ public function testOnKernelRequestRegistersExceptionListener()
5253

5354
public function testOnKernelRequestStopsWhenThereIsAResponse()
5455
{
55-
$called = [];
56-
57-
$first = function () use (&$called) {
58-
$called[] = 1;
59-
};
60-
61-
$second = function () use (&$called) {
62-
$called[] = 2;
56+
$listener = new class extends AbstractListener
57+
{
58+
public int $callCount = 0;
59+
60+
public function supports(Request $request): ?bool
61+
{
62+
return true;
63+
}
64+
65+
public function authenticate(RequestEvent $event): void
66+
{
67+
++$this->callCount;
68+
}
69+
70+
public static function getPriority(): int
71+
{
72+
return 0;
73+
}
6374
};
6475

6576
$map = $this->createMock(FirewallMapInterface::class);
6677
$map
6778
->expects($this->once())
6879
->method('getListeners')
69-
->willReturn([[$first, $second], null, null])
80+
->willReturn([[$listener, $listener], null, null])
7081
;
7182

7283
$event = new RequestEvent($this->createMock(HttpKernelInterface::class), new Request(), HttpKernelInterface::MAIN_REQUEST);
@@ -75,7 +86,7 @@ public function testOnKernelRequestStopsWhenThereIsAResponse()
7586
$firewall = new Firewall($map, $this->createMock(EventDispatcherInterface::class));
7687
$firewall->onKernelRequest($event);
7788

78-
$this->assertSame([1], $called);
89+
$this->assertSame(1, $listener->callCount);
7990
}
8091

8192
public function testOnKernelRequestWithSubRequest()

0 commit comments

Comments
 (0)
0