bug symfony#57493 [SecurityBundle] Make security schema deterministic (MatTheCat)

fabpot · fabpot · commit ca84bcccf180 · 2024-08-19T11:13:30.000+02:00
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [SecurityBundle] Make security schema deterministic | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#57463 | License | MIT Currently you cannot put a namespace on custom providers/authenticators because the `XmlFileLoader` will ignore every element whose namespace is not its parent’s. This means the only way to configure e.g. [SymfonyConnect](https://github.com/symfonycorp/connect)’s provider is ```xml <provider name="connect_memory"> <connect_memory> <user username="whatever">ROLE_ADMIN</user> </connect_memory> </provider> ``` This requires providers and authenticators `xsd:any`’s `namespace` to be `##any`, which is no longer possible with libxml ≥ 2.12 (already available on Alpine 3.20 and Arch Linux e.g.) because it will report their content model as non-determinist. > Unable to parse file "/srv/backend/config/packages/security.xml": [ERROR 3070] complex type 'provider': The content model is not determinist. (in file:////srv/backend/vendor/symfony/security-bundle/DependencyInjection/../Resources/config/schema//security-1.0.xsd - line 88, column 0) [ERROR 3070] complex type 'firewall': The content model is not determinist. (in file:////srv/backend/vendor/symfony/security-bundle/DependencyInjection/../Resources/config/schema//security-1.0.xsd - line 134, column 0) in /srv/backend/config/packages/security.xml (which is being imported from "/srv/backend/src/Kernel.php"). As a result, any XML security config will fail to be loaded once libxml is updated. Fixing this issue requires to change `xsd:any`s’ `namespace` to `##other` so that content models become deterministic. A side-effect is that any custom providers/authenticators XML configuration will become invalid. > Unable to parse file "/srv/backend/config/packages/security.xml": [ERROR 1871] Element '{http://symfony.com/schema/dic/security}connect_memory': This element is not expected. Expected is one of ( {http://symfony.com/schema/dic/security}chain, {http://symfony.com/schema/dic/security}memory, {http://symfony.com/schema/dic/security}ldap, ##other{http://symfony.com/schema/dic/security}* ). (in /srv/backend/public/ - line 13, column 0) in /srv/backend/config/packages/security.xml (which is being imported from "/srv/backend/src/Kernel.php"). To avoid a BC break this PR allows such errors to occur. A deprecation will have to be triggered on 7.2 for users to namespace their custom providers/authenticators elements, e.g. ```xml <provider name="symfony_connect"> <connect_memory xmlns="whatever"> <user username="whatever">ROLE_ADMIN</custom:user> </connect_memory> </provider> ``` Because custom providers/authenticators’ `processContents` is `lax` there won’t be any validation if the namespace has no schema. If it has, it will be better to provide it: ```diff <srv:container xmlns="http://symfony.com/schema/dic/security" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:srv="http://symfony.com/schema/dic/services" + xmlns:custom="whatever" xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd http://symfony.com/schema/dic/security https://symfony.com/schema/dic/security/security-1.0.xsd + whatever the/schema.xsd"> <config> <provider name="symfony_connect"> - <connect_memory xmlns="whatever"> - <user username="whatever">ROLE_ADMIN</custom:user> - </connect_memory> + <custom:connect_memory> + <custom:user username="whatever">ROLE_ADMIN</custom:user> + </custom:connect_memory> </provider> </srv:container> ``` Commits ------- ce030e8 [SecurityBundle] Make security schema deterministic
diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd b/src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd
@@ -117,7 +117,7 @@
             <xsd:element name="memory" type="memory" />
             <xsd:element name="ldap" type="ldap" />
             <!-- allow factories to use dynamic elements -->
-            <xsd:any processContents="lax" />
+            <xsd:any processContents="lax" namespace="##other" />
         </xsd:choice>
         <xsd:attribute name="name" type="xsd:string" use="required" />
         <xsd:attribute name="id" type="xsd:string" />
@@ -176,7 +176,7 @@
             <xsd:element name="x509" type="x509" minOccurs="0" maxOccurs="1" />
             <xsd:element name="required-badge" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
             <!-- allow factories to use dynamic elements -->
-            <xsd:any processContents="lax" minOccurs="0" maxOccurs="unbounded" />
+            <xsd:any processContents="lax" minOccurs="0" maxOccurs="unbounded" namespace="##other" />
         </xsd:choice>
         <xsd:attribute name="name" type="xsd:string" use="required" />
         <xsd:attribute name="pattern" type="xsd:string" />
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/Authenticator/CustomAuthenticator.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/Authenticator/CustomAuthenticator.php
@@ -0,0 +1,29 @@
+<?php
+
+namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Fixtures\Authenticator;
+
+use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AuthenticatorFactoryInterface;
+use Symfony\Component\Config\Definition\Builder\NodeDefinition;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+class CustomAuthenticator implements AuthenticatorFactoryInterface
+{
+    public function createAuthenticator(ContainerBuilder $container, string $firewallName, array $config, string $userProviderId): string
+    {
+        return 'security.authenticator.custom.'.$firewallName;
+    }
+
+    public function getKey(): string
+    {
+        return 'custom';
+    }
+
+    public function addConfiguration(NodeDefinition $builder)
+    {
+    }
+
+    public function getPriority(): int
+    {
+        return 0;
+    }
+}
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/UserProvider/CustomProvider.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/UserProvider/CustomProvider.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Fixtures\UserProvider;
+
+use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\UserProvider\UserProviderFactoryInterface;
+use Symfony\Component\Config\Definition\Builder\NodeDefinition;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+class CustomProvider implements UserProviderFactoryInterface
+{
+    public function create(ContainerBuilder $container, string $id, array $config)
+    {
+    }
+
+    public function getKey(): string
+    {
+        return 'custom';
+    }
+
+    public function addConfiguration(NodeDefinition $builder)
+    {
+        $builder
+            ->children()
+                ->scalarNode('foo')->defaultValue('bar')->end()
+            ->end()
+        ;
+    }
+}
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/custom_authenticator_under_own_namespace.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/custom_authenticator_under_own_namespace.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xmlns:sec="http://symfony.com/schema/dic/security"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services
+        https://symfony.com/schema/dic/services/services-1.0.xsd
+        http://symfony.com/schema/dic/security
+        https://symfony.com/schema/dic/security/security-1.0.xsd">
+
+    <sec:config enable-authenticator-manager="true">
+        <sec:firewall name="main">
+            <custom xmlns="http://example.com/schema" />
+        </sec:firewall>
+    </sec:config>
+
+</container>
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/custom_authenticator_under_security_namespace.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/custom_authenticator_under_security_namespace.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xmlns:sec="http://symfony.com/schema/dic/security"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services
+        https://symfony.com/schema/dic/services/services-1.0.xsd
+        http://symfony.com/schema/dic/security
+        https://symfony.com/schema/dic/security/security-1.0.xsd">
+
+    <sec:config enable-authenticator-manager="true">
+        <sec:firewall name="main">
+            <sec:custom />
+        </sec:firewall>
+    </sec:config>
+
+</container>
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/custom_provider_under_own_namespace.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/custom_provider_under_own_namespace.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xmlns:sec="http://symfony.com/schema/dic/security"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services
+        https://symfony.com/schema/dic/services/services-1.0.xsd
+        http://symfony.com/schema/dic/security
+        https://symfony.com/schema/dic/security/security-1.0.xsd">
+
+    <sec:config enable-authenticator-manager="true">
+        <sec:provider name="foo">
+            <custom xmlns="http://example.com/schema" />
+        </sec:provider>
+
+        <sec:firewall name="main" provider="foo" />
+    </sec:config>
+
+</container>
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/custom_provider_under_security_namespace.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/custom_provider_under_security_namespace.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xmlns:sec="http://symfony.com/schema/dic/security"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services
+        https://symfony.com/schema/dic/services/services-1.0.xsd
+        http://symfony.com/schema/dic/security
+        https://symfony.com/schema/dic/security/security-1.0.xsd">
+
+    <sec:config enable-authenticator-manager="true">
+        <sec:provider name="foo">
+            <sec:custom />
+        </sec:provider>
+
+        <sec:firewall name="main" provider="foo" />
+    </sec:config>
+
+</container>
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/XmlCustomAuthenticatorTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/XmlCustomAuthenticatorTest.php
@@ -0,0 +1,51 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
+use Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Fixtures\Authenticator\CustomAuthenticator;
+use Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Fixtures\UserProvider\CustomProvider;
+use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
+
+class XmlCustomAuthenticatorTest extends TestCase
+{
+    /**
+     * @dataProvider provideXmlConfigurationFile
+     */
+    public function testCustomProviderElement(string $configurationFile)
+    {
+        $container = new ContainerBuilder();
+        $container->setParameter('kernel.debug', false);
+        $container->register('cache.system', \stdClass::class);
+
+        $security = new SecurityExtension();
+        $security->addAuthenticatorFactory(new CustomAuthenticator());
+        $container->registerExtension($security);
+
+        (new XmlFileLoader($container, new FileLocator(__DIR__.'/Fixtures/xml')))->load($configurationFile);
+
+        $container->getCompilerPassConfig()->setRemovingPasses([]);
+        $container->getCompilerPassConfig()->setAfterRemovingPasses([]);
+        $container->compile();
+
+        $this->addToAssertionCount(1);
+    }
+
+    public static function provideXmlConfigurationFile(): iterable
+    {
+        yield 'Custom authenticator element under SecurityBundle’s namespace' => ['custom_authenticator_under_security_namespace.xml'];
+        yield 'Custom authenticator element under its own namespace' => ['custom_authenticator_under_own_namespace.xml'];
+    }
+}
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/XmlCustomProviderTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/XmlCustomProviderTest.php
@@ -0,0 +1,50 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
+use Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Fixtures\UserProvider\CustomProvider;
+use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
+
+class XmlCustomProviderTest extends TestCase
+{
+    /**
+     * @dataProvider provideXmlConfigurationFile
+     */
+    public function testCustomProviderElement(string $configurationFile)
+    {
+        $container = new ContainerBuilder();
+        $container->setParameter('kernel.debug', false);
+        $container->register('cache.system', \stdClass::class);
+
+        $security = new SecurityExtension();
+        $security->addUserProviderFactory(new CustomProvider());
+        $container->registerExtension($security);
+
+        (new XmlFileLoader($container, new FileLocator(__DIR__.'/Fixtures/xml')))->load($configurationFile);
+
+        $container->getCompilerPassConfig()->setRemovingPasses([]);
+        $container->getCompilerPassConfig()->setAfterRemovingPasses([]);
+        $container->compile();
+
+        $this->addToAssertionCount(1);
+    }
+
+    public static function provideXmlConfigurationFile(): iterable
+    {
+        yield 'Custom provider element under SecurityBundle’s namespace' => ['custom_provider_under_security_namespace.xml'];
+        yield 'Custom provider element under its own namespace' => ['custom_provider_under_own_namespace.xml'];
+    }
+}
diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json
@@ -19,7 +19,7 @@
         "php": ">=7.2.5",
         "ext-xml": "*",
         "symfony/config": "^4.4|^5.0|^6.0",
-        "symfony/dependency-injection": "^5.3|^6.0",
+        "symfony/dependency-injection": "^5.4.43|^6.4.11",
         "symfony/deprecation-contracts": "^2.1|^3",
         "symfony/event-dispatcher": "^5.1|^6.0",
         "symfony/http-kernel": "^5.3|^6.0",
diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
@@ -404,7 +404,33 @@ private function parseFileToDOM(string $file): \DOMDocument
         try {
             $dom = XmlUtils::loadFile($file, [$this, 'validateSchema']);
         } catch (\InvalidArgumentException $e) {
-            throw new InvalidArgumentException(sprintf('Unable to parse file "%s": ', $file).$e->getMessage(), $e->getCode(), $e);
+            $invalidSecurityElements = [];
+            $errors = explode("\n", $e->getMessage());
+            foreach ($errors as $i => $error) {
+                if (preg_match("#^\[ERROR 1871] Element '\{http://symfony\.com/schema/dic/security}([^']+)'#", $error, $matches)) {
+                    $invalidSecurityElements[$i] = $matches[1];
+                }
+            }
+            if ($invalidSecurityElements) {
+                $dom = XmlUtils::loadFile($file);
+
+                foreach ($invalidSecurityElements as $errorIndex => $tagName) {
+                    foreach ($dom->getElementsByTagNameNS('http://symfony.com/schema/dic/security', $tagName) as $element) {
+                        if (!$parent = $element->parentNode) {
+                            continue;
+                        }
+                        if ('http://symfony.com/schema/dic/security' !== $parent->namespaceURI) {
+                            continue;
+                        }
+                        if ('provider' === $parent->localName || 'firewall' === $parent->localName) {
+                            unset($errors[$errorIndex]);
+                        }
+                    }
+                }
+            }
+            if ($errors) {
+                throw new InvalidArgumentException(sprintf('Unable to parse file "%s": ', $file).implode("/n", $errors), $e->getCode(), $e);
+            }
         }
 
         $this->validateExtensions($dom, $file);
@@ -777,6 +803,6 @@ private function loadFromExtensions(\DOMDocument $xml)
      */
     public static function convertDomElementToArray(\DOMElement $element)
     {
-        return XmlUtils::convertDomElementToArray($element);
+        return XmlUtils::convertDomElementToArray($element, false);
     }
 }
