feature #24378 [SecurityBundle] Deprecate auto picking the first prov…

…ider (ogizanagi)

This PR was merged into the 3.4 branch.

Discussion
----------

[SecurityBundle] Deprecate auto picking the first provider

when no provider is explicitly configured on a firewall

| Q             | A
| ------------- | ---
| Branch?       | 3.4 <!-- see comment below -->
| Bug fix?      | no
| New feature?  | no <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | yes <!-- don't forget updating UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | https://symfony-devs.slack.com/archives/C3A2XAQ20/p1506626210000345 <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

From @Pierstoval on Slack:

> Hey, guys, I learnt a few days ago that if you don't specify a user provider in a firewall configuration, the security will use the first one in the list. Don't anyone think specifying the user provider should be mandatory ? Or at least mandatory if we have more than one provider registered?

- [x] UPGRADE files
- [x] CHANGELOG
- [x] Fix other tests
- [x] Removal PR #24380

Commits
-------

2d1e334 [SecurityBundle] Deprecate auto picking the first provider

UPGRADE-3.4.md

@@ -320,6 +320,10 @@ SecurityBundle
  * Deprecated setting the `switch_user.stateless` option to false when the firewall is `stateless`.
    Setting it to false will have no effect in 4.0.
 
+ * Not configuring explicitly the provider on a firewall is ambiguous when there is more than one registered provider. 
+   Using the first configured provider is deprecated since 3.4 and will throw an exception on 4.0.
+   Explicitly configure the provider to use on your firewalls.
+
 Translation
 -----------
 

UPGRADE-4.0.md

@@ -696,6 +696,10 @@ SecurityBundle
 
  * The `switch_user.stateless` option is now always true if the firewall is stateless.
 
+ * Not configuring explicitly the provider on a firewall is ambiguous when there is more than one registered provider. 
+   The first configured provider is not used anymore and an exception is thrown instead.
+   Explicitly configure the provider to use on your firewalls.
+
 Serializer
 ----------
 

src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

@@ -18,6 +18,7 @@ CHANGELOG
  * deprecated command `init:acl` along with `InitAclCommand` class
  * Added support for the new Argon2i password encoder
  * added `stateless` option to the `switch_user` listener
+ * deprecated auto picking the first registered provider when no configured provider on a firewall and ambiguous
 
 3.3.0
 -----

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

@@ -359,6 +359,10 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
             $defaultProvider = $providerIds[$normalizedName];
         } else {
             $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);

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php

@@ -60,8 +60,9 @@
     ),
 
     'firewalls' => array(
-        'simple' => array('pattern' => '/login', 'security' => false),
+        'simple' => array('provider' => 'default', 'pattern' => '/login', 'security' => false),
         'secure' => array('stateless' => true,
+            'provider' => 'default',
             'http_basic' => true,
             'form_login' => true,
             'anonymous' => true,
@@ -74,6 +75,7 @@
             'logout_on_user_change' => true,
         ),
         'host' => array(
+            'provider' => 'default',
             'pattern' => '/test',
             'host' => 'foo\\.example\\.org',
             'methods' => array('GET', 'POST'),
@@ -82,6 +84,7 @@
             'logout_on_user_change' => true,
         ),
         'with_user_checker' => array(
+            'provider' => 'default',
             'user_checker' => 'app.user_checker',
             'anonymous' => true,
             'http_basic' => true,

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1_with_acl.php

@@ -61,8 +61,9 @@
     ),
 
     'firewalls' => array(
-        'simple' => array('pattern' => '/login', 'security' => false),
+        'simple' => array('provider' => 'default', 'pattern' => '/login', 'security' => false),
         'secure' => array('stateless' => true,
+            'provider' => 'default',
             'http_basic' => true,
             'http_digest' => array('secret' => 'TheSecret'),
             'form_login' => true,
@@ -75,13 +76,15 @@
             'user_checker' => null,
         ),
         'host' => array(
+            'provider' => 'default',
             'pattern' => '/test',
             'host' => 'foo\\.example\\.org',
             'methods' => array('GET', 'POST'),
             'anonymous' => true,
             'http_basic' => true,
         ),
         'with_user_checker' => array(
+            'provider' => 'default',
             'user_checker' => 'app.user_checker',
             'anonymous' => true,
             'http_basic' => true,

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1_with_digest.php

@@ -61,8 +61,9 @@
     ),
 
     'firewalls' => array(
-        'simple' => array('pattern' => '/login', 'security' => false),
+        'simple' => array('provider' => 'default', 'pattern' => '/login', 'security' => false),
         'secure' => array('stateless' => true,
+            'provider' => 'default',
             'http_basic' => true,
             'http_digest' => array('secret' => 'TheSecret'),
             'form_login' => true,
@@ -76,6 +77,7 @@
             'logout_on_user_change' => true,
         ),
         'host' => array(
+            'provider' => 'default',
             'pattern' => '/test',
             'host' => 'foo\\.example\\.org',
             'methods' => array('GET', 'POST'),
@@ -84,6 +86,7 @@
             'logout_on_user_change' => true,
         ),
         'with_user_checker' => array(
+            'provider' => 'default',
             'user_checker' => 'app.user_checker',
             'anonymous' => true,
             'http_basic' => true,

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml

@@ -43,9 +43,9 @@
             <chain providers="service, basic" />
         </provider>
 
-        <firewall name="simple" pattern="/login" security="false" />
+        <firewall name="simple" pattern="/login" security="false" provider="default" />
 
-        <firewall name="secure" stateless="true">
+        <firewall name="secure" stateless="true" provider="default">
             <http-basic />
             <form-login />
             <anonymous />
@@ -57,12 +57,12 @@
             <remember-me secret="TheSecret"/>
         </firewall>
 
-        <firewall name="host" pattern="/test" host="foo\.example\.org" methods="GET,POST" logout-on-user-change="true">
+        <firewall name="host" pattern="/test" host="foo\.example\.org" methods="GET,POST" logout-on-user-change="true" provider="default">
             <anonymous />
             <http-basic />
         </firewall>
 
-        <firewall name="with_user_checker" logout-on-user-change="true">
+        <firewall name="with_user_checker" logout-on-user-change="true" provider="default">
             <anonymous />
             <http-basic />
             <user-checker>app.user_checker</user-checker>

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1_with_acl.xml

@@ -44,9 +44,9 @@
             <chain providers="service, basic" />
         </provider>
 
-        <firewall name="simple" pattern="/login" security="false" />
+        <firewall name="simple" pattern="/login" security="false" provider="default" />
 
-        <firewall name="secure" stateless="true">
+        <firewall name="secure" stateless="true" provider="default">
             <http-basic />
             <http-digest secret="TheSecret" />
             <form-login />
@@ -59,12 +59,12 @@
             <remember-me secret="TheSecret"/>
         </firewall>
 
-        <firewall name="host" pattern="/test" host="foo\.example\.org" methods="GET,POST">
+        <firewall name="host" pattern="/test" host="foo\.example\.org" methods="GET,POST" provider="default">
             <anonymous />
             <http-basic />
         </firewall>
 
-        <firewall name="with_user_checker">
+        <firewall name="with_user_checker" provider="default">
             <anonymous />
             <http-basic />
             <user-checker>app.user_checker</user-checker>

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1_with_digest.xml

@@ -45,9 +45,9 @@
             <chain providers="service, basic" />
         </provider>
 
-        <firewall name="simple" pattern="/login" security="false" />
+        <firewall name="simple" pattern="/login" security="false" provider="default" />
 
-        <firewall name="secure" stateless="true">
+        <firewall name="secure" stateless="true" provider="default">
             <http-basic />
             <http-digest secret="TheSecret" />
             <form-login />
@@ -60,12 +60,12 @@
             <remember-me secret="TheSecret"/>
         </firewall>
 
-        <firewall name="host" pattern="/test" host="foo\.example\.org" methods="GET,POST" logout-on-user-change="true">
+        <firewall name="host" pattern="/test" host="foo\.example\.org" methods="GET,POST" logout-on-user-change="true" provider="default">
             <anonymous />
             <http-basic />
         </firewall>
 
-        <firewall name="with_user_checker" logout-on-user-change="true">
+        <firewall name="with_user_checker" logout-on-user-change="true" provider="default">
             <anonymous />
             <http-basic />
             <user-checker>app.user_checker</user-checker>

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml

@@ -43,6 +43,7 @@ security:
     firewalls:
         simple: { pattern: /login, security: false }
         secure:
+            provider: default
             stateless: true
             http_basic: true
             form_login: true
@@ -57,6 +58,7 @@ security:
             user_checker: ~
 
         host:
+            provider: default
             pattern: /test
             host: foo\.example\.org
             methods: [GET,POST]
@@ -65,6 +67,7 @@ security:
             logout_on_user_change: true
 
         with_user_checker:
+            provider: default
             anonymous: ~
             http_basic: ~
             user_checker: app.user_checker

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1_with_acl.yml

@@ -44,6 +44,7 @@ security:
     firewalls:
         simple: { pattern: /login, security: false }
         secure:
+            provider: default
             stateless: true
             http_basic: true
             http_digest:
@@ -60,13 +61,15 @@ security:
             user_checker: ~
 
         host:
+            provider: default
             pattern: /test
             host: foo\.example\.org
             methods: [GET,POST]
             anonymous: true
             http_basic: true
 
         with_user_checker:
+            provider: default
             anonymous: ~
             http_basic: ~
             user_checker: app.user_checker

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1_with_digest.yml

@@ -44,6 +44,7 @@ security:
     firewalls:
         simple: { pattern: /login, security: false }
         secure:
+            provider: default
             stateless: true
             http_basic: true
             http_digest:
@@ -60,6 +61,7 @@ security:
             user_checker: ~
 
         host:
+            provider: default
             pattern: /test
             host: foo\.example\.org
             methods: [GET,POST]
@@ -68,6 +70,7 @@ security:
             logout_on_user_change: true
 
         with_user_checker:
+            provider: default
             anonymous: ~
             http_basic: ~
             user_checker: app.user_checker

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

@@ -174,6 +174,31 @@ public function testSwitchUserNotStatelessOnStatelessFirewall()
         $container->compile();
     }
 
+    /**
+     * @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.
+     */
+    public function testDeprecationForAmbiguousProvider()
+    {
+        $container = $this->getRawContainer();
+
+        $container->loadFromExtension('security', array(
+            'providers' => array(
+                'first' => array('id' => 'foo'),
+                'second' => array('id' => 'bar'),
+            ),
+
+            'firewalls' => array(
+                'default' => array(
+                    'http_basic' => null,
+                    'logout_on_user_change' => true,
+                ),
+            ),
+        ));
+
+        $container->compile();
+    }
+
     protected function getRawContainer()
     {
         $container = new ContainerBuilder();