[Security] simplified encoder factory implementation

symfony · Feb 14, 2011 · 5c7fe8f · 5c7fe8f
1 parent 0643dc4
commit 5c7fe8f
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 53 deletions.
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -72,6 +72,10 @@ public function configLoad(array $configs, ContainerBuilder $container)
         $this->createFirewalls($config, $container);
         $this->createAuthorization($config, $container);
         $this->createRoleHierarchy($config, $container);
+
+        if ($config['encoders']) {
+            $this->createEncoders($config['encoders'], $container);
+        }
     }
 
     public function aclLoad(array $configs, ContainerBuilder $container)
@@ -156,8 +160,6 @@ protected function createFirewalls($config, ContainerBuilder $container)
         $firewalls = $config['firewalls'];
         $providerIds = $this->createUserProviders($config, $container);
 
-        $this->createEncoders($config, $container);
-
         // make the ContextListener aware of the configured user providers
         $definition = $container->getDefinition('security.context_listener');
         $arguments = $definition->getArguments();
@@ -329,15 +331,11 @@ protected function createAuthenticationListeners($container, $id, $firewall, $de
         return array($listeners, $providers, $defaultEntryPoint);
     }
 
-    protected function createEncoders($config, ContainerBuilder $container)
+    protected function createEncoders($encoders, ContainerBuilder $container)
     {
-        if (!isset($config['encoders'])) {
-            return;
-        }
-
         $encoderMap = array();
-        foreach ($config['encoders'] as $class => $encoder) {
-            $encoderMap = $this->createEncoder($encoderMap, $class, $encoder, $container);
+        foreach ($encoders as $class => $encoder) {
+            $encoderMap[$class] = $this->createEncoder($class, $encoder, $container);
         }
 
         $container
@@ -346,16 +344,11 @@ protected function createEncoders($config, ContainerBuilder $container)
         ;
     }
 
-    protected function createEncoder(array $encoderMap, $accountClass, $config, ContainerBuilder $container)
+    protected function createEncoder($accountClass, $config, ContainerBuilder $container)
     {
         // a custom encoder service
         if (isset($config['id'])) {
-            $container
-                ->getDefinition('security.encoder_factory.generic')
-                ->addMethodCall('addEncoder', array($accountClass, new Reference($config['id'])))
-            ;
-
-            return $encoderMap;
+            return new Reference($config['id']);
         }
 
         // plaintext encoder
@@ -366,12 +359,10 @@ protected function createEncoder(array $encoderMap, $accountClass, $config, Cont
                 $arguments[0] = $config['ignore_case'];
             }
 
-            $encoderMap[$accountClass] = array(
+            return array(
                 'class' => new Parameter('security.encoder.plain.class'),
                 'arguments' => $arguments,
             );
-
-            return $encoderMap;
         }
 
         // message digest encoder
@@ -390,12 +381,10 @@ protected function createEncoder(array $encoderMap, $accountClass, $config, Cont
             $arguments[2] = 1;
         }
 
-        $encoderMap[$accountClass] = array(
+        return array(
             'class' => new Parameter('security.encoder.digest.class'),
             'arguments' => $arguments,
         );
-
-        return $encoderMap;
     }
 
     // Parses user providers and returns an array of their ids

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php
@@ -1,6 +1,20 @@
 <?php
 
 $container->loadFromExtension('security', 'config', array(
+    'encoders' => array(
+        'JMS\FooBundle\Entity\User1' => 'plaintext',
+        'JMS\FooBundle\Entity\User2' => array(
+            'algorithm' => 'sha1',
+            'encode_as_base64' => true,
+            'iterations' => 5,
+        ),
+        'JMS\FooBundle\Entity\User3' => array(
+            'algorithm' => 'md5',
+        ),
+        'JMS\FooBundle\Entity\User4' => array(
+            'id' => 'security.encoder.foo',
+        ),
+    ),
     'providers' => array(
         'default' => array(
             'users' => 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
@@ -6,6 +6,14 @@
     xsi:schemaLocation="http://www.symfony-project.org/schema/dic/services http://www.symfony-project.org/schema/dic/services/services-1.0.xsd">
 
     <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\User3" algorithm="md5" />
+
+        <encoder class="JMS\FooBundle\Entity\User4" id="security.encoder.foo" />
+
         <provider name="default">
             <user name="foo" password="foo" roles="ROLE_USER" />
         </provider>

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml
@@ -1,4 +1,15 @@
 security.config:
+    encoders:
+        JMS\FooBundle\Entity\User1: plaintext
+        JMS\FooBundle\Entity\User2:
+            algorithm: sha1
+            encode_as_base64: true
+            iterations: 5
+        JMS\FooBundle\Entity\User3:
+            algorithm: md5
+        JMS\FooBundle\Entity\User4:
+            id: security.encoder.foo
+
     providers:
         default:
             users:

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php
@@ -11,6 +11,10 @@
 
 namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection;
 
+use Symfony\Component\DependencyInjection\Reference;
+
+use Symfony\Component\DependencyInjection\Parameter;
+
 use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 
@@ -117,6 +121,27 @@ public function testMerge()
         ), $container->getParameter('security.role_hierarchy.roles'));
     }
 
+    public function testEncoders()
+    {
+        $container = $this->getContainer('container1');
+
+        $this->assertEquals(array(array(
+            'JMS\FooBundle\Entity\User1' => array(
+                'class' => new Parameter('security.encoder.plain.class'),
+                'arguments' => array(),
+            ),
+            'JMS\FooBundle\Entity\User2' => array(
+                'class' => new Parameter('security.encoder.digest.class'),
+                'arguments' => array('sha1', true, 5),
+            ),
+            'JMS\FooBundle\Entity\User3' => array(
+                'class' => new Parameter('security.encoder.digest.class'),
+                'arguments' => array('md5', false, 1),
+            ),
+            'JMS\FooBundle\Entity\User4' => new Reference('security.encoder.foo'),
+        )), $container->getDefinition('security.encoder_factory.generic')->getArguments());
+    }
+
     protected function getContainer($file)
     {
         $container = new ContainerBuilder();

diff --git a/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php b/src/Symfony/Component/Security/Core/Encoder/EncoderFactory.php
@@ -21,12 +21,10 @@
 class EncoderFactory implements EncoderFactoryInterface
 {
     protected $encoders;
-    protected $encoderMap;
 
-    public function __construct(array $encoderMap)
+    public function __construct(array $encoders)
     {
-        $this->encoders = array();
-        $this->encoderMap = $encoderMap;
+        $this->encoders = $encoders;
     }
 
     /**
@@ -35,43 +33,37 @@ public function __construct(array $encoderMap)
     public function getEncoder(AccountInterface $account)
     {
         foreach ($this->encoders as $class => $encoder) {
-            if ($account instanceof $class) {
-                return $encoder;
+            if (!$account instanceof $class) {
+                continue;
             }
-        }
 
-        return $this->createEncoder($account);
-    }
+            if (!$encoder instanceof PasswordEncoderInterface) {
+                return $this->encoders[$class] = $this->createEncoder($encoder);
+            }
 
-    /**
-     * Adds an encoder instance to the factory
-     *
-     * @param string $class
-     * @param PasswordEncoderInterface $encoder
-     * @return void
-     */
-    public function addEncoder($class, PasswordEncoderInterface $encoder)
-    {
-        $this->encoders[$class] = $encoder;
+            return $this->encoders[$class];
+        }
+
+        throw new \RuntimeException(sprintf('No encoder has been configured for account "%s".', get_class($account)));
     }
 
     /**
      * Creates the actual encoder instance
      *
-     * @param AccountInterface $account
+     * @param array $config
      * @return PasswordEncoderInterface
      */
-    protected function createEncoder($account)
+    protected function createEncoder(array $config)
     {
-        foreach ($this->encoderMap as $class => $config) {
-            if ($account instanceof $class) {
-                $reflection = new \ReflectionClass($config['class']);
-                $this->encoders[$class] = $reflection->newInstanceArgs($config['arguments']);
-
-                return $this->encoders[$class];
-            }
+        if (!isset($config['class'])) {
+            throw new \InvalidArgumentException(sprintf('"class" must be set in %s.', json_encode($config)));
+        }
+        if (!isset($config['arguments'])) {
+            throw new \InvalidArgumentException(sprintf('"arguments" must be set in %s.', json_encode($config)));
         }
 
-        throw new \InvalidArgumentException(sprintf('No encoder has been configured for account "%s".', get_class($account)));
+        $reflection = new \ReflectionClass($config['class']);
+
+        return $reflection->newInstanceArgs($config['arguments']);
     }
 }
diff --git a/tests/Symfony/Tests/Component/Security/Core/Encoder/EncoderFactoryTest.php b/tests/Symfony/Tests/Component/Security/Core/Encoder/EncoderFactoryTest.php
@@ -31,8 +31,9 @@ public function testGetEncoderWithMessageDigestEncoder()
 
     public function testGetEncoderWithService()
     {
-        $factory = new EncoderFactory(array());
-        $factory->addEncoder('Symfony\Component\Security\Core\User\AccountInterface', new MessageDigestPasswordEncoder('sha1'));
+        $factory = new EncoderFactory(array(
+            'Symfony\Component\Security\Core\User\AccountInterface' => new MessageDigestPasswordEncoder('sha1'),
+        ));
 
         $encoder = $factory->getEncoder($this->getMock('Symfony\Component\Security\Core\User\AccountInterface'));
         $expectedEncoder = new MessageDigestPasswordEncoder('sha1');