8000 feature #42420 [Security] Deprecate legacy signatures (wouterj) · symfony/symfony@be948ea · GitHub
[go: up one dir, main page]
Skip to content

Commit be948ea

Browse files
fabpotfabpot
committed
feature #42420 [Security] Deprecate legacy signatures (wouterj)
This PR was merged into the 5.4 branch. Discussion ---------- [Security] Deprecate legacy signatures | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | Ref #41613 | License | MIT | Doc PR | n/a Deprecates the left-over legacy constructor signatures in the Security system. Commits ------- bbc00c8 [Security] Deprecate legacy signatures
2 parents 8e984fa + bbc00c8 commit be948ea

File tree

16 files changed

+95
-54
lines changed
  • Component/Security

    • 16 files changed

    +95
    -54
    lines changed

    ‎UPGRADE-5.4.md‎

    Lines changed: 8 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -37,6 +37,7 @@ Messenger
    3737
    SecurityBundle
    3838
    --------------
    3939

    40+
    * Deprecate not setting `$authenticatorManagerEnabled` to `true` in `SecurityDataCollector` and `DebugFirewallCommand`
    4041
    * Deprecate `SecurityFactoryInterface` and `SecurityExtension::addSecurityListenerFactory()` in favor of
    4142
    `AuthenticatorFactoryInterface` and `SecurityExtension::addAuthenticatorFactory()`
    4243
    * Add `AuthenticatorFactoryInterface::getPriority()` which replaces `SecurityFactoryInterface::getPosition()`.
    @@ -57,10 +58,14 @@ SecurityBundle
    5758
    Security
    5859
    --------
    5960

    60-
    * Deprecate setting the 4th argument (`$alwaysAuthenticate`) to `true` and not setting the
    61-
    5th argument (`$exceptionOnNoToken`) to `false` of `AuthorizationChecker` (this is the default
    61+
    * Deprecate the `$authManager` argument of `AccessListener`
    62+
    * Deprecate the `$authenticationManager` argument of the `AuthorizationChecker` constructor
    63+
    * Deprecate not setting `$authenticatorManagerEnabled` to `true` in `SecurityDataCollector` and `DebugFirewallCommand`
    64+
    (this is the default behavior when using `enable_authenticator_manager: true`)
    65+
    * Deprecate setting the `$alwaysAuthenticate` argument to `true` and not setting the
    66+
    `$exceptionOnNoToken argument to `false` of `AuthorizationChecker` (this is the default
    6267
    behavior when using `enable_authenticator_manager: true`)
    63-
    * Deprecate not setting the 5th argument (`$exceptionOnNoToken`) of `AccessListener` to `false`
    68+
    * Deprecate not setting the `$exceptionOnNoToken` argument of `AccessListener` to `false`
    6469
    (this is the default behavior when using `enable_authenticator_manager: true`)
    6570
    * Deprecate `TokenInterface:isAuthenticated()` and `setAuthenticated()` methods without replacement.
    6671
    Security tokens won't have an "authenticated" flag anymore, so they will always be considered authenticated

    ‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md‎

    Lines changed: 1 addition & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -4,6 +4,7 @@ CHANGELOG
    44
    5.4
    55
    ---
    66

    7+
    * Deprecate not setting `$authenticatorManagerEnabled` to `true` in `SecurityDataCollector` and `DebugFirewallCommand`
    78
    * Deprecate `SecurityFactoryInterface` and `SecurityExtension::addSecurityListenerFactory()` in favor of
    89
    `AuthenticatorFactoryInterface` and `SecurityExtension::addAuthenticatorFactory()`
    910
    * Add `AuthenticatorFactoryInterface::getPriority()` which replaces `SecurityFactoryInterface::getPosition()`

    ‎src/Symfony/Bundle/SecurityBundle/Command/DebugFirewallCommand.php‎

    Lines changed: 4 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -43,6 +43,10 @@ final class DebugFirewallCommand extends Command
    4343
    */
    4444
    public function __construct(array $firewallNames, ContainerInterface $contexts, ContainerInterface $eventDispatchers, array $authenticators, bool $authenticatorManagerEnabled)
    4545
    {
    46+
    if (!$authenticatorManagerEnabled) {
    47+
    trigger_deprecation('symfony/security-bundle', '5.4', 'Setting the $authenticatorManagerEnabled argument of "%s" to "false" is deprecated, use the new authenticator system instead.', __METHOD__);
    48+
    }
    49+
    4650
    $this->firewallNames = $firewallNames;
    4751
    $this->contexts = $contexts;
    4852
    $this->eventDispatchers = $eventDispatchers;

    ‎src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php‎

    Lines changed: 4 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -48,6 +48,10 @@ class SecurityDataCollector extends DataCollector implements LateDataCollectorIn
    4848

    4949
    public function __construct(TokenStorageInterface $tokenStorage = null, RoleHierarchyInterface $roleHierarchy = null, LogoutUrlGenerator $logoutUrlGenerator = null, AccessDecisionManagerInterface $accessDecisionManager = null, FirewallMapInterface $firewallMap = null, TraceableFirewallListener $firewall = null, bool $authenticatorManagerEnabled = false)
    5050
    {
    51+
    if (!$authenticatorManagerEnabled) {
    52+
    trigger_deprecation('symfony/security-bundle', '5.4', 'Setting the $authenticatorManagerEnabled argument of "%s" to "false" is deprecated, use the new authenticator system instead.', __METHOD__);
    53+
    }
    54+
    5155
    $this->tokenStorage = $tokenStorage;
    5256
    $this->roleHierarchy = $roleHierarchy;
    5357
    $this->logoutUrlGenerator = $logoutUrlGenerator;

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

    Lines changed: 9 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -103,12 +103,19 @@ public function load(array $configs, ContainerBuilder $container)
    103103
    // The authenticator system no longer has anonymous tokens. This makes sure AccessListener
    104104
    // and AuthorizationChecker do not throw AuthenticationCredentialsNotFoundException when no
    105105
    // token is available in the token storage.
    106-
    $container->getDefinition('security.access_listener')->setArgument(4, false);
    106+
    $container->getDefinition('security.access_listener')->setArgument(3, false);
    107+
    $container->getDefinition('security.authorization_checker')->setArgument(3, false);
    107108
    $container->getDefinition('security.authorization_checker')->setArgument(4, false);
    108-
    $container->getDefinition('security.authorization_checker')->setArgument(5, false);
    109109
    } else {
    110110
    trigger_deprecation('symfony/security-bundle', '5.3', 'Not setting the "security.enable_authenticator_manager" config option to true is deprecated.');
    111111

    112+
    if ($config['always_authenticate_before_granting']) {
    113+
    $authorizationChecker = $container->getDefinition('security.authorization_checker');
    114+
    $authorizationCheckerArgs = $authorizationChecker->getArguments();
    115+
    array_splice($authorizationCheckerArgs, 1, 0, [new Reference('security.authentication_manager')]);
    116+
    $authorizationChecker->setArguments($authorizationCheckerArgs);
    117+
    }
    118+
    112119
    $loader->load('security_legacy.php');
    113120
    }
    114121

    ‎src/Symfony/Bundle/SecurityBundle/Resources/config/security.php‎

    Lines changed: 0 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -64,7 +64,6 @@
    6464
    ->public()
    6565
    ->args([
    6666
    service('security.token_storage'),
    67-
    service('security.authentication.manager'),
    6867
    service('security.access.decision_manager'),
    6968
    param('security.access.always_authenticate_before_granting'),
    7069
    ])

    ‎src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php‎

    Lines changed: 11 additions & 11 deletions
    Original file line numberDiff line numberDiff line change
    @@ -37,7 +37,7 @@ class SecurityDataCollectorTest extends TestCase
    3737
    {
    3838
    public function testCollectWhenSecurityIsDisabled()
    3939
    {
    40-
    $collector = new SecurityDataCollector();
    40+
    $collector = new SecurityDataCollector(null, null, null, null, null, null, true);
    4141
    $collector->collect(new Request(), new Response());
    4242

    4343
    $this->assertSame('security', $collector->getName());
    @@ -57,7 +57,7 @@ public function testCollectWhenSecurityIsDisabled()
    5757
    public function testCollectWhenAuthenticationTokenIsNull()
    5858
    {
    5959
    $tokenStorage = new TokenStorage();
    60-
    $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
    60+
    $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy(), null, null, null, null, true);
    6161
    $collector->collect(new Request(), new Response());
    6262

    6363
    $this->assertTrue($collector->isEnabled());
    @@ -71,7 +71,7 @@ public function testCollectWhenAuthenticationTokenIsNull()
    7171
    $this->assertCount(0, $collector->getInheritedRoles());
    7272
    $this->assertEmpty($collector->getUser());
    7373
    $this->assertNull($collector->getFirewall());
    74-
    $this->assertFalse($collector->isAuthenticatorManagerEnabled());
    74+
    $this->assertTrue($collector->isAuthenticatorManagerEnabled());
    7575
    }
    7676

    7777
    /** @dataProvider provideRoles */
    @@ -80,7 +80,7 @@ public function testCollectAuthenticationTokenAndRoles(array $roles, array $norm
    8080
    $tokenStorage = new TokenStorage();
    8181
    $tokenStorage->setToken(new UsernamePasswordToken('hhamon', 'P4$$w0rD', 'provider', $roles));
    8282

    83-
    $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
    83+
    $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy(), null, null, null, null, true);
    8484
    $collector->collect(new Request(), new Response());
    8585
    $collector->lateCollect();
    8686

    @@ -94,7 +94,7 @@ public function testCollectAuthenticationTokenAndRoles(array $roles, array $norm
    9494
    $this->assertSame($normalizedRoles, $collector->getRoles()->getValue(true));
    9595
    $this->assertSame($inheritedRoles, $collector->getInheritedRoles()->getValue(true));
    9696
    $this->assertSame('hhamon', $collector->getUser());
    97-
    $this->assertFalse($collector->isAuthenticatorManagerEnabled());
    97+
    $this->assertTrue($collector->isAuthenticatorManagerEnabled());
    9898
    }
    9999

    100100
    public function testCollectSwitchUserToken()
    @@ -104,7 +104,7 @@ public function testCollectSwitchUserToken()
    104104
    $tokenStorage = new TokenStorage();
    105105
    $tokenStorage->setToken(new SwitchUserToken('hhamon', 'P4$$w0rD', 'provider', ['ROLE_USER', 'ROLE_PREVIOUS_ADMIN'], $adminToken));
    106106

    107-
    $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
    107+
    $collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy(), null, null, null, null, true);
    108108
    $collector->collect(new Request(), new Response());
    109109
    $collector->lateCollect();
    110110

    @@ -160,7 +160,7 @@ public function testGetFirewallReturnsNull()
    160160
    $response = new Response();
    161161

    162162
    // Don't inject any firewall map
    163-
    $collector = new SecurityDataCollector();
    163+
    $collector = new SecurityDataCollector(null, null, null, null, null, null, true);
    164164
    $collector->collect($request, $response);
    165165
    $this->assertNull($collector->getFirewall());
    166166

    @@ -170,7 +170,7 @@ public function testGetFirewallReturnsNull()
    170170
    ->disableOriginalConstructor()
    171171
    ->getMock();
    172172

    173-
    $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()));
    173+
    $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()), true);
    174174
    $collector->collect($request, $response);
    175175
    $this->assertNull($collector->getFirewall());
    176176

    @@ -180,7 +180,7 @@ public function testGetFirewallReturnsNull()
    180180
    ->disableOriginalConstructor()
    181181
    ->getMock();
    182182

    183-
    $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()));
    183+
    $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()), true);
    184184
    $collector->collect($request, $response);
    185185
    $this->assertNull($collector->getFirewall());
    186186
    }
    @@ -214,7 +214,7 @@ public function testGetListeners()
    214214
    $firewall = new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator());
    215215
    $firewall->onKernelRequest($event);
    216216

    217-
    $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, $firewall);
    217+
    $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, $firewall, true);
    218218
    $collector->collect($request, $response);
    219219

    220220
    $this->assertNotEmpty($collected = $collector->getListeners()[0]);
    @@ -339,7 +339,7 @@ public function testCollectDecisionLog(string $strategy, array $decisionLog, arr
    339339
    ->method('getDecisionLog')
    340340
    ->willReturn($decisionLog);
    341341

    342-
    $dataCollector = new SecurityDataCollector(null, null, null, $accessDecisionManager);
    342+
    $dataCollector = new SecurityDataCollector(null, null, null, $accessDecisionManager, null, null, true);
    343343
    $dataCollector->collect(new Request(), new Response());
    344344

    345345
    $this->assertEquals($dataCollector->getAccessDecisionLog(), $expectedDecisionLog, 'Wrong value returned by getAccessDecisionLog');

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

    Lines changed: 20 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -788,6 +788,26 @@ public function testConfigureCustomFirewallListener()
    788788
    $this->assertContains('custom_firewall_listener_id', $firewallListeners);
    789789
    }
    790790

    791+
    /**
    792+
    * @group legacy
    793+
    */
    794+
    public function testLegacyAuthorizationManagerSignature()
    795+
    {
    796+
    $container = $this->getRawContainer();
    797+
    $container->loadFromExtension('security', [
    798+
    'always_authenticate_before_granting' => true,
    799+
    'firewalls' => ['main' => ['http_basic' => true]],
    800+
    ]);
    801+
    802+
    $container->compile();
    803+
    804+
    $args = $container->getDefinition('security.authorization_checker')->getArguments();
    805+
    $this->assertEquals('security.token_storage', (string) $args[0]);
    806+
    $this->assertEquals('security.authentication_manager', (string) $args[1]);
    807+
    $this->assertEquals('security.access.decision_manager', (string) $args[2]);
    808+
    $this->assertEquals('%security.access.always_authenticate_before_granting%', (string) $args[3]);
    809+
    }
    810+
    791811
    protected function getRawContainer()
    792812
    {
    793813
    $container = new ContainerBuilder();

    ‎src/Symfony/Bundle/SecurityBundle/composer.json‎

    Lines changed: 2 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -26,10 +26,10 @@
    2626
    "symfony/http-foundation": "^5.3|^6.0",
    2727
    "symfony/password-hasher": "^5.3|^6.0",
    2828
    "symfony/polyfill-php80": "^1.16",
    29-
    "symfony/security-core": "^5.3|^6.0",
    29+
    "symfony/security-core": "^5.4|^6.0",
    3030
    "symfony/security-csrf": "^4.4|^5.0|^6.0",
    3131
    "symfony/security-guard": "^5.3|^6.0",
    32-
    "symfony/security-http": "^5.3.2|^6.0"
    32+
    "symfony/security-http": "^5.4|^6.0"
    3333
    },
    3434
    "require-dev": {
    3535
    "doctrine/annotations": "^1.10.4",

    ‎src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php‎

    Lines changed: 14 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -32,17 +32,29 @@ class AuthorizationChecker implements AuthorizationCheckerInterface
    3232
    private $alwaysAuthenticate;
    3333
    private $exceptionOnNoToken;
    3434

    35-
    public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, AccessDecisionManagerInterface $accessDecisionManager, bool $alwaysAuthenticate = false, bool $exceptionOnNoToken = true)
    35+
    public function __construct(TokenStorageInterface $tokenStorage, /*AccessDecisionManagerInterface*/ $accessDecisionManager, /*bool*/ $alwaysAuthenticate = false, /*bool*/ $exceptionOnNoToken = true)
    3636
    {
    37+
    if ($accessDecisionManager instanceof AuthenticationManagerInterface) {
    38+
    trigger_deprecation('symfony/security-core', '5.4', 'The $autenticationManager argument of "%s" is deprecated.', __METHOD__);
    39+
    40+
    $this->authenticationManager = $accessDecisionManager;
    41+
    $accessDecisionManager = $alwaysAuthenticate;
    42+
    $alwaysAuthenticate = $exceptionOnNoToken;
    43+
    $exceptionOnNoToken = \func_num_args() > 4 ? func_get_arg(4) : true;
    44+
    }
    45+
    3746
    if (false !== $alwaysAuthenticate) {
    3847
    trigger_deprecation('symfony/security-core', '5.4', 'Not setting the 4th argument of "%s" to "false" is deprecated.', __METHOD__);
    3948
    }
    4049
    if (false !== $exceptionOnNoToken) {
    4150
    trigger_deprecation('symfony/security-core', '5.4', 'Not setting the 5th argument of "%s" to "false" is deprecated.', __METHOD__);
    4251
    }
    4352

    53+
    if (!$accessDecisionManager instanceof AccessDecisionManagerInterface) {
    54+
    throw new \TypeError(sprintf('Argument 2 of "%s" must be instance of "%s", "%s" given.', __METHOD__, AccessDecisionManagerInterface::class, get_debug_type($accessDecisionManager)));
    55+
    }
    56+
    4457
    $this->tokenStorage = $tokenStorage;
    45-
    $this->authenticationManager = $authenticationManager;
    4658
    $this->accessDecisionManager = $accessDecisionManager;
    4759
    $this->alwaysAuthenticate = $alwaysAuthenticate;
    4860
    $this->exceptionOnNoToken = $exceptionOnNoToken;

    0 commit comments

    Comments
     (0)
    0