bug #25045 [SecurityBundle] Don't trigger auto-picking notice if provider is set per listener (chalasr)

nicolas-grekas · nicolas-grekas · commit e3efa2fec797 · 2017-11-20T19:04:24.000+01:00
This PR was merged into the 3.4 branch. Discussion ---------- [SecurityBundle] Don't trigger auto-picking notice if provider is set per listener | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #24980 | License | MIT | Doc PR | n/a Commits ------- 19e891a [SecurityBundle] Don't trigger auto-picking notice if provider is set per listener
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -346,17 +346,14 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
         $config->replaceArgument(4, $firewall['stateless']);
 
         // Provider id (take the first registered provider if none defined)
+        $defaultProvider = null;
         if (isset($firewall['provider'])) {
             if (!isset($providerIds[$normalizedName = str_replace('-', '_', $firewall['provider'])])) {
                 throw new InvalidConfigurationException(sprintf('Invalid firewall "%s": user provider "%s" not found.', $id, $firewall['provider']));
             }
             $defaultProvider = $providerIds[$normalizedName];
-        } else {
+        } elseif (1 === count($providerIds)) {
             $defaultProvider = reset($providerIds);
-
-            if (count($providerIds) > 1) {
-                @trigger_error(sprintf('Firewall "%s" has no "provider" set but multiple providers exist. Using the first configured provider (%s) is deprecated since 3.4 and will throw an exception in 4.0, set the "provider" key on the firewall instead.', $id, key($providerIds)), E_USER_DEPRECATED);
-            }
         }
 
         $config->replaceArgument(5, $defaultProvider);
@@ -500,7 +497,7 @@ private function createContextListener($container, $contextKey)
         return $this->contextListeners[$contextKey] = $listenerId;
     }
 
-    private function createAuthenticationListeners($container, $id, $firewall, &$authenticationProviders, $defaultProvider, array $providerIds, $defaultEntryPoint)
+    private function createAuthenticationListeners($container, $id, $firewall, &$authenticationProviders, $defaultProvider = null, array $providerIds, $defaultEntryPoint)
     {
         $listeners = array();
         $hasListeners = false;
@@ -516,7 +513,7 @@ private function createAuthenticationListeners($container, $id, $firewall, &$aut
                         }
                         $userProvider = $providerIds[$normalizedName];
                     } else {
-                        $userProvider = $defaultProvider;
+                        $userProvider = $defaultProvider ?: $this->getFirstProvider($id, $key, $providerIds);
                     }
 
                     list($provider, $listenerId, $defaultEntryPoint) = $factory->create($container, $id, $firewall[$key], $userProvider, $defaultEntryPoint);
@@ -699,7 +696,7 @@ private function createExceptionListener($container, $config, $id, $defaultEntry
 
     private function createSwitchUserListener($container, $id, $config, $defaultProvider, $stateless)
     {
-        $userProvider = isset($config['provider']) ? $this->getUserProviderId($config['provider']) : $defaultProvider;
+        $userProvider = isset($config['provider']) ? $this->getUserProviderId($config['provider']) : ($defaultProvider ?: $this->getFirstProvider($id, 'switch_user', $providerIds));
 
         // in 4.0, ignore the `switch_user.stateless` key if $stateless is `true`
         if ($stateless && false === $config['stateless']) {
@@ -803,4 +800,14 @@ private function getExpressionLanguage()
 
         return $this->expressionLanguage;
     }
+
+    /**
+     * @deprecated since version 3.4, to be removed in 4.0
+     */
+    private function getFirstProvider($firewallName, $listenerName, array $providerIds)
+    {
+        @trigger_error(sprintf('Listener "%s" on firewall "%s" has no "provider" set but multiple providers exist. Using the first configured provider (%s) is deprecated since 3.4 and will throw an exception in 4.0, set the "provider" key on the firewall instead.', $listenerName, $firewallName, $first = array_keys($providerIds)[0]), E_USER_DEPRECATED);
+
+        return $providerIds[$first];
+    }
 }
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php
@@ -176,7 +176,7 @@ public function testSwitchUserNotStatelessOnStatelessFirewall()
 
     /**
      * @group legacy
-     * @expectedDeprecation Firewall "default" has no "provider" set but multiple providers exist. Using the first configured provider (first) is deprecated since 3.4 and will throw an exception in 4.0, set the "provider" key on the firewall instead.
+     * @expectedDeprecation Listener "http_basic" on firewall "default" has no "provider" set but multiple providers exist. Using the first configured provider (first) is deprecated since 3.4 and will throw an exception in 4.0, set the "provider" key on the firewall instead.
      */
     public function testDeprecationForAmbiguousProvider()
     {
@@ -199,6 +199,27 @@ public function testDeprecationForAmbiguousProvider()
         $container->compile();
     }
 
+    public function testPerListenerProvider()
+    {
+        $container = $this->getRawContainer();
+        $container->loadFromExtension('security', array(
+            'providers' => array(
+                'first' => array('id' => 'foo'),
+                'second' => array('id' => 'bar'),
+            ),
+
+            'firewalls' => array(
+                'default' => array(
+                    'http_basic' => array('provider' => 'second'),
+                    'logout_on_user_change' => true,
+                ),
+            ),
+        ));
+
+        $container->compile();
+        $this->addToAssertionCount(1);
+    }
+
     protected function getRawContainer()
     {
         $container = new ContainerBuilder();
