Security manager update

[ricohumme]

This PR enables users wanting to use the authentication manager introduced in Symfony 5.3.
Tests are as of yet failing due the fact I'm unaware how to do this because this feature is enabled via a config setting in the symfony security component.
If anyone is aware of such methods, please enlighten me.

This fix can be applied for:
#291, #289, #286 and #249

[VoodooPrograms]

Hey, I wanted to now if anything will be happening on this PR or it will be in short time merged to release branch?

[hafkenscheid]

Any news (or help needed) regarding this PR?

[ricohumme]

@hafkenscheid help would be great, especially with testing in different environments of Symfony, e.g. with and without using the new authenticator system

[nathanjrobertson]

With Symfony 6 due this month, the old guard authenticators will disappear. This PR (or something like it) will need to be merged for oauth2-bundle to work with Symfony 6.

[XartIrok]

ETA for accepting this pull request?

[hafkenscheid]

We have been testing today with symfony 5.2.14 and symfony 5.3.12, both with the old security system. The good news is that the old security system seems still fine, but we cannot get this to work with the new authentication system on symfony 5.3.12.

How is it supposed to be implemented in security.yaml?

We tried adding Trikoder\Bundle\OAuth2Bundle\Security\Guard\Authenticator\OAuth2Authenticator as a second custom authenticator in our apps, but that does not work. Also adding a second firewall with the authenticator does not do the job.

[hafkenscheid]

We succeeded implementing this in SF5.3.12 using the new security system. Will continue testing.

[XartIrok]

We succeeded implementing this in SF5.3.12 using the new security system. Will continue testing.

great news

[hafkenscheid]

Managed to get it working on SF5.4.0. Still many deprecations:

Method "Symfony\Component\DependencyInjection\Extension\Extension::getAlias()" might add "string" as a native return type declaration in the future. Do the same in child class "Trikoder\Bundle\OAuth2Bundle\DependencyInjection\TrikoderOAuth2Extension" now to avoid errors or add an explicit @return annotation to suppress this message.

The "Trikoder\Bundle\OAuth2Bundle\DependencyInjection\Security\OAuth2Factory" class implements "Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface" that is deprecated since Symfony 5.3, use AuthenticatorFactoryInterface instead.

Method "Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface::create()" might add "array" as a native return type declaration in the future. Do the same in implementation "Trikoder\Bundle\OAuth2Bundle\DependencyInjection\Security\OAuth2Factory" now to avoid errors or add an explicit @return annotation to suppress this message.

Method "Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface::getPosition()" might add "string" as a native return type declaration in the future. Do the same in implementation "Trikoder\Bundle\OAuth2Bundle\DependencyInjection\Security\OAuth2Factory" now to avoid errors or add an explicit @return annotation to suppress this message.

Method "Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface::getKey()" might add "string" as a native return type declaration in the future. Do the same in implementation "Trikoder\Bundle\OAuth2Bundle\DependencyInjection\Security\OAuth2Factory" now to avoid errors or add an explicit @return annotation to suppress this message.

// vendor/trikoder/oauth2-bundle/TrikoderOAuth2Bundle.php line 39
Since symfony/security-bundle 5.4: Method "Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension::addSecurityListenerFactory()" is deprecated, use "addAuthenticatorFactory()" instead.

Since symfony/dependency-injection 5.1: Not setting the attribute "package" of the node "deprecated" in "/var/www/html/vendor/trikoder/oauth2-bundle/DependencyInjection/../Resources/config/services.xml" is deprecated.

Since symfony/dependency-injection 5.1: Not setting the attribute "version" of the node "deprecated" in "/var/www/html/vendor/trikoder/oauth2-bundle/DependencyInjection/../Resources/config/services.xml" is deprecated.

Since symfony/dependency-injection 5.1: Not setting the attribute "package" of the node "deprecated" in "/var/www/html/vendor/trikoder/oauth2-bundle/DependencyInjection/../Resources/config/storage/doctrine.xml" is deprecated.

Since symfony/dependency-injection 5.1: Not setting the attribute "version" of the node "deprecated" in "/var/www/html/vendor/trikoder/oauth2-bundle/DependencyInjection/../Resources/config/storage/doctrine.xml" is deprecated.

Method "Doctrine\DBAL\Types\Type::convertToDatabaseValue()" might add "mixed" as a native return type declaration in the future. Do the same in child class "Trikoder\Bundle\OAuth2Bundle\DBAL\Type\ImplodedArray" now to avoid errors or add an explicit @return annotation to suppress this message.

Method "Doctrine\DBAL\Types\Type::convertToPHPValue()" might add "mixed" as a native return type declaration in the future. Do the same in child class "Trikoder\Bundle\OAuth2Bundle\DBAL\Type\ImplodedArray" now to avoid errors or add an explicit @return annotation to suppress this message.

Method "Doctrine\DBAL\Types\Type::getSQLDeclaration()" might add "string" as a native return type declaration in the future. Do the same in child class "Trikoder\Bundle\OAuth2Bundle\DBAL\Type\ImplodedArray" now to avoid errors or add an explicit @return annotation to suppress this message.

Method "Doctrine\DBAL\Types\Type::requiresSQLCommentHint()" might add "bool" as a native return type declaration in the future. Do the same in child class "Trikoder\Bundle\OAuth2Bundle\DBAL\Type\ImplodedArray" now to avoid errors or add an explicit @return annotation to suppress this message.

Method "Doctrine\DBAL\Types\Type::getName()" might add "string" as a native return type declaration in the future. Do the same in child class "Trikoder\Bundle\OAuth2Bundle\DBAL\Type\Grant" now to avoid errors or add an explicit @return annotation to suppress this message.

Method "Doctrine\DBAL\Types\Type::getName()" might add "string" as a native return type declaration in the future. Do the same in child class "Trikoder\Bundle\OAuth2Bundle\DBAL\Type\RedirectUri" now to avoid errors or add an explicit @return annotation to suppress this message.

Method "Doctrine\DBAL\Types\Type::getName()" might add "string" as a native return type declaration in the future. Do the same in child class "Trikoder\Bundle\OAuth2Bundle\DBAL\Type\Scope" now to avoid errors or add an explicit @return annotation to suppress this message.

Method "League\OAuth2\Server\Entities\ClientEntityInterface::getName()" might add "string" as a native return type declaration in the future. Do the same in implementation "Trikoder\Bundle\OAuth2Bundle\League\Entity\Client" now to avoid errors or add an explicit @return annotation to suppress this message.

[ricohumme]

Thanks for testing all of this.
this was not tested with 5.4 as it was not out yet at the time.
I could have a peek at it again with 5.4 and try and resolve some of the deps in this bundle. Deps outside of this bundle I can’t solve right now

[ricohumme]

I would like some feedback from the maintainers by now though. To make sure another PR is not already taking care of the deprecations in 5.4
ping @X-Coder264

[X-Coder264]

@ricohumme Thanks for the PR. You've targeted the v3 branch with this PR, but it looks like you've originally branched from master so there's a bunch of breaking changes in the diff here that were commited to master and which were never supposed to be in v3. So either you wanted your changes to go into master and you've picked the wrong target branch for this PR or you actually wanted to make this PR to target v3 and you've just branched off from the wrong branch. Please fix this so that the PR diff is not such a mess so that the PR can actually be reviewed.

On the other hand the general status of this bundle is that it's (semi)abandoned as we've decided with the Symfony/PHP League guys to move it under their umbrella for better visibility/maintenance -> so https://github.com/thephpleague/oauth2-server-bundle was created.

As soon as version 1.0 of that bundle gets tagged we'll probably officially abandon this bundle so I encourage you to upgrade to that bundle instead (that bundle already got support for the new authentication manager stuff as well as Symfony 6 support).

TLDR; The PR diff is currently messy and I'm not sure if it's actually worth it to fix it as active development switched over to the new Symfony PHP League bundle, especially if this PR ends up having breaking changes as there are no plans currently to tag a v4 release.

[hafkenscheid]

Aah, we were not aware of this move, otherwise we would have started using/testing/contributing there.
Thanks for mentioning!

[toxicity1985]

Maybe you have to add update of the readme file so people can know that there are a new repository.

[dsiemensma-move]

@X-Coder264 I've created #304 for you to reflect this project's status and future plans