scheb / two-factor-bundle

[ABANDONED] Two-factor authentication for Symfony 2 & 3 applications 🔐. Please use the newer versions from https://github.com/scheb/2fa.
https://github.com/scheb/2fa
MIT License
385 stars 111 forks source link

Symfony >4.3 compatibility layer conflicts #263

Closed scheb closed 4 years ago

scheb commented 4 years ago

Bundle version: >= 4.6.0 Symfony version: < 4.4

Original message by @PierrickMartos in #233

Hi @scheb there is a conflict with the bundle named Sentry, since their version 3.3.0 they create an alias for RequestEvent to prepare Symfony 5.x : https://github.com/getsentry/sentry-symfony/pull/266/files#diff-80a02960cf5e92e3f2df25a1f0f103f4R15

Do you think we can check the current symfony version by another way than relying on RequestEvent class?

scheb commented 4 years ago

Hmm, tricky issue 🤔

Might look selfish, but the fact that other bundles will have the same issue (thanks @maxhelias for finding that), I tend to say the solution from the Sentry bundle may be problem here. Because using class_alias may be a clever solution on the first view, but effectively it creates a class name in a foreign namespace, one that should be under control of Symfony. And this change becomes globally visible to everything in that PHP runtime context.

If you want to use class_alias for it, I think a better way would be instead of creating an alias in the foreign Symfony namespace, create a "proxy alias" in the local namespace. One that points either to RequestEvent or GetResponseEvent.

scheb commented 4 years ago

If you want to use class_alias for it, I think a better way would be instead of creating an alias in the foreign Symfony namespace, create a "proxy alias" in the local namespace. One that points either to RequestEvent or GetResponseEvent.

Looks like FriendsOfSymfony/FOSHttpCacheBundle is doing exactly that, with an alias in the local namespace:

https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/master/src/EventListener/UserContextListener.php#L31-L37

scheb commented 4 years ago

^ They're working on it.

I expect the next Sentry release will solve this issue. Therefore closing this issue here.

scheb commented 4 years ago

Sentry version >= 3.4.1 solves the problem.

PierrickMartos commented 4 years ago

Awesome, thanks!