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

Symfony 4.4.* / 5.0.* #639

Open
wants to merge 63 commits into
base: master
Choose a base branch
from
Open

Conversation

elchris
Copy link

@elchris elchris commented Feb 14, 2020

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

scootstah and others added 21 commits May 24, 2018 14:42
Call Form::isSubmitted() before Form::isValid()
…t least get tests to run and fail spectacularly, as expected with:

Errors: 21, Failures: 6, Warnings: 18, Incomplete: 2.
Tests: 186, Assertions: 387, Errors: 7, Failures: 3, Warnings: 23, Incomplete: 2.
Tests: 186, Assertions: 409, Errors: 1, Failures: 3, Warnings: 23, Incomplete: 2.
Tests: 186, Assertions: 431, Failures: 1, Warnings: 23, Incomplete: 2.
…e('security.context')); when no token_storage service is set, and throw a new LogicException instead with a message indicating as much. Updated the test to expect the exception instead of the call to replaceArgument.
… what happens when no token storage is available. marked the test as incomplete.

- removed reference to templating service from authorize.xml
…cess had to add public getters on objects which really ought to not be there ... which is not great.
…ead of random bytes, to match the repository API's return tyoe of object.
…sing token storage with deprecated security context
Symfony 5 compatibility updates - cleanups and improvements by @iisisrael
@elchris elchris changed the title Symfony 4.4.* / 5.0.* (Issue #625) Symfony 4.4.* / 5.0.* Feb 14, 2020
@napestershine
Copy link

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

@elchris
Copy link
Author

elchris commented Apr 3, 2020

@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
Copy link

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

@elchris
Copy link
Author

elchris commented Apr 9, 2020

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
Copy link

patrickbussmann commented Apr 9, 2020

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
Copy link

ruudk commented Sep 9, 2020

Any news on this?

@deguif
Copy link
Collaborator

deguif commented Sep 17, 2020

@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
Copy link
Author

elchris commented Sep 17, 2020

@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
Copy link
Collaborator

deguif commented Sep 18, 2020

Symfony 5 is now supported on master.

@Cocray
Copy link

Cocray commented Sep 22, 2020

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
Copy link

MatthieuMan commented Sep 2, 2021

@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
Copy link

@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.

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.

10 participants