laravel / jetstream

Tailwind scaffolding for the Laravel framework.
https://jetstream.laravel.com
MIT License
3.97k stars 822 forks source link

Use custom ConfirmPassword hook for Inertia CurrentUserController@destroy password validation #831

Closed Gimcrack closed 3 years ago

Gimcrack commented 3 years ago

Background:

Problem:

I'm using a non-laravel user provider (Active Directory via LdapRecord package) and therefore need a custom hook for confirming passwords:

// AuthServiceProvider
public function boot() 
{
    $this->registerPolicies();

    Fortify::confirmPasswordsUsing(function($user, $password) {
        return Auth::validate([
            'mail' => $user->email,
            'password' => $password
        ]);
    });
}

This works great for confirming passwords, but when I try to delete an account the @destroy method on Interia's CurrentUserController does not use the custom hook to verify the password and the validation fails.

// Laravel\Jetstream\Http\Controllers\Inertia\CurrentUserController
public function destroy(Request $request, StatefulGuard $auth)
{
    // this does not use the custom hook confirmPasswordsUsing and the validation fails.
    $request->validate([
        'password' => 'required|string|password',
    ]);

   ...
}

I can get it to work if I modify the @destroy method. Code comments added for clarity.

// Laravel\Jetstream\Http\Controllers\Inertia\CurrentUserController
public function destroy(Request $request, StatefulGuard $auth)
{
    // check for presence of password in request, but don't check it yet
    $request->validate([
        'password' => 'required|string', 
    ]);

    // confirm password using ConfirmPassword action
    $confirmed = app(Laravel\Fortify\Actions\ConfirmPassword::class)(
        $auth, $request->user(), $request->input('password')
    );

   // throw validation exception on bad password
    if (! $confirmed)
        throw new ValidationException("The password is incorrect.");

    // continue to delete account ...
   ...
}

It seems like there should be some consistency to how passwords are confirmed, but I'm not sure if this is the best approach. Is there a way to hook into the request's password validation to make it use a different method? I'm happy to try my hand at a PR, but would probably need some assistance to make sure it follows the right conventions.

Thanks, -Jeremy

Gimcrack commented 3 years ago

I found another way to solve the issue that does not require editing framework code, but it still seems a little gross.

I can substitute my own validator by passing it in via the Validator@resolve method.

// AppServiceProvider
    Validator::resolver( function($translator, $data, $rules, $messages, $customAttributes) {
        return new App\Validation\Validator($translator, $data, $rules, $messages, $customAttributes);
    });

Then I can override the @validateCurrentPassword method on my custom validator class

// App\Validation\Validator
protected function validateCurrentPassword($attribute, $password, $parameters)
{
    Log::debug("Using custom validator");

    return app(ConfirmPassword::class)(
        Auth::guard(), Auth::user(), $password
    );
}
driesvints commented 3 years ago

Hi there,

Thanks for reporting but it looks like this is a question which can be asked on a support channel. Please only use this issue tracker for reporting bugs with the library itself. If you have a question on how to use functionality provided by this repo you can try one of the following channels:

However, this issue will not be locked and everyone is still free to discuss solutions to your problem!

Thanks.

Gimcrack commented 3 years ago

@driesvints Thanks for the reply. I posted the question to the Laracasts forum.