[Security] Deprecate legacy signatures

Author: wouterj

CHANGELOG.md

@@ -4,6 +4,7 @@ CHANGELOG
 5.4
 ---
 
+ * Deprecate not setting `$authenticatorManagerEnabled` to `true` in `SecurityDataCollector` and `DebugFirewallCommand`
  * Deprecate `SecurityFactoryInterface` and `SecurityExtension::addSecurityListenerFactory()` in favor of
    `AuthenticatorFactoryInterface` and `SecurityExtension::addAuthenticatorFactory()`
  * Add `AuthenticatorFactoryInterface::getPriority()` which replaces `SecurityFactoryInterface::getPosition()`

Command/DebugFirewallCommand.php

@@ -43,6 +43,10 @@ final class DebugFirewallCommand extends Command
      */
     public function __construct(array $firewallNames, ContainerInterface $contexts, ContainerInterface $eventDispatchers, array $authenticators, bool $authenticatorManagerEnabled)
     {
+        if (!$authenticatorManagerEnabled) {
+            trigger_deprecation('symfony/security-bundle', '5.4', 'Setting the $authenticatorManagerEnabled argument of "%s" to "false" is deprecated, use the new authenticator system instead.', __METHOD__);
+        }
+
         $this->firewallNames = $firewallNames;
         $this->contexts = $contexts;
         $this->eventDispatchers = $eventDispatchers;

DataCollector/SecurityDataCollector.php

@@ -48,6 +48,10 @@ class SecurityDataCollector extends DataCollector implements LateDataCollectorIn
 
     public function __construct(TokenStorageInterface $tokenStorage = null, RoleHierarchyInterface $roleHierarchy = null, LogoutUrlGenerator $logoutUrlGenerator = null, AccessDecisionManagerInterface $accessDecisionManager = null, FirewallMapInterface $firewallMap = null, TraceableFirewallListener $firewall = null, bool $authenticatorManagerEnabled = false)
     {
+        if (!$authenticatorManagerEnabled) {
+            trigger_deprecation('symfony/security-bundle', '5.4', 'Setting the $authenticatorManagerEnabled argument of "%s" to "false" is deprecated, use the new authenticator system instead.', __METHOD__);
+        }
+
         $this->tokenStorage = $tokenStorage;
         $this->roleHierarchy = $roleHierarchy;
         $this->logoutUrlGenerator = $logoutUrlGenerator;

DependencyInjection/SecurityExtension.php

@@ -103,12 +103,19 @@ public function load(array $configs, ContainerBuilder $container)
             // The authenticator system no longer has anonymous tokens. This makes sure AccessListener
             // and AuthorizationChecker do not throw AuthenticationCredentialsNotFoundException when no
             // token is available in the token storage.
-            $container->getDefinition('security.access_listener')->setArgument(4, false);
+            $container->getDefinition('security.access_listener')->setArgument(3, false);
+            $container->getDefinition('security.authorization_checker')->setArgument(3, false);
             $container->getDefinition('security.authorization_checker')->setArgument(4, false);
-            $container->getDefinition('security.authorization_checker')->setArgument(5, false);
         } else {
             trigger_deprecation('symfony/security-bundle', '5.3', 'Not setting the "security.enable_authenticator_manager" config option to true is deprecated.');
 
+            if ($config['always_authenticate_before_granting']) {
+                $authorizationChecker = $container->getDefinition('security.authorization_checker');
+                $authorizationCheckerArgs = $authorizationChecker->getArguments();
+                array_splice($authorizationCheckerArgs, 1, 0, [new Reference('security.authentication_manager')]);
+                $authorizationChecker->setArguments($authorizationCheckerArgs);
+            }
+
             $loader->load('security_legacy.php');
         }
 

Resources/config/security.php

@@ -64,7 +64,6 @@
             ->public()
             ->args([
                 service('security.token_storage'),
-                service('security.authentication.manager'),
                 service('security.access.decision_manager'),
                 param('security.access.always_authenticate_before_granting'),
             ])

Tests/DataCollector/SecurityDataCollectorTest.php

@@ -37,7 +37,7 @@ class SecurityDataCollectorTest extends TestCase
 {
     public function testCollectWhenSecurityIsDisabled()
     {
-        $collector = new SecurityDataCollector();
+        $collector = new SecurityDataCollector(null, null, null, null, null, null, true);
         $collector->collect(new Request(), new Response());
 
         $this->assertSame('security', $collector->getName());
@@ -57,7 +57,7 @@ public function testCollectWhenSecurityIsDisabled()
     public function testCollectWhenAuthenticationTokenIsNull()
     {
         $tokenStorage = new TokenStorage();
-        $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
+        $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy(), null, null, null, null, true);
         $collector->collect(new Request(), new Response());
 
         $this->assertTrue($collector->isEnabled());
@@ -71,7 +71,7 @@ public function testCollectWhenAuthenticationTokenIsNull()
         $this->assertCount(0, $collector->getInheritedRoles());
         $this->assertEmpty($collector->getUser());
         $this->assertNull($collector->getFirewall());
-        $this->assertFalse($collector->isAuthenticatorManagerEnabled());
+        $this->assertTrue($collector->isAuthenticatorManagerEnabled());
     }
 
     /** @dataProvider provideRoles */
@@ -80,7 +80,7 @@ public function testCollectAuthenticationTokenAndRoles(array $roles, array $norm
         $tokenStorage = new TokenStorage();
         $tokenStorage->setToken(new UsernamePasswordToken('hhamon', 'P4$$w0rD', 'provider', $roles));
 
-        $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
+        $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy(), null, null, null, null, true);
         $collector->collect(new Request(), new Response());
         $collector->lateCollect();
 
@@ -94,7 +94,7 @@ public function testCollectAuthenticationTokenAndRoles(array $roles, array $norm
         $this->assertSame($normalizedRoles, $collector->getRoles()->getValue(true));
         $this->assertSame($inheritedRoles, $collector->getInheritedRoles()->getValue(true));
         $this->assertSame('hhamon', $collector->getUser());
-        $this->assertFalse($collector->isAuthenticatorManagerEnabled());
+        $this->assertTrue($collector->isAuthenticatorManagerEnabled());
     }
 
     public function testCollectSwitchUserToken()
@@ -104,7 +104,7 @@ public function testCollectSwitchUserToken()
         $tokenStorage = new TokenStorage();
         $tokenStorage->setToken(new SwitchUserToken('hhamon', 'P4$$w0rD', 'provider', ['ROLE_USER', 'ROLE_PREVIOUS_ADMIN'], $adminToken));
 
-        $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
+        $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy(), null, null, null, null, true);
         $collector->collect(new Request(), new Response());
         $collector->lateCollect();
 
@@ -160,7 +160,7 @@ public function testGetFirewallReturnsNull()
         $response = new Response();
 
         // Don't inject any firewall map
-        $collector = new SecurityDataCollector();
+        $collector = new SecurityDataCollector(null, null, null, null, null, null, true);
         $collector->collect($request, $response);
         $this->assertNull($collector->getFirewall());
 
@@ -170,7 +170,7 @@ public function testGetFirewallReturnsNull()
             ->disableOriginalConstructor()
             ->getMock();
 
-        $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()));
+        $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()), true);
         $collector->collect($request, $response);
         $this->assertNull($collector->getFirewall());
 
@@ -180,7 +180,7 @@ public function testGetFirewallReturnsNull()
             ->disableOriginalConstructor()
             ->getMock();
 
-        $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()));
+        $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()), true);
         $collector->collect($request, $response);
         $this->assertNull($collector->getFirewall());
     }
@@ -214,7 +214,7 @@ public function testGetListeners()
         $firewall = new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator());
         $firewall->onKernelRequest($event);
 
-        $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, $firewall);
+        $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, $firewall, true);
         $collector->collect($request, $response);
 
         $this->assertNotEmpty($collected = $collector->getListeners()[0]);
@@ -339,7 +339,7 @@ public function testCollectDecisionLog(string $strategy, array $decisionLog, arr
             ->method('getDecisionLog')
             ->willReturn($decisionLog);
 
-        $dataCollector = new SecurityDataCollector(null, null, null, $accessDecisionManager);
+        $dataCollector = new SecurityDataCollector(null, null, null, $accessDecisionManager, null, null, true);
         $dataCollector->collect(new Request(), new Response());
 
         $this->assertEquals($dataCollector->getAccessDecisionLog(), $expectedDecisionLog, 'Wrong value returned by getAccessDecisionLog');

Tests/DependencyInjection/SecurityExtensionTest.php

@@ -788,6 +788,26 @@ public function testConfigureCustomFirewallListener()
         $this->assertContains('custom_firewall_listener_id', $firewallListeners);
     }
 
+    /**
+     * @group legacy
+     */
+    public function testLegacyAuthorizationManagerSignature()
+    {
+        $container = $this->getRawContainer();
+        $container->loadFromExtension('security', [
+            'always_authenticate_before_granting' => true,
+            'firewalls' => ['main' => ['http_basic' => true]],
+        ]);
+
+        $container->compile();
+
+        $args = $container->getDefinition('security.authorization_checker')->getArguments();
+        $this->assertEquals('security.token_storage', (string) $args[0]);
+        $this->assertEquals('security.authentication_manager', (string) $args[1]);
+        $this->assertEquals('security.access.decision_manager', (string) $args[2]);
+        $this->assertEquals('%security.access.always_authenticate_before_granting%', (string) $args[3]);
+    }
+
     protected function getRawContainer()
     {
         $container = new ContainerBuilder();

composer.json

@@ -26,10 +26,10 @@
         "symfony/http-foundation": "^5.3|^6.0",
         "symfony/password-hasher": "^5.3|^6.0",
         "symfony/polyfill-php80": "^1.16",
-        "symfony/security-core": "^5.3|^6.0",
+        "symfony/security-core": "^5.4|^6.0",
         "symfony/security-csrf": "^4.4|^5.0|^6.0",
         "symfony/security-guard": "^5.3|^6.0",
-        "symfony/security-http": "^5.3.2|^6.0"
+        "symfony/security-http": "^5.4|^6.0"
     },
     "require-dev": {
         "doctrine/annotations": "^1.10.4",