Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[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

Conversation

Simperfit
Copy link
Contributor

@Simperfit Simperfit commented Apr 17, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #26902
License MIT
Doc PR symfony/symfony-docs#11450

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.

@Simperfit Simperfit force-pushed the feature/add-a-new-event-when-the-user-has-been-logout-on-change branch 2 times, most recently from 1396daf to 452978a Compare April 17, 2019 07:05
@nicolas-grekas nicolas-grekas added this to the next milestone Apr 17, 2019
@Simperfit Simperfit force-pushed the feature/add-a-new-event-when-the-user-has-been-logout-on-change branch 2 times, most recently from 1129413 to 8f767ee Compare April 18, 2019 04:50
@Simperfit Simperfit marked this pull request as ready for review April 18, 2019 04:51
Copy link
Contributor

@linaori linaori left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea!

@Simperfit Simperfit force-pushed the feature/add-a-new-event-when-the-user-has-been-logout-on-change branch from 8f767ee to 36bb3b2 Compare April 18, 2019 14:34
@Simperfit
Copy link
Contributor Author

Status: Needs Review

@Simperfit Simperfit force-pushed the feature/add-a-new-event-when-the-user-has-been-logout-on-change branch from 36bb3b2 to b6ad28a Compare April 18, 2019 14:44
@Simperfit
Copy link
Contributor Author

Status: Needs Review

@Simperfit Simperfit force-pushed the feature/add-a-new-event-when-the-user-has-been-logout-on-change branch from b6ad28a to 5765f69 Compare April 18, 2019 14:44
@Simperfit
Copy link
Contributor Author

Thanks @linaori and @noniagriconomie for the review !

@noniagriconomie
Copy link
Contributor

noniagriconomie commented Apr 18, 2019

@Simperfit do not forget to update the doc accordingly when PR is finished :)
https://github.com/symfony/symfony-docs/pull/11450/files#diff-2f19b023cc3b9935394ee5a983d40cdcR286

@chalasr chalasr force-pushed the feature/add-a-new-event-when-the-user-has-been-logout-on-change branch 2 times, most recently from 95f1071 to ecdeab8 Compare April 26, 2019 21:16
@chalasr
Copy link
Member

chalasr commented Apr 26, 2019

Reverted my suggestion about allowing to reauthenticate the token from a listener, bad idea. Also renamed to DeauthenticatedEvent to save us from renaming the event in 5.0 as we want to get rid of the User concept (#30914).
Ready to go 👍

src/Symfony/Component/Security/CHANGELOG.md Outdated Show resolved Hide resolved
@xabbuh
Copy link
Member

xabbuh commented Apr 27, 2019

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?

@chalasr chalasr force-pushed the feature/add-a-new-event-when-the-user-has-been-logout-on-change branch 2 times, most recently from b901b0b to abec9c1 Compare April 27, 2019 13:38
@chalasr
Copy link
Member

chalasr commented Apr 27, 2019

@xabbuh 👍 Changed to getOriginalToken() and getDeauthenticatedToken().
Wording suggestions welcome (getRefreshedToken() vs getDeauthenticatedToken()?)

@Simperfit
Copy link
Contributor Author

I like getRefreshedToken() better :p

@chalasr chalasr force-pushed the feature/add-a-new-event-when-the-user-has-been-logout-on-change branch from abec9c1 to 40e4218 Compare April 27, 2019 14:12
@chalasr
Copy link
Member

chalasr commented Apr 27, 2019

Renamed

@Simperfit
Copy link
Contributor Author

Thanks for helping me finishing this @chalasr ;).

@chalasr
Copy link
Member

chalasr commented Apr 28, 2019

Thank you @Simperfit.

@chalasr chalasr merged commit 40e4218 into symfony:master Apr 28, 2019
chalasr pushed a commit that referenced this pull request Apr 28, 2019
…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
@Simperfit Simperfit deleted the feature/add-a-new-event-when-the-user-has-been-logout-on-change branch April 28, 2019 16:51
@@ -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
Copy link
Member

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?

Copy link
Member

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.
Copy link
Member

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

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
OskarStark added a commit to symfony/symfony-docs that referenced this pull request May 7, 2019
…" 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
@fabpot fabpot mentioned this pull request May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.