Skip to content

[Security] Merge UserAuthorizationCheckerInterface into AuthorizationCheckerInterface #59793

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[Security] Merge UserAuthorizationCheckerInterface into AuthorizationCheckerInterface #59793

wants to merge 1 commit into from

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Feb 17, 2025 •
edited
Loading

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues -
License MIT

This is a follow up of #48142 which was merged recently into 7.3

Having UserAuthorizationCheckerInterface separate from AuthorizationCheckerInterface doesn't make sense to me.
It makes things confusing IMHO, as in "which interface should I implement?"

Of course, AccessDecisionManagerInterface is the central point, but still, I don't see how a separate UserAuthorizationCheckerInterface is useful here.

The PR also fixes a few issues found in tests and replaces the internal UserAuthorizationCheckerToken by an anonymous class.

@carsonbot carsonbot added this to the 7.3 milestone Feb 17, 2025
@nicolas-grekas nicolas-grekas force-pushed the is-granted-for-user branch 2 times, most recently from dda6575 to d4a5ea2 Compare February 17, 2025 15:41
@OskarStark OskarStark changed the title [Security] Merge UserAuthorizationCheckerInterface into AuthorizationCheckerInterface [Security] Merge UserAuthorizationCheckerInterface into AuthorizationCheckerInterface Feb 17, 2025
@nicolas-grekas
Copy link
Member Author

I'm going to submit again with a different approach. Closing for now.

@nicolas-grekas nicolas-grekas deleted the is-granted-for-user branch February 18, 2025 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexandre-daubois alexandre-daubois alexandre-daubois left review comments

@chalasr chalasr Awaiting requested review from chalasr chalasr is a code owner

Assignees
No one assigned
Labels
Feature Security Status: Needs Review
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.
3 participants
@nicolas-grekas @alexandre-daubois @carsonbot