symfony · fabpot · Aug 8, 2021 · Aug 7, 2021
diff --git a/UPGRADE-5.4.md b/UPGRADE-5.4.md
@@ -37,6 +37,7 @@ Messenger
 SecurityBundle
 --------------
 
+ * 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()`.
@@ -57,10 +58,14 @@ SecurityBundle
 Security
 --------
 
- * Deprecate setting the 4th argument (`$alwaysAuthenticate`) to `true` and not setting the
-   5th argument (`$exceptionOnNoToken`) to `false` of `AuthorizationChecker` (this is the default
+ * Deprecate the `$authManager` argument of `AccessListener`
+ * Deprecate the `$authenticationManager` argument of the `AuthorizationChecker` constructor
+ * Deprecate not setting `$authenticatorManagerEnabled` to `true` in `SecurityDataCollector` and `DebugFirewallCommand`
+   (this is the default behavior when using `enable_authenticator_manager: true`)
+ * Deprecate setting the `$alwaysAuthenticate` argument to `true` and not setting the
+   `$exceptionOnNoToken argument to `false` of `AuthorizationChecker` (this is the default
    behavior when using `enable_authenticator_manager: true`)
- * Deprecate not setting the 5th argument (`$exceptionOnNoToken`) of `AccessListener` to `false`
+ * Deprecate not setting the `$exceptionOnNoToken` argument of `AccessListener` to `false`
    (this is the default behavior when using `enable_authenticator_manager: true`)
  * Deprecate `TokenInterface:isAuthenticated()` and `setAuthenticated()` methods without replacement.
    Security tokens won't have an "authenticated" flag anymore, so they will always be considered authenticated

@@ -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()`

@@ -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;

@@ -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;

@@ -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');
         }
 

@@ -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'),
             ])

@@ -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');

@@ -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();

@@ -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",

@@ -32,17 +32,29 @@ class AuthorizationChecker implements AuthorizationCheckerInterface
     private $alwaysAuthenticate;
     private $exceptionOnNoToken;
 
-    public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, AccessDecisionManagerInterface $accessDecisionManager, bool $alwaysAuthenticate = false, bool $exceptionOnNoToken = true)
+    public function __construct(TokenStorageInterface $tokenStorage, /*AccessDecisionManagerInterface*/ $accessDecisionManager, /*bool*/ $alwaysAuthenticate = false, /*bool*/ $exceptionOnNoToken = true)
     {
+        if ($accessDecisionManager instanceof AuthenticationManagerInterface) {
+            trigger_deprecation('symfony/security-core', '5.4', 'The $autenticationManager argument of "%s" is deprecated.', __METHOD__);
+
+            $this->authenticationManager = $accessDecisionManager;
+            $accessDecisionManager = $alwaysAuthenticate;
+            $alwaysAuthenticate = $exceptionOnNoToken;
+            $exceptionOnNoToken = \func_num_args() > 4 ? func_get_arg(4) : true;
+        }
+
         if (false !== $alwaysAuthenticate) {
             trigger_deprecation('symfony/security-core', '5.4', 'Not setting the 4th argument of "%s" to "false" is deprecated.', __METHOD__);
         }
         if (false !== $exceptionOnNoToken) {
             trigger_deprecation('symfony/security-core', '5.4', 'Not setting the 5th argument of "%s" to "false" is deprecated.', __METHOD__);
         }
 
+        if (!$accessDecisionManager instanceof AccessDecisionManagerInterface) {
+            throw new \TypeError(sprintf('Argument 2 of "%s" must be instance of "%s", "%s" given.', __METHOD__, AccessDecisionManagerInterface::class, get_debug_type($accessDecisionManager)));
+        }
+
         $this->tokenStorage = $tokenStorage;
-        $this->authenticationManager = $authenticationManager;
         $this->accessDecisionManager = $accessDecisionManager;
         $this->alwaysAuthenticate = $alwaysAuthenticate;
         $this->exceptionOnNoToken = $exceptionOnNoToken;

@@ -4,8 +4,9 @@ CHANGELOG
 5.4
 ---
 
- * Deprecate setting the 4th argument (`$alwaysAuthenticate`) to `true` and not setting the
-   5th argument (`$exceptionOnNoToken`) to `false` of `AuthorizationChecker`
+ * Deprecate the `$authenticationManager` argument of the `AuthorizationChecker` constructor
+ * Deprecate setting the `$alwaysAuthenticate` argument to `true` and not setting the
+   `$exceptionOnNoToken` argument to `false` of `AuthorizationChecker`
  * Deprecate methods `TokenInterface::isAuthenticated()` and `setAuthenticated`,
    tokens will always be considered authenticated in 6.0
 

@@ -29,13 +29,11 @@ class AuthorizationCheckerTest extends TestCase
 
     protected function setUp(): void
     {
-        $this->authenticationManager = $this->createMock(AuthenticationManagerInterface::class);
         $this->accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class);
         $this->tokenStorage = new TokenStorage();
 
         $this->authorizationChecker = new AuthorizationChecker(
             $this->tokenStorage,
-            $this->authenticationManager,
             $this->accessDecisionManager,
             false,
             false
@@ -52,7 +50,9 @@ public function testVoteAuthenticatesTokenIfNecessary()
 
         $newToken = new UsernamePasswordToken('username', 'password', 'provider');
 
-        $this->authenticationManager
+        $authenticationManager = $this->createMock(AuthenticationManagerInterface::class);
+        $this->authorizationChecker = new AuthorizationChecker($this->tokenStorage, $authenticationManager, $this->accessDecisionManager, false, false);
+        $authenticationManager
             ->expects($this->once())
             ->method('authenticate')
             ->with($this->equalTo($token))
@@ -81,11 +81,7 @@ public function testVoteAuthenticatesTokenIfNecessary()
      */
     public function testLegacyVoteWithoutAuthenticationToken()
     {
-        $authorizationChecker = new AuthorizationChecker(
-            $this->tokenStorage,
-            $this->authenticationManager,
-            $this->accessDecisionManager
-        );
+        $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $this->accessDecisionManager);
 
         $this->expectException(AuthenticationCredentialsNotFoundException::class);
 
@@ -94,7 +90,7 @@ public function testLegacyVoteWithoutAuthenticationToken()
 
     public function testVoteWithoutAuthenticationToken()
     {
-        $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $this->authenticationManager, $this->accessDecisionManager, false, false);
+        $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $this->accessDecisionManager, false, false);
 
         $this->accessDecisionManager
             ->expects($this->once())

@@ -12,7 +12,6 @@
 namespace Symfony\Component\Security\Core\Tests\Authorization;
 
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
 use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver;
 use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
 use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
@@ -37,7 +36,7 @@ public function testIsAuthenticated($token, $expression, $result)
         $tokenStorage = new TokenStorage();
         $tokenStorage->setToken($token);
         $accessDecisionManager = new AccessDecisionManager([new RoleVoter(), new AuthenticatedVoter($trustResolver)]);
-        $authChecker = new AuthorizationChecker($tokenStorage, $this->createMock(AuthenticationManagerInterface::class), $accessDecisionManager, false, false);
+        $authChecker = new AuthorizationChecker($tokenStorage, $accessDecisionManager, false, false);
 
         $context = [];
         $context['auth_checker'] = $authChecker;

@@ -4,7 +4,8 @@ CHANGELOG
 5.4
 ---
 
- * Deprecate not setting the 5th argument (`$exceptionOnNoToken`) of `AccessListener` to `false`
+ * Deprecate the `$authManager` argument of `AccessListener`
+ * Deprecate not setting the `$exceptionOnNoToken` argument of `AccessListener` to `false`
  * Deprecate `DeauthenticatedEvent`, use `TokenDeauthenticatedEvent` instead
  * Deprecate `CookieClearingLogoutHandler`, `SessionLogoutHandler` and `CsrfTokenClearingLogoutHandler`.
    Use `CookieClearingLogoutListener`, `SessionLogoutListener` and `CsrfTokenClearingLogoutListener` instead