NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Security] Allow switching to another user when already switched#35839

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

Jump to bottom

Merged

nicolas-grekas merged 1 commit intosymfony:3.4fromchalasr:sec-multi-switchuser

Feb 26, 2020

Merged

[Security] Allow switching to another user when already switched#35839

nicolas-grekas merged 1 commit intosymfony:3.4fromchalasr:sec-multi-switchuser

Feb 26, 2020

Conversation

Copy link

Member

chalasr commentedFeb 23, 2020•
edited
Loading

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix#34690
License	MIT
Doc PR	-

carsonbot added Status: Needs Review Security Feature labels

Feb 23, 2020

chalasr added this to thenext milestone

Feb 23, 2020

chalasr force-pushed thesec-multi-switchuser branch 2 times, most recently from07d4598 toee219efCompare

February 23, 2020 22:58

noniagriconomie reviewed

Feb 24, 2020

View reviewed changes

src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml OutdatedShow resolvedHide resolved

noniagriconomie approved these changes

Feb 24, 2020

View reviewed changes

Copy link

Contributor

noniagriconomie left a comment

Choose a reason for hiding this comment

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

thx

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Feb 24, 2020

ogizanagi reviewed

Feb 24, 2020

View reviewed changes

src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php OutdatedShow resolvedHide resolved

ogizanagi reviewed

Feb 24, 2020

View reviewed changes

src/Symfony/Component/Security/Core/Exception/AlreadySwitchedException.php OutdatedShow resolvedHide resolved

ogizanagi reviewed

Feb 24, 2020

View reviewed changes

src/Symfony/Bundle/SecurityBundle/CHANGELOG.md OutdatedShow resolvedHide resolved

chalasr force-pushed thesec-multi-switchuser branch from91d8855 to14199ddCompare

February 24, 2020 08:23

wouterj approved these changes

Feb 24, 2020

View reviewed changes

Copy link

Member

wouterj left a comment

Choose a reason for hiding this comment

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

Great, thanks!

Copy link

Member

nicolas-grekas commentedFeb 24, 2020

What would be the drawback of always allowing the switch? Adding a new option should be thought twice.

Copy link

Contributor

ogizanagi commentedFeb 24, 2020

We've discussed a bit about this with@chalasr. It's not easy to provide a satisfying default out-of-the-box
to show an explanation about the error to the user anyway. I'd be in favor of always allowing the switch, and this could even be considered a bug fix to me.