-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Security] Improve DX of recent additions #59805
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
a07496e
to
8d7316b
Compare
src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php
Show resolved
Hide resolved
8d7316b
to
d74e07a
Compare
Thanks for the reviews, comments addressed. Status: needs review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvement to the DX, thanks!
d74e07a
to
aa38eb8
Compare
I just removed the |
isn't fabbot's patch legit? |
I think this alignment rule doesn't work when the type description is too long, so I ignored fabbot on purpose... |
Thank you @nicolas-grekas. |
This is a follow up of #48142 and #59150 which were merged recently into 7.3.
Summary of the changes:
UserAuthorizationChecker
, the implementation of the corresponding interface is merged into the existingAuthorizationChecker
.AuthorizationChecker::isGranted()
is made aware of token changed by its newisGrantedForUser()
method, so that calls tois_granted()
nested intois_granted_for_user()
calls will check the provided user, not the logged in one.IsGranted
's closures by 1. a newIsGrantedContext
and 2. the$subject
. This makes everything simpler, easier to discover, and more extensible. Thanks to the previous item, IsGrantedContext only needs the auth-checker, not the access-decision-manager anymore. Simpler again.