8000 [Security] EncoderFactory regression while updating from 4.3 to 4.4 · Issue #35058 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Security] EncoderFactory regression while updating from 4.3 to 4.4 #35058

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.

Sign up for GitHub

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

Jump to bottom
Closed
FabienPapet opened this issue Dec 19, 2019 · 1 comment
Closed

[Security] EncoderFactory regression while updating from 4.3 to 4.4 #35058

FabienPapet opened this issue Dec 19, 2019 · 1 comment
Labels
Bug Security Status: Needs Review

Comments

@FabienPapet
Copy link
FabienPapet commented Dec 19, 2019 •
edited
Loading

Symfony version(s) affected: 4.4, 5.0

Description
Updating Symfony 4.3 to 4.4 breaks tests on my application. I think the cause of the regression might be the new password migration feature introduced in 4.4.

There is a notice thrown when we pass an array as configuration to an encoder.
Once we have created the NativePasswordEncoder object, we try to build extra encoders (pbkdf2, sha512). But the pbkdf2 encoder needs configs which are not present in the $rawConfig variable.

  • I can try to fix the problem, with some help (I don't know why we try to build extra encoders).
  • I tried to figure out why we build other PasswordEncoders in the array_map callback but could find any reason perhaps you have an idea :-) .
  • This PR is related [Security] Add migrating encoder configuration #34139 (review)

ping @chalasr @nicolas-grekas

How to reproduce
Create a test in Symfony\Component\Security\Core\Tests\Encoder\EncoderFactoryTest with the following code on a symfony/symfony:4.3 branch this will be successful in 4.3 branch and will produce notice in 4.4.

    // Symfony\Component\Security\Core\Tests\Encoder\EncoderFactoryTest
    public function testGetEncoderWithArrayConfiguration()
    {
        $factory = new EncoderFactory([
            'Symfony\Component\Security\Core\User\UserInterface' => [
                'algorithm' => 'bcrypt',
                'cost' => 11,
            ],
        ]);

        $factory->getEncoder(new User('user', 'pass'));
        $this->assertInstanceOf(PasswordEncoderInterface::class, $factory->getEncoder(new User('user', 'pass')));
    }
Symfony\Component\Security\Core\Tests\Encoder\EncoderFactoryTest::testGetEncoderWithArrayConfiguration
Undefined index: hash_algorithm

Possible Solutions

I think there are 2 options here

  • Do not create extra encoders with NativePasswordEncoder
  • Provide default configuration keys for extra encoders

Additional context

I can provide if needed.
@chalasr chalasr mentioned this issue Dec 19, 2019
Merged
@chalasr
Copy link
Member
chalasr commented Dec 19, 2019

Thank you for the perfect report. See #35060

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Security Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@FabienPapet @xabbuh @chalasr @carsonbot
0