Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Security] Deprecate the old authentication mechanisms#41247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:5.xfromchalasr:deprec-old-sec
May 19, 2021

Conversation

@chalasr
Copy link
Member

@chalasrchalasr commentedMay 16, 2021
edited
Loading

QA
Branch?5.3
Bug fix?no
New feature?no
Deprecations?yes/
Tickets#39308
LicenseMIT
Doc PRtodo

Now that the authenticator system proven working well and is considered stable, we can deprecate the old authentication listeners as well as the Guard component (+ integrations).

fbourigault and matyo91 reacted with hooray emoji
@chalasr
Copy link
MemberAuthor

Needs some work to make it green.

@chalasrchalasrforce-pushed thedeprec-old-sec branch 8 times, most recently fromd9d281b to03edd82CompareMay 18, 2021 00:30
@nicolas-grekas
Copy link
Member

Why 5.3? That's for 5.4 to me.

derrabus reacted with thumbs up emoji

@chalasr
Copy link
MemberAuthor

Deprecating the old security system as a late change for 5.3 was discussed on slack with@wouterj and@fabpot.

nicolas-grekas reacted with thumbs up emoji

@wouterj
Copy link
Member

The reasoning is similar to#39308: I'm almost certain that we didn't yet test all uses of the old authentication system on the new one. So I want to buy us time to add the missing features/cases in 5.4, before removing the old system in 6.0. Deprecating the old system gives us the most chance to get this type of feedback from bigger applications.

@nicolas-grekas
Copy link
Member

Got it, thanks.
(Rebase needed)

@chalasr
Copy link
MemberAuthor

This PR misses deprecating some service definitions, I'm going to finish it tonight.

@chalasr
Copy link
MemberAuthor

This is ready.

Note: this PR deprecates the main part of the old system, but it's not exhaustive: there are some classes which won't be needed in 6.0 that this PR does not deprecate. e.g.NoopAuthenticationManager is still needed internally for now.
Why it's not an issue? Fixing the deprecations introduced here implies that you are not using the remaining to-be-deprecated stuff anymore, thus you won't be bothered for the same topic in the next minor.

Copy link
Member

@wouterjwouterj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Wow, thanks Robin! I feel very sorry for you after seeing all the SecurityBundle tests that needed fixes as well..

Why it's not an issue? Fixing the deprecations introduced here implies that you are not using the remaining to-be-deprecated stuff anymore, thus you won't be bothered for the same topic in the next minor.

Is this also the reason for not havingtrigger_deprecation() calls in almost all Guard component classes?

chalasr reacted with heart emoji
@chalasrchalasrforce-pushed thedeprec-old-sec branch 3 times, most recently fromddcdb9a to93d4bafCompareMay 18, 2021 22:53
@chalasr
Copy link
MemberAuthor

chalasr commentedMay 18, 2021
edited
Loading

Is this also the reason for not having trigger_deprecation() calls in almost all Guard component classes?

@wouterj Nope, that was a mistake. It's fixed now.
The only files that have notrigger_deprecation() calls but only@deprecated annotations are abstract classes and interfaces as it causes such issues:

. 1x: Since symfony/security-guard 5.3: The "Symfony\Component\Security\Guard\Authenticator\AbstractFormLoginAuthenticator" class is deprecated, use the new authenticator system instead.
1x in ClassLoader::loadClass from Composer\Autoload

I asked@nicolas-grekas who confirmed that we have to skip triggering in this case.

Thanks for the review!

@fabpot
Copy link
Member

Thank you@chalasr.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@wouterjwouterjwouterj left review comments

@derrabusderrabusderrabus left review comments

@fabpotfabpotfabpot approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

5.3

Development

Successfully merging this pull request may close these issues.

6 participants

@chalasr@nicolas-grekas@wouterj@fabpot@derrabus@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp