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

16 files changed

+95
-54
lines changed

‎UPGRADE-5.4.md

+8-3
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

+1
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

+4
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

+4
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

+9-2
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

-1
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

+11-11
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

+20
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

+2-2
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

+14-2
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;

‎src/Symfony/Component/Security/Core/CHANGELOG.md

+3-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ CHANGELOG
44
5.4
55
---
66

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

‎src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php

+5-9
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,11 @@ class AuthorizationCheckerTest extends TestCase
2929

3030
protected function setUp(): void
3131
{
32-
$this->authenticationManager = $this->createMock(AuthenticationManagerInterface::class);
3332
$this->accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class);
3433
$this->tokenStorage = new TokenStorage();
3534

3635
$this->authorizationChecker = new AuthorizationChecker(
3736
$this->tokenStorage,
38-
$this->authenticationManager,
3937
$this->accessDecisionManager,
4038
false,
4139
false
@@ -52,7 +50,9 @@ public function testVoteAuthenticatesTokenIfNecessary()
5250

5351
$newToken = new UsernamePasswordToken('username', 'password', 'provider');
5452

55-
$this->authenticationManager
53+
$authenticationManager = $this->createMock(AuthenticationManagerInterface::class);
54+
$this->authorizationChecker = new AuthorizationChecker($this->tokenStorage, $authenticationManager, $this->accessDecisionManager, false, false);
55+
$authenticationManager
5656
->expects($this->once())
5757
->method('authenticate')
5858
->with($this->equalTo($token))
@@ -81,11 +81,7 @@ public function testVoteAuthenticatesTokenIfNecessary()
8181
*/
8282
public function testLegacyVoteWithoutAuthenticationToken()
8383
{
84-
$authorizationChecker = new AuthorizationChecker(
85-
$this->tokenStorage,
86-
$this->authenticationManager,
87-
$this->accessDecisionManager
88-
);
84+
$authorizationChecker = new AuthorizationChecker($this->tokenStorage, $this->accessDecisionManager);
8985

9086
$this->expectException(AuthenticationCredentialsNotFoundException::class);
9187

@@ -94,7 +90,7 @@ public function testLegacyVoteWithoutAuthenticationToken()
9490

9591
public function testVoteWithoutAuthenticationToken()
9692
{
97-
$authorizationChecker = new AuthorizationChecker($this->tokenStorage, $this->authenticationManager, $this->accessDecisionManager, false, false);
93+
$authorizationChecker = new AuthorizationChecker($this->tokenStorage, $this->accessDecisionManager, false, false);
9894

9995
$this->accessDecisionManager
10096
->expects($this->once())

‎src/Symfony/Component/Security/Core/Tests/Authorization/ExpressionLanguageTest.php

+1-2
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
namespace Symfony\Component\Security\Core\Tests\Authorization;
1313

1414
use PHPUnit\Framework\TestCase;
15-
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
1615
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver;
1716
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
1817
use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
@@ -37,7 +36,7 @@ public function testIsAuthenticated($token, $expression, $result)
3736
$tokenStorage = new TokenStorage();
3837
$tokenStorage->setToken($token);
3938
$accessDecisionManager = new AccessDecisionManager([new RoleVoter(), new AuthenticatedVoter($trustResolver)]);
40-
$authChecker = new AuthorizationChecker($tokenStorage, $this->createMock(AuthenticationManagerInterface::class), $accessDecisionManager, false, false);
39+
$authChecker = new AuthorizationChecker($tokenStorage, $accessDecisionManager, false, false);
4140

4241
$context = [];
4342
$context['auth_checker'] = $authChecker;

‎src/Symfony/Component/Security/Http/CHANGELOG.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ CHANGELOG
44
5.4
55
---
66

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

0 commit comments

Comments
 (0)