laravel / fortify

Backend controllers and scaffolding for Laravel authentication.
https://laravel.com/docs/fortify
MIT License
1.62k stars 294 forks source link

UpdateUserPassword action throws an error if $input array does not have 'current_password' index #174

Closed scibuff closed 3 years ago

scibuff commented 3 years ago

Description:

The UpdateUserPassword action throws an error if the $input array does not have current_password index

Steps To Reproduce:

Create and run a new (unit) test for checking that current password is required, e.g.

    /** @test */
    public function current_password_is_required_to_update_password()
    {
        $old_password = 'password';

        $user = User::factory()->create([
            'name' => 'John Doe',
            'email' => 'jonhdoe@example.com',
            'password' => Hash::make( $old_password )
        ]);

        $data = [];

        $this->assertTrue( Hash::check( $old_password, $user->password ) );

        $this->actingAs( $user )
            ->put( route('user-password.update'), $data )
            ->assertSessionHasErrorsIn('updatePassword', ['current_password']);

        $user = $user->fresh();
        $this->assertTrue(Hash::check( $old_password, $user->password ) );
    }

running the test gives the following error

PHPUnit 9.5.0 by Sebastian Bergmann and contributors.

E                                                                   1 / 1 (100%)

Time: 00:00.282, Memory: 28.00 MB

There was 1 error:

1) Tests\Feature\UserProfileTest::current_password_is_required_to_update_password
ErrorException: Undefined index: current_password

/home/vagrant/my-app/app/Actions/Fortify/UpdateUserPassword.php:26
/home/vagrant/my-app/vendor/laravel/framework/src/Illuminate/Validation/Validator.php:349
/home/vagrant/my-app/vendor/laravel/framework/src/Illuminate/Validation/Validator.php:395
/home/vagrant/my-app/vendor/laravel/framework/src/Illuminate/Validation/Validator.php:408
/home/vagrant/my-app/vendor/laravel/framework/src/Illuminate/Validation/Validator.php:451
/home/vagrant/my-app/vendor/laravel/framework/src/Illuminate/Validation/Validator.php:469
/home/vagrant/my-app/app/Actions/Fortify/UpdateUserPassword.php:29
/home/vagrant/my-app/vendor/laravel/fortify/src/Http/Controllers/PasswordController.php:21
...

Changing the line

if ( ! Hash::check($input['current_password'], $user->password)) {

to

if ( !isset( $input['current_password'] ) || ! Hash::check($input['current_password'], $user->password)) {

does fix the problem

driesvints commented 3 years ago

Make sure you pass that key.

scibuff commented 3 years ago

Make sure you pass that key.

What if someone makes a PUT request without it (e.g. with disabled front end validation)

driesvints commented 3 years ago

@scibuff good point. Can you send in a PR?

driesvints commented 3 years ago

I've sent in a fix for this here: https://github.com/laravel/fortify/pull/194