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] Allow disabling redirect on logout#46320
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
1bbe1d9
to799d8ae
Compare799d8ae
to9a52fd4
Comparecarsonbot commentedMay 12, 2022
Hey! I think@scheb has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
9a52fd4
to4077513
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.
Hi@jvasseur,
Thank you for the proposed changes. This looks good to me.
Just a few changes rebase and we are good to go.
@@ -8,6 +8,7 @@ CHANGELOG | |||
* Deprecate the `Symfony\Component\Security\Core\Security` service alias, use `Symfony\Bundle\SecurityBundle\Security\Security` instead | |||
* Add `Security::getFirewallConfig()` to help to get the firewall configuration associated to the Request | |||
* Add `Security::login()` to login programmatically | |||
* Allow disabling the redirection on successful logout by passing `null` to the `target` option |
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.
Should move to 7.3
👍 from me also, once rebased and retargeted. |
Uh oh!
There was an error while loading.Please reload this page.
When using JSON authentication, customizing the success response can be done easily in the corresponding controller, but customizing the logout success is more tiresome and requires to create a listener for it (while still requiring to create a route for it to work).
This PR propose to add the possibility to disable the redirect on successful logout and let the request fallback to the defined route in this case.
This allow customizing both login and logout response in the same place like this:
It means I had to remove the Exception in case no logout listener set a response but I don't think it should be considered a BC break since it's an error case that is removed and the corresponding exception was just the base
RuntimeException
and thus wasn't specifically catchable.The only downside is that other logout listeners can't modify the response anymore because it doesn't exists in the event but since this is an opt-in feature I don't think it's a problem.