bug #12296 [SecurityBundle] Authentication entry point is only registered with firewall exception listener, not with authentication listeners (rjkip)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #12296).

Discussion
----------

[SecurityBundle] Authentication entry point is only registered with firewall exception listener, not with authentication listeners

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | when relying on this configuration behaviour
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #12261
| License       | MIT
| Doc PR        | —

See symfony/symfony#12261.

I configured a different firewall entry point for one firewall. However, when authentication had to be performed, it still called BasicAuthenticationEntryPoint::start() instead of my service's start(). My service was instantiated, yet never used.

The issue appears to be that the entry point is registered with the firewall's exception listener, but not with the BasicAuthenticationListener. This means that when the BasicAuthenticationListener determines the user has  provided wrong credentials, BasicAuthenticationEntryPoint is still used. Only in case of an exception would my  entry point service be used.

In my opinion, this is not correct behaviour. Can someone confirm this? Are there currently tests that pertain to the `entry_point` configuration on which I can base a test?

---

Test setup:

```yaml
# security.yml
security:
    firewalls:
        api:
            pattern: ^/api/
            http_basic: ~
            entry_point: my.service
        default:
            anonymous: ~
```

Commits
-------

92c8dfb [SecurityBundle] Authentication entry point is only registered with firewall exception listener, not with authentication listeners

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

@@ -333,8 +333,11 @@ class SecurityExtension extends Extension
             ;
         }
 
+        // Determine default entry point
+        $defaultEntryPoint = isset($firewall['entry_point']) ? $firewall['entry_point'] : null;
+
         // Authentication listeners
-        list($authListeners, $defaultEntryPoint) = $this->createAuthenticationListeners($container, $id, $firewall, $authenticationProviders, $defaultProvider);
+        list($authListeners, $defaultEntryPoint) = $this->createAuthenticationListeners($container, $id, $firewall, $authenticationProviders, $defaultProvider, $defaultEntryPoint);
 
         $listeners = array_merge($listeners, $authListeners);
 
@@ -346,11 +349,6 @@ class SecurityExtension extends Extension
         // Access listener
         $listeners[] = new Reference('security.access_listener');
 
-        // Determine default entry point
-        if (isset($firewall['entry_point'])) {
-            $defaultEntryPoint = $firewall['entry_point'];
-        }
-
         // Exception listener
         $exceptionListener = new Reference($this->createExceptionListener($container, $firewall, $id, $defaultEntryPoint));
 
@@ -370,11 +368,10 @@ class SecurityExtension extends Extension
         return $this->contextListeners[$contextKey] = $listenerId;
     }
 
-    private function createAuthenticationListeners($container, $id, $firewall, &$authenticationProviders, $defaultProvider)
+    private function createAuthenticationListeners($container, $id, $firewall, &$authenticationProviders, $defaultProvider, $defaultEntryPoint)
     {
         $listeners = array();
         $hasListeners = false;
-        $defaultEntryPoint = null;
 
         foreach ($this->listenerPositions as $position) {
             foreach ($this->factories[$position] as $factory) {

src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FirewallEntryPointBundle/DependencyInjection/FirewallEntryPointExtension.php

@@ -0,0 +1,26 @@
+<?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\Functional\Bundle\FirewallEntryPointBundle\DependencyInjection;
+
+use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
+use Symfony\Component\HttpKernel\DependencyInjection\Extension;
+
+class FirewallEntryPointExtension extends Extension
+{
+    public function load(array $config, ContainerBuilder $container)
+    {
+        $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
+        $loader->load('services.xml');
+    }
+}

src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FirewallEntryPointBundle/FirewallEntryPointBundle.php

@@ -0,0 +1,18 @@
+<?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\Functional\Bundle\FirewallEntryPointBundle;
+
+use Symfony\Component\HttpKernel\Bundle\Bundle;
+
+class FirewallEntryPointBundle extends Bundle
+{
+}

src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FirewallEntryPointBundle/Resources/config/services.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0" ?>
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+    <services>
+        <service id="firewall_entry_point.entry_point.stub"
+                 class="Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FirewallEntryPointBundle\Security\EntryPointStub"
+        />
+    </services>
+</container>

src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FirewallEntryPointBundle/Security/EntryPointStub.php

@@ -0,0 +1,27 @@
+<?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\Functional\Bundle\FirewallEntryPointBundle\Security;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
+
+class EntryPointStub implements AuthenticationEntryPointInterface
+{
+    const RESPONSE_TEXT = '2be8e651259189d841a19eecdf37e771e2431741';
+
+    public function start(Request $request, AuthenticationException $authException = null)
+    {
+        return new Response(self::RESPONSE_TEXT);
+    }
+}

src/Symfony/Bundle/SecurityBundle/Tests/Functional/FirewallEntryPointTest.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\Functional;
+
+use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FirewallEntryPointBundle\Security\EntryPointStub;
+
+/**
+ * @group functional
+ */
+class FirewallEntryPointTest extends WebTestCase
+{
+    public function testItUsesTheConfiguredEntryPointWhenUsingUnknownCredentials()
+    {
+        $client = $this->createClient(array('test_case' => 'FirewallEntryPoint'));
+        $client->insulate();
+
+        $client->request('GET', '/secure/resource', array(), array(), array(
+            'PHP_AUTH_USER' => 'unknown',
+            'PHP_AUTH_PW'   => 'credentials',
+        ));
+
+        $this->assertEquals(
+            EntryPointStub::RESPONSE_TEXT,
+            $client->getResponse()->getContent(),
+            "Custom entry point wasn't started"
+        );
+    }
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->deleteTmpDir('FirewallEntryPoint');
+    }
+
+    protected function tearDown()
+    {
+        parent::tearDown();
+
+        $this->deleteTmpDir('FirewallEntryPoint');
+    }
+}

src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/FirewallEntryPoint/bundles.php

@@ -0,0 +1,7 @@
+<?php
+
+return array(
+    new Symfony\Bundle\FrameworkBundle\FrameworkBundle(),
+    new Symfony\Bundle\SecurityBundle\SecurityBundle(),
+    new Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FirewallEntryPointBundle\FirewallEntryPointBundle(),
+);

src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/FirewallEntryPoint/config.yml

@@ -0,0 +1,33 @@
+framework:
+    secret:        test
+    csrf_protection:
+        enabled: true
+    router:        { resource: "%kernel.root_dir%/%kernel.test_case%/routing.yml" }
+    validation:    { enabled: true, enable_annotations: true }
+    form: ~
+    test: ~
+    default_locale: en
+    session:
+        storage_id:     session.storage.mock_file
+    profiler: { only_exceptions: false }
+
+services:
+    logger: { class: Symfony\Component\HttpKernel\Log\NullLogger }
+
+security:
+    firewalls:
+        secure:
+            pattern: ^/secure/
+            http_basic: { realm: "Secure Gateway API" }
+            entry_point: firewall_entry_point.entry_point.stub
+        default:
+            anonymous: ~
+    access_control:
+        - { path: ^/secure/, roles: ROLE_SECURE }
+    providers:
+        in_memory:
+            memory:
+                users:
+                    john: { password: doe, roles: [ROLE_SECURE] }
+    encoders:
+        Symfony\Component\Security\Core\User\User: plaintext

src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/FirewallEntryPoint/routing.yml

@@ -0,0 +1,2 @@
+secure_resource:
+    path: /secure/resource