[Security] Deprecate PassportInterface
#42198
Conversation
carsonbot
added
Status: Needs Review
Deprecation
Security
and removed
Status: Needs Work
labels
carsonbot
changed the title
PassportInterfacePassportInterface
carsonbot
changed the title
PassportInterfacePassportInterface
chalasr
force-pushed
the
burn-passport-inter
branch
from
b001801 to
0652225
Compare
wouterj
changed the title
PassportInterfacePassportInterface
chalasr
force-pushed
the
burn-passport-inter
branch
from
0652225 to
c1c2d5b
Compare
| * @return Passport | ||
| */ | ||
| public function authenticate(Request $request): PassportInterface; | ||
| public function authenticate(Request $request); /*: Passport;*/ |
There was a problem hiding this comment.
Well, maybe we should keep the existing return type while documenting the new one, instead of widening the explicit return type now (the new one is only documented anyway).
There was a problem hiding this comment.
👍 if the DebugClassLoader still correctly reports the deprecation.
There was a problem hiding this comment.
Keeping the return type would forbid upgrading userland authenticators on PHP versions prior to 7.4 https://3v4l.org/LcLRN#veol
There was a problem hiding this comment.
I don't see much issue in requiring a user to be on PHP 7.4 to fix a deprecation (i.e. it's not breaking support for older versions, it's just triggering a deprecation if you stay on : PassportInterface). They would need PHP 8 anyways if they want to upgrade to Symfony 6 and even PHP 7.4 will be eom when we release Symfony 5.4.
There was a problem hiding this comment.
Pushing our users to upgrade their PHP versions has never been our take, at least not by making symfony upgrades harder. Moreover, that would break our CI on 7.2 given that core authenticators have been upgraded.
chalasr
force-pushed
the
burn-passport-inter
branch
5 times, most recently
from
e2dfd5f to
db572e3
Compare
chalasr
changed the title
PassportInterfacePassportInterface
chalasr
force-pushed
the
burn-passport-inter
branch
from
db572e3 to
234a423
Compare
|
Renamed PR is ready. |
src/Symfony/Component/Security/Http/Authenticator/AuthenticatorInterface.php
Outdated
Show resolved
Hide resolved
chalasr
force-pushed
the
burn-passport-inter
branch
from
234a423 to
a446030
Compare
|
Thank you @chalasr. |
5 participants
As explained in #42181, the right extension point is badges, not passports.
Also renames
AuthenticatorInterface::createAuthenticatedToken()tocreateToken()because of the signature change and the recent abandon of theauthenticatedstate for tokens.