Search code examples
phplaravellamp

Resetting passwords out of the box on Laravel project with database provider: "user must implement canresetpassword interface"

Ok, it's my first Laravel project. First time developing anything on lamp also (I'm a windows .NET guy, so be gentle, plz).

I'm using Laravel 5.8 (limited by our hosting) and have created standard out of the box authentication with

php artisan make:auth

I'm using mySql as database for the app, created a users table resembling the fields of the standard User model and changed my users provider to this:

'users' => [
         'driver' => 'database',
         'table' => 'myDatabaseUserTableName',
     ],

So... to this point everything seems to work fine. I can register users, login and logout.

Curiously, when I try to execute the remember password option, I receive an UnexpectedValueException: User must implement CanResetPassword interface on PasswordBroker.php

As far as I reached on this topic (seems to have been an issue for lots of developers since early 5.x versions) the problem seems to come from broker's getUser method not using the User model class to retrieve the user, and seems to be related to the fact that I'm not using the Eloquent standard User provider (although I'm using the standard User model).

I didn't found any explained solution to this issue and I was wondering if anyone could help me with this before I started tickling with the framework code to see if I can make it work (or break it worse trying!)

EDIT: Ok, finally I've managed to fix the functionality. I don't feel totally comfortable with the solution, but it works and it's quite simple.

I'll post my solution as an answer, I'm totally open to criticism and to comments regarding how to improve it. Also I'd love to hear about better aproachs to this problem.

Solution

  • To get rid of the interface error I added the CanResetPasswordContract interface to GenericUser class, and use used Illuminate\Auth\Passwords\CanResetPassword class to implement the interface.

    So I've just added:

    use Illuminate\Auth\Passwords\CanResetPassword;
use Illuminate\Contracts\Auth\CanResetPassword as CanResetPasswordContract;

    at the beginning of laravel/framework/src/Illuminate/Auth/GenericUser.php, added CanResetPasswordContract to the implements of the class and called CanResetPassword class with:

    use canResetPassword;

    Inside the class code.

    So now, GenericUser, the class which is being used by PasswordBroker to retrieve the user on the password restoration, implements the interface and the error doesn't shows again...

    Of course, this is not enough to fix the whole functionality, as GenericUser is not able by itself to send emails. You'll need to allow it to be Notifiable, to do this you have to add 

    use Illuminate\Notifications\Notifiable;

    To the file, also use Notifiable inside the class.

    At this point, if you try again to ask for password reset, you'll receive an email with the url that allows for password resetting (suposing you have your email.php configuration correct, that is). But if you use it, you'll receive an error, as GenericUser is not able to persist changes to database.

    So you'll need a model able to save data to database.

    I've added Eloquent Model, although I'm using database provider, Eloquent model seems to work on user creation, so I've decided to try:

    use Illuminate\Database\Eloquent\Model;

    And ensure you add a variable with the name of your users table. Both these changes should be made into GenericUser class:

    protected $table = 'yourUserTableName';

    Also you'll need to indicate that your user model already exists (this is checked before sending the email, otherwise, the email is not sent). So I've added: 

      $user->exists = true;

    just before saving on resetPassword method of vendor/laravel/framework/src/Illuminate/Foundation/Auth/ResetsPasswords.php

    With all this modifications in place, the reset password functionality finally works.

    However, I feel that I'm doing the opposite that I should be doing... I'd like to tell the broker to use my User Model class, instead of changing GenericUser into something that resembles my User Model.