Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Security] Add a Not_Full_Fledged_handler in ExceptionListener from security login#57661
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
base:7.4
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
64b0bdd
to7e36014
Compare7e36014
toa7ace88
Comparebdb9552
tof64b805
Comparesrc/Symfony/Component/Security/Http/Authorization/SameAsNotFullFledgedHandle.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/Tests/Firewall/ExceptionListenerTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
a5ff172
to05ca8dd
CompareThere 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.
I'm not convinced by this PR. To me, it does not actually solve the issue it claims to solve (it solves only a subset of cases, bringing aninvalid behavior for the other cases).
So for now, it gets a -1 vote from me.
src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/Authorization/NotFullFledgedHandlerInterface.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/Authorization/NotFullFledgedHandlerInterface.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/Authorization/SameAsNotFullFledgedHandler.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
} | ||
foreach($accessDeniedException->getAttributes() as $attribute) { | ||
if(in_array($attribute, [AuthenticatedVoter::IS_AUTHENTICATED_FULLY])) { |
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.
This is not enough to identify all cases where an access denied exception might depend on the trust level of a token (see my comment on the issue).
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.
it totally a case whitch can be set in a custom handler.
SameAsNotFullFledgedHandler is here to anwser to many person, when PR will be merged (yes I beleive on it) if many persons have the same idea it can be possible to propose another PR for add core handlers
src/Symfony/Component/Security/Http/Authorization/NotFullFledgedHandlerInterface.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/Tests/Firewall/ExceptionListenerTest.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
I don't undestand why you say it does not actually solve the issue. the problem is to obtain a control of AccessDenied Exception when RememberMe login is active, this PR allow a custom handler to choose:
I don't see whitch case is not cover ? |
@eltharin have you read my comment on the issue ? |
I do but I don't understand where you find a problem, |
src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
60cb97e
to2c019ce
Compare818ff65
to91dc0c2
Comparesrc/Symfony/Component/Security/Http/Authorization/NotFullFledgedHandlerInterface.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
0815414
to3e1d1ea
Compare3e1d1ea
toa96aecb
CompareWhat can we do to advance this PR ? |
if not authenticated at alluse callback instead boolean
add r to handleR
…Extension.phpCo-authored-by: Oskar Stark <oskarstark@googlemail.com>
a96aecb
to0fb0b24
Compare
Uh oh!
There was an error while loading.Please reload this page.
My proposal for behaviour for acces Denied When User is logged with a remeberme token
Without set anything, the behaviour is the same as actually.
In the Firewall config adding a not_full_fledged_handler waiting a service handler or a predefined value :
"original""redirect" for the same behavior than actual replaced bySymfony\Component\Security\Http\Authorization\NotFullFledgedRedirectToStartAuthenticationHandler
"same""equal" for if a user is logged but not not full fledged (and the attribute is not IS_AUTHENTICATED_FULLY) he has the same acces than if he was full logged. this value is replaced bySymfony\Component\Security\Http\Authorization\NotFullFledgedEqualNormalLoginHandler
Symfony\Component\Security\Http\Authorization\NotFullFledgedHandlerInterface
with one method "handle"this method can return a reponse to change original Exception Response or null for continue to throw the original access denied exception.