Skip to content

Commit 7996efe

Browse files
fabpotfabpot
committed
[HttpFoundation] Extract request matchers for better reusability
1 parent d645e21 commit 7996efe

File tree

4 files changed

+94
-35
lines changed

4 files changed

+94
-35
lines changed

‎DependencyInjection/SecurityExtension.php

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,13 @@
3333
use Symfony\Component\EventDispatcher\EventDispatcher;
3434
use Symfony\Component\ExpressionLanguage\Expression;
3535
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
36-
use Symfony\Component\HttpFoundation\RequestMatcher;
36+
use Symfony\Component\HttpFoundation\ChainRequestMatcher;
37+
use Symfony\Component\HttpFoundation\RequestMatcher\AttributesRequestMatcher;
38+
use Symfony\Component\HttpFoundation\RequestMatcher\HostRequestMatcher;
39+
use Symfony\Component\HttpFoundation\RequestMatcher\IpsRequestMatcher;
40+
use Symfony\Component\HttpFoundation\RequestMatcher\MethodRequestMatcher;
41+
use Symfony\Component\HttpFoundation\RequestMatcher\PathRequestMatcher;
42+
use Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher;
3743
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
3844
use Symfony\Component\HttpKernel\KernelEvents;
3945
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
@@ -893,7 +899,7 @@ private function createRequestMatcher(ContainerBuilder $container, string $path
893899
$methods = array_map('strtoupper', $methods);
894900
}
895901

896-
if (null !== $ips) {
902+
if ($ips) {
897903
foreach ($ips as $ip) {
898904
$container->resolveEnvPlaceholders($ip, null, $usedEnvs);
899905

@@ -905,22 +911,58 @@ private function createRequestMatcher(ContainerBuilder $container, string $path
905911
}
906912
}
907913

908-
$id = '.security.request_matcher.'.ContainerBuilder::hash([$path, $host, $port, $methods, $ips, $attributes]);
914+
$id = '.security.request_matcher.'.ContainerBuilder::hash([ChainRequestMatcher::class, $path, $host, $port, $methods, $ips, $attributes]);
909915

910916
if (isset($this->requestMatchers[$id])) {
911917
return $this->requestMatchers[$id];
912918
}
913919

914-
// only add arguments that are necessary
915-
$arguments = [$path, $host, $methods, $ips, $attributes, null, $port];
916-
while (\count($arguments) > 0 && !end($arguments)) {
917-
array_pop($arguments);
920+
$arguments = [];
921+
if ($methods) {
922+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([MethodRequestMatcher::class, $methods]))) {
923+
$container->register($lid, MethodRequestMatcher::class)->setArguments([$methods]);
924+
}
925+
$arguments[] = new Reference($lid);
926+
}
927+
928+
if ($path) {
929+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([PathRequestMatcher::class, $path]))) {
930+
$container->register($lid, PathRequestMatcher::class)->setArguments([$path]);
931+
}
932+
$arguments[] = new Reference($lid);
933+
}
934+
935+
if ($host) {
936+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([HostRequestMatcher::class, $host]))) {
937+
$container->register($lid, HostRequestMatcher::class)->setArguments([$host]);
938+
}
939+
$arguments[] = new Reference($lid);
940+
}
941+
942+
if ($ips) {
943+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([IpsRequestMatcher::class, $ips]))) {
944+
$container->register($lid, IpsRequestMatcher::class)->setArguments([$ips]);
945+
}
946+
$arguments[] = new Reference($lid);
947+
}
948+
949+
if ($attributes) {
950+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([AttributesRequestMatcher::class, $attributes]))) {
951+
$container->register($lid, AttributesRequestMatcher::class)->setArguments([$attributes]);
952+
}
953+
$arguments[] = new Reference($lid);
954+
}
955+
956+
if ($port) {
957+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([PortRequestMatcher::class, $port]))) {
958+
$container->register($lid, PortRequestMatcher::class)->setArguments([$port]);
959+
}
960+
$arguments[] = new Reference($lid);
918961
}
919962

920963
$container
921-
->register($id, RequestMatcher::class)
922-
->setPublic(false)
923-
->setArguments($arguments)
964+
->register($id, ChainRequestMatcher::class)
965+
->setArguments([$arguments])
924966
;
925967

926968
return $this->requestMatchers[$id] = new Reference($id);

‎Tests/DependencyInjection/CompleteConfigurationTest.php

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
use Symfony\Component\DependencyInjection\ContainerBuilder;
2020
use Symfony\Component\DependencyInjection\Definition;
2121
use Symfony\Component\DependencyInjection\Reference;
22+
use Symfony\Component\HttpFoundation\RequestMatcher\HostRequestMatcher;
23+
use Symfony\Component\HttpFoundation\RequestMatcher\MethodRequestMatcher;
24+
use Symfony\Component\HttpFoundation\RequestMatcher\PathRequestMatcher;
25+
use Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher;
2226
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
2327
use Symfony\Component\PasswordHasher\Hasher\Pbkdf2PasswordHasher;
2428
use Symfony\Component\PasswordHasher\Hasher\PlaintextPasswordHasher;
@@ -131,7 +135,7 @@ public function testFirewalls()
131135
[
132136
'simple',
133137
'security.user_checker',
134-
'.security.request_matcher.xmi9dcw',
138+
'.security.request_matcher.h5ibf38',
135139
false,
136140
false,
137141
'',
@@ -180,7 +184,7 @@ public function testFirewalls()
180184
[
181185
'host',
182186
'security.user_checker',
183-
'.security.request_matcher.iw4hyjb',
187+
'.security.request_matcher.bcmu4fb',
184188
true,
185189
false,
186190
'security.user.provider.concrete.default',
@@ -248,20 +252,27 @@ public function testFirewallRequestMatchers()
248252
foreach ($arguments[1]->getValues() as $reference) {
249253
if ($reference instanceof Reference) {
250254
$definition = $container->getDefinition((string) $reference);
251-
$matchers[] = $definition->getArguments();
255+
$matchers[] = $definition->getArgument(0);
252256
}
253257
}
254258

255-
$this->assertEquals([
256-
[
257-
'/login',
258-
],
259-
[
260-
'/test',
261-
'foo\\.example\\.org',
262-
['GET', 'POST'],
263-
],
264-
], $matchers);
259+
$this->assertCount(2, $matchers);
260+
261+
$this->assertCount(1, $matchers[0]);
262+
$def = $container->getDefinition((string) $matchers[0][0]);
263+
$this->assertSame(PathRequestMatcher::class, $def->getClass());
264+
$this->assertSame('/login', $def->getArgument(0));
265+
266+
$this->assertCount(3, $matchers[1]);
267+
$def = $container->getDefinition((string) $matchers[1][0]);
268+
$this->assertSame(MethodRequestMatcher::class, $def->getClass());
269+
$this->assertSame(['GET', 'POST'], $def->getArgument(0));
270+
$def = $container->getDefinition((string) $matchers[1][1]);
271+
$this->assertSame(PathRequestMatcher::class, $def->getClass());
272+
$this->assertSame('/test', $def->getArgument(0));
273+
$def = $container->getDefinition((string) $matchers[1][2]);
274+
$this->assertSame(HostRequestMatcher::class, $def->getClass());
275+
$this->assertSame('foo\\.example\\.org', $def->getArgument(0));
265276
}
266277

267278
public function testUserCheckerAliasIsRegistered()
@@ -294,17 +305,23 @@ public function testAccess()
294305
if (1 === $i) {
295306
$this->assertEquals(['ROLE_USER'], $attributes);
296307
$this->assertEquals('https', $channel);
297-
$this->assertEquals(
298-
['/blog/524', null, ['GET', 'POST'], [], [], null, 8000],
299-
$requestMatcher->getArguments()
300-
);
308+
$this->assertCount(3, $requestMatcher->getArgument(0));
309+
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[0]);
310+
$this->assertSame(MethodRequestMatcher::class, $def->getClass());
311+
$this->assertSame(['GET', 'POST'], $def->getArgument(0));
312+
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[1]);
313+
$this->assertSame(PathRequestMatcher::class, $def->getClass());
314+
$this->assertSame('/blog/524', $def->getArgument(0));
315+
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[2]);
316+
$this->assertSame(PortRequestMatcher::class, $def->getClass());
317+
$this->assertSame(8000, $def->getArgument(0));
301318
} elseif (2 === $i) {
302319
$this->assertEquals(['IS_AUTHENTICATED_ANONYMOUSLY'], $attributes);
303320
$this->assertNull($channel);
304-
$this->assertEquals(
305-
['/blog/.*'],
306-
$requestMatcher->getArguments()
307-
);
321+
$this->assertCount(1, $requestMatcher->getArgument(0));
322+
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[0]);
323+
$this->assertSame(PathRequestMatcher::class, $def->getClass());
324+
$this->assertSame('/blog/.*', $def->getArgument(0));
308325
} elseif (3 === $i) {
309326
$this->assertEquals('IS_AUTHENTICATED_ANONYMOUSLY', $attributes[0]);
310327
$expression = $container->getDefinition((string) $attributes[1])->getArgument(0);

‎Tests/DependencyInjection/SecurityExtensionTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
use Symfony\Component\DependencyInjection\Reference;
2727
use Symfony\Component\ExpressionLanguage\Expression;
2828
use Symfony\Component\HttpFoundation\Request;
29-
use Symfony\Component\HttpFoundation\RequestMatcher;
29+
use Symfony\Component\HttpFoundation\RequestMatcher\PathRequestMatcher;
3030
use Symfony\Component\HttpFoundation\Response;
3131
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
3232
use Symfony\Component\Security\Core\Exception\AuthenticationException;
@@ -256,7 +256,7 @@ public function testRegisterAccessControlWithSpecifiedRequestMatcherService()
256256
$container = $this->getRawContainer();
257257

258258
$requestMatcherId = 'My\Test\RequestMatcher';
259-
$requestMatcher = new RequestMatcher('/');
259+
$requestMatcher = new PathRequestMatcher('/');
260260
$container->set($requestMatcherId, $requestMatcher);
261261

262262
$container->loadFromExtension('security', [
@@ -291,7 +291,7 @@ public function testRegisterAccessControlWithRequestMatcherAndAdditionalOptionsT
291291
$container = $this->getRawContainer();
292292

293293
$requestMatcherId = 'My\Test\RequestMatcher';
294-
$requestMatcher = new RequestMatcher('/');
294+
$requestMatcher = new PathRequestMatcher('/');
295295
$container->set($requestMatcherId, $requestMatcher);
296296

297297
$container->loadFromExtension('security', [

‎composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
"symfony/dependency-injection": "^6.2",
2424
"symfony/event-dispatcher": "^5.4|^6.0",
2525
"symfony/http-kernel": "^6.2",
26-
"symfony/http-foundation": "^5.4|^6.0",
26+
"symfony/http-foundation": "^6.2",
2727
"symfony/password-hasher": "^5.4|^6.0",
2828
"symfony/security-core": "^6.2",
2929
"symfony/security-csrf": "^5.4|^6.0",

0 commit comments

Comments
 (0)