-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Security] Dispatch an event when "logout user on change" steps in #31138
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
[Security] Dispatch an event when "logout user on change" steps in #31138
Conversation
1396daf
to
452978a
Compare
1129413
to
8f767ee
Compare
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 idea!
src/Symfony/Component/Security/Http/Event/LogoutUserChangeEvent.php
Outdated
Show resolved
Hide resolved
8f767ee
to
36bb3b2
Compare
Status: Needs Review |
src/Symfony/Component/Security/Http/Event/LogoutOnChangeEvent.php
Outdated
Show resolved
Hide resolved
36bb3b2
to
b6ad28a
Compare
Status: Needs Review |
b6ad28a
to
5765f69
Compare
Thanks @linaori and @noniagriconomie for the review ! |
@Simperfit do not forget to update the doc accordingly when PR is finished :) |
95f1071
to
ecdeab8
Compare
Reverted my suggestion about allowing to reauthenticate the token from a listener, bad idea. Also renamed to |
If we also do not want to rely on the user concept, would it make sense to pass the old and the refreshed token instead of the user? |
b901b0b
to
abec9c1
Compare
@xabbuh 👍 Changed to |
I like |
abec9c1
to
40e4218
Compare
Renamed |
Thanks for helping me finishing this @chalasr ;). |
Thank you @Simperfit. |
…ge" steps in (Simperfit) This PR was merged into the 4.3-dev branch. Discussion ---------- [Security] Dispatch an event when "logout user on change" steps in | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes <!-- don't forget to update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #26902 <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#11450 <!-- required for new features --> <!-- Write a short README entry for your feature/bugfix here (replace this comment block.) This will help people understand your PR and can be used as a start of the Doc PR. Additionally: - Bug fixes must be submitted against the lowest branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the master branch. --> This adds a new event when the user has been changed and has been log out from the apps, it allow someone to register to this event and do something with either to token or the refreshedUser. Commits ------- 40e4218 [Security] Dispatch an event when "logout user on change" steps in
@@ -23,6 +23,7 @@ CHANGELOG | ||
* Dispatch `SwitchUserEvent` on `security.switch_user` | ||
* Deprecated `Argon2iPasswordEncoder`, use `SodiumPasswordEncoder` instead | ||
* Deprecated `BCryptPasswordEncoder`, use `NativePasswordEncoder` instead | ||
* Added `DeauthenticatedEvent` dispatched in case the user has changed when trying to refresh it |
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.
What is "it" here? The user?
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.
I think "it" refers to the Token here. What about "Added DeauthenticatedEvent
dispatched in case the user has changed when trying to refresh the token"?
use Symfony\Contracts\EventDispatcher\Event; | ||
|
||
/** | ||
* Deauthentication happens in case the user has changed when trying to refresh it. |
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.
Sentence should be changed as well
…" steps in (Simperfit) This PR was merged into the master branch. Discussion ---------- [Security] Dispatch an event when "logout user on change" steps in <!-- If your pull request fixes a BUG, use the oldest maintained branch that contains the bug (see https://symfony.com/roadmap for the list of maintained branches). If your pull request documents a NEW FEATURE, use the same Symfony branch where the feature was introduced (and `master` for features of unreleased versions). --> This documents the new event implemented in symfony/symfony#31138 Commits ------- b5e6038 [Security] Dispatch an event when "logout user on change" steps in
This adds a new event when the user has been changed and has been log out from the apps, it allow someone to register to this event and do something with either to token or the refreshedUser.