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

Update Security for Symfony 6.2 deprecation notice #116

Closed
wants to merge 1 commit into from

Conversation

mogilvie
Copy link

Resolve issue #115

@@ -57,7 +57,7 @@
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\Security\Core\Security;
use Symfony\Bundle\SecurityBundle\Security;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a proper solution. The bundle currently supports >= Symfony 5.3, but this new class was only introduced in Symfony 6.2 so this PR would actually break the bundle for all apps that are not on Symfony 6.2

@RobertMe
Copy link
Contributor

RobertMe commented Mar 7, 2023

In addition to the comment of @X-Coder264 this fix would also break when Symfony actually removes the Security helper in the component (so with Symfony 7.0). This as the AuthorizationRequestUserResolvingListener still uses Symfony\Component\Security\Core\Security which will then obviously be gone. To fix the deprecation this class should thus be changed as well. (If this bundle only supported PHP >= 8 I think using a union could have fixed it, but as this bundle supports PHP >= 7.2 a union can't be used. So this needs to conditionally switch between two implementations for which one uses the Security helper of the bundle and the other the helper of the component).

@mogilvie
Copy link
Author

mogilvie commented Mar 7, 2023

At which point are you going to consider dropping symfony 5.3 and php 7 with a new backward breaking release?

From other pull request comments, looks like November 2024 possibly.

@mogilvie mogilvie closed this Mar 7, 2023
@RobertMe
Copy link
Contributor

RobertMe commented Mar 8, 2023

Based on the fact that @mogilvie closed this PR I've created a new PR utilizing my proposed solution: #129

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants