[Security] changed defaults for MessageDigestEncoder

- encode_as_base64 set to true - iterations increased to 5000 from 1
symfony · Mar 5, 2011 · f82b89c · f82b89c · jmikola · Mar 9, 2011
1 parent f010742
commit f82b89c
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 33 deletions.
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Configuration.php
@@ -237,9 +237,9 @@ protected function addEncodersSection($rootNode)
                     ->performNoDeepMerging()
                     ->beforeNormalization()->ifString()->then(function($v) { return array('algorithm' => $v); })->end()
                     ->scalarNode('algorithm')->cannotBeEmpty()->end()
-                    ->booleanNode('ignore_case')->end()
-                    ->booleanNode('encode_as_base64')->end()
-                    ->scalarNode('iterations')->end()
+                    ->booleanNode('ignore_case')->defaultFalse()->end()
+                    ->booleanNode('encode_as_base64')->defaultTrue()->end()
+                    ->scalarNode('iterations')->defaultValue(5000)->end()
                     ->scalarNode('id')->end()
                 ->end()
             ->end()

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -381,11 +381,7 @@ protected function createEncoder($accountClass, $config, ContainerBuilder $conta
 
         // plaintext encoder
         if ('plaintext' === $config['algorithm']) {
-            $arguments = array();
-
-            if (isset($config['ignore_case'])) {
-                $arguments[0] = $config['ignore_case'];
-            }
+            $arguments = array($config['ignore_case']);
 
             return array(
                 'class' => new Parameter('security.encoder.plain.class'),
@@ -394,20 +390,11 @@ protected function createEncoder($accountClass, $config, ContainerBuilder $conta
         }
 
         // message digest encoder
-        $arguments = array($config['algorithm']);
-
-        // add optional arguments
-        if (isset($config['encode_as_base64'])) {
-            $arguments[1] = $config['encode_as_base64'];
-        } else {
-            $arguments[1] = false;
-        }
-
-        if (isset($config['iterations'])) {
-            $arguments[2] = $config['iterations'];
-        } else {
-            $arguments[2] = 1;
-        }
+        $arguments = array(
+            $config['algorithm'],
+            $config['encode_as_base64'],
+            $config['iterations'],
+        );
 
         return array(
             'class' => new Parameter('security.encoder.digest.class'),

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php
@@ -5,7 +5,7 @@
         'JMS\FooBundle\Entity\User1' => 'plaintext',
         'JMS\FooBundle\Entity\User2' => array(
             'algorithm' => 'sha1',
-            'encode_as_base64' => true,
+            'encode_as_base64' => false,
             'iterations' => 5,
         ),
         'JMS\FooBundle\Entity\User3' => array(

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml
@@ -8,7 +8,7 @@
     <config>
         <encoder class="JMS\FooBundle\Entity\User1" algorithm="plaintext" />
 
-        <encoder class="JMS\FooBundle\Entity\User2" algorithm="sha1" encode-as-base64="true" iterations="5" />
+        <encoder class="JMS\FooBundle\Entity\User2" algorithm="sha1" encode-as-base64="false" iterations="5" />
 
         <encoder class="JMS\FooBundle\Entity\User3" algorithm="md5" />
 

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml
@@ -3,7 +3,7 @@ security:
         JMS\FooBundle\Entity\User1: plaintext
         JMS\FooBundle\Entity\User2:
             algorithm: sha1
-            encode_as_base64: true
+            encode_as_base64: false
             iterations: 5
         JMS\FooBundle\Entity\User3:
             algorithm: md5

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php
@@ -136,15 +136,15 @@ public function testEncoders()
         $this->assertEquals(array(array(
             'JMS\FooBundle\Entity\User1' => array(
                 'class' => new Parameter('security.encoder.plain.class'),
-                'arguments' => array(),
+                'arguments' => array(false),
             ),
             'JMS\FooBundle\Entity\User2' => array(
                 'class' => new Parameter('security.encoder.digest.class'),
-                'arguments' => array('sha1', true, 5),
+                'arguments' => array('sha1', false, 5),
             ),
             'JMS\FooBundle\Entity\User3' => array(
                 'class' => new Parameter('security.encoder.digest.class'),
-                'arguments' => array('md5', false, 1),
+                'arguments' => array('md5', true, 5000),
             ),
             'JMS\FooBundle\Entity\User4' => new Reference('security.encoder.foo'),
         )), $container->getDefinition('security.encoder_factory.generic')->getArguments());

diff --git a/src/Symfony/Component/Security/Core/Encoder/MessageDigestPasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/MessageDigestPasswordEncoder.php
@@ -28,7 +28,7 @@ class MessageDigestPasswordEncoder extends BasePasswordEncoder
      * @param Boolean $encodeHashAsBase64 Whether to base64 encode the password hash
      * @param integer $iterations         The number of iterations to use to stretch the password hash
      */
-    public function __construct($algorithm = 'sha256', $encodeHashAsBase64 = false, $iterations = 1)
+    public function __construct($algorithm = 'sha512', $encodeHashAsBase64 = true, $iterations = 5000)
     {
         $this->algorithm = $algorithm;
         $this->encodeHashAsBase64 = $encodeHashAsBase64;

diff --git a/tests/Symfony/Tests/Component/Security/Core/Encoder/MessageDigestPasswordEncoderTest.php b/tests/Symfony/Tests/Component/Security/Core/Encoder/MessageDigestPasswordEncoderTest.php
@@ -17,21 +17,21 @@ class MessageDigestPasswordEncoderTest extends \PHPUnit_Framework_TestCase
 {
     public function testIsPasswordValid()
     {
-        $encoder = new MessageDigestPasswordEncoder();
+        $encoder = new MessageDigestPasswordEncoder('sha256', false, 1);
 
         $this->assertTrue($encoder->isPasswordValid(hash('sha256', 'password'), 'password', ''));
     }
 
     public function testEncodePassword()
     {
-        $encoder = new MessageDigestPasswordEncoder();
+        $encoder = new MessageDigestPasswordEncoder('sha256', false, 1);
         $this->assertSame(hash('sha256', 'password'), $encoder->encodePassword('password', ''));
 
-        $encoder = new MessageDigestPasswordEncoder('sha256', true);
+        $encoder = new MessageDigestPasswordEncoder('sha256', true, 1);
         $this->assertSame(base64_encode(hash('sha256', 'password', true)), $encoder->encodePassword('password', ''));
 
         $encoder = new MessageDigestPasswordEncoder('sha256', false, 2);
-        $this->assertSame(hash('sha256', hash('sha256', 'password', true)), $encoder->encodePassword('password', ''));
+        $this->assertSame(hash('sha256', hash('sha256', 'password', true).'password'), $encoder->encodePassword('password', ''));
     }
 
     /**