[11.x] Rehash user passwords when validating credentials

[valorin]

Summary

As was discovered after updating bcrypt rounds from 10 to 12 in laravel/laravel#6245 and #48494, user passwords are not currently being rehashed during login when the hashing configuration has changed. This should be considered a security risk as rehashing passwords is an important part of the authentication process. Rehashing passwords during login ensures updates to the hashing configuration, such as increasing rounds/cost, is applied to existing hashes when the plaintext password is available during login.

This PR implements rehashing directly within the Eloquent and Database User Providers, making it a part of the core framework authentication system. The rehashing will happen automatically when user credentials are validated by *UserProvider::validateCredentials(), which is called by the Auth::attempt() helper. This helper is used by both Breeze and Fortify/Jetstream to authenticate users, and is the recommended method in the Laravel docs. This ensures that with Laravel 11, any apps using the authentication system will be properly rehashing passwords.

I modelled the new DatabaseUserProvider::rehashUserPassword() method on the existing rehashUserPassword() method, to ensure it is compatible for apps that don't use Eloquent.

Breaking Changes

Since rehashing needs to update the database, it needs to know the name of the password attribute on the user model. There is not an existing way to retrieve this information, so I had to add the getAuthPasswordName() method to the Authenticatable contract. I've implemented it within the Authenticatable trait and GenericUser so most apps won't be impacted, but apps that don't use or extend these two, or have a different password attribute, will need implement this method.

This is the reason I'm targeting Laravel 11. I'd love to get it into 10, but I can't see a way without breaking some obscure configuration. It's not a critical security risk, so it doesn't require an immediate fix.

Upgrade Impacts

1. Rehashing passwords incurs a performance hit, and after the upgrade to 11, every user who logs in will trigger a password rehash. This is unlike to have an impact as logins are generally spread out and servers usually have spare resources. However apps with large numbers of concurrent logins and limited resources may notice increased resource usage.
2. It's not technically a breaking change, but when a password is rehashed it will force all other active sessions, remember-me tokens, and anything else that relies on the current password hash to be reset automatically. This may cause an increase in login attempts as users reauthenticate different devices.

Patching Laravel 10

Since I know it'll be a question asked many times, this change cannot be directly patched in Laravel 10 due to the BC break.

If you use Laravel Breeze, you can easily patch it to perform rehashing as it copies auth code to the user's app. This isn't directly possible within Jetstream/Fortify, as that code is within the package. There is no point patching these now when a better fix is coming in v11.

Patching Breeze Instructions

1. Open the \App\Http\Requests\Auth\LoginRequest file
2. Find the authenticate() method
3. Find the RateLimiter::clear($this->throttleKey()); and add the following:

$user = Auth::user();

if (Hash::needsRehash($user->password)) {
    $user->forceFill(['password' => Hash::make($this->input('password'))])->save();
}

The method should look something like this:

public function authenticate(): void
{
    $this->ensureIsNotRateLimited();

    if (! Auth::attempt($this->only('email', 'password'), $this->boolean('remember'))) {
        RateLimiter::hit($this->throttleKey());

        throw ValidationException::withMessages([
            'email' => trans('auth.failed'),
        ]);
    }

    RateLimiter::clear($this->throttleKey());

    $user = Auth::user();

    if (Hash::needsRehash($user->password)) {
        $user->forceFill(['password' => Hash::make($this->input('password'))])->save();
    }
}

[PerryvanderMeer]

Does this approach update updated_at with every successful login attempt? That might not be the desired behavior?

[valorin]

It'll only update when the password is rehashed, which will happen once.

[chrysanthos]

@valorin can we potentially use method_exists to check if getAuthPasswordName method exists on a model and do the rehashing only then? This way we would make this an opt-in feature for L10 and add it in the contract for L11.

[valorin]

Oooh, that's a great idea!

I'd like to check with the Laravel team to make sure it's a pattern they want, but I'd love to do it if we can. 😁

[claudiodekker]

@valorin can we potentially use method_exists to check if getAuthPasswordName method exists on a model and do the rehashing only then? This way we would make this an opt-in feature for L10 and add it in the contract for L11.

Yeah, that's probably a better approach as to not break implementations that rely on the current version of the Authenticatable contract, since they wouldn't have that new getAuthPasswordName method. 👍

Besides, I doubt anyone has a reason to change the field name from password pretty much ever anyway?

[valorin]

Converting this to draft as it depends on the status of #48673, which applies the rehashing to Laravel 10 in a backwards compatible way. If that PR is merged, then changes will be required to clean up and finish the upgrade.

[roycewilliams]

Naive question - where in the code are the criteria for checking whether an upgrade is needed? Is it keyed on any hash with bcrypt cost less than the current standard, or something else?

[valorin]

@roycewilliams

It first runs this to check if it needs to rehash:

if ($this->hasher->needsRehash($hash)) {
    $this->rehashUserPassword($user, $plain);
}

The rehash is triggered if the hashing configuration is changed, which is defined in the application configuration and needs to be manually changed in the app to be triggered. Any changes to that will trigger a rehash though, even if you go down. but it's not something you change randomly. So only apps that specifically change it will have rehashing triggered.

[deleugpn]

@claudiodekker

Besides, I doubt anyone has a reason to change the field name from password pretty much ever anyway?

Reason 1: A company starts a PHP project in 2006 before Composer even existed. There are millions of PHP code without automation tests. In 2017 the company starts using Laravel side-by-side with the original application. It's not a brand new database. You have to comply with a database that was modelled in 2006.

Reason 2: A government contract with local regulations requires the "storage layer which will hold citizen information MUST be defined using the country's local language"

Reason 3: A 2019 Laravel project that migrated from Bcrypt to Argon2id decided to create a new column, rehash upon login and keep both hashes in parallel during rollout until they were confident in dropoping the bcrypt hash, but now the project just runs with a password_v2 column 🤷

I could probably remember of another reason, but I think I've made my point 😅

[bigship-prashant]

I thing we need to give config key in auth.php for developers to enable rehashing or not

[trevorgehman]

Just adding my two cents: I think it's a no-brainer that this should be in the default Laravel authentication scaffolding. It's a very simple way to ensure security best-practices are being followed, which greatly benefits the Laravel ecosystem as a whole.

[driesvints]

@valorin guess we can move this out of draft now?

[valorin]

I've got some changes to make before it can go out of draft. Been a busy few weeks, but I'll try and get them done this week. 🤞

[valorin]

This is good to review and (hopefully) merge into 11.x. 🤞
I've made the following changes:

1. I moved the code that triggers the rehashing into SessionGuard::attempt() and out of validateCredentials() on the user provider. This feels like a better place for it, as rehashing is part of the login attempt, rather than the credential validation itself.
2. I've added a config key hashing.rehash_on_login which toggles the rehashing, so it can be disabled if needed. This may be useful for custom auth scaffolding. I've PR'ed the config key: Add rehash_on_login to config toggle rehashing on login laravel#6282

I've tested this change with both Breeze and Jetstream and they both successfully rehashed the password on login.

Something interesting I noticed is SessionGuard does include a different rehashUserPassword() method: https://github.com/laravel/framework/blob/master/src/Illuminate/Auth/SessionGuard.php#L682-L700
However this appears to only be used for Jetstream (as part of logging out other devices), and assumes the EloquentUserProvider is being used, so it wouldn't be useful in this case. My implementation is supposed to work across all User Providers and password attributes.

[Shkeats]

need to recheck the return type docblocks on the three concrete versions of this method, the contract one is correct.

[valorin]

Nice catch! Fixed. 👍

[taylorotwell]

@valorin what is the behavior of this when the AuthenticateSession middleware is enabled? I assume the user is still able to stay logged in if they login and their password is immediately rehashed? Are other sessions logged out?

[valorin]

@taylorotwell Yep, just confirmed with Breeze. The password is rehashed on login and the user is successfully logged in, while other sessions are ended.

There is no way to avoid the other sessions being ended, but it makes sense that it's triggered by a login rather than just randomly happening, and the user should hopefully associate the two events and not be concerned. It'll also only happen once per user, so won't be a continuous issue.

[akulmehta]

@valorin would this also work with older apps using Laravel UI instead of Breeze?

[valorin]

Yep, it should work after upgrading to 11. 👍

From what I can see in the code, UI uses Auth::guard()->attempt(), which calls \Illuminate\Auth\SessionGuard::attempt() (when you're using a session guard), and that includes the rehashing code.