[5.4] Password reset tokens are now stored as bcrypt of sha256

[ctoma]

The change:

From 5.3 to 5.4 I noticed a breaking, undocumented change on my application. The password_resets table now stores tokens as bcrypt of a sha256 token.

The 5.3 DatabaseTokenRepository is here and the 5.4 DatabaseTokenRepository is here.

The major change is the addition of $hasher which is implemented as Illuminate\Hashing\BcryptHasher.

On line 83 we call

$token = $this->createNewToken();

which creates a random SHA256 token.

Then on line 85 we call $this->getPayload($email, $token)

Which leads to line 110:

        return ['email' => $email, 'token' => $this->hasher->make($token), 'created_at' => new Carbon];

The addition here of $this->hasher->make($token) now bcrypts the SHA256 token in the database.

So on 5.4 we generate a SHA256 token, but then we hash it again with bcrypt and store it in the database. Why? This is a breaking change for three reasons

• Any password resets generated before a 5.4 upgrade will no longer work
• Any custom implementations (such as mine) that check the token in the database will no longer work
• Any custom implementations that send an email using the token from the database will no longer work (due to bcrypt using slashes in their hashing algorithm)

Additionally this obfuscation could make it harder to debug in certain situations, say for example you have a user whose password reset link isn't working. How do you now go about investigating this given the log files will have a completely different url (SHA256) compared to what is in the database (bcrypt of the SHA256)?

This seems to be a completely unnecessary level of obfuscation and I can't understand why it was implemented. SHA256 itself is perfectly secure for a temporary reset password token. Instead we now have a SHA256 token which is bcrypted and then stored in the database.

Solution

I propose we go back to the way it was and keep it as a simple SHA256 token. There is no need for this second hashing and it makes extending Laravel's native password reset functionality more difficult for no benefit.

[themsaid]

Rolling back such change is not possible now as it'll break the token comparison in current apps and we can't introduce such breaking change.

The Password reset functionality along with many other ones shipped with laravel is added as a convenient way of doing things, you can customise it as you want in your application or use it as is, so I don't think it really matters how it works since you already customize it in your application :)

[ctoma]

@themsaid Fair enough, may I ask why it was implemented in the first place (out of curiosity)? I can't really think of a solid reason as to why we would double hash.

[themsaid]

Here's the original PR: #16850

Going to close this issue then since it's not actually a bug report, feel free to ping me though :)

[zackify]

Not sure why this didn't get added the the upgrade guide since this issue was created. A mention of checking for password reset tokens will fail now would be nice haha.

[EmilMoe]

I noticed this too, it broke our production application. Very bad choice to make this!

[zackify]

@EmilMoe I got around it by doing this:

use Illuminate\Contracts\Hashing\Hasher as HasherContract;
...
    public function __construct(HasherContract $hasher)
    {
        $this->hasher = $hasher;
    }
...
    //in controller method
    $reset = PasswordReset::where('email', $request->email)->firstOrFail();
     if(!$this->hasher->check($request->token, $reset->token)) //token is invalid

Using the hasher that the new stuff uses to validate if the token matches. Sucks, because I didn't send along the email address with the password resets but now I have to. Also due to lack of docs on customizing resets it took a bit to find out what was wrong...

At least it's all good now. Still shocked it hasn't been added to the upgrade guide....

[EmilMoe]

I am rewriting it to my own, I can't depend my production software on something that just changes like this. I think we were many who found the email from the token. I can't even see how this adds up to the security, it just seems like an irrelevant change.

[tokict]

can someone explain to me how am I to use this if I have an API? If I send user the link and he opens it from a device that did not made the request, I have no way of knowing users mail without asking him for it again. Why are you forcing me to use your idea of how auth should work by making me ask user for his mail yet again? Or is there a way to use sent token to search for mail in password_resets table?

[zackify]

Send the email address in your password reset link.

…

On Wed, Jun 14, 2017 at 05:11 Tino ***@***.***> wrote: can someone explain to me how am I to use this if I have an API? If I send user the link and he opens it from a device that did not made the request, I have no way of knowing users mail without asking him for it again. Why are you forcing me to use your idea of how auth should work by makin me ask user for his mail yet again? Or is there a way to user sent token to search for mail in password_resets table? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#18570 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAbacKaHXwjbFpM28ChDmE5XTUWpqnwJks5sD6PAgaJpZM4MuMmp> .

[devcircus]

The auth scaffold is there as a convenience with sensible defaults, all are made to be easily overwritten to match your use case.

[tokict]

I understand, I just think this particular decision was a bad one and will make a lot of people spend time fixing apps broken by an irrelevant change

[berkayk]

@zackify can you please explain the fix in a more detailed way. Our production app is in need of this fix. Thanks.

[zackify]

@berkayk you just need to pull in the hasher contract to check if the token emailed out matches the encrypted token stored in the table as I mentioned above. What more do you need help with? I’d love to help you out, just don’t know what else you need.

[berkayk]

@zackify I am currently sending the token in sendPasswordResetNotification method in User model. I don't have access to users email I guess.

public function sendPasswordResetNotification($token)
    {
        $this->notify(new CustomResetPassword($token));
    }

Then in my getReset method in PasswordController,

public function getReset(Request $request, $token = null)
	{
		$email = null;
		$user = null;
		if ($token != null){
		    return Hash::make($token);
			$email = DB::table("password_resets")->where('token', $token)->pluck('email');

			$user = User::where('email', '=', $email)->first();
		}

		return view('password.reset', ['token' => $token, 'email' => $email, 'user' => $user]);
	}

I am unsure where to use the hasher contract.

[zackify]

@berkayk you have to do exactly what I put above, inside your token check:

if ($token != null){
    $reset = DB::table("password_resets")->where('email', $request->email)->firstOrFail();
    
    if(!$this->hasher->check($request->token, $reset->token)) //throw error, token invalid
    
    $user = User::where('email', '=', $request->email)->first();
}

I send along the email address in a query parameter in my reset email template, this way the user doesn't have to type that in on the reset page.

[mfentonaxis41]

We ran into this with a client, make sure that your DB table "password_resets" field "token" type is acceptable of all the characters, ours was set to VARCHAR at 191 characters, we set to LONGTEXT and everything worked great. Maybe this will work for you, maybe not, but worth a try.

[oxycoder]

I make this gist, hope it works for you guys: https://gist.github.com/oxycoder/7fe6ac9708a0e1ec90c0f624d76fb8a1

[usamamashkoor]

Hi i dont have email i only have the token how can i then verify that token is valid and not expired i am using laravel 5.4.

[zackify]

@usamamashkoor you can’t. You need to pass along the email in the token url.

[usamamashkoor]

Thanks @zackify yes i did that and can you please tell me if this code is correct to verify if the token is valid

$reset = DB::table("password_resets")->where('email', $email)->first();
		    
   if(\Hash::check($token, $reset->token)){
	$error = false;
    }else{
	$error = true;
    } 

[zackify]

looks good to me! @usamamashkoor

[usamamashkoor]

Thanks @zackify

[algorhythm]

I wrote following php artisan tinker lines to update my password_resets table to make the old token valid again. Hope this helps someone.

$items = \DB::table('password_resets')->where('token', 'not like', '$%')->orderBy('created_at', 'desc')->get();
foreach ($items as $item) { \DB::table('password_resets')->where('login', $item->login)->update(['token' => bcrypt($item->token)]); }

For better readability here the same script formatted:

/* select all entries which token is not bcryted */
$items = \DB::table('password_resets')
    ->where('token', 'not like', '$%')
    /* start with the newest one because the possibility that they used their link is bigger */
    ->orderBy('created_at', 'desc')
    ->get();

/* update them */
foreach ($items as $item) {
    \DB::table('password_resets')
        ->where('login', $item->login)
        ->update([
            'token' => bcrypt($item->token)
        ]);
}

Note: I know, that script is very inefficient but it works correct and I needed a quick solutions, because my update goes live and I realized later that the tokens are invalid.

[coreation]

@EmilMoe I got around it by doing this:

use Illuminate\Contracts\Hashing\Hasher as HasherContract;
...
    public function __construct(HasherContract $hasher)
    {
        $this->hasher = $hasher;
    }
...
    //in controller method
    $reset = PasswordReset::where('email', $request->email)->firstOrFail();
     if(!$this->hasher->check($request->token, $reset->token)) //token is invalid

Using the hasher that the new stuff uses to validate if the token matches. Sucks, because I didn't send along the email address with the password resets but now I have to. Also due to lack of docs on customizing resets it took a bit to find out what was wrong...

At least it's all good now. Still shocked it hasn't been added to the upgrade guide....

Thanks a bunch!

[tatsatchudasama]

$passwordResetToken = DB::table("password_reset_tokens")->where('token', $token)->pluck('email');

    $email_get = $passwordResetToken[0];


    if ($request->email !== $email_get) {
        return redirect()->back()->withErrors(['email' => 'This email does not match the token provided.']);
    }

    $user = User::where('email', $email_get)->firstOrFail();

    $user->password = bcrypt($request->password);
    $user->save();

    $passwordResetToken->delete();