10000 feature #42595 Fix incompatibilities with upcoming security 6.0 (wout… · symfony/symfony@f1643e8 · GitHub
[go: up one dir, main page]
Skip to content

Commit f1643e8

Browse files
fabpotfabpot
committed
feature #42595 Fix incompatibilities with upcoming security 6.0 (wouterj)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- Fix incompatibilities with upcoming security 6.0 | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | yes | Tickets | - | License | MIT | Doc PR | - Hats of to the person that invented the flipped tests on a new major branch! All these incompatibility bugs were discovered by the flipped tests of #41613 Commits ------- 96532e5 [SecurityHttp] Fix incompatibility with 6.0 fb45f6b [SecurityGuard] Fix incompatibility with 6.0 d2a1abf [SecurityBundle] Fix incompatibility with 6.0 4628689 [FrameworkBundle] Fix incompatibility with 6.0 98328ad [SecurityHttp] Fix incompatibility with 6.0 9137242 [PasswordHasher] Fix incompatibility with 6.0 915f75b [MonologBridge] Fix incompatibility with 6.0 0b59bc2 [Security] Minor fixes
2 parents 1fa2aab + 96532e5 commit f1643e8

25 files changed

+99
-73
lines changed

‎UPGRADE-5.4.md

+1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ SecurityBundle
6666
Security
6767
--------
6868

69+
* Deprecate `AuthenticationEvents::AUTHENTICATION_FAILURE`, use the `LoginFailureEvent` instead
6970
* Deprecate the `$authenticationEntryPoint` argument of `ChannelListener`, and add `$httpPort` and `$httpsPort` arguments
7071
* Deprecate `RetryAuthenticationEntryPoint`, this code is now inlined in the `ChannelListener`
7172
* Deprecate `FormAuthenticationEntryPoint` and `BasicAuthenticationEntryPoint`, in the new system the `FormLoginAuthenticator`

‎UPGRADE-6.0.md

+1
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ Routing
208208
Security
209209
--------
210210

211+
* Remove `AuthenticationEvents::AUTHENTICATION_FAILURE`, use the `LoginFailureEvent` instead
211212
* Remove the `$authenticationEntryPoint` argument of `ChannelListener`
212213
* Remove `RetryAuthenticationEntryPoint`, this code was inlined in the `ChannelListener`
213214
* Remove `FormAuthenticationEntryPoint` and `BasicAuthenticationEntryPoint`, the `FormLoginAuthenticator` and `HttpBasicAuthenticator` should be used instead.

‎src/Symfony/Bridge/Monolog/Processor/AbstractTokenProcessor.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public function __invoke(array $record): array
4242

4343
if (null !== $token = $this->getToken()) {
4444
$record['extra'][$this->getKey()] = [
45-
'authenticated' => $token->isAuthenticated(false), // @deprecated since Symfony 5.4, always true in 6.0
45+
'authenticated' => method_exists($token, 'isAuthenticated') ? $token->isAuthenticated(false) : true, // @deprecated since Symfony 5.4, always true in 6.0
4646
'roles' => $token->getRoleNames(),
4747
];
4848

‎src/Symfony/Bundle/FrameworkBundle/KernelBrowser.php

+4-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,10 @@ public function loginUser(object $user, string $firewallContext = 'main'): self
123123
}
124124

125125
$token = new TestBrowserToken($user->getRoles(), $user, $firewallContext);
126-
$token->setAuthenticated(true, false);
126+
// @deprecated since Symfony 5.4
127+
if (method_exists($token, 'isAuthenticated')) {
128+
$token->setAuthenticated(true, false);
129+
}
127130

128131
$container = $this->getContainer();
129132
$container->get('security.untracked_token_storage')->setToken($token);

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ public function collect(Request $request, Response $response, \Throwable $except
127127

128128
$this->data = [
129129
'enabled' => true,
130-
'authenticated' => $token->isAuthenticated(false),
130+
'authenticated' => method_exists($token, 'isAuthenticated') ? $token->isAuthenticated(false) : true,
131131
'impersonated' => null !== $impersonatorUser,
132132
'impersonator_user' => $impersonatorUser,
133133
'impersonation_exit_path' => null,

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/RegisterEntryPointsPassTest.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
2626
use Symfony\Component\Security\Http\Authentication\AuthenticatorManager;
2727
use Symfony\Component\Security\Http\Authenticator\AbstractAuthenticator;
28-
use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface;
28+
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
2929
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
3030
use Symfony\Component\Security\Http\Firewall\ExceptionListener;
3131

@@ -76,7 +76,7 @@ public function supports(Request $request): ?bool
7676
return false;
7777
}
7878

79-
public function authenticate(Request $request): PassportInterface
79+
public function authenticate(Request $request): Passport
8080
{
8181
throw new BadCredentialsException();
8282
}

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
use Symfony\Component\Security\Guard\AuthenticatorInterface as GuardAuthenticatorInterface;
3939
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
4040
use Symfony\Component\Security\Http\Authenticator\HttpBasicAuthenticator;
41+
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
4142
use Symfony F438 \Component\Security\Http\Authenticator\Passport\PassportInterface;
4243

4344
class SecurityExtensionTest extends TestCase
@@ -841,7 +842,7 @@ public function supports(Request $request): ?bool
841842
{
842843
}
843844

844-
public function authenticate(Request $request): PassportInterface
845+
public function authenticate(Request $request): Passport
845846
{
846847
}
847848

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/AuthenticatorBundle/ApiAuthenticator.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
use Symfony\Component\Security\Core\User\InMemoryUser;
2121
use Symfony\Component\Security\Http\Authenticator\AbstractAuthenticator;
2222
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
23-
use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface;
23+
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
2424
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
2525

2626
class ApiAuthenticator extends AbstractAuthenticator
@@ -37,7 +37,7 @@ public function supports(Request $request): ?bool
3737
return $request->headers->has('X-USER-EMAIL');
3838
}
3939

40-
public function authenticate(Request $request): PassportInterface
40+
public function authenticate(Request $request): Passport
4141
{
4242
$email = $request->headers->get('X-USER-EMAIL');
4343
if (false === strpos($email, '@')) {

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/AuthenticatorBundle/LoginFormAuthenticator.php

+1-2
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
2222
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
2323
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
24-
use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface;
2524
use Symfony\Component\Security\Http\Util\TargetPathTrait;
2625

2726
class LoginFormAuthenticator extends AbstractLoginFormAuthenticator
@@ -36,7 +35,7 @@ public function __construct(UrlGeneratorInterface $urlGenerator)
3635
$this->urlGenerator = $urlGenerator;
3736
}
3837

39-
public function authenticate(Request $request): PassportInterface
38+
public function authenticate(Request $request): Passport
4039
{
4140
$username = $request->request->get('_username', '');
4241

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php

+9-9
Original file line numberDiff line numberDiff line change
@@ -177,60 +177,60 @@ public function getPassword(): ?string
177177
/**
178178
* {@inheritdoc}
179179
*/
180-
public function getSalt()
180+
public function getSalt(): string
181181
{
182-
return null;
182+
return '';
183183
}
184184

185185
/**
186186
* {@inheritdoc}
187187
*/
188-
public function getUsername()
188+
public function getUsername(): string
189189
{
190190
return $this->username;
191191
}
192192

193-
public function getUserIdentifier()
193+
public function getUserIdentifier(): string
194194
{
195195
return $this->username;
196196
}
197197

198198
/**
199199
* {@inheritdoc}
200200
*/
201-
public function isAccountNonExpired()
201+
public function isAccountNonExpired(): bool
202202
{
203203
return $this->accountNonExpired;
204204
}
205205

206206
/**
207207
* {@inheritdoc}
208208
*/
209-
public function isAccountNonLocked()
209+
public function isAccountNonLocked(): bool
210210
{
211211
return $this->accountNonLocked;
212212
}
213213

214214
/**
215215
* {@inheritdoc}
216216
*/
217-
public function isCredentialsNonExpired()
217+
public function isCredentialsNonExpired(): bool
218218
{
219219
return $this->credentialsNonExpired;
220220
}
221221

222222
/**
223223
* {@inheritdoc}
224224
*/
225-
public function isEnabled()
225+
public function isEnabled(): bool
226226
{
227227
return $this->enabled;
228228
}
229229

230230
/**
231231
* {@inheritdoc}
232232
*/
233-
public function eraseCredentials()
233+
public function eraseCredentials(): void
234234
{
235235
}
236236
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/base_config.yml

+12-12
Original file line numberDiff line numberDiff line change
@@ -39,19 +39,19 @@ security:
3939
path: /second/logout
4040

4141
access_control:
42-
- { path: ^/en/$, roles: IS_AUTHENTICATED_ANONYMOUSLY }
43-
- { path: ^/unprotected_resource$, roles: IS_AUTHENTICATED_ANONYMOUSLY }
44-
- { path: ^/secure-but-not-covered-by-access-control$, roles: IS_AUTHENTICATED_ANONYMOUSLY }
45-
- { path: ^/secured-by-one-ip$, ip: 10.10.10.10, roles: IS_AUTHENTICATED_ANONYMOUSLY }
46-
- { path: ^/secured-by-two-ips$, ips: [1.1.1.1, 2.2.2.2], roles: IS_AUTHENTICATED_ANONYMOUSLY }
42+
- { path: ^/en/$, roles: PUBLIC_ACCESS }
43+
- { path: ^/unprotected_resource$, roles: PUBLIC_ACCESS }
44+
- { path: ^/secure-but-not-covered-by-access-control$, roles: PUBLIC_ACCESS }
45+
- { path: ^/secured-by-one-ip$, ip: 10.10.10.10, roles: PUBLIC_ACCESS }
46+
- { path: ^/secured-by-two-ips$, ips: [1.1.1.1, 2.2.2.2], roles: PUBLIC_ACCESS }
4747
# these real IP addresses are reserved for docs/examples (https://tools.ietf.org/search/rfc5737)
48-
- { path: ^/secured-by-one-real-ip$, ips: 198.51.100.0, roles: IS_AUTHENTICATED_ANONYMOUSLY }
49-
- { path: ^/secured-by-one-real-ip-with-mask$, ips: '203.0.113.0/24', roles: IS_AUTHENTICATED_ANONYMOUSLY }
50-
- { path: ^/secured-by-one-real-ipv6$, ips: 0:0:0:0:0:ffff:c633:6400, roles: IS_AUTHENTICATED_ANONYMOUSLY }
51-
- { path: ^/secured-by-one-env-placeholder$, ips: '%env(APP_IP)%', roles: IS_AUTHENTICATED_ANONYMOUSLY }
52-
- { path: ^/secured-by-one-env-placeholder-multiple-ips$, ips: '%env(APP_IPS)%', roles: IS_AUTHENTICATED_ANONYMOUSLY }
53-
- { path: ^/secured-by-one-env-placeholder-and-one-real-ip$, ips: ['%env(APP_IP)%', 198.51.100.0], roles: IS_AUTHENTICATED_ANONYMOUSLY }
54-
- { path: ^/secured-by-one-env-placeholder-multiple-ips-and-one-real-ip$, ips: ['%env(APP_IPS)%', 198.51.100.0], roles: IS_AUTHENTICATED_ANONYMOUSLY }
48+
- { path: ^/secured-by-one-real-ip$, ips: 198.51.100.0, roles: PUBLIC_ACCESS }
49+
- { path: ^/secured-by-one-real-ip-with-mask$, ips: '203.0.113.0/24', roles: PUBLIC_ACCESS }
50+
- { path: ^/secured-by-one-real-ipv6$, ips: 0:0:0:0:0:ffff:c633:6400, roles: PUBLIC_ACCESS }
51+
- { path: ^/secured-by-one-env-placeholder$, ips: '%env(APP_IP)%', roles: PUBLIC_ACCESS }
52+
- { path: ^/secured-by-one-env-placeholder-multiple-ips$, ips: '%env(APP_IPS)%', roles: PUBLIC_ACCESS }
53+
- { path: ^/secured-by-one-env-placeholder-and-one-real-ip$, ips: ['%env(APP_IP)%', 198.51.100.0], roles: PUBLIC_ACCESS }
54+
- { path: ^/secured-by-one-env-placeholder-multiple-ips-and-one-real-ip$, ips: ['%env(APP_IPS)%', 198.51.100.0], roles: PUBLIC_ACCESS }
5555
- { path: ^/highly_protected_resource$, roles: IS_ADMIN }
5656
- { path: ^/protected-via-expression$, allow_if: "(!is_authenticated() and request.headers.get('user-agent') matches '/Firefox/i') or is_granted('ROLE_USER')" }
5757
- { path: .*, roles: IS_AUTHENTICATED_FULLY }

‎src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasher.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function hashPassword($user, string $plainPassword): string
4848
if ($user instanceof LegacyPasswordAuthenticatedUserInterface) {
4949
$salt = $user->getSalt();
5050
} elseif ($user instanceof UserInterface) {
51-
$salt = $user->getSalt();
51+
$salt = method_exists($user, 'getSalt') ? $user->getSalt() : null;
5252

5353
if ($salt) {
5454
trigger_deprecation('symfony/password-hasher', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user));

‎src/Symfony/Component/PasswordHasher/Tests/Fixtures/TestLegacyPasswordAuthenticatedUser.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -30,23 +30,23 @@ public function getPassword(): ?string
3030
return $this->password;
3131
}
3232

33-
public function getRoles()
33+
public function getRoles(): array
3434
{
3535
return $this->roles;
3636
}
3737

38-
public function eraseCredentials()
38+
public function eraseCredentials(): void
3939
{
4040
// Do nothing
4141
return;
4242
}
4343

44-
public function getUsername()
44+
public function getUsername(): string
4545
{
4646
return $this->username;
4747
}
4848

49-
public function getUserIdentifier()
49+
public function getUserIdentifier(): string
5050
{
5151
return $this->username;
5252
}

‎src/Symfony/Component/Security/Core/AuthenticationEvents.php

+2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ final class AuthenticationEvents
2929
* authenticated by any of the providers.
3030
*
3131
* @Event("Symfony\Component\Security\Core\Event\AuthenticationFailureEvent")
32+
*
33+
* @deprecated since Symfony 5.4, use {@see Event\LoginFailureEvent} instead
3234
*/
3335
public const AUTHENTICATION_FAILURE = 'security.authentication.failure';
3436

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

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

7+
* Deprecate `AuthenticationEvents::AUTHENTICATION_FAILURE`, use the `LoginFailureEvent` instead
78
* Deprecate `AnonymousToken`, as the related authenticator was deprecated in 5.3
89
* Deprecate `Token::getCredentials()`, tokens should no longer contain credentials (as they represent authenticated sessions)
910
* Deprecate returning `string|\Stringable` from `Token::getUser()` (it must return a `UserInterface`)

‎src/Symfony/Component/Security/Core/User/ChainUserProvider.php

+5-5
Original file line numberDiff line numberDiff line change
@@ -56,25 +56,25 @@ public function loadUserByUsername(string $username)
5656
return $this->loadUserByIdentifier($username);
5757
}
5858

59-
public function loadUserByIdentifier(string $userIdentifier): UserInterface
59+
public function loadUserByIdentifier(string $identifier): UserInterface
6060
{
6161
foreach ($this->providers as $provider) {
6262
try {
6363
// @deprecated since Symfony 5.3, change to $provider->loadUserByIdentifier() in 6.0
6464
if (!method_exists($provider, 'loadUserByIdentifier')) {
6565
trigger_deprecation('symfony/security-core', '5.3', 'Not implementing method "loadUserByIdentifier()" in user provider "%s" is deprecated. This method will replace "loadUserByUsername()" in Symfony 6.0.', get_debug_type($provider));
6666

67-
return $provider->loadUserByUsername($userIdentifier);
67+
return $provider->loadUserByUsername($identifier);
6868
}
6969

70-
return $provider->loadUserByIdentifier($userIdentifier);
70+
return $provider->loadUserByIdentifier($identifier);
7171
} catch (UserNotFoundException $e) {
7272
// try next one
7373
}
7474
}
7575

76-
$ex = new UserNotFoundException(sprintf('There is no user with identifier "%s".', $userIdentifier));
77-
$ex->setUserIdentifier($userIdentifier);
76+
$ex = new UserNotFoundException(sprintf('There is no user with identifier "%s".', $identifier));
77+
$ex->setUserIdentifier($identifier);
7878
throw $ex;
7979
}
8080

‎src/Symfony/Component/Security/Guard/Authenticator/GuardBridgeAuthenticator.php

+5
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,11 @@ public function createAuthenticatedToken(PassportInterface $passport, string $fi
121121
return $this->guard->createAuthenticatedToken($passport->getUser(), $firewallName);
122122
}
123123

124+
public function createToken(Passport $passport, string $firewallName): TokenInterface
125+
{
126+
return $this->guard->createAuthenticatedToken($passport->getUser(), $firewallName);
127+
}
128+
124129
public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response
125130
{
126131
return $this->guard->onAuthenticationSuccess($request, $token, $firewallName);

‎src/Symfony/Component/Security/Http/Authenticator/AbstractPreAuthenticatedAuthenticator.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public function createAuthenticatedToken(PassportInterface $passport, string $fi
113113

114114
public function createToken(Passport $passport, string $firewallName): TokenInterface
115115
{
116-
return new PreAuthenticatedToken($passport->getUser(), null, $firewallName, $passport->getUser()->getRoles());
116+
return new PreAuthenticatedToken($passport->getUser(), $firewallName, $passport->getUser()->getRoles());
117117
}
118118

119119
public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response

‎src/Symfony/Component/Security/Http/Authenticator/HttpBasicAuthenticator.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public function createAuthenticatedToken(PassportInterface $passport, string $fi
9595

9696
public function createToken(Passport $passport, string $firewallName): TokenInterface
9797
{
98-
return new UsernamePasswordToken($passport->getUser(), null, $firewallName, $passport->getUser()->getRoles());
98+
return new UsernamePasswordToken($passport->getUser(), $firewallName, $passport->getUser()->getRoles());
9999
}
100100

101101
public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response

‎src/Symfony/Component/Security/Http/Authenticator/Token/PostAuthenticationToken.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,10 @@ public function __construct(UserInterface $user, string $firewallName, array $ro
3535
$this->firewallName = $firewallName;
3636

3737
// @deprecated since Symfony 5.4
38-
// this token is meant to be used after authentication success, so it is always authenticated
39-
$this->setAuthenticated(true, false);
38+
if (method_exists($this, 'setAuthenticated')) {
39+
// this token is meant to be used after authentication success, so it is always authenticated
40+
$this->setAuthenticated(true, false);
41+
}
4042
}
4143

4244
/**

‎src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public function checkPassport(CheckPassportEvent $event): void
7474
throw new BadCredentialsException('The presented password is invalid.');
7575
}
7676

77-
$salt = $user->getSalt();
77+
$salt = method_exists($user, 'getSalt') ? $user->getSalt() : '';
7878
if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) {
7979
trigger_deprecation('symfony/security-http', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user));
8080
}

0 commit comments

Comments
 (0)
0