Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 897f287

Browse filesBrowse files
bug #40902 [Security] Allow ips parameter in access_control to accept comma-separated string (edefimov)
This PR was merged into the 5.2 branch. Discussion ---------- [Security] Allow ips parameter in access_control to accept comma-separated string | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #40881, #40864, #40865 | License | MIT PR #38149 introduced a new feature to accept a comma-separated string in ip adresses setting in `access_control` configuration section of security bundle. However the feature works in inconsistent manner: comma-separated string can be successfully passed via environment variable, but can not be passed as plain string. This PR changes this inconsistent behavior by allowing validation pass if comma-separated list of ip addresses is given in plain string. More detailed explanation about the inconsistent behavior can be found [here](#40881 (comment)) Commits ------- 8947482 [SecurityBundle] Allow ips parameter in access_control accept comma-separated string
2 parents 38a991e + 8947482 commit 897f287
Copy full SHA for 897f287

File tree

Expand file treeCollapse file tree

2 files changed

+57
-1
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+57
-1
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
+20-1Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -872,7 +872,7 @@ private function createRequestMatcher(ContainerBuilder $container, string $path
872872
foreach ($ips as $ip) {
873873
$container->resolveEnvPlaceholders($ip, null, $usedEnvs);
874874

875-
if (!$usedEnvs && !$this->isValidIp($ip)) {
875+
if (!$usedEnvs && !$this->isValidIps($ip)) {
876876
throw new \LogicException(sprintf('The given value "%s" in the "security.access_control" config option is not a valid IP address.', $ip));
877877
}
878878

@@ -930,6 +930,25 @@ public function getConfiguration(array $config, ContainerBuilder $container)
930930
return new MainConfiguration($this->factories, $this->userProviderFactories);
931931
}
932932

933+
private function isValidIps($ips): bool
934+
{
935+
$ipsList = array_reduce((array) $ips, static function (array $ips, string $ip) {
936+
return array_merge($ips, preg_split('/\s*,\s*/', $ip));
937+
}, []);
938+
939+
if (!$ipsList) {
940+
return false;
941+
}
942+
943+
foreach ($ipsList as $cidr) {
944+
if (!$this->isValidIp($cidr)) {
945+
return false;
946+
}
947+
}
948+
949+
return true;
950+
}
951+
933952
private function isValidIp(string $cidr): bool
934953
{
935954
$cidrParts = explode('/', $cidr);

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php
+37Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,33 @@ public function testRememberMeCookieInheritFrameworkSessionCookie($config, $same
388388
$this->assertEquals($secure, $definition->getArgument(3)['secure']);
389389
}
390390

391+
/**
392+
* @dataProvider acceptableIpsProvider
393+
*/
394+
public function testAcceptableAccessControlIps($ips)
395+
{
396+
$container = $this->getRawContainer();
397+
398+
$container->loadFromExtension('security', [
399+
'providers' => [
400+
'default' => ['id' => 'foo'],
401+
],
402+
'firewalls' => [
403+
'some_firewall' => [
404+
'pattern' => '/.*',
405+
'http_basic' => [],
406+
],
407+
],
408+
'access_control' => [
409+
['ips' => $ips, 'path' => '/somewhere', 'roles' => 'IS_AUTHENTICATED_FULLY'],
410+
],
411+
]);
412+
413+
$container->compile();
414+
415+
$this->assertTrue(true, 'Ip addresses is successfully consumed: '.(\is_string($ips) ? $ips : json_encode($ips)));
416+
}
417+
391418
public function sessionConfigurationProvider()
392419
{
393420
return [
@@ -408,6 +435,16 @@ public function sessionConfigurationProvider()
408435
];
409436
}
410437

438+
public function acceptableIpsProvider(): iterable
439+
{
440+
yield [['127.0.0.1']];
441+
yield ['127.0.0.1'];
442+
yield ['127.0.0.1, 127.0.0.2'];
443+
yield ['127.0.0.1/8, 127.0.0.2/16'];
444+
yield [['127.0.0.1/8, 127.0.0.2/16']];
445+
yield [['127.0.0.1/8', '127.0.0.2/16']];
446+
}
447+
411448
public function testSwitchUserWithSeveralDefinedProvidersButNoFirewallRootProviderConfigured()
412449
{
413450
$container = $this->getRawContainer();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.