feature #31170 [Security] deprecate BCryptPasswordEncoder in favor of…

… NativePasswordEncoder (nicolas-grekas) This PR was merged into the 4.3-dev branch. Discussion ---------- [Security] deprecate BCryptPasswordEncoder in favor of NativePasswordEncoder | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Follow up of #31140 Commits ------- e197398 [Security] deprecate BCryptPasswordEncoder in favor of NativePasswordEncoder
symfony · Apr 19, 2019 · 823d375 · 823d375
2 parents d9bcfc3 + e197398
commit 823d375
Show file tree

Hide file tree

Showing 18 changed files with 151 additions and 46 deletions.
diff --git a/UPGRADE-4.3.md b/UPGRADE-4.3.md
@@ -168,13 +168,14 @@ Security
    ```
 
  * The `Argon2iPasswordEncoder` class has been deprecated, use `SodiumPasswordEncoder` instead.
+ * The `BCryptPasswordEncoder` class has been deprecated, use `NativePasswordEncoder` instead.
  * Not implementing the methods `__serialize` and `__unserialize` in classes implementing
    the `TokenInterface` is deprecated
 
 SecurityBundle
 --------------
 
- * Configuring encoders using `argon2i` as algorithm has been deprecated, use `auto` instead.
+ * Configuring encoders using `argon2i` or `bcrypt` as algorithm has been deprecated, use `auto` instead.
 
 TwigBridge
 ----------

diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md
@@ -342,6 +342,7 @@ Security
    ```
 
  * The `Argon2iPasswordEncoder` class has been removed, use `SodiumPasswordEncoder` instead.
+ * The `BCryptPasswordEncoder` class has been removed, use `NativePasswordEncoder` instead.
  * Classes implementing the `TokenInterface` must implement the two new methods
    `__serialize` and `__unserialize`
 
@@ -364,7 +365,7 @@ SecurityBundle
    changed to underscores.
    Before: `my-cookie` deleted the `my_cookie` cookie (with an underscore).
    After: `my-cookie` deletes the `my-cookie` cookie (with a dash).
- * Configuring encoders using `argon2i` as algorithm is not supported anymore, use `sodium` instead.
+ * Configuring encoders using `argon2i` or `bcrypt` as algorithm is not supported anymore, use `auto` instead.
 
 Serializer
 ----------

diff --git a/src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php b/src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php
@@ -70,7 +70,7 @@ protected function configure()
 security:
     encoders:
         Symfony\Component\Security\Core\User\User: plaintext
-        App\Entity\User: bcrypt
+        App\Entity\User: auto
 </comment>
 
 If you execute the command non-interactively, the first available configured

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -558,6 +558,8 @@ private function createEncoder($config, ContainerBuilder $container)
 
         // bcrypt encoder
         if ('bcrypt' === $config['algorithm']) {
+            @trigger_error('Configuring an encoder with "bcrypt" as algorithm is deprecated since Symfony 4.3, use "auto" instead.', E_USER_DEPRECATED);
+
             return [
                 'class' => 'Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder',
                 'arguments' => [$config['cost'] ?? 13],

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php
@@ -306,14 +306,10 @@ public function testEncoders()
                 'arguments' => ['sha1', false, 5, 30],
             ],
             'JMS\FooBundle\Entity\User6' => [
-                'class' => 'Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder',
-                'arguments' => [15],
-            ],
-            'JMS\FooBundle\Entity\User7' => [
                 'class' => 'Symfony\Component\Security\Core\Encoder\NativePasswordEncoder',
                 'arguments' => [8, 102400, 15],
             ],
-            'JMS\FooBundle\Entity\User8' => [
+            'JMS\FooBundle\Entity\User7' => [
                 'algorithm' => 'auto',
                 'hash_algorithm' => 'sha512',
                 'key_length' => 40,
@@ -371,25 +367,13 @@ public function testEncodersWithLibsodium()
                 'arguments' => ['sha1', false, 5, 30],
             ],
             'JMS\FooBundle\Entity\User6' => [
-                'class' => 'Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder',
-                'arguments' => [15],
+                'class' => 'Symfony\Component\Security\Core\Encoder\NativePasswordEncoder',
+                'arguments' => [8, 102400, 15],
             ],
             'JMS\FooBundle\Entity\User7' => [
                 'class' => 'Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder',
                 'arguments' => [8, 128 * 1024 * 1024],
             ],
-            'JMS\FooBundle\Entity\User8' => [
-                'algorithm' => 'auto',
-                'hash_algorithm' => 'sha512',
-                'key_length' => 40,
-                'ignore_case' => false,
-                'encode_as_base64' => true,
-                'iterations' => 5000,
-                'cost' => null,
-                'memory_cost' => null,
-                'time_cost' => null,
-                'threads' => null,
-            ],
         ]], $container->getDefinition('security.encoder_factory.generic')->getArguments());
     }
 
@@ -441,15 +425,42 @@ public function testEncodersWithArgon2i()
                 'arguments' => ['sha1', false, 5, 30],
             ],
             'JMS\FooBundle\Entity\User6' => [
-                'class' => 'Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder',
-                'arguments' => [15],
+                'class' => 'Symfony\Component\Security\Core\Encoder\NativePasswordEncoder',
+                'arguments' => [8, 102400, 15],
             ],
             'JMS\FooBundle\Entity\User7' => [
                 'class' => 'Symfony\Component\Security\Core\Encoder\Argon2iPasswordEncoder',
                 'arguments' => [256, 1, 2],
             ],
-            'JMS\FooBundle\Entity\User8' => [
-                'algorithm' => 'auto',
+        ]], $container->getDefinition('security.encoder_factory.generic')->getArguments());
+    }
+
+    /**
+     * @group legacy
+     */
+    public function testEncodersWithBCrypt()
+    {
+        $container = $this->getContainer('bcrypt_encoder');
+
+        $this->assertEquals([[
+            'JMS\FooBundle\Entity\User1' => [
+                'class' => 'Symfony\Component\Security\Core\Encoder\PlaintextPasswordEncoder',
+                'arguments' => [false],
+            ],
+            'JMS\FooBundle\Entity\User2' => [
+                'algorithm' => 'sha1',
+                'encode_as_base64' => false,
+                'iterations' => 5,
+                'hash_algorithm' => 'sha512',
+                'key_length' => 40,
+                'ignore_case' => false,
+                'cost' => null,
+                'memory_cost' => null,
+                'time_cost' => null,
+                'threads' => null,
+            ],
+            'JMS\FooBundle\Entity\User3' => [
+                'algorithm' => 'md5',
                 'hash_algorithm' => 'sha512',
                 'key_length' => 40,
                 'ignore_case' => false,
@@ -460,6 +471,19 @@ public function testEncodersWithArgon2i()
                 'time_cost' => null,
                 'threads' => null,
             ],
+            'JMS\FooBundle\Entity\User4' => new Reference('security.encoder.foo'),
+            'JMS\FooBundle\Entity\User5' => [
+                'class' => 'Symfony\Component\Security\Core\Encoder\Pbkdf2PasswordEncoder',
+                'arguments' => ['sha1', false, 5, 30],
+            ],
+            'JMS\FooBundle\Entity\User6' => [
+                'class' => 'Symfony\Component\Security\Core\Encoder\NativePasswordEncoder',
+                'arguments' => [8, 102400, 15],
+            ],
+            'JMS\FooBundle\Entity\User7' => [
+                'class' => 'Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder',
+                'arguments' => [15],
+            ],
         ]], $container->getDefinition('security.encoder_factory.generic')->getArguments());
     }
 

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/bcrypt_encoder.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/bcrypt_encoder.php
@@ -0,0 +1,12 @@
+<?php
+
+$this->load('container1.php', $container);
+
+$container->loadFromExtension('security', [
+    'encoders' => [
+        'JMS\FooBundle\Entity\User7' => [
+            'algorithm' => 'bcrypt',
+            'cost' => 15,
+        ],
+    ],
+]);
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php
@@ -22,16 +22,12 @@
             'key_length' => 30,
         ],
         'JMS\FooBundle\Entity\User6' => [
-            'algorithm' => 'bcrypt',
-            'cost' => 15,
-        ],
-        'JMS\FooBundle\Entity\User7' => [
             'algorithm' => 'native',
             'time_cost' => 8,
             'memory_cost' => 100,
             'cost' => 15,
         ],
-        'JMS\FooBundle\Entity\User8' => [
+        'JMS\FooBundle\Entity\User7' => [
             'algorithm' => 'auto',
         ],
     ],

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/bcrypt_encoder.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/bcrypt_encoder.xml
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+   xmlns:sec="http://symfony.com/schema/dic/security"
+   xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+
+    <imports>
+        <import resource="container1.xml"/>
+    </imports>
+
+    <sec:config>
+        <sec:encoder class="JMS\FooBundle\Entity\User7" algorithm="bcrypt" cost="15" />
+    </sec:config>
+
+</container>
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml
@@ -16,11 +16,9 @@
 
         <encoder class="JMS\FooBundle\Entity\User5" algorithm="pbkdf2" hash-algorithm="sha1" encode-as-base64="false" iterations="5" key-length="30" />
 
-        <encoder class="JMS\FooBundle\Entity\User6" algorithm="bcrypt" cost="15" />
+        <encoder class="JMS\FooBundle\Entity\User6" algorithm="native" time-cost="8" memory-cost="100" cost="15" />
 
-        <encoder class="JMS\FooBundle\Entity\User7" algorithm="native" time-cost="8" memory-cost="100" cost="15" />
-
-        <encoder class="JMS\FooBundle\Entity\User8" algorithm="auto" />
+        <encoder class="JMS\FooBundle\Entity\User7" algorithm="auto" />
 
         <provider name="default">
             <memory>

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/bcrypt_encoder.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/bcrypt_encoder.yml
@@ -0,0 +1,8 @@
+imports:
+    - { resource: container1.yml }
+
+security:
+    encoders:
+        JMS\FooBundle\Entity\User7:
+            algorithm: bcrypt
+            cost: 15
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml
@@ -16,14 +16,11 @@ security:
             iterations: 5
             key_length: 30
         JMS\FooBundle\Entity\User6:
-            algorithm: bcrypt
-            cost: 15
-        JMS\FooBundle\Entity\User7:
             algorithm: native
             time_cost: 8
             memory_cost: 100
             cost: 15
-        JMS\FooBundle\Entity\User8:
+        JMS\FooBundle\Entity\User7:
             algorithm: auto
 
     providers:

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/UserPasswordEncoderCommandTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/UserPasswordEncoderCommandTest.php
@@ -18,6 +18,7 @@
 use Symfony\Component\Security\Core\Encoder\Argon2iPasswordEncoder;
 use Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder;
 use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
+use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
 use Symfony\Component\Security\Core\Encoder\Pbkdf2PasswordEncoder;
 use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder;
 
@@ -54,8 +55,12 @@ public function testEncodeNoPasswordNoInteraction()
         $this->assertEquals($statusCode, 1);
     }
 
+    /**
+     * @group legacy
+     */
     public function testEncodePasswordBcrypt()
     {
+        $this->setupBcrypt();
         $this->passwordEncoderCommandTester->execute([
             'command' => 'security:encode-password',
             'password' => 'password',
@@ -95,6 +100,23 @@ public function testEncodePasswordArgon2i()
         $this->assertTrue($encoder->isPasswordValid($hash, 'password', null));
     }
 
+    public function testEncodePasswordNative()
+    {
+        $this->passwordEncoderCommandTester->execute([
+            'command' => 'security:encode-password',
+            'password' => 'password',
+            'user-class' => 'Custom\Class\Native\User',
+        ], ['interactive' => false]);
+
+        $output = $this->passwordEncoderCommandTester->getDisplay();
+        $this->assertContains('Password encoding succeeded', $output);
+
+        $encoder = new NativePasswordEncoder();
+        preg_match('# Encoded password\s{1,}([\w+\/$.,=]+={0,2})\s+#', $output, $matches);
+        $hash = $matches[1];
+        $this->assertTrue($encoder->isPasswordValid($hash, 'password', null));
+    }
+
     public function testEncodePasswordSodium()
     {
         if (!SodiumPasswordEncoder::isSupported()) {
@@ -162,12 +184,12 @@ public function testEncodePasswordEmptySaltOutput()
         $this->assertNotContains(' Generated salt ', $this->passwordEncoderCommandTester->getDisplay());
     }
 
-    public function testEncodePasswordBcryptOutput()
+    public function testEncodePasswordNativeOutput()
     {
         $this->passwordEncoderCommandTester->execute([
             'command' => 'security:encode-password',
             'password' => 'p@ssw0rd',
-            'user-class' => 'Custom\Class\Bcrypt\User',
+            'user-class' => 'Custom\Class\Native\User',
         ], ['interactive' => false]);
 
         $this->assertNotContains(' Generated salt ', $this->passwordEncoderCommandTester->getDisplay());
@@ -233,8 +255,8 @@ public function testEncodePasswordAsksNonProvidedUserClass()
         ], ['decorated' => false]);
 
         $this->assertContains(<<<EOTXT
- For which user class would you like to encode a password? [Custom\Class\Bcrypt\User]:
-  [0] Custom\Class\Bcrypt\User
+ For which user class would you like to encode a password? [Custom\Class\Native\User]:
+  [0] Custom\Class\Native\User
   [1] Custom\Class\Pbkdf2\User
   [2] Custom\Class\Test\User
   [3] Symfony\Component\Security\Core\User\User
@@ -301,6 +323,19 @@ private function setupArgon2i()
         $this->passwordEncoderCommandTester = new CommandTester($passwordEncoderCommand);
     }
 
+    private function setupBcrypt()
+    {
+        putenv('COLUMNS='.(119 + \strlen(PHP_EOL)));
+        $kernel = $this->createKernel(['test_case' => 'PasswordEncode', 'root_config' => 'bcrypt.yml']);
+        $kernel->boot();
+
+        $application = new Application($kernel);
+
+        $passwordEncoderCommand = $application->get('security:encode-password');
+
+        $this->passwordEncoderCommandTester = new CommandTester($passwordEncoderCommand);
+    }
+
     private function setupSodium()
     {
         putenv('COLUMNS='.(119 + \strlen(PHP_EOL)));

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/bcrypt.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/bcrypt.yml
@@ -0,0 +1,7 @@
+imports:
+    - { resource: config.yml }
+
+security:
+    encoders:
+        Custom\Class\Bcrypt\User:
+            algorithm: bcrypt
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordEncode/config.yml
@@ -4,8 +4,8 @@ imports:
 security:
     encoders:
         Symfony\Component\Security\Core\User\User: plaintext
-        Custom\Class\Bcrypt\User:
-            algorithm: bcrypt
+        Custom\Class\Native\User:
+            algorithm: native
             cost:      10
         Custom\Class\Pbkdf2\User:
             algorithm: pbkdf2

diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md
@@ -21,7 +21,8 @@ CHANGELOG
  * Dispatch `AuthenticationFailureEvent` on `security.authentication.failure`
  * Dispatch `InteractiveLoginEvent` on `security.interactive_login`
  * Dispatch `SwitchUserEvent` on `security.switch_user`
- * Deprecated `Argon2iPasswordEncoder`, use `SodiumPasswordEncoder`
+ * Deprecated `Argon2iPasswordEncoder`, use `SodiumPasswordEncoder` instead
+ * Deprecated `BCryptPasswordEncoder`, use `NativePasswordEncoder` instead
 
 4.2.0
 -----

diff --git a/src/Symfony/Component/Security/Core/Encoder/BCryptPasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/BCryptPasswordEncoder.php
@@ -11,11 +11,15 @@
 
 namespace Symfony\Component\Security\Core\Encoder;
 
+@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.3, use "%s" instead.', BCryptPasswordEncoder::class, NativePasswordEncoder::class), E_USER_DEPRECATED);
+
 use Symfony\Component\Security\Core\Exception\BadCredentialsException;
 
 /**
  * @author Elnur Abdurrakhimov <elnur@elnur.pro>
  * @author Terje Bråten <terje@braten.be>
+ *
+ * @deprecated since Symfony 4.3, use NativePasswordEncoder instead
  */
 class BCryptPasswordEncoder extends BasePasswordEncoder implements SelfSaltingEncoderInterface
 {

diff --git a/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php b/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php
@@ -106,6 +106,7 @@ private function getEncoderConfigFromAlgorithm($config)
                     ],
                 ];
 
+            /* @deprecated since Symfony 4.3 */
             case 'bcrypt':
                 return [
                     'class' => BCryptPasswordEncoder::class,