Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Algorithm fix #32

Merged
1 commit merged into from over 3 years ago

2 participants

Lukas Kahwe Smith Antoine Hérault
Lukas Kahwe Smith

ok, this gets things working again but requires a new config parameter "password_encoder". one day we can hopefully just somehow inject the right MessageDigestPasswordEncoder() instance, but right now i dont see how to determine the instance to set, or does it support fetching the algorithm from the instance so that we can store it with the user.

furthermore right now i do not know if we can make it possible to use DoctrineUserBundle in different providers with different settings. seems like a really tricky issue to handle and probably requires stopping to read the dependencies and parameters from the container service directly.

Antoine Hérault

Just merged.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Dec 01, 2010
Lukas Kahwe Smith fixed password hashing (hardcoding sha1 for now, but this needs to be…
… addressed in Symfony2)
5e954e7
This page is out of date. Refresh to see the latest.
25  Controller/UserController.php
@@ -15,6 +15,7 @@
15 15
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
16 16
 use Symfony\Component\HttpKernel\Exception\ForbiddenHttpException;
17 17
 use Symfony\Component\Security\Authentication\Token\UsernamePasswordToken;
  18
+use Symfony\Component\Security\Encoder\MessageDigestPasswordEncoder;
18 19
 
19 20
 /**
20 21
  * RESTful controller managing user CRUD
@@ -66,6 +67,7 @@ public function updateAction($username)
66 67
         if ($data = $this->get('request')->request->get($form->getName())) {
67 68
             $form->bind($data);
68 69
             if ($form->isValid()) {
  70
+                $this->hashUserPassword($user);
69 71
                 $this->saveUser($user);
70 72
                 $this->get('session')->setFlash('doctrine_user_user_update', 'success');
71 73
                 $userUrl = $this->generateUrl('doctrine_user_user_show', array('username' => $user->getUsername()));
@@ -85,7 +87,7 @@ public function updateAction($username)
85 87
     public function newAction()
86 88
     {
87 89
         $form = $this->createForm();
88  
-        
  90
+
89 91
         return $this->render('DoctrineUserBundle:User:new.'.$this->getRenderer(), array(
90 92
             'form' => $form
91 93
         ));
@@ -102,6 +104,7 @@ public function createAction()
102 104
 
103 105
         if ($form->isValid()) {
104 106
             $user = $form->getData();
  107
+            $this->hashUserPassword($user);
105 108
             if ($this->container->getParameter('doctrine_user.confirmation_email.enabled')) {
106 109
                 $user->setIsActive(false);
107 110
                 $this->saveUser($user);
@@ -251,7 +254,10 @@ public function changePasswordUpdateAction()
251 254
         $form = $this->createChangePasswordForm($user);
252 255
         $form->bind($this->get('request')->request->get($form->getName()));
253 256
         if($form->isValid()) {
254  
-            $user->setPassword($form->getNewPassword());
  257
+            $password = $form->getNewPassword();
  258
+            $user->setPassword($password);
  259
+            $this->hashUserPassword($user);
  260
+
255 261
             $this->get('doctrine_user.repository.user')->getObjectManager()->flush();
256 262
             $userUrl = $this->generateUrl('doctrine_user_user_show', array('username' => $user->getUsername()));
257 263
             return $this->redirect($userUrl);
@@ -356,4 +362,19 @@ protected function getRenderer()
356 362
     {
357 363
         return $this->container->getParameter('doctrine_user.template.renderer');
358 364
     }
  365
+
  366
+    protected function hashUserPassword($user)
  367
+    {
  368
+        $password = $user->getPassword();
  369
+        $algorithm = $this->container->getParameter('doctrine_user.password_encoder');
  370
+        if (empty($password)) {
  371
+            $hashPassword = null;
  372
+        } else {
  373
+            // TODO: ideally inject the message encoder, but there is no way to retrieve the encoding from it atm
  374
+            $encoder = new MessageDigestPasswordEncoder($algorithm);
  375
+            $hashPassword = $encoder->encodePassword($password, $user->getSalt());
  376
+        }
  377
+
  378
+        $user->setPasswordHash($hashPassword, $algorithm);
  379
+    }
359 380
 }
7  DependencyInjection/DoctrineUserExtension.php
@@ -31,6 +31,13 @@ public function configLoad(array $config, ContainerBuilder $container)
31 31
             throw new \InvalidArgumentException('You must define your user model class');
32 32
         }
33 33
 
  34
+        // TODO: this needs to be removed eventually once we figure out how to be able to determine the encoder
  35
+        if (!isset($config['password_encoder'])) {
  36
+            throw new \InvalidArgumentException('You must define your password_encoder');
  37
+        }
  38
+
  39
+        $container->setParameter('doctrine_user.password_encoder', $config['password_encoder']);
  40
+
34 41
         $namespaces = array(
35 42
             '' => array(
36 43
                 'session_create_success_route' => 'doctrine_user.session_create.success_route',
37  Model/User.php
@@ -12,7 +12,6 @@
12 12
 use Doctrine\Common\Collections\Collection;
13 13
 use Doctrine\Common\Collections\ArrayCollection;
14 14
 use Symfony\Component\Security\User\AdvancedAccountInterface;
15  
-use Symfony\Component\Security\Encoder\MessageDigestPasswordEncoder;
16 15
 
17 16
 /**
18 17
  * Storage agnostic user object
@@ -77,6 +76,11 @@
77 76
     /**
78 77
      * @var string
79 78
      */
  79
+    protected $algorithm;
  80
+
  81
+    /**
  82
+     * @var string
  83
+     */
80 84
     protected $salt;
81 85
 
82 86
     /**
@@ -257,12 +261,22 @@ public function setEmail($email)
257 261
     }
258 262
 
259 263
     /**
260  
-     * Hashed password
  264
+     * Return the algorithm used to hash the password
  265
+     *
  266
+     * @return string the algorithm
  267
+     **/
  268
+    public function getAlgorithm()
  269
+    {
  270
+        return $this->algorithm;
  271
+    }
  272
+
  273
+    /**
  274
+     * Get password
261 275
      * @return string
262 276
      */
263 277
     public function getPassword()
264 278
     {
265  
-        return $this->passwordHash;
  279
+        return $this->password;
266 280
     }
267 281
 
268 282
     /**
@@ -281,7 +295,6 @@ public function getSalt()
281 295
     public function setPassword($password)
282 296
     {
283 297
         $this->password = $password;
284  
-        $this->hashPassword();
285 298
     }
286 299
 
287 300
     /**
@@ -367,18 +380,14 @@ public function incrementUpdatedAt()
367 380
     }
368 381
 
369 382
     /**
370  
-     * Encode the user password
371  
-     * @return null
  383
+     * Encode the user hashed password
  384
+     * @param string $hashPassword
  385
+     * @param string $algorithm
372 386
      */
373  
-    protected function hashPassword()
  387
+    public function setPasswordHash($hashPassword, $algorithm)
374 388
     {
375  
-        // TODO: the Symfony2 security layer should take care of hashing the password
376  
-        if (empty($this->password)) {
377  
-            $this->hashPassword = null;
378  
-        } else {
379  
-            $encoder = new MessageDigestPasswordEncoder('sha1');
380  
-            $this->passwordHash = $encoder->encodePassword($this->password, $this->getSalt());
381  
-        }
  389
+        $this->passwordHash = $hashPassword;
  390
+        $this->algorithm = $algorithm;
382 391
     }
383 392
 
384 393
     /**
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.