hwi / HWIOAuthBundle

OAuth client integration for Symfony. Supports both OAuth1.0a and OAuth2.
MIT License
2.27k stars 800 forks source link

Symfony 5 Support #1581

Closed JoppeDC closed 4 years ago

JoppeDC commented 4 years ago

This should be as easy as bumping the composer.json requirements, but it would be the best if someone can test this to make sure there are no issues.

JoppeDC commented 4 years ago

Turns out friendsofsymfony/user-bundle does also not support SF5 yet

Chrysweel commented 4 years ago

I think is required solve warnings with symfony 4.3.

There are one issue open: https://github.com/hwi/HWIOAuthBundle/issues/1567

wucdbm commented 4 years ago

The "Symfony\Component\Config\Definition\Builder\TreeBuilder::root()" method called for the "hwi_oauth" configuration is deprecated since Symfony 4.3, pass the root name to the constructor instead.

Here's another one. Fix is simple, but it requires either 1) drop support for ~3.x <4.3 2) some weird if-chains

$builder = new TreeBuilder();
$rootNode = $builder->root('hwi_oauth');
$builder = new TreeBuilder('hwi_oauth');
$rootNode = $builder->getRootNode();

Update: This seems done in 1.0.0, so no longer relevant

wucdbm commented 4 years ago

For the sake of keeping it in the same thread:

The "Symfony\Bundle\FrameworkBundle\Controller\ControllerTrait" trait is considered internal. It may change without further notice. You should not use it from "HWI\Bundle\OAuthBundle\Controller\ConnectController".

I could refactor that, are there any specific requirements that I should follow?

dmaicher commented 4 years ago

See https://github.com/hwi/HWIOAuthBundle/pull/1592

I currently don't have any project using this bundle and Symfony 5 yet... can anyone test my PR by any chance?

wucdbm commented 4 years ago

I'll be upgrading one to 5 this or next week, I'll keep this updated in case of any issues. May take some time as it's a 3.4 -> 5.0 migration.

theedov commented 4 years ago

Turns out friendsofsymfony/user-bundle does also not support SF5 yet

I don't think user-bundle will support symfony5 https://github.com/FriendsOfSymfony/FOSUserBundle/issues/2874

carlos-mg89 commented 4 years ago

@dmaicher could you help me to test that PR? My project is on Symfony 5.0.1 and I'm willing to give it a try, since I'm precisely introducing Facebook and Google login into this project.

dmaicher commented 4 years ago

@carlos-mg89 since my PR was merged you can just require dev-master or 1.1-dev via composer to test this.

Not sure how else I can help you :blush:

carlos-mg89 commented 4 years ago

Yeah! I managed to made it work by telling composer to get a concrete commit from master (the last one). I faced other issues but managed to continue almost to the end.

Right now I'm facing this issue:

Uncaught PHP Exception RuntimeException: "No property defined for entity for resource owner 'facebook'."

Which happens here:

/vendor/hwi/oauth-bundle/Security/Core/User/EntityUserProvider.php line 93

I understand that the issue could be related with the fact that my User entity has no 'facebook' property??? So I'm gonna try to add it to the Entity and to the DB table.

Would that be correct?

carlos-mg89 commented 4 years ago

By the way @dmaicher this is my security.yaml:

oauth:
    resource_owners:
        facebook:          facebook_login
    login_path:        /login
    use_forward:       false
    failure_path:      /login

    oauth_user_provider:
        orm:
            class: App\Entity\User
            properties: ~

I'm starting to think, by checking the code, that the issue is in the properties field in this file...

carlos-mg89 commented 4 years ago

I finally got it right and is working very nicely with Symfony 5.0.1 :)

My issue was definitely with the UserProvider, and since I wasn't able to make it work with the one from the HWIOAuthBundle (HWI\Bundle\OAuthBundle\Security\Core\User\EntityUserProvider) then I opted to create one following these tips: https://stackoverflow.com/questions/18264800/symfony2-how-to-login-using-oauth-hwioauthbundle-custom-roles-by-default-a

However, I believe it'd be much better to use the EntityUserProvider. But I haven't found proper documentation about how to enabe it to work with the different OAuth Providers.

Thanks for your PR by the way. And if you happen to know about enabling EntityUserProvider, please, let me know. I'm keen to learn how to enable it.

dmaicher commented 4 years ago

@carlos-mg89 nice. Thanks for the info.

@XWB so we could close this issue and maybe tag a new release? :blush:

carlos-mg89 commented 4 years ago

I have to say, I had troubles with the Flex recipe. However, I did what I read and I manually added the commit reference to the composer.json, since the only thing that was wrong is that it got removed from the composer.json after getting installed.

Aside from this, the issues I faced where about configuration, not problems with Symfony 5.

wucdbm commented 4 years ago

Sorry for the delay.

Can confirm it works on 5.0.4. Not in production yet, but it does work.

Other than that, we have a custom implementation on one of the interfaces, that works fine too, so nothing broken for us.

@carlos-mg89

# security.yaml
        some_area:
            pattern: ^/something
            oauth:
                resource_owners:
                        # ...
                oauth_user_provider:
                    # Your own user provider service that implements
                    # HWI\Bundle\OAuthBundle\Security\Core\User\OAuthAwareUserProviderInterface
                    service: app.auth.user_provider

In our project we have a customer oauth user provider as we also need users updated with the account IDs, in case Facebook flips a middle finger and randomly denies access to the email. That makes it more robust, and we have more control over how users are loaded. Hope that helps. It also allows us to sign in users via social networks and ask for the email later.

EDIT: Can confirm it works well in production.

ThomasVandenabeele commented 4 years ago

Any news on when there will be a new release for Symfony 5? I'm trying to install this using: composer require hwi/oauth-bundle. This gives me unmet dependency for the symfony framework-bundle as it is not included in the latest release. I tried installing it using: composer require hwi/oauth-bundle:dev-master. However, this gave me the following error: Script cache:clear returned with error code 1 !!
!! In CheckExceptionOnInvalidReferenceBehaviorPass.php line 86: !!
!! The service "HWI\Bundle\OAuthBundle\Controller\ConnectController" has a dep
!! endency on a non-existent service "hwi_oauth.resource_ownermap.main".
!!
!!
!!
Script @auto-scripts was called via post-update-cmd

Is there some other way using composer to include this into my symfony 5 project (I'm a symfony beginner).

wucdbm commented 4 years ago

@ThomasVandenabeele Add a line to your composer.json manually and run composer update

This is most likely due to an error in the configuration. On our end, it's locked to "hwi/oauth-bundle": "dev-master#b042ddd" and it works just fine.

As far as I remember, owner map services are generated based on your hwi_oauth firewall_names config

wimvds commented 4 years ago

@wucdbm On a cleanly created Symfony 4.4 project that doesn't seem to work, we're still getting the error ...

composer require hwi/oauth-bundle:dev-master#b042ddd php-http/guzzle6-adapter php-http/httplug-bundle Using version ^2.0 for php-http/guzzle6-adapter Using version ^1.17 for php-http/httplug-bundle ./composer.json has been updated Loading composer repositories with package information Updating dependencies (including require-dev) Restricting packages listed in "symfony/symfony" to "4.4.*" Package operations: 0 installs, 1 update, 0 removals

  • Installing hwi/oauth-bundle (dev-master b042ddd): Downloading (100%)
    Writing lock file Generating autoload files ocramius/package-versions: Generating version class... ocramius/package-versions: ...done generating version class Executing script cache:clear [KO] [KO] Script cache:clear returned with error code 1 !!
    !! In CheckExceptionOnInvalidReferenceBehaviorPass.php line 86: !!
    !! The service "HWI\Bundle\OAuthBundle\Controller\ConnectController" has a dep
    !! endency on a non-existent service "hwi_oauth.resource_ownermap.main".
    !!
    !!
    !!
    Script @auto-scripts was called via post-update-cmd

Installation failed, reverting ./composer.json to its original content.

I only succeeded in getting a working version by running the install

composer require hwi/oauth-bundle php-http/guzzle6-adapter php-http/httplug-bundle

which fails with

The service "HWI\Bundle\OAuthBundle\Controller\ConnectController" has a dependency on a non-existent service "hwi_oauth.resource_ownermap.main".

But this still leaves the config files, so you can then start modifying config/packages/hwi_oauth.yaml (and config/packages/security.yaml) to wire everything and afterwards run the composer require again. Then - if all dependencies have been configured correctly - it should stop complaining.

wucdbm commented 4 years ago

composer require hwi/oauth-bundle:dev-master#b042ddd php-http/guzzle6-adapter php-http/httplug-bundle This is where your problem is, because after executing its recipe, it creates a blank config file (?? if i remember correctly) and expects it to be properly configured (which it isn't), after which it fails because it's not and reverts your composer.json to its original state, so you can not continue from there fixing the issue.

Instead, add the requirements in your require in composer.json manually, then run composer update. Let it update, then error out @ cache:clear, but those will be installed and you will then be able to modify the configs & make sure it works. Had the same problem upgrading another project and installing flex and all of its dependencies afterwards. Basically, Symfony Flex creates this mess. I had the same issue with other packages. But this is no big deal, just quite annoying if it happens.

EDIT: You should really go for v1.0.0 if you're using 4.4. We're using master @ that commit only because we upgraded to Symfony 5 and composer won't let us use 1.0.0. No guarantee something won't break at some point for us, but I'll definitely open a PR if we have any issues with the bundle on Sf v5.0.0.

wucdbm commented 4 years ago

@dmaicher Should we head towards tagging a version with 5.0 support or wait for additional feedback from other Symfony 5.0 projects?

dmaicher commented 4 years ago

@dmaicher Should we head towards tagging a version with 5.0 support or wait for additional feedback from other Symfony 5.0 projects?

Tagging a new release seems fine for me. We have to wait for the maintainers :wink:

mateuszmalecki commented 4 years ago

@wucdbm On a cleanly created Symfony 4.4 project that doesn't seem to work, we're still getting the error ...

composer require hwi/oauth-bundle:dev-master#b042ddd php-http/guzzle6-adapter php-http/httplug-bundle Using version ^2.0 for php-http/guzzle6-adapter Using version ^1.17 for php-http/httplug-bundle ./composer.json has been updated Loading composer repositories with package information Updating dependencies (including require-dev) Restricting packages listed in "symfony/symfony" to "4.4.*" Package operations: 0 installs, 1 update, 0 removals

  • Installing hwi/oauth-bundle (dev-master b042ddd): Downloading (100%) Writing lock file Generating autoload files ocramius/package-versions: Generating version class... ocramius/package-versions: ...done generating version class Executing script cache:clear [KO] [KO] Script cache:clear returned with error code 1 !! !! In CheckExceptionOnInvalidReferenceBehaviorPass.php line 86: !! !! The service "HWI\Bundle\OAuthBundle\Controller\ConnectController" has a dep !! endency on a non-existent service "hwi_oauth.resource_ownermap.main". !! !! !! Script @auto-scripts was called via post-update-cmd

Installation failed, reverting ./composer.json to its original content.

I only succeeded in getting a working version by running the install

composer require hwi/oauth-bundle php-http/guzzle6-adapter php-http/httplug-bundle

which fails with

The service "HWI\Bundle\OAuthBundle\Controller\ConnectController" has a dependency on a non-existent service "hwi_oauth.resource_ownermap.main".

But this still leaves the config files, so you can then start modifying config/packages/hwi_oauth.yaml (and config/packages/security.yaml) to wire everything and afterwards run the composer require again. Then - if all dependencies have been configured correctly - it should stop complaining.

Do you fix this? I have the same problem with Symfony5

NicoHaase commented 4 years ago

@XWB are there any news about this? Or is there anything open that needs help to get a new release tagged?

adrienfr commented 4 years ago

Hi @XWB, could we help you with anything to release a new tag?

stephanvierkant commented 4 years ago

This isn't the first time it took ages before a new version got tagged. I see @XWB hasn't been active for a while. Are there any other maintainers we can ping?

Dear maintainers, please create a new release ASAP. And for future releases, please create tags more frequently as it helps this bundle forward. If you're looking for more maintainers, please let us know.

XWB commented 4 years ago

A new release is now available: https://github.com/hwi/HWIOAuthBundle/releases/tag/1.1.0

dmaicher commented 4 years ago

@XWB the new tag does not show up on packagist though:

https://packagist.org/packages/hwi/oauth-bundle

EDIT: I just pressed the "update button" in the bottom right. Seems to be there now :smile:

HeyIAmBob commented 4 years ago

Hi, It seems that the error The service "HWI\Bundle\OAuthBundle\Controller\ConnectController" has a dependency on a non-existent service "hwi_oauth.resource_ownermap.main". is still here if you allow the execution of the recipe. It turns out that the recipe is this one : https://github.com/symfony/recipes-contrib/tree/master/hwi/oauth-bundle/0.6

When you refuse to execute the recipe it works pretty well (with manual config).

shodanto commented 4 years ago

The service "HWI\Bundle\OAuthBundle\Controller\ConnectController" has a dependency on a non-existent service "hwi_oauth.resource_ownermap.main".

The problem is the default firewall name in hwi_oauth.yml. firewall_names: [main] Change it to your firewall name.

SekmSet commented 4 years ago

still doesn't work with S5 ?

XWB commented 4 years ago

Symfony 5 support was added months ago.

SekmSet commented 4 years ago

@XWB Thank You ! I tried to use this bundle but I had mistake/error (like @mateuszmalecki ) :(

heather817 commented 4 years ago

The service "HWI\Bundle\OAuthBundle\Controller\ConnectController" has a dependency on a non-existent service "hwi_oauth.resource_ownermap.main".

The problem is the default firewall name in hwi_oauth.yml. firewall_names: [main] Change it to your firewall name.

I have a fresh installation of symfony 5, and am getting this error. The main firewall is set up by symfony, and I have not changed it. There must be something else missing from the recipe.

playerro commented 3 years ago

I have a fresh installation of symfony 5, and am getting this error. The main firewall is set up by symfony, and I have not changed it. There must be something else missing from the recipe.

Same here. Anybody can help?

playerro commented 3 years ago

@XWB can you help with this error please?

wucdbm commented 3 years ago

@playerro add your composer require manually to your require section, run composer update, let it error out and then edit the configuration manually Once your config is fine, you should be good to go

See https://github.com/hwi/HWIOAuthBundle/issues/1581#issuecomment-596031362