Merge pull request #223 from Wyorma/bugfix

Fix for validation bug on password change. Thanks @Wyorma
Zizaco · Feb 18, 2014 · a8b09d1 · a8b09d1
2 parents aadac7d + 0d04321
commit a8b09d1
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 2 deletions.
diff --git a/src/Zizaco/Confide/ConfideUser.php b/src/Zizaco/Confide/ConfideUser.php
@@ -131,9 +131,14 @@ public function forgotPassword()
     public function resetPassword( $params )
     {
         $password = array_get($params, 'password', '');
-        $passwordConfirmation = array_get($params, 'password_confirmation', '');
 
-        if ( $password == $passwordConfirmation )
+        $passwordValidators = array(
+            'password' => static::$rules['password'],
+            'password_confirmation' => static::$rules['password_confirmation'],
+        );
+        $validationResult = static::$app['confide.repository']->validate($passwordValidators);
+
+        if ( $validationResult )
         {
             return static::$app['confide.repository']
                 ->changePassword( $this, static::$app['hash']->make($password) );

diff --git a/tests/ConfideUserTest.php b/tests/ConfideUserTest.php
@@ -105,13 +105,67 @@ public function testShouldChangePassword()
             ->andReturn( true )
             ->once();
 
+        // Should call validate method
+        ConfideUser::$app['confide.repository']->shouldReceive('validate')
+            ->andReturn( true )
+            ->once();
+
         $this->populateUser();
 
         $old_password = $this->confide_user->password;
 
         $this->assertTrue( $this->confide_user->resetPassword( $credentials ) );
     }
 
+    public function testShouldNotChangePassword()
+    {
+        // Password should not be changed because it is empty
+        $credentials = array(
+            'email'=>'[email protected]',
+            'password'=>'',
+            'password_confirmation'=>''
+        );
+
+        // Should not call changePassword of the repository
+        ConfideUser::$app['confide.repository'] = m::mock( 'ConfideRepository' );
+        ConfideUser::$app['confide.repository']->shouldReceive( 'changePassword' )
+            ->never();
+
+        // Should call validate method
+        ConfideUser::$app['confide.repository']->shouldReceive('validate')
+            ->andReturn( false )
+            ->times(4);
+
+        $this->populateUser();
+
+        $this->assertFalse( $this->confide_user->resetPassword( $credentials ) );
+
+        // Additional asserts
+        // Password should not be changed because it is too short
+        $credentials = array(
+            'email'=>'[email protected]',
+            'password'=>'39a',
+            'password_confirmation'=>'39a'
+        );
+        $this->assertFalse( $this->confide_user->resetPassword( $credentials ) );
+
+        // Password should not be changed because it is too long
+        $credentials = array(
+            'email'=>'[email protected]',
+            'password'=>'1a2f34g5uj887n',
+            'password_confirmation'=>'1a2f34g5uj887n'
+        );
+        $this->assertFalse( $this->confide_user->resetPassword( $credentials ) );
+
+        // Password should not be changed because it is not confirmed
+        $credentials = array(
+            'email'=>'[email protected]',
+            'password'=>'987987',
+            'password_confirmation'=>'562906'
+        );
+        $this->assertFalse( $this->confide_user->resetPassword( $credentials ) );
+    }
+
     public function testShouldNotSaveDuplicated()
     {
         // Make sure that userExists return 1 to simulates a duplicated user