#40881 [Security] Allow ips parameter in access_control accept comma-separated string

edefimov · edefimov · commit 93efd0575e16 · 2021-04-25T20:41:17.000+03:00
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -872,7 +872,7 @@ private function createRequestMatcher(ContainerBuilder $container, string $path
             foreach ($ips as $ip) {
                 $container->resolveEnvPlaceholders($ip, null, $usedEnvs);
 
-                if (!$usedEnvs && !$this->isValidIp($ip)) {
+                if (!$usedEnvs && !$this->isValidIps($ip)) {
                     throw new \LogicException(sprintf('The given value "%s" in the "security.access_control" config option is not a valid IP address.', $ip));
                 }
 
@@ -930,6 +930,25 @@ public function getConfiguration(array $config, ContainerBuilder $container)
         return new MainConfiguration($this->factories, $this->userProviderFactories);
     }
 
+    private function isValidIps($ips): bool
+    {
+        $ipsList = array_reduce((array) $ips, static function (array $ips, string $ip) {
+            return array_merge($ips, preg_split('/\s*,\s*/', $ip));
+        }, []);
+
+        if (empty($ipsList)) {
+            return false;
+        }
+
+        foreach ($ipsList as $cidr) {
+            if (!$this->isValidIp($cidr)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     private function isValidIp(string $cidr): bool
     {
         $cidrParts = explode('/', $cidr);
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php
@@ -388,6 +388,33 @@ public function testRememberMeCookieInheritFrameworkSessionCookie($config, $same
         $this->assertEquals($secure, $definition->getArgument(3)['secure']);
     }
 
+    /**
+     * @dataProvider acceptableIpsProvider
+     */
+    public function testAcceptableAccessControlIps($ips)
+    {
+        $container = $this->getRawContainer();
+
+        $container->loadFromExtension('security', [
+            'providers' => [
+                'default' => ['id' => 'foo'],
+            ],
+            'firewalls' => [
+                'some_firewall' => [
+                    'pattern' => '/.*',
+                    'http_basic' => [],
+                ],
+            ],
+            'access_control' => [
+                ['ips' => $ips, 'path' => '/somewhere', 'roles' => 'IS_AUTHENTICATED_FULLY'],
+            ],
+        ]);
+
+        $container->compile();
+
+        $this->assertTrue(true, 'Ip addresses is successfully consumed: '.(\is_string($ips) ? $ips : json_encode($ips)));
+    }
+
     public function sessionConfigurationProvider()
     {
         return [
@@ -408,6 +435,21 @@ public function sessionConfigurationProvider()
         ];
     }
 
+    public function acceptableIpsProvider(): iterable
+    {
+        yield [['127.0.0.1']];
+
+        yield ['127.0.0.1'];
+
+        yield ['127.0.0.1, 127.0.0.2'];
+
+        yield ['127.0.0.1/8, 127.0.0.2/16'];
+
+        yield [['127.0.0.1/8, 127.0.0.2/16']];
+
+        yield [['127.0.0.1/8', '127.0.0.2/16']];
+    }
+
     public function testSwitchUserWithSeveralDefinedProvidersButNoFirewallRootProviderConfigured()
     {
         $container = $this->getRawContainer();
