Skip to content
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

[SecurityBundle] Fix the session listener registration under the new authentication manager #37126

Merged
merged 1 commit into from Jun 8, 2020

Conversation

johnvandeweghe
Copy link
Contributor

Q A
Branch? 5.1
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #37119
License MIT
Doc PR N/A

Fixes the logic that adds session listeners for firewalls to properly add them only for statefull firewalls. Adds tests to confirm that it is only added to statefull ones. Also remove unused abstract field on session listener

Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Looks (almost 😉) perfect! Thanks for your quick reactions & testing the new system!

@@ -60,10 +60,8 @@
</service>

<service id="security.listener.session"
class="Symfony\Component\Security\Http\EventListener\SessionStrategyListener"
abstract="true">
Copy link
Member

@wouterj wouterj Jun 7, 2020

Choose a reason for hiding this comment

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

this abstract="true" must be kept (the service itself should not be used, only the ones created based on this in the SecurityExtension)

@fabpot
Copy link
Member

fabpot commented Jun 8, 2020

Thank you @johnvandeweghe.

@fabpot fabpot merged commit 9760d37 into symfony:5.1 Jun 8, 2020
@fabpot fabpot mentioned this pull request Jun 12, 2020
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.

None yet

5 participants