laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.55k stars 11.03k forks source link

Hasher::check on DatabaseTokenRepository failing on Test Environment #29188

Closed TiagoSilvaPereira closed 5 years ago

TiagoSilvaPereira commented 5 years ago

Hi,

Recently, I created a reset password feature for an API using the Laravel ResetsPasswords and SendsPasswordResetEmails traits. It worked very well on the Development Environment, but the tests to reset the password are failing. Investigating, I noticed that what is failing is this method inside DatabaseTokenRepository:

public function exists(CanResetPasswordContract $user, $token)
    {
        $record = (array) $this->getTable()->where(
            'email', $user->getEmailForPasswordReset()
        )->first();

        return $record &&
               ! $this->tokenExpired($record['created_at']) &&
                 $this->hasher->check($token, $record['token']);
    }

Most specifically, this line is returning false on Test enviroment:

$this->hasher->check($token, $record['token']);

I printed both $token and $record['token'] before the return method and it shows:

image

So both tokens are identical. Is this the correct behavior? Am I doing something wrong?

Thanks :smile: !


This is the test I'm doing:

public function testItResetsThePassword()
    {
        $user = $this->signInSubscribed();

        $this->postJson(route('api2.accounts.reset'), [
            'email' => $user->email
        ]);

        // Here we have the token
        $token = DB::table('password_resets')
            ->whereEmail($user->email)
            ->first()
            ->token;

        $response = $this->postJson(route('api2.accounts.new-password'), [
            'email' => $user->email,
            'password' => 'password123',
            'password_confirmation' => 'password123',
            'token' => $token
        ]);

        // Fails here with 400 cause is going to my custom sendResetFailedResponse method
        $response->assertStatus(200);

        // ...

    }

And this is the Trait I created to reset the password (I'm using on my AccountController):

trait RefreshablePassword {

    use ResetsPasswords;
    use SendsPasswordResetEmails;

    /**
     * Get the password reset credentials from the request.
     *
     * @param  \Illuminate\Http\Request  $request
     * @return array
     */
    protected function credentials(Request $request)
    {
        return $request->only(
            'email', 'password', 'password_confirmation', 'token'
        );
    }

    /**
     * Get the broker to be used during password reset.
     *
     * @return \Illuminate\Contracts\Auth\PasswordBroker
     */
    public function broker()
    {
        return Password::broker();
    }

    /**
     * Send password reset link. 
     */
    public function sendPasswordResetLink(Request $request)
    {
        return $this->sendResetLinkEmail($request);
    }

    /**
     * Get the response for a successful password reset link.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  string  $response
     * @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\JsonResponse
     */
    protected function sendResetLinkResponse(Request $request, $response)
    {
        return response()->json(true, 204);
    }
    /**
     * Get the response for a failed password reset link.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  string  $response
     * @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\JsonResponse
     */
    protected function sendResetLinkFailedResponse(Request $request, $response)
    {
        return response()->json(['message' => 'Email could not be sent to this email address.'], 400);
    }

    /**
     * Override the mail body for reset password notification mail.
     */
    public function sendPasswordResetNotification($token)
    {
        $this->notify(new ResetPasswordMail($token));
    }

    /**
     * Handle reset password 
     */
    public function callResetPassword(Request $request)
    {
        return $this->reset($request);
    }

    /**
     * Reset the given user's password.
     *
     * @param  \Illuminate\Contracts\Auth\CanResetPassword  $user
     * @param  string  $password
     * @return void
     */
    protected function resetPassword($user, $password)
    {
        $user->password = Hash::make($password);
        $user->save();
        event(new PasswordReset($user));
    }

    /**
     * Get the response for a successful password reset.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  string  $response
     * @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\JsonResponse
     */
    protected function sendResetResponse(Request $request, $response)
    {
        return response()->json(true, 204);
    }
    /**
     * Get the response for a failed password reset.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  string  $response
     * @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\JsonResponse
     */
    protected function sendResetFailedResponse(Request $request, $response)
    {
        return response()->json(['message' => 'Failed, Invalid Token.'], 400);
    }

}

And the routes:

Route::post('reset-password', 'Api2\AccountController@sendPasswordResetLink')->name('api2.accounts.reset');
    Route::post('new-password', 'Api2\AccountController@callResetPassword')->name('api2.accounts.new-password');
driesvints commented 5 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.

TiagoSilvaPereira commented 5 years ago

Hi, @driesvints thanks for responding :smile: ... I think the problem is occurring inside the library because I noticed that what is failing is the exits() method inside DatabaseTokenRepository. It fails only on Test Env.

Can you reconsider opening this issue?

Thanks

TiagoSilvaPereira commented 5 years ago

Hi @driesvints can you reconsider opening the issue? It seems like the problem occurs inside the library (I debugged the code and the problem seems to happen on DatabaseTokenRepository).

thanks

driesvints commented 5 years ago

@TiagoSilvaPereira you're using the check method wrong. The first argument should be your unhashed password. Please try a support channel first next time. Thanks!

TiagoSilvaPereira commented 5 years ago

Hi @driesvints thanks for responding. I'm not using the check() method, I'm using the native ResetsPasswords and SendsPasswordResetEmails traits only. I think the issue is inside DatabaseTokenRepository that is internal Laravel code. I debugged and noticed that it is happening inside this code.

Please note that this code is inside Illuminate\Auth\Passwords\DatabaseTokenRepository and the issue happens only on Test Environment:

public function exists(CanResetPasswordContract $user, $token)
{
        $record = (array) $this->getTable()->where(
            'email', $user->getEmailForPasswordReset()
        )->first();

        return $record &&
               ! $this->tokenExpired($record['created_at']) &&
                 $this->hasher->check($token, $record['token']);
}

Thanks

taylorotwell commented 5 years ago

There is no way the Laravel code is incorrect here. Otherwise, Forge, Envoyer, Vapor, etc. would all have a broken password reset?

driesvints commented 5 years ago

Hey @TiagoSilvaPereira. I was wrong myself at first. The first argument is your random generated token for your reset password record. The same token is hashed first before it's stored in the database record.

TiagoSilvaPereira commented 5 years ago

@taylorotwell, @driesvints thanks for responding... It is happening only on Test Environment, as explained above (Works well on production). May be an issue with my test env?

Thanks

driesvints commented 5 years ago

@TiagoSilvaPereira

// Here we have the token
        $token = DB::table('password_resets')
            ->whereEmail($user->email)
            ->first()
            ->token;

You're just retrieving the hashed token from the DB here which won't work. You need the token before it was hashed.

TiagoSilvaPereira commented 5 years ago

@driesvints Thank you very much, I'll take a look at it. Sorry for bothering you. Best Regards