FriendsOfSymfony / FOSOAuthServerBundle

A server side OAuth2 Bundle for Symfony
1.09k stars 451 forks source link

Symfony 4.4.* / 5.0.* #639

Open elchris opened 4 years ago

elchris commented 4 years ago

See #625 starting here.

This pull request also features contributions from @iisisrael - Some more cleanups are still in progress.

If you wish to test this out against my branch in your project ahead of merging, consider looking at this comment by @patrickbussmann

napestershine commented 4 years ago

@elchris I guess FOS team is not having time, but is it possible to use your fork in project ? TIA

elchris commented 4 years ago

@elchris I guess FOS team is not having time, but is it possible to use your fork in project ? TIA

Yes it should be fine, see @patrickbussmann 's comment for how to do it.

napestershine commented 4 years ago

I still get error about: https://github.com/FriendsOfSymfony/FOSOAuthServerBundle/issues/638 while installing it.

elchris commented 4 years ago

I still get error about: #638 while installing it.

@patrickbussmann would you care to share a sample configuration and installation steps for @napestershine ?

@napestershine are you sure that you followed the steps that Patrick outlined here?

patrickbussmann commented 4 years ago

I still get error about: #638 while installing it.

@napestershine: You must read the error message.

The child node "db_driver" at path "fos_oauth_server" must be configured.

So please go to your file and add the db_driver. This is even specified in the documentation. https://github.com/FriendsOfSymfony/FOSOAuthServerBundle/blob/master/Resources/doc/index.md#step-5-configure-fosoauthserverbundle

# config/packages/fos_oauth_server.yaml
fos_oauth_server:
    db_driver: orm

Or when you not using ORM then specify something else - like written in the documentation.

ruudk commented 4 years ago

Any news on this?

deguif commented 3 years ago

@ruudk @elchris @napestershine many PRs were merged on master to remove deprecations. Symfony 5 support is not yet ready there but it's a matter of time.

@elchris could you rebase your PR?

With all the work merged, numbers of changes introduced in this PR should decrease.

elchris commented 3 years ago

@ruudk @elchris @napestershine many PRs were merged on master to remove deprecations. Symfony 5 support is not yet ready there but it's a matter of time.

@elchris could you rebase your PR?

With all the work merged, numbers of changes introduced in this PR should decrease.

yy..yeees I will try tonight. I may need some help if I run into some conflicts for which I'll be uncertain how to resolve but I'll give it a try :)

deguif commented 3 years ago

Symfony 5 is now supported on master.

Cocray commented 3 years ago

IMHO I think there's something missing in the OAuthStorage class: Password Migration (https://symfony.com/blog/new-in-symfony-4-4-password-migrations). function checkUserCredentials: if the password is valid then there must be a condition to check if a better hashing algorithm is available and rehashes the password. Here a code example from DaoAuthenticationProvider class (https://github.com/symfony/security-core/blob/master/Authentication/Provider/DaoAuthenticationProvider.php#L68):

if (!$encoder->isPasswordValid($user->getPassword(), $presentedPassword, $user->getSalt())) {
    throw new BadCredentialsException('The presented password is invalid.');
}

if ($this->userProvider instanceof PasswordUpgraderInterface && method_exists($encoder, 'needsRehash') && $encoder->needsRehash($user->getPassword())) {
    $this->userProvider->upgradePassword($user, $encoder->encodePassword($presentedPassword, $user->getSalt()));
}
MatthieuMan commented 3 years ago

@elchris any support for 5.3 with the new new Authenticator-based Security ?

Cannot configure AuthenticatorManager as "fos_oauth" authentication does not support it, set "security.enable_authenticator_manager" to false.

iisisrael commented 2 years ago

@elchris any support for 5.3 with the new new Authenticator-based Security ?

@MatthieuMan see #685 - it's based off of main, rather than this branch.